Skip to content
This repository was archived by the owner on Feb 21, 2023. It is now read-only.

Conversation

@PavanGM23
Copy link

No description provided.

src/AvField.js Outdated
disabled={disabled}
readOnly={readOnly}
{...attributes}
{...{...attributes, ...{'aria-describedby':'errorMsg'}}}
Copy link
Member

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

Copy link
Author

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
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;
Copy link
Member

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

Copy link
Author

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
size={size}
disabled={disabled}
readOnly={readOnly}
aria-describedby = {`${id}-error`}
Copy link
Member

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants