-
Notifications
You must be signed in to change notification settings - Fork 2k
(fix password-validator) Simple Mistype #1156
Conversation
@@ -11,7 +11,7 @@ angular.module('users').factory('PasswordValidator', ['$window', | |||
return result; | |||
}, | |||
getPopoverMsg: function () { | |||
var popoverMsg = 'Please enter a passphrase or password with greater than 10 characters, numbers, lowercase, upppercase, and special characters.'; | |||
var popoverMsg = 'Please enter a passphrase or password with more than 10 characters, numbers, lowercase, uppercase, and special characters.'; |
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.
@lirantal @codydaig Why do this when result tells us the errors? I think the popoverMsg should be result.requiredTestErrors if possible.
https://www.npmjs.com/package/owasp-password-strength-test#in-browser-1
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.
@ilanbiala I think the point is to give the User a heads up on what the requirements are for the password. Rather than displaying the message on the page, I think this is a good way to convey this info to the User.
This doesn't have anything to do with the errors. The error result is handled through ng-messages, and displays on the page just like other form validation errors we have.
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.
Thanks for catching this @Meistercoach83! @mleanos is right, the popover was intended to give the user instructions for the password requirements when they click into the form field.
Fix the commit message, and I'll merge it in. |
@Meistercoach83 the actual commit message needs to be changed, not the title of the PR. See https://github.com/meanjs/mean/blob/master/CONTRIBUTING.md for what it should look like. |
@Meistercoach83 Can you change the commit message? |
Closes meanjs#1156 adjust language
No description provided.