From 5ec400dcc5bc7d9f5cc231b145b6241203ff7915 Mon Sep 17 00:00:00 2001 From: paliprandi_microsoft Date: Mon, 11 May 2026 16:08:54 +0200 Subject: [PATCH 1/7] docs(react-dialog): add comprehensive nested dialogs documentation 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> --- .../stories/src/Dialog/DialogBestPractices.md | 2 +- .../stories/src/Dialog/DialogNestedDialogs.md | 94 +++++++++++++++++++ .../Dialog/DialogNestedDialogs.stories.tsx | 81 ++++++++++++++++ ...DialogNestedDialogsWithTrigger.stories.tsx | 71 ++++++++++++++ .../stories/src/Dialog/index.stories.tsx | 2 + 5 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md create mode 100644 packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx create mode 100644 packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md index 3717da47e72ef..50cfffce2f956 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md @@ -11,5 +11,5 @@ ### Don't - 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. If you need nested dialogs, use `DialogTrigger` for automatic focus restoration or `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks for programmatic control. See the [Nested Dialogs](/docs/components-dialog--nested-dialogs) example for details. - Don't use a `Dialog` with no focusable elements diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md new file mode 100644 index 0000000000000..7c9d8646a387b --- /dev/null +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md @@ -0,0 +1,94 @@ +# Nested Dialogs + +When implementing nested dialogs (a dialog opened from within another dialog), proper focus management is critical to ensure users can navigate back through the dialog stack correctly. + +## Important: Nested Dialogs Should Be Closed Programmatically + +Nested dialogs should **always be closed programmatically** through state management, not manually by clicking outside or pressing Escape. This ensures predictable focus behavior and a better user experience. + +## Two Approaches + +### Approach 1: Using DialogTrigger (Recommended for User-Triggered Opens) + +If the nested dialog is opened by a button click (user interaction), use `DialogTrigger` for automatic focus restoration: + +```tsx + + + + + + + Outer Dialog + + + + + {/* Inner dialog content */} + + + + +``` + +**Benefits:** + +- Focus is automatically restored when dialogs close +- Simpler to implement +- No need for manual focus management hooks + +### Approach 2: Using Focus Restoration Hooks (For Programmatic Control) + +If the nested dialog is opened programmatically (not triggered by user interaction), use `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks: + +```tsx +const [outerOpen, setOuterOpen] = React.useState(false); +const [innerOpen, setInnerOpen] = React.useState(false); + +const outerSourceAttrs = useRestoreFocusSource(); +const outerTargetAttrs = useRestoreFocusTarget(); +const innerSourceAttrs = useRestoreFocusSource(); +const innerTargetAttrs = useRestoreFocusTarget(); + +return ( + setOuterOpen(data.open)}> + + + + + + setInnerOpen(data.open)}> + {/* Inner dialog content */} + + +); +``` + +**How it works:** + +- `useRestoreFocusTarget()` attributes go on the button/element that opens the dialog +- `useRestoreFocusSource()` attributes go on the DialogSurface +- When the dialog closes, focus returns to the element with `useRestoreFocusTarget()` attributes +- Each dialog pair (source/target) manages its own focus restoration + +## Best Practices + +1. **Use DialogTrigger by default** - It provides automatic focus restoration for user-triggered opens +2. **Use focus hooks for programmatic dialogs** - When you open dialogs from code (not user clicks), use the focus restoration hooks +3. **Always apply focus attributes** - Don't skip focus management; it's essential for accessibility +4. **Close dialogs programmatically** - Don't rely on manual close (clicking outside, pressing Escape) for nested dialogs +5. **Test with keyboard navigation** - Verify that Tab and Shift+Tab work correctly through the dialog stack + +## Accessibility + +Proper focus management in nested dialogs is crucial for: + +- **Keyboard users** - They can navigate back through the dialog stack using Tab +- **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. diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx new file mode 100644 index 0000000000000..caa0539808f47 --- /dev/null +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx @@ -0,0 +1,81 @@ +import * as React from 'react'; +import type { JSXElement } from '@fluentui/react-components'; +import { + Dialog, + DialogTrigger, + DialogSurface, + DialogTitle, + DialogBody, + DialogContent, + DialogActions, + Button, + useRestoreFocusSource, + useRestoreFocusTarget, +} from '@fluentui/react-components'; +import story from './DialogNestedDialogs.md'; + +export const NestedDialogs = (): JSXElement => { + const [outerOpen, setOuterOpen] = React.useState(false); + const [innerOpen, setInnerOpen] = React.useState(false); + + // Focus restoration for outer dialog + const outerRestoreFocusSourceAttributes = useRestoreFocusSource(); + const outerRestoreFocusTargetAttributes = useRestoreFocusTarget(); + + // Focus restoration for inner dialog + const innerRestoreFocusSourceAttributes = useRestoreFocusSource(); + const innerRestoreFocusTargetAttributes = useRestoreFocusTarget(); + + return ( +
+ {/* Outer Dialog */} + setOuterOpen(data.open)}> + + + + + Outer Dialog + This is the outer dialog. Click the button below to open a nested dialog. + + + + + + + + + {/* Inner Dialog */} + setInnerOpen(data.open)}> + + + Inner Dialog + + This is a nested dialog inside the outer dialog. Focus will be restored to the outer dialog when this one + closes. + + + + + + + + +
+ ); +}; + +NestedDialogs.parameters = { + docs: { + description: { + story, + }, + }, +}; diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx new file mode 100644 index 0000000000000..88aaba34cfd63 --- /dev/null +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx @@ -0,0 +1,71 @@ +import * as React from 'react'; +import type { JSXElement } from '@fluentui/react-components'; +import { + Dialog, + DialogTrigger, + DialogSurface, + DialogTitle, + DialogBody, + DialogContent, + DialogActions, + Button, +} from '@fluentui/react-components'; + +export const NestedDialogsWithTrigger = (): JSXElement => { + return ( + + + + + + + + Outer Dialog + + This is the outer dialog. Click the button below to open a nested dialog. When using DialogTrigger, focus is + automatically restored. + + + + + + + + + + Inner Dialog + + 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. + + + + + + + + + + + + + + + + + + + ); +}; + +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'), + }, + }, +}; diff --git a/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx index 653aa7e52a251..6f1fc4419b783 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx +++ b/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx @@ -23,6 +23,8 @@ export { TitleCustomAction } from './DialogTitleCustomAction.stories'; export { TitleNoAction } from './DialogTitleNoAction.stories'; export { Confirmation } from './DialogConfirmation.stories'; export { MotionCustom } from './DialogMotionCustom.stories'; +export { NestedDialogs } from './DialogNestedDialogs.stories'; +export { NestedDialogsWithTrigger } from './DialogNestedDialogsWithTrigger.stories'; // Typing with Meta generates a type error for the `subcomponents` property. // https://github.com/storybookjs/storybook/issues/27535 From cb6dcd3b1bdedb0a66faf4d338affa8b36e98691 Mon Sep 17 00:00:00 2001 From: paliprandi_microsoft Date: Mon, 11 May 2026 16:23:56 +0200 Subject: [PATCH 2/7] fix: remove unused DialogTrigger import 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> --- .../stories/src/Dialog/DialogNestedDialogs.stories.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx index caa0539808f47..2d545208ce219 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx @@ -2,7 +2,6 @@ import * as React from 'react'; import type { JSXElement } from '@fluentui/react-components'; import { Dialog, - DialogTrigger, DialogSurface, DialogTitle, DialogBody, From 7fdda0070b94094db59b7ad725d1eaed4393166b Mon Sep 17 00:00:00 2001 From: paliprandi_microsoft Date: Mon, 11 May 2026 16:38:00 +0200 Subject: [PATCH 3/7] chore: retrigger flaky CodeQL check Transient GitHub fetch 500 during actions/checkout caused a non-code failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> From 4089a0b699cc207071d941bcc34379d5ef055989 Mon Sep 17 00:00:00 2001 From: paliprandi_microsoft Date: Tue, 12 May 2026 11:26:49 +0200 Subject: [PATCH 4/7] docs(react-dialog): address review comments on nested dialogs documentation - 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> --- .../stories/src/Dialog/DialogBestPractices.md | 2 +- .../stories/src/Dialog/DialogNestedDialogs.md | 23 ++++++++----------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md index 50cfffce2f956..3c6c7212d91ca 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md @@ -11,5 +11,5 @@ ### Don't - Don't use more than three buttons between `DialogActions`. -- Don't open nested `Dialog`s without proper focus management. If you need nested dialogs, use `DialogTrigger` for automatic focus restoration or `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks for programmatic control. See the [Nested Dialogs](/docs/components-dialog--nested-dialogs) example for details. +- 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. - Don't use a `Dialog` with no focusable elements diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md index 7c9d8646a387b..4be892980c076 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md @@ -1,16 +1,14 @@ # Nested Dialogs -When implementing nested dialogs (a dialog opened from within another dialog), proper focus management is critical to ensure users can navigate back through the dialog stack correctly. +Nested dialogs (opening a dialog from within another dialog) require proper focus management to ensure accessibility and predictable user experience. -## Important: Nested Dialogs Should Be Closed Programmatically +## Key Principle -Nested dialogs should **always be closed programmatically** through state management, not manually by clicking outside or pressing Escape. This ensures predictable focus behavior and a better user experience. +When using nested dialogs, **always use `DialogTrigger` for focus restoration**. If you open a dialog programmatically without `DialogTrigger`, you become responsible for managing focus restoration using `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks. -## Two Approaches +## Recommended: Using DialogTrigger -### Approach 1: Using DialogTrigger (Recommended for User-Triggered Opens) - -If the nested dialog is opened by a button click (user interaction), use `DialogTrigger` for automatic focus restoration: +Use `DialogTrigger` for opening nested dialogs. This provides automatic focus restoration when dialogs close: ```tsx @@ -35,11 +33,11 @@ If the nested dialog is opened by a button click (user interaction), use `Dialog - Focus is automatically restored when dialogs close - Simpler to implement -- No need for manual focus management hooks +- No manual focus management needed -### Approach 2: Using Focus Restoration Hooks (For Programmatic Control) +## Programmatic Control: Using Focus Restoration Hooks -If the nested dialog is opened programmatically (not triggered by user interaction), use `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks: +If you must open dialogs programmatically (without user click), use `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks. Note that you are responsible for ensuring focus is correctly restored: ```tsx const [outerOpen, setOuterOpen] = React.useState(false); @@ -80,14 +78,13 @@ return ( 1. **Use DialogTrigger by default** - It provides automatic focus restoration for user-triggered opens 2. **Use focus hooks for programmatic dialogs** - When you open dialogs from code (not user clicks), use the focus restoration hooks 3. **Always apply focus attributes** - Don't skip focus management; it's essential for accessibility -4. **Close dialogs programmatically** - Don't rely on manual close (clicking outside, pressing Escape) for nested dialogs -5. **Test with keyboard navigation** - Verify that Tab and Shift+Tab work correctly through the dialog stack +4. **Test with keyboard navigation** - Verify that Escape key and backdrop clicks work correctly, and Tab/Shift+Tab navigate through the dialog stack ## Accessibility Proper focus management in nested dialogs is crucial for: -- **Keyboard users** - They can navigate back through the dialog stack using Tab +- **Keyboard users** - They can close dialogs with Escape and navigate through the dialog stack using Tab - **Screen reader users** - Focus announcements help users understand which dialog is active - **Motor control users** - They depend on consistent focus behavior for reliable navigation From 4383df2f05c46ea32044e0df863d52bd489a5de0 Mon Sep 17 00:00:00 2001 From: Paolo Aliprandi Date: Tue, 12 May 2026 11:38:18 +0200 Subject: [PATCH 5/7] docs(react-dialog): remove nested dialogs anti-pattern example and consolidate 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> --- .../Dialog/DialogNestedDialogs.stories.tsx | 80 ------------------- .../src/Dialog/DialogTriggerOutsideDialog.md | 13 ++- .../stories/src/Dialog/index.stories.tsx | 1 - 3 files changed, 9 insertions(+), 85 deletions(-) delete mode 100644 packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx deleted file mode 100644 index 2d545208ce219..0000000000000 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.stories.tsx +++ /dev/null @@ -1,80 +0,0 @@ -import * as React from 'react'; -import type { JSXElement } from '@fluentui/react-components'; -import { - Dialog, - DialogSurface, - DialogTitle, - DialogBody, - DialogContent, - DialogActions, - Button, - useRestoreFocusSource, - useRestoreFocusTarget, -} from '@fluentui/react-components'; -import story from './DialogNestedDialogs.md'; - -export const NestedDialogs = (): JSXElement => { - const [outerOpen, setOuterOpen] = React.useState(false); - const [innerOpen, setInnerOpen] = React.useState(false); - - // Focus restoration for outer dialog - const outerRestoreFocusSourceAttributes = useRestoreFocusSource(); - const outerRestoreFocusTargetAttributes = useRestoreFocusTarget(); - - // Focus restoration for inner dialog - const innerRestoreFocusSourceAttributes = useRestoreFocusSource(); - const innerRestoreFocusTargetAttributes = useRestoreFocusTarget(); - - return ( -
- {/* Outer Dialog */} - setOuterOpen(data.open)}> - - - - - Outer Dialog - This is the outer dialog. Click the button below to open a nested dialog. - - - - - - - - - {/* Inner Dialog */} - setInnerOpen(data.open)}> - - - Inner Dialog - - This is a nested dialog inside the outer dialog. Focus will be restored to the outer dialog when this one - closes. - - - - - - - - -
- ); -}; - -NestedDialogs.parameters = { - docs: { - description: { - story, - }, - }, -}; diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md index d0ff162cbbe8a..58aaab450ad20 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md @@ -1,6 +1,11 @@ -When using a `Dialog` without a `DialogTrigger` (or when using a `DialogTrigger` outside of a `Dialog`), it becomes your responsibility to control some of the dialog's behavior. +When using a `Dialog` without a `DialogTrigger`, you become responsible for managing the dialog's behavior. This applies to: -1. You must make sure that the `open` state is set accordingly to the dialog's visibility (mostly this means to properly react to the events provided by `onOpenChange` callback on `Dialog` component). -2. You must make sure that focus is properly restored once the dialog is closed (this can be achieved by using the `useRestoreFocusTarget` hook, or by manually invoking `.focus()` on the target element). +- Opening dialogs programmatically (via state, API calls, side effects) +- Opening nested dialogs where the inner dialog is not wrapped in a `DialogTrigger` -The example bellow showcases both explicit responsibilities: +**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. + +The example below showcases both responsibilities: diff --git a/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx index 6f1fc4419b783..2d47b0c5acce7 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx +++ b/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx @@ -23,7 +23,6 @@ export { TitleCustomAction } from './DialogTitleCustomAction.stories'; export { TitleNoAction } from './DialogTitleNoAction.stories'; export { Confirmation } from './DialogConfirmation.stories'; export { MotionCustom } from './DialogMotionCustom.stories'; -export { NestedDialogs } from './DialogNestedDialogs.stories'; export { NestedDialogsWithTrigger } from './DialogNestedDialogsWithTrigger.stories'; // Typing with Meta generates a type error for the `subcomponents` property. From 32c849a824f1f94c00c3e176c73f94590eb08fa9 Mon Sep 17 00:00:00 2001 From: Paolo Aliprandi Date: Wed, 13 May 2026 18:49:35 +0200 Subject: [PATCH 6/7] docs(react-dialog): address PR feedback for nested dialog guidance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../stories/src/Dialog/DialogBestPractices.md | 4 +- .../stories/src/Dialog/DialogNestedDialogs.md | 50 ++++++++++--------- ...DialogNestedDialogsWithTrigger.stories.tsx | 11 ++-- .../src/Dialog/DialogTriggerOutsideDialog.md | 2 +- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md index 3c6c7212d91ca..f55e2bc61cfe4 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md @@ -4,12 +4,12 @@ - Dialog boxes consist of a header (`DialogTitle`), content (`DialogContent`), and footer (`DialogActions`), which should all be included inside a body (`DialogBody`). - Validate that people’s entries are acceptable before closing the dialog. Show an inline validation error near the field they must correct. -- Modal dialogs should be used very sparingly—only when it’s critical that people make a choice or provide information before they can proceed. Thee dialogs are generally used for irreversible or potentially destructive tasks. They’re typically paired with an backdrop without a light dismiss. +- Modal dialogs should be used very sparingly—only when it’s critical that people make a choice or provide information before they can proceed. These dialogs are generally used for irreversible or potentially destructive tasks. They’re typically paired with a backdrop without a light dismiss. - Add a `aria-describedby` attribute on `DialogSurface` pointing to the dialog content on short confirmation like dialogs. - Add a `aria-label` or `aria-labelledby` attribute on `DialogSurface` if there is no `DialogTitle` ### Don't - Don't use more than three buttons between `DialogActions`. -- 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. +- 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 `useRestoreFocusTarget()` when opening dialogs programmatically. `DialogSurface` already provides restore-focus source attributes when used inside `Dialog`. See the [Nested Dialogs](https://react.fluentui.dev/?path=/docs/components-dialog--docs#nested-dialogs-with-trigger) example for details. - Don't use a `Dialog` with no focusable elements diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md index 4be892980c076..d3c0028b48141 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md @@ -1,10 +1,11 @@ # Nested Dialogs -Nested dialogs (opening a dialog from within another dialog) require proper focus management to ensure accessibility and predictable user experience. +Nested dialogs (opening a dialog from within another dialog) are an anti-pattern and should be avoided whenever possible. +If you must use them, ensure focus management is correct to keep keyboard navigation predictable and accessible. ## Key Principle -When using nested dialogs, **always use `DialogTrigger` for focus restoration**. If you open a dialog programmatically without `DialogTrigger`, you become responsible for managing focus restoration using `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks. +When using nested dialogs, **prefer `DialogTrigger` for focus restoration**. If you open a dialog programmatically without `DialogTrigger`, you become responsible for managing open state and focus restoration. ## Recommended: Using DialogTrigger @@ -35,49 +36,52 @@ Use `DialogTrigger` for opening nested dialogs. This provides automatic focus re - Simpler to implement - No manual focus management needed -## Programmatic Control: Using Focus Restoration Hooks +## Programmatic Control: Managing open state and focus -If you must open dialogs programmatically (without user click), use `useRestoreFocusSource()` and `useRestoreFocusTarget()` hooks. Note that you are responsible for ensuring focus is correctly restored: +If you must open dialogs programmatically (without user click), use `useRestoreFocusTarget()` on the elements that open each dialog. Note that you are responsible for ensuring focus is correctly restored: ```tsx const [outerOpen, setOuterOpen] = React.useState(false); const [innerOpen, setInnerOpen] = React.useState(false); -const outerSourceAttrs = useRestoreFocusSource(); const outerTargetAttrs = useRestoreFocusTarget(); -const innerSourceAttrs = useRestoreFocusSource(); const innerTargetAttrs = useRestoreFocusTarget(); return ( - setOuterOpen(data.open)}> + <> - - - - - setInnerOpen(data.open)}> - {/* Inner dialog content */} + setOuterOpen(data.open)}> + + + + setInnerOpen(data.open)}> + {/* Inner dialog content */} + + - + ); ``` +`DialogSurface` already provides restore-focus source attributes internally when used inside `Dialog`. + **How it works:** - `useRestoreFocusTarget()` attributes go on the button/element that opens the dialog -- `useRestoreFocusSource()` attributes go on the DialogSurface -- When the dialog closes, focus returns to the element with `useRestoreFocusTarget()` attributes -- Each dialog pair (source/target) manages its own focus restoration +- When a dialog closes, focus returns to the element with `useRestoreFocusTarget()` attributes +- Each dialog opener should have its own restore-focus target + +For a complete programmatic-open example, see [Trigger outside Dialog](https://react.fluentui.dev/?path=/docs/components-dialog--docs#trigger-outside-dialog). ## Best Practices -1. **Use DialogTrigger by default** - It provides automatic focus restoration for user-triggered opens -2. **Use focus hooks for programmatic dialogs** - When you open dialogs from code (not user clicks), use the focus restoration hooks -3. **Always apply focus attributes** - Don't skip focus management; it's essential for accessibility +1. **Avoid nested dialogs when possible** - Prefer simpler flows when the design allows it +2. **Use DialogTrigger by default** - It provides automatic focus restoration for user-triggered opens +3. **Use restore-focus targets for programmatic dialogs** - Add `useRestoreFocusTarget()` to elements that open dialogs 4. **Test with keyboard navigation** - Verify that Escape key and backdrop clicks work correctly, and Tab/Shift+Tab navigate through the dialog stack ## Accessibility @@ -88,4 +92,4 @@ Proper focus management in nested dialogs is crucial for: - **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. +See [focus management utilities documentation](https://react.fluentui.dev/?path=/docs/utilities-focus-management-userestorefocussource--docs) for more details on focus management utilities. diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx index 88aaba34cfd63..621ac827239db 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogsWithTrigger.stories.tsx @@ -10,6 +10,7 @@ import { DialogActions, Button, } from '@fluentui/react-components'; +import story from './DialogNestedDialogs.md'; export const NestedDialogsWithTrigger = (): JSXElement => { return ( @@ -35,8 +36,8 @@ export const NestedDialogsWithTrigger = (): JSXElement => { Inner Dialog - 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. + This is a nested dialog inside the outer dialog. Focus will automatically be restored to the Open + Inner Dialog button when this one closes thanks to DialogTrigger. @@ -61,11 +62,7 @@ export const NestedDialogsWithTrigger = (): JSXElement => { 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'), + story, }, }, }; diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md index 58aaab450ad20..50aec58904ec4 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogTriggerOutsideDialog.md @@ -6,6 +6,6 @@ When using a `Dialog` without a `DialogTrigger`, you become responsible for mana **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. +2. **Restore focus** - When the dialog closes, you must restore focus to the element that triggered the open. Use `useRestoreFocusTarget` on the trigger element, or manually invoke `.focus()` on the target element. `DialogSurface` already applies the restore-focus source attributes internally when used inside `Dialog`. The example below showcases both responsibilities: From cd1577052cc33ad517536b99e14489fcdbc4cf5e Mon Sep 17 00:00:00 2001 From: Paolo Aliprandi Date: Wed, 20 May 2026 14:48:21 +0200 Subject: [PATCH 7/7] Remove nested dialog example story per feedback Address @bsunderhus's comment: Remove the NestedDialogsWithTrigger story example entirely since nested dialogs are an anti-pattern. Even with warnings, developers copy and use the pattern. Focus documentation on discouraging the pattern and recommending alternatives. Changes: - Remove NestedDialogsWithTrigger export from Dialog stories index - Update DialogNestedDialogs.md to remove code examples and implementation details - Simplify guidance to explain why nested dialogs are problematic - Recommend design alternatives (multi-step flows, sequential dialogs, etc.) - Update DialogBestPractices.md to discourage the pattern without referencing example story Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../stories/src/Dialog/DialogBestPractices.md | 2 +- .../stories/src/Dialog/DialogNestedDialogs.md | 103 ++++-------------- .../stories/src/Dialog/index.stories.tsx | 1 - 3 files changed, 20 insertions(+), 86 deletions(-) diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md index f55e2bc61cfe4..ac1e4529c2307 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogBestPractices.md @@ -11,5 +11,5 @@ ### Don't - Don't use more than three buttons between `DialogActions`. -- 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 `useRestoreFocusTarget()` when opening dialogs programmatically. `DialogSurface` already provides restore-focus source attributes when used inside `Dialog`. See the [Nested Dialogs](https://react.fluentui.dev/?path=/docs/components-dialog--docs#nested-dialogs-with-trigger) example for details. +- Don't open nested `Dialog`s. They are an anti-pattern and should be avoided. Nested dialogs create complex focus restoration logic and confuse users. If your design requires stacking dialogs, consider using a multi-step wizard within a single dialog, sequential dialogs, or a different UI component (panels, sidebars, popovers). - Don't use a `Dialog` with no focusable elements diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md index d3c0028b48141..10fe92fae2ea4 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md +++ b/packages/react-components/react-dialog/stories/src/Dialog/DialogNestedDialogs.md @@ -1,95 +1,30 @@ # Nested Dialogs -Nested dialogs (opening a dialog from within another dialog) are an anti-pattern and should be avoided whenever possible. -If you must use them, ensure focus management is correct to keep keyboard navigation predictable and accessible. +Nested dialogs (opening a dialog from within another dialog) are **an anti-pattern and should be avoided** whenever possible. Even when implementing proper focus management, nested dialogs create complex focus restoration logic and confuse users about their context. -## Key Principle +## Why Nested Dialogs Are Problematic -When using nested dialogs, **prefer `DialogTrigger` for focus restoration**. If you open a dialog programmatically without `DialogTrigger`, you become responsible for managing open state and focus restoration. +1. **Focus Management Complexity** - Multiple dialog layers require careful keyboard and focus handling, increasing the risk of navigation bugs +2. **User Confusion** - Users lose sense of their context when multiple overlays stack on top of each other +3. **Accessibility Challenges** - Screen reader users and keyboard-only users struggle to manage multiple modal overlays +4. **Anti-pattern by Design** - Modal dialogs are intentionally disruptive; stacking them compounds this problem -## Recommended: Using DialogTrigger +## What to Do Instead -Use `DialogTrigger` for opening nested dialogs. This provides automatic focus restoration when dialogs close: +**Redesign your workflow** to eliminate the need for nested dialogs: -```tsx - - - - - - - Outer Dialog - - - - - {/* Inner dialog content */} - - - - -``` +- **Single multi-step flow** - Use tabs, accordions, or numbered steps within a single dialog +- **Sequential dialogs** - Close the first dialog before opening the next one +- **Different UI patterns** - Consider panels, sidebars, popovers, or non-modal overlays +- **Inline content** - Expand/collapse sections within the dialog instead of opening new dialogs -**Benefits:** +## If You Must Use Nested Dialogs (Rare) -- Focus is automatically restored when dialogs close -- Simpler to implement -- No manual focus management needed +Should your design truly require nested dialogs: -## Programmatic Control: Managing open state and focus +1. Use `DialogTrigger` for user-triggered opens (it automatically restores focus) +2. Use `useRestoreFocusTarget()` on elements that programmatically open dialogs +3. Test thoroughly with keyboard navigation (Escape, Tab, Shift+Tab) +4. Verify focus restoration works correctly for screen readers -If you must open dialogs programmatically (without user click), use `useRestoreFocusTarget()` on the elements that open each dialog. Note that you are responsible for ensuring focus is correctly restored: - -```tsx -const [outerOpen, setOuterOpen] = React.useState(false); -const [innerOpen, setInnerOpen] = React.useState(false); - -const outerTargetAttrs = useRestoreFocusTarget(); -const innerTargetAttrs = useRestoreFocusTarget(); - -return ( - <> - - setOuterOpen(data.open)}> - - - - setInnerOpen(data.open)}> - {/* Inner dialog content */} - - - - -); -``` - -`DialogSurface` already provides restore-focus source attributes internally when used inside `Dialog`. - -**How it works:** - -- `useRestoreFocusTarget()` attributes go on the button/element that opens the dialog -- When a dialog closes, focus returns to the element with `useRestoreFocusTarget()` attributes -- Each dialog opener should have its own restore-focus target - -For a complete programmatic-open example, see [Trigger outside Dialog](https://react.fluentui.dev/?path=/docs/components-dialog--docs#trigger-outside-dialog). - -## Best Practices - -1. **Avoid nested dialogs when possible** - Prefer simpler flows when the design allows it -2. **Use DialogTrigger by default** - It provides automatic focus restoration for user-triggered opens -3. **Use restore-focus targets for programmatic dialogs** - Add `useRestoreFocusTarget()` to elements that open dialogs -4. **Test with keyboard navigation** - Verify that Escape key and backdrop clicks work correctly, and Tab/Shift+Tab navigate through the dialog stack - -## Accessibility - -Proper focus management in nested dialogs is crucial for: - -- **Keyboard users** - They can close dialogs with Escape and navigate through the dialog stack using Tab -- **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 [focus management utilities documentation](https://react.fluentui.dev/?path=/docs/utilities-focus-management-userestorefocussource--docs) for more details on focus management utilities. +For details on focus management utilities, see the [focus management documentation](https://react.fluentui.dev/?path=/docs/utilities-focus-management--docs). diff --git a/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx index 2d47b0c5acce7..653aa7e52a251 100644 --- a/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx +++ b/packages/react-components/react-dialog/stories/src/Dialog/index.stories.tsx @@ -23,7 +23,6 @@ export { TitleCustomAction } from './DialogTitleCustomAction.stories'; export { TitleNoAction } from './DialogTitleNoAction.stories'; export { Confirmation } from './DialogConfirmation.stories'; export { MotionCustom } from './DialogMotionCustom.stories'; -export { NestedDialogsWithTrigger } from './DialogNestedDialogsWithTrigger.stories'; // Typing with Meta generates a type error for the `subcomponents` property. // https://github.com/storybookjs/storybook/issues/27535