-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add import seed phrase. Closes #1727 #1742
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I did not test the new functionality, though.
I think a UI with a password input that does not show the secret phrase would provide a better UX than asking the user to copy the seed phrase to the clipboard. It's a small improvement that may not be worth the effort required to implement it.
const button = showDialogSync({ | ||
title: 'Import Seed Phrase', | ||
// eslint-disable-next-line max-len | ||
message: 'The seed phrase is used in order to back up your wallet, or move it to a different machine. Please copy it to your clipboard before proceeding. Please be cautious, as this will overwrite the seed phrase currently used, which will be permanently lost.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like @patrickwoodhead to review this message.
Do we need to explain what is the seed phrase? I would expect the user to understand that if they decide to import the seed phrase,
I think it's also best to put the call to action as the last sentence in the message.
Should we also tell the user that they may want to export the current seed phrase?
message: 'The seed phrase is used in order to back up your wallet, or move it to a different machine. Please copy it to your clipboard before proceeding. Please be cautious, as this will overwrite the seed phrase currently used, which will be permanently lost.', | |
message: 'Please be cautious. This action will overwrite the seed phrase currently used, which will be permanently lost. Please copy the seed phrase to import to your clipboard before proceeding.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, we're also not explaining what a seed phrase is in the export section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your suggestion it says Please copy the seed phrase to import
, but I think it needs to be Please copye the seed phrase to your clipboard, to import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to moving the CTA to the end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree that we need to ask the user to be cautious as the first thing. Also, I'm a bit unclear what the CTA is asking the user to do. Is it asking them to copy their current seed phrase out, or copy their new one into the clipboard so that they can paste it into the input box?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The user needs to have their current seed phrase copied in their clipboard, from which we will then import it
An input field will require styling, which we don't yet have for this element in this location. It will also require a form, a button, with states, etc, which we don't have here. I agree that that would be nicer though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hang on, this isn't going to work. When we change the wallet private key, we will derive a different passphrase for Station Core and it won't be able to decrypt the file storing the Station ID.
See #1726
|
See #1726 (comment) |
Closes #1727