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

UX: Multichain: Fix useAccountTotalFiatBalance when getWeiHexFromDecimalValue returns "NaN" #21359

Merged
merged 3 commits into from
Oct 13, 2023

Conversation

darkwing
Copy link
Contributor

@darkwing darkwing commented Oct 12, 2023

Description

While onboarding with MULTICHAIN environment variable turned on, I noticed a crash upon initial loading screen. The cause was getWeiHexFromDecimalValue returning "NaN" in this block:

const totalWeiBalance = getWeiHexFromDecimalValue({
    value: totalFiatBalance,
    fromCurrency: currentCurrency,
    conversionRate,
    invertConversionRate: true,
  });

with params:

  • value: "0"
  • fromCurrency: "usd"
  • conversionRate: 0

Checking for valid output and returning 0x0 prevents the balance error

Manual testing steps

  1. MULTICHAIN=1 yarn start
    _1. Remove your local Metamask
    _2. Re-install and go through onboarding
    _3. Get to home wallet screen without crashing

Related issues

Fixes #???

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained:
    • What problem this PR is solving.
    • How this problem was solved.
    • How reviewers can test my changes.
  • I’ve indicated what issue this PR is linked to: Fixes #???
  • I’ve included tests if applicable.
  • I’ve documented any added code.
  • I’ve applied the right labels on the PR (see labeling guidelines).
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

@darkwing darkwing requested a review from a team as a code owner October 12, 2023 20:47
@github-actions
Copy link
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.

@darkwing darkwing added team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead needs-ux-ds-review labels Oct 12, 2023
@darkwing darkwing force-pushed the getWeiHexFromDecimalValue-error branch from 681be5d to 8d4ec74 Compare October 12, 2023 21:03
@darkwing darkwing force-pushed the getWeiHexFromDecimalValue-error branch from 8d4ec74 to f91683a Compare October 12, 2023 21:15
@darkwing darkwing changed the title UX: Multichain: Fix useAccountTotalFiatBalance when getWeiHexFromDecimalValue returns undefined UX: Multichain: Fix useAccountTotalFiatBalance when getWeiHexFromDecimalValue returns "NaN" Oct 12, 2023
@metamaskbot
Copy link
Collaborator

Builds ready [4cf4d2b]
Page Load Metrics (1234 ± 393 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint851931182512
domContentLoaded701941112914
load8220571234819393
domInteractive701941112914
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 33 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (8235fa2) 68.59% compared to head (4cf4d2b) 68.59%.
Report is 7 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #21359      +/-   ##
===========================================
- Coverage    68.59%   68.59%   -0.00%     
===========================================
  Files         1026     1026              
  Lines        40979    40981       +2     
  Branches     10936    10937       +1     
===========================================
+ Hits         28108    28109       +1     
- Misses       12871    12872       +1     
Files Coverage Δ
ui/hooks/useAccountTotalFiatBalance.js 92.31% <66.67%> (-3.53%) ⬇️

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

@darkwing darkwing merged commit 6e82ec2 into develop Oct 13, 2023
9 checks passed
@darkwing darkwing deleted the getWeiHexFromDecimalValue-error branch October 13, 2023 14:59
@github-actions github-actions bot locked and limited conversation to collaborators Oct 13, 2023
@metamaskbot metamaskbot added the release-11.5.0 Issue or pull request that will be included in release 11.5.0 label Oct 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-11.5.0 Issue or pull request that will be included in release 11.5.0 team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants