feat: add a unified formatting standard#545
Open
Roaring30s wants to merge 10 commits intolivepeer:mainfrom
Open
feat: add a unified formatting standard#545Roaring30s wants to merge 10 commits intolivepeer:mainfrom
Roaring30s wants to merge 10 commits intolivepeer:mainfrom
Conversation
|
@Roaring30s is attempting to deploy a commit to the Livepeer Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Author
|
@ECWireless Out of all the PRs I have, I recommend checking this one first since this touches everywhere and the likelihood of annoying conflicts emerging throughout time are very high here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR standardizes the number and currency formatting across the Explorer, specifically targeting the Root, Treasury, Orchestrator, Account and Voting pages. It replaces ad-hoc manual formatting (e.g., hardcoded division
/ 10000, manual" LPT"string concatenation) with our centralized utility functions in@utils/numberFormatters.tsType of Change
Related Issue(s)
Fixes: #442
Changes Made
🏛 Treasury & Proposals
formatWeightwhich relies on formatLPT.🗳 Voting
🎻 Orchestrators
👤 Accounts
We have established the following "Big Rules" for how numbers are displayed:
1. LPT Amounts (formatLPT)
15K LPT,2.5M LPT) by default, unless specified otherwise (e.g., in detailed tables).< 0.01show as< 0.01 LPT.< 0.0001show as< 0.0001 LPT.0 LPT(never0.00 LPT).2. Voting Power (formatVotingPower)
12.50K LPT).3. Percentages (formatPercent)
12.34%).50%instead of50.00%) for cleaner UI strings, unless a fixed precision is forced.10000or1000000) with named constantsPERCENTAGE_PRECISION_TEN_THOUSANDandPERCENTAGE_PRECISION_MILLIONfrom@utils/web3.4. ETH Amounts (formatETH)
< 0.0001are explicitly shown as< 0.0001 ETH.1,234.5678 ETHinstead of1.23K ETH), as precise ETH values are often critical.5. USD Currency (formatUSD)
$prefix.$1,234.56).Testing
How to test (optional unless test is not trivial)
Youre going to need to make a coffee and sit down and just inspect all the numbers on the pages to ensure formatting meets the standard.
Impact / Risk
Risk level: Low
Impacted areas: UI
User impact: Better number visual
Rollback plan: PR revert
Screenshots / Recordings (if applicable)
None
Additional Notes
Warning to Reviewer
I don't have the livepeer com key so I couldn't visually test the charts found on root page. If you could eyeball that part thoroughly, that should give us 100% coverage.
I am also slowly ridding of excessive useMemo's that may get in the way of my PR's throughout the project. We are currently using them to memoize simple arithmetic. The overhead introduced by the memos are heavier than re-rendering simple arithmetic unless it causing a render cascade elsewhere which I am checking for.