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 Initialize account banner on dashboard - Closes #1660 #1662

Merged
merged 11 commits into from Jan 22, 2019

Conversation

massao
Copy link
Contributor

@massao massao commented Jan 17, 2019

What issue have I solved?

#1660

How have I implemented/fixed it?

Created new Banner component that have title. children and footer props, title being an String, and children and footer being nodes.
Added the same conditions that we have on the wallet, for showing or not the banner for initializing the account.
The background and button are different from the design, to be more consistent with the layout.

How has this been tested?

The banner should not appear if the user is not logged in any account.
Create a new account, and transfer some LSK to it, or login into an account that has LSK and didn't activate the account yet, and check if the banner shows up on the /dashboard, after activating the account, the banner shouldn't appear again.

Review checklist

@massao massao self-assigned this Jan 17, 2019
@massao massao added this to Pull Requests in Version 1.10.0 via automation Jan 17, 2019
@slaweet
Copy link
Contributor

slaweet commented Jan 17, 2019

As discussed, some changes to the specification:

  • The "Create first transaction" button should skip the current first step of "Account initialization", as it was duplicate to the banner.
  • Second step of initializaiton should contain to link from the frist step
  • going to /wallet/send page (e.g. Send button on wallet page) should never show Account initialization.

This might require some e2e test to be updated.

slaweet
slaweet previously approved these changes Jan 18, 2019
Copy link
Contributor

@slaweet slaweet left a comment

Choose a reason for hiding this comment

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

Nice, Massao 👍

Copy link
Contributor

@Efefefef Efefefef left a comment

Choose a reason for hiding this comment

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

👍 Thanks Massao. Everything works
Regarding tests please check comments and tell me if you need further clarification or help

test/cypress/e2e/send.spec.js Show resolved Hide resolved
@massao massao merged commit 6fd10b7 into 1.10.0 Jan 22, 2019
Version 1.10.0 automation moved this from Pull Requests to Merged Pull Requests Jan 22, 2019
@massao massao deleted the 1660-add-initialization-banner-dashboard branch January 22, 2019 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Version 1.10.0
  
Merged Pull Requests
Development

Successfully merging this pull request may close these issues.

None yet

3 participants