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(dataTable): search accessibility issue #7359

Conversation

CassidyJensen
Copy link
Contributor

@CassidyJensen CassidyJensen commented Nov 30, 2020

Closes #7358

This PR removes the aria-hidden on the DataTable Toolbar Search Input to remove a11y violations. The search is an input and a focusable element, therefore aria-hidden should never be true.

Changelog

Changed

aria-hidden: {!expanded} [removed]
Aria-hidden was being set by whether the search was expanded. Because it is focusable either way, aria-hidden should always be false. It does not need to be set.

Testing / Reviewing

Inspect the search element on a data table. The input element should have no value for aria-hidden

@CassidyJensen CassidyJensen requested a review from a team as a code owner November 30, 2020 17:32
@netlify
Copy link

netlify bot commented Nov 30, 2020

Deploy preview for carbon-elements ready!

Built with commit bcbc6f8

https://deploy-preview-7359--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Nov 30, 2020

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit bcbc6f8

https://deploy-preview-7359--carbon-components-react.netlify.app

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

should the attribute just be removed instead of set to false?

@CassidyJensen
Copy link
Contributor Author

CassidyJensen commented Dec 1, 2020

should the attribute just be removed instead of set to false?

A good point, yes it probably should just be removed. I will update it. Thank you!

@CassidyJensen CassidyJensen requested a review from emyarod December 1, 2020 18:09
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

thanks for the update, looks good to me

@emyarod
Copy link
Member

emyarod commented Dec 1, 2020

you most likely will need to update snapshots to pass CI (run yarn test -u)

@CassidyJensen
Copy link
Contributor Author

CassidyJensen commented Dec 1, 2020

you most likely will need to update snapshots to pass CI (run yarn test -u)

@emyarod
I seem to be running into an issue when I run 'yarn test -u' :
image

Do you have any advice for how to fix it?

@emyarod emyarod force-pushed the table-toolbar-search-accessibility branch from b406185 to 69c0d46 Compare December 1, 2020 18:46
@emyarod
Copy link
Member

emyarod commented Dec 1, 2020

I'm not sure what the test failures are without being able to see the output but I went ahead and updated the snapshots and pushed them up

@CassidyJensen
Copy link
Contributor Author

Thank you!

@kodiakhq kodiakhq bot merged commit 38b009b into carbon-design-system:master Dec 7, 2020
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.

[Search a11y] aria-hidden property is set to true on focusable search elements
4 participants