DialogV2: re-set focusTrap when footer content changes

.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

packages/react/package.json

@@ -95,7 +95,7 @@
     "@github/tab-container-element": "^4.8.0",
     "@lit-labs/react": "1.2.1",
     "@oddbird/popover-polyfill": "^0.3.1",
-    "@primer/behaviors": "^1.7.0",
+    "@primer/behaviors": "^1.7.2",
     "@primer/live-region-element": "^0.7.0",
     "@primer/octicons-react": "^19.9.0",
     "@primer/primitives": "^7.16.0",

packages/react/src/Dialog/Dialog.features.stories.tsx

@@ -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}
@@ -164,6 +164,10 @@ export const ReproMultistepDialogWithConditionalFooter = ({width, height}: Dialo
   const onDialogClose = useCallback(() => setIsOpen(false), [])
   const [step, setStep] = React.useState(1)
 
+  const [inputText, setInputText] = React.useState('')
+
+  const dialogRef = useRef<HTMLDivElement>(null)
+
   const renderFooterConditionally = () => {
     if (step === 1) return null
 
@@ -174,6 +178,14 @@ export const ReproMultistepDialogWithConditionalFooter = ({width, height}: Dialo
     )
   }
 
+  React.useEffect(() => {
+    // focus the close button when the step changes
+    const focusTarget = dialogRef.current?.querySelector('button[aria-label="Close"]') as HTMLButtonElement
+    if (step === 2) {
+      focusTarget.focus()
+    }
+  }, [step])
+
   return (
     <>
       <Button onClick={() => setIsOpen(!isOpen)}>Show dialog</Button>
@@ -185,6 +197,7 @@ export const ReproMultistepDialogWithConditionalFooter = ({width, height}: Dialo
           renderFooter={renderFooterConditionally}
           onClose={onDialogClose}
           footerButtons={[{buttonType: 'primary', content: 'Proceed'}]}
+          ref={dialogRef}
         >
           {step === 1 ? (
             <Box sx={{display: 'flex', flexDirection: 'column', gap: 4}}>
@@ -196,12 +209,17 @@ export const ReproMultistepDialogWithConditionalFooter = ({width, height}: Dialo
               </Box>
             </Box>
           ) : (
-            <p>
+            <div>
               <Box sx={{display: 'flex', flexDirection: 'column', gap: 1}}>
                 <label htmlFor="description">Description</label>
-                <TextInput id="description" placeholder="Write the description here" />
+                <TextInput
+                  id="description"
+                  placeholder="Write the description here"
+                  value={inputText}
+                  onChange={event => setInputText(event.target.value)}
+                />
               </Box>
-            </p>
+            </div>
           )}
         </Dialog>
       )}
@@ -330,3 +348,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>
+      )}
+    </>
+  )
+}

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}

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,13 @@ const _Dialog = React.forwardRef<HTMLDivElement, React.PropsWithChildren<DialogP
           }),
         )
 
+  useFocusTrap({
+    containerRef: dialogRef,
+    initialFocusRef: initialFocusRef ?? autoFocusedFooterButtonRef,
+    restoreFocusOnCleanUp: returnFocusRef?.current ? false : true,
+    returnFocusRef,
+  })
+
   return (
     <>
       <Portal>