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(data-table): make persistent search bar keyboard accessible #6574

Merged
merged 4 commits into from
Jul 31, 2020

Conversation

janhassel
Copy link
Member

@janhassel janhassel commented Jul 29, 2020

Closes #6573

When using persistent prop on TableToolbarSearch component, the search bar was not keyboard accessible since its tabindex was set to -1.

Changelog

Changed

  • Control container and input tabindex as well as input's aria-hidden based on expanded || persistent instead of expanded in TableToolbarSearch
  • Updated DataTable snapshots regarding this

Testing / Reviewing

  • Testing should make sure PR has no side effects
  • For review: run react storybook and change defaultExpanded to persistent in with-batch-actions.js story. Search bar should be in tab order now. For previous behaviour, refer to the code sandbox in the linked issue or change
const searchExpanded = expanded || persistent;

in TableToolbarSearch.js with

const searchExpanded = expanded;

Search bar shouldn't be in the tab order anymore then.

@janhassel janhassel requested a review from a team as a code owner July 29, 2020 09:31
@netlify
Copy link

netlify bot commented Jul 29, 2020

Deploy preview for carbon-elements ready!

Built with commit 41cf276

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

@netlify
Copy link

netlify bot commented Jul 29, 2020

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit 41cf276

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

@tw15egan
Copy link
Collaborator

Thanks for taking the time to contribute a fix! I'm noticing a small issue, however. In the original issue, I do see the problem in the second search bar with persistent. However, if I switch the wording to persistant (the old, deprecated prop name), it works properly.

In this PR, it now works for persistent, but it now seems broken if I set the prop name to persistant 🤔

@janhassel
Copy link
Member Author

@tw15egan True, didn't notice that earlier. Should be fixed now! 👍

Copy link
Collaborator

@tw15egan tw15egan 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 fixing that 🙏 👍 ✅

Copy link
Contributor

@dakahn dakahn left a comment

Choose a reason for hiding this comment

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

Thank you so much for taking care of this 🏄

@kodiakhq kodiakhq bot merged commit 09d99c1 into carbon-design-system:master Jul 31, 2020
@janhassel janhassel deleted the fix-6573 branch September 23, 2020 07:36
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]: Search input is not focused when persistent prop is used.
4 participants