h icon indicating copy to clipboard operation
h copied to clipboard

Improve accessibility of create/edit group pages

Open seanh opened this issue 1 year ago • 4 comments

seanh avatar Aug 20 '24 17:08 seanh

This draft PR just adds a couple of ARIA attrs. I think there's more that can be done but I haven't read up on ARIA yet. Some suggestions from GitHub Copilot (I haven't learned the ARIA/accessibility stuff here so can't yet make a judgement about the quality of these suggestions):

  • Use aria-label or aria-labelledby for form controls
  • Use aria-required for required fields (may be something to implement in the pattern library rather than here)
  • Put aria-busy on the form while in loading state
  • Add role="img" and aria-label to the <CancelIcon /> and <CheckIcon /> components (again may be something we want to implement in the pattern library rather than here)
  • When showing an error message use tabindex to move the keyboard focus to the error message to ensure that it gets read out by screen readers

seanh avatar Aug 20 '24 17:08 seanh

Use aria-label or aria-labelledby for form controls Use aria-required for required fields (may be something to implement in the pattern library rather than here)

This should be handled already, but I'll verify.

Put aria-busy on the form while in loading state

Yes, this will be needed.

Edit: Maybe not. Although it sounds relevant, aria-busy is actually a way of suppressing screen reader notifications. See https://www.tpgi.com/short-note-on-being-busy/.

Add role="img" and aria-label to the <CancelIcon /> and <CheckIcon /> components (again may be something we want to implement in the pattern library rather than here)

A label ("Changes saved") or similar will be need here.

When showing an error message use tabindex to move the keyboard focus to the error message to ensure that it gets read out by screen readers

The way we settled on handling this is to make the error message a description for the field using aria-describedby. The description is then read out when the field itself is focused. The field itself does need to be focused after any async validation error happens for this to be read out.

robertknight avatar Aug 21 '24 11:08 robertknight

Put aria-busy on the form while in loading state Add role="img" and aria-label to the <CancelIcon /> and <CheckIcon /> components (again may be something we want to implement in the pattern library rather than here)

This was handled in https://github.com/hypothesis/h/pull/8892, which created a component that will likely be moved to the component library in future.

robertknight avatar Aug 22 '24 14:08 robertknight

I'm going to leave this PR open to track remaining work. Some notes from testing different browsers on macOS this morning:

  • A universal issue in all browsers is that the reason why the input is invalid is not communicated to screen readers until the form is submitted
  • The character counter needs an accessible description
  • In Safari clicking the "Save changes" button does not announce "Changes saved"
  • In Firefox clicking the "Save changes" button reads "Changes saved" twice
  • In Chrome the "Save changes" button announces the changes are being saved correctly

robertknight avatar Aug 27 '24 08:08 robertknight

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Sep 26 '24 10:09 github-actions[bot]