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

Adding ChainID Currency Symbol Map #22292

Merged
merged 5 commits into from
Dec 18, 2023
Merged

Adding ChainID Currency Symbol Map #22292

merged 5 commits into from
Dec 18, 2023

Conversation

vthomas13
Copy link
Contributor

Description

We want a ChainID: Ticker Map for future validation on some of our more commonly used networks. This PR adds that mapping constant, changes Optimism's "OP" ticker to "ETH" and fixes a resulting unit test failure.

Related issues

Fixes: #1708

Manual testing steps

Screenshots/Recordings

Before

After

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.

@vthomas13 vthomas13 requested a review from a team as a code owner December 14, 2023 18:03
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.

@vthomas13 vthomas13 force-pushed the 1708-chainid-symbol-map branch from 635a59b to efe78af Compare December 14, 2023 18:03
@vthomas13 vthomas13 added team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead needs-ux-ds-review labels Dec 14, 2023
@vthomas13 vthomas13 marked this pull request as draft December 14, 2023 18:34
Copy link

codecov bot commented Dec 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3f113bd) 67.94% compared to head (181f89c) 67.96%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #22292      +/-   ##
===========================================
+ Coverage    67.94%   67.96%   +0.01%     
===========================================
  Files         1067     1067              
  Lines        41290    41293       +3     
  Branches     11087    11086       -1     
===========================================
+ Hits         28054    28061       +7     
+ Misses       13236    13232       -4     

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

@metamaskbot
Copy link
Collaborator

Builds ready [efe78af]
Page Load Metrics (1498 ± 124 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1103371936130
domContentLoaded11196576029
load98618861498259124
domInteractive11196576029
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 353 Bytes (0.01%)

@vthomas13 vthomas13 marked this pull request as ready for review December 14, 2023 21:35
@vthomas13 vthomas13 force-pushed the 1708-chainid-symbol-map branch from efe78af to 3c69b7f Compare December 14, 2023 21:35
@vthomas13 vthomas13 force-pushed the 1708-chainid-symbol-map branch from 643eaca to e7d3c7b Compare December 15, 2023 19:16
vthomas13 and others added 5 commits December 15, 2023 17:20
## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
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.
@vthomas13 vthomas13 force-pushed the 1708-chainid-symbol-map branch from 40244ea to 181f89c Compare December 15, 2023 22:21
@metamaskbot
Copy link
Collaborator

Builds ready [181f89c]
Page Load Metrics (1392 ± 144 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint944561888641
domContentLoaded9202546531
load97619411392299144
domInteractive9202546531
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: -116 Bytes (-0.00%)
  • common: 4.74 KiB (0.09%)

@danjm danjm merged commit 727e859 into develop Dec 18, 2023
67 checks passed
@danjm danjm deleted the 1708-chainid-symbol-map branch December 18, 2023 12:39
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2023
@metamaskbot metamaskbot added the release-11.9.0 Issue or pull request that will be included in release 11.9.0 label Dec 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-11.9.0 Issue or pull request that will be included in release 11.9.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.

6 participants