Skip to content

feat: Refactor and update permission-connect UI#23557

Merged
david0xd merged 1 commit intodevelopfrom
dd/refactor-permission-ui
Apr 24, 2024
Merged

feat: Refactor and update permission-connect UI#23557
david0xd merged 1 commit intodevelopfrom
dd/refactor-permission-ui

Conversation

@david0xd
Copy link
Copy Markdown
Contributor

@david0xd david0xd commented Mar 18, 2024 •
edited
Loading

Description

This PR introduces some major code refactoring and UI updates to Connect and Permission Approval screens.

Motivation

There are several blockers encountered while trying to introduce (integrate) new features provided by Snaps Core Platform such as Dynamic Permissions.

The major reason for a blocker is the state of the current implementation of the Permission Approval component. More specifically, the large difference and inconsistencies between the UI components and overall UI structure used for the native and Snaps flows related to the permission approval features are making it close to impossible to maintain and extend with the new functionalities. Building even more fragmented conditional UI in order to introduce new features, as part of the same flows and features, would dramatically increase code complexity and maintenance demands, while resulting in poor UX/UI in the final product.

Hence, after multiple discussions inside the teams and between the different teams, it is determined that the best approach would be complete UI refactoring and consolidation of the user interface between native and Snaps flows. This PR is the first major step made in order to achieve the proposed results and unblock the new features waiting for integration.

What is done in this PR

  • Updated UI for Account Connection and Permission Approval screens.
  • Refactoring of the Connection and Permission Approval components.
  • Refactoring of the other components related to Connect and Permission pages.
  • Replaced old way of UI styling with the new one aligned with Design System approach. This includes replacement of all deprecated Design System components with the current.
  • Removed redundant SCSS code. UI styling is now mostly handled by Design System components.
  • Updated some parts of the Storybook for the related components.

Open in GitHub Codespaces

Related issues

Fixes: #23316

Design: https://www.figma.com/file/jr7XsFvcPCnf4HOMvNWFfn/Permission-System?node-id=1%3A2&mode=dev (see facelift section).

Blocks: MetaMask/snaps#1821
Blocks: MetaMask/snaps#2088
Blocks: MetaMask/snaps#2198

Manual testing steps

  1. Go to E2E Test Dapp or Test Snaps website.
  2. Try connecting the accounts to the Test Dapp. Try approving permission requested. Check the UI.
  3. Try installing a Snap and check all the related UI starting from a connection to the website.
  4. Try installing Ethereum Provider Snap and verify the UI for the complete flow including triggering option for getting Ethereum accounts.
  5. Try using Multi Install Snap feature provided by the Test Snaps web application.
  6. Check any other website connection or permission approval related flows and features and make sure that there are no regressions.
  7. Verify that there are no regressions in all the UX/UI related to the all features mentioned above. Also, verify the functionality of the related features.

Screenshots/Recordings

Before

Screenshot 2024-03-28 at 16 05 48
Screenshot 2024-03-29 at 15 35 51
Screenshot 2024-03-29 at 15 37 26
Screenshot 2024-03-29 at 15 39 52

After

Screenshot 2024-04-22 at 18 40 35
Screenshot 2024-04-22 at 18 41 08
Screenshot 2024-04-22 at 18 41 50
Screenshot 2024-04-22 at 18 42 35
Screenshot 2024-03-29 at 15 48 17
Screenshot 2024-04-17 at 16 28 03

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.

@david0xd david0xd added the team-snaps-platform-deprecated DEPRECATED: please use "team-core-platform" instead (or "team-new-networks" for Solana snap issues) label Mar 18, 2024
@david0xd david0xd self-assigned this Mar 18, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 18, 2024

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.

@david0xd david0xd force-pushed the dd/refactor-permission-ui branch from 95818cd to 28db694 Compare March 18, 2024 17:20
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 18, 2024

Builds ready [6394d20]
Page Load Metrics (1641 ± 396 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint912121293316
domContentLoaded116830178
load7728471641825396
domInteractive116830178
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 313 Bytes (0.00%)
  • common: -959 Bytes (-0.02%)

FrederikBolding
FrederikBolding reviewed Mar 19, 2024
View reviewed changes
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 19, 2024

Builds ready [976d99a]
Page Load Metrics (894 ± 484 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint832491154019
domContentLoaded118328147
load6824748941007484
domInteractive118328147
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 282 Bytes (0.00%)
  • common: -959 Bytes (-0.02%)

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 19, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 47.72727% with 23 lines in your changes are missing coverage. Please review.

Project coverage is 67.52%. Comparing base (cc928f6) to head (510eb77).

❗ Current head 510eb77 differs from pull request most recent head 08cbb10. Consider uploading reports for the commit 08cbb10 to get more accurate results

Files Patch % Lines
...ent/permission-page-container-content.component.js 0.00% 5 Missing ⚠️
ui/components/ui/account-list/account-list.js 64.29% 5 Missing ⚠️
...e-container/permission-page-container.component.js 0.00% 4 Missing ⚠️
...rmissions-connect/permissions-connect.component.js 0.00% 4 Missing ⚠️
...ect-footer/permissions-connect-footer.component.js 0.00% 2 Missing ⚠️
...ission-list/permissions-connect-permission-list.js 0.00% 2 Missing ⚠️
...ission-connect-header/permission-connect-header.js 92.31% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #23557      +/-   ##
===========================================
+ Coverage    67.47%   67.52%   +0.05%     
===========================================
  Files         1257     1258       +1     
  Lines        49228    49207      -21     
  Branches     12819    12809      -10     
===========================================
+ Hits         33216    33227      +11     
+ Misses       16012    15980      -32

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

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 20, 2024

Builds ready [73c71a1]
Page Load Metrics (485 ± 380 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint701931092914
domContentLoaded107232178
load542175485791380
domInteractive107232178
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.9 KiB (0.04%)
  • common: -959 Bytes (-0.02%)

@david0xd david0xd force-pushed the dd/refactor-permission-ui branch 4 times, most recently from d913c2d to b9516c8 Compare March 27, 2024 11:25
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 27, 2024

Builds ready [b9516c8]
Page Load Metrics (1174 ± 520 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint713551527637
domContentLoaded107430188
load58239711741083520
domInteractive107430189
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.78 KiB (0.04%)
  • common: -1.03 KiB (-0.02%)

@david0xd david0xd force-pushed the dd/refactor-permission-ui branch from e7ca2c4 to b6e78e4 Compare March 28, 2024 13:18
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 28, 2024

Builds ready [b6e78e4]
Page Load Metrics (1589 ± 479 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint803671557034
domContentLoaded157329178
load6728401589998479
domInteractive157329178
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.78 KiB (0.04%)
  • common: -1.03 KiB (-0.02%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 28, 2024

Builds ready [dfb3ab8]
Page Load Metrics (808 ± 477 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint793401285928
domContentLoaded136225136
load632309808993477
domInteractive136225136
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.76 KiB (0.04%)
  • common: -1.03 KiB (-0.02%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 28, 2024

Builds ready [0c69724]
Page Load Metrics (762 ± 441 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint662491144019
domContentLoaded106830178
load542090762918441
domInteractive106830178
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.84 KiB (0.04%)
  • common: -1.03 KiB (-0.02%)

@david0xd david0xd force-pushed the dd/refactor-permission-ui branch from 0c69724 to bd88e53 Compare March 29, 2024 13:31
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 29, 2024

Builds ready [bd88e53]
Page Load Metrics (674 ± 433 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint711551072813
domContentLoaded87330178
load582177674902433
domInteractive87330178
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.84 KiB (0.04%)
  • common: -1.03 KiB (-0.02%)

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 29, 2024

Builds ready [a612af6]
Page Load Metrics (1403 ± 527 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint803541537637
domContentLoaded149631199
load67310814031098527
domInteractive149631199
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 2.88 KiB (0.04%)
  • common: -1.03 KiB (-0.02%)

@david0xd david0xd marked this pull request as ready for review March 29, 2024 15:44
@david0xd david0xd requested review from a team as code owners March 29, 2024 15:44
hmalik88
hmalik88 reviewed Mar 29, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@hmalik88 hmalik88 left a comment

Choose a reason for hiding this comment

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

LGTM. Since we're re-doing these components, it might be worth our while to convert to functional components?

@david0xd david0xd force-pushed the dd/refactor-permission-ui branch from a612af6 to 52debfa Compare April 5, 2024 20:53
@david0xd david0xd mentioned this pull request Apr 5, 2024
Closed
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 8, 2024

Builds ready [6ef9bbd]
Page Load Metrics (85 ± 11 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint64143992110
domContentLoaded96730189
load49134852411
domInteractive96730189
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 3.31 KiB (0.05%)
  • common: -1.03 KiB (-0.02%)

hmalik88
hmalik88 previously approved these changes Apr 9, 2024
View reviewed changes
GuillaumeRx
GuillaumeRx reviewed Apr 19, 2024
View reviewed changes
GuillaumeRx
GuillaumeRx reviewed Apr 19, 2024
View reviewed changes
@GuillaumeRx
Copy link
Copy Markdown
Contributor

GuillaumeRx commented Apr 19, 2024

The account identicon seems to not be aligned with the text.

image

@GuillaumeRx
Copy link
Copy Markdown
Contributor

GuillaumeRx commented Apr 19, 2024

The design strips the protocol part of the URL

image

@GuillaumeRx
Copy link
Copy Markdown
Contributor

GuillaumeRx commented Apr 19, 2024

The letter here is not aligned in the middle

image

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 22, 2024

Builds ready [da0dcb9]
Page Load Metrics (1807 ± 674 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint814461657737
domContentLoaded176743178
load67344718071403674
domInteractive176643178
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1016 Bytes (0.02%)
  • common: -957 Bytes (-0.02%)

@david0xd david0xd force-pushed the dd/refactor-permission-ui branch 2 times, most recently from c96b1e7 to 510eb77 Compare April 22, 2024 11:39
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 22, 2024

Builds ready [510eb77]
Page Load Metrics (1427 ± 646 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint802811355225
domContentLoaded106529147
load68324114271345646
domInteractive106529147
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1016 Bytes (0.02%)
  • common: -957 Bytes (-0.02%)

@darkwing darkwing added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Apr 22, 2024
@david0xd david0xd force-pushed the dd/refactor-permission-ui branch from 510eb77 to 08cbb10 Compare April 22, 2024 16:26
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 22, 2024

Builds ready [cc81a95]
Page Load Metrics (980 ± 581 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint642911305828
domContentLoaded1079312110
load5328389801211581
domInteractive1079312110
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1016 Bytes (0.02%)
  • common: -957 Bytes (-0.02%)

eriknson
eriknson previously approved these changes Apr 22, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@eriknson eriknson left a comment

Choose a reason for hiding this comment

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

LGTM

@david0xd david0xd force-pushed the dd/refactor-permission-ui branch from cc81a95 to b84ed15 Compare April 23, 2024 07:16
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 23, 2024

Builds ready [b84ed15]
Page Load Metrics (984 ± 587 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint572241153818
domContentLoaded9107332512
load4630079841223587
domInteractive9106332512
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 1016 Bytes (0.02%)
  • common: -957 Bytes (-0.02%)

hmalik88
hmalik88 previously approved these changes Apr 23, 2024
View reviewed changes
darkwing
darkwing previously approved these changes Apr 23, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

Everything looks good visually but I did have some non-blocking nits.

@david0xd david0xd dismissed stale reviews from darkwing, hmalik88, and eriknson via fea872f April 23, 2024 20:15
@david0xd
Refactor and update permission-connect UI
f1ce1a6 
Fix test and refactor code

Fix lint error for unused translations

Update data-test-id for select all checkbox

Update permission request screen and refactor code

Add PermissionConnectHeader component

Revert changes in Snap Authorship Header

Update button names and actions

Update Snap connection UI and colors

Add more UI adjustments

Fix e2e tests

Fix Snap e2e tests

Fix account Snaps e2e test

Fix test snap eth-provider

Fix other snap insight related e2e tests

Add another UI fixing iteration

Add more UI updates for the connection flow

Refactor and fix lint

Fix lint

Update title displaying logic for Permission Connect Header

Refactor Snap permissions and fix small UI issue with permission display

Fix position of last-connected account status icon

Update background colors and fix some stuff

Revert Snap avatar changes

Fix width for Snap connect content

Fix and update loading screens

Update font size on account list page

Update final connection result/redirect screen

Update translation and font variant

Adapt new e2e test

Apply fix for account list pre-selected account

Fix storybook error

Address review requests

Small refactoring

Replace deprecated component for website icon

Fix lint problems with locales

Fix recent e2e test changes after rebase

Adjust size of the titles (headingLg -> headingMd)

Refactor way of getting fallback icon
@david0xd david0xd force-pushed the dd/refactor-permission-ui branch from fea872f to f1ce1a6 Compare April 23, 2024 20:49
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Apr 23, 2024

Builds ready [f1ce1a6]
Page Load Metrics (1343 ± 554 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint752071153215
domContentLoaded136629168
load62277213431153554
domInteractive136629168
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 0 Bytes (0.00%)
  • ui: 977 Bytes (0.01%)
  • common: -808 Bytes (-0.01%)

@david0xd david0xd merged commit ec03702 into develop Apr 24, 2024
@david0xd david0xd deleted the dd/refactor-permission-ui branch April 24, 2024 18:26
@github-actions github-actions bot locked and limited conversation to collaborators Apr 24, 2024
@metamaskbot metamaskbot added the release-11.16.0 Issue or pull request that will be included in release 11.16.0 label Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@FrederikBolding FrederikBolding FrederikBolding left review comments

@GuillaumeRx GuillaumeRx GuillaumeRx approved these changes

@hmalik88 hmalik88 hmalik88 left review comments

+2 more reviewers

@darkwing darkwing darkwing approved these changes

@eriknson eriknson eriknson left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@david0xd david0xd

Labels

needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-11.16.0 Issue or pull request that will be included in release 11.16.0 team-snaps-platform-deprecated DEPRECATED: please use "team-core-platform" instead (or "team-new-networks" for Solana snap issues) type-refactor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

New connection / account selection screen

7 participants

@david0xd @metamaskbot @GuillaumeRx @darkwing @FrederikBolding @eriknson @hmalik88