-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Signup: Desktop: Show signup in desktop when logged out #2813
Conversation
@@ -186,7 +186,7 @@ function boot() { | |||
analytics.setSuperProps( superProps ); | |||
|
|||
if ( config.isEnabled( 'oauth' ) ) { | |||
LoggedOutLayout = require( 'layout/logged-out-oauth' ); | |||
LoggedOutLayout = require( 'layout/logged-out' ); |
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.
We need to display two different layout styles before the user logs in, based on the whether or not the user is on /login
or /start
. The only way I can think to accomplish this is to update LoggedOutLayout
to display both styles based on whether or not props.section === 'signup'
.
This could be wrong, but I don't believe we can use two separate layouts here, because switching would wipe out #primary
, which is what we render the other faux-top-level React components into.
|
b92462d
to
85e49ca
Compare
1c5df71
to
a3b40c6
Compare
There are a few other issues we need to deal with here:
We might be able to split this into two PRs (one that enables signup in the desktop app, and another that obviates the full page reload at the end of signup) if we address just the third point above. What do you think? |
Actually, we could break this down one PR further, by updating Calypso to use the OAuth token, but still triggering a new page load after signup. This would allow us to make the changes to the layout/
|
a3b40c6
to
d04ed4c
Compare
WIP. This is just a very quick commit to demonstrate some changes needed to show the signup in the desktop app.
This will fix #2960 eventually.