Skip to content
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

feat: Improve label accessibility #278

Merged
merged 6 commits into from
Nov 10, 2022
Merged

Conversation

jonathonherbert
Copy link
Contributor

What does this change?

#275 requires some React-based unit tests, but in order for testing-library to pass tests using our React wrappers, our accessibility story for labels needed to be improved, or we get the error:

Found a label with the text of: field1, however no form control was found associated to that label. Make sure you're using the "for" attribute or "aria-labelledby" attribute correctly.

(How lovely for a testing library to point that out!)

This PR adds a test that verifies the accessibility of our labels by proxy, and adds the functionality that satisfies the test. Because we cannot add a for attribute to our label (contenteditable elements cannot be addressed by this attribute per the spec), we add an a tag with a link to simulate the usual label behaviour – clicking on the label should give a pointer on hover, and focus the field input on click.

How to test

  • The automated test should pass.
  • Have a play with the labels that relate to text fields of any sort, on any element. Hovering should show a pointer cursor. Clicking should focus the field.

@jonathonherbert jonathonherbert requested a review from a team November 1, 2022 15:08
@jonathonherbert jonathonherbert changed the title Improve label accessibility feat: Improve label accessibility Nov 1, 2022
@jonathonherbert
Copy link
Contributor Author

cc. @marjisound and @abeddow91 – I will rebase #275 on this to enable us to add the appropriate tests for onRemove, so we'll want to merge this first.

…ent's field data (#275)

* Add onRemove handler to createReactElementSpec

* feat: generate field values and pass to onRemove

* fix: demo type

* Add getElementData fn to consumer, and apply to onRemove

* revert the change of using function syntax for ElementWrapper

* Add test to verify onRemove callback passes field values

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>
Co-authored-by: Marjan Kalanaki <marjan.kalanaki@guardian.co.uk>
Copy link
Contributor

@abeddow91 abeddow91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this looks good! Tested locally and labels have a unique id of label-field-${Id} with a mathing aria-labelledby. All unit tests are passing.

@jonathonherbert jonathonherbert merged commit ca50766 into main Nov 10, 2022
@jonathonherbert jonathonherbert deleted the jsh/add-label-accessibility branch November 10, 2022 12:19
@github-actions
Copy link
Contributor

🎉 This PR is included in version 5.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants