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

fix: Snap Address component UI/UX (Snaps custom UI) #26477

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

david0xd
Copy link
Contributor

@david0xd david0xd commented Aug 16, 2024

Description

Because of concerns around Snap custom UI address component not working as intended with Pet Names, decision was made to disable it's interactive UX and display shortened hexadecimal address instead of showing Pet name.

Changes on this PR will make Address component to ignore clicks and triggering the modal for nicknames.

Changes are applied only to Snaps related flows where Snaps components (custom UI) are used.

Open in GitHub Codespaces

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2965

Manual testing steps

  1. Try using a Snap with Custom UI which has Address component.
  2. Try clicking on the Address component.
  3. Nothing should happen when Address component is clicked.
  4. Make sure that address component is not displaying nicknames (only short hex address is required).

Screenshots/Recordings

Before

Screenshot 2024-08-19 at 16 49 24

After

Customized Interactive UI Snap for the test case, just for reference:
Screenshot 2024-08-19 at 16 44 51

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.

@david0xd david0xd added the team-snaps-platform Snaps Platform team label Aug 16, 2024
@david0xd david0xd self-assigned this Aug 16, 2024
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.

Copy link

codecov bot commented Aug 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.97%. Comparing base (97246c5) to head (de76f97).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #26477   +/-   ##
========================================
  Coverage    69.96%   69.97%           
========================================
  Files         1405     1405           
  Lines        48996    48998    +2     
  Branches     13697    13699    +2     
========================================
+ Hits         34280    34282    +2     
  Misses       14716    14716           

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

@metamaskbot
Copy link
Collaborator

Builds ready [23331d5]
Page Load Metrics (73 ± 10 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint781641062211
domContentLoaded117629147
load48120732210
domInteractive117629147
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 11 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@david0xd david0xd force-pushed the dd/fix-snap-address-component branch from 31bebad to 4492e21 Compare August 19, 2024 14:50
@david0xd david0xd force-pushed the dd/fix-snap-address-component branch from 4492e21 to de76f97 Compare August 19, 2024 15:14
Copy link

@david0xd david0xd marked this pull request as ready for review August 19, 2024 15:33
@david0xd david0xd requested a review from a team as a code owner August 19, 2024 15:33
@metamaskbot
Copy link
Collaborator

Builds ready [de76f97]
Page Load Metrics (84 ± 9 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint751671132412
domContentLoaded5411480188
load6011884189
domInteractive175429105
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 164 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@david0xd david0xd changed the title fix: Snap address component behavior fix: Snap address component UI/UX Aug 20, 2024
@david0xd david0xd changed the title fix: Snap address component UI/UX fix: Snap Address component UI/UX (Snaps custom UI) Aug 20, 2024
@david0xd david0xd added the area-UI Relating to the user interface. label Aug 20, 2024
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

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

Change look good, will be nicer to have unit test coverage also.

@david0xd david0xd merged commit 8355bcf into develop Aug 20, 2024
92 checks passed
@david0xd david0xd deleted the dd/fix-snap-address-component branch August 20, 2024 13:14
@github-actions github-actions bot locked and limited conversation to collaborators Aug 20, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 20, 2024
@gauthierpetetin 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-UI Relating to the user interface. release-12.4.0 Issue or pull request that will be included in release 12.4.0 team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants