Skip to content

feat: reduce the number of decimals shown for simulation detail amounts#24036

Merged
dbrans merged 8 commits intodevelopfrom
dbrans/amounts
Apr 23, 2024
Merged

feat: reduce the number of decimals shown for simulation detail amounts#24036
dbrans merged 8 commits intodevelopfrom
dbrans/amounts

Conversation

@dbrans
Copy link
Copy Markdown
Contributor

@dbrans dbrans commented Apr 15, 2024 •
edited
Loading

Description

This PR does two things:

  • Converts balance change amounts to BigNumber. The previous Amount definition was redundant.
  • Formats token amounts as follows:

|amount| < 1: We display a maximum of 3 significant digits.

|amount| >= 1: We display all digits left of the decimal point. We also display some decimal places for smaller amounts, and gradually reduce the decimal precision as the amount gets bigger until we only show whole numbers.

Users are still able to see the original amount, subject to the maximum precision supported by Intl.NumberFormat.

Here is how different amounts are rendered:

Screenshot 2024-04-08 at 14 14 36

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2343

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 15, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@dbrans dbrans added the team-confirmations-system-deprecated DEPRECATED: please use "team-confirmations" instead label Apr 15, 2024
@dbrans dbrans marked this pull request as ready for review April 16, 2024 12:47
@dbrans dbrans requested review from a team as code owners April 16, 2024 12:47
@sleepytanya
Copy link
Copy Markdown
Contributor

sleepytanya commented Apr 16, 2024

Works great!

Screenshot 2024-04-16 at 16 54 51 Screenshot 2024-04-16 at 16 56 37 Screenshot 2024-04-16 at 16 58 11

sleepytanya
sleepytanya previously approved these changes Apr 16, 2024
View reviewed changes
matthewwalsh0
matthewwalsh0 reviewed Apr 16, 2024
View reviewed changes
@dbrans dbrans requested review from a team as code owners April 16, 2024 23:09
@dbrans dbrans changed the base branch from develop to dbrans/locale-fix April 16, 2024 23:14
@dbrans dbrans changed the base branch from dbrans/locale-fix to develop April 17, 2024 00:06
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 17, 2024

Builds ready [93843bd]
Page Load Metrics (1489 ± 761 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint583881397335
domContentLoaded9119372914
load46397214891585761
domInteractive9119372914
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1.26 KiB (0.02%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 17, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.48%. Comparing base (358f0bb) to head (85924bc).
Report is 14 commits behind head on develop.

❗ Current head 85924bc differs from pull request most recent head 1086855. Consider uploading reports for the commit 1086855 to get more accurate results

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #24036      +/-   ##
===========================================
+ Coverage    67.47%   67.48%   +0.01%     
===========================================
  Files         1257     1259       +2     
  Lines        49227    49239      +12     
  Branches     12822    12826       +4     
===========================================
+ Hits         33215    33227      +12     
  Misses       16012    16012

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 17, 2024

Builds ready [ee0d144]
Page Load Metrics (1098 ± 646 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7352516210752
domContentLoaded12150423316
load60318810981345646
domInteractive12150423316
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1.26 KiB (0.02%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 22, 2024

Builds ready [1086855]
Page Load Metrics (1586 ± 738 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint793791628239
domContentLoaded1289352311
load56385015861537738
domInteractive1289352311
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1.26 KiB (0.02%)
  • common: 0 Bytes (0.00%)

matthewwalsh0
matthewwalsh0 previously approved these changes Apr 23, 2024
View reviewed changes
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 23, 2024

Builds ready [85924bc]
Page Load Metrics (969 ± 592 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint621741003015
domContentLoaded105119105
load5029769691232592
domInteractive95119105
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1.36 KiB (0.02%)
  • common: 0 Bytes (0.00%)

@sleepytanya
Copy link
Copy Markdown
Contributor

sleepytanya commented Apr 23, 2024

Just re-testing - works beautifully:
Screenshot 2024-04-23 at 11 24 09
Screenshot 2024-04-23 at 11 24 02

@dbrans dbrans merged commit c33c5df into develop Apr 23, 2024
@dbrans dbrans deleted the dbrans/amounts branch April 23, 2024 17:24
@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 2024
@metamaskbot metamaskbot added the release-11.16.0 Issue or pull request that will be included in release 11.16.0 label Apr 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@matthewwalsh0 matthewwalsh0 matthewwalsh0 approved these changes

@sleepytanya sleepytanya sleepytanya approved these changes

Assignees

No one assigned

Labels

QA Passed release-11.16.0 Issue or pull request that will be included in release 11.16.0 team-confirmations-system-deprecated DEPRECATED: please use "team-confirmations" instead

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dbrans @sleepytanya @metamaskbot @matthewwalsh0