Skip to content

Commit dd3698c

Browse files
francineluccaandreancardonakodiakhq[bot]
authored
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 <cardona.n.andrea@gmail.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
1 parent c0aa65a commit dd3698c

7 files changed

Lines changed: 148 additions & 14 deletions

File tree

packages/react/src/components/DataTable/__tests__/__snapshots__/DataTable-test.js.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,7 @@ exports[`DataTable behaves as expected selection should render and match snapsho
484484
>
485485
<div
486486
class="cds--search-magnifier"
487+
tabindex="-1"
487488
>
488489
<svg
489490
aria-hidden="true"
@@ -514,7 +515,6 @@ exports[`DataTable behaves as expected selection should render and match snapsho
514515
id="custom-id"
515516
placeholder="Filter table"
516517
role="searchbox"
517-
tabindex="0"
518518
type="text"
519519
value=""
520520
/>
@@ -890,6 +890,7 @@ exports[`DataTable renders as expected - Component API should render and match s
890890
>
891891
<div
892892
class="cds--search-magnifier"
893+
tabindex="-1"
893894
>
894895
<svg
895896
aria-hidden="true"
@@ -920,7 +921,6 @@ exports[`DataTable renders as expected - Component API should render and match s
920921
id="custom-id"
921922
placeholder="Filter table"
922923
role="searchbox"
923-
tabindex="0"
924924
type="text"
925925
value=""
926926
/>

packages/react/src/components/DataTable/__tests__/__snapshots__/TableToolbarSearch-test.js.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ exports[`TableToolbarSearch renders as expected - Component API should render 1`
99
>
1010
<div
1111
class="cds--search-magnifier"
12+
tabindex="-1"
1213
>
1314
<svg
1415
aria-hidden="true"
@@ -39,7 +40,6 @@ exports[`TableToolbarSearch renders as expected - Component API should render 1`
3940
id="custom-id"
4041
placeholder="Filter table"
4142
role="searchbox"
42-
tabindex="0"
4343
type="text"
4444
value=""
4545
/>

packages/react/src/components/ExpandableSearch/ExpandableSearch-test.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,30 @@ describe('ExpandableSearch', () => {
5858
expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`);
5959
});
6060

61+
it('expands on enter', async () => {
62+
const { container } = render(
63+
<ExpandableSearch labelText="test-search" />
64+
);
65+
66+
await screen.getAllByRole('button')[0].focus();
67+
68+
await userEvent.keyboard('[Enter]');
69+
70+
expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`);
71+
});
72+
73+
it('expands on space', async () => {
74+
const { container } = render(
75+
<ExpandableSearch labelText="test-search" />
76+
);
77+
78+
await screen.getAllByRole('button')[0].focus();
79+
80+
await userEvent.keyboard('[Space]');
81+
82+
expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`);
83+
});
84+
6185
it('places focus on the input after expansion', async () => {
6286
render(<ExpandableSearch labelText="test-search" />);
6387

@@ -107,5 +131,31 @@ describe('ExpandableSearch', () => {
107131

108132
expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`);
109133
});
134+
135+
it('closes and clears value on escape', async () => {
136+
const { container } = render(
137+
<ExpandableSearch labelText="test-search" />
138+
);
139+
140+
await userEvent.click(screen.getAllByRole('button')[0]);
141+
142+
expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`);
143+
144+
await userEvent.type(screen.getByRole('searchbox'), 'test-value');
145+
146+
expect(screen.getByRole('searchbox')).toHaveValue('test-value');
147+
148+
await userEvent.keyboard('[Escape]');
149+
150+
expect(screen.getByRole('searchbox')).not.toHaveValue('test-value');
151+
152+
expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`);
153+
154+
await userEvent.keyboard('[Escape]');
155+
156+
expect(container.firstChild).not.toHaveClass(
157+
`${prefix}--search--expanded`
158+
);
159+
});
110160
});
111161
});

packages/react/src/components/ExpandableSearch/ExpandableSearch.tsx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,13 @@ import classnames from 'classnames';
1010
import Search, { type SearchProps } from '../Search';
1111
import { usePrefix } from '../../internal/usePrefix';
1212
import { composeEventHandlers } from '../../tools/events';
13+
import { match, keys } from '../../internal/keyboard';
1314

1415
function ExpandableSearch({
1516
onBlur,
1617
onChange,
1718
onExpand,
18-
onFocus,
19+
onKeyDown,
1920
defaultValue,
2021
isExpanded,
2122
...props
@@ -25,12 +26,6 @@ function ExpandableSearch({
2526
const searchRef = useRef<HTMLInputElement>(null);
2627
const prefix = usePrefix();
2728

28-
function handleFocus() {
29-
if (!expanded) {
30-
setExpanded(true);
31-
}
32-
}
33-
3429
function handleBlur(evt) {
3530
const relatedTargetIsAllowed =
3631
evt.relatedTarget &&
@@ -46,9 +41,21 @@ function ExpandableSearch({
4641
}
4742

4843
function handleExpand() {
44+
setExpanded(true);
4945
searchRef.current?.focus?.();
5046
}
5147

48+
function handleKeyDown(evt) {
49+
if (expanded && match(evt, keys.Escape)) {
50+
evt.stopPropagation();
51+
52+
// escape key only clears if the input is empty, otherwise it clears the input
53+
if (!evt.target?.value) {
54+
setExpanded(false);
55+
}
56+
}
57+
}
58+
5259
const classes = classnames(
5360
`${prefix}--search--expandable`,
5461
{
@@ -64,10 +71,10 @@ function ExpandableSearch({
6471
isExpanded={expanded}
6572
ref={searchRef}
6673
className={classes}
67-
onFocus={composeEventHandlers([onFocus, handleFocus])}
6874
onBlur={composeEventHandlers([onBlur, handleBlur])}
6975
onChange={composeEventHandlers([onChange, handleChange])}
7076
onExpand={composeEventHandlers([onExpand, handleExpand])}
77+
onKeyDown={composeEventHandlers([onKeyDown, handleKeyDown])}
7178
/>
7279
);
7380
}

packages/react/src/components/Search/Search-test.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,18 @@ describe('Search', () => {
103103
await userEvent.click(screen.getAllByRole('button')[0]);
104104

105105
expect(onExpand).toHaveBeenCalled();
106+
107+
await screen.getAllByRole('button')[0].focus();
108+
109+
await userEvent.keyboard('[Space]');
110+
111+
expect(onExpand).toHaveBeenCalledTimes(2);
112+
113+
await screen.getAllByRole('button')[0].focus();
114+
115+
await userEvent.keyboard('[Enter]');
116+
117+
expect(onExpand).toHaveBeenCalledTimes(3);
106118
});
107119

108120
it('should call onKeyDown when expected', async () => {
@@ -114,6 +126,42 @@ describe('Search', () => {
114126
expect(onKeyDown).toHaveBeenCalled();
115127
});
116128

129+
it('should call focus expand button on Escape when expanded', async () => {
130+
render(
131+
<Search labelText="test-search" onExpand={() => {}} isExpanded={true} />
132+
);
133+
134+
await screen.getByRole('searchbox').focus();
135+
136+
await userEvent.keyboard('[Escape]');
137+
138+
expect(screen.getAllByRole('button')[0]).toHaveFocus();
139+
});
140+
141+
it('should have tabbable button and untabbable input if expandable and not expanded', async () => {
142+
render(
143+
<Search
144+
labelText="test-search"
145+
onExpand={() => {}}
146+
isExpanded={false}
147+
/>
148+
);
149+
150+
expect(screen.getAllByRole('button')[0]).toHaveAttribute('tabIndex', '1');
151+
expect(screen.getByRole('searchbox')).toHaveAttribute('tabIndex', '-1');
152+
});
153+
154+
it('should have tabbable input and untabbable button if not expandable', async () => {
155+
render(<Search labelText="test-search" />);
156+
157+
// will only have 1 button which is the close button
158+
expect(screen.getAllByRole('button').length).toBe(1);
159+
expect(screen.getByRole('searchbox')).not.toHaveAttribute(
160+
'tabIndex',
161+
'-1'
162+
);
163+
});
164+
117165
it('should respect placeholder prop', () => {
118166
render(<Search labelText="test-search" placeholder="test-placeholder" />);
119167

packages/react/src/components/Search/Search.tsx

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import React, {
1717
type KeyboardEvent,
1818
type ComponentType,
1919
type FunctionComponent,
20+
type MouseEvent,
2021
} from 'react';
2122
import { focus } from '../../internal/focus';
2223
import { keys, match } from '../../internal/keyboard';
@@ -83,7 +84,9 @@ export interface SearchProps extends InputPropsBase {
8384
/**
8485
* Optional callback called when the magnifier icon is clicked in ExpandableSearch.
8586
*/
86-
onExpand?(): void;
87+
onExpand?(
88+
e: MouseEvent<HTMLDivElement> | KeyboardEvent<HTMLDivElement>
89+
): void;
8790

8891
/**
8992
* Provide an optional placeholder text for the Search.
@@ -150,6 +153,7 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(function Search(
150153
const { isFluid } = useContext(FormContext);
151154
const inputRef = useRef<HTMLInputElement>(null);
152155
const ref = useMergedRefs<HTMLInputElement>([forwardRef, inputRef]);
156+
const expandButtonRef = useRef<HTMLDivElement>(null);
153157
const inputId = useId('search-input');
154158
const uniqueId = id || inputId;
155159
const searchId = `${uniqueId}-search`;
@@ -199,7 +203,22 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(function Search(
199203
function handleKeyDown(event: KeyboardEvent) {
200204
if (match(event, keys.Escape)) {
201205
event.stopPropagation();
202-
clearInput();
206+
if (inputRef.current?.value) {
207+
clearInput();
208+
}
209+
// ExpandableSearch closes on escape when isExpanded, focus search activation button
210+
else if (onExpand && isExpanded) {
211+
expandButtonRef.current?.focus();
212+
}
213+
}
214+
}
215+
216+
function handleExpandButtonKeyDown(event: KeyboardEvent<HTMLDivElement>) {
217+
if (match(event, keys.Enter) || match(event, keys.Space)) {
218+
event.stopPropagation();
219+
if (onExpand) {
220+
onExpand(event);
221+
}
203222
}
204223
}
205224

@@ -212,7 +231,12 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(function Search(
212231
aria-labelledby={onExpand ? uniqueId : undefined}
213232
role={onExpand ? 'button' : undefined}
214233
className={`${prefix}--search-magnifier`}
215-
onClick={onExpand}>
234+
onClick={onExpand}
235+
onKeyDown={handleExpandButtonKeyDown}
236+
tabIndex={onExpand && !isExpanded ? 1 : -1}
237+
ref={expandButtonRef}
238+
aria-expanded={onExpand && isExpanded ? true : undefined}
239+
aria-controls={onExpand ? uniqueId : undefined}>
216240
<CustomSearchIcon icon={renderIcon} />
217241
</div>
218242
<label id={searchId} htmlFor={uniqueId} className={`${prefix}--label`}>
@@ -232,6 +256,7 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(function Search(
232256
placeholder={placeholder}
233257
type={type}
234258
value={value}
259+
tabIndex={onExpand && !isExpanded ? -1 : undefined}
235260
/>
236261
<button
237262
aria-label={closeButtonLabelText}

packages/styles/scss/components/search/_search.scss

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,10 @@
350350
.#{$prefix}--search--expandable .#{$prefix}--search-magnifier {
351351
position: absolute;
352352
cursor: pointer;
353+
354+
&:focus {
355+
outline: 2px solid $focus;
356+
}
353357
}
354358

355359
.#{$prefix}--search--expandable .#{$prefix}--search-magnifier:hover {

0 commit comments

Comments
 (0)