Only control focus when toggling Popover

.changeset/soft-pumas-cry.md

@@ -0,0 +1,5 @@
+---
+'@sumup/circuit-ui': patch
+---
+
+Improved the Popover's focus handling to prevent it from hijacking the focus on render.

packages/circuit-ui/components/Popover/Popover.spec.tsx

@@ -86,15 +86,10 @@ describe('Popover', () => {
     jest.clearAllMocks();
   });
 
-  const renderPopover = (props: Omit<PopoverProps, 'component'>) =>
-    render(
-      <Popover
-        component={(triggerProps) => <button {...triggerProps}>Button</button>}
-        {...props}
-      />,
-    );
+  const renderPopover = (props: PopoverProps) => render(<Popover {...props} />);
 
-  const baseProps: Omit<PopoverProps, 'component'> = {
+  const baseProps: PopoverProps = {
+    component: (triggerProps) => <button {...triggerProps}>Button</button>,
     actions: [
       {
         onClick: jest.fn(),
@@ -215,9 +210,35 @@ describe('Popover', () => {
       expect(baseProps.onToggle).toHaveBeenCalledTimes(1);
     });
 
-    /**
-     * Accessibility tests.
-     */
+    it('should move focus to the first popover item after opening', () => {
+      const { getAllByRole, rerender } = renderPopover({
+        ...baseProps,
+        isOpen: false,
+      });
+
+      act(() => {
+        rerender(<Popover {...baseProps} isOpen />);
+      });
+
+      const popoverItems = getAllByRole('menuitem');
+
+      expect(document.activeElement).toEqual(popoverItems[0]);
+    });
+
+    it('should move focus to the trigger element after closing', () => {
+      const { getByRole, rerender } = renderPopover(baseProps);
+
+      act(() => {
+        rerender(<Popover {...baseProps} isOpen={false} />);
+      });
+
+      const popoverTrigger = getByRole('button');
+
+      expect(document.activeElement).toEqual(popoverTrigger);
+    });
+  });
+
+  describe('accessibility', () => {
     it('should meet accessibility guidelines', async () => {
       const { container } = renderPopover(baseProps);
 

packages/circuit-ui/components/Popover/Popover.tsx

@@ -29,6 +29,7 @@ import {
   useEffect,
 } from 'react';
 import useLatest from 'use-latest';
+import usePrevious from 'use-previous';
 import { Dispatch as TrackingProps } from '@sumup/collector';
 import { usePopper } from 'react-popper';
 import { Placement, State, Modifier } from '@popperjs/core';
@@ -323,21 +324,25 @@ export const Popover = ({
   useEscapeKey(() => onToggle(false), isOpen);
   useClickOutside(popperRef, () => onToggle(false), isOpen);
 
+  const prevOpen = usePrevious(isOpen);
+
   useEffect(() => {
-    // Focus the first or last element when opening
-    if (isOpen) {
+    // Focus the first or last element after opening
+    if (!prevOpen && isOpen) {
       const element = (triggerKey.current && triggerKey.current === 'ArrowUp'
         ? wrapperEl.current?.lastElementChild
         : wrapperEl.current?.firstElementChild) as HTMLElement;
       element.focus();
-    } else {
-      // Focus the trigger button when closing
+    }
+
+    // Focus the trigger button after closing
+    if (prevOpen && !isOpen) {
       const triggerButton = triggerEl.current?.firstElementChild as HTMLElement;
       triggerButton.focus();
     }
 
     triggerKey.current = null;
-  }, [isOpen]);
+  }, [isOpen, prevOpen]);
 
   const focusProps = useFocusList();
 

packages/circuit-ui/package.json

@@ -45,6 +45,7 @@
     "react-number-format": "^4.4.1",
     "react-popper": "^2.2.5",
     "use-latest": "^1.2.0",
+    "use-previous": "^1.1.0",
     "yargs": "^17.0.1"
   },
   "devDependencies": {

packages/circuit-ui/util/test-utils.tsx

@@ -17,13 +17,7 @@ import { FunctionComponent, ReactElement } from 'react';
 import { renderToStaticMarkup } from 'react-dom/server';
 import '@testing-library/jest-dom/extend-expect';
 import { configureAxe } from 'jest-axe';
-import {
-  render as renderTest,
-  waitFor,
-  act,
-  RenderResult,
-  fireEvent,
-} from '@testing-library/react';
+import { render as renderTest, RenderResult } from '@testing-library/react';
 import { renderHook, act as actHook } from '@testing-library/react-hooks';
 import userEvent from '@testing-library/user-event';
 import { ThemeProvider } from 'emotion-theming';
@@ -34,6 +28,8 @@ import {
   defaultComponents,
 } from '../components/ComponentsContext';
 
+export * from '@testing-library/react';
+
 export type RenderFn<T = any> = (component: ReactElement, ...rest: any) => T;
 
 const WithProviders: FunctionComponent = ({ children }) => (
@@ -60,15 +56,4 @@ const axe = configureAxe({
   },
 });
 
-export {
-  create,
-  render,
-  renderToHtml,
-  renderHook,
-  act,
-  actHook,
-  userEvent,
-  fireEvent,
-  waitFor,
-  axe,
-};
+export { create, render, renderToHtml, renderHook, actHook, userEvent, axe };

yarn.lock

@@ -17904,7 +17904,7 @@ use-composed-ref@^1.0.0:
   dependencies:
     ts-essentials "^2.0.3"
 
-use-isomorphic-layout-effect@^1.0.0:
+use-isomorphic-layout-effect@^1.0.0, use-isomorphic-layout-effect@^1.1.0:
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/use-isomorphic-layout-effect/-/use-isomorphic-layout-effect-1.1.1.tgz#7bb6589170cd2987a152042f9084f9effb75c225"
   integrity sha512-L7Evj8FGcwo/wpbv/qvSfrkHFtOpCzvM5yl2KVyDJoylVuSvzphiiasmjgQPttIGBAy2WKiBNR98q8w7PiNgKQ==
@@ -17916,6 +17916,13 @@ use-latest@^1.0.0, use-latest@^1.2.0:
   dependencies:
     use-isomorphic-layout-effect "^1.0.0"
 
+use-previous@^1.1.0:
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/use-previous/-/use-previous-1.1.0.tgz#9d08a4cff2bc96679139053cc4ee3d97169ec661"
+  integrity sha512-t6ltKuYIpvcqD+VBnlUOaT+N94bN7HwUdB1u7rSpCIh9XvMJQIjsw/G8DoJlSDh029khFQ2L8q88oUWfKoWy5w==
+  dependencies:
+    use-isomorphic-layout-effect "^1.1.0"
+
 use-subscription@^1.3.0:
   version "1.5.0"
   resolved "https://registry.yarnpkg.com/use-subscription/-/use-subscription-1.5.0.tgz#0df66fdf97b9a340147ad72f76fac1db6f56d240"