Overlay: only prevent focus on open when component is open (#5636)

* only prevent focus when component is open

Otherwise, prevents focus from returning to given returnFocusRef.

* fixup callback

* add component tests asserting expected behavior

* update hook tests

* add changeset

* add dev story for preventFocusOnOpen

* document preventFocusOnOpen prop

Based on hooks docs:
* https://primer.style/guides/react/hooks/use-open-and-close-focus

Related:
* primer/design#926

.changeset/bright-terms-listen.md

@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Update useOpenAndCloseFocus hook to apply return focus when preventFocusOnOpen prop is given

packages/react/src/Overlay/Overlay.dev.stories.tsx

@@ -89,3 +89,145 @@ export const SxProps = (args: Args) => {
     </Box>
   )
 }
+
+export const PreventFocusOnOpen = (args: Args) => {
+  const [isOpen, setIsOpen] = useState(false)
+  const openButtonRef = useRef<HTMLButtonElement>(null)
+  const confirmButtonRef = useRef<HTMLButtonElement>(null)
+  const anchorRef = useRef<HTMLDivElement>(null)
+  const closeOverlay = () => setIsOpen(false)
+  const containerRef = useRef<HTMLDivElement>(null)
+
+  return (
+    <Box ref={anchorRef}>
+      <Button
+        ref={openButtonRef}
+        onClick={() => {
+          setIsOpen(!isOpen)
+        }}
+      >
+        Open overlay
+      </Button>
+      {isOpen || args.open ? (
+        <Overlay
+          initialFocusRef={confirmButtonRef}
+          returnFocusRef={openButtonRef}
+          ignoreClickRefs={[openButtonRef]}
+          onEscape={closeOverlay}
+          onClickOutside={closeOverlay}
+          width={args.width}
+          height={args.height}
+          aria-modal={args.role === 'dialog'}
+          aria-label={args.role === 'dialog' ? 'Sample overlay' : undefined}
+          preventFocusOnOpen={args.preventFocusOnOpen}
+          ref={containerRef}
+          {...args}
+        >
+          <Box
+            sx={{
+              width: ['350px', '500px'],
+            }}
+          >
+            <Box
+              sx={{
+                height: '100vh',
+                maxWidth: 'calc(-1rem + 100vw)',
+                display: 'flex',
+                justifyContent: 'center',
+                alignItems: 'center',
+              }}
+            >
+              <IconButton
+                aria-label="Close"
+                onClick={closeOverlay}
+                icon={XIcon}
+                variant="invisible"
+                sx={{
+                  position: 'absolute',
+                  left: '5px',
+                  top: '5px',
+                }}
+              />
+              <Box display="flex" flexDirection="column" alignItems="center">
+                <Text>Are you sure?</Text>
+                <Box display="flex" mt={2}>
+                  <Button variant="danger" onClick={closeOverlay} sx={{marginRight: 1}}>
+                    Cancel
+                  </Button>
+                  <Button onClick={closeOverlay} ref={confirmButtonRef} sx={{marginLeft: 1}}>
+                    Confirm
+                  </Button>
+                </Box>
+              </Box>
+            </Box>
+          </Box>
+        </Overlay>
+      ) : null}
+    </Box>
+  )
+}
+PreventFocusOnOpen.args = {
+  width: 'auto',
+  height: 'auto',
+  side: 'outside-bottom',
+  preventOverflow: 'false',
+  role: 'dialog',
+  visibility: 'visible',
+  open: false,
+  preventFocusOnOpen: false,
+}
+PreventFocusOnOpen.argTypes = {
+  width: {
+    type: {
+      name: 'enum',
+      value: ['small', 'medium', 'large', 'xlarge', 'xxlarge', 'auto'],
+    },
+  },
+  height: {
+    type: {
+      name: 'enum',
+      value: ['xsmall', 'small', 'medium', 'large', 'xlarge', 'auto', 'initial'],
+    },
+  },
+  side: {
+    type: {
+      name: 'enum',
+      value: [
+        'inside-top',
+        'inside-bottom',
+        'inside-left',
+        'inside-right',
+        'inside-center',
+        'outside-top',
+        'outside-bottom',
+        'outside-left',
+        'outside-right',
+      ],
+    },
+  },
+  open: {
+    control: false,
+    visible: false,
+  },
+  portalContainerName: {
+    control: false,
+  },
+  style: {
+    control: false,
+  },
+  preventOverflow: {
+    type: 'boolean',
+  },
+  role: {
+    type: 'string',
+  },
+  visibility: {
+    type: {
+      name: 'enum',
+      value: ['visible', 'hidden'],
+    },
+  },
+  preventFocusOnOpen: {
+    type: 'boolean',
+  },
+}

packages/react/src/Overlay/Overlay.test.tsx

@@ -13,10 +13,16 @@ type TestComponentSettings = {
   initialFocus?: 'button'
   width?: 'small' | 'medium' | 'large' | 'auto' | 'xlarge' | 'xxlarge'
   callback?: () => void
+  preventFocusOnOpen?: boolean
 }
-const TestComponent = ({initialFocus, width = 'small', callback}: TestComponentSettings) => {
+const TestComponent = ({
+  initialFocus,
+  width = 'small',
+  preventFocusOnOpen = undefined,
+  callback,
+}: TestComponentSettings) => {
   const [isOpen, setIsOpen] = useState(false)
-  const buttonRef = useRef<HTMLButtonElement>(null)
+  const openButtonRef = useRef<HTMLButtonElement>(null)
   const confirmButtonRef = useRef<HTMLButtonElement>(null)
   const anchorRef = useRef<HTMLDivElement>(null)
   const closeOverlay = () => {
@@ -29,18 +35,20 @@ const TestComponent = ({initialFocus, width = 'small', callback}: TestComponentS
     <ThemeProvider theme={theme}>
       <BaseStyles>
         <Box position="absolute" top={0} left={0} bottom={0} right={0} ref={anchorRef}>
-          <Button ref={buttonRef} onClick={() => setIsOpen(!isOpen)}>
+          <Button ref={openButtonRef} onClick={() => setIsOpen(!isOpen)}>
             open overlay
           </Button>
           <Button>outside</Button>
           {isOpen ? (
             <Overlay
               initialFocusRef={initialFocus === 'button' ? confirmButtonRef : undefined}
-              returnFocusRef={buttonRef}
-              ignoreClickRefs={[buttonRef]}
+              returnFocusRef={openButtonRef}
+              ignoreClickRefs={[openButtonRef]}
               onEscape={closeOverlay}
               onClickOutside={closeOverlay}
               width={width}
+              preventFocusOnOpen={preventFocusOnOpen}
+              role="dialog"
             >
               <Box display="flex" flexDirection="column" p={2}>
                 <Text>Are you sure?</Text>
@@ -66,7 +74,7 @@ describe('Overlay', () => {
     expect(results).toHaveNoViolations()
   })
 
-  it('should focus element passed into function', async () => {
+  it('should focus initialFocusRef element passed into function on open', async () => {
     const user = userEvent.setup()
     const {getByRole} = render(<TestComponent initialFocus="button" />)
     await user.click(getByRole('button', {name: 'open overlay'}))
@@ -75,7 +83,7 @@ describe('Overlay', () => {
     expect(document.activeElement).toEqual(confirmButton)
   })
 
-  it('should focus first element when nothing is passed', async () => {
+  it('should focus first element on open when no initialFocusRef is passed', async () => {
     const user = userEvent.setup()
     const {getByRole} = render(<TestComponent />)
     await user.click(getByRole('button', {name: 'open overlay'}))
@@ -84,6 +92,53 @@ describe('Overlay', () => {
     expect(document.activeElement).toEqual(cancelButton)
   })
 
+  it('should not focus any element within the overlay on open when preventFocusOnOpen prop is true', async () => {
+    const user = userEvent.setup()
+    const {getByRole} = render(<TestComponent initialFocus="button" preventFocusOnOpen={true} />)
+    await user.click(getByRole('button', {name: 'open overlay'}))
+
+    const overlayContainer = getByRole('dialog')
+    const focusedElement = document.activeElement! as HTMLElement
+    expect(focusedElement).toBeInTheDocument()
+    expect(overlayContainer).not.toContainElement(focusedElement)
+  })
+
+  it('should focus returnFocusRef element on close', async () => {
+    const user = userEvent.setup()
+    const {getByRole} = render(<TestComponent />)
+
+    // Open overlay
+    await waitFor(() => getByRole('button', {name: 'open overlay'}))
+    const openButton = getByRole('button', {name: 'open overlay'})
+    await user.click(openButton)
+
+    // Close overlay
+    await waitFor(() => getByRole('button', {name: 'Cancel'}))
+    const cancelButton = getByRole('button', {name: 'Cancel'})
+    await user.click(cancelButton)
+
+    // Focus should return to button that was originally clicked to open overlay
+    expect(document.activeElement).toEqual(openButton)
+  })
+
+  it('should focus returnFocusRef element on close when preventFocusOnOpen prop is true', async () => {
+    const user = userEvent.setup()
+    const {getByRole} = render(<TestComponent initialFocus="button" preventFocusOnOpen={true} />)
+
+    // Open overlay
+    await waitFor(() => getByRole('button', {name: 'open overlay'}))
+    const openButton = getByRole('button', {name: 'open overlay'})
+    await user.click(openButton)
+
+    // Close overlay
+    await waitFor(() => getByRole('button', {name: 'Cancel'}))
+    const cancelButton = getByRole('button', {name: 'Cancel'})
+    await user.click(cancelButton)
+
+    // Focus should return to button that was originally clicked to open overlay
+    expect(document.activeElement).toEqual(openButton)
+  })
+
   it('should call function when user clicks outside container', async () => {
     const user = userEvent.setup()
     const mockFunction = jest.fn()
@@ -296,7 +351,7 @@ describe('Overlay', () => {
 
     await user.click(getByRole('button', {name: 'open overlay'}))
 
-    const container = getByRole('none')
+    const container = getByRole('dialog')
     expect(container).not.toHaveAttribute('data-reflow-container')
   })
 
@@ -310,7 +365,7 @@ describe('Overlay', () => {
 
     await user.click(getByRole('button', {name: 'open overlay'}))
 
-    const container = getByRole('none')
+    const container = getByRole('dialog')
     expect(container).toHaveAttribute('data-reflow-container')
   })
 })

packages/react/src/Overlay/Overlay.tsx

@@ -232,6 +232,7 @@ type internalOverlayProps = Merge<OwnOverlayProps, ContainerProps>
  * @param onEscape Required. Function to call when user presses `Escape`. Typically this function removes the Overlay.
  * @param portalContainerName Optional. The name of the portal container to render the Overlay into.
  * @param preventOverflow Optional. The Overlay width will be adjusted responsively if there is not enough space to display the Overlay. If `preventOverflow` is `true`, the width of the `Overlay` will not be adjusted.
+ * @param preventFocusOnOpen Optional. If 'true', focus will not be applied when the component is first mounted, even if initialFocusRef prop is given.
  * @param returnFocusRef Required. Ref for the element to focus when the `Overlay` is closed.
  * @param right Optional. Horizontal right position of the overlay, relative to its closest positioned ancestor (often its `Portal`).
  * @param width Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `480px`, `xlarge` corresponds to `640px`, `xxlarge` corresponds to `960px`.

packages/react/src/__tests__/hooks/useOpenAndCloseFocus.test.tsx

@@ -1,5 +1,6 @@
-import React, {useRef} from 'react'
+import React, {useRef, useState} from 'react'
 import {render, waitFor} from '@testing-library/react'
+import userEvent from '@testing-library/user-event'
 import {useOpenAndCloseFocus} from '../../hooks/useOpenAndCloseFocus'
 
 const Component = () => {
@@ -39,16 +40,59 @@ const ComponentTwo = () => {
   )
 }
 
-it('should focus element passed into function', async () => {
+const ComponentThree = () => {
+  const [isOpen, setIsOpen] = useState(true)
+
+  const containerRef = useRef<HTMLDivElement>(null)
+  const returnFocusRef = useRef<HTMLButtonElement>(null)
+  const noButtonRef = useRef<HTMLButtonElement>(null)
+  useOpenAndCloseFocus({containerRef, initialFocusRef: noButtonRef, returnFocusRef, preventFocusOnOpen: true})
+
+  return (
+    <>
+      <button ref={returnFocusRef} type="button" onClick={() => setIsOpen(!isOpen)}>
+        toggle
+      </button>
+      {isOpen && (
+        <div ref={containerRef}>
+          <button type="button">yes</button>
+          <button ref={noButtonRef} type="button">
+            no
+          </button>
+        </div>
+      )}
+    </>
+  )
+}
+
+it('should focus initialFocusRef element passed into function', async () => {
   const {getByText} = render(<Component />)
   await waitFor(() => getByText('no'))
   const noButton = getByText('no')
   expect(document.activeElement).toEqual(noButton)
 })
 
-it('should focus first element when nothing is passed', async () => {
+it('should focus first element when no initialFocusRef prop is passed', async () => {
   const {getByText} = render(<ComponentTwo />)
   await waitFor(() => getByText('yes'))
   const yesButton = getByText('yes')
   expect(document.activeElement).toEqual(yesButton)
 })
+
+it('should not focus any element if preventFocusOnOpen prop is passed', async () => {
+  render(<ComponentThree />)
+  expect(document.activeElement).toEqual(document.body)
+})
+
+it('should focus returnFocusRef element when rendered', async () => {
+  const user = userEvent.setup()
+  const {getByText} = render(<ComponentThree />)
+
+  await waitFor(() => getByText('toggle'))
+  const toggleButton = getByText('toggle')
+
+  // Close container, so containerRef and initialFocusRef elements are no longer rendered
+  await user.click(toggleButton)
+
+  expect(document.activeElement).toEqual(toggleButton)
+})

packages/react/src/hooks/useOpenAndCloseFocus.ts

@@ -16,18 +16,21 @@ export function useOpenAndCloseFocus({
   preventFocusOnOpen,
 }: UseOpenAndCloseFocusSettings): void {
   useEffect(() => {
-    if (preventFocusOnOpen) {
-      return
-    }
-    const returnRef = returnFocusRef.current
-    if (initialFocusRef && initialFocusRef.current) {
-      initialFocusRef.current.focus()
-    } else if (containerRef.current) {
-      const firstItem = iterateFocusableElements(containerRef.current).next().value
-      firstItem?.focus()
+    // If focus should be applied on open, apply focus to correct element,
+    // either the initialFocusRef if given, otherwise the first focusable element
+    if (!preventFocusOnOpen) {
+      if (initialFocusRef && initialFocusRef.current) {
+        initialFocusRef.current.focus()
+      } else if (containerRef.current) {
+        const firstItem = iterateFocusableElements(containerRef.current).next().value
+        firstItem?.focus()
+      }
     }
+
+    // If returnFocusRef element is rendered, apply focus
+    const returnFocusRefCurrent = returnFocusRef.current
     return function () {
-      returnRef?.focus()
+      returnFocusRefCurrent?.focus()
     }
   }, [initialFocusRef, returnFocusRef, containerRef, preventFocusOnOpen])
 }