Skip to content

fix: asset list showing fiat symbol instead of native#22760

Merged
bergeron merged 1 commit intodevelopfrom
brian/asset-list-usd-instead-of-eth
Feb 13, 2024
Merged

fix: asset list showing fiat symbol instead of native#22760
bergeron merged 1 commit intodevelopfrom
brian/asset-list-usd-instead-of-eth

Conversation

@bergeron
Copy link
Copy Markdown
Contributor

@bergeron bergeron commented Jan 31, 2024

Description

Problem: With Primary currency = Fiat, the home page showed Ethereum's symbol as USD instead of ETH.

Related issues

Manual testing steps

  • Set primary currency in settings to Fiat
  • Ethereum's symbol should be "ETH" on the home page

Screenshots/Recordings

Before

Screen.Recording.2024-01-31.at.12.02.59.PM.mov

After

Screen.Recording.2024-01-31.at.12.01.01.PM.mov

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • 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.
  • 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.

@bergeron bergeron requested a review from a team as a code owner January 31, 2024 22:33
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 31, 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.

@bergeron bergeron added team-extension-ux needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. labels Jan 31, 2024
@darkwing darkwing requested a review from NidhiKJha February 7, 2024 16:51
@darkwing
Copy link
Copy Markdown
Contributor

darkwing commented Feb 9, 2024

Do you know if this is a recent regression? I'm not seeing this in production. Flagging @NidhiKJha for review.

@bergeron
Copy link
Copy Markdown
Contributor Author

bergeron commented Feb 9, 2024 •
edited
Loading

Do you know if this is a recent regression? I'm not seeing this in production. Flagging @NidhiKJha for review.

Correct. This occurs on develop, not the current prod release. This appears to be the change that exposed the issue: #22601. But I think bug was there before, we just weren't showing symbol until that PR.

develop w/bug:
image

prod:
image

@darkwing darkwing requested a review from vthomas13 February 13, 2024 16:44
@bergeron bergeron merged commit 3844eac into develop Feb 13, 2024
@bergeron bergeron deleted the brian/asset-list-usd-instead-of-eth branch February 13, 2024 17:51
@github-actions github-actions bot locked and limited conversation to collaborators Feb 13, 2024
@metamaskbot metamaskbot added the release-11.12.0 Issue or pull request that will be included in release 11.12.0 label Feb 13, 2024
@gauthierpetetin
Copy link
Copy Markdown
Contributor

gauthierpetetin commented Mar 14, 2024 •
edited
Loading

Hello @bergeron , we just had a look at this during bug triage session and noticed there's still ETH displayed on the "After" video posted in this PR's description.

Screenshot 2024-03-14 at 12 29 59

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@NidhiKJha NidhiKJha NidhiKJha approved these changes

+1 more reviewer

@vthomas13 vthomas13 vthomas13 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-11.12.0 Issue or pull request that will be included in release 11.12.0 team-assets

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@bergeron @darkwing @gauthierpetetin @vthomas13 @NidhiKJha @metamaskbot