Repository navigation
fix(react-headless-components-preview): track controlled and keyed InfoLabel ownership - #36953
Open
Paul Mardling (PaulGMardling) wants to merge 7 commits into
Conversation
Track ownership from controlled open state or the current optional button lifetime. Ignore stale lifetime callbacks, consume defaultOpen once, and delegate dismissal to the shared Popover engine. Integrated focus dismissal validation remains blocked on PR microsoft#36929 reaching the base. Custom render-function slot key forwarding remains unverified. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d006ca2-c036-45bf-bac0-93275c5dfe08
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
Contributor
Author
|
Before and after browser captures of the same controlled-open InfoLabel demo, compiled against base
|
…daries Forward root and InfoButton keys through the slot runtime so custom render functions cannot bypass the keyed boundary. Reset ownership for root-driven remounts and cover normalized keys, defaults, stale callbacks, and commit-time ownership. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d006ca2-c036-45bf-bac0-93275c5dfe08
…tions Add explicit outside controls and assert destination focus in InfoLabel Tab dismissal tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…cus dismissal Defer shared focus-dismissal integration and preserve the existing blur behavior. Keep controlled/default/keyed ownership fixes, and exercise their accepted and rejected requests through trigger toggles without excluding tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d006ca2-c036-45bf-bac0-93275c5dfe08
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d006ca2-c036-45bf-bac0-93275c5dfe08
Paul Mardling (PaulGMardling)
changed the title
…import Use the repository's react-hooks alias so ownership lifecycle tests run with React 17's Testing Library as well as React 18 and 19. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d006ca2-c036-45bf-bac0-93275c5dfe08
…rship lifetime Replace the callback-driven open mirror with private mounted-instance reports. Reconcile ownership during layout for renderer-owned keys, renderer-mode changes, omitted buttons, and stale callbacks, while isolating nested buttons and preserving SSR bootstrap and consumer ownership. Keep legacy blur behavior, public API signatures, and shared JSX/Popover implementations unchanged. Add before/after regression coverage and real-browser returned-key replacement checks. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3d006ca2-c036-45bf-bac0-93275c5dfe08
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
1 participant
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
InfoLabel mirrored open-change requests rather than the mounted InfoButton's effective state. Ownership could disagree with controlled parent decisions, miss
defaultOpen, or survive a removed/replaced button. Tracking shorthand keys alone did not cover default/custom renderer transitions or keys chosen by a render function's returned element: the popup could disappear whilearia-ownsstill referenced it.New Behavior
defaultOpenlifetimes. Ordinary rerenders with new inline render functions do not reset a mounted button.onOpenChange; stale callbacks on an unmounted button cannot change a newer button's ownership.open/defaultOpenuntil mounted-state reporting is available. A client renderer that omits the button clears that projection during layout.aria-owns, refs, and handlers.defaultOpenonce in InfoButton, clearing it before passing the resolved controlled state to the nested Popover.Latest fix:
78a76c1a1cddd0f4c95328cf0d604f5face54ec6. This resolves the reported renderer-transition/returned-key ownership finding. Source-of-truth reporting is layout-based; it does not claim that a parent can predict an opaque renderer's remount before the commit's child layout effects run.Scope preserved: the user-approved deferral of permanent focus-dismissal integration remains. The existing blur path still closes local state directly and retains its previous unknown/null-destination behavior; merged-callback/confirmed-destination dismissal is not claimed fixed. No other work item's commits or substitute focus observer are imported. The existing React 19 metadata-key warning comes from the unchanged shared JSX runtime.
Related Issue(s)
Validation
Local results for the latest fix:
yarn nx run react-headless-components-preview:test --maxWorkers=1 --workerIdleMemoryLimit=512MB --silent --skipNxCache— passed: 89 suites / 1,314 tests. Earlier two-worker attempts encountered native Jest-workerSIGSEGVfailures in unrelated suites; the complete single-worker run passed without test exclusions or unrelated code changes.yarn nx run react-headless-components-preview:test-rit--17--test— passed: 89 suites / 1,314 tests under React 17.yarn nx run react-headless-components-preview:test-rit--18--test— passed: 89 suites / 1,314 tests under React 18.yarn nx run react-headless-components-preview:test --runTestsByPath src/components/InfoLabel/InfoLabelOwnership.test.tsx src/components/InfoLabel/InfoLabel.test.tsx src/components/InfoLabel/InfoButton/InfoButton.test.tsx --runInBand— passed: all 68 scoped tests.yarn nx run react-headless-components-preview:test --runTestsByPath src/components/InfoLabel/InfoLabelOwnership.test.tsx --testNamePattern='renderer-owned|same-key.*renderer transitions|callbacks retained from an actual|omits the button' --runInBand— passed: all 7 selected regressions. The same regressions, using a scratch Jest configuration substituting the previouscabe151f18hook/renderer source without editing the checkout, failed all 7.yarn nx run react-headless-components-preview:e2e --spec=src/components/InfoLabel/InfoLabel.cy.tsx --browser=chrome --headless --skipNxCache— passed: all 6 InfoLabel browser cases, including two renderer-owned-key replacements and original Tab/Shift+Tab destination checks.CYPRESS_specPattern='/Users/paulmardling/dev/copilot-worktrees/fluentui/paulgmardling-probable-happiness/packages/react-components/react-headless-components-preview/library/src/components/InfoLabel/InfoLabel.cy.tsx' yarn nx run react-headless-components-preview:test-rit--17--e2e— passed: all 6 InfoLabel browser cases under React 17.CYPRESS_specPattern='/Users/paulmardling/dev/copilot-worktrees/fluentui/paulgmardling-probable-happiness/packages/react-components/react-headless-components-preview/library/src/components/InfoLabel/InfoLabel.cy.tsx' yarn nx run react-headless-components-preview:test-rit--18--e2e— passed: all 6 InfoLabel browser cases under React 18.yarn nx run-many --projects=react-headless-components-preview --targets=test-rit--17--type-check,test-rit--18--type-check --parallel=2— passed.yarn nx run react-headless-components-preview:type-check— passed.yarn nx run react-headless-components-preview:lint— passed.yarn nx run react-headless-components-preview:generate-api— passed; no public API report changes.yarn nx run react-headless-components-preview:format --configuration=check— passed.git diff --check— passed.Unit coverage includes Strict Mode, renderer-owned keys, same-key default/custom transitions in both directions, inline-render-function stability, actual stale callbacks, consumer ownership overrides, omitted buttons, nested/multiple instances, controlled requests, default lifetime, and initial SSR ownership. The old hook-only callback probes were replaced by actual mounted-button callback regressions.
Browser coverage is limited to the InfoLabel spec, not the entire component browser matrix. No Windows/TCS or deferred permanent-dismissal signoff is claimed. The attached screenshots demonstrate controlled-open DOM ownership. CI was green on
cabe151f18; fresh CI for this implementation must complete before claiming the latest head is green.