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.

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

Merged
merged 5 commits into from
Sep 4, 2023
Merged

Conversation

Jon-edge
Copy link
Collaborator

@Jon-edge Jon-edge commented Sep 2, 2023

image image

CHANGELOG

  • changed: Retheme wallet activation scene

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

  • Tested on iOS device
  • Tested on Android device
  • Tested on small-screen device (iPod Touch)
  • Tested on large-screen device (tablet)

Copy link
Contributor

@swansontec swansontec left a 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.

Comment on lines 84 to 91
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
})
Copy link
Contributor

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.
@swansontec swansontec merged commit a6bbd8f into develop Sep 4, 2023
2 checks passed
@swansontec swansontec deleted the jon/hbar-activation-ui2 branch September 4, 2023 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants