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.

Sign up for GitHub

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

Jump to bottom

feat: unified confirmation navigation #28761

Merged
merged 34 commits into from
Dec 10, 2024
Merged

feat: unified confirmation navigation #28761

merged 34 commits into from
Dec 10, 2024
+1,301 −659

Conversation

matthewwalsh0
Copy link
Member

@matthewwalsh0 matthewwalsh0 commented Nov 27, 2024 •
edited
Loading

Description

Support a single navigation queue between all types of pending confirmation, including:

  • Transactions
  • Signatures
  • Template Confirmations (e.g. Snap Dialogs, Smart Transaction Status Page)
  • Get Encryption Public Key
  • Decrypt
  • Add Token
  • Add NFT
  • Connect

Specifically:

  • Add useConfirmationNavigation hook to centralise all confirmation routing.
  • Use routing hook in:
    • ConfirmPageContainerNavigation
    • Home
    • Nav
    • syncConfirmPath
  • Replace custom navigation in ConfirmationPage with Nav component.
  • Add confirmationId property to Nav component and remove coupling to useCurrentConfirmation hook.
  • Add Nav component to:
    • ConfirmAddSuggestedNFT
    • ConfirmAddSuggestedToken
    • ConfirmDecryptMessage
    • ConfirmEncryptionPublicKey
    • PermissionConnectHeader
  • Add associated E2E test.
  • Remove manual header sizing from ConfirmationPage component.
  • Fix template confirmation storybooks.
  • Add snap dialog storybooks.

Open in GitHub Codespaces

Related issues

Manual testing steps

  1. Add unapproved transaction.
  2. Add unapproved signature request
  3. Install test dialog snap.
  4. Create confirmation via test dialog snap.
  5. Verify confirmation count is 3 and navigation functions between them.

Screenshots/Recordings

Before

After

Navigation.mov

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.

@matthewwalsh0
Add template confirmations to transaction and signature navigation
e700000 
Add Storybook CSS fixes for confirmation templates.
Add useConfirmationNavigation hook.
Update redesigned and legacy navigation components.
Add nav component to template confirmation.
Add snap dialog stories.
Support template confirmations in useCurrentConfirmation.
@matthewwalsh0
Remove unused state
a4a3ec9
@metamaskbot metamaskbot added the team-confirmations Push issues to confirmations team label Nov 27, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [71bea94]
Page Load Metrics (1812 ± 55 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15982020181012058
domContentLoaded15911991177811555
load16092029181211555
domInteractive2410934189
backgroundConnect984342110
firstReactRender16352053
getState762251262814
initialActions01000
loadScripts11591548135111756
setupStore77713157
uiStartup19052535209916378
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.39 KiB (0.02%)
  • common: 5 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [e7d5084]
Page Load Metrics (1928 ± 96 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint17092291193118388
domContentLoaded16382281190019091
load16512293192820196
domInteractive249336168
backgroundConnect97529199
firstReactRender15252031
getState881961352411
initialActions01000
loadScripts12461754145514770
setupStore77814168
uiStartup193526632242234113
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.39 KiB (0.02%)
  • common: 5 Bytes (0.00%)

@matthewwalsh0 matthewwalsh0 force-pushed the feat/shared-confirmation-navigation branch from e7d5084 to 5e3c824 Compare December 6, 2024 00:54
@metamaskbot
Copy link
Collaborator

Builds ready [5e3c824]
Page Load Metrics (1782 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15682091177912560
domContentLoaded15262079175413063
load15722093178212460
domInteractive23652994
backgroundConnect95728168
firstReactRender15109272411
getState743021334923
initialActions01000
loadScripts11351672134712962
setupStore79315199
uiStartup17952614210419794
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.42 KiB (0.02%)
  • common: 5 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [2ceb912]
Page Load Metrics (1805 ± 57 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16322006181211857
domContentLoaded15971997177712058
load16332006180512057
domInteractive258942209
backgroundConnect986332311
firstReactRender1593292613
getState713581448139
initialActions00000
loadScripts12181591137411455
setupStore68014178
uiStartup184626472169235113
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.39 KiB (0.02%)
  • common: 5 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [97a70a9]
Page Load Metrics (2193 ± 64 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint43625122098399192
domContentLoaded19582448215013062
load19912512219313364
domInteractive2410938188
backgroundConnect1183402412
firstReactRender169325168
getState814381587737
initialActions01000
loadScripts14781971167613062
setupStore78314168
uiStartup23633128255519996
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 1.78 KiB (0.02%)
  • common: 5 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [ec62382]
Page Load Metrics (2197 ± 130 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint61628272104434208
domContentLoaded179227422162277133
load185427502197270130
domInteractive26196635024
backgroundConnect998432311
firstReactRender16462484
getState862291403014
initialActions00000
loadScripts134321341678225108
setupStore620932
uiStartup206733142516350168
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 651 Bytes (0.01%)
  • ui: 2.24 KiB (0.03%)
  • common: 2 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [1f03058]
Page Load Metrics (1893 ± 70 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint39122821810356171
domContentLoaded15452221185914268
load15542285189314570
domInteractive23533384
backgroundConnect966342010
firstReactRender158323147
getState962181352814
initialActions01000
loadScripts11701715145211655
setupStore713921
uiStartup174926882196211102
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 651 Bytes (0.01%)
  • ui: 2.24 KiB (0.03%)
  • common: 2 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [1bc93b0]
Page Load Metrics (1761 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29519331674332160
domContentLoaded15332023173612158
load15412071176112660
domInteractive226633147
backgroundConnect85924178
firstReactRender1594362914
getState742981547134
initialActions01000
loadScripts11681629136811957
setupStore67110147
uiStartup175428982130282135
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 1.25 KiB (0.02%)
  • ui: 2.26 KiB (0.03%)
  • common: 9 Bytes (0.00%)

FrederikBolding
FrederikBolding approved these changes Dec 10, 2024
View reviewed changes
Copy link
Member

@FrederikBolding FrederikBolding left a comment

Choose a reason for hiding this comment

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

Seems super solid - LGTM!

@matthewwalsh0 matthewwalsh0 added this pull request to the merge queue Dec 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 10, 2024
@matthewwalsh0 matthewwalsh0 added this pull request to the merge queue Dec 10, 2024
Merged via the queue into main with commit ac4bdea Dec 10, 2024
75 checks passed
@matthewwalsh0 matthewwalsh0 deleted the feat/shared-confirmation-navigation branch December 10, 2024 17:52
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2024
@metamaskbot metamaskbot added the release-12.10.0 Issue or pull request that will be included in release 12.10.0 label Dec 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bschorchit bschorchit bschorchit left review comments

@NidhiKJha NidhiKJha NidhiKJha approved these changes

@OGPoyraz OGPoyraz OGPoyraz approved these changes

@FrederikBolding FrederikBolding FrederikBolding approved these changes

Assignees
No one assigned
Labels
release-12.10.0 Issue or pull request that will be included in release 12.10.0 team-confirmations Push issues to confirmations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@matthewwalsh0 @metamaskbot @FrederikBolding @OGPoyraz @bschorchit @NidhiKJha