fix(LoginPage): Improve inputs validation #936
Conversation
Ron-Lavi
force-pushed
the
fix/login-page-validation
branch
from
00c21df to
ec1ec6e
Compare
Ron-Lavi
changed the title
Ron-Lavi
force-pushed
the
fix/login-page-validation
branch
from
ec1ec6e to
ab9ec07
Compare
|
PatternFly-React preview: https://936-pr-patternfly-react-patternfly.surge.sh |
Pull Request Test Coverage Report for Build 3281
💛 - Coveralls |
| const { usernameField, passwordField } = this.state; | ||
| if (usernameField.value) { | ||
| !this.isUserNameValid() && this.handleOnInvalidUsername(); | ||
| !this.isEmailValid() && this.handleOnInvalidUsername(); |
There was a problem hiding this comment.
why? do you always assume that username is an email?
There was a problem hiding this comment.
Inside this function I validate only if it's an email, else return true
There was a problem hiding this comment.
what about that its not empty? e.g. I would expect the login btn to be disabled if there is no username / password or an ongoing authentication event in progress?
There was a problem hiding this comment.
The submit event will fire only if the form is valid,
Our validations currently covers:
- empty inputs errors
- capslock warning for passwords
- valid email
Your suggestion can be made by the consumer
by passing a disabled prop to the submitButton on submit event,
And it can be a great follow up PR.
| }; | ||
|
|
||
| isUserNameValid = () => { | ||
| isEmailValid = () => { |
There was a problem hiding this comment.
I think the method should remain as isUserNameValid, if follow-up PRs add more validation cases there would be no need to re-rename this method.
Ron-Lavi
force-pushed
the
fix/login-page-validation
branch
from
ab9ec07 to
4630769
Compare
7 participants
As part of the LoginPage implementation in Foreman there need to be some fixes in inputs validation.