-
Notifications
You must be signed in to change notification settings - Fork 102
feat: Add validators fee voting tab & reorganize Network tab #1098
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Conversation
@@ -509,6 +509,17 @@ const getServerInfo = (rippledSocket) => | |||
return resp | |||
}) | |||
|
|||
const getServerState = (rippledSocket) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - might be worth it to add comments between this and getServerInfo, at a glance it is hard to tell what each is for. I don't feel strongly about this though so either way
base_fee?: number | ||
reserve_base?: number | ||
reserve_inc?: number |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you make this a union of objects that includes feesettings? seems like all 3 would always be included together (you wouldn't just have base fee or reserve fee for example) since its all part of the same object below
@@ -22,6 +22,7 @@ export const FETCH_INTERVAL_VHS_MILLIS = 60 * 1000 // 1 minute | |||
export const FETCH_INTERVAL_NODES_MILLIS = 60000 | |||
export const FETCH_INTERVAL_ERROR_MILLIS = 300 | |||
export const FETCH_INTERVAL_XRP_USD_ORACLE_MILLIS = 60 * 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export const FETCH_INTERVAL_XRP_USD_ORACLE_MILLIS = 60 * 1000 | |
export const FETCH_INTERVAL_XRP_USD_ORACLE_MILLIS = 60 * 1000 // 1 minute |
import { useLanguage } from '../shared/hooks' | ||
import { renderXRP } from '../shared/utils' | ||
|
||
const DROPS_TO_XRP_FACTOR = 1000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: is this not a const somewhere global for the repo? seems like it would be used elsewhere
const renderDomain = (domain) => domain && <DomainLink domain={domain} /> | ||
|
||
const renderAgreement = (className, d) => | ||
d ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we rename variable d to a more interpretive param name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only nitpicks on my end, looks good overall though
function fetchFeeSettingsData() { | ||
if (tab === 'voting') { | ||
getServerState(rippledSocket) | ||
.then((res) => res.state) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiny nit: since line 86 isnt an async operation, can we just extract in the following then clause instead of separately here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks for this
High Level Overview of Change
This PR is trying to achieve the following:
Resolve: #1058
Type of Change
TypeScript/Hooks Update
Before / After
Validators page (Uptime):

Validators page (Voting):

Voting Mobile View:
Upgrade Status:
