-
Notifications
You must be signed in to change notification settings - Fork 74
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
Error messages aren't displaying on the remote container create form #2411
Error messages aren't displaying on the remote container create form #2411
Conversation
…e form Issue: AAH-1845 PR Checks PR checks
48501e2
to
e6d2343
Compare
|
||
return ( | ||
nameError && ( | ||
<Alert title={nameError.title} variant='danger' isInline> |
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.
so, these are eseentially validation errors, right?
We're usually using code like
<FormGroup
...
validated={this.toError(!('token' in errorMessages))}
helperTextInvalid={errorMessages['token']}
>
<TextInput
...
validated={this.toError(!('token' in errorMessages))}
with errorMessages: ErrorMessagesType
that comes from mapErrorMessages(error)
.
formErrors
is used here instead, that's probably fine (for now; also used in user-form, but nowhere else) but we should still be using helperTextInvalid
instead of an inline alert, unless there's an actual error
(this is also missing a screenshot, so I'm only guessing these are validation errors?)
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.
Ok, I will try to use your way.
@@ -207,6 +208,7 @@ export class RepositoryForm extends React.Component<IProps, IState> { | |||
onChange={(value) => this.setState({ name: value })} | |||
validated={this.validateName(name)} | |||
/> | |||
{this.renderNameError(formError)} |
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.
This should check for errors on other fields too, not just "name"
(replaced with #2601) |
https://issues.redhat.com/browse/AAH-1845
Messages were in fact displayed, but at the bottom of the form and many users dont have the bottom displayed without scrolling. Those errors could be easily missed.
This PR added the error message right under the name field.
And it also repairs another mistake - errors were not cleared when modal closed (on success or cancel).
Issue: AAH-1845