Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

Change mnemonic save screen to be similar to MetaMask. #179

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

melatron
Copy link
Contributor

No description provided.

@melatron melatron requested a review from shelbyd April 11, 2022 17:59
setMnemonicArr((mnemonic as string).split(" "));
const sortedMnemonic = (mnemonic as string).split(" ").sort();
setMnemonic((await getProtocolOptIn().getMnemonic()) as string);
const sortedMnemonic = mnemonic.split(" ").sort();
Copy link
Contributor

Choose a reason for hiding this comment

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

The mnemonic value in here won't be updated from the setMnemonic call on the previous line. If you override the variable with the result of the getMnemonic call, then you won't need to run this effect multiple times.

Comment on lines 85 to 90
setPageOverride(
<SetupSuccess
mnemonic={mnemonic as string}
onContinue={() => setPageOverride(null)}
/>,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't have this component do the first showing of the SetupSuccess screen. I would have this only show that if the user clicks to see the mnemonic again or fails.


const [counter, setCounter] = React.useState(0);
export function EnsureUserSavedMnemonic(props: { onComplete: () => void }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the logic for this component is getting complex enough to warrant extracting a (unit tested) class to manage this state. Could either be functional (as in returns a new instance each time a mutation happens) or could have a subscription for when mutations happen that triggers a re-render.

Copy link
Contributor

@shelbyd shelbyd left a comment

Choose a reason for hiding this comment

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

Misclicked.

@melatron melatron requested a review from shelbyd May 16, 2022 13:27
@shelbyd shelbyd removed their request for review October 6, 2022 01:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants