Skip to content

fix: Reduce usage of scientific notation#27992

Merged
pedronfigueiredo merged 2 commits intodevelopfrom
pnf/27971
Oct 28, 2024
Merged

fix: Reduce usage of scientific notation#27992
pedronfigueiredo merged 2 commits intodevelopfrom
pnf/27971

Conversation

@pedronfigueiredo
Copy link
Copy Markdown
Contributor

@pedronfigueiredo pedronfigueiredo commented Oct 21, 2024

Description

Implements the decimals rounding strategy outlined in this document.

When the transfer value is rounded, a tooltip is shown with the full value without rounding. If the number has up to 18 decimal places, it is shown in a user readable format, without rounding. If the number has more than 18 decimal places, the tooltip defaults to showing the scientific notation format. This scenario is unlikely, and only possible through dApp API initiated transfers.

Includes unit tests.

Fixes: #27971

Open in GitHub Codespaces

Related issues

Fixes: #27971

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Screenshot 2024-10-21 at 17 27 13 Screenshot 2024-10-21 at 18 19 08

Pre-merge author checklist

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.

@pedronfigueiredo pedronfigueiredo added the team-confirmations Push issues to confirmations team label Oct 21, 2024
@pedronfigueiredo pedronfigueiredo self-assigned this Oct 21, 2024
@pedronfigueiredo pedronfigueiredo requested a review from a team as a code owner October 21, 2024 17:24
@github-actions
Copy link
Copy Markdown
Contributor

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.

@pedronfigueiredo pedronfigueiredo force-pushed the pnf/27971 branch 2 times, most recently from 6464543 to 2394961 Compare October 22, 2024 13:04
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [2394961]
Page Load Metrics (1866 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16422177187113062
domContentLoaded16302161184112259
load16472178186612560
domInteractive16195503818
backgroundConnect879302010
firstReactRender522651245124
getState569212211
initialActions01000
loadScripts11851660135311455
setupStore118821199
uiStartup18442409213614469
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1010 Bytes (0.01%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [8dd6ae4]
Page Load Metrics (1887 ± 100 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29323351809406195
domContentLoaded15742241185320699
load158322891887209100
domInteractive16107502210
backgroundConnect9101402813
firstReactRender45198953919
getState5100192713
initialActions01000
loadScripts11631763138517283
setupStore1189372713
uiStartup173326262131273131

@pedronfigueiredo pedronfigueiredo force-pushed the pnf/27971 branch 3 times, most recently from 781e0d3 to f42b721 Compare October 24, 2024 13:12

// keep in scientific notation
return num.toString();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will be nice to extract this to a util.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I extracted the logic into a contained function but since this function is only used on this hook I think it makes sense for it to live on the same file for now.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [c805139]
Page Load Metrics (2123 ± 112 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint32926462029454218
domContentLoaded170625512064208100
load176727312123234112
domInteractive17103482211
backgroundConnect13155553617
firstReactRender641621152411
getState6123263115
initialActions01000
loadScripts12221820151714771
setupStore1373342210
uiStartup197030582367285137
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 1.52 KiB (0.02%)
  • common: 0 Bytes (0.00%)

@pedronfigueiredo pedronfigueiredo added this pull request to the merge queue Oct 28, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Oct 28, 2024
@pedronfigueiredo pedronfigueiredo added this pull request to the merge queue Oct 28, 2024
Merged via the queue into develop with commit 46532ad Oct 28, 2024
@pedronfigueiredo pedronfigueiredo deleted the pnf/27971 branch October 28, 2024 15:52
@github-actions github-actions bot locked and limited conversation to collaborators Oct 28, 2024
@metamaskbot metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-12.7.0 Issue or pull request that will be included in release 12.7.0 team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Scientific notation show in redesigned erc20 transfer

4 participants