Skip to content

feat: import tokens redesign#21704

Merged
sahar-fehri merged 6 commits intodevelopfrom
feat/MMASSETS-62-import-token-redesign
Dec 14, 2023
Merged

feat: import tokens redesign#21704
sahar-fehri merged 6 commits intodevelopfrom
feat/MMASSETS-62-import-token-redesign

Conversation

@sahar-fehri
Copy link
Copy Markdown
Contributor

@sahar-fehri sahar-fehri commented Nov 6, 2023 •
edited
Loading

Description

This ticket aims to redesign the import token flow.
Here is the ticket: https://consensyssoftware.atlassian.net/browse/MMASSETS-62
And Figma: https://www.figma.com/file/TfVzSMJA8KwpWX8TTWQ2iO/MMASSETS-15%3A-Asset-list-and-details?type=design&node-id=1491-56878&mode=design

Notes:
1- In the Figma designs you will notice that the notification you get when you import a token is a black box notif; but the one in this PR is a green box; we have decided to eventually go with a notification component that already existed.
2- In the Figma design (2.0A) the confirmation page should also show the balance in Fiat. We decided to split that part in a separate ticket and for this PR use the same tokenBalance component used in the old confirmation modal.

Related issues

Fixes: ##21052

Manual testing steps

  1. Go to home page
  2. Click on import tokens button
  3. try searching for your token
  4. Select the tokens you need
  5. Click Next
  6. If you want to exit at this point, click the "x" and confirm; and if you want to go back click "back"
  7. If you clicked back on step 6; click import and you should be redirected to home page.
  8. You should be able to see your tokens on home page.

Screenshots/Recordings

Before

MetaMask.-.import-before.mp4

After

MetaMask.-.Google.Chrome.2023-12-11.12-16-32.mp4

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.

@sahar-fehri sahar-fehri requested a review from a team as a code owner November 6, 2023 17:54
@sahar-fehri sahar-fehri self-assigned this Nov 6, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 6, 2023

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.

@sahar-fehri sahar-fehri force-pushed the feat/MMASSETS-62-import-token-redesign branch 4 times, most recently from f44789a to b94d01a Compare November 7, 2023 20:13
@legobeat legobeat marked this pull request as draft November 8, 2023 10:39
@sahar-fehri sahar-fehri force-pushed the feat/MMASSETS-62-import-token-redesign branch 4 times, most recently from 13131ed to 6d95b4e Compare November 9, 2023 12:56
@sahar-fehri sahar-fehri changed the title [WIP]: import tokens redesign feat: import tokens redesign Nov 9, 2023
@sahar-fehri sahar-fehri marked this pull request as ready for review November 9, 2023 14:09
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Nov 9, 2023

Builds ready [1f92997]
Page Load Metrics (487 ± 262 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint841269795
domContentLoaded6612285126
load781335487546262
domInteractive6612285126
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 25.47 KiB (0.31%)
  • common: 340 Bytes (0.01%)

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 9, 2023 •
edited
Loading

Codecov Report

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

Comparison is base (ba90599) 67.95% compared to head (65f13df) 67.93%.

Files Patch % Lines
...tichain/import-tokens-modal/import-tokens-modal.js 55.42% 37 Missing ⚠️
ui/helpers/utils/token-util.js 47.62% 11 Missing ⚠️
...pp/import-token/token-list/token-list.component.js 16.67% 10 Missing ⚠️
...import-tokens-modal/import-tokens-modal-confirm.js 60.00% 4 Missing ⚠️
...mport-token/token-search/token-search.component.js 25.00% 3 Missing ⚠️
ui/pages/home/home.component.js 25.00% 3 Missing ⚠️
ui/ducks/app/app.ts 0.00% 2 Missing ⚠️
ui/pages/home/home.container.js 0.00% 2 Missing ⚠️
ui/store/actions.ts 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #21704      +/-   ##
===========================================
- Coverage    67.95%   67.93%   -0.02%     
===========================================
  Files         1067     1067              
  Lines        41274    41341      +67     
  Branches     11073    11090      +17     
===========================================
+ Hits         28047    28084      +37     
- Misses       13227    13257      +30

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

sahar-fehri
sahar-fehri commented Nov 9, 2023
View reviewed changes
@sahar-fehri
Copy link
Copy Markdown
Contributor Author

sahar-fehri commented Nov 9, 2023

Apologies for all the changed files; half of those are just locales; the lint was failing because of unused locales and I had to manually remove them, let me know if there was a better way to do this 🙏

@sahar-fehri
Copy link
Copy Markdown
Contributor Author

sahar-fehri commented Nov 9, 2023

I might have done some CSS gymnastics, please let me know; happy to fix those with your suggestions 🙏 :D

@sahar-fehri sahar-fehri requested a review from darkwing November 9, 2023 14:38
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Nov 10, 2023

Builds ready [b6ddfe1]
Page Load Metrics (669 ± 283 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8312595115
domContentLoaded6912086115
load811595669590283
domInteractive6912086115
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 25.47 KiB (0.31%)
  • common: 340 Bytes (0.01%)

@kevinghim
Copy link
Copy Markdown
Contributor

kevinghim commented Nov 14, 2023

@sahar-fehri I know the design was originally a full page, but wondering if it's beneficial to convert it to a modal. We'd want full complex flows to be a page, but quick interactions to be a modal. This way, on the full screen, it can 'float' above the wallet page when tokens are imported. wdyt? cc: @darkwing

@sahar-fehri sahar-fehri force-pushed the feat/MMASSETS-62-import-token-redesign branch from b6ddfe1 to f6689c2 Compare November 17, 2023 13:28
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Nov 17, 2023

Builds ready [f6689c2]
Page Load Metrics (360 ± 224 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint84144101189
domContentLoaded6712488168
load791549360466224
domInteractive6712488168
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 25.47 KiB (0.37%)
  • common: 340 Bytes (0.01%)

@alfeng6
Copy link
Copy Markdown

alfeng6 commented Nov 17, 2023

@kevinghim as we talked about on Slack, we can merge this in now and have a follow up ticket to turn this into a modal view.

darkwing
darkwing suggested changes Nov 17, 2023
View reviewed changes
@sahar-fehri sahar-fehri force-pushed the feat/MMASSETS-62-import-token-redesign branch from 64064c2 to 8c8174f Compare November 20, 2023 11:57
darkwing
darkwing previously approved these changes Dec 11, 2023
View reviewed changes
@sahar-fehri sahar-fehri force-pushed the feat/MMASSETS-62-import-token-redesign branch 5 times, most recently from 8b72e40 to 7680187 Compare December 12, 2023 22:03
NidhiKJha
NidhiKJha previously approved these changes Dec 13, 2023
View reviewed changes
@sahar-fehri sahar-fehri mentioned this pull request Dec 13, 2023
Merged
12 tasks
@sahar-fehri sahar-fehri force-pushed the feat/MMASSETS-62-import-token-redesign branch from 773b7ff to b4776cf Compare December 13, 2023 13:53
@sahar-fehri sahar-fehri force-pushed the feat/MMASSETS-62-import-token-redesign branch from b4776cf to 5f8ad34 Compare December 14, 2023 10:13
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Dec 14, 2023

Builds ready [5f8ad34]
Page Load Metrics (1244 ± 109 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint882751395928
domContentLoaded9185395727
load82016111244227109
domInteractive9185395727
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 4.9 KiB (0.07%)
  • common: 70 Bytes (0.00%)

NidhiKJha
NidhiKJha previously approved these changes Dec 14, 2023
View reviewed changes
@sahar-fehri sahar-fehri force-pushed the feat/MMASSETS-62-import-token-redesign branch from 5f8ad34 to 5476bf1 Compare December 14, 2023 13:38
@sahar-fehri sahar-fehri merged commit eca75b5 into develop Dec 14, 2023
@sahar-fehri sahar-fehri deleted the feat/MMASSETS-62-import-token-redesign branch December 14, 2023 14:28
@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2023
@metamaskbot metamaskbot added the release-11.9.0 Issue or pull request that will be included in release 11.9.0 label Dec 14, 2023
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Dec 14, 2023

Builds ready [65f13df]
Page Load Metrics (1339 ± 124 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint933531777938
domContentLoaded10196667637
load90117791339258124
domInteractive10196667637
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 4.9 KiB (0.07%)
  • common: 70 Bytes (0.00%)

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

Reviewers

@pedronfigueiredo pedronfigueiredo pedronfigueiredo approved these changes

@NidhiKJha NidhiKJha NidhiKJha approved these changes

+1 more reviewer

@darkwing darkwing darkwing approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@sahar-fehri sahar-fehri

Labels

release-11.9.0 Issue or pull request that will be included in release 11.9.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@sahar-fehri @metamaskbot @kevinghim @alfeng6 @darkwing @pedronfigueiredo @NidhiKJha