-
Notifications
You must be signed in to change notification settings - Fork 258
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
Retheme CreateWalletAccountSelectScene #4429
Conversation
c31024c
to
cd5bb2c
Compare
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.
This looks much nicer, both in terms of code and in terms of visuals.
We eventually need to clear out the redux state involved in this scene, but that can be a project for another day. For now this is much closer to the way it should be.
let createdWallet: Promise<EdgeCurrencyWallet> | ||
if (existingCoreWallet != null) { | ||
createdWallet = renameAndReturnWallet(existingCoreWallet) | ||
} else { | ||
createdWallet = dispatch(createCurrencyWallet(accountName, selectedWalletType.walletType, fixFiatCurrencyCode(selectedFiat.value))).then(wallet => { | ||
setIsCreatingWallet(false) | ||
return wallet | ||
}) |
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.
We can't have this logic at the top level of the component (maybe it needs useEffect
or something). Maybe it's fine for now, though, since I see this code gets deleted in a later commit.
This value is always set in the current usage of these scenes
The scene has been updated to make it obvious what wallet is being activated with icons and wallet name. Spitting out the info of the wallet to be activated including the fiat type in the ugly text block is no longer necessary.
cd5bb2c
to
a6fe8bb
Compare
CHANGELOG
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have: