refactor: extract Send-specific functionality out of AssetPicker by micaelae · Pull Request #26558 · MetaMask/metamask-extension

micaelae · 2024-08-20T23:49:26Z

Description

The goal of this PR is to make the AssetPicker component experience-agnostic so that it can be reused within other experiences.

Since the AssetPicker was initially built for the Swap+Send experience, most changes here are for moving send-specific logic out of the component, including:

move Send event tracking callbacks to send page
add new AssetPicker props for setting custom modal header and visible tabs
define an experience-agnostic asset prop, which contains the minimal required information about the selected asset
use accurate type definitions for assets and props

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/METABRIDGE-890

Manual testing steps

Swap+Send asset selection experience should not change

Screenshots/Recordings

Before -> After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
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.

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.

metamaskbot · 2024-08-21T01:54:48Z

Builds ready [4430252]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (70 ± 17 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	61	248	96	41	20
		domContentLoaded	38	210	66	37	18
		load	45	210	70	36	17
		domInteractive	9	114	29	24	12

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 2.46 KiB (0.03%)
common: 0 Bytes (0.00%)

codecov · 2024-08-21T01:55:08Z

Codecov Report

Attention: Patch coverage is 72.38095% with 29 lines in your changes missing coverage. Please review.

Project coverage is 70.09%. Comparing base (fb61b0f) to head (4dbfad9).
Report is 83 commits behind head on develop.

Files with missing lines	Patch %	Lines
.../asset-picker-modal/asset-picker-modal-nft-tab.tsx	36.36%	7 Missing ⚠️
...ichain/asset-picker-amount/asset-picker-amount.tsx	70.59%	5 Missing ⚠️
ui/components/multichain/pages/send/send.js	16.67%	5 Missing ⚠️
...r-amount/asset-picker-modal/asset-picker-modal.tsx	87.50%	3 Missing ⚠️
.../asset-picker-amount/asset-picker/asset-picker.tsx	76.92%	3 Missing ⚠️
...ichain/pages/send/components/recipient-content.tsx	75.00%	3 Missing ⚠️
...unt/asset-picker-modal/asset-picker-modal-tabs.tsx	83.33%	2 Missing ⚠️
...t/asset-picker-modal/asset-picker-modal-search.tsx	87.50%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #26558      +/-   ##
===========================================
+ Coverage    70.08%   70.09%   +0.01%     
===========================================
  Files         1413     1416       +3     
  Lines        49318    49352      +34     
  Branches     13780    13786       +6     
===========================================
+ Hits         34562    34593      +31     
- Misses       14756    14759       +3

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

metamaskbot · 2024-08-28T23:45:23Z

Builds ready [5a0f942]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1755 ± 70 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	226	2141	1609	476	229
		domContentLoaded	1580	2132	1730	127	61
		load	1588	2140	1755	145	70
		domInteractive	18	103	40	25	12

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 2.47 KiB (0.03%)
common: 0 Bytes (0.00%)

ui/components/multichain/asset-picker-amount/asset-picker-modal/types.ts

sahar-fehri · 2024-08-29T11:53:04Z

LGTM! Thank you 🙏

sonarqubecloud · 2024-08-29T16:18:08Z

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-08-29T16:50:17Z

Builds ready [4dbfad9]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1640 ± 48 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	247	1908	1573	323	155
		domContentLoaded	1485	1900	1609	100	48
		load	1494	1909	1640	100	48
		domInteractive	14	64	31	12	6

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 2.47 KiB (0.03%)
common: 0 Bytes (0.00%)

ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx

micaelae requested a review from a team as a code owner August 20, 2024 23:49

micaelae marked this pull request as draft August 20, 2024 23:49

metamaskbot added the team-bridge-deprecated DEPRECATED: please use "team-swaps-and-bridge" instead label Aug 20, 2024

micaelae force-pushed the mb890-decouple-send-and-asset-picker branch 2 times, most recently from 3819a56 to 4430252 Compare August 21, 2024 01:14

micaelae added 12 commits August 28, 2024 15:59

refactor: reuse isTokenDisabled callback in AssetPickerModal/AssetList

abf4738

refactor: move Search outside of AssetPickerModal

4e6f428

refactor: move sendAssetSelected event tracking to send page

e5e4b65

style: asset picker modal spacing consistency

134e5ca

refactor: move sendTokenModalOpened event tracking to send page

083f622

refactor: pass in asset-picker header as prop

f99cda6

refactor: extract visible tab logic

a66d447

refactor: move NFT selectors to asset-picker NFT tab

b99da91

refactor: build sendingAsset prop in recipient-content

fef46a2

refactor: pass in experience-agnostic asset to AssetPicker

c46c521

refactor: refine asset/token types

cbd1360

test: add storybook for AssetPicker

5a0f942

micaelae force-pushed the mb890-decouple-send-and-asset-picker branch from 4430252 to 5a0f942 Compare August 28, 2024 23:13

micaelae marked this pull request as ready for review August 29, 2024 00:41

sahar-fehri reviewed Aug 29, 2024

View reviewed changes

ui/components/multichain/asset-picker-amount/asset-picker-modal/types.ts Outdated Show resolved Hide resolved

fix: rm TODO comment

4dbfad9

sahar-fehri approved these changes Aug 29, 2024

View reviewed changes

ejwessel reviewed Aug 29, 2024

View reviewed changes

ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx Show resolved Hide resolved

ejwessel approved these changes Aug 29, 2024

View reviewed changes

micaelae merged commit f086572 into develop Aug 29, 2024

micaelae deleted the mb890-decouple-send-and-asset-picker branch August 29, 2024 20:30

github-actions bot locked and limited conversation to collaborators Aug 29, 2024

metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 29, 2024

gauthierpetetin added release-12.4.0 Issue or pull request that will be included in release 12.4.0 and removed release-12.5.0 Issue or pull request that will be included in release 12.5.0 labels Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: extract Send-specific functionality out of AssetPicker#26558

refactor: extract Send-specific functionality out of AssetPicker#26558
micaelae merged 13 commits intodevelopfrom
mb890-decouple-send-and-asset-picker

micaelae commented Aug 20, 2024 •

edited

Loading

Uh oh!

metamaskbot commented Aug 21, 2024

Uh oh!

codecov bot commented Aug 21, 2024 •

edited

Loading

Uh oh!

metamaskbot commented Aug 28, 2024

Uh oh!

Uh oh!

sahar-fehri commented Aug 29, 2024

Uh oh!

sonarqubecloud bot commented Aug 29, 2024

Uh oh!

metamaskbot commented Aug 29, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

micaelae commented Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before -> After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

metamaskbot commented Aug 21, 2024

Uh oh!

codecov bot commented Aug 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

metamaskbot commented Aug 28, 2024

Uh oh!

Uh oh!

sahar-fehri commented Aug 29, 2024

Uh oh!

sonarqubecloud bot commented Aug 29, 2024

Quality Gate passed

Uh oh!

metamaskbot commented Aug 29, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

micaelae commented Aug 20, 2024 •

edited

Loading

codecov bot commented Aug 21, 2024 •

edited

Loading