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 most of verification loading screen and skip modal #1566

Merged
merged 33 commits into from
Nov 7, 2019

Conversation

jmrossy
Copy link
Contributor

@jmrossy jmrossy commented Nov 1, 2019

Description

  • Add Lottie package and new loading animation
  • Implement skip modal in verification education screen
  • Implement most of verification loading screen
  • Remove unused dropdown modal package

Tested

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

Related issues

Backwards compatibility

Yes

wa-verloading
wa-ver-skip-modal

@codecov
Copy link

codecov bot commented Nov 5, 2019

Codecov Report

Merging #1566 into master will increase coverage by 0.08%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1566      +/-   ##
==========================================
+ Coverage   73.86%   73.94%   +0.08%     
==========================================
  Files         283      284       +1     
  Lines        7537     7565      +28     
  Branches      954      955       +1     
==========================================
+ Hits         5567     5594      +27     
- Misses       1858     1859       +1     
  Partials      112      112
Flag Coverage Δ
#mobile 73.94% <94.44%> (+0.08%) ⬆️
Impacted Files Coverage Δ
packages/mobile/src/navigator/Headers.tsx 100% <100%> (ø) ⬆️
packages/mobile/src/components/Carousel.tsx 88.88% <100%> (ø) ⬆️
...es/mobile/src/verify/VerificationLoadingScreen.tsx 100% <100%> (ø) ⬆️
packages/mobile/src/icons/LoadingSpinner.tsx 100% <100%> (ø)
.../mobile/src/verify/VerificationEducationScreen.tsx 88.23% <81.81%> (-0.23%) ⬇️
packages/mobile/src/localCurrency/consts.ts 100% <0%> (ø) ⬆️

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 75cd910...68086d5. Read the comment docs.

@@ -389,8 +392,6 @@ PODS:
- React
- RNPermissions (2.0.2):
- React
- RNRandomBytes (3.0.0):
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why this is being removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No but we don't seem to use it. Not sure why my cocopods removed it when Jean's didn't

@@ -99,8 +101,7 @@
"react-native-keyboard-aware-scroll-view": "^0.9.1",
"react-native-localize": "^1.3.0",
"react-native-mail": "^4.0.0",
"react-native-modal": "^11.4.0",
"react-native-modal-dropdown": "^0.7.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

packages/mobile/src/components/Carousel.tsx Outdated Show resolved Hide resolved
expect(tree).toMatchSnapshot()
expect(toJSON()).toMatchSnapshot()

// And snapshot again after showing the modal
Copy link
Contributor

Choose a reason for hiding this comment

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

🙏

class VerificationLoadingScreen extends React.Component<Props> {
static navigationOptions = { header: null }

onCancel = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

what should happen here?

Copy link
Contributor Author

@jmrossy jmrossy Nov 5, 2019

Choose a reason for hiding this comment

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

We cancel the verification saga and go back to the education screen. Will do in next PR

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.

Great 👍

Just need to address the Podfile.lock issue.

packages/mobile/ios/Podfile.lock Outdated Show resolved Hide resolved
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.

All good! Ship it 👍

@jmrossy jmrossy merged commit 466958c into master Nov 7, 2019
@jmrossy jmrossy deleted the rossy/wa-ver-loading branch November 7, 2019 13:42
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.

6 participants