Merge e066cd2 into 0b57996

primer · Jul 29, 2024 · f738446 · f738446
2 parents 0b57996 + e066cd2
commit f738446
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 11 deletions.
diff --git a/.changeset/thirty-pets-impress.md b/.changeset/thirty-pets-impress.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+Adds dependencies to `Dialog` focus trap to ensure focus trap is reset when content within changes
diff --git a/packages/react/src/Dialog/Dialog.features.stories.tsx b/packages/react/src/Dialog/Dialog.features.stories.tsx
@@ -1,4 +1,4 @@
-import React, {useState, useRef, useCallback} from 'react'
+import React, {useState, useRef, useCallback, useEffect} from 'react'
 import {Box, TextInput, Text, Button, ActionList} from '..'
 import type {DialogProps, DialogWidth, DialogHeight} from './Dialog'
 import {Dialog} from './Dialog'
@@ -143,7 +143,7 @@ export const StressTest = ({width, height, subtitle}: DialogStoryProps) => {
           footerButtons={[
             ...manyButtons,
             {buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
-            {buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog, autoFocus: true},
+            {buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog},
           ]}
         >
           {lipsum}
@@ -330,3 +330,56 @@ export const NewIssues = () => {
     </>
   )
 }
+
+export const RetainsFocusTrapWithDynamicContent = () => {
+  const [isOpen, setIsOpen] = useState(false)
+  const [secondOpen, setSecondOpen] = useState(false)
+  const [expandContent, setExpandContent] = useState(false)
+  const [changeBodyContent, setChangeBodyContent] = useState(false)
+
+  const buttonRef = useRef<HTMLButtonElement>(null)
+  const onDialogClose = useCallback(() => setIsOpen(false), [])
+  const onSecondDialogClose = useCallback(() => setSecondOpen(false), [])
+  const openSecondDialog = useCallback(() => setSecondOpen(true), [])
+
+  const renderFooterConditionally = () => {
+    if (!changeBodyContent) return null
+
+    return (
+      <Dialog.Footer>
+        <Button variant="primary">Submit</Button>
+      </Dialog.Footer>
+    )
+  }
+
+  return (
+    <>
+      <Button ref={buttonRef} onClick={() => setIsOpen(!isOpen)}>
+        Show dialog
+      </Button>
+      {isOpen && (
+        <Dialog title="My Dialog" onClose={onDialogClose} renderFooter={renderFooterConditionally}>
+          <Button onClick={() => setExpandContent(!expandContent)}>
+            Click me to dynamically {expandContent ? 'remove' : 'render'} content
+          </Button>
+          <Button onClick={() => setChangeBodyContent(!changeBodyContent)}>
+            Click me to {changeBodyContent ? 'remove' : 'add'} a footer
+          </Button>
+          <Button onClick={openSecondDialog}>Click me to open a new dialog</Button>
+          {expandContent && (
+            <Box>
+              {lipsum}
+              <Button>Dialog Button Example 1</Button>
+              <Button>Dialog Button Example 2</Button>
+            </Box>
+          )}
+          {secondOpen && (
+            <Dialog title="Inner dialog!" onClose={onSecondDialogClose} width="small">
+              Hello world
+            </Dialog>
+          )}
+        </Dialog>
+      )}
+    </>
+  )
+}
diff --git a/packages/react/src/Dialog/Dialog.stories.tsx b/packages/react/src/Dialog/Dialog.stories.tsx
@@ -77,7 +77,7 @@ export const Default = () => {
           footerButtons={[
             {buttonType: 'default', content: 'Open Second Dialog', onClick: openSecondDialog},
             {buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
-            {buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog, autoFocus: true},
+            {buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog},
           ]}
         >
           {lipsum}
@@ -119,7 +119,7 @@ export const Playground = (
           footerButtons={[
             {buttonType: 'default', content: 'Open Second Dialog', onClick: openSecondDialog},
             {buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
-            {buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog, autoFocus: true},
+            {buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog},
           ]}
         >
           {lipsum}

diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx
@@ -433,13 +433,6 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
   useRefObjectAsForwardedRef(forwardedRef, dialogRef)
   const backdropRef = useRef<HTMLDivElement>(null)
 
-  useFocusTrap({
-    containerRef: dialogRef,
-    initialFocusRef: initialFocusRef ?? autoFocusedFooterButtonRef,
-    restoreFocusOnCleanUp: returnFocusRef?.current ? false : true,
-    returnFocusRef,
-  })
-
   useOnEscapePress(
     (event: KeyboardEvent) => {
       onClose('escape')
@@ -476,6 +469,16 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
           }),
         )
 
+  useFocusTrap(
+    {
+      containerRef: dialogRef,
+      initialFocusRef: initialFocusRef ?? autoFocusedFooterButtonRef,
+      restoreFocusOnCleanUp: returnFocusRef?.current ? false : true,
+      returnFocusRef,
+    },
+    [dialogRef, body, footer],
+  )
+
   return (
     <>
       <Portal>