docs(react-dialog): add comprehensive nested dialogs documentation#36187
docs(react-dialog): add comprehensive nested dialogs documentation#36187paolo-aliprandi wants to merge 5 commits into
Conversation
Addresses issue #36119 by providing clear examples and best practices for nested dialog patterns with proper focus management. CHANGES: - Added DialogNestedDialogs.stories.tsx: Manual focus restoration example using useRestoreFocusSource() and useRestoreFocusTarget() hooks for programmatic dialog opens - Added DialogNestedDialogsWithTrigger.stories.tsx: DialogTrigger-based example showing automatic focus restoration (recommended approach) - Added DialogNestedDialogs.md: Comprehensive guide covering both approaches, best practices, accessibility considerations, and keyboard navigation testing - Updated DialogBestPractices.md: Changed unhelpful blanket "don't" rule to actionable guidance with links to working examples PROBLEM SOLVED: Developers previously saw "Don't open a Dialog from a Dialog" without knowing how to properly implement nested dialogs. Now they see specific guidance: - Use DialogTrigger for user-triggered opens (automatic focus) - Use focus hooks for programmatic opens (manual but explicit) This prevents confusion about focus loss in nested dialogs and provides clear working patterns for both common use cases. Closes #36119 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@microsoft-github-policy-service agree company="Microsoft" |
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
The DialogTrigger import was declared but never used in DialogNestedDialogs.stories.tsx, causing TypeScript strict mode errors in the RIT test suite. This example uses manual focus restoration hooks, not DialogTrigger. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Transient GitHub fetch 500 during actions/checkout caused a non-code failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| @@ -0,0 +1,94 @@ | |||
| # Nested Dialogs | |||
There was a problem hiding this comment.
I suggest removing this example since it is an anti-pattern and does not align with our guidance or recommendations. Instead, we should include a 'Programmatically open' example that explains if you use a dialog without a DialogTrigger, you are responsible for managing focus return. We offer hooks to assist with this, but it remains the consumer’s responsibility.
There was a problem hiding this comment.
We also have this in docs
So maybe we can just tweak the description a bit to make it easier to understand.
There was a problem hiding this comment.
Example removed. I improved the description of the existing section. @dmytrokirpa let me know if it is now clearer
…tation - Remove incorrect statement that nested dialogs should only close programmatically - Clarify that dialogs can be closed via Escape and backdrop clicks per a11y requirements - Restructure documentation to emphasize DialogTrigger as recommended approach - Add responsibility clarity for programmatic dialog opens with focus hooks - Update accessibility section to acknowledge keyboard close methods - Improve best practices guidance with clearer language Addresses: - Comment about keeping the 'don't use nested dialogs' recommendation but making it about proper focus management - Comment that dialogs must support Escape and backdrop clicks per a11y - Comment about explaining consumer responsibility for programmatic opens - Comment about leveraging existing documentation patterns Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nsolidate guidance - Remove DialogNestedDialogs.stories.tsx (programmatic hooks example) as anti-pattern - Remove export from index.stories.tsx - Update DialogTriggerOutsideDialog.md with responsibility messaging for programmatic opens - Clarify that developers using dialogs without DialogTrigger must manage focus restoration - This consolidates guidance - DialogTrigger is recommended, but if using programmatic opens, responsibility is on consumer Note: DialogNestedDialogsWithTrigger.stories.tsx remains as the recommended approach. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds nested dialog guidance to the v9 @fluentui/react-dialog Storybook docs to address confusion around focus restoration when stacking dialogs, with a new nested-dialog story and updated best-practices copy.
Changes:
- Adds a new
NestedDialogsWithTriggerstory demonstrating nested dialogs usingDialogTrigger. - Adds a new “Nested Dialogs” markdown guide and updates existing focus-restoration guidance for non-
DialogTriggerpatterns. - Updates Dialog best practices to replace the blanket “don’t nest dialogs” guidance with actionable focus-management guidance and a link to examples.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx | Exposes a new nested-dialog story export in the Dialog story index. |
| packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md | Updates guidance for programmatic/non-trigger usage and focus restoration responsibilities. |
| packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx | Adds a nested-dialog example using DialogTrigger for automatic focus restoration. |
| packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md | Adds a dedicated nested-dialogs documentation page with recommended patterns and examples. |
| packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md | Updates best-practices guidance to include nested-dialog focus-management recommendations and links. |
| **Your responsibilities:** | ||
|
|
||
| 1. **Control the open state** - React to the `onOpenChange` callback and ensure the `open` state reflects the dialog's visibility | ||
| 2. **Restore focus** - When the dialog closes, you must restore focus to the element that triggered the open. Use `useRestoreFocusTarget` on the trigger element and `useRestoreFocusSource` on the `DialogSurface`, or manually invoke `.focus()` on the target element. |
| </DialogSurface> | ||
|
|
||
| <Dialog open={innerOpen} onOpenChange={(e, data) => setInnerOpen(data.open)}> | ||
| <DialogSurface {...innerSourceAttrs}>{/* Inner dialog content */}</DialogSurface> | ||
| </Dialog> |
| const outerSourceAttrs = useRestoreFocusSource(); | ||
| const outerTargetAttrs = useRestoreFocusTarget(); | ||
| const innerSourceAttrs = useRestoreFocusSource(); | ||
| const innerTargetAttrs = useRestoreFocusTarget(); | ||
|
|
||
| return ( | ||
| <Dialog open={outerOpen} onOpenChange={(e, data) => setOuterOpen(data.open)}> | ||
| <Button {...outerTargetAttrs} onClick={() => setOuterOpen(true)}> | ||
| Open Outer Dialog | ||
| </Button> | ||
| <DialogSurface {...outerSourceAttrs}> | ||
| <Button {...innerTargetAttrs} onClick={() => setInnerOpen(true)}> |
| NestedDialogsWithTrigger.parameters = { | ||
| docs: { | ||
| description: { | ||
| story: [ | ||
| 'Using DialogTrigger for nested dialogs provides automatic focus restoration.', | ||
| 'This is the simpler and recommended approach when the dialogs are opened by user interaction.', | ||
| 'Focus management is handled automatically without needing useRestoreFocusSource and useRestoreFocusTarget hooks.', | ||
| ].join('\n'), | ||
| }, | ||
| }, |
|
|
||
| - Don't use more than three buttons between `DialogActions`. | ||
| - Don't open a `Dialog` from a `Dialog` | ||
| - Don't open nested `Dialog`s without proper focus management. Use `DialogTrigger` for automatic focus restoration when dialogs are opened by user interaction, or use `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks when opening dialogs programmatically. See the [Nested Dialogs](/docs/components-dialog--nested-dialogs) example for details. |
| export { TitleNoAction } from './DialogTitleNoAction.stories'; | ||
| export { Confirmation } from './DialogConfirmation.stories'; | ||
| export { MotionCustom } from './DialogMotionCustom.stories'; | ||
| export { NestedDialogsWithTrigger } from './DialogNestedDialogsWithTrigger.stories'; |
| This is a nested dialog inside the outer dialog. Focus will automatically be restored to the outer | ||
| dialog when this one closes thanks to DialogTrigger. |
| - **Screen reader users** - Focus announcements help users understand which dialog is active | ||
| - **Motor control users** - They depend on consistent focus behavior for reliable navigation | ||
|
|
||
| See [useRestoreFocusSource hook documentation](/docs/utilities-focus-management-userestorefocussource--docs) for more details on focus management utilities. |
Problem
Addresses issue #36119 by providing clear examples and best practices for nested dialog patterns with proper focus management.
Developers were confused about focus loss in nested dialogs because the only guidance was "Don't open a Dialog from a Dialog" without knowing how to do it properly.
Solution
This PR provides:
Documentation
useRestoreFocusSource()anduseRestoreFocusTarget()hooks for programmatic dialog opensBest Practices Update
Updated DialogBestPractices.md to replace unhelpful "don't open nested dialogs" rule with actionable guidance that links to working examples.
Impact
Testing
Closes #36119