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
Add/anchor gutenboarding signup page #50052
Conversation
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~534 bytes added 📈 [gzipped])
Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
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.
This LGTM! I left one comment but not a bloocker to merging I think.
onChange={ setEmailVal } | ||
placeholder={ __( 'Email address' ) } | ||
required | ||
autoFocus={ ! isMobile } // eslint-disable-line jsx-a11y/no-autofocus |
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.
Is there a reason we need to autofocus here and ignore the lint rule? It seems like an unnecessary skip of something we're presumably checking for for a reason. (https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/master/docs/rules/no-autofocus.md)
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.
That's a good question. Mostly, I am taking the current Gutenboarding sign up page and extending it, so that's coming from the current sign-up page here:
autoFocus={ ! isMobile } // eslint-disable-line jsx-a11y/no-autofocus |
I think I traced it back to first being added here: #39586 0cc43dad2c8
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.
Fair enough. I actually didn't notice it autofocusing when I loaded it anyway, so I'm not super sure this is doing anything and it could probably be taken out. But again, not a blocker.
We can go with just bullet points for launch (which would make it a little more readable I think).
Agreed – in general, we don't want to be changing the "canonical" form like that, and even if considering it, we're out of time before launch, yes. I'm wondering if the two sides of content can be vertically centered to their divider. |
Non-Anchor In the normal signup flow, I'm unable to log in; I get a redirect to this login screen after creating my account. The account creation screen: It starts to create my site, at which point I'd expect to be redirected to the editor, but then I get stuck on this page and can't sign in: I'm guessing this is a side effect of being on localhost instead of wordpress.com proper, or maybe incognito mode? 🤷♀️ Anchor I'm unable to see the two-column signup design in the Anchor flow, and I'm getting an error when I get to the site creation -> editor step. Here's the first screen in the flow: I signed up with a new emaill/password from there, then I run into an error when I get to the site creation step: |
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.
I wasn't able to see the design live, but assuming it's like the screenshots, it looks good. I agree with Kirk, we should use bullet points rather than custom icons. Vertically centering the content would also work, I think.
How does the two-column layout adapt on mobile devices or small screens?
d17ed13
to
8a2b0fd
Compare
That looks way better @mreishus ! In the single column, can we better balance the whitespace between the two sides, so that the first paragraph of bullet points isn't so squished up against the bottom of the signup form? @sixhours or @sfougnier Should the bullet points section come before the form? |
I'd say no, after is better in this case, since we don't want to lose folks' interest or make them guess where the form is with scrolling. I would also make sure the left margin is consistent for list items that wrap to two lines: I think you can also reduce the whitespace between the divider and the form a little for tablet-sized screens to un-squish the columns a bit (that's the technical designer term 😜 ) |
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.
Fantastic! 🎉 Ship it
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/5525606 Thank you @mreishus for including a screenshot in the description! This is really helpful for our translators. |
Translation for this Pull Request has now been finished. |
Changes proposed in this Pull Request
Testing instructions
(Non-Anchor)
(Anchor)
Needs Design Help / Design Review
Compare the screenshot above to the original mockup:
There are still some differences, so any design help would be appreciated. For example:
Related to 358-gh-Automattic/dotcom-manage