Skip to content

fix(react-dialog): stale aria-hidden breaks focus trap when closing stacked non-nested sibling dialogs#35990

Draft
Copilot wants to merge 3 commits intomasterfrom
copilot/fix-dialog-aria-hidden-issue
Draft

fix(react-dialog): stale aria-hidden breaks focus trap when closing stacked non-nested sibling dialogs#35990
Copilot wants to merge 3 commits intomasterfrom
copilot/fix-dialog-aria-hidden-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 15, 2026

When two sibling (non-nested) Dialog components are stacked, closing the top dialog can cause focus to escape to the page-level trigger instead of returning to the button inside the underlying dialog. Tabster's Modalizer does not currently maintain an activation stack, so focus restoration requires the caller to properly mark trigger elements as restore targets.

Changes

  • Dialog.cy.tsx — Added describe('stacked non-nested dialogs (sibling)') with three Cypress regression tests:

    1. Focus returns to the trigger button inside Dialog 1 (not the page-level trigger) when Dialog 2 closes
    2. No ancestor of Dialog 1's portal carries a stale aria-hidden="true" after Dialog 2 closes
    3. Tab focus trap inside Dialog 1 cycles correctly after Dialog 2 is dismissed

    When using controlled (unmanaged) open state without <DialogTrigger>, trigger buttons must have useRestoreFocusTarget() spread on them so Tabster's Restorer knows where to return focus when the stacked dialog closes. This follows the same documented pattern as DialogTriggerOutsideDialog.

  • Change file — Patch bump for @fluentui/react-dialog.

Example scenario

// Both dialogs mounted as siblings — Dialog 2 opened from a button inside Dialog 1
const restoreFocusTargetForDialog2 = useRestoreFocusTarget();

<>
  <Dialog open={dialog1Open}>
    <DialogSurface>
      {/* spread restoreFocusTargetForDialog2 so Tabster can restore focus here when Dialog 2 closes */}
      <Button {...restoreFocusTargetForDialog2} onClick={() => setDialog2Open(true)}>
        Open Dialog 2
      </Button>
    </DialogSurface>
  </Dialog>
  <Dialog open={dialog2Open}>...</Dialog>   {/* sibling, NOT nested */}
</>
Original prompt

Context

This PR addresses #35985: when two independent (non-nested, sibling) Dialog components are stacked — e.g. Dialog 1 is open, and from a button inside it Dialog 2 is opened programmatically — closing Dialog 2 leaves a stale aria-hidden="true" on Dialog 1's portal mount node. This blocks browser focus from entering that subtree, so the Tabster Restorer fires focus back to the original page-level trigger rather than back into Dialog 1.

Root cause

Each Dialog independently calls useModalAttributes which configures a Tabster Modalizer + Restorer pair. When Dialog 2's Modalizer activates, it aria-hiddens everything outside itself, including Dialog 1's portal. When Dialog 2 unmounts, its Modalizer cleanup removes the aria-hidden it set. However, Dialog 1 is still open and its own Modalizer's aria-hidden walk marked Dialog 1's portal container as hidden from its own perspective. The net result is a stale aria-hidden="true" left on Dialog 1's portal node after Dialog 2 closes. The browser refuses .focus() into an aria-hidden subtree, so RestorerTypes.Target focus restoration silently fails and focus bubbles up to the page-level trigger.

The ultimate fix belongs in Tabster's Modalizer (it should maintain an activation stack). Until that lands, a client-side mitigation is needed in useFocusFirstElement.


Required changes

1. Cypress regression test

File: packages/react-components/react-dialog/library/src/components/Dialog/Dialog.cy.tsx

Add a new describe('stacked non-nested dialogs (sibling)', ...) block appended after the existing 'should allow nested dialogs' test (after line 702, before the closing }); of the top-level describe('Dialog', ...)).

The block should use the existing imports already present in the file (React, Button, Dialog, DialogSurface, DialogBody, DialogTitle, DialogContent, DialogActions, DialogTrigger, and the selectors dialogTriggerOpenId, dialogTriggerOpenSelector, dialogTriggerCloseId, dialogTriggerCloseSelector, dialogSurfaceSelector).

Add this describe block:

  describe('stacked non-nested dialogs (sibling)', () => {
    /**
     * Regression test for https://github.com/microsoft/fluentui/issues/35985
     *
     * Two sibling Dialogs (NOT nested inside each other's JSX tree).
     * Dialog 1 is opened via a page-level trigger.
     * Dialog 2 is opened via a button inside Dialog 1, but is a sibling in the React tree.
     * When Dialog 2 closes, focus must return to the button inside Dialog 1 — NOT to the page trigger.
     *
     * Bug: Dialog 2's Modalizer leaves stale aria-hidden="true" on Dialog 1's portal mount node,
     * blocking browser focus restoration back into Dialog 1.
     */
    it('should restore focus to underlying dialog when top stacked dialog closes', () => {
      const StackedDialogsTest = () => {
        const [dialog1Open, setDialog1Open] = React.useState(false);
        const [dialog2Open, setDialog2Open] = React.useState(false);

        return (
          <>
            <Button id={dialogTriggerOpenId} onClick={() => setDialog1Open(true)}>
              Open Dialog 1
            </Button>

            {/* Dialog 1 — opened from the page-level trigger */}
            <Dialog open={dialog1Open} onOpenChange={(_, data) => setDialog1Open(data.open)}>
              <DialogSurface id="dialog-1-surface">
                <DialogBody>
                  <DialogTitle>Dialog 1</DialogTitle>
                  <DialogContent>Dialog 1 content</DialogContent>
                  <DialogActions>
                    <Button id="open-dialog-2-btn" onClick={() => setDialog2Open(true)}>
                      Open Dialog 2
                    </Button>
                    <Button id={dialogTriggerCloseId} onClick={() => setDialog1Open(false)}>
                      Close Dialog 1
                    </Button>
                  </DialogActions>
                </DialogBody>
              </DialogSurface>
            </Dialog>

            {/* Dialog 2 — sibling in the React tree, NOT nested inside Dialog 1 */}
            <Dialog modalType="alert" open={dialog2Open} onOpenChange={(_, data) => setDialog2Open(data.open)}>
              <DialogSurface id="dialog-2-surface">
                <DialogBody>
                  <DialogTitle>Dialog 2 (alert)</DialogTitle>
                  <DialogContent>Dialog 2 content</DialogContent>
                  <DialogActions>
                    <Button id="close-dialog-2-btn" onClick={() => setDialog2Open(false)}>
                      Close Dialog 2
                    </Button>
                  </DialogActions>
                </DialogBody>
              </DialogSurface>
            </Dialog>
          </>
        );
      };

      mount(<StackedDialogsTest />);

      // Open Dialog 1
      cy.get(dialogTriggerOpenSelector).realClick();
      cy.get('#dialog-1-surface').should('e...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

Copilot AI changed the title [WIP] Fix aria-hidden issue when closing sibling dialogs fix(react-dialog): stale aria-hidden breaks focus trap when closing stacked non-nested sibling dialogs Apr 15, 2026
Copilot AI requested a review from Hotell April 15, 2026 13:04
@Hotell
Copy link
Copy Markdown
Contributor

Hotell commented Apr 17, 2026

… revert incorrect aria-hidden workaround

Agent-Logs-Url: https://github.com/microsoft/fluentui/sessions/c4585627-39c5-4f06-8884-09fab6e1fd81

Co-authored-by: Hotell <1223799+Hotell@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 17, 2026

@copilot tests are failing, try using instead your workaround
https://storybooks.fluentui.dev/react/?path=/docs/utilities-focus-management-userestorefocussource--docs

Done in commit 693ddf48.

The root issue was that the previous removeStaleAriaHiddenFromAncestors workaround in useFocusFirstElement.ts was in the wrong place — it only fires when a dialog opens (open dependency), not when a sibling dialog closes while Dialog 1 stays open. Also, the CI showed the aria-hidden test already passes without any library change (Tabster cleans up the attribute correctly on its own).

The actual fix was in the test fixtures: the trigger buttons opening the dialogs need useRestoreFocusTarget() spread on them, following the documented pattern from DialogTriggerOutsideDialog. Without that, Tabster's Restorer has no Target to restore focus to when Dialog 2 unmounts. useFocusFirstElement.ts has been reverted to its original state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants