Skip to content
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

fix: formatter now rounds numbers down and displays only 1 decimal #232

Merged
merged 4 commits into from
Sep 5, 2023

Conversation

DiogoSoaress
Copy link
Member

@DiogoSoaress DiogoSoaress commented Aug 22, 2023

What it solves

The displayed Safe stats should never be rounded up.

How this PR fixes it

  • Removes decimal part if number has more than 1 integer after the thousands order of magnitude formatting
  • Truncates decimal part to one digit
  • Use en-US locale for number formatting

@github-actions
Copy link

github-actions bot commented Aug 22, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@github-actions
Copy link

github-actions bot commented Aug 22, 2023

Branch preview

✅ Deployed to dev:

https://safe-web-landing.dev.5afe.dev

@iamacook
Copy link
Member

You might be able to leverage the roundingMode option here.

@DiogoSoaress
Copy link
Member Author

I found that roundingMode is not supported yet in TypeScript microsoft/TypeScript#43336 (comment).

I've also checked the available libs in https://github.com/microsoft/TypeScript/tree/main/src/lib and the updates of Intl do not include NumberFormat so this option is unusable at the moment.

@DiogoSoaress DiogoSoaress marked this pull request as ready for review September 5, 2023 12:33
@@ -33,7 +33,7 @@ export const fetchTotalSafesDeployed = async (): Promise<number | null> => {
}

export const useSafeStats = (): Array<string | null> => {
const { totalAssets, totalSafesDeployed, totalTransactions } = useContext(SafeStatsContext)
const { totalTransactions, totalAssets, totalSafesDeployed } = useContext(SafeStatsContext)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the variables order to keep consistency with the following code.

@DiogoSoaress DiogoSoaress merged commit 59ec552 into main Sep 5, 2023
5 checks passed
@DiogoSoaress DiogoSoaress deleted the round-stats-down branch September 5, 2023 13:22
@github-actions github-actions bot locked and limited conversation to collaborators Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants