-
Notifications
You must be signed in to change notification settings - Fork 974
Make welcome screen show up on first time run #9480
Conversation
Moving to 0.18.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the manual test plan works.
@cezaraugusto could you rebase please? I'll def review / merge after that 😄 thanks! |
ok rebased |
Auditors: @bsclifton Close #9423
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look great! 😄
I updated the tests- If everything looks good, please go ahead and merge. Thanks! 😄 👍
app/browser/tabs.js
Outdated
@@ -673,6 +678,19 @@ const api = { | |||
} | |||
if (!createProperties.url) { | |||
createProperties.url = newFrameUrl() | |||
// don't open welcome screen for general tests | |||
if (process.env.NODE_ENV === 'test') { | |||
shouldShowWelcomeScreen = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be cleaner to set this above, when the value is gotten from appState. Something like:
const shouldShowWelcomeScreen = process.env.NODE_ENV !== 'test' &&
appState.getIn(['about', 'welcome', 'showOnLoad'])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually... nevermind, I have a solution for this 😄
@bsclifton awesome thanks for updating, looks great |
Make welcome screen show up on first time run
Follow up of Welcome page #9480
Follow up of Welcome page #9480
Follow up of Welcome page #9480
Follow up of Welcome page #9480
Submitter Checklist:
git rebase -i
to squash commits (if needed).Auditors: @bsclifton
Close #9423
Test Plan:
Manual test plan:
Reviewer Checklist:
Tests