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

fix(search): fix a11y structure #4801

Merged
merged 4 commits into from
Dec 2, 2019

Conversation

asudoh
Copy link
Contributor

@asudoh asudoh commented Dec 2, 2019

This change does the following things:

  • Ensures that the <label> in <Search> is hidden when <TableToolbarSearch> hides it. This fixes the orphaned relationship between <input> and <label> when search box is collpased and fixes "INPUT LABEL references must be valid" DAP error
  • Changes the ARIA role of the search container in <TableToolbarSearch> to search rather than searchbox, to better reflect its "container" nature. Also added searchbox role to the underlying <input> to reflect where the search box is. This lets VoiceOver announce the <input> inside <Search> as a "search text field" rather than "edit text", and fixes "An interactive element/widget must have an accessible name" DAP error when search box is expanded

Fixes #3806.

Changelog

Changed

  • Ensures that the <label> in <Search> is hidden when <TableToolbarSearch> hides it
  • Changes the ARIA role of the search container in <TableToolbarSearch> to search rather than searchbox

Testing / Reviewing

Testing should make sure <Search> and <TableToolbarSearch> are not broken.

This change does the following things:

* Ensures that the `<label>` in `<Search>` is hidden when
  `<TableToolbarSearch>` hides it. This fixes the orphaned relationship
  between `<input>` and `<label>` when search box is collpased and
  fixes "INPUT LABEL references must be valid" DAP error
* Changes the ARIA role of the search container in
  `<TableToolbarSearch>` to `search` rather than `searchbox`, to better
  reflect its "container" nature. Also added `searchbox` role to the
  underlying `<input>` to reflect where the search box is. This lets
  VoiceOver announce the `<input>` inside `<Search>` as a "search text
  field" rather than "edit text", and fixes "An interactive
  element/widget must have an accessible name" DAP error when search
  box is expanded

Fixes carbon-design-system#3806.
@netlify
Copy link

netlify bot commented Dec 2, 2019

Deploy preview for carbon-elements ready!

Built with commit 53603e2

https://deploy-preview-4801--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Dec 2, 2019

Deploy preview for the-carbon-components ready!

Built with commit 53603e2

https://deploy-preview-4801--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Dec 2, 2019

Deploy preview for carbon-components-react ready!

Built with commit 53603e2

https://deploy-preview-4801--carbon-components-react.netlify.com

Copy link
Contributor

@abbeyhrt abbeyhrt left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@asudoh asudoh merged commit f129993 into carbon-design-system:master Dec 2, 2019
@asudoh asudoh deleted the search-roles branch December 2, 2019 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TableToolbarSearch [role=searchbox] cannot be labeled
3 participants