Skip to content

Commit

Permalink
fix(TableToolbarSearch): support back-tab (#3836)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
asudoh authored and tw15egan committed Aug 27, 2019
1 parent 3e99fc7 commit 151b18e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@
.#{$prefix}--toolbar-action:focus:not([disabled]),
.#{$prefix}--toolbar-action:active:not([disabled]) {
@include focus-outline('outline');

&.#{$prefix}--toolbar-search-container-expandable {
// The focus style is handled by search input in it, need to avoid duplicate animation
outline: none;
}
}

.#{$prefix}--toolbar-action ~ .#{$prefix}--btn {
Expand Down
13 changes: 7 additions & 6 deletions packages/react/src/components/DataTable/TableToolbarSearch.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import cx from 'classnames';
import PropTypes from 'prop-types';
import React, { useRef, useState, useEffect } from 'react';
import React, { useMemo, useRef, useState, useEffect } from 'react';
import { settings } from 'carbon-components';
import Search from '../Search';
import setupGetInstanceId from './tools/instanceId';
Expand All @@ -34,20 +34,21 @@ const TableToolbarSearch = ({
onExpand,
persistent,
persistant,
id = `data-table-search-${getInstanceId()}`,
id,
...rest
}) => {
const { current: controlled } = useRef(expandedProp !== undefined);
const [expandedState, setExpandedState] = useState(defaultExpanded);
const expanded = controlled ? expandedProp : expandedState;
const searchRef = useRef(null);
const [value, setValue] = useState('');
const uniqueId = useMemo(getInstanceId, []);

useEffect(() => {
if (searchRef.current) {
if (!controlled && expandedState && searchRef.current) {
searchRef.current.querySelector('input').focus();
}
});
}, [controlled, expandedState]);

const searchContainerClasses = cx({
[searchContainerClass]: searchContainerClass,
Expand Down Expand Up @@ -77,7 +78,7 @@ const TableToolbarSearch = ({

return (
<div
tabIndex="0"
tabIndex={expandedState ? '-1' : '0'}
role="searchbox"
ref={searchRef}
onClick={event => handleExpand(event, true)}
Expand All @@ -89,7 +90,7 @@ const TableToolbarSearch = ({
small
className={className}
value={value}
id={id}
id={typeof id !== 'undefined' ? id : uniqueId}
aria-hidden={!expanded}
labelText={labelText || t('carbon.table.toolbar.search.label')}
placeHolderText={
Expand Down

0 comments on commit 151b18e

Please sign in to comment.