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

Components: Adds LoggedOutForm component #1521

Merged
merged 3 commits into from
Dec 15, 2015

Conversation

ebinnion
Copy link
Contributor

In an effort to factor the logged out link that shows at the bottom of the form at /start/account/user and to allow reuse of the logged out form styles, this PR refactors the signup/logged-out-form component and brings much of its functionality to client/components/logged-out-form-container component.

cc @mtias and @rickybanister for design review, @lezama, and @drewblaisdell for code review.

To test:

  • Checkout add/components-logged-out-form branch
  • Test invite flows
    • On a WP.com site, invite a test user by going to $site/wp-admin/users.php?page=wpcom-invite-users
    • In the invite email, make note of the invitation key
    • Go to /accept-invite/$site/$invite_key
    • While logged in or out, can you accept an invite? Are there errors? Does everything look OK?
  • Test user creation
    • Go to /start/account/user
    • Attempt to create a user
    • Does everything work and look good?
  • Test site creation
    • Go to /start/site-user/site
    • Does everything look ok? Are there any errors?

Note: You'll notice that the /log-in form is off centered, and during testing you will likely run into a JS error if you try to submit the form on /log-in. This is not related to this PR and /log-in is only enabled in development.

Also note: In some screenshots you'll notice there's quite a bit of padding between the form footer and the input directly above it. This seems to be caused by:

.validation-fieldset + .logged-out-form__footer {
    margin-top: 20px;
}

I intentionally added a screenshot with an invalid form field so that you could see what that would look like.

After screenshots:
screen shot
screen shot 1
screen shot 2
screen shot 3
screen shot 4
screen shot

@ebinnion ebinnion added Framework [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Feature] Signup & Account Creation All screens and flows for making a new WordPress.com account. People Management labels Dec 11, 2015
@ebinnion ebinnion self-assigned this Dec 11, 2015
@ebinnion ebinnion added this to the People Management: m6 milestone Dec 11, 2015
import Card from 'components/card';

module.exports = React.createClass( {
displayName: 'LoggedOutFormFooter',
Copy link
Member

Choose a reason for hiding this comment

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

missing propTypes: {} The same is true for the rest of the new components.

@enejb
Copy link
Member

enejb commented Dec 11, 2015

I left some minor comments. Overall in my testing I didn't find any errors beside the once that you pointed out already. I was able to accept the invitation + create a new user and a new site.

@enejb enejb added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 11, 2015
@ebinnion
Copy link
Contributor Author

@enejb I went ahead and ES6-ified and added propTypes to address your comments.

@ebinnion ebinnion added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Author Reply labels Dec 11, 2015
@ebinnion
Copy link
Contributor Author

Added missing border-top to the form footer.

screen shot

@ebinnion ebinnion force-pushed the add/components-logged-out-form branch from c95d99c to f94e09a Compare December 12, 2015 00:19
@rickybanister
Copy link

What's the email-address-only 'log me in' form for?

The footer looks good in all the examples. This seems like a useful refactor.

@ebinnion
Copy link
Contributor Author

@rickybanister I'm not entirely sure. I think it was an attempt at mobile login, but it hasn't launched yet.

The idea seems to be that you enter your email, and then you'll receive a one time pin which you can enter in the next step. But, the form was broken against master last time I tested.

@lezama
Copy link
Contributor

lezama commented Dec 15, 2015

Tested and everything seems to work including /start 👍
This looks much cleaner and ready to go to me :)

@lezama lezama added [Status] Ready to Merge and removed [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 15, 2015
ebinnion added a commit that referenced this pull request Dec 15, 2015
@ebinnion ebinnion merged commit e2e2214 into master Dec 15, 2015
@ebinnion ebinnion deleted the add/components-logged-out-form branch December 15, 2015 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Signup & Account Creation All screens and flows for making a new WordPress.com account. Framework People Management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants