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] Implement remaning UI for new verification flow #1584

Merged
merged 51 commits into from
Nov 8, 2019

Conversation

jmrossy
Copy link
Contributor

@jmrossy jmrossy commented Nov 5, 2019

Description

  • Add new icons
  • Implement verification interstitial screen
  • Implement verification input screen
  • Style tweaks
  • Add hitslop to paste / clear icons to make them easier to target

Still TODO: Tie the screens into the verification saga. Will do that in the next PR

Tested

Relevant screens tested locally on Android
still TODO test iOS, will do after done verification implementation

Related issues

Backwards compatibility

Yes

wa-ver-icons
wa-ver-interstitial
wa-ver-input

Use updated RN Geth to support hot reloading
…component

Clean up related fonts and styles
Implement VerificationEducationScreen
Implement skip modal in verification education screen
Implement most of verification loading screen
Remove unused dropdown modal package
@@ -182,12 +182,12 @@ export function messageContainsAttestationCode(message: string) {

export function extractAttestationCodeFromMessage(message: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i feel that we are a little inconsistent with throwing vs nulls - wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I'm generally in favor of exceptions but nulls were helpful here. Hbu WDYT?

@codecov
Copy link

codecov bot commented Nov 7, 2019

Codecov Report

Merging #1584 into master will increase coverage by 0.08%.
The diff coverage is 88.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1584      +/-   ##
==========================================
+ Coverage   74.16%   74.24%   +0.08%     
==========================================
  Files         287      287              
  Lines        7651     7703      +52     
  Branches      666      959     +293     
==========================================
+ Hits         5674     5719      +45     
- Misses       1864     1871       +7     
  Partials      113      113
Flag Coverage Δ
#mobile 74.24% <88.13%> (+0.08%) ⬆️
Impacted Files Coverage Δ
...es/mobile/src/verify/VerificationLoadingScreen.tsx 100% <100%> (ø) ⬆️
.../mobile/src/verify/VerificationLearnMoreScreen.tsx 100% <100%> (ø) ⬆️
.../mobile/src/verify/VerificationEducationScreen.tsx 88.23% <100%> (ø) ⬆️
packages/mobile/src/invite/EnterInviteCode.tsx 82.66% <100%> (ø) ⬆️
packages/mobile/src/components/Carousel.tsx 88.88% <100%> (ø) ⬆️
...bile/src/verify/VerificationInterstitialScreen.tsx 90.47% <83.33%> (-9.53%) ⬇️
...ages/mobile/src/verify/VerificationInputScreen.tsx 89.79% <86.48%> (-10.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58339a0...1067628. Read the comment docs.

Copy link
Contributor

@jeanregisser jeanregisser left a comment

Choose a reason for hiding this comment

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

Looking good 👍

<View style={styles.modalContainer}>
<View style={styles.modalTimerContainer}>
<LoadingSpinner />
<Text style={fontStyles.body}>{'0:49'}</Text>
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the real value will be added in the next PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, already done in my latest

@jmrossy jmrossy merged commit 2788391 into master Nov 8, 2019
@jmrossy jmrossy deleted the rossy/wa-ver-input branch November 8, 2019 15:17
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.

3 participants