Repository navigation
Support picker methods in ReactorHostControl-hosted content - #1397
Merged
Alexandre Zollinger Chohfi (azchohfi) merged 8 commits intoOct 22, 2026
Merged
Alexandre Zollinger Chohfi (azchohfi) merged 8 commits into
Alexandre Zollinger Chohfi (azchohfi) merged 8 commits into
Conversation
Replace the ReactorWindow-typed picker owner with a small internal owner resolver (IContentOwner, Reconciler.Owner) implemented by ReactorHost (the Window it was constructed over) and ReactorHostControl (the window it is loaded in, via XamlRoot.ContentIslandEnvironment.AppWindowId, resolved at call time). Release on unmount/root retirement, the closed and off-thread checks, and distinct errors are kept; a detached host control gets its own error. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0095f9f3-4194-447b-bb62-a5a9914004c3
Alexandre Zollinger Chohfi (azchohfi)
requested a review
from Chris Anderson (codemonkeychris)
as a code owner
October 21, 2026 21:20
Alexandre Zollinger Chohfi (azchohfi)
added
the
agent-preparing
Contributor
📦 Build metricsArtifact sizes for Packages (compressed .nupkg)
Assemblies in Microsoft.UI.Reactor
Assemblies in Microsoft.UI.Reactor.Advanced
Assemblies in Microsoft.UI.Reactor.Devtools
✅ smaller / |
Contributor
🧪 Merged coverageCoverage for
No coverage change beyond the noise floor. ✅ ✅ higher / |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Add EmbeddedTeardownAndFunctionRoots selftest: picker calls from cleanups during ReactorHostControl dispose and raw-window close are refused, and function roots mounted on either host record their owner. Document every throw condition in the Pick*Async remarks, fix tense in spec 054, rename the raw-host post-close checks to _Released_, and revert accidental thumbnail churn. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0095f9f3-4194-447b-bb62-a5a9914004c3
Alexandre Zollinger Chohfi (azchohfi)
requested a balanced review from Copilot
October 21, 2026 21:37
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The owner propagation, lifecycle guards, regression coverage, and documentation consistently support the requested behavior, with no unresolved blocking findings.
No open issues 🎉
What changed in this PR
Extends Reactor’s picker helpers to embedded ReactorHostControl content and hosts built over raw WinUI windows, addressing #1395 without changing the public API.
Changes:
- Introduces an internal owner resolver that determines the hosting window at call time and preserves distinct lifecycle and thread errors.
- Adds unit tests and selftests for routing, reparenting, teardown, function roots, and off-thread rejection.
- Updates the guide, windowing skills, specification, and changelog to document the expanded support.
| File | Description |
|---|---|
| tests/Reactor.Tests/PickerTests.cs | Tests resolver states, routing, and owner release. |
| tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs | Registers three new picker fixtures. |
| tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs | Covers embedded and raw-window picker lifecycles. |
| src/Reactor/Hosting/ReactorHostControl.cs | Resolves the current containing window with lifecycle guards. |
| src/Reactor/Hosting/ReactorHost.cs | Resolves picker ownership for raw and Reactor windows. |
| src/Reactor/Core/RenderContext.cs | Records the owner resolver and maps unavailable states to errors. |
| src/Reactor/Core/Reconciler.Mount.cs | Passes ownership to newly mounted components. |
| src/Reactor/Core/Reconciler.cs | Carries ownership through component updates. |
| src/Reactor/Core/ContentOwner.cs | Defines the internal owner contract and resolution states. |
| skills/windowing.md | Documents embedded picker support. |
| plugins/reactor/skills/reactor-windowing/SKILL.md | Updates shipped windowing guidance. |
| docs/specs/tasks/054-windowing-evolution-implementation.md | Records implementation and validation of embedded owners. |
| docs/specs/054-windowing-evolution.md | Specifies call-time owner resolution and failure states. |
| docs/guide/windows.md | Replaces the workaround with supported hosting behavior. |
| docs/_pipeline/templates/windows.md.dt | Updates the source template for the windows guide. |
| CHANGELOG.md | Announces embedded and raw-window picker support. |
Review effort: Balanced
Use a using declaration for the ReactorHostControl in Pickers_HostControl. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0095f9f3-4194-447b-bb62-a5a9914004c3
Alexandre Zollinger Chohfi (azchohfi)
requested a balanced review from Copilot
October 21, 2026 21:53
…rt-reactor-host-control
Alexandre Zollinger Chohfi (azchohfi)
requested a balanced review from Copilot
October 21, 2026 22:17
…rt-reactor-host-control
Alexandre Zollinger Chohfi (azchohfi)
requested a balanced review from Copilot
October 21, 2026 22:40
…rt-reactor-host-control
Alexandre Zollinger Chohfi (azchohfi)
requested a balanced review from Copilot
October 21, 2026 23:02
Alexandre Zollinger Chohfi (azchohfi)
added
ready-for-review
Resolve against #1399's window-hook owner: Reconciler.OwningWindow reads become Owner?.ReactorWindow (TitleBar mount/update/unmount), keep Reconciler.HostWindow, and RenderContext.OwningReactorWindow reads the owner resolver with #1399's no-fallback rule. Specs 036/054 describe the shared owner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0095f9f3-4194-447b-bb62-a5a9914004c3
…rt-reactor-host-control
Alexandre Zollinger Chohfi (azchohfi)
deleted the
azchohfi-picker-support-reactor-host-control
branch
October 22, 2026 17:06
Alexandre Zollinger Chohfi (azchohfi)
removed
the
ready-for-review
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.
Summary
PickFileAsync/PickMultipleFilesAsync/PickFolderAsync/PickMultipleFoldersAsync/PickSaveFileAsyncnow work in content hosted byReactorHostControland by aReactorHostbuilt over a rawWindow. Before this change they threw "requires a component rendered in a ReactorWindow", and the docs pointed at a native-picker workaround.The reconciler now carries a small internal owner resolver instead of a
ReactorWindow:IContentOwner(src/Reactor/Core/ContentOwner.cs) exposesReactorWindow?andTryGetWindowId(out WindowId). The second returns one ofAvailable,Closed,DetachedorWrongThread. Both hosts implement it explicitly, so there is no public API change.ReactorHostresolves_window.AppWindow.Id, whether or not aReactorWindowclaimed it. It refuses withClosedonce the host is disposed, the window has closed, or the claimingReactorWindowis closing. Hosts owned by aReactorWindowbehave as before.ReactorHostControlresolvesXamlRoot.ContentIslandEnvironment.AppWindowIdwhen the picker is called, so a control moved to another window follows it. While the control is unloaded it reportsDetached. Unloaded does not unmount the tree ([Bug] ReactorHostControl is permanently disposed on Unloaded — blocks WinUI.Dock / TabView / NavigationView hosting #344), so this is checked on each call rather than releasing the owner.Reconciler.OwningWindowbecomesReconciler.Owner, andRenderContext._owningWindow/_owningWindowReleasedbecome_owner/_ownerReleased.ReactorHostControl's rootBeginRendercalls now pass the owner. The owner is still released on unmount and when the root is replaced.ReactorWindow. The Migrate picker helpers to Microsoft.Windows.Storage.Pickers #1374 picker change is still Unreleased.RenderContext.OwningReactorWindow => _owner?.ReactorWindowis added for Resolve window hooks (UseWindow, UseDpi, …) from the reconciler-supplied owner, not ActiveHostInternal #1396 (window hooks). That shape was agreed with the Resolve window hooks (UseWindow, UseDpi, …) from the reconciler-supplied owner, not ActiveHostInternal #1396 session; whichever PR lands second rebases.Docs: the windows guide template drops the workaround caveat and the guide is recompiled. Also updated: both windowing skills, spec 054 §8.3, the 054 task doc, the CHANGELOG and the
Pick*AsyncXML remarks (every throw condition).Linked issue / spec
Fixes #1395. Part of spec 054 §8.3. Follow-up to #1394.
Test plan
tests/Reactor.Tests/PickerTests.cs. A fakeIContentOwnerchecks that each status maps to its own error and never reaches the picker service.Availableroutes the id resolved at call time.RunCleanupsandDetachFromHostgive the "no longer mounted" error, and rendering again re-records the owner.Pickers_HostControl: a control in a separate raw window. All 5 operations, plus the nested class, function, memo and virtualized-row components, route to that window's id; guards check it is not the harness or active-host window. Calls off the UI thread are rejected. Unloaded calls are rejected for all 5 operations. Moved into a second window, calls route to the new id. AfterDispose(), calls get "no longer mounted".Pickers_RawWindowHost: calls route to the raw window. They are rejected off the UI thread, and after the window closes they get "no longer mounted".Pickers_EmbeddedTeardownAndFunctionRoots: a component cleanup calls a sibling's picker. That call is refused with the closed error in two cases: while aReactorHostControlis disposed (still loaded), and while a raw window closes. A function root mounted directly on either host routes to its window.DetachedturnsPickers_HostControlred.BeginRender, turns all 8 teardown and function-root checks red. I confirmed the rebuilt dll was fresh before trusting this.dotnet test tests/Reactor.Tests --filter-class "*PickerTests*": passed (48).dotnet test tests/Reactor.Tests: passed (14,929 passed, 64 skipped, 0 failed).dotnet run --project tests/Reactor.AppTests.Host --no-build -c Debug -p:Platform=x64 -- --self-test --filter "Pickers": 0 failures, on binaries whose timestamps I confirmed were fresh.-- --self-test): 2 failures,PDM_Stack_Swap_AllSurvivorsKeepIdentityand anAutoSuggestBoxCollectedStateTicktimeout. That run overlapped with the full unit suite. Neither fixture touches pickers or owners, and both passed when re-run alone (PDM 3×).dotnet restore Reactor.slnxthendotnet build Reactor.slnx --no-restore -c Release: compiled with no C# or IDE warnings or errors. This machine cannot reach nuget.org (TLS proxy), so I restored from the local package cache with deps: Windows NuGet updates #1393's package bumps reverted locally (not committed). For the same reason, the CLI's nestedReactor.SignaturesGenbuild fails withNU1900; CI is not affected.dotnet run --project tools/Reactor.DocPipeline -- compile --topic windows --no-screenshots --skip-diagrams --skip-reference: onlydocs/guide/windows.mdchanged.Risk / breaking changes
ReactorHostbuilt directly over aWindowcan now open pickers, including the tray flyout host and test harnesses. Before, it threw. In the tray flyout, opening a picker deactivates the flyout, and light-dismiss closes it. The awaiting handler still receives the result.