-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Enforce strong passwords in UI #1266
Conversation
f050ce1
to
a72d6ae
Compare
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.
Very nice! Tested and working great
If we don't allow either level 1 or level 2, should we keep the error state the same between both levels? ... Or is there more value in showing users that their password has gotten more secure but still isn't enough? Currently leaning towards what we have already, but still worth a question. |
I for one like the incremental changes as a way to know you're on the right track |
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.
Minor changes, looks great! :D
@SuaYoo I just realized we'll want to add this new password validation to the Account Settings password change flow and the reset a password from a "forgot password" email flow as well |
Can you clarify what you mean? This is implemented in the account settings and reset password, see screenshots! Edit: oh bummer, I see that my manual testing steps got truncated. |
Co-authored-by: Henry Wilkinson <[email protected]>
Oh good, then never mind me! :) |
Resolves #1233
Changes
Out-of-scope change: made name required so that reorder makes sense, and also so that we don't have to deal with displaying long email addresses in the UI. cc @Shrinks99 @ikreymer
Manual testing
Test password input works as expected on following pages:
Screenshots