Repository navigation
fix: restore headless TeachingPopover close focus - #36843
Open
Paul Mardling (PaulGMardling) wants to merge 8 commits into
Open
Paul Mardling (PaulGMardling) wants to merge 8 commits into
Paul Mardling (PaulGMardling) wants to merge 8 commits into
Conversation
Restore trigger focus after a focused TeachingPopover surface is removed on close, without overriding native restoration or deliberate outside focus. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
Track focus ownership using the surface root while preserving native restoration and intentional outside focus. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Paul Mardling (PaulGMardling)
marked this pull request as ready for review
October 9, 2026 09:20
Copilot started reviewing on behalf of
Paul Mardling (PaulGMardling)
October 9, 2026 09:21
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused behavior change is guarded against unwanted focus movement and has comprehensive unit and browser coverage.
0 open findings
What changed in this PR
Restores trigger focus when an initially open headless TeachingPopover closes after losing surface-owned focus.
Changes:
- Tracks surface focus across controlled close transitions and shadow roots.
- Adds unit and Cypress coverage for restoration and intentional focus movement.
- Updates documentation and adds a patch change file.
| File | Description |
|---|---|
TeachingPopoverDescription.md |
Documents close-focus behavior. |
useTeachingPopover.ts |
Implements guarded trigger-focus restoration. |
TeachingPopover.test.tsx |
Adds unit coverage for focus ownership and shadow roots. |
TeachingPopover.cy.tsx |
Adds browser coverage for controlled tours and dismissal. |
@fluentui-react-headless-components-preview-*.json |
Records the patch release. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Move close focus handling into shared Popover and remove the TeachingPopover-specific document listener. Preserve native restoration and intentional outside focus, with a guarded fallback for orphaned surface focus. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d39e88a-f6af-41d5-a788-7aa0152d5210
Paul Mardling (PaulGMardling)
requested review from
Dmytro Kirpa (dmytrokirpa)
and
a balanced review from Copilot
October 12, 2026 12:54
Copilot started reviewing on behalf of
Paul Mardling (PaulGMardling)
October 12, 2026 12:55
View session
Comment on lines
+25
to
+32
| let activeElement = | ||
| ShadowRootConstructor && root instanceof ShadowRootConstructor | ||
| ? root.activeElement ?? surface.ownerDocument.activeElement | ||
| : surface.ownerDocument.activeElement; | ||
| while (activeElement?.shadowRoot?.activeElement) { | ||
| activeElement = activeElement.shadowRoot.activeElement; | ||
| } | ||
| return activeElement; |
Keep active element lookup relative to the surface root so containment checks recognize focused shadow hosts. Add unit and browser regressions for nested shadow focus restoration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d39e88a-f6af-41d5-a788-7aa0152d5210
This branch has not been deployed
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.
Previous Behavior
Headless Popover unmounted its surface before native close could restore focus, and the non-modal path never called
hidePopover(). This could leave focus on the document body when a TeachingPopover tour closed.New Behavior
Shared headless Popover keeps the surface connected until
hidePopover()orclose()completes, then unmounts it. TeachingPopover inherits the fix without a document-wide focus listener. A guarded trigger fallback handles surface-owned focus when native restoration has no viable target, including initially-open tours and shadow roots; native restoration and intentional outside focus take precedence.Includes regression coverage, documentation, updated hook-state API reports, and a patch change file. Styled v9 components and carousel navigation are unchanged.
Validation
Checked locally on 2026-10-12: 62 unit tests and 66 Cypress tests passed (Electron, retries disabled; three existing skipped cases). Browser coverage checks native close before unmounting, Got it/Escape dismissal, modal/non-modal tours, shadow roots, reopening, and outside focus preservation. Build, type-check, lint, and API generation passed.
Related Issue(s)