fix(react-headless-components-preview): update non-modal dialog implementation to use popover API#36244
Open
dmytrokirpa wants to merge 4 commits into
Open
fix(react-headless-components-preview): update non-modal dialog implementation to use popover API#36244dmytrokirpa wants to merge 4 commits into
dmytrokirpa wants to merge 4 commits into
Conversation
…mentation to use popover API
|
Pull request demo site: URL |
dmytrokirpa
force-pushed
the
fix/switch-headless-non-modal-dialog-to-popover
branch
from
71e32c9 to
32b9621
Compare
📊 Bundle size report
🤖 This report was generated against aa727a4aac5088916fe7d22e11355ba4938951e2 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the headless preview Dialog non-modal surface implementation to use the native Popover API (top-layer) instead of relying on dialog.show() + portalling, adds a new open-change event type for native surface toggles, and extends Cypress coverage to include nested modal + non-modal dialogs.
Changes:
- Switch non-modal
DialogSurfaceopen/close behavior topopover="manual"+showPopover()/hidePopover()(with ashow()fallback) and add asurfaceToggleopen-change event for native toggle synchronization. - Remove the non-modal Portal rendering path and adjust story styling to scope
::backdropstyles to modal dialogs only. - Update Cypress tests/selectors for non-modal open state and add an accessibility-focused nested dialog test.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react-components/react-headless-components-preview/stories/src/Dialog/dialog.module.css | Scope backdrop styling to modal dialogs via :modal::backdrop. |
| packages/react-components/react-headless-components-preview/library/src/components/Dialog/useDialog.ts | Allow default-prevent handling for both React synthetic events and native events. |
| packages/react-components/react-headless-components-preview/library/src/components/Dialog/DialogSurface/useDialogSurface.ts | Implement popover-based non-modal open/close + toggle event syncing + focus restoration. |
| packages/react-components/react-headless-components-preview/library/src/components/Dialog/DialogSurface/renderDialogSurface.tsx | Render DialogSurface inline (remove non-modal Portal path). |
| packages/react-components/react-headless-components-preview/library/src/components/Dialog/DialogSurface/DialogSurface.types.ts | Update docs/comments to reflect popover-based non-modal behavior. |
| packages/react-components/react-headless-components-preview/library/src/components/Dialog/DialogSurface/DialogSurface.tsx | Update docs/comments to mention showPopover() usage. |
| packages/react-components/react-headless-components-preview/library/src/components/Dialog/dialogContext.ts | Add surfaceToggle to DialogOpenChangeData. |
| packages/react-components/react-headless-components-preview/library/src/components/Dialog/Dialog.cy.tsx | Update selectors and add nested modal + non-modal accessibility test coverage. |
| packages/react-components/react-headless-components-preview/library/etc/dialog.api.md | Update extracted API to include surfaceToggle event type. |
| change/@fluentui-react-headless-components-preview-6def4baa-5ccb-46a3-92a8-da4416b5111b.json | Beachball change file for the patch release. |
Comment on lines
64
to
+68
| if (!open) { | ||
| // Close while the element is still connected so the browser runs its native | ||
| // close-the-dialog steps (including focus restoration). If `unmountOnClose` is | ||
| // true, schedule the actual DOM removal now that close has run. | ||
| if (dialog.open) { | ||
| if (modalType === 'non-modal') { | ||
| if (typeof dialog.hidePopover === 'function') { | ||
| const isPopoverOpen = SUPPORTS_POPOVER_OPEN_SELECTOR && dialog.matches(':popover-open'); | ||
| if (isPopoverOpen) { |
| ); | ||
|
|
||
| return state.modalType === 'non-modal' ? <Portal>{content}</Portal> : content; | ||
| return content; |
Contributor
Author
There was a problem hiding this comment.
the whole goal of this PR is to not use Portal, so we'll rely on Popover
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the implementation of non-modal dialogs in
@fluentui/react-headless-components-previewto use the native popover API, improving accessibility and alignment with browser standards. It also introduces new event handling for dialog state changes, updates the test suite to cover nested dialogs, and refines both code comments and styles for clarity and correctness.Non-modal Dialog Implementation Improvements
showPopover()API (withpopover="manual") instead ofshow(), enabling them to enter the browser top layer and improving stacking and accessibility. Fallback toshow()is used if the popover API is unavailable. (DialogSurface.tsx,DialogSurface.types.ts,useDialogSurface.ts,renderDialogSurface.tsx) [1] [2] [3] [4] [5] [6] [7] [8] [9]Event Handling and API Changes
surfaceToggleevent type toDialogOpenChangeDatato handle open/close events triggered by the native popover toggle, ensuring React state stays in sync with the DOM. (dialogContext.ts,dialog.api.md,useDialog.ts) [1] [2] [3]Testing Enhancements
dialog[data-open]for non-modal dialogs and added a comprehensive test for accessible nested modal and non-modal dialogs, verifying ARIA attributes and correct rendering. (Dialog.cy.tsx) [1] [2] [3]Documentation and Code Comments
DialogSurface.tsx,DialogSurface.types.ts,useDialogSurface.ts,renderDialogSurface.tsx) [1] [2] [3] [4] [5]Styling Adjustments
dialog.module.css)