Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

first run: skip account creation if they already have accounts #3827

Merged
merged 3 commits into from
Dec 15, 2016

Conversation

derhuerst
Copy link
Contributor

This PR addresses #3583 by adding a simple "skip" button to the first run modal, if the user has one or more accounts.

@derhuerst derhuerst added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Dec 12, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b980e7a on jr-first-run into ** on master**.

@gavofyork
Copy link
Contributor

they should click through the license whatever; skip button should only be there after that.

@derhuerst
Copy link
Contributor Author

@gavofyork It is the case. The "skip" button is shown in the "create an account" step.

];
if (hasAccounts) {
buttons.unshift(
<Button
Copy link
Contributor

Choose a reason for hiding this comment

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

Would have preferred the labels/hint/text changes into FormattedMessage while we are in the component. (Saves work later, especially when adding new features).

Not a crisis here though, so will just comment on it - small enough change overall.

@jacogr
Copy link
Contributor

jacogr commented Dec 13, 2016

Additional prop verification issues introduced:

vendor.js:6509 Warning: Failed prop type: Invalid prop `showFirstRun` of type `number` supplied to `Container`, expected `boolean`.
    in Container (created by Application)
    in Application (created by Connect(Application))
    in Connect(Application) (created by RouterContext)
    in RouterContext (created by Router)
    in Router (created by MainApplication)
    in MainApplication
    in IntlProvider (created by ContextProvider)
    in ContextProvider

&

vendor.js:6509 Warning: Failed prop type: Invalid prop `visible` of type `number` supplied to `FirstRun`, expected `boolean`.
    in FirstRun (created by Connect(FirstRun))
    in Connect(FirstRun) (created by Container)
    in div (created by ParityBackground)
    in ParityBackground (created by Connect(ParityBackground))
    in Connect(ParityBackground) (created by Container)
    in Container (created by Application)
    in div (created by Application)
    in Application (created by Connect(Application))
    in Connect(Application) (created by RouterContext)
    in RouterContext (created by Router)
    in Router (created by MainApplication)
    in MainApplication
    in IntlProvider (created by ContextProvider)
    in ContextProvider
    in AppContainer

@jacogr jacogr added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 13, 2016
@derhuerst
Copy link
Contributor Author

derhuerst commented Dec 13, 2016

Additional prop verification issues introduced:

This should only be because there's still 0/1 in your localStorage. After reload, this should be set to false/true and therefore gone.

Edit: can you confirm?

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling bbcae3d on jr-first-run into ** on master**.

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Dec 15, 2016
@jacogr
Copy link
Contributor

jacogr commented Dec 15, 2016

Yes, you are right, it is a once-off

@gavofyork gavofyork merged commit be0d5ca into master Dec 15, 2016
@gavofyork gavofyork deleted the jr-first-run branch December 15, 2016 12:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants