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

[Wallet] Restore Account Key screen redesign #4201

Merged
merged 8 commits into from
Jun 25, 2020

Conversation

i1skn
Copy link
Contributor

@i1skn i1skn commented Jun 24, 2020

Description

Redesigning Account Key screen in onboarding https://www.figma.com/file/zt7aTQ5wuXycIwxq5oAmF9/Wallet--Refresh?node-id=5404%3A440. I've straggled to make multiline input to work as expected on both platform, when placeholder is multiline, but no luck. So I've decided to use short placeholder for now and later come back to this problem. This means, that instead of full 24 words as a placeholder you would see only a first couple of them followed by ... (see Screenshots)

Tested

Android and iPhone

Related issues

Backwards compatibility

Yes

Screenshots

simulator
XPlONPN7yZEDpKct8IL2QBZpxRipauge
pp0WKxKQPjOIRLQ0BRxJqLXHhZipEAD1
vnQTSxr89UcVzJHl7IISIoZ0SpeTzQz4

@i1skn
Copy link
Contributor Author

i1skn commented Jun 24, 2020

@coreycelo please take a look on the temporary proposed design changes if that's ok, thanks!

Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

One optional comment but looks good to me.

<View style={styles.loadingSpinnerContainer} testID="ImportWalletLoadingCircle">
<ActivityIndicator size="large" color={colors.celoGreen} />
</View>
<HeaderHeightContext.Consumer>
Copy link
Contributor

Choose a reason for hiding this comment

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

Will other onboarding screen need these wrappers HeaderHeight and SafeArea wrappers? Wondering if we should just make a single wrapper component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, tbh I've just copied from what @jeanregisser has implemented. I guess we could created a wrapper out of it if need it more. @jeanregisser wdyt?

@coreycelo
Copy link

@coreycelo please take a look on the temporary proposed design changes if that's ok, thanks!

@i1skn Cool. This is something that will confuse some users so it'd be good to get it in eventually, but makes sense. In the second screenshot after the video, the word "medal" is underlined. Is that an Android text field feature or part of the app?

@i1skn
Copy link
Contributor Author

i1skn commented Jun 24, 2020

@coreycelo please take a look on the temporary proposed design changes if that's ok, thanks!

@i1skn Cool. This is something that will confuse some users so it'd be good to get it in eventually, but makes sense. In the second screenshot after the video, the word "medal" is underlined. Is that an Android text field feature or part of the app?

It's Android feature.

@i1skn i1skn added the automerge Have PR merge automatically when checks pass label Jun 25, 2020
@mergify mergify bot merged commit 1f1ca2c into master Jun 25, 2020
@mergify mergify bot deleted the i1skn/restore-key-redesign branch June 25, 2020 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants