Skip to content

Commit 551aff3

Browse files
TylerJDevsiddharthkpjoshblacklangermank
authored
DialogV2: re-set focusTrap when footer content changes (#4779)
* 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 <[email protected]> * 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 <[email protected]> * chore: add package version numbers (#4796) Co-authored-by: Josh Black <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * changes from PR feedback * Update .changeset/lovely-days-march.md * Update snapshots * Update lovely-days-march.md --------- Co-authored-by: Siddharth Kshetrapal <[email protected]> * Wrap `header` and `footer` with `<Box>` * Move `<Box>` * Add back focus to story * Update behaviors package * Move back `useFocusTrap` --------- Co-authored-by: Siddharth Kshetrapal <[email protected]> Co-authored-by: Josh Black <[email protected]> Co-authored-by: Josh Black <[email protected]> Co-authored-by: Katie Langerman <[email protected]> Co-authored-by: langermank <[email protected]>
1 parent caf4bcf commit 551aff3

File tree

5 files changed

+87
-15
lines changed

5 files changed

+87
-15
lines changed

.changeset/thirty-pets-impress.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': minor
3+
---
4+
5+
Adds dependencies to `Dialog` focus trap to ensure focus trap is reset when content within changes

package-lock.json

Lines changed: 4 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/react/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@
9595
"@github/tab-container-element": "^4.8.0",
9696
"@lit-labs/react": "1.2.1",
9797
"@oddbird/popover-polyfill": "^0.3.1",
98-
"@primer/behaviors": "^1.7.0",
98+
"@primer/behaviors": "^1.7.2",
9999
"@primer/live-region-element": "^0.7.0",
100100
"@primer/octicons-react": "^19.9.0",
101101
"@primer/primitives": "^9.0.3",

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

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ export const StressTest = ({width, height, subtitle}: DialogStoryProps) => {
143143
footerButtons={[
144144
...manyButtons,
145145
{buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
146-
{buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog, autoFocus: true},
146+
{buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog},
147147
]}
148148
>
149149
{lipsum}
@@ -164,6 +164,10 @@ export const ReproMultistepDialogWithConditionalFooter = ({width, height}: Dialo
164164
const onDialogClose = useCallback(() => setIsOpen(false), [])
165165
const [step, setStep] = React.useState(1)
166166

167+
const [inputText, setInputText] = React.useState('')
168+
169+
const dialogRef = useRef<HTMLDivElement>(null)
170+
167171
const renderFooterConditionally = () => {
168172
if (step === 1) return null
169173

@@ -174,6 +178,14 @@ export const ReproMultistepDialogWithConditionalFooter = ({width, height}: Dialo
174178
)
175179
}
176180

181+
React.useEffect(() => {
182+
// focus the close button when the step changes
183+
const focusTarget = dialogRef.current?.querySelector('button[aria-label="Close"]') as HTMLButtonElement
184+
if (step === 2) {
185+
focusTarget.focus()
186+
}
187+
}, [step])
188+
177189
return (
178190
<>
179191
<Button onClick={() => setIsOpen(!isOpen)}>Show dialog</Button>
@@ -185,6 +197,7 @@ export const ReproMultistepDialogWithConditionalFooter = ({width, height}: Dialo
185197
renderFooter={renderFooterConditionally}
186198
onClose={onDialogClose}
187199
footerButtons={[{buttonType: 'primary', content: 'Proceed'}]}
200+
ref={dialogRef}
188201
>
189202
{step === 1 ? (
190203
<Box sx={{display: 'flex', flexDirection: 'column', gap: 4}}>
@@ -196,12 +209,17 @@ export const ReproMultistepDialogWithConditionalFooter = ({width, height}: Dialo
196209
</Box>
197210
</Box>
198211
) : (
199-
<p>
212+
<div>
200213
<Box sx={{display: 'flex', flexDirection: 'column', gap: 1}}>
201214
<label htmlFor="description">Description</label>
202-
<TextInput id="description" placeholder="Write the description here" />
215+
<TextInput
216+
id="description"
217+
placeholder="Write the description here"
218+
value={inputText}
219+
onChange={event => setInputText(event.target.value)}
220+
/>
203221
</Box>
204-
</p>
222+
</div>
205223
)}
206224
</Dialog>
207225
)}
@@ -330,3 +348,56 @@ export const NewIssues = () => {
330348
</>
331349
)
332350
}
351+
352+
export const RetainsFocusTrapWithDynamicContent = () => {
353+
const [isOpen, setIsOpen] = useState(false)
354+
const [secondOpen, setSecondOpen] = useState(false)
355+
const [expandContent, setExpandContent] = useState(false)
356+
const [changeBodyContent, setChangeBodyContent] = useState(false)
357+
358+
const buttonRef = useRef<HTMLButtonElement>(null)
359+
const onDialogClose = useCallback(() => setIsOpen(false), [])
360+
const onSecondDialogClose = useCallback(() => setSecondOpen(false), [])
361+
const openSecondDialog = useCallback(() => setSecondOpen(true), [])
362+
363+
const renderFooterConditionally = () => {
364+
if (!changeBodyContent) return null
365+
366+
return (
367+
<Dialog.Footer>
368+
<Button variant="primary">Submit</Button>
369+
</Dialog.Footer>
370+
)
371+
}
372+
373+
return (
374+
<>
375+
<Button ref={buttonRef} onClick={() => setIsOpen(!isOpen)}>
376+
Show dialog
377+
</Button>
378+
{isOpen && (
379+
<Dialog title="My Dialog" onClose={onDialogClose} renderFooter={renderFooterConditionally}>
380+
<Button onClick={() => setExpandContent(!expandContent)}>
381+
Click me to dynamically {expandContent ? 'remove' : 'render'} content
382+
</Button>
383+
<Button onClick={() => setChangeBodyContent(!changeBodyContent)}>
384+
Click me to {changeBodyContent ? 'remove' : 'add'} a footer
385+
</Button>
386+
<Button onClick={openSecondDialog}>Click me to open a new dialog</Button>
387+
{expandContent && (
388+
<Box>
389+
{lipsum}
390+
<Button>Dialog Button Example 1</Button>
391+
<Button>Dialog Button Example 2</Button>
392+
</Box>
393+
)}
394+
{secondOpen && (
395+
<Dialog title="Inner dialog!" onClose={onSecondDialogClose} width="small">
396+
Hello world
397+
</Dialog>
398+
)}
399+
</Dialog>
400+
)}
401+
</>
402+
)
403+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export const Default = () => {
7777
footerButtons={[
7878
{buttonType: 'default', content: 'Open Second Dialog', onClick: openSecondDialog},
7979
{buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
80-
{buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog, autoFocus: true},
80+
{buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog},
8181
]}
8282
>
8383
{lipsum}
@@ -119,7 +119,7 @@ export const Playground = (
119119
footerButtons={[
120120
{buttonType: 'default', content: 'Open Second Dialog', onClick: openSecondDialog},
121121
{buttonType: 'danger', content: 'Delete the universe', onClick: onDialogClose},
122-
{buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog, autoFocus: true},
122+
{buttonType: 'primary', content: 'Proceed', onClick: openSecondDialog},
123123
]}
124124
>
125125
{lipsum}

0 commit comments

Comments
 (0)