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

Set password #3190

Merged
merged 11 commits into from
Jun 12, 2021
Merged

Set password #3190

merged 11 commits into from
Jun 12, 2021

Conversation

anthony-hull
Copy link
Contributor

@anthony-hull anthony-hull commented May 27, 2021

Details

Create a new UX around setting a password during account signup.
Giving feedback to the user before submission on:
password complexity requirements
passwords matching

Fixed Issues

Fixes #1517

QA Steps

To get to this page:
trigger password reset
make sure you're logged out
navigate to here: http://localhost:8080/setpassword/[USERID]/[RESETCODE]
make sure you have credentials.login key set to your email address in local storage

Then follow the video:
https://user-images.githubusercontent.com/47184118/120825774-a23f1c80-c551-11eb-9614-a5cf2f00414e.mp4

Testing the form

Try to break the form by different combinations e.g. put in an invalid password, then replicate that in the next box. Then make the first password valid and see if the red hint turns green. And see if the passwords match error disappears.

  • You shouldn't be able to submit the form if the passwords don't match or the password doesn't meet the complexity requirements.
  • The hint should only turn red if an invalid password is entered and the focus leaves the input. After this condition has been met it should give real time feedback if this requirement is met or not.
  • The hint should turn green when a valid password is entered.
  • The passwords should match error should only show if the confirm box input loses focus while the passwords don't match. After this condition has been met it should give real time feedback if this requirement is met or not.

Testing submission:

When pressing the Set Password button:
If you use an invalid password or they don't match the set password button shouldn't do anything.
If you use an expired code the Validated code is likely expired error should shot.
If you used a password that meets the complexity then you should be logged into the app and your password will have been set/changed

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Mobile Web

Desktop

iOS

Android

@github-actions
Copy link
Contributor

github-actions bot commented May 27, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@anthony-hull
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@anthony-hull
Copy link
Contributor Author

@thienlnam
@shawnborton

Hey I've made a draft PR here so we can discuss the UX better.

@shawnborton
Copy link
Contributor

Mind sharing a quick screen capture of where we're at with this?

@anthony-hull
Copy link
Contributor Author

Expensify.cash.mp4

@shawnborton
Copy link
Contributor

Nice, this is looking pretty good! After you correctly fulfilled the password requirements at 0:11 of your video, I think the hint text below the input should not be red any more:
image

Then at 0:25 of the video, I would think that the Passwords must match error message would remain there until the passwords finally do match up - right now you have it so that the error goes away when you focus back into the field, but the error technically still remains.

@anthony-hull
Copy link
Contributor Author

Expensify.cash.mp4

I have made it so the match error only appears after the user has onblurred once on the confirm box with some input in there. The error will disappear and appear based on the passwords matching after that.

The hint text of the enter a password box now turns green once the password requirements are met.

@anthony-hull anthony-hull marked this pull request as ready for review June 4, 2021 17:46
@anthony-hull anthony-hull requested a review from a team as a code owner June 4, 2021 17:46
@anthony-hull anthony-hull marked this pull request as draft June 4, 2021 17:46
@MelvinBot MelvinBot requested review from sketchydroide and removed request for a team June 4, 2021 17:46
@anthony-hull
Copy link
Contributor Author

sorry @sketchydroide I didn't mean to make this a full PR

@sketchydroide
Copy link
Contributor

no worries let me know when it's ready

@anthony-hull
Copy link
Contributor Author

sorry @shawnborton I missed your emoji reaction. I was waiting on feedback :)
I'll progress this to a full PR today

@anthony-hull anthony-hull marked this pull request as ready for review June 8, 2021 15:14
Copy link
Contributor

@thienlnam thienlnam left a comment

Choose a reason for hiding this comment

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

Nice work! These changes are looking good, just a few comments about better variable names and styling

src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/SetPasswordPage.js Outdated Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
Copy link
Contributor

@thienlnam thienlnam left a comment

Choose a reason for hiding this comment

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

Nice! Naming nomenclature and styling is looking much better - just a few more comments

src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
src/pages/SetPasswordPage.js Show resolved Hide resolved
src/pages/settings/NewPassword.js Outdated Show resolved Hide resolved
Copy link
Contributor

@thienlnam thienlnam left a comment

Choose a reason for hiding this comment

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

Changes look good, just looks like this needs some QA steps and this is good to go

@thienlnam thienlnam merged commit 05aedfe into Expensify:main Jun 12, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging in version: 1.0.68-5🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production in version: 1.0.73-3🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Pay on 6/18] When setting an unacceptable password the error message doesn't fit user's expectation
5 participants