Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: update non-modal dialog implementation to use popover API",
"packageName": "@fluentui/react-headless-components-preview",
"email": "dmytrokirpa@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ export type DialogOpenChangeData = {
type: 'backdropClick';
open: boolean;
event: React_2.MouseEvent<HTMLDialogElement>;
} | {
type: 'surfaceToggle';
open: boolean;
event: Event;
} | {
type: 'triggerClick';
open: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const dialogTriggerId = 'dialog-trigger';
const dialogTriggerOpenId = `${dialogTriggerId}-open`;
const dialogTriggerCloseId = `${dialogTriggerId}-close`;
const dialogPrimaryButtonId = 'do-something-btn';
const dialogSurfaceSelector = `dialog[open]`;
const dialogSurfaceSelector = `dialog[data-open]`;
Comment thread
dmytrokirpa marked this conversation as resolved.
const dialogSurfaceElementSelector = `dialog`;
const dialogTriggerOpenSelector = `#${dialogTriggerOpenId}`;
const dialogTriggerCloseSelector = `#${dialogTriggerCloseId}`;
Expand Down Expand Up @@ -310,10 +310,9 @@ describe('Dialog', () => {
<>
{/*
The headless component ships no default styles. The UA stylesheet combined
with the portal mount node (`position: absolute; top: 0; left: 0; right: 0`)
pins the non-modal dialog to the top of the viewport, where it would overlap
and intercept clicks on the sibling "outside" button. Position it out of the
way so this test can verify focus behaviour rather than layout.
with top-layer placement can visually overlap the sibling "outside" button.
Position it out of the way so this test can verify focus behaviour rather
than layout.
*/}
<BasicDialog
modalType="non-modal"
Expand All @@ -330,6 +329,28 @@ describe('Dialog', () => {
cy.get(dialogPrimaryButtonSelector).realClick().should('be.focused').realType('{esc}');
cy.get(dialogSurfaceSelector).should('not.exist');
});

it('should move focus into dialog when Tab is pressed after clicking dialog surface', () => {
mount(
<>
<BasicDialog
modalType="non-modal"
surfaceProps={{
style: { position: 'fixed', top: 'auto', bottom: 0, right: 0, left: 'auto', margin: 0 },
}}
/>
<Button id="extra-btn-outside">Button outside dialog</Button>
</>,
);
cy.get(dialogTriggerOpenSelector).realClick();
cy.get('#extra-btn-outside').realClick().should('be.focused');
// Click on the dialog surface itself (not on a focusable descendant).
// Focus lands on the <dialog> via its tabIndex=-1; pressing Tab should
// step into the dialog's first focusable descendant.
cy.get(dialogSurfaceElementSelector).realClick();
cy.realPress('Tab');
cy.get(dialogTriggerCloseSelector).should('be.focused');
});
});

it('should allow nested dialogs', () => {
Expand Down Expand Up @@ -376,4 +397,60 @@ describe('Dialog', () => {
cy.get('#close-first-dialog-btn').should('exist').realClick();
cy.get(dialogSurfaceSelector).should('not.exist');
});

it('should render nested non-modal dialog in modal dialog accessibly', () => {
mount(
<Dialog modalType="modal">
<DialogTrigger>
<Button id="open-outer-dialog-btn">Open outer dialog</Button>
</DialogTrigger>
<DialogSurface>
<DialogBody>
<DialogTitle>Outer dialog title</DialogTitle>
<Dialog modalType="non-modal">
<DialogTrigger>
<Button id="open-inner-dialog-btn">Open inner dialog</Button>
</DialogTrigger>
<DialogSurface>
<DialogBody>
<DialogTitle>Inner non-modal dialog title</DialogTitle>
<Button id="inner-non-modal-action">Inner action</Button>
</DialogBody>
</DialogSurface>
</Dialog>
</DialogBody>
</DialogSurface>
</Dialog>,
);

cy.get('dialog[data-modal-type="modal"]').should('not.exist');
cy.get('dialog[data-modal-type="non-modal"]').should('not.exist');

cy.get('#open-outer-dialog-btn').realClick();
cy.get('dialog[data-modal-type="modal"]').should('have.length', 1);
cy.get('dialog[data-modal-type="non-modal"]').should('not.exist');

cy.get('#open-inner-dialog-btn').realClick();
cy.get('dialog[data-modal-type="non-modal"]').should('have.length', 1);

// Outer dialog should be modal and have an accessible name from title linkage.
cy.contains('h2', 'Outer dialog title')
.invoke('attr', 'id')
.then(outerTitleId => {
cy.get('dialog[data-modal-type="modal"]').should('have.attr', 'aria-modal', 'true');
cy.get('dialog[data-modal-type="modal"]').should('have.attr', 'aria-labelledby', outerTitleId);
});

// Inner dialog should be non-modal, remain non-aria-modal, and expose title linkage.
cy.contains('h2', 'Inner non-modal dialog title')
.invoke('attr', 'id')
.then(innerTitleId => {
cy.get('dialog[data-modal-type="non-modal"]').should('not.have.attr', 'aria-modal');
cy.get('dialog[data-modal-type="non-modal"]').should('have.attr', 'aria-labelledby', innerTitleId);
});

cy.get('#inner-non-modal-action').should('exist');
cy.get('dialog[data-modal-type="modal"]').should('have.length', 1);
cy.get('dialog[data-modal-type="non-modal"]').should('have.length', 1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { DialogSurfaceProps } from './DialogSurface.types';
* `DialogSurface` renders the native HTML `<dialog>` element.
*
* Place it as a direct child of `Dialog` (alongside an optional `DialogTrigger`).
* It manages open/close via `showModal()` / `show()` / `close()` imperatively,
* It manages open/close via `showModal()` / `showPopover()` / `close()` imperatively,
* keeping the native element in sync with the controlled `open` prop.
*
* The native `::backdrop` pseudo-element provides the modal backdrop.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { DialogModalType } from '../dialogContext';
export type DialogSurfaceSlots = {
/**
* The native HTML `<dialog>` element.
* Opened as modal via `showModal()` or non-modal via `show()`.
* Opened as modal via `showModal()` or non-modal via `showPopover()`.
* The `::backdrop` CSS pseudo-element provides the native backdrop for modal dialogs.
*/
root: Slot<'dialog'>;
Expand All @@ -31,8 +31,8 @@ export type DialogSurfaceState = ComponentState<DialogSurfaceSlots> & {
/**
* Modality of the dialog. Mirrors DialogContext's `modalType`.
* - `modal` / `alert`: opened via `showModal()`; rendered into the browser top layer.
* - `non-modal`: opened via `show()`; portalled into `document.body` to escape ancestor
* stacking contexts (`overflow`, `clip-path`, `transform`).
* - `non-modal`: opened via `showPopover()` with `popover="manual"`, entering
* the browser top layer while keeping open/close fully React-controlled.
*/
modalType: DialogModalType;
/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/** @jsxRuntime automatic */
/** @jsxImportSource @fluentui/react-jsx-runtime */

import { Portal } from '@fluentui/react-portal';
import { assertSlots } from '@fluentui/react-utilities';
import type { JSXElement } from '@fluentui/react-utilities';
import { DialogSurfaceContext } from '../dialogContext';
Expand All @@ -12,11 +11,10 @@ import type { DialogSurfaceSlots, DialogSurfaceState } from './DialogSurface.typ
* Returns null when the dialog is closed and unmountOnClose is true.
* Provides DialogSurfaceContext=true so DialogTrigger inside defaults to action="close".
*
* Non-modal dialogs are rendered via a React portal into `document.body`.
* Unlike `showModal()`, `dialog.show()` does not enter the browser top layer, so the
* element is still subject to ancestor `overflow`, `clip-path`, and `transform`
* stacking constraints. Portalling to body moves it outside any such container.
* React context (including DialogContext) is preserved across portals.
* DialogSurface is always rendered inline. For `modal`/`alert`, `<dialog showModal()>`
* enters the browser top layer. For `non-modal`, the surface uses the native
* popover API (`popover="manual"` + `showPopover()`), which promotes it to the
* top layer without enabling native light-dismiss.
*/
export const renderDialogSurface = (state: DialogSurfaceState): JSXElement | null => {
if (!state.shouldRender) {
Expand All @@ -31,5 +29,5 @@ export const renderDialogSurface = (state: DialogSurfaceState): JSXElement | nul
</DialogSurfaceContext.Provider>
);

return state.modalType === 'non-modal' ? <Portal>{content}</Portal> : content;
return content;
Comment thread
dmytrokirpa marked this conversation as resolved.
};
Loading
Loading