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(TableToolbarSearch): support back-tab #3836

Merged
merged 3 commits into from
Aug 27, 2019

Conversation

asudoh
Copy link
Contributor

@asudoh asudoh commented Aug 27, 2019

This change fixes the issue with <TableToolbarSearch> that happens when the <input> in it has focus and user performs back-tab gesture (Shift-Tab key).

In such scenario (in before-change version), the root element of <TableToolbarSearch> (<div class="bx--toolbar-search-container-expandable">) gets focus given it has tabindex="0", and the onFocus() handler of the element attempts to send focus back to the <input> no matter what.

This change also fixes the following found in the debugging effort:

  • An issue where id of <Search> in <TableToolbarSearch> is regenerated for every render
  • Issues focus on <input> happening:
    • For every render, regardless of change in expanded state
    • Even for controlled change in expanded state (without user gesture)

Fixes #3762.

Changelog

New

  • Code to memorize auto-generated unique ID
  • Code to prevent focusing-on-<input> code from running
    • In controlled mode
    • Unless the expanded state changes

Changed

  • The tabindex of the outer container of <TableToolbarSearch>, so it's focusable only when it's collapsed

Removed

  • Focus style of the outer container of <TableToolbarSearch> given the <input> in it handles that

Testing / Reviewing

Testing should make sure <TableToolbarSearch> is not broken.

This change fixes the issue with `<TableToolbarSearch>` that happens
when the `<input>` in it has focus and user performs back-tab gesture
(Shift-Tab key).

In such scenario (in before-change version), the root element of
`<TableToolbarSearch>`
(`<div class="bx--toolbar-search-container-expandable">`) gets focus
given it has `tabindex="0"`, and the `onFocus()` handler of the element
attempts to send focus back to the `<input>` no matter what.

This change also fixes the following found in the debugging effort:

* An issue where `id` of `<Search>` in `<TableToolbarSearch>` is
  regenerated for every render
* Issues focus on `<input>` happening:
  * For every render, regardless of change in expanded state
  * Even for controlled change in expanded state (without user gesture)

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

netlify bot commented Aug 27, 2019

Deploy preview for the-carbon-components ready!

Built with commit bc0ab54

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

@netlify
Copy link

netlify bot commented Aug 27, 2019

Deploy preview for carbon-elements ready!

Built with commit bc0ab54

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

@netlify
Copy link

netlify bot commented Aug 27, 2019

Deploy preview for carbon-components-react ready!

Built with commit bc0ab54

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

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.

looks good to me

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.

Nice! 🎉 LGTM

@tw15egan tw15egan merged commit 151b18e into carbon-design-system:master Aug 27, 2019
@asudoh asudoh deleted the tabletoolbarsearch-focus branch August 27, 2019 20:25
@jnm2377 jnm2377 mentioned this pull request Sep 4, 2019
17 tasks
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.

DataTable - Backwards keyboard traversal from search field doesn't work
4 participants