Repository navigation
Migrate picker helpers to Microsoft.Windows.Storage.Pickers - #1394
Open
Alexandre Zollinger Chohfi (azchohfi) wants to merge 1 commit into
Open
Alexandre Zollinger Chohfi (azchohfi) wants to merge 1 commit into
Alexandre Zollinger Chohfi (azchohfi) wants to merge 1 commit into
Conversation
Rebuild the picker helpers on Microsoft.Windows.Storage.Pickers (WindowId constructor, no InitializeWithWindow) and return path-based PickFileResult / PickFolderResult. Rename them as non-hooks (PickFileAsync, PickMultipleFilesAsync, PickFolderAsync, PickSaveFileAsync) and add PickMultipleFoldersAsync, so calling them from handlers no longer raises REACTOR_HOOKS_001/005. Options gain Title, SuggestedFolder, SuggestedStartFolder, InitialFileTypeIndex, open-picker FileTypeChoices, DefaultFileExtension and ShowOverwritePrompt. FileTypeChoices becomes an ordered IReadOnlyList<FileTypeChoice>, validated before the owner lookup. FolderPickerOptions.FileTypeFilter and SuggestedSaveFile are removed (no native equivalent). RenderContext now captures its owning window during render instead of reading ReactorApp.ActiveHostInternal at call time, which parented A's picker to window B and threw after any window closed. Fixes #1374 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 890c9be9-295c-487f-a96e-93eaa64994b0
Alexandre Zollinger Chohfi (azchohfi)
requested a review
from Chris Anderson (codemonkeychris)
as a code owner
October 19, 2026 23:20
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 / |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| public object? Options; | ||
| public string? Operation; | ||
| public int Calls; | ||
| public IReadOnlyList<PickFileResult> Files = new List<PickFileResult>(); |
| public string? Operation; | ||
| public int Calls; | ||
| public IReadOnlyList<PickFileResult> Files = new List<PickFileResult>(); | ||
| public IReadOnlyList<PickFolderResult> Folders = new List<PickFolderResult>(); |
| stub.Error = error; | ||
| Exception? actual = null; | ||
| try { await component.Operations[i](); } | ||
| catch (Exception ex) { actual = ex; } |
| var calls = stub.Calls; | ||
| Exception? error = null; | ||
| try { await a.Operations[i](); } | ||
| catch (Exception ex) { error = ex; } |
Contributor
🧪 Merged coverageCoverage for
No coverage change beyond the noise floor. ✅ ✅ higher / |
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.
Summary
Moves Reactor's picker helpers from the UWP
Windows.Storage.Pickers+InitializeWithWindowtoMicrosoft.Windows.Storage.Pickers, which ships in the Foundation 2.1.0 package thatMicrosoft.WindowsAppSDK.WinUI2.2.1 already pulls in. Pickers are now constructed from the owner'sAppWindow.Id, so there is no HWND/COM interop, and they work in elevated apps. The PR also fixes the owning-window bug and the hook-analyzer false positives from #1372's review.PickFileAsync,PickMultipleFilesAsync,PickFolderAsync,PickSaveFileAsync, plus the newPickMultipleFoldersAsync, onRenderContextwith protected forwarders onComponent.HookRulesAnalyzer. Calling them from a click lambda or a named handler no longer raises REACTOR_HOOKS_001/005, and the analyzer itself is unchanged.mur checkwrapsdotnet build, so it picks up the same behaviour.PickFileResult?/PickFolderResult?/IReadOnlyList<…>, all path-based. Cancellation returnsnullor an empty list.Title,SuggestedFolder,SuggestedStartFolder,InitialFileTypeIndex, open-pickerFileTypeChoices, and save-onlyDefaultFileExtension/ShowOverwritePrompt.FileTypeChoicesis now an orderedIReadOnlyList<FileTypeChoice>. The native map keeps insertion order, and that order sets the default entry. Reactor rejects null entries, blank or duplicate labels, and empty extension lists before the owner lookup.FolderPickerOptions.FileTypeFilterandSaveFilePickerOptions.SuggestedSaveFile, which have no native equivalent."*"fallback is dropped, because the native picker already treats an empty filter as All Files.ReactorApp.ActiveHostInternalat call time. That field names the most recently rendered or constructed host, and becomesnullafter any host is disposed. So with windows A then B open, A's picker parented to B, and every picker threw once any window closed.RenderContextnow captures the owning window inBeginRender, whereReactorHost.Renderscopes that field to itself. It clears the capture on detach and rejects a closed owner.Linked issue / spec
Fixes #1374. Updates
docs/specs/054-windowing-evolution.md§8.3 and its task file (new §7.3a).Test plan
tests/Reactor.Tests/PickerTests.cs(renamed fromPickerHookTests.cs): null options, missing-owner message for all five operations, and choice validation through the public API.HookRulesAnalyzerTests:Pick*Asynccalled from lambdas, a conditional lambda and named handlers, through bothComponentandRenderContext, report nothing. AUse*-named positive control in the same call shapes reports HOOKS_001 (lambda) and HOOKS_005 (named handler), so the clean result isn't vacuous.Pickers_Suite(renamed fromPickerHooks_Suite): routing, cancellation, collection pass-through, off-UI-thread rejection, error propagation, and read-back of every option on realMicrosoft.Windows.Storage.Pickersobjects, including non-alphabeticalFileTypeChoicesorder on both the open and save pickers.Pickers_OwningWindow: with A then B open, A's five helpers route to A before and after B closes, and B's helpers are rejected after B closes.ActiveHostInternalnames B.GetPickerOwnerreverted to the call-time lookup, all 10 routing checks fail.Phase7WindowingFixturespicker checks.dotnet test tests/Reactor.Tests -c Release -p:Platform=x64→ passed: 14,971 total, 0 failed, 64 skipped. This includes the doc, API-index and search-index gates.dotnet run --project tests/Reactor.AppTests.Host -- --self-test(full corpus) → passed: 8,864 checks, 0 failures.dotnet restore Reactor.slnx+dotnet build Reactor.slnx --no-restore -c Release→ no compile errors. The only failures were environmental:Reactor.AppTests,Reactor.PackagedTests,Reactor.SelfTests).NuGetAudit=falselocally to get past it.Reactor.Testsitself was built separately in Release (warnings as errors) with-p:Platform=x64.dotnet test tests/Reactor.SelfTestsand the packaged tier were not run locally because their packages couldn't be restored offline. CI covers both.windowsandhookstopics, reverted unrelated screenshot churn, and deleted the four stalereference/hooks/Use*PickerAsync.mdpages (the pipeline doesn't prune).mur --regen-apiregenerated both byte-identicalreactor.api.txtcopies, and both windowing skills are updated.Risk / breaking changes
Use*PickerAsyncmethods are renamed and returnPickFileResult/PickFolderResultinstead ofStorageFile/StorageFolder.Microsoft.Windows.Storage.Pickers. Both namespaces definePickerLocationId/PickerViewMode, so import only the new one.SaveFilePickerOptions.FileTypeChoiceschanges from a dictionary toIReadOnlyList<FileTypeChoice>.FolderPickerOptions.FileTypeFilterandSuggestedSaveFileare removed.BeginRenderadds one volatile static read per component render.ReactorHostControldoesn't scopeActiveHostInternal, so it can capture an unrelated window. Every window hook (UseWindow,UseDpi, …) shares this today. Documented in the windows guide and spec 054 §8.3; I suggest a follow-up issue.