-
Notifications
You must be signed in to change notification settings - Fork 70
Added aria-describedby atttribute to avfield to link error messages #260
base: master
Are you sure you want to change the base?
Conversation
src/AvField.js
Outdated
@@ -89,14 +89,14 @@ export default class AvField extends Component { | |||
size={size} | |||
disabled={disabled} | |||
readOnly={readOnly} | |||
{...attributes} | |||
{...{...attributes, ...{'aria-describedby':'errorMsg'}}} |
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.
I think we should change this to
aria-describedby="errorMsg"
{ ...attributes }
This way the user can override it if needed
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.
Addressed
src/AvField.js
Outdated
> | ||
{children} | ||
</AvInput>); | ||
|
||
const validation = this.context.FormCtrl.getInputState(this.props.name); | ||
|
||
const feedback = validation.errorMessage ? (<AvFeedback>{validation.errorMessage}</AvFeedback>) : null; | ||
const feedback = validation.errorMessage ? (<AvFeedback id="errorMsg">{validation.errorMessage}</AvFeedback>) : null; |
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.
We should make the id dynamic. If multiple fields have an error, then there will be more than one element with id="errorMsg"
.
We can generate it off the id for the field
<AvFeedback id={`${id}-error`} ...`
we should use this value with the aria-describedby
prop as well
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.
Addressed
src/AvField.js
Outdated
@@ -89,14 +89,15 @@ export default class AvField extends Component { | |||
size={size} | |||
disabled={disabled} | |||
readOnly={readOnly} | |||
aria-describedby = {`${id}-error`} |
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.
Let's also make this conditional using the same logic for AvFeedback
. We don't want to point to an element that doesn't exist.
Also, please remove the spaces around the =
to match the other props.
No description provided.