Skip to content

fix: import srp error handling and style #31662

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

Merged
merged 15 commits into from
Apr 10, 2025
Merged

fix: import srp error handling and style #31662

merged 15 commits into from
Apr 10, 2025

Conversation

montelaidev
Copy link
Contributor

@montelaidev montelaidev commented Apr 7, 2025

Description

This PR fixes the following:

  1. The import button is now floating
  2. Importing a 12 word srp when 24 word is selected is now blocked.
  3. Add 12 word srp option.

Open in GitHub Codespaces

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/MMMULTISRP-151?atlOrigin=eyJpIjoiY2E4MzA0ZTg4NjlkNDExY2IyZDAyODFjNzIzNTk5MjYiLCJwIjoiaiJ9

Fixes: https://consensyssoftware.atlassian.net/browse/MMMULTISRP-147?atlOrigin=eyJpIjoiMjFhOWYxOTc1MTM5NGMwMmE0ZjBiYTk5YTQxNjAyNDEiLCJwIjoiaiJ9

Fixes:https://consensyssoftware.atlassian.net/browse/MMMULTISRP-146?atlOrigin=eyJpIjoiYjQyNDdhYjRmODgxNGQwMjhkNzRiOTgwNzAyY2U1Y2IiLCJwIjoiaiJ9

Manual testing steps

  1. Go to account menu
  2. Click on Secret Recovery Phrase
  3. See that the import button is always visible
  4. Click on 24 word option
  5. Fill in a 12 word srp and see that the import button is still disabled
  6. See that there is a button to change back to 12 word srp.

Screenshots/Recordings

Before

After

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.

Copy link
Contributor

github-actions bot commented Apr 7, 2025

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.

@metamaskbot
Copy link
Collaborator

✨ Files requiring CODEOWNER review ✨

🖥️ @MetaMask/wallet-ux

  • ui/components/multichain/multi-srp/import-srp/import-srp.test.tsx
  • ui/components/multichain/multi-srp/import-srp/import-srp.tsx
  • ui/components/multichain/multi-srp/import-srp/index.scss

@montelaidev montelaidev changed the title Fix/mmmultisrp 148 fix: import srp error handling and style Apr 8, 2025
@montelaidev montelaidev marked this pull request as ready for review April 8, 2025 12:50
@montelaidev montelaidev requested a review from a team as a code owner April 8, 2025 12:50
darioAnongba
darioAnongba previously approved these changes Apr 9, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [59fba05]
UI Startup Metrics (1217 ± 58 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1217111513785812621319
load10649641198551148993
domContentLoaded10589601191541144984
domInteractive17136081631
firstPaint797831178399224984
backgroundConnect97232910
firstReactRender20154762136
getState12533878
initialActions001001
loadScripts81271694753849899
setupStore8419378
WebpackHomeuiStartup21541725255716922712382
load16761241204514517591865
domContentLoaded16701236203314517551859
domInteractive171258101446
firstPaint158644506227084
backgroundConnect291177133757
firstReactRender189513721095997
getState1843534679
initialActions318146
loadScripts16611229201014317471849
setupStore22625028287
FirefoxBrowserifyHomeuiStartup13491165179711814161589
load12101041168011712621461
domContentLoaded12101041167911712621461
domInteractive9835188299098
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect24166182449
firstReactRender21195042126
getState7492979
initialActions001001
loadScripts11881024166211812391438
setupStore6433467
WebpackHomeuiStartup15661416188910216191783
load1344120716889714041524
domContentLoaded1344120716889714041523
domInteractive9959200278995
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect27185682847
firstReactRender36305853846
getState10432789
initialActions002111
loadScripts1320118516699713811504
setupStore9541689

darioAnongba
darioAnongba previously approved these changes Apr 9, 2025
width={BlockSize.Full}
marginTop={4}
paddingBottom={6}
backgroundColor={
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Can we add padding at the top of the button as well?

Current Example
current example

Copy link
Contributor

@vinnyhoward vinnyhoward left a comment

Choose a reason for hiding this comment

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

LGTM

@montelaidev montelaidev added this pull request to the merge queue Apr 9, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [dbae5ab]
UI Startup Metrics (1250 ± 61 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1250114614346112821374
load10919691252581137969
domContentLoaded10859561246581140971
domInteractive17136971732
firstPaint74181117642422981
backgroundConnect106182910
firstReactRender22146282540
getState155741178
initialActions001001
loadScripts83169995856860934
setupStore9525479
WebpackHomeuiStartup21911683266017122992418
load17271351209013317932020
domContentLoaded17171346208312717831995
domInteractive171267111451
firstPaint168644896724074
backgroundConnect4811417594365
firstReactRender168523481045691
getState1742972969
initialActions317135
loadScripts17061341207812517631962
setupStore27625343328
FirefoxBrowserifyHomeuiStartup18221531256320918962348
load16351338229820316992163
domContentLoaded16341338229720316982162
domInteractive145406658417386
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect4024104164279
firstReactRender26223532732
getState1562713589
initialActions001001
loadScripts15971309226020216672109
setupStore1152662689
WebpackHomeuiStartup15161370184611115871753
load1312116616229813901491
domContentLoaded1312116616219913901491
domInteractive9438199258794
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect25186682640
firstReactRender35285653844
getState8428389
initialActions001011
loadScripts1289114115969813651459
setupStore9551779

@montelaidev montelaidev added this pull request to the merge queue Apr 9, 2025
Teng-Web3 pushed a commit to Teng-Web3/metamask-extension-Teng that referenced this pull request Apr 9, 2025
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

This PR fixes the following:

1. The import button is now floating
2. Importing a 12 word srp when 24 word is selected is now blocked.
3. Add 12 word srp option. 

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
4. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31662?quickstart=1)

## **Related issues**

Fixes:
https://consensyssoftware.atlassian.net/browse/MMMULTISRP-151?atlOrigin=eyJpIjoiY2E4MzA0ZTg4NjlkNDExY2IyZDAyODFjNzIzNTk5MjYiLCJwIjoiaiJ9

Fixes:
https://consensyssoftware.atlassian.net/browse/MMMULTISRP-147?atlOrigin=eyJpIjoiMjFhOWYxOTc1MTM5NGMwMmE0ZjBiYTk5YTQxNjAyNDEiLCJwIjoiaiJ9


Fixes:https://consensyssoftware.atlassian.net/browse/MMMULTISRP-146?atlOrigin=eyJpIjoiYjQyNDdhYjRmODgxNGQwMjhkNzRiOTgwNzAyY2U1Y2IiLCJwIjoiaiJ9



## **Manual testing steps**

1. Go to account menu
2. Click on `Secret Recovery Phrase`
3. See that the import button is always visible 
4. Click on 24 word option
5. Fill in a 12 word srp and see that the import button is still
disabled
6. See that there is a button to change back to 12 word srp. 

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 9, 2025
@montelaidev montelaidev added this pull request to the merge queue Apr 10, 2025
Merged via the queue into main with commit d27e110 Apr 10, 2025
167 checks passed
@montelaidev montelaidev deleted the fix/mmmultisrp-148 branch April 10, 2025 01:00
@github-actions github-actions bot locked and limited conversation to collaborators Apr 10, 2025
@metamaskbot metamaskbot added the release-12.17.0 Issue or pull request that will be included in release 12.17.0 label Apr 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.17.0 Issue or pull request that will be included in release 12.17.0 team-accounts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants