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

feat: add network picker to AssetPicker #26559

Merged
merged 20 commits into from
Oct 10, 2024

Conversation

micaelae
Copy link
Member

@micaelae micaelae commented Aug 20, 2024

Description

Changes included in this PR:

  • Add a network picker to the AssetPicker modal component so that it can be reused within the cross-chain swaps experience
  • Update AssetPicker components to enable displaying network and asset data when the selected network is not the same as the wallet's active network. Example usecase: destination asset for cross-chain swaps. Specifically:
    • when selected network is not the same as wallet's network, display
      • selected network's icons in asset list
      • selected network's native token icons
    • add customTokenListGenerator prop to AssetPicker that allows upstream components to override the default displayed token list

Figma design: https://www.figma.com/design/bC6RgeriyERMtMlZE8xwkm/Cross-Chain-Swaps?node-id=1490-18690&t=pnpoVVaJTqh15I0a-0

Open in GitHub Codespaces

Related issues

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

Manual testing steps

  1. Swap+Send asset selection experience should not change
  2. Storybook should show an AssetPicker variation that has a network picker

Screenshots/Recordings

Before

Screenshot 2024-08-21 at 2 10 06 PM

After

Screenshot 2024-10-02 at 4 09 51 PMScreenshot 2024-08-21 at 2 09 04 PM

Pre-merge author checklist

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.

@micaelae micaelae requested a review from a team as a code owner August 20, 2024 23:53
@micaelae micaelae marked this pull request as draft August 20, 2024 23:53
@micaelae 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 micaelae changed the base branch from mb890-decouple-send-and-asset-picker to develop August 21, 2024 01:41
@micaelae micaelae changed the base branch from develop to mb890-decouple-send-and-asset-picker August 21, 2024 01:42
@micaelae micaelae force-pushed the mb890-asset-picker-with-selectable-network branch from d1ff94a to 5c540a4 Compare August 21, 2024 01:46
@metamaskbot
Copy link
Collaborator

Builds ready [5c540a4]
Page Load Metrics (90 ± 10 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint892051242512
domContentLoaded48157882211
load54157902110
domInteractive159434178
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 6.17 KiB (0.09%)
  • common: 0 Bytes (0.00%)

Copy link

codecov bot commented Aug 21, 2024

Codecov Report

Attention: Patch coverage is 73.23944% with 19 lines in your changes missing coverage. Please review.

Project coverage is 69.98%. Comparing base (4430252) to head (931e829).

Files Patch % Lines
.../asset-picker-amount/asset-picker/asset-picker.tsx 50.00% 10 Missing ⚠️
...r-amount/asset-picker-modal/asset-picker-modal.tsx 85.71% 4 Missing ⚠️
.../asset-picker-modal/asset-picker-modal-network.tsx 80.00% 3 Missing ⚠️
...set-picker-amount/asset-picker-modal/AssetList.tsx 75.00% 2 Missing ⚠️
Additional details and impacted files
@@                           Coverage Diff                            @@
##           mb890-decouple-send-and-asset-picker   #26559      +/-   ##
========================================================================
- Coverage                                 70.03%   69.98%   -0.05%     
========================================================================
  Files                                      1407     1408       +1     
  Lines                                     49066    49066              
  Branches                                  13719    13727       +8     
========================================================================
- Hits                                      34360    34335      -25     
- Misses                                    14706    14731      +25     

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

@micaelae micaelae force-pushed the mb890-asset-picker-with-selectable-network branch from 5c540a4 to 931e829 Compare August 21, 2024 21:16
@metamaskbot
Copy link
Collaborator

Builds ready [931e829]
Page Load Metrics (105 ± 15 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint962381373517
domContentLoaded531631013115
load601821053215
domInteractive197742168
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 6.17 KiB (0.09%)
  • common: 49 Bytes (0.00%)

@micaelae micaelae force-pushed the mb890-decouple-send-and-asset-picker branch from 4430252 to 5a0f942 Compare August 28, 2024 23:13
@micaelae micaelae force-pushed the mb890-asset-picker-with-selectable-network branch from 931e829 to 4f03684 Compare August 28, 2024 23:26
Base automatically changed from mb890-decouple-send-and-asset-picker to develop August 29, 2024 20:30
@micaelae micaelae force-pushed the mb890-asset-picker-with-selectable-network branch 3 times, most recently from c6b0816 to c2fad63 Compare September 5, 2024 01:38
@micaelae micaelae marked this pull request as ready for review September 5, 2024 01:51
@micaelae micaelae force-pushed the mb890-asset-picker-with-selectable-network branch from c2fad63 to eb0ad0e Compare September 5, 2024 01:52
Copy link

sonarcloud bot commented Sep 5, 2024

@sahar-fehri
Copy link
Contributor

Hi @micaelae 👋
When testing the send flow, i see this behaviour, is this intentional?

Screen.Recording.2024-09-10.at.16.50.44.mov

@micaelae micaelae force-pushed the mb890-asset-picker-with-selectable-network branch from eb0ad0e to 7aac977 Compare September 25, 2024 22:09
@micaelae micaelae requested a review from a team as a code owner September 25, 2024 22:09
@metamaskbot
Copy link
Collaborator

Builds ready [7aac977]
Page Load Metrics (1776 ± 85 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15672201177816981
domContentLoaded15542193175617182
load15632211177617785
domInteractive248042168
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 4.25 KiB (0.06%)
  • common: 47 Bytes (0.00%)

@MetaMask MetaMask deleted a comment from sonarcloud bot Sep 26, 2024
@micaelae micaelae force-pushed the mb890-asset-picker-with-selectable-network branch from 622b27e to d1cf04f Compare September 26, 2024 15:54
@metamaskbot
Copy link
Collaborator

Builds ready [d1cf04f]
Page Load Metrics (2031 ± 155 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30628911870611293
domContentLoaded165128432001307147
load165928942031322155
domInteractive267849178
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 4.24 KiB (0.06%)
  • common: 47 Bytes (0.00%)

@micaelae
Copy link
Member Author

Hi @micaelae 👋 When testing the send flow, i see this behaviour, is this intentional?

Screen.Recording.2024-09-10.at.16.50.44.mov

This was not intentional! Removed it in the latest version of the PR

@micaelae micaelae force-pushed the mb890-asset-picker-with-selectable-network branch from d14b34b to 2309fa5 Compare October 9, 2024 16:18
@sahar-fehri
Copy link
Contributor

@sahar-fehri Will this break with your recent work re: primary/secondary currency?

Hello @darkwing , it wont break the recent aggregated stuff; the PR is up to date with develop and its no longer using the useNativeCurrencyAsPrimaryCurrency that we have removed

Copy link

sonarcloud bot commented Oct 9, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [2309fa5]
Page Load Metrics (1861 ± 89 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint42122531703432207
domContentLoaded15542369182218890
load16032380186118589
domInteractive25216514220
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 3.84 KiB (0.05%)
  • common: 47 Bytes (0.00%)

Copy link

@andreahaku andreahaku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@micaelae micaelae added this pull request to the merge queue Oct 10, 2024
Merged via the queue into develop with commit 68dd6f5 Oct 10, 2024
78 checks passed
@micaelae micaelae deleted the mb890-asset-picker-with-selectable-network branch October 10, 2024 13:32
@github-actions github-actions bot locked and limited conversation to collaborators Oct 10, 2024
@metamaskbot metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 10, 2024
@gauthierpetetin gauthierpetetin added release-12.6.0 Issue or pull request that will be included in release 12.6.0 and removed release-12.7.0 Issue or pull request that will be included in release 12.7.0 labels Oct 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.6.0 Issue or pull request that will be included in release 12.6.0 team-bridge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants