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>
carbon-design-system · Jul 12, 2023 · dd3698c · dd3698c
1 parent c0aa65a
commit dd3698c
Show file tree

Hide file tree

Showing 7 changed files with 148 additions and 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
@@ -484,6 +484,7 @@ exports[`DataTable behaves as expected selection should render and match snapsho
         >
           <div
             class="cds--search-magnifier"
+            tabindex="-1"
           >
             <svg
               aria-hidden="true"
@@ -514,7 +515,6 @@ exports[`DataTable behaves as expected selection should render and match snapsho
             id="custom-id"
             placeholder="Filter table"
             role="searchbox"
-            tabindex="0"
             type="text"
             value=""
           />
@@ -890,6 +890,7 @@ exports[`DataTable renders as expected - Component API should render and match s
         >
           <div
             class="cds--search-magnifier"
+            tabindex="-1"
           >
             <svg
               aria-hidden="true"
@@ -920,7 +921,6 @@ exports[`DataTable renders as expected - Component API should render and match s
             id="custom-id"
             placeholder="Filter table"
             role="searchbox"
-            tabindex="0"
             type="text"
             value=""
           />

diff --git a/...es/react/src/components/DataTable/__tests__/__snapshots__/TableToolbarSearch-test.js.snap b/...es/react/src/components/DataTable/__tests__/__snapshots__/TableToolbarSearch-test.js.snap
@@ -9,6 +9,7 @@ exports[`TableToolbarSearch renders as expected - Component API should render 1`
   >
     <div
       class="cds--search-magnifier"
+      tabindex="-1"
     >
       <svg
         aria-hidden="true"
@@ -39,7 +40,6 @@ exports[`TableToolbarSearch renders as expected - Component API should render 1`
       id="custom-id"
       placeholder="Filter table"
       role="searchbox"
-      tabindex="0"
       type="text"
       value=""
     />

diff --git 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(
+        <ExpandableSearch labelText="test-search" />
+      );
+
+      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(
+        <ExpandableSearch labelText="test-search" />
+      );
+
+      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(<ExpandableSearch labelText="test-search" />);
 
@@ -107,5 +131,31 @@ describe('ExpandableSearch', () => {
 
       expect(container.firstChild).toHaveClass(`${prefix}--search--expanded`);
     });
+
+    it('closes and clears value on escape', async () => {
+      const { container } = render(
+        <ExpandableSearch labelText="test-search" />
+      );
+
+      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
@@ -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<HTMLInputElement>(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
@@ -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(
+        <Search labelText="test-search" onExpand={() => {}} 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(
+        <Search
+          labelText="test-search"
+          onExpand={() => {}}
+          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(<Search labelText="test-search" />);
+
+      // 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(<Search labelText="test-search" placeholder="test-placeholder" />);
 

diff --git 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<HTMLDivElement> | KeyboardEvent<HTMLDivElement>
+  ): void;
 
   /**
    * Provide an optional placeholder text for the Search.
@@ -150,6 +153,7 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(function Search(
   const { isFluid } = useContext(FormContext);
   const inputRef = useRef<HTMLInputElement>(null);
   const ref = useMergedRefs<HTMLInputElement>([forwardRef, inputRef]);
+  const expandButtonRef = useRef<HTMLDivElement>(null);
   const inputId = useId('search-input');
   const uniqueId = id || inputId;
   const searchId = `${uniqueId}-search`;
@@ -199,7 +203,22 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(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<HTMLDivElement>) {
+    if (match(event, keys.Enter) || match(event, keys.Space)) {
+      event.stopPropagation();
+      if (onExpand) {
+        onExpand(event);
+      }
     }
   }
 
@@ -212,7 +231,12 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(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}>
         <CustomSearchIcon icon={renderIcon} />
       </div>
       <label id={searchId} htmlFor={uniqueId} className={`${prefix}--label`}>
@@ -232,6 +256,7 @@ const Search = React.forwardRef<HTMLInputElement, SearchProps>(function Search(
         placeholder={placeholder}
         type={type}
         value={value}
+        tabIndex={onExpand && !isExpanded ? -1 : undefined}
       />
       <button
         aria-label={closeButtonLabelText}

diff --git a/packages/styles/scss/components/search/_search.scss b/packages/styles/scss/components/search/_search.scss
@@ -350,6 +350,10 @@
   .#{$prefix}--search--expandable .#{$prefix}--search-magnifier {
     position: absolute;
     cursor: pointer;
+
+    &:focus {
+      outline: 2px solid $focus;
+    }
   }
 
   .#{$prefix}--search--expandable .#{$prefix}--search-magnifier:hover {