Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DialogV2: re-set focusTrap when footer content changes #4779

Merged
merged 20 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thirty-pets-impress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Adds dependencies to `Dialog` focus trap to ensure focus trap is reset when content within changes
55 changes: 54 additions & 1 deletion packages/react/src/Dialog/Dialog.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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>
)}
</>
)
}
4 changes: 2 additions & 2 deletions packages/react/src/Dialog/Dialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
17 changes: 10 additions & 7 deletions packages/react/src/Dialog/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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>
Expand Down
Loading