From dd3698c008fd3274d21835f78413e7e1dfcba645 Mon Sep 17 00:00:00 2001 From: Francine Lucca <40550942+francinelucca@users.noreply.github.com> Date: Wed, 12 Jul 2023 16:04:10 -0400 Subject: [PATCH] fix(Search): close expandable search on escape key (#14076) * fix(Search): close expandable search on escape key * fix: format * fix(Search): open expandable search on button activation not input focus * fix(test): add tests for new functionality on Search & ExpandableSearch * fix: format * chore(test): update snapshot * fix(Search): add aria-controlss & aria-expanded to button * fix: remove comment * fix(ExpandableSearch): implement 2-step close-on-escape behavior * fix(test): adjust expandable search test to new escape behavior * fix: format * fix(Search): make search button focus outline 2px --------- Co-authored-by: Andrea N. Cardona Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../__snapshots__/DataTable-test.js.snap | 4 +- .../TableToolbarSearch-test.js.snap | 2 +- .../ExpandableSearch/ExpandableSearch-test.js | 50 +++++++++++++++++++ .../ExpandableSearch/ExpandableSearch.tsx | 23 ++++++--- .../src/components/Search/Search-test.js | 48 ++++++++++++++++++ .../react/src/components/Search/Search.tsx | 31 ++++++++++-- .../scss/components/search/_search.scss | 4 ++ 7 files changed, 148 insertions(+), 14 deletions(-) diff --git a/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap b/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap index 29ef8b3b353c..6c2b45d94c35 100644 --- a/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap +++ b/packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap @@ -484,6 +484,7 @@ exports[`DataTable behaves as expected selection should render and match snapsho >
@@ -890,6 +890,7 @@ exports[`DataTable renders as expected - Component API should render and match s >
diff --git a/packages/react/src/components/DataTable/__tests__/__snapshots__/TableToolbarSearch-test.js.snap b/packages/react/src/components/DataTable/__tests__/__snapshots__/TableToolbarSearch-test.js.snap index 53d9d2aaace3..6eb7f7836dd1 100644 --- a/packages/react/src/components/DataTable/__tests__/__snapshots__/TableToolbarSearch-test.js.snap +++ b/packages/react/src/components/DataTable/__tests__/__snapshots__/TableToolbarSearch-test.js.snap @@ -9,6 +9,7 @@ exports[`TableToolbarSearch renders as expected - Component API should render 1` >
diff --git a/packages/react/src/components/ExpandableSearch/ExpandableSearch-test.js b/packages/react/src/components/ExpandableSearch/ExpandableSearch-test.js index 2128a0e60019..628f82bfa7e8 100644 --- a/packages/react/src/components/ExpandableSearch/ExpandableSearch-test.js +++ b/packages/react/src/components/ExpandableSearch/ExpandableSearch-test.js @@ -58,6 +58,30 @@ describe('ExpandableSearch', () => { expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`); }); + it('expands on enter', async () => { + const { container } = render( + + ); + + await screen.getAllByRole('button')[0].focus(); + + await userEvent.keyboard('[Enter]'); + + expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`); + }); + + it('expands on space', async () => { + const { container } = render( + + ); + + await screen.getAllByRole('button')[0].focus(); + + await userEvent.keyboard('[Space]'); + + expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`); + }); + it('places focus on the input after expansion', async () => { render(); @@ -107,5 +131,31 @@ describe('ExpandableSearch', () => { expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`); }); + + it('closes and clears value on escape', async () => { + const { container } = render( + + ); + + await userEvent.click(screen.getAllByRole('button')[0]); + + expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`); + + await userEvent.type(screen.getByRole('searchbox'), 'test-value'); + + expect(screen.getByRole('searchbox')).toHaveValue('test-value'); + + await userEvent.keyboard('[Escape]'); + + expect(screen.getByRole('searchbox')).not.toHaveValue('test-value'); + + expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`); + + await userEvent.keyboard('[Escape]'); + + expect(container.firstChild).not.toHaveClass( + `${prefix}--search--expanded` + ); + }); }); }); diff --git a/packages/react/src/components/ExpandableSearch/ExpandableSearch.tsx b/packages/react/src/components/ExpandableSearch/ExpandableSearch.tsx index d97f7d46b177..412451f747b6 100644 --- a/packages/react/src/components/ExpandableSearch/ExpandableSearch.tsx +++ b/packages/react/src/components/ExpandableSearch/ExpandableSearch.tsx @@ -10,12 +10,13 @@ import classnames from 'classnames'; import Search, { type SearchProps } from '../Search'; import { usePrefix } from '../../internal/usePrefix'; import { composeEventHandlers } from '../../tools/events'; +import { match, keys } from '../../internal/keyboard'; function ExpandableSearch({ onBlur, onChange, onExpand, - onFocus, + onKeyDown, defaultValue, isExpanded, ...props @@ -25,12 +26,6 @@ function ExpandableSearch({ const searchRef = useRef(null); const prefix = usePrefix(); - function handleFocus() { - if (!expanded) { - setExpanded(true); - } - } - function handleBlur(evt) { const relatedTargetIsAllowed = evt.relatedTarget && @@ -46,9 +41,21 @@ function ExpandableSearch({ } function handleExpand() { + setExpanded(true); searchRef.current?.focus?.(); } + function handleKeyDown(evt) { + if (expanded && match(evt, keys.Escape)) { + evt.stopPropagation(); + + // escape key only clears if the input is empty, otherwise it clears the input + if (!evt.target?.value) { + setExpanded(false); + } + } + } + const classes = classnames( `${prefix}--search--expandable`, { @@ -64,10 +71,10 @@ function ExpandableSearch({ isExpanded={expanded} ref={searchRef} className={classes} - onFocus={composeEventHandlers([onFocus, handleFocus])} onBlur={composeEventHandlers([onBlur, handleBlur])} onChange={composeEventHandlers([onChange, handleChange])} onExpand={composeEventHandlers([onExpand, handleExpand])} + onKeyDown={composeEventHandlers([onKeyDown, handleKeyDown])} /> ); } diff --git a/packages/react/src/components/Search/Search-test.js b/packages/react/src/components/Search/Search-test.js index 3dd7fb323b17..0a4b16ccf745 100644 --- a/packages/react/src/components/Search/Search-test.js +++ b/packages/react/src/components/Search/Search-test.js @@ -103,6 +103,18 @@ describe('Search', () => { await userEvent.click(screen.getAllByRole('button')[0]); expect(onExpand).toHaveBeenCalled(); + + await screen.getAllByRole('button')[0].focus(); + + await userEvent.keyboard('[Space]'); + + expect(onExpand).toHaveBeenCalledTimes(2); + + await screen.getAllByRole('button')[0].focus(); + + await userEvent.keyboard('[Enter]'); + + expect(onExpand).toHaveBeenCalledTimes(3); }); it('should call onKeyDown when expected', async () => { @@ -114,6 +126,42 @@ describe('Search', () => { expect(onKeyDown).toHaveBeenCalled(); }); + it('should call focus expand button on Escape when expanded', async () => { + render( + {}} isExpanded={true} /> + ); + + await screen.getByRole('searchbox').focus(); + + await userEvent.keyboard('[Escape]'); + + expect(screen.getAllByRole('button')[0]).toHaveFocus(); + }); + + it('should have tabbable button and untabbable input if expandable and not expanded', async () => { + render( + {}} + isExpanded={false} + /> + ); + + expect(screen.getAllByRole('button')[0]).toHaveAttribute('tabIndex', '1'); + expect(screen.getByRole('searchbox')).toHaveAttribute('tabIndex', '-1'); + }); + + it('should have tabbable input and untabbable button if not expandable', async () => { + render(); + + // will only have 1 button which is the close button + expect(screen.getAllByRole('button').length).toBe(1); + expect(screen.getByRole('searchbox')).not.toHaveAttribute( + 'tabIndex', + '-1' + ); + }); + it('should respect placeholder prop', () => { render(); diff --git a/packages/react/src/components/Search/Search.tsx b/packages/react/src/components/Search/Search.tsx index ad0be45a7d38..ad9e10bbaa14 100644 --- a/packages/react/src/components/Search/Search.tsx +++ b/packages/react/src/components/Search/Search.tsx @@ -17,6 +17,7 @@ import React, { type KeyboardEvent, type ComponentType, type FunctionComponent, + type MouseEvent, } from 'react'; import { focus } from '../../internal/focus'; import { keys, match } from '../../internal/keyboard'; @@ -83,7 +84,9 @@ export interface SearchProps extends InputPropsBase { /** * Optional callback called when the magnifier icon is clicked in ExpandableSearch. */ - onExpand?(): void; + onExpand?( + e: MouseEvent | KeyboardEvent + ): void; /** * Provide an optional placeholder text for the Search. @@ -150,6 +153,7 @@ const Search = React.forwardRef(function Search( const { isFluid } = useContext(FormContext); const inputRef = useRef(null); const ref = useMergedRefs([forwardRef, inputRef]); + const expandButtonRef = useRef(null); const inputId = useId('search-input'); const uniqueId = id || inputId; const searchId = `${uniqueId}-search`; @@ -199,7 +203,22 @@ const Search = React.forwardRef(function Search( function handleKeyDown(event: KeyboardEvent) { if (match(event, keys.Escape)) { event.stopPropagation(); - clearInput(); + if (inputRef.current?.value) { + clearInput(); + } + // ExpandableSearch closes on escape when isExpanded, focus search activation button + else if (onExpand && isExpanded) { + expandButtonRef.current?.focus(); + } + } + } + + function handleExpandButtonKeyDown(event: KeyboardEvent) { + if (match(event, keys.Enter) || match(event, keys.Space)) { + event.stopPropagation(); + if (onExpand) { + onExpand(event); + } } } @@ -212,7 +231,12 @@ const Search = React.forwardRef(function Search( aria-labelledby={onExpand ? uniqueId : undefined} role={onExpand ? 'button' : undefined} className={`${prefix}--search-magnifier`} - onClick={onExpand}> + onClick={onExpand} + onKeyDown={handleExpandButtonKeyDown} + tabIndex={onExpand && !isExpanded ? 1 : -1} + ref={expandButtonRef} + aria-expanded={onExpand && isExpanded ? true : undefined} + aria-controls={onExpand ? uniqueId : undefined}>