Skip to content

Commit

Permalink
DialogV2: re-set focusTrap when footer content changes (#4779)
Browse files Browse the repository at this point in the history
* Focus close button on second step

* Remove `autofocus` prop

* Add dependencies to focus trap

* Add changeset

* lint fix

* Remove `body` from dependency

* add state to Dialog example

* chore(changeset): enter prerelease mode for v37 (#4789)

* chore(changeset): enter pre-release mode for v37

* ci: remove snapshots when in pre mode

* chore: add version info to all packages

* Revert "chore: add version info to all packages"

This reverts commit 4665bb3.

* chore: update canary to remove pre.json when running

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* Autocomplete: Only open menu on click (#4771)

* Only open menu on click instead of just focus

* Update tests

* Add changeset

* Fix typo

* Set `openOnFocus` to `true`

* Add test, move `onFocus` function

* Update docs

* Adjust changeset

* Remove `useCallback`

* Add deprecated notice

* Prep for high contrast theme border-color changes (#4774)

* borders

* fallback

* test(vrt): update snapshots

* test color changes

* alright

* bump

* test(vrt): update snapshots

* more fixes

* test(vrt): update snapshots

* copy from main

* test(vrt): update snapshots

* Create fluffy-ravens-thank.md

* snippy snaps

* update seg control border

* test(vrt): update snapshots

* remove fallbacks

* copy from main

* test(vrt): update snapshots

---------

Co-authored-by: langermank <langermank@users.noreply.github.com>

* chore: add package version numbers (#4796)

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* feat: add postcss-preset-primer (#4751)

* feat: add postcss-preset-primer

* docs: update usage snippet

* chore: fix eslint rules and remove postcss-mixins

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>

* Utilize `aria-describedby` on all `ActionList` descriptions (#4666)

* Add `aria-describedby` to inline description; add `aria-labelledby` to `TrailingVisual`

* Update snapshots

* Add changeset

* Update snapshot

* Update packages/react/src/ActionList/Item.tsx

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

* changes from PR feedback

* Update .changeset/lovely-days-march.md

* Update snapshots

* Update lovely-days-march.md

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>

* Wrap `header` and `footer` with `<Box>`

* Move `<Box>`

* Add back focus to story

* Update behaviors package

* Move back `useFocusTrap`

---------

Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Co-authored-by: Josh Black <joshblack@github.com>
Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Co-authored-by: Katie Langerman <18661030+langermank@users.noreply.github.com>
Co-authored-by: langermank <langermank@users.noreply.github.com>
  • Loading branch information
6 people authored Sep 9, 2024
1 parent caf4bcf commit 551aff3
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 15 deletions.
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
12 changes: 4 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "^9.0.3",
Expand Down
79 changes: 75 additions & 4 deletions 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 All @@ -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

Expand All @@ -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>
Expand All @@ -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}}>
Expand All @@ -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>
)}
Expand Down Expand Up @@ -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>
)}
</>
)
}
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

0 comments on commit 551aff3

Please sign in to comment.