fix(tearsheet): resolve focusing elements multiple times while render…

…ing (#6513) * refactor(useFocus): refactor repeated useEffect code * fix(tearsheetshell): remove unwanted useEffect * fix(tearsheet): resolve unwanted onblur call * fix(createTearsheet): resolve focus out issue * test(tearsheet): implement test for onBlur method
carbon-design-system · Dec 12, 2024 · 1c918d1 · 1c918d1
1 parent 13e845c
commit 1c918d1
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 88 deletions.
diff --git a/packages/ibm-products/src/components/CreateTearsheet/CreateTearsheet.tsx b/packages/ibm-products/src/components/CreateTearsheet/CreateTearsheet.tsx
@@ -310,6 +310,7 @@ export let CreateTearsheet = forwardRef(
           verticalPosition,
           closeIconDescription: '',
         }}
+        currentStep={currentStep}
       >
         <div className={`${blockClass}__content`} ref={contentRef}>
           <Form aria-label={title}>

diff --git a/packages/ibm-products/src/components/Tearsheet/Tearsheet.test.js b/packages/ibm-products/src/components/Tearsheet/Tearsheet.test.js
@@ -36,6 +36,7 @@ const componentNameCreateNarrow = CreateTearsheetNarrow.displayName;
 const onClick = jest.fn();
 const onCloseReturnsFalse = jest.fn(() => false);
 const onCloseReturnsTrue = jest.fn(() => true);
+const onBlur = jest.fn();
 
 const createButton = `Create ${uuidv4()}`;
 const actions = [
@@ -92,6 +93,41 @@ const navigation = (
 );
 const title = `Title of the ${uuidv4()} tearsheet`;
 
+const mainText = 'Main content 1';
+const inputId = 'stacked-input-1';
+
+// eslint-disable-next-line react/prop-types
+const DummyComponent = ({ props, open }) => {
+  const buttonRef = React.useRef(undefined);
+
+  return (
+    <>
+      <Button ref={buttonRef}>Open</Button>
+      <Tearsheet
+        {...{ ...props, closeIconDescription }}
+        {...{
+          open: open,
+        }}
+        hasCloseIcon={true}
+        onClose={onCloseReturnsTrue}
+        open={open}
+        selectorPrimaryFocus={`#${inputId}`}
+        launcherButtonRef={buttonRef}
+      >
+        <div className="tearsheet-stories__dummy-content-block">
+          {mainText}
+          <TextInput
+            id={inputId}
+            data-testid={inputId}
+            labelText="Enter an important value here"
+            onBlur={onBlur}
+          />
+        </div>
+      </Tearsheet>
+    </>
+  );
+};
+
 // These are tests than apply to both Tearsheet and TearsheetNarrow
 // and also (with extra props and omitting button tests) to CreateTearsheetNarrow
 let tooManyButtonsTestedAlready = false;
@@ -262,40 +298,6 @@ const commonTests = (Ts, name, props, testActions) => {
     });
 
     it('should return focus to the launcher button', async () => {
-      const mainText = 'Main content 1';
-      const inputId = 'stacked-input-1';
-
-      // eslint-disable-next-line react/prop-types
-      const DummyComponent = ({ open }) => {
-        const buttonRef = React.useRef(undefined);
-
-        return (
-          <>
-            <Button ref={buttonRef}>Open</Button>
-            <Ts
-              {...{ ...props, closeIconDescription }}
-              {...{
-                open: open,
-              }}
-              hasCloseIcon={true}
-              onClose={onCloseReturnsTrue}
-              open={open}
-              selectorPrimaryFocus={`#${inputId}`}
-              launcherButtonRef={buttonRef}
-            >
-              <div className="tearsheet-stories__dummy-content-block">
-                {mainText}
-                <TextInput
-                  id={inputId}
-                  data-testid={inputId}
-                  labelText="Enter an important value here"
-                />
-              </div>
-            </Ts>
-          </>
-        );
-      };
-
       const { rerender, getByText, getByTestId } = render(
         <DummyComponent open={true} />
       );
@@ -320,6 +322,19 @@ const commonTests = (Ts, name, props, testActions) => {
       await act(() => new Promise((resolve) => setTimeout(resolve, 0)));
       expect(launchButtonEl).toHaveFocus();
     });
+
+    it('should call onBlur only once', async () => {
+      const { getByTestId } = render(<DummyComponent open={true} />);
+
+      const inputEl = getByTestId(inputId);
+      const closeButton = screen.getByRole('button', {
+        name: closeIconDescription,
+      });
+
+      expect(inputEl).toHaveFocus();
+      await act(() => userEvent.click(closeButton));
+      expect(onBlur).toHaveBeenCalledTimes(1);
+    });
   }
 
   it('is visible when open is true', async () => {

diff --git a/packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx b/packages/ibm-products/src/components/Tearsheet/TearsheetShell.tsx
@@ -65,6 +65,11 @@ interface TearsheetShellProps extends PropsWithChildren {
    */
   className?: string;
 
+  /**
+   * Used to track the current step on components which use `StepsContext` and `TearsheetShell`
+   */
+  currentStep?: number;
+
   /**
    * A description of the flow, displayed in the header area of the tearsheet.
    */
@@ -199,13 +204,9 @@ export type CloseIconDescriptionTypes =
 type stackTypes = {
   open: Array<{
     (a: number, b: number): void;
-    checkFocus?: () => void;
-    claimFocus?: () => void;
   }>;
   all: Array<{
     (a: number, b: number): void;
-    checkFocus?: () => void;
-    claimFocus?: () => void;
   }>;
   sizes: Array<string>;
 };
@@ -242,6 +243,7 @@ export const TearsheetShell = React.forwardRef(
       children,
       className,
       closeIconDescription,
+      currentStep,
       description,
       hasCloseIcon,
       headerActions,
@@ -274,7 +276,7 @@ export const TearsheetShell = React.forwardRef(
     const modalRef = (ref || localRef) as MutableRefObject<HTMLDivElement>;
     const { width } = useResizeObserver(resizer);
     const prevOpen = usePreviousValue(open);
-    const { firstElement, keyDownListener, specifiedElement } = useFocus(
+    const { firstElement, keyDownListener } = useFocus(
       modalRef,
       selectorPrimaryFocus
     );
@@ -309,29 +311,22 @@ export const TearsheetShell = React.forwardRef(
       setDepth(newDepth);
       setPosition(newPosition);
     }
-    handleStackChange.checkFocus = function () {
-      // if we are now the topmost tearsheet, ensure we have focus
-      if (
-        open &&
-        position === depth &&
-        modalRefValue &&
-        !modalRefValue.contains(document.activeElement)
-      ) {
-        handleStackChange.claimFocus();
-      }
-    };
-
-    // Callback to give the tearsheet the opportunity to claim focus
-    handleStackChange.claimFocus = function () {
-      claimFocus(firstElement, modalRef, selectorPrimaryFocus);
-    };
 
     useEffect(() => {
-      if (open) {
+      if (open && position === depth) {
         // Focusing the first element or selectorPrimaryFocus element
         claimFocus(firstElement, modalRef, selectorPrimaryFocus);
       }
-    }, [firstElement, modalRef, open, selectorPrimaryFocus]);
+    }, [
+      currentStep,
+      depth,
+      firstElement,
+      modalRef,
+      modalRefValue,
+      open,
+      position,
+      selectorPrimaryFocus,
+    ]);
 
     useEffect(() => {
       if (prevOpen && !open && launcherButtonRef) {
@@ -341,32 +336,13 @@ export const TearsheetShell = React.forwardRef(
       }
     }, [launcherButtonRef, open, prevOpen]);
 
-    useEffect(() => {
-      if (open && position !== depth) {
-        setTimeout(() => {
-          if (selectorPrimaryFocus) {
-            return specifiedElement?.focus();
-          }
-          firstElement?.focus();
-        }, 0);
-      }
-    }, [
-      position,
-      depth,
-      firstElement,
-      open,
-      specifiedElement,
-      selectorPrimaryFocus,
-    ]);
-
     useEffect(() => {
       const notify = () =>
         stack.all.forEach((handler) => {
           handler(
             Math.min(stack.open.length, maxDepth),
             stack.open.indexOf(handler) + 1
           );
-          handler.checkFocus?.();
         });
 
       // Register this tearsheet's stack change callback/listener.
@@ -401,14 +377,6 @@ export const TearsheetShell = React.forwardRef(
       };
     }, [open, size]);
 
-    function handleFocus() {
-      // If something within us is receiving focus but we are not the topmost
-      // stacked tearsheet, transfer focus to the topmost tearsheet instead
-      if (position < depth) {
-        stack.open[stack.open.length - 1].claimFocus?.();
-      }
-    }
-
     if (position <= depth) {
       // Include a modal header if and only if one or more of these is given.
       // We can't use a Wrap for the ModalHeader because ComposedModal requires
@@ -464,7 +432,6 @@ export const TearsheetShell = React.forwardRef(
                 !areAllSameSizeVariant(),
             })}
             {...{ onClose, open, selectorPrimaryFocus }}
-            onFocus={handleFocus}
             onKeyDown={keyDownListener}
             preventCloseOnClickOutside={!isPassive}
             ref={modalRef}

diff --git a/packages/ibm-products/src/global/js/hooks/useFocus.js b/packages/ibm-products/src/global/js/hooks/useFocus.js
@@ -122,9 +122,9 @@ export const claimFocus = (
       specifiedEl &&
       window?.getComputedStyle(specifiedEl)?.display !== 'none'
     ) {
-      return specifiedEl.focus();
+      setTimeout(() => specifiedEl.focus(), 0);
     }
+  } else {
+    setTimeout(() => firstElement?.focus(), 0);
   }
-
-  setTimeout(() => firstElement?.focus(), 0);
 };