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: simulation Fiat precision and Fiat flickers different value before decimals are applied #13371

Merged
merged 19 commits into from
Feb 13, 2025

Conversation

digiwand
Copy link
Contributor

@digiwand digiwand commented Feb 6, 2025

Description

  • Fix fiat precision. We should show expected digits instead of 0s.
    • Replaces areas where value is coerced to number
  • Fix related FiatDisplay test
    • invalid test where we called toBeDefined instead of toBeTruthy from queryByText
    • removed useFiatFormatter mock
    • fix invalid showFiatInTestnets mock
    • fix hideFiatForTestnet test related mocks since in useHideFiatForTestnet
      TEST_NETWORK_IDS.includes(chainId) && !showFiatInTestnets; was returning true in all cases as chainId was undefined.
  • Update FiatDisplay value to replace $100.00 → $100 and $100.2 → $100.20. useFiatFormatter formatted as expected.
  • Fixes fiat flickering issue while data is still being fetched - adds placeholder element while token details are pending

Related issues

Fixes: #13387

Manual testing steps

  1. Go to https://develop.d3bkcslj57l47p.amplifyapp.com/
  2. Trigger Permit Batch

Screenshots/Recordings

Before

After example

**Real after with hidden value if "Unlimited" is shown w/ some token icons still loading **

**Before flickering issue **

CleanShot.2025-02-13.at.00.22.48.mp4

After

CleanShot.2025-02-13.at.00.30.17.mp4

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.

Copy link
Contributor

github-actions bot commented Feb 6, 2025

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.

@metamaskbot metamaskbot added the team-confirmations Push issues to confirmations team label Feb 6, 2025
@digiwand digiwand added the Run Smoke E2E Triggers smoke e2e on Bitrise label Feb 6, 2025
Copy link
Contributor

github-actions bot commented Feb 6, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 66dc61e
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9f4ba873-b989-4b85-a043-3438d42855a8

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@digiwand digiwand marked this pull request as ready for review February 6, 2025 21:31
@digiwand digiwand requested review from a team as code owners February 6, 2025 21:31
fix: related tests - mocking useSelector prevented chain Id from loading causing hideFiatForTestnet to be true
Copy link
Member

@OGPoyraz OGPoyraz left a comment

Choose a reason for hiding this comment

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

Forgot to request changes because of this
#13371 (comment)

Apart from that LGTM

@digiwand digiwand requested a review from OGPoyraz February 12, 2025 23:37
@digiwand
Copy link
Contributor Author

thanks @OGPoyraz! Great catch! updated 1ab354b

I also found that there was a flickering issue where while data is loaded, we could show e.g. $25,000,559.10 before $25. This has been updated 1ca9ca4

@digiwand digiwand changed the title fix: simulation Fiat precision fix: simulation Fiat precision and Fiat flickers different value before decimals are applied Feb 12, 2025
@digiwand digiwand enabled auto-merge February 13, 2025 13:10
@digiwand digiwand added this pull request to the merge queue Feb 13, 2025
Merged via the queue into main with commit 9911443 Feb 13, 2025
44 of 45 checks passed
@digiwand digiwand deleted the fix-fiat-precision branch February 13, 2025 14:36
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2025
@metamaskbot metamaskbot added the release-7.41.0 Issue or pull request that will be included in release 7.41.0 label Feb 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-7.41.0 Issue or pull request that will be included in release 7.41.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-confirmations Push issues to confirmations team
Projects
None yet
3 participants