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

Implement wallet on-boarding - Closes #2163 #2311

Merged
merged 7 commits into from Aug 5, 2019

Conversation

@slaweet
Copy link
Member

commented Aug 2, 2019

What issue have I solved?

#2163

How have I implemented/fixed it?

  • Created onboarding using the same component we use in delegates and dashboard
  • Remove old wallet "New account" banner. I'm not sure if this was expected, it's not part of acceptance criteria in #2163, but it's also not in the designs https://projects.invisionapp.com/d/main#/console/17570736/365248394/preview and it looks weird with both.
  • I didn't put the dots at the end of headlines (e.g. "Manage your LSK with ease") even though they are in the designs because delegates and dashboard also don't use them.

How has this been tested?

  • Log in
  • Go to wallet
  • See the onboarding, click through it, close it...

Review checklist

@slaweet slaweet self-assigned this Aug 2, 2019

@slaweet slaweet added this to Pull Requests in Version 1.20.0 via automation Aug 2, 2019

slaweet added some commits Aug 2, 2019

@slaweet slaweet force-pushed the 2163-wallet-onboarding branch from dbebf16 to acc0e72 Aug 2, 2019

@slaweet

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

@reyraa please confirm if acc0e72 was expected. It's not part of acceptance criteria in #2163, but it's also not in the designs https://projects.invisionapp.com/d/main#/console/17570736/365248394/preview and it looks weird with both.

@reyraa

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

@slaweet It’s removed deliberately. so you did ok.

@slaweet slaweet requested a review from osvaldovega Aug 2, 2019

@osvaldovega
Copy link
Contributor

left a comment

@slaweet I checked the code and looks good, but when I see the Onboarding there are just 2 things that are different from design.

The first one is that in the first page the button it is smaller and should be bigger, (widht)
This is the image from your branch
Screen Shot 2019-08-02 at 17 35 40

Then you can check the design and see that is bigger.

And the second thing is the Ilustration in the second slide should be centered it and but is at the button, check this screenshot, this is from your branch.

Screen Shot 2019-08-02 at 17 35 48

Then when you check the design you will noticed that is aligned to the center.

This is how looks in the design
Screen Shot 2019-08-02 at 17 40 38

slaweet added some commits Aug 5, 2019

@slaweet

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2019

I posted a comment about it to the designs. I also posted a few more comments there about other inconsistencies I found in the design: https://projects.invisionapp.com/d/main#/console/17570736/365248393/comments

@slaweet slaweet requested review from osvaldovega and massao Aug 5, 2019

@massao

massao approved these changes Aug 5, 2019

Copy link
Contributor

left a comment

Nice job Vit! 👍

@slaweet slaweet requested a review from Efefefef Aug 5, 2019

Fixed and approved by @massao

@slaweet slaweet removed the request for review from osvaldovega Aug 5, 2019

@Efefefef
Copy link
Contributor

left a comment

👍

@Efefefef Efefefef added the ready label Aug 5, 2019

@slaweet slaweet merged commit 9613a21 into development Aug 5, 2019

4 checks passed

Jenkins e2e tests e2e tests passed
Details
Jenkins test deployment Commit was deployed to test
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
coverage/coveralls Coverage remained the same at 94.773%
Details

Version 1.20.0 automation moved this from Pull Requests to Merged Pull Requests Aug 5, 2019

@slaweet slaweet deleted the 2163-wallet-onboarding branch Aug 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.