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.

Sign up for GitHub

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

Jump to bottom

The Sign In Page and Input Consistency #1169

Merged
merged 16 commits into from
May 2, 2019
Merged

The Sign In Page and Input Consistency #1169

merged 16 commits into from
May 2, 2019

Conversation

supernova-at
Copy link
Contributor

@supernova-at supernova-at commented Apr 25, 2019 •
edited
Loading

Description

This PR:

  • Removes the old Input component
  • Updates the two usages of the Input component (Sign In and Create Account page) to use the TextInput component instead
  • Removes the ErrorDisplay component
  • Updates the two usages of the ErrorDisplay component (Sign In and Create Account page) to use less descriptive, simple error messages instead
  • Updates the Sign In page to match its mocks
  • Adds a "Signing In" LoadingIndicator to the Sign In page
  • Minor updates for the Create Account page to match its mocks (833 refactor the create account form #835 to follow)
  • Minor updates for the Forgot Password page to match its mocks
  • Refactors the user store slice for consistency for signIn and getDetails actions
  • Updates unit tests

Related Issue

Closes #912 .

Verification Steps

The Sign In Page

Get to this page by:

  1. Top Left Hamburger Menu
  2. Click the "Sign In" button at the bottom of the slide out

Matches the Mocks

  1. Ensure that the screen you see matches the desired mocks: https://magento.invisionapp.com/share/WQN5F7BYBPG#/screens/318790635.

Email is Required

  1. Ensure that the Email field has a dot indicating it is a required field
  2. Ensure that you are presented with an error message if you exit the field without entering any information:

Screen Shot 2019-04-25 at 3 04 34 PM

Password is Required

  1. Ensure that the Password field has a dot indicating it is a required field
  2. Ensure that you are presented with an error message if you exit the field without entering any information:

Screen Shot 2019-04-25 at 3 05 40 PM

Seeding the Forgot Password Page

  1. Enter a value into the Email field
  2. Click the "Forgot Password" button
  3. See that the value entered in step 1 appears in the "Email Address" field on the Forgot Password page
  4. Go back to the Sign In page

Seeding the Create Account Page

  1. Enter a value into the Email field
  2. Click the "Create Account" button
  3. See that the value entered in step 1 appears in the "Email" field on the Create Account page
  4. Go back to the Sign In page

Successful Sign In

  1. Enter a valid Email / Password
  2. Click the "Sign In" button
  3. See that a "Signing In" loading indicator appears while the request is in flight
  4. See that the "Signing In" loading indicator disappears when the request finishes
  5. See that you are successfully signed in

The Create Account Page

  1. Load up the Create Account Page
  2. See that the fields that are required have the required field indicator (black circle) next to them
  3. See that a "Subscribe to News and Updates" checkbox appears
  4. See that you can successfully create a new account

📝 This page doesn't fully match its mock (https://magento.invisionapp.com/share/WQN5F7BYBPG#/screens/320064949) because PR #835 addresses a lot of that work and I didn't want to duplicate effort.

The Forgot Password Page

  1. Load up the Forgot Password page
  2. See that the input text area is no longer the full width of the screen (it looks like the other mocks)

Signing In and Out

  1. See that you can successfully go back and forth between signed in and signed out

How Have YOU Tested this?

  1. The verification steps above
  2. yarn test

Screenshots / Screen Captures (if appropriate):

Proposed Labels for Change Type/Package

  • major (e.g x.0.0 - a breaking change)
  • minor (e.g 0.x.0 - a backwards compatible addition)
  • patch (e.g 0.0.x - a bug fix)

I removed two components that are part of our public API. This necessitates a major version change.

Checklist:

  • I have updated the documentation accordingly, if necessary.
  • I have added tests to cover my changes, if necessary.

@supernova-at
Updates usages of Input to TextInput.
de8245a 
* Affects Sign In page  and Forgot Password Page.
@supernova-at
Updates Field component to accept required property.
70af6ac 
* Field component will render the required indicator if the field is required.
@supernova-at
Removes ErrorDisplay component.
6a23bbe 
* For security reasons, we purposefully provide less informative error messages.
@supernova-at
Sign In and Create Account refactors.
165b306 
* Refactors User Redux / Action / Reducer for consistency
* Updates Sign In page to match the mocks
* Updates Create Account page to almost match the mocks. More updates coming in 835.
* Splits user sign in and user get details
* Updates UserInformation to show a loading message while loading user details
* Updates Sign In page to show a loading progress spinner while signing in
@supernova-at
Updates Unit Tests & Runs prettier
f96beb3
@supernova-at
Runs linter
976eddc
@vercel
Copy link

vercel bot commented Apr 25, 2019 •
edited
Loading

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@coveralls
Copy link

coveralls commented Apr 25, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.4%) to 76.63% when pulling 2ffb56b on supernova/912-input_consistency into ec278d3 on develop.

@supernova-at supernova-at added the version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump. label Apr 25, 2019
@supernova-at supernova-at mentioned this pull request Apr 25, 2019
Closed
8 tasks
jimbo
jimbo reviewed Apr 26, 2019
View reviewed changes
Copy link
Contributor

@jimbo jimbo left a comment

Choose a reason for hiding this comment

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

@supernova-at I ran through the code and everything looks very good. Great work on this. 👍

I'll approve this once I've tested it out locally.

packages/venia-concept/src/components/MyAccountMenuPage/UserInformation/userInformation.js Outdated Show resolved Hide resolved
packages/venia-concept/src/components/CreateAccount/createAccount.js
field="customer.email"
autoComplete="email"
validate={validators.get('email')}
asyncValidate={asyncValidators.get('email')}
Copy link
Contributor

Choose a reason for hiding this comment

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

In the event that #1062 gets merged before this PR, we'll have to remove asyncValidate and any associated code, since informed v2.x doesn't support it anymore.

@supernova-at supernova-at mentioned this pull request May 1, 2019
Merged
5 tasks
jimbo
jimbo previously approved these changes May 1, 2019
View reviewed changes
@dpatil-magento dpatil-magento self-assigned this May 2, 2019
@vercel vercel bot temporarily deployed to staging May 2, 2019 16:00 Inactive
@dpatil-magento dpatil-magento merged commit ec831b4 into develop May 2, 2019
@supernova-at supernova-at deleted the supernova/912-input_consistency branch July 25, 2019 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jimbo jimbo jimbo approved these changes

Assignees

@dpatil-magento dpatil-magento

Labels
version: Major This changeset includes incompatible API changes and its release necessitates a Major version bump.
Projects
None yet
Milestone
Next PWA Release (Core 2.3.2)
Development

Successfully merging this pull request may close these issues.

[BUG] Create Account form input style is different from Sign In
5 participants
@supernova-at @coveralls @jimbo @dpatil-magento @cherdman