react icon indicating copy to clipboard operation
react copied to clipboard

Should `FormControl.Validation` have `aria-live="polite"` by default?

Open iansan5653 opened this issue 2 years ago • 13 comments

When a user is using a screen reader to input data into a FormControl, they need to be notified of validation error messages. However, error messages within FormControl.Validation do not have aria-live set by default, so the screen reader may not read them until the user leaves the input.

I am not an expert in this area (I don't use a screen reader), so I'm not totally sure about this - just something that should maybe be considered.

iansan5653 avatar Feb 25 '22 15:02 iansan5653

cc @primer/accessibility 👀

inkblotty avatar Feb 25 '22 15:02 inkblotty

Thanks for the feedback @iansan5653

I think it makes sense to use aria-live="polite" to support inputs that validate as the user makes changes. I'd love to hear if somebody from the accessibility team has some feedback.

mperrotti avatar Feb 25 '22 16:02 mperrotti

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

github-actions[bot] avatar Aug 24 '22 17:08 github-actions[bot]

Bump

iansan5653 avatar Aug 24 '22 17:08 iansan5653

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

github-actions[bot] avatar Feb 20 '23 19:02 github-actions[bot]

🙃

iansan5653 avatar Feb 21 '23 14:02 iansan5653

Thanks for the patience on this one 🙏🏻

@TylerJDev when you have a moment, could you take a look at this proposal and let us know if you would advise moving forward with it?

lesliecdubs avatar Feb 22 '23 03:02 lesliecdubs

@iansan5653 👋 - Thank you for bringing this up! I totally understand where you're coming from in regards to the error being announced. Currently we utilize aria-describedby to link the input to the error warning. In my opinion, I typically see live regions utilized for when the errors are listed in bulk, (e.g. on submit, a group of errors found might be announced via a live region), whereas for inline errors like the ones in FormControl.Validation a live region might not be as needed. This is mainly because the errors should still be announced through that linkage, aria-describedby. The one con to the aria-live is that it might add some extra verbosity, mainly depending on when the error appears.

I think that the current implementation is in a decent state for the errors, though we might want to do some discovery on this topic though during this component's a11y review. I have not looked at this component too deeply, but would love to hear your thoughts and if you had any opinions on this? 🤔

TylerJDev avatar Feb 22 '23 18:02 TylerJDev

🤔 I guess I can see where it would make sense to not have aria-live if this is used in a traditional-style form where the errors only appear after clicking the submit button. I was picturing an 'as you type' validation event where the accessible description would not automatically be re-read because you are already focused on the input. In that case, aria-live is in fact necessary or the error will likely never be read. But maybe the existence of both cases means we shouldn't enable this by default after all.

iansan5653 avatar Feb 23 '23 15:02 iansan5653

@iansan5653 - I understand what you mean, there's definitely some discovery we'll need to do around this during the a11y engineer review for this component. Even if it's not by default, it might be a good thing to allow. I'd love to keep this issue open until the a11y engineer review is finished, if that's okay with you? I think this is something that we can definitely consider adding, even if it's not by default. Thanks a bunch for the input too!

TylerJDev avatar Feb 27 '23 14:02 TylerJDev

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

github-actions[bot] avatar Oct 08 '23 16:10 github-actions[bot]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

github-actions[bot] avatar Apr 06 '24 15:04 github-actions[bot]

I believe this PR should resolve this issue: https://github.com/primer/react/pull/4445. Essentially, we could rely on just the aria-describedby to link the errors, as most screen readers that we test with should announce the change, but this isn't exactly the case for VoiceOver in some browsers 🙃.

For now, we'll include the live region to ensure that the change in validation status (i.e. when there's an error) is announced to AT.

TylerJDev avatar Apr 08 '24 14:04 TylerJDev