From 497f2ff1eaaead9e5df7cb39e13348446cc944eb Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Mon, 19 Oct 2026 16:19:52 -0700 Subject: [PATCH 1/5] Migrate picker helpers to Microsoft.Windows.Storage.Pickers 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, 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 --- docs/_pipeline/apps/windows/App.cs | 31 +- docs/_pipeline/templates/hooks.md.dt | 13 +- docs/_pipeline/templates/windows.md.dt | 75 +++-- docs/guide/hooks.md | 44 +-- .../reference/hooks/UseFilePickerAsync.md | 20 -- .../reference/hooks/UseFolderPickerAsync.md | 20 -- .../hooks/UseMultipleFilesPickerAsync.md | 20 -- .../reference/hooks/UseSaveFilePickerAsync.md | 20 -- docs/guide/windows.md | 106 ++++--- docs/specs/002-winui3-gap-analysis.md | 4 +- docs/specs/054-windowing-evolution.md | 58 +++- ...activity-and-representation-exploration.md | 4 +- .../054-windowing-evolution-implementation.md | 27 +- .../reactor-dsl/references/reactor.api.txt | 45 ++- .../reactor/skills/reactor-windowing/SKILL.md | 31 +- samples/ReactorGallery/GAPS.md | 2 +- skills/reactor.api.txt | 45 ++- skills/windowing.md | 30 +- src/Reactor/Core/Component.cs | 24 +- src/Reactor/Core/PickerHooks.cs | 130 --------- src/Reactor/Core/Pickers.cs | 230 +++++++++++++++ src/Reactor/Core/RenderContext.cs | 91 ++++-- src/Reactor/Hosting/ReactorWindow.cs | 2 + .../Fixtures/Phase7WindowingFixtures.cs | 51 ++-- .../SelfTest/Fixtures/PickerHooksFixtures.cs | 177 ----------- .../SelfTest/Fixtures/PickersFixtures.cs | 276 ++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 6 +- .../AnalyzerTests/HookRulesAnalyzerTests.cs | 104 +++++++ tests/Reactor.Tests/PickerHookTests.cs | 54 ---- tests/Reactor.Tests/PickerTests.cs | 121 ++++++++ 30 files changed, 1167 insertions(+), 694 deletions(-) delete mode 100644 docs/guide/reference/hooks/UseFilePickerAsync.md delete mode 100644 docs/guide/reference/hooks/UseFolderPickerAsync.md delete mode 100644 docs/guide/reference/hooks/UseMultipleFilesPickerAsync.md delete mode 100644 docs/guide/reference/hooks/UseSaveFilePickerAsync.md delete mode 100644 src/Reactor/Core/PickerHooks.cs create mode 100644 src/Reactor/Core/Pickers.cs delete mode 100644 tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickerHooksFixtures.cs create mode 100644 tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs delete mode 100644 tests/Reactor.Tests/PickerHookTests.cs create mode 100644 tests/Reactor.Tests/PickerTests.cs diff --git a/docs/_pipeline/apps/windows/App.cs b/docs/_pipeline/apps/windows/App.cs index afa9287e7..27da2a834 100644 --- a/docs/_pipeline/apps/windows/App.cs +++ b/docs/_pipeline/apps/windows/App.cs @@ -576,35 +576,36 @@ class PickerDemo : Component public override Element Render() { // - // Capture helpers during Render; invoke them only from UI-thread events. - var pickFile = UseFilePickerAsync; - var pickFiles = UseMultipleFilesPickerAsync; - var pickFolder = UseFolderPickerAsync; - var saveFile = UseSaveFilePickerAsync; - + // Pickers are not hooks: call them directly from UI-thread event handlers. return VStack( Button("Open one...", async () => { - var file = await pickFile(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); + var file = await PickFileAsync(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); if (file is null) return; // User cancelled. + var text = await File.ReadAllTextAsync(file.Path); }), Button("Open files...", async () => { - var files = await pickFiles(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); + var files = await PickMultipleFilesAsync(new FilePickerOptions + { + Title = "Choose notes", + FileTypeChoices = [new("Notes", [".txt", ".md"]), new("JSON", [".json"])], + }); if (files.Count == 0) return; // User cancelled. }), - Button("Select folder...", async () => + Button("Select folders...", async () => { - var folder = await pickFolder(new FolderPickerOptions()); - if (folder is null) return; + var folders = await PickMultipleFoldersAsync(new FolderPickerOptions()); + if (folders.Count == 0) return; }), Button("Save as...", async () => { - var file = await saveFile(new SaveFilePickerOptions( - new Dictionary> { ["Text"] = [".txt"] }, + var target = await PickSaveFileAsync(new SaveFilePickerOptions( + [new("Text", [".txt"]), new("Markdown", [".md"])], SuggestedFileName: "report")); - if (file is null) return; - // Write the desired content here; selecting a target is not a save. + if (target is null) return; + // Selecting a target is not a save: write the content yourself. + await File.WriteAllTextAsync(target.Path, "report body"); })); // } diff --git a/docs/_pipeline/templates/hooks.md.dt b/docs/_pipeline/templates/hooks.md.dt index 8e9e3120a..290e60da7 100644 --- a/docs/_pipeline/templates/hooks.md.dt +++ b/docs/_pipeline/templates/hooks.md.dt @@ -47,16 +47,15 @@ lifecycle methods. | | `TSnapshot` | Bridge subscribe/getSnapshot stores into Reactor and re-render only when the snapshot changes. | | | `AsyncValue` | Cached async read (see [Async Resources](async-resources.md)). | | | `(T, Action)` | `UseState` that survives unmount/remount within the process — in-memory, cleared on exit (see [Persistence](persistence.md)). | -| / | `Task` / `Task>` | Open one or multiple files (see [Pickers](windows.md#pickers)). | -| / | `Task` / `Task` | Select a folder or save-file target (see [Pickers](windows.md#pickers)). | ### Pattern: pick files from a click handler -Picker helpers are the exception to positional hook rules: they do not allocate -state slots and must run from UI-thread event handlers, not during `Render()`. -Cancellation returns `null` for single-file, folder, and save dialogs, or an empty -list for multiple files. See [Pickers](windows.md#pickers) for the full options and -the open/folder/save pattern: +The picker methods (`PickFileAsync`, `PickMultipleFilesAsync`, `PickFolderAsync`, +`PickMultipleFoldersAsync`, `PickSaveFileAsync`) are deliberately **not** hooks: they +reserve no state slot and must run from UI-thread event handlers, not during +`Render()`. Call them directly inside the handler. Cancellation returns `null` for +single-file, folder, and save dialogs, or an empty list for multiple selections. See +[Pickers](windows.md#pickers) for the full options and the open/folder/save pattern: ```csharp snippet="windows/pickers" ``` diff --git a/docs/_pipeline/templates/windows.md.dt b/docs/_pipeline/templates/windows.md.dt index 9b8b53b5d..681ff0a36 100644 --- a/docs/_pipeline/templates/windows.md.dt +++ b/docs/_pipeline/templates/windows.md.dt @@ -553,41 +553,62 @@ Caveats: ## Pickers -Picker helpers create native `Windows.Storage.Pickers` dialogs and initialize them with the owning -window HWND, so the picker is modal to the correct window without app code doing -HWND interop. Both `Component` and `RenderContext` expose the same operations: +Picker methods create `Microsoft.Windows.Storage.Pickers` dialogs owned by the +component's window, so each dialog is modal to the right window without any +HWND interop. They also work in elevated (administrator) apps, unlike the UWP +`Windows.Storage.Pickers` dialogs. `Component` and `RenderContext` expose the +same operations: -| Helper | Options | Cancellation | -| --- | --- | --- | -| | `FilePickerOptions` | `null` | -| | `FilePickerOptions` | Empty `IReadOnlyList` | -| | `FolderPickerOptions` | `null` | -| | `SaveFilePickerOptions` | `null` | - -Open and folder options include file-type filters (null or empty means `"*"`), -suggested location, commit-button text, an optional view mode, and a settings -identifier for Windows to remember dialog state. Existing positional constructors -are unchanged; additional options are init properties. Save options require a -non-empty dictionary mapping display labels to extension lists, and support a -suggested file name, location, button text, settings identifier, and existing save -file. Windows validates extension syntax. The save helper only selects/creates -the target; the caller must write its contents. +| Method | Options | Returns | Cancellation | +| --- | --- | --- | --- | +| `PickFileAsync` | `FilePickerOptions` | `PickFileResult?` | `null` | +| `PickMultipleFilesAsync` | `FilePickerOptions` | `IReadOnlyList` | Empty list | +| `PickFolderAsync` | `FolderPickerOptions` | `PickFolderResult?` | `null` | +| `PickMultipleFoldersAsync` | `FolderPickerOptions` | `IReadOnlyList` | Empty list | +| `PickSaveFileAsync` | `SaveFilePickerOptions` | `PickFileResult?` | `null` | + +Each result exposes the chosen `Path`. Open the file with `System.IO`, or with +`StorageFile.GetFileFromPathAsync` when you need a `StorageFile`. + +All three option records share a suggested start location, commit-button text, +a dialog `Title`, a `SettingsIdentifier` that Windows uses to remember dialog state, +`SuggestedFolder` (always open there), and `SuggestedStartFolder` (open there until +Windows remembers a location). Both folder properties take absolute paths. Open +and folder options add an optional `ViewMode`. File options add: + +- `FileTypeFilter`: extensions such as `".txt"`. Null or empty shows all files. +- `FileTypeChoices`: an ordered list of `FileTypeChoice(label, extensions)` groups. + When non-empty, it replaces `FileTypeFilter`. +- `InitialFileTypeIndex`: the entry selected first. Null lets Windows choose. + +Save options require at least one `FileTypeChoice`. The list order is the order +Windows shows, and the first entry is the default unless `InitialFileTypeIndex` +says otherwise. Labels must be unique and non-blank, and every choice needs at +least one extension. Save options also take a suggested file name, +`DefaultFileExtension`, and `ShowOverwritePrompt` (default `true`). The save +method only selects a target path; the caller must create and write the file. + +Both `Windows.Storage.Pickers` and `Microsoft.Windows.Storage.Pickers` define +`PickerLocationId` and `PickerViewMode`, so import only the +`Microsoft.Windows.Storage.Pickers` namespace in files that build picker options. ```csharp snippet="windows/pickers" ``` Caveats: -- Unlike positional state hooks, these helpers open a dialog when called. Invoke - them from UI-thread event handlers, not during `Render()` or in `Task.Run`. -- Calling without an owning Reactor window throws `InvalidOperationException`. +- These methods are not hooks: they reserve no hook slot and open a dialog when + called. Call them from UI-thread event handlers, not during `Render()` or in + `Task.Run`. Their `Pick*` names keep the hook-rules analyzer quiet in handlers. +- The owner is the window that last rendered the component, not whichever window + is active when the handler runs. If that window has closed, or the component + never rendered in a Reactor window, the call throws `InvalidOperationException`. +- Option validation (missing or duplicate file-type choices) throws `ArgumentException` + before any dialog opens. Windows validates extension syntax and folder paths. - Picker errors propagate to the caller; cancellation is not an error. -- Reactor never accepts arbitrary HWNDs; it always uses `UseWindow().NativeWindow`. - Framework tests use an internal picker-service seam instead of opening modal dialogs. -- These helpers retain `Windows.Storage` results; they do not migrate to the - separate `Microsoft.Windows.Storage.Pickers` API or change its OS requirements. -- The native `FolderPicker` supports one folder at a time, not multi-selection. - +- Content hosted through `ReactorHostControl` has no owning Reactor window of its + own, so it cannot rely on these methods to pick the correct owner. ## WPF / UWP migration map Coming from a pre-054 Reactor app instead? See @@ -606,7 +627,7 @@ fields that were removed and what replaced them. | Manual settings persistence | `.WithPersistence(id)` | Opt-in, one line. | | `TaskbarItemInfo` | `TaskbarItem` | Facade over progress, overlay, description, thumb buttons. | | WPF `JumpTask` / `JumpList` | `JumpListItem` / `JumpList` | Process-wide, not per-window; `UpdateAsync` replaces the whole list. | -| UWP/WinUI picker HWND setup | Open, multiple-file, folder, and save picker helpers | Owning HWND is wired automatically. | +| UWP/WinUI picker HWND setup | `PickFileAsync`, `PickMultipleFilesAsync`, `PickFolderAsync`, `PickMultipleFoldersAsync`, `PickSaveFileAsync` | Owner window is wired automatically; results are paths. | ## Finding and enumerating windows diff --git a/docs/guide/hooks.md b/docs/guide/hooks.md index 1325037cf..9a3f82462 100644 --- a/docs/guide/hooks.md +++ b/docs/guide/hooks.md @@ -36,47 +36,47 @@ lifecycle methods. | [UseExternalStore](reference/hooks/UseExternalStore.md) | `TSnapshot` | Bridge subscribe/getSnapshot stores into Reactor and re-render only when the snapshot changes. | | [UseResource](reference/hooks/UseResource.md) | `AsyncValue` | Cached async read (see [Async Resources](async-resources.md)). | | [UsePersisted](reference/hooks/UsePersisted.md) | `(T, Action)` | `UseState` that survives unmount/remount within the process — in-memory, cleared on exit (see [Persistence](persistence.md)). | -| [UseFilePickerAsync](reference/hooks/UseFilePickerAsync.md) / [UseMultipleFilesPickerAsync](reference/hooks/UseMultipleFilesPickerAsync.md) | `Task` / `Task>` | Open one or multiple files (see [Pickers](windows.md#pickers)). | -| [UseFolderPickerAsync](reference/hooks/UseFolderPickerAsync.md) / [UseSaveFilePickerAsync](reference/hooks/UseSaveFilePickerAsync.md) | `Task` / `Task` | Select a folder or save-file target (see [Pickers](windows.md#pickers)). | ### Pattern: pick files from a click handler -Picker helpers are the exception to positional hook rules: they do not allocate -state slots and must run from UI-thread event handlers, not during `Render()`. -Cancellation returns `null` for single-file, folder, and save dialogs, or an empty -list for multiple files. See [Pickers](windows.md#pickers) for the full options and -the open/folder/save pattern: +The picker methods (`PickFileAsync`, `PickMultipleFilesAsync`, `PickFolderAsync`, +`PickMultipleFoldersAsync`, `PickSaveFileAsync`) are deliberately **not** hooks: they +reserve no state slot and must run from UI-thread event handlers, not during +`Render()`. Call them directly inside the handler. Cancellation returns `null` for +single-file, folder, and save dialogs, or an empty list for multiple selections. See +[Pickers](windows.md#pickers) for the full options and the open/folder/save pattern: ```csharp -// Capture helpers during Render; invoke them only from UI-thread events. -var pickFile = UseFilePickerAsync; -var pickFiles = UseMultipleFilesPickerAsync; -var pickFolder = UseFolderPickerAsync; -var saveFile = UseSaveFilePickerAsync; - +// Pickers are not hooks: call them directly from UI-thread event handlers. return VStack( Button("Open one...", async () => { - var file = await pickFile(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); + var file = await PickFileAsync(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); if (file is null) return; // User cancelled. + var text = await File.ReadAllTextAsync(file.Path); }), Button("Open files...", async () => { - var files = await pickFiles(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); + var files = await PickMultipleFilesAsync(new FilePickerOptions + { + Title = "Choose notes", + FileTypeChoices = [new("Notes", [".txt", ".md"]), new("JSON", [".json"])], + }); if (files.Count == 0) return; // User cancelled. }), - Button("Select folder...", async () => + Button("Select folders...", async () => { - var folder = await pickFolder(new FolderPickerOptions()); - if (folder is null) return; + var folders = await PickMultipleFoldersAsync(new FolderPickerOptions()); + if (folders.Count == 0) return; }), Button("Save as...", async () => { - var file = await saveFile(new SaveFilePickerOptions( - new Dictionary> { ["Text"] = [".txt"] }, + var target = await PickSaveFileAsync(new SaveFilePickerOptions( + [new("Text", [".txt"]), new("Markdown", [".md"])], SuggestedFileName: "report")); - if (file is null) return; - // Write the desired content here; selecting a target is not a save. + if (target is null) return; + // Selecting a target is not a save: write the content yourself. + await File.WriteAllTextAsync(target.Path, "report body"); })); ``` diff --git a/docs/guide/reference/hooks/UseFilePickerAsync.md b/docs/guide/reference/hooks/UseFilePickerAsync.md deleted file mode 100644 index fb0938954..000000000 --- a/docs/guide/reference/hooks/UseFilePickerAsync.md +++ /dev/null @@ -1,20 +0,0 @@ -# UseFilePickerAsync - -`method` -_cref_: `M:Microsoft.UI.Reactor.Core.RenderContext.UseFilePickerAsync(Microsoft.UI.Reactor.Core.FilePickerOptions)` - -> **Learn more:** [Hooks](../../hooks.md), [Effects](../../effects.md) - -## Summary - -Opens a single-file picker initialized with the owning window HWND. Must be called on the UI thread. - -## Discussion - -Call from an event handler, not during Render. Cancellation returns null. - -## Featured in - -- [Hooks](../../hooks.md) -- [Windows](../../windows.md) - diff --git a/docs/guide/reference/hooks/UseFolderPickerAsync.md b/docs/guide/reference/hooks/UseFolderPickerAsync.md deleted file mode 100644 index 9596ae8f3..000000000 --- a/docs/guide/reference/hooks/UseFolderPickerAsync.md +++ /dev/null @@ -1,20 +0,0 @@ -# UseFolderPickerAsync - -`method` -_cref_: `M:Microsoft.UI.Reactor.Core.RenderContext.UseFolderPickerAsync(Microsoft.UI.Reactor.Core.FolderPickerOptions)` - -> **Learn more:** [Hooks](../../hooks.md), [Effects](../../effects.md) - -## Summary - -Opens a folder picker initialized with the owning window HWND. Must be called on the UI thread. - -## Discussion - -Call from an event handler, not during Render. Cancellation returns null. - -## Featured in - -- [Hooks](../../hooks.md) -- [Windows](../../windows.md) - diff --git a/docs/guide/reference/hooks/UseMultipleFilesPickerAsync.md b/docs/guide/reference/hooks/UseMultipleFilesPickerAsync.md deleted file mode 100644 index 98fc3498e..000000000 --- a/docs/guide/reference/hooks/UseMultipleFilesPickerAsync.md +++ /dev/null @@ -1,20 +0,0 @@ -# UseMultipleFilesPickerAsync - -`method` -_cref_: `M:Microsoft.UI.Reactor.Core.RenderContext.UseMultipleFilesPickerAsync(Microsoft.UI.Reactor.Core.FilePickerOptions)` - -> **Learn more:** [Hooks](../../hooks.md), [Effects](../../effects.md) - -## Summary - -Opens a multiple-file picker on the owning window's UI thread. - -## Discussion - -Call from an event handler, not during Render. Cancellation returns an empty list. - -## Featured in - -- [Hooks](../../hooks.md) -- [Windows](../../windows.md) - diff --git a/docs/guide/reference/hooks/UseSaveFilePickerAsync.md b/docs/guide/reference/hooks/UseSaveFilePickerAsync.md deleted file mode 100644 index d149ff0e1..000000000 --- a/docs/guide/reference/hooks/UseSaveFilePickerAsync.md +++ /dev/null @@ -1,20 +0,0 @@ -# UseSaveFilePickerAsync - -`method` -_cref_: `M:Microsoft.UI.Reactor.Core.RenderContext.UseSaveFilePickerAsync(Microsoft.UI.Reactor.Core.SaveFilePickerOptions)` - -> **Learn more:** [Hooks](../../hooks.md), [Effects](../../effects.md) - -## Summary - -Opens a save-file picker on the owning window's UI thread. - -## Discussion - -Call from an event handler, not during Render. Cancellation returns null. Writing to the returned file is the caller's responsibility. - -## Featured in - -- [Hooks](../../hooks.md) -- [Windows](../../windows.md) - diff --git a/docs/guide/windows.md b/docs/guide/windows.md index 4b6583b38..13e5e1276 100644 --- a/docs/guide/windows.md +++ b/docs/guide/windows.md @@ -855,71 +855,93 @@ Caveats: ## Pickers -Picker helpers create native `Windows.Storage.Pickers` dialogs and initialize them with the owning -window HWND, so the picker is modal to the correct window without app code doing -HWND interop. Both `Component` and `RenderContext` expose the same operations: +Picker methods create `Microsoft.Windows.Storage.Pickers` dialogs owned by the +component's window, so each dialog is modal to the right window without any +HWND interop. They also work in elevated (administrator) apps, unlike the UWP +`Windows.Storage.Pickers` dialogs. `Component` and `RenderContext` expose the +same operations: -| Helper | Options | Cancellation | -| --- | --- | --- | -| [UseFilePickerAsync](reference/hooks/UseFilePickerAsync.md) | `FilePickerOptions` | `null` | -| [UseMultipleFilesPickerAsync](reference/hooks/UseMultipleFilesPickerAsync.md) | `FilePickerOptions` | Empty `IReadOnlyList` | -| [UseFolderPickerAsync](reference/hooks/UseFolderPickerAsync.md) | `FolderPickerOptions` | `null` | -| [UseSaveFilePickerAsync](reference/hooks/UseSaveFilePickerAsync.md) | `SaveFilePickerOptions` | `null` | - -Open and folder options include file-type filters (null or empty means `"*"`), -suggested location, commit-button text, an optional view mode, and a settings -identifier for Windows to remember dialog state. Existing positional constructors -are unchanged; additional options are init properties. Save options require a -non-empty dictionary mapping display labels to extension lists, and support a -suggested file name, location, button text, settings identifier, and existing save -file. Windows validates extension syntax. The save helper only selects/creates -the target; the caller must write its contents. +| Method | Options | Returns | Cancellation | +| --- | --- | --- | --- | +| `PickFileAsync` | `FilePickerOptions` | `PickFileResult?` | `null` | +| `PickMultipleFilesAsync` | `FilePickerOptions` | `IReadOnlyList` | Empty list | +| `PickFolderAsync` | `FolderPickerOptions` | `PickFolderResult?` | `null` | +| `PickMultipleFoldersAsync` | `FolderPickerOptions` | `IReadOnlyList` | Empty list | +| `PickSaveFileAsync` | `SaveFilePickerOptions` | `PickFileResult?` | `null` | + +Each result exposes the chosen `Path`. Open the file with `System.IO`, or with +`StorageFile.GetFileFromPathAsync` when you need a `StorageFile`. + +All three option records share a suggested start location, commit-button text, +a dialog `Title`, a `SettingsIdentifier` that Windows uses to remember dialog state, +`SuggestedFolder` (always open there), and `SuggestedStartFolder` (open there until +Windows remembers a location). Both folder properties take absolute paths. Open +and folder options add an optional `ViewMode`. File options add: + +- `FileTypeFilter`: extensions such as `".txt"`. Null or empty shows all files. +- `FileTypeChoices`: an ordered list of `FileTypeChoice(label, extensions)` groups. + When non-empty, it replaces `FileTypeFilter`. +- `InitialFileTypeIndex`: the entry selected first. Null lets Windows choose. + +Save options require at least one `FileTypeChoice`. The list order is the order +Windows shows, and the first entry is the default unless `InitialFileTypeIndex` +says otherwise. Labels must be unique and non-blank, and every choice needs at +least one extension. Save options also take a suggested file name, +`DefaultFileExtension`, and `ShowOverwritePrompt` (default `true`). The save +method only selects a target path; the caller must create and write the file. + +Both `Windows.Storage.Pickers` and `Microsoft.Windows.Storage.Pickers` define +`PickerLocationId` and `PickerViewMode`, so import only the +`Microsoft.Windows.Storage.Pickers` namespace in files that build picker options. ```csharp -// Capture helpers during Render; invoke them only from UI-thread events. -var pickFile = UseFilePickerAsync; -var pickFiles = UseMultipleFilesPickerAsync; -var pickFolder = UseFolderPickerAsync; -var saveFile = UseSaveFilePickerAsync; - +// Pickers are not hooks: call them directly from UI-thread event handlers. return VStack( Button("Open one...", async () => { - var file = await pickFile(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); + var file = await PickFileAsync(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); if (file is null) return; // User cancelled. + var text = await File.ReadAllTextAsync(file.Path); }), Button("Open files...", async () => { - var files = await pickFiles(new FilePickerOptions(FileTypeFilter: [".txt", ".md"])); + var files = await PickMultipleFilesAsync(new FilePickerOptions + { + Title = "Choose notes", + FileTypeChoices = [new("Notes", [".txt", ".md"]), new("JSON", [".json"])], + }); if (files.Count == 0) return; // User cancelled. }), - Button("Select folder...", async () => + Button("Select folders...", async () => { - var folder = await pickFolder(new FolderPickerOptions()); - if (folder is null) return; + var folders = await PickMultipleFoldersAsync(new FolderPickerOptions()); + if (folders.Count == 0) return; }), Button("Save as...", async () => { - var file = await saveFile(new SaveFilePickerOptions( - new Dictionary> { ["Text"] = [".txt"] }, + var target = await PickSaveFileAsync(new SaveFilePickerOptions( + [new("Text", [".txt"]), new("Markdown", [".md"])], SuggestedFileName: "report")); - if (file is null) return; - // Write the desired content here; selecting a target is not a save. + if (target is null) return; + // Selecting a target is not a save: write the content yourself. + await File.WriteAllTextAsync(target.Path, "report body"); })); ``` Caveats: -- Unlike positional state hooks, these helpers open a dialog when called. Invoke - them from UI-thread event handlers, not during `Render()` or in `Task.Run`. -- Calling without an owning Reactor window throws `InvalidOperationException`. +- These methods are not hooks: they reserve no hook slot and open a dialog when + called. Call them from UI-thread event handlers, not during `Render()` or in + `Task.Run`. Their `Pick*` names keep the hook-rules analyzer quiet in handlers. +- The owner is the window that last rendered the component, not whichever window + is active when the handler runs. If that window has closed, or the component + never rendered in a Reactor window, the call throws `InvalidOperationException`. +- Option validation (missing or duplicate file-type choices) throws `ArgumentException` + before any dialog opens. Windows validates extension syntax and folder paths. - Picker errors propagate to the caller; cancellation is not an error. -- Reactor never accepts arbitrary HWNDs; it always uses `UseWindow().NativeWindow`. - Framework tests use an internal picker-service seam instead of opening modal dialogs. -- These helpers retain `Windows.Storage` results; they do not migrate to the - separate `Microsoft.Windows.Storage.Pickers` API or change its OS requirements. -- The native `FolderPicker` supports one folder at a time, not multi-selection. - +- Content hosted through `ReactorHostControl` has no owning Reactor window of its + own, so it cannot rely on these methods to pick the correct owner. ## WPF / UWP migration map Coming from a pre-054 Reactor app instead? See @@ -938,7 +960,7 @@ fields that were removed and what replaced them. | Manual settings persistence | `.WithPersistence(id)` | Opt-in, one line. | | `TaskbarItemInfo` | `TaskbarItem` | Facade over progress, overlay, description, thumb buttons. | | WPF `JumpTask` / `JumpList` | `JumpListItem` / `JumpList` | Process-wide, not per-window; `UpdateAsync` replaces the whole list. | -| UWP/WinUI picker HWND setup | Open, multiple-file, folder, and save picker helpers | Owning HWND is wired automatically. | +| UWP/WinUI picker HWND setup | `PickFileAsync`, `PickMultipleFilesAsync`, `PickFolderAsync`, `PickMultipleFoldersAsync`, `PickSaveFileAsync` | Owner window is wired automatically; results are paths. | ## Finding and enumerating windows diff --git a/docs/specs/002-winui3-gap-analysis.md b/docs/specs/002-winui3-gap-analysis.md index 2b0bf7468..9ba3adc35 100644 --- a/docs/specs/002-winui3-gap-analysis.md +++ b/docs/specs/002-winui3-gap-analysis.md @@ -910,7 +910,7 @@ redirection before calling `ReactorApp.Run`. |---|---|---| | **Printing** | Passthrough | Use PrintManagerInterop directly with the HWND from `ReactorHost.Window` | | **Clipboard** | Passthrough | Use Windows.ApplicationModel.DataTransfer.Clipboard | -| **File Pickers** | Exposed | `UseFilePickerAsync(FilePickerOptions)` → `Task` — owner-window initialization is handled for you | +| **File Pickers** | Exposed | `PickFileAsync(FilePickerOptions)` → `Task` (`Microsoft.Windows.Storage.Pickers`) — owner-window initialization is handled for you | **Verdict: mostly passthrough, with file pickers wrapped.** File picking was the one OS service where the HWND-initialization dance was both mandatory and easy to get wrong, so it has a hook. @@ -1012,7 +1012,7 @@ is kept as a record so the document's own history stays auditable. | P2 | ManipulationMode modifier | **Still open** | | P3 | Activation kinds | **Shipped** — `LaunchActivation` / `LaunchKind` / `DeepLinkMap` (§19). **Single-instancing did not** — `AppInstance` redirection is still the app's job | | P3 | InkCanvas / InkToolbar | **Still open** | -| P3 | App service wrappers | **Partly** — `UseFilePickerAsync` shipped; clipboard and printing still raw (§20) | +| P3 | App service wrappers | **Partly** — `PickFileAsync` and the other picker methods shipped; clipboard and printing still raw (§20) | ### Top Gaps to Address (Priority Order) diff --git a/docs/specs/054-windowing-evolution.md b/docs/specs/054-windowing-evolution.md index fff144a07..f63de9c62 100644 --- a/docs/specs/054-windowing-evolution.md +++ b/docs/specs/054-windowing-evolution.md @@ -1100,22 +1100,54 @@ return Border(...).PointerPressed(_ => beginDrag()); `beginDrag` is stable across re-renders (same delegate identity). -### 8.3 `UseFolderPicker` / `UseFilePicker` — auto-`InitializeWithWindow` +### 8.3 Owner-window pickers — `PickFileAsync` / `PickFolderAsync` / … -Already-built helpers for the WinRT pickers that need `IInitializeWithWindow` -called with the host HWND. Removes the boilerplate the source proposal -calls out in §5.2. +Helpers for the native file and folder pickers that remove the owner-window +boilerplate the source proposal calls out in §5.2. ```csharp -public sealed class FilePickerOptions { /* … */ } -public Task RenderContext.UseFilePickerAsync(FilePickerOptions options); -public Task RenderContext.UseFolderPickerAsync(FolderPickerOptions options); +// using Microsoft.Windows.Storage.Pickers; +public Task RenderContext.PickFileAsync(FilePickerOptions options); +public Task> RenderContext.PickMultipleFilesAsync(FilePickerOptions options); +public Task RenderContext.PickFolderAsync(FolderPickerOptions options); +public Task> RenderContext.PickMultipleFoldersAsync(FolderPickerOptions options); +public Task RenderContext.PickSaveFileAsync(SaveFilePickerOptions options); +public sealed record FileTypeChoice(string Label, IReadOnlyList Extensions); ``` -Implementation: `UseWindow().NativeWindow.GetWindowHandle()` → -`WinRT.Interop.InitializeWithWindow.Initialize`. The pickers themselves -are out of scope of windowing, but the boilerplate-elimination story -naturally belongs here. +`Component` exposes the same five as protected forwarders. + +**Revision (issue #1374).** The first cut shipped as `Use*PickerAsync` helpers +over the UWP `Windows.Storage.Pickers` + `InitializeWithWindow`, returning +`StorageFile` / `StorageFolder`. It was revised before 1.0: + +- **API.** Built on `Microsoft.Windows.Storage.Pickers` (Windows App SDK 1.8+; + spec 003 §20.3's recommendation). Pickers are constructed from the owner's + `AppWindow.Id`, so there is no HWND/COM interop, and they work in elevated + apps. Results are path-based `PickFileResult` / `PickFolderResult`. +- **Options.** `Title`, `SuggestedFolder`, `SuggestedStartFolder`, + `InitialFileTypeIndex`, open-picker `FileTypeChoices`, and save-only + `DefaultFileExtension` / `ShowOverwritePrompt` were added. + `FolderPickerOptions.FileTypeFilter` and `SaveFilePickerOptions.SuggestedSaveFile` + were removed (no native equivalent). `FileTypeChoices` is an ordered + `IReadOnlyList` because order sets the default entry; labels must + be unique and non-blank and each choice needs an extension, validated before the + owner lookup. +- **Owner window.** The owner was read from `ReactorApp.ActiveHostInternal` at call + time. Pickers run from event handlers, outside render, where that field names + the most recently rendered or constructed host (or null after any host is + disposed). With two windows, A's picker parented to B, and every picker threw + once any window closed. `RenderContext` now captures the owning window in + `BeginRender` (where `ReactorHost.Render` scopes that field to itself), clears + it on detach, and refuses a closed owner. Content rendered through + `ReactorHostControl`, which does not scope the field, keeps the pre-existing + limitation shared by every window hook. +- **Naming.** `HookRulesAnalyzer` treats any `Use[A-Z]*` method on + `Component` / `RenderContext` as a hook, so calling the old names from a click + lambda raised REACTOR_HOOKS_001 and from a named handler REACTOR_HOOKS_005. The + pickers reserve no hook slot and must not run during render, so they were + renamed `Pick*Async` rather than exempted from the analyzer. +- **Additions.** `PickMultipleFoldersAsync` (the new `FolderPicker` supports it). --- @@ -1241,7 +1273,7 @@ No risk — purely additive. ### Phase 7 — Picker boilerplate elimination _(small)_ -- `UseFilePickerAsync` / `UseFolderPickerAsync`. +- `PickFileAsync` / `PickMultipleFilesAsync` / `PickFolderAsync` / `PickMultipleFoldersAsync` / `PickSaveFileAsync` (originally shipped as `Use*PickerAsync`; see §8.3). Lives more in the data-system area than windowing; could be split off into its own spec if scoping requires. @@ -1373,7 +1405,7 @@ response. | `WindowMessageReceived` | ◯ deferred (design preserved) | §7.6 | | Monitor enumeration | ▲ `ReactorDisplay` | §6.7 | | Splash screen | ❌ separate spec | — | -| Picker `InitializeWithWindow` helpers | ▲ `UseFilePickerAsync` etc. | §8.3 | +| Picker `InitializeWithWindow` helpers | ▲ `PickFileAsync` etc. (`Microsoft.Windows.Storage.Pickers`) | §8.3 | | `Window.Cursor` per-window cursor | ❌ deferred | — | | `Window.AllowsTransparency` (WPF) | ❌ rejected (see `IsTransparent`) | §7.1 | | `Window.WindowChrome` (WPF) | ✅ already covered by `TitleBar(...)` + `CornerStyle` + `WindowStyle` | §6.1 / §6.2 / §6.6 | diff --git a/docs/specs/055-reactivity-and-representation-exploration.md b/docs/specs/055-reactivity-and-representation-exploration.md index 9452759d7..e8074ca4a 100644 --- a/docs/specs/055-reactivity-and-representation-exploration.md +++ b/docs/specs/055-reactivity-and-representation-exploration.md @@ -1860,8 +1860,8 @@ and accessibility remain design work. | `UseDisplays()` | Display-change controller exposes `Signal>`. | Display service signal/list. | | `UseWindowAspectRatio(double?)` | Controller applies/removes aspect lock on mount/update/unmount. | Imperative host/window effect with cleanup/update. | | `UseWindowDragMove()` | Instance method or service callback field. | Direct host/window action exposed by context/service. | -| `UseFilePickerAsync(options)` | Plain async service method; store result in a signal only if UI should react. | Direct injected service call; not reactive unless result is stored in a signal. | -| `UseFolderPickerAsync(options)` | Same as file picker. | Same as file picker. | +| `PickFileAsync(options)` | Plain async service method; store result in a signal only if UI should react. | Direct injected service call; not reactive unless result is stored in a signal. | +| `PickFolderAsync(options)` | Same as file picker. | Same as file picker. | | `UseWindowState()` | Window-state controller exposes `Signal`. | Window state signal. | | `UseIsActive()` | Activation controller exposes `Signal`. | Activation signal. | | `UseClosingGuard(canClose)` | Controller registers/unregisters the predicate. | Effect registering closing handler; cleanup unregisters. | diff --git a/docs/specs/tasks/054-windowing-evolution-implementation.md b/docs/specs/tasks/054-windowing-evolution-implementation.md index ab78d8b26..1eba3ed89 100644 --- a/docs/specs/tasks/054-windowing-evolution-implementation.md +++ b/docs/specs/tasks/054-windowing-evolution-implementation.md @@ -164,7 +164,7 @@ underestimating restore/save gating. | R9 | `PositionChanged` / `ZOrderChanged` fire eagerly during drag (`AppWindow.Changed` is high-rate). Spec §11 Q4: fire eagerly, don't coalesce; but hooks (`UseWindowPosition`) must short-circuit re-render when the DIP value is unchanged after conversion/rounding to avoid spamming components. | Tasks 1.1, 1.2 | | R10 | **Persistence schema migration is not needed for enum removal** (codec doesn't store the enum) — but if any external surface serializes `WindowSpec` (devtools `windows.open` arguments, future config files), the removed numeric enum value would deserialize as garbage. Audit + reject legacy numeric `4` (= old `RestoreFromPersistence`) at deserialization with a clear migration message. | Task 11.2 | | R11 | `WM_DISPLAYCHANGE` listener for `ReactorDisplay.DisplayLayoutChanged` is a singleton riding the first opened window's `WindowMessageMonitor`. Must ref-count: closing the registering window without others open must re-host on the next open, not silently lose the event. | Task 1.3 | -| R12 | `UseFilePickerAsync` / `UseFolderPickerAsync` call `IInitializeWithWindow.Initialize` with `UseWindow().NativeWindow.GetWindowHandle()` on the UI thread. Must not accept arbitrary HWNDs from user code; must throw if called off the UI thread. (Same-process trust model: there is no untrusted code in Reactor today, but this keeps the contract tight.) | Task 7.3 | +| R12 | The picker methods (`PickFileAsync` etc.; originally `UseFilePickerAsync` / `UseFolderPickerAsync` over `IInitializeWithWindow`) construct `Microsoft.Windows.Storage.Pickers` pickers from the owning window's `AppWindow.Id`, captured on the `RenderContext` during render, on the UI thread. Must not accept arbitrary HWNDs or window ids from user code; must throw if called off the UI thread or after the owner closed. (Same-process trust model: there is no untrusted code in Reactor today, but this keeps the contract tight.) | Task 7.3 | | R13 | Trim / AOT: new typed event-args (`WindowDipPositionChangedEventArgs`, `WindowZOrderChangedEventArgs`, `DisplayInfo`) and new enums must have **no reflection paths**. Records + plain enums are fine; verify no `JsonSerializer.Serialize(args)` slips into the new code. | Tasks 1.1, 1.2, 1.3 | | R14 | Hot reload: changing element / `WindowSpec` field types (e.g. `ExtendsContentIntoTitleBar` `bool → bool?`) across an HR boundary is not supported by `ReactorHotReloadCopier`. Document the limit; no code change. | Task 7.1 | | R15 | Devtools `WindowRegistry.Snapshot()` exposes title / hwnd / bounds / DPI / state today; renamed/removed fields must not break the snapshot JSON shape, and the new live values (Position, Level, Style) should be added when useful so the inspector stays in sync. | Task 11.2 | @@ -762,6 +762,29 @@ Purely additive. flags it). If scoping requires, lift this section out into a follow-up spec; otherwise land here. +### 7.3a Picker migration to `Microsoft.Windows.Storage.Pickers` (issue #1374) + +- [x] Rebuild the helpers on `Microsoft.Windows.Storage.Pickers` (`WindowId` + constructor; no `InitializeWithWindow`). Results become path-based + `PickFileResult` / `PickFolderResult`. +- [x] Rename to non-hook names `PickFileAsync`, `PickMultipleFilesAsync`, + `PickFolderAsync`, `PickSaveFileAsync`, and add `PickMultipleFoldersAsync`, + so direct calls from handlers no longer raise REACTOR_HOOKS_001/005. + Analyzer regression tests cover `Component` and `RenderContext` call + shapes, with a `Use*`-named positive control. +- [x] Options: add `Title`, `SuggestedFolder`, `SuggestedStartFolder`, + `InitialFileTypeIndex`, open-picker `FileTypeChoices`, and save-only + `DefaultFileExtension` / `ShowOverwritePrompt`. Make `FileTypeChoices` an + ordered `IReadOnlyList`, validated before the owner lookup. + Remove `FolderPickerOptions.FileTypeFilter` and + `SaveFilePickerOptions.SuggestedSaveFile`. +- [x] Capture the owning window on `RenderContext` during render instead of + reading `ReactorApp.ActiveHostInternal` at call time; reject a closed owner. +- [x] Selftest `Pickers_OwningWindow`: with A then B open, A's helpers route to + A before and after B closes; B's helpers are rejected after B closes. + Mutation-checked against the call-time lookup. +- [x] Selftest `Pickers_Suite`: native option read-back, including + insertion-ordered `FileTypeChoices` on open and save pickers. ### 7.4 Phase 7 exit gate - [x] Selftests registered in both places. @@ -778,7 +801,7 @@ Purely additive. PositionChanged, ZOrderChanged, CenterOnCurrent, WithPersistence + SavePlacement, ShowInTaskbar / ShowInSwitcher split, WindowStyle, CornerStyle, WindowLevel, SizeToContent, TaskbarItem facade + - Description, ReactorDisplay + UseDisplays, UseFilePickerAsync. + Description, ReactorDisplay + UseDisplays, pickers (`PickFileAsync` etc.). - [x] Each section: short prose + minimal runnable snippet + xmldoc-style caveats. - [x] Update the existing "Tips" + "Next Steps" sections to reference diff --git a/plugins/reactor/skills/reactor-dsl/references/reactor.api.txt b/plugins/reactor/skills/reactor-dsl/references/reactor.api.txt index dd541eae9..7b2ca8ec2 100644 --- a/plugins/reactor/skills/reactor-dsl/references/reactor.api.txt +++ b/plugins/reactor/skills/reactor-dsl/references/reactor.api.txt @@ -909,10 +909,8 @@ RenderContext.UseEffect(Func effectWithCleanup, T1 d1) → void RenderContext.UseElementFocus(FocusState state = FocusState.Programmatic) → ValueTuple RenderContext.UseElementRef() → ElementRef RenderContext.UseExternalStore(Func subscribe, Func getSnapshot, IEqualityComparer comparer = null) → TSnapshot -RenderContext.UseFilePickerAsync(FilePickerOptions options) → Task RenderContext.UseFocus() → FocusManager RenderContext.UseFocusTrap(bool isActive) → FocusTrapHandle -RenderContext.UseFolderPickerAsync(FolderPickerOptions options) → Task RenderContext.UseHighContrast() → bool RenderContext.UseHighContrastScheme() → string RenderContext.UseInfiniteResource(Func>> fetchPage, QueryCache cache, object[] deps, InfiniteResourceOptions options = null, IHookDispatcher dispatcher = null, Func cursorFromPageIndex = null) → InfiniteResource @@ -928,7 +926,6 @@ RenderContext.UseMemo(Func factory, object[] dependencies) → T RenderContext.UseMemoCells(IReadOnlyList items, Func builder, object[] dependencies) → Element[] RenderContext.UseMemoCellsByIndex(IReadOnlyList items, IReadOnlyList changedIndices, Func builder, object[] dependencies) → Element[] RenderContext.UseMemoCellsByKey(IReadOnlyList items, Func keySelector, Func builder, object[] dependencies) → Element[] -RenderContext.UseMultipleFilesPickerAsync(FilePickerOptions options) → Task> RenderContext.UseMutation(Func> mutator, MutationOptions options = null, IHookDispatcher dispatcher = null) → Mutation RenderContext.UseMutation(Func> mutator, QueryCache cache, MutationOptions options = null, IHookDispatcher dispatcher = null) → Mutation RenderContext.UseNavigation() → NavigationHandle @@ -946,7 +943,6 @@ RenderContext.UseReducer(Func reducer, RenderContext.UseRef(T initialValue = default) → Ref RenderContext.UseResource(Func> fetcher, QueryCache cache, object[] deps, ResourceOptions options = null, IHookDispatcher dispatcher = null) → AsyncValue RenderContext.UseResource(Func> fetcher, object[] deps, ResourceOptions options = null, IHookDispatcher dispatcher = null) → AsyncValue -RenderContext.UseSaveFilePickerAsync(SaveFilePickerOptions options) → Task RenderContext.UseState(T initialValue, bool threadSafe = false) → ValueTuple> RenderContext.UseSystemBackButton(NavigationHandle nav, Window window) → void RenderContext.UseTrayIcon(TrayIconSpec spec) → ReactorTrayIcon @@ -2547,17 +2543,32 @@ ToString() → string ### record FilePickerOptions new(IReadOnlyList FileTypeFilter = null, PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, string CommitButtonText = "") +IReadOnlyList FileTypeChoices { get; init; } IReadOnlyList FileTypeFilter { get; init; } PickerLocationId SuggestedStartLocation { get; init; } PickerViewMode? ViewMode { get; init; } +int? InitialFileTypeIndex { get; init; } string CommitButtonText { get; init; } string SettingsIdentifier { get; init; } +string SuggestedFolder { get; init; } +string SuggestedStartFolder { get; init; } +string Title { get; init; } Deconstruct(ref IReadOnlyList FileTypeFilter, ref PickerLocationId SuggestedStartLocation, ref string CommitButtonText) → void Equals(FilePickerOptions other) → bool Equals(object obj) → bool GetHashCode() → int ToString() → string +### record FileTypeChoice +new(string Label, IReadOnlyList Extensions) +IReadOnlyList Extensions { get; init; } +string Label { get; init; } +Deconstruct(ref string Label, ref IReadOnlyList Extensions) → void +Equals(FileTypeChoice other) → bool +Equals(object obj) → bool +GetHashCode() → int +ToString() → string + ### record FilterDescriptor new(string Field, FilterOperator Operator, object Value = null, object ValueTo = null) FilterOperator Operator { get; init; } @@ -2716,11 +2727,13 @@ bool IsActive { get; } ### record FolderPickerOptions new(PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, string CommitButtonText = "") -IReadOnlyList FileTypeFilter { get; init; } PickerLocationId SuggestedStartLocation { get; init; } PickerViewMode? ViewMode { get; init; } string CommitButtonText { get; init; } string SettingsIdentifier { get; init; } +string SuggestedFolder { get; init; } +string SuggestedStartFolder { get; init; } +string Title { get; init; } Deconstruct(ref PickerLocationId SuggestedStartLocation, ref string CommitButtonText) → void Equals(FolderPickerOptions other) → bool Equals(object obj) → bool @@ -4873,6 +4886,11 @@ ToString() → string ### class RenderContext new() +PickFileAsync(FilePickerOptions options) → Task +PickFolderAsync(FolderPickerOptions options) → Task +PickMultipleFilesAsync(FilePickerOptions options) → Task> +PickMultipleFoldersAsync(FolderPickerOptions options) → Task> +PickSaveFileAsync(SaveFilePickerOptions options) → Task UseBreakpoint(Window window, double minWidth) → bool UseBreakpoint(double minWidth) → bool UseCallback(Action callback, object[] dependencies) → Action @@ -4896,8 +4914,6 @@ UseEffect(Func effectWithCleanup, T1 d1, T2 d2) → void UseEffect(Action effect, T1 d1) → void UseEffect(Func effectWithCleanup, T1 d1) → void UseExternalStore(Func subscribe, Func getSnapshot, IEqualityComparer comparer = null) → TSnapshot -UseFilePickerAsync(FilePickerOptions options) → Task -UseFolderPickerAsync(FolderPickerOptions options) → Task UseHighContrast() → bool UseHighContrastScheme() → string UseIntl() → IntlAccessor @@ -4908,7 +4924,6 @@ UseMemo(Func factory, T1 d1, T2 d2, T3 d3) → T UseMemo(Func factory, T1 d1, T2 d2) → T UseMemo(Func factory, T1 d1) → T UseMemo(Func factory, object[] dependencies) → T -UseMultipleFilesPickerAsync(FilePickerOptions options) → Task> UseNavigation() → NavigationHandle UseNavigation(TRoute initial) → NavigationHandle UseNavigationLifecycle(Action onNavigatingTo = null, Action onNavigatedTo = null, Action onNavigatingFrom = null, Action onNavigatedFrom = null) → void @@ -4922,7 +4937,6 @@ UseReducedMotion() → bool UseReducer(T initialValue, bool threadSafe = false) → ValueTuple>> UseReducer(Func reducer, TState initialValue, bool threadSafe = false) → ValueTuple> UseRef(T initialValue = default) → Ref -UseSaveFilePickerAsync(SaveFilePickerOptions options) → Task UseState(T initialValue, bool threadSafe = false) → ValueTuple> UseSystemBackButton(NavigationHandle nav, Window window) → void UseTrayIcon(TrayIconSpec spec) → ReactorTrayIcon @@ -5193,14 +5207,19 @@ ToString() → string IsRtlLocale(string locale) → bool ### record SaveFilePickerOptions -new(IReadOnlyDictionary> FileTypeChoices, string SuggestedFileName = "", PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, string CommitButtonText = "") -IReadOnlyDictionary> FileTypeChoices { get; init; } +new(IReadOnlyList FileTypeChoices, string SuggestedFileName = "", PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, string CommitButtonText = "") +IReadOnlyList FileTypeChoices { get; init; } PickerLocationId SuggestedStartLocation { get; init; } -StorageFile SuggestedSaveFile { get; init; } +bool ShowOverwritePrompt { get; init; } +int? InitialFileTypeIndex { get; init; } string CommitButtonText { get; init; } +string DefaultFileExtension { get; init; } string SettingsIdentifier { get; init; } string SuggestedFileName { get; init; } -Deconstruct(ref IReadOnlyDictionary> FileTypeChoices, ref string SuggestedFileName, ref PickerLocationId SuggestedStartLocation, ref string CommitButtonText) → void +string SuggestedFolder { get; init; } +string SuggestedStartFolder { get; init; } +string Title { get; init; } +Deconstruct(ref IReadOnlyList FileTypeChoices, ref string SuggestedFileName, ref PickerLocationId SuggestedStartLocation, ref string CommitButtonText) → void Equals(SaveFilePickerOptions other) → bool Equals(object obj) → bool GetHashCode() → int diff --git a/plugins/reactor/skills/reactor-windowing/SKILL.md b/plugins/reactor/skills/reactor-windowing/SKILL.md index 4f71caad0..e30108e7b 100644 --- a/plugins/reactor/skills/reactor-windowing/SKILL.md +++ b/plugins/reactor/skills/reactor-windowing/SKILL.md @@ -1,6 +1,6 @@ --- name: reactor-windowing -description: "Reactor top-level windowing cookbook: WindowSpec, OpenWindow, draggable windows, borderless/tool windows, placement persistence, taskbar visibility, z-order, aspect ratio, SizeToContent, displays, taskbar integration, and picker HWND wiring." +description: "Reactor top-level windowing cookbook: WindowSpec, OpenWindow, draggable windows, borderless/tool windows, placement persistence, taskbar visibility, z-order, aspect ratio, SizeToContent, displays, taskbar integration, and owner-window file/folder pickers." when_to_invoke: - "open a window" - "secondary window" @@ -257,22 +257,25 @@ taskbar.Progress.Value = 0.25; var displays = UseDisplays(); var nearest = ReactorDisplay.NearestTo(window.Position.X, window.Position.Y); -// Capture the helpers during Render; invoke them only from UI event handlers. -var pickFile = UseFilePickerAsync; -var pickFiles = UseMultipleFilesPickerAsync; -var pickFolder = UseFolderPickerAsync; -var saveFile = UseSaveFilePickerAsync; +// Pickers are not hooks: call them directly from UI-thread event handlers, never during Render. +// using Microsoft.Windows.Storage.Pickers; (not Windows.Storage.Pickers: both define PickerLocationId) return VStack( - Button("Open", async () => { var file = await pickFile(new(FileTypeFilter: [".txt"])); }), - Button("Open multiple", async () => { var files = await pickFiles(new(FileTypeFilter: [".txt"])); }), - Button("Folder", async () => { var folder = await pickFolder(new()); }), - Button("Save", async () => { var file = await saveFile(new( - new Dictionary> { ["Text"] = [".txt"] }, - SuggestedFileName: "report")); })); + Button("Open", async () => { var file = await PickFileAsync(new(FileTypeFilter: [".txt"])); if (file is null) return; var text = await File.ReadAllTextAsync(file.Path); }), + Button("Open multiple", async () => { var files = await PickMultipleFilesAsync(new() { FileTypeChoices = [new("Notes", [".txt", ".md"])] }); }), + Button("Folder", async () => { var folder = await PickFolderAsync(new()); }), + Button("Folders", async () => { var folders = await PickMultipleFoldersAsync(new()); }), + Button("Save", async () => { var target = await PickSaveFileAsync(new( + [new("Text", [".txt"]), new("Markdown", [".md"])], SuggestedFileName: "report")); + if (target is not null) await File.WriteAllTextAsync(target.Path, "body"); })); ``` -Picker helpers must be invoked from UI-thread event handlers, not during Render, and use the owning HWND; -there is no arbitrary HWND parameter. +`PickFileAsync` / `PickMultipleFilesAsync` / `PickFolderAsync` / `PickMultipleFoldersAsync` / `PickSaveFileAsync` +return `PickFileResult` / `PickFolderResult` (`Microsoft.Windows.Storage.Pickers`, exposing `.Path`); cancellation +returns `null` or an empty list. The owner is the window that last rendered the component (not whichever window is +active), the dialog works in elevated apps, and the call throws `InvalidOperationException` if that window has +closed. `FileTypeChoices` is an ordered `IReadOnlyList` whose order sets the default entry; labels +must be unique. Options also take `Title`, `SuggestedFolder`, `SuggestedStartFolder`, `InitialFileTypeIndex`, and +(save) `DefaultFileExtension` / `ShowOverwritePrompt`. There is no arbitrary HWND parameter. ## Render errors diff --git a/samples/ReactorGallery/GAPS.md b/samples/ReactorGallery/GAPS.md index e9ad01b70..953b44516 100644 --- a/samples/ReactorGallery/GAPS.md +++ b/samples/ReactorGallery/GAPS.md @@ -32,7 +32,7 @@ This document lists WinUI Gallery controls, features, and XAML concepts that **c ### FilePicker / FolderPicker **WinUI:** System file/folder picker dialogs. -**Reactor Alternative:** **Picker helpers.** Call `UseFilePickerAsync`, `UseMultipleFilesPickerAsync`, `UseFolderPickerAsync`, or `UseSaveFilePickerAsync` from UI-thread event handlers. Reactor initializes the native `Windows.Storage.Pickers` dialog with the owning window HWND. These are imperative helpers, not elements in the UI tree. See [Pickers](../../docs/guide/windows.md#pickers) for options and cancellation behavior. +**Reactor Alternative:** **Picker methods.** Call `PickFileAsync`, `PickMultipleFilesAsync`, `PickFolderAsync`, `PickMultipleFoldersAsync`, or `PickSaveFileAsync` from UI-thread event handlers. Reactor opens the native `Microsoft.Windows.Storage.Pickers` dialog owned by the component's window and returns path-based results. These are imperative helpers, not elements in the UI tree. See [Pickers](../../docs/guide/windows.md#pickers) for options and cancellation behavior. --- diff --git a/skills/reactor.api.txt b/skills/reactor.api.txt index dd541eae9..7b2ca8ec2 100644 --- a/skills/reactor.api.txt +++ b/skills/reactor.api.txt @@ -909,10 +909,8 @@ RenderContext.UseEffect(Func effectWithCleanup, T1 d1) → void RenderContext.UseElementFocus(FocusState state = FocusState.Programmatic) → ValueTuple RenderContext.UseElementRef() → ElementRef RenderContext.UseExternalStore(Func subscribe, Func getSnapshot, IEqualityComparer comparer = null) → TSnapshot -RenderContext.UseFilePickerAsync(FilePickerOptions options) → Task RenderContext.UseFocus() → FocusManager RenderContext.UseFocusTrap(bool isActive) → FocusTrapHandle -RenderContext.UseFolderPickerAsync(FolderPickerOptions options) → Task RenderContext.UseHighContrast() → bool RenderContext.UseHighContrastScheme() → string RenderContext.UseInfiniteResource(Func>> fetchPage, QueryCache cache, object[] deps, InfiniteResourceOptions options = null, IHookDispatcher dispatcher = null, Func cursorFromPageIndex = null) → InfiniteResource @@ -928,7 +926,6 @@ RenderContext.UseMemo(Func factory, object[] dependencies) → T RenderContext.UseMemoCells(IReadOnlyList items, Func builder, object[] dependencies) → Element[] RenderContext.UseMemoCellsByIndex(IReadOnlyList items, IReadOnlyList changedIndices, Func builder, object[] dependencies) → Element[] RenderContext.UseMemoCellsByKey(IReadOnlyList items, Func keySelector, Func builder, object[] dependencies) → Element[] -RenderContext.UseMultipleFilesPickerAsync(FilePickerOptions options) → Task> RenderContext.UseMutation(Func> mutator, MutationOptions options = null, IHookDispatcher dispatcher = null) → Mutation RenderContext.UseMutation(Func> mutator, QueryCache cache, MutationOptions options = null, IHookDispatcher dispatcher = null) → Mutation RenderContext.UseNavigation() → NavigationHandle @@ -946,7 +943,6 @@ RenderContext.UseReducer(Func reducer, RenderContext.UseRef(T initialValue = default) → Ref RenderContext.UseResource(Func> fetcher, QueryCache cache, object[] deps, ResourceOptions options = null, IHookDispatcher dispatcher = null) → AsyncValue RenderContext.UseResource(Func> fetcher, object[] deps, ResourceOptions options = null, IHookDispatcher dispatcher = null) → AsyncValue -RenderContext.UseSaveFilePickerAsync(SaveFilePickerOptions options) → Task RenderContext.UseState(T initialValue, bool threadSafe = false) → ValueTuple> RenderContext.UseSystemBackButton(NavigationHandle nav, Window window) → void RenderContext.UseTrayIcon(TrayIconSpec spec) → ReactorTrayIcon @@ -2547,17 +2543,32 @@ ToString() → string ### record FilePickerOptions new(IReadOnlyList FileTypeFilter = null, PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, string CommitButtonText = "") +IReadOnlyList FileTypeChoices { get; init; } IReadOnlyList FileTypeFilter { get; init; } PickerLocationId SuggestedStartLocation { get; init; } PickerViewMode? ViewMode { get; init; } +int? InitialFileTypeIndex { get; init; } string CommitButtonText { get; init; } string SettingsIdentifier { get; init; } +string SuggestedFolder { get; init; } +string SuggestedStartFolder { get; init; } +string Title { get; init; } Deconstruct(ref IReadOnlyList FileTypeFilter, ref PickerLocationId SuggestedStartLocation, ref string CommitButtonText) → void Equals(FilePickerOptions other) → bool Equals(object obj) → bool GetHashCode() → int ToString() → string +### record FileTypeChoice +new(string Label, IReadOnlyList Extensions) +IReadOnlyList Extensions { get; init; } +string Label { get; init; } +Deconstruct(ref string Label, ref IReadOnlyList Extensions) → void +Equals(FileTypeChoice other) → bool +Equals(object obj) → bool +GetHashCode() → int +ToString() → string + ### record FilterDescriptor new(string Field, FilterOperator Operator, object Value = null, object ValueTo = null) FilterOperator Operator { get; init; } @@ -2716,11 +2727,13 @@ bool IsActive { get; } ### record FolderPickerOptions new(PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, string CommitButtonText = "") -IReadOnlyList FileTypeFilter { get; init; } PickerLocationId SuggestedStartLocation { get; init; } PickerViewMode? ViewMode { get; init; } string CommitButtonText { get; init; } string SettingsIdentifier { get; init; } +string SuggestedFolder { get; init; } +string SuggestedStartFolder { get; init; } +string Title { get; init; } Deconstruct(ref PickerLocationId SuggestedStartLocation, ref string CommitButtonText) → void Equals(FolderPickerOptions other) → bool Equals(object obj) → bool @@ -4873,6 +4886,11 @@ ToString() → string ### class RenderContext new() +PickFileAsync(FilePickerOptions options) → Task +PickFolderAsync(FolderPickerOptions options) → Task +PickMultipleFilesAsync(FilePickerOptions options) → Task> +PickMultipleFoldersAsync(FolderPickerOptions options) → Task> +PickSaveFileAsync(SaveFilePickerOptions options) → Task UseBreakpoint(Window window, double minWidth) → bool UseBreakpoint(double minWidth) → bool UseCallback(Action callback, object[] dependencies) → Action @@ -4896,8 +4914,6 @@ UseEffect(Func effectWithCleanup, T1 d1, T2 d2) → void UseEffect(Action effect, T1 d1) → void UseEffect(Func effectWithCleanup, T1 d1) → void UseExternalStore(Func subscribe, Func getSnapshot, IEqualityComparer comparer = null) → TSnapshot -UseFilePickerAsync(FilePickerOptions options) → Task -UseFolderPickerAsync(FolderPickerOptions options) → Task UseHighContrast() → bool UseHighContrastScheme() → string UseIntl() → IntlAccessor @@ -4908,7 +4924,6 @@ UseMemo(Func factory, T1 d1, T2 d2, T3 d3) → T UseMemo(Func factory, T1 d1, T2 d2) → T UseMemo(Func factory, T1 d1) → T UseMemo(Func factory, object[] dependencies) → T -UseMultipleFilesPickerAsync(FilePickerOptions options) → Task> UseNavigation() → NavigationHandle UseNavigation(TRoute initial) → NavigationHandle UseNavigationLifecycle(Action onNavigatingTo = null, Action onNavigatedTo = null, Action onNavigatingFrom = null, Action onNavigatedFrom = null) → void @@ -4922,7 +4937,6 @@ UseReducedMotion() → bool UseReducer(T initialValue, bool threadSafe = false) → ValueTuple>> UseReducer(Func reducer, TState initialValue, bool threadSafe = false) → ValueTuple> UseRef(T initialValue = default) → Ref -UseSaveFilePickerAsync(SaveFilePickerOptions options) → Task UseState(T initialValue, bool threadSafe = false) → ValueTuple> UseSystemBackButton(NavigationHandle nav, Window window) → void UseTrayIcon(TrayIconSpec spec) → ReactorTrayIcon @@ -5193,14 +5207,19 @@ ToString() → string IsRtlLocale(string locale) → bool ### record SaveFilePickerOptions -new(IReadOnlyDictionary> FileTypeChoices, string SuggestedFileName = "", PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, string CommitButtonText = "") -IReadOnlyDictionary> FileTypeChoices { get; init; } +new(IReadOnlyList FileTypeChoices, string SuggestedFileName = "", PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, string CommitButtonText = "") +IReadOnlyList FileTypeChoices { get; init; } PickerLocationId SuggestedStartLocation { get; init; } -StorageFile SuggestedSaveFile { get; init; } +bool ShowOverwritePrompt { get; init; } +int? InitialFileTypeIndex { get; init; } string CommitButtonText { get; init; } +string DefaultFileExtension { get; init; } string SettingsIdentifier { get; init; } string SuggestedFileName { get; init; } -Deconstruct(ref IReadOnlyDictionary> FileTypeChoices, ref string SuggestedFileName, ref PickerLocationId SuggestedStartLocation, ref string CommitButtonText) → void +string SuggestedFolder { get; init; } +string SuggestedStartFolder { get; init; } +string Title { get; init; } +Deconstruct(ref IReadOnlyList FileTypeChoices, ref string SuggestedFileName, ref PickerLocationId SuggestedStartLocation, ref string CommitButtonText) → void Equals(SaveFilePickerOptions other) → bool Equals(object obj) → bool GetHashCode() → int diff --git a/skills/windowing.md b/skills/windowing.md index a448e356b..f6c8ce9f6 100644 --- a/skills/windowing.md +++ b/skills/windowing.md @@ -3,7 +3,7 @@ name: reactor-windowing description: > Reactor windowing cookbook: WindowSpec, OpenWindow, placement persistence, draggable/borderless windows, taskbar visibility, z-order, aspect ratio, - SizeToContent, displays, taskbar integration, and picker HWND wiring. + SizeToContent, displays, taskbar integration, and owner-window file/folder pickers. --- # Reactor Windowing @@ -235,22 +235,24 @@ var nearest = ReactorDisplay.NearestTo(window.Position.X, window.Position.Y); ## Pickers ```csharp -// Capture during Render; invoke from UI event handlers, not during Render. -var pickFile = UseFilePickerAsync; -var pickFiles = UseMultipleFilesPickerAsync; -var pickFolder = UseFolderPickerAsync; -var saveFile = UseSaveFilePickerAsync; +// Pickers are not hooks: call them directly from UI-thread event handlers, never during Render. +// using Microsoft.Windows.Storage.Pickers; (not Windows.Storage.Pickers: both define PickerLocationId) return VStack( - Button("Open", async () => { var file = await pickFile(new(FileTypeFilter: [".txt", ".md"])); }), - Button("Open multiple", async () => { var files = await pickFiles(new(FileTypeFilter: [".txt", ".md"])); }), - Button("Folder", async () => { var folder = await pickFolder(new()); }), - Button("Save", async () => { var file = await saveFile(new( - new Dictionary> { ["Text"] = [".txt"] }, - SuggestedFileName: "report")); })); + Button("Open", async () => { var file = await PickFileAsync(new(FileTypeFilter: [".txt", ".md"])); if (file is null) return; var text = await File.ReadAllTextAsync(file.Path); }), + Button("Open multiple", async () => { var files = await PickMultipleFilesAsync(new() { FileTypeChoices = [new("Notes", [".txt", ".md"])] }); }), + Button("Folder", async () => { var folder = await PickFolderAsync(new()); }), + Button("Folders", async () => { var folders = await PickMultipleFoldersAsync(new()); }), + Button("Save", async () => { var target = await PickSaveFileAsync(new( + [new("Text", [".txt"]), new("Markdown", [".md"])], SuggestedFileName: "report")); + if (target is not null) await File.WriteAllTextAsync(target.Path, "body"); })); ``` -Picker hooks run on the owning window's UI thread and automatically initialize -with that window's HWND. Do not pass arbitrary HWNDs. +Results are `PickFileResult` / `PickFolderResult` (`Microsoft.Windows.Storage.Pickers`) exposing `.Path`; +cancellation returns `null` or an empty list. The dialog is owned by the window that last rendered the +component, works in elevated apps, and throws `InvalidOperationException` if that window has closed. +`FileTypeChoices` is an ordered `IReadOnlyList`: order sets the default entry, labels must be +unique. Options also take `Title`, `SuggestedFolder`, `SuggestedStartFolder`, `InitialFileTypeIndex`, and (save) +`DefaultFileExtension` / `ShowOverwritePrompt`. Do not pass arbitrary HWNDs. ## Recipe: Command Palette diff --git a/src/Reactor/Core/Component.cs b/src/Reactor/Core/Component.cs index 62677be2a..5849b59fd 100644 --- a/src/Reactor/Core/Component.cs +++ b/src/Reactor/Core/Component.cs @@ -162,17 +162,25 @@ protected bool UseIsActive() protected void UseClosingGuard(Func canClose) => Context.UseClosingGuard(canClose); - protected Task UseFilePickerAsync(FilePickerOptions options) - => Context.UseFilePickerAsync(options); + /// Opens a single-file picker owned by this component's window. Not a hook: call from a UI-thread event handler. + protected Task PickFileAsync(FilePickerOptions options) + => Context.PickFileAsync(options); - protected Task> UseMultipleFilesPickerAsync(FilePickerOptions options) - => Context.UseMultipleFilesPickerAsync(options); + /// Opens a multiple-file picker owned by this component's window. Not a hook: call from a UI-thread event handler. + protected Task> PickMultipleFilesAsync(FilePickerOptions options) + => Context.PickMultipleFilesAsync(options); - protected Task UseFolderPickerAsync(FolderPickerOptions options) - => Context.UseFolderPickerAsync(options); + /// Opens a single-folder picker owned by this component's window. Not a hook: call from a UI-thread event handler. + protected Task PickFolderAsync(FolderPickerOptions options) + => Context.PickFolderAsync(options); - protected Task UseSaveFilePickerAsync(SaveFilePickerOptions options) - => Context.UseSaveFilePickerAsync(options); + /// Opens a multiple-folder picker owned by this component's window. Not a hook: call from a UI-thread event handler. + protected Task> PickMultipleFoldersAsync(FolderPickerOptions options) + => Context.PickMultipleFoldersAsync(options); + + /// Opens a save-file picker owned by this component's window. Not a hook: call from a UI-thread event handler. + protected Task PickSaveFileAsync(SaveFilePickerOptions options) + => Context.PickSaveFileAsync(options); /// Subscribes to the host window's position-changed event and re-renders on move. (spec 054 §5.5) protected (double X, double Y) UseWindowPosition() diff --git a/src/Reactor/Core/PickerHooks.cs b/src/Reactor/Core/PickerHooks.cs deleted file mode 100644 index e7f0550cd..000000000 --- a/src/Reactor/Core/PickerHooks.cs +++ /dev/null @@ -1,130 +0,0 @@ -using Windows.Storage; -using Windows.Storage.Pickers; - -namespace Microsoft.UI.Reactor.Core; - -/// Options for single-file and multiple-file open pickers. -public sealed record FilePickerOptions( - IReadOnlyList? FileTypeFilter = null, - PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, - string CommitButtonText = "") -{ - /// The native view mode; null preserves the platform default. - public PickerViewMode? ViewMode { get; init; } - - /// Identifies the picker state remembered by Windows. - public string SettingsIdentifier { get; init; } = ""; -} - -/// Options for . -public sealed record FolderPickerOptions( - PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, - string CommitButtonText = "") -{ - /// Displayed file types; null or empty lists default to all types. - public IReadOnlyList? FileTypeFilter { get; init; } - - /// The native view mode; null preserves the platform default. - public PickerViewMode? ViewMode { get; init; } - - /// Identifies the picker state remembered by Windows. - public string SettingsIdentifier { get; init; } = ""; -} - -/// Options for . -/// Display labels mapped to allowed extensions, such as ".txt". At least one choice is required. -/// The name suggested to the user, without requiring an extension. -/// The initial location suggested to Windows. -/// The commit button label; empty uses the platform default. -public sealed record SaveFilePickerOptions( - IReadOnlyDictionary> FileTypeChoices, - string SuggestedFileName = "", - PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, - string CommitButtonText = "") -{ - /// Identifies the picker state remembered by Windows. - public string SettingsIdentifier { get; init; } = ""; - - /// An existing file suggested as the save target. - public StorageFile? SuggestedSaveFile { get; init; } -} - -internal interface IPickerService -{ - Task PickFileAsync(nint hwnd, FilePickerOptions options); - Task> PickMultipleFilesAsync(nint hwnd, FilePickerOptions options); - Task PickFolderAsync(nint hwnd, FolderPickerOptions options); - Task PickSaveFileAsync(nint hwnd, SaveFilePickerOptions options); -} - -internal sealed class DefaultPickerService : IPickerService -{ - public async Task PickFileAsync(nint hwnd, FilePickerOptions options) - => await CreateFileOpenPicker(hwnd, options).PickSingleFileAsync(); - - public async Task> PickMultipleFilesAsync(nint hwnd, FilePickerOptions options) - // Preserve the WinRT projection instead of materializing another collection of - // StorageFile objects, which would require extra ABI generation in AOT apps. - => await CreateFileOpenPicker(hwnd, options).PickMultipleFilesAsync(); - - public async Task PickFolderAsync(nint hwnd, FolderPickerOptions options) - => await CreateFolderPicker(hwnd, options).PickSingleFolderAsync(); - - public async Task PickSaveFileAsync(nint hwnd, SaveFilePickerOptions options) - => await CreateFileSavePicker(hwnd, options).PickSaveFileAsync(); - - internal static FileOpenPicker CreateFileOpenPicker(nint hwnd, FilePickerOptions options) - { - var picker = new FileOpenPicker - { - SuggestedStartLocation = options.SuggestedStartLocation, - CommitButtonText = options.CommitButtonText ?? string.Empty, - SettingsIdentifier = options.SettingsIdentifier ?? string.Empty, - }; - if (options.ViewMode is { } viewMode) - picker.ViewMode = viewMode; - var filters = options.FileTypeFilter is { Count: > 0 } ? options.FileTypeFilter : ["*"]; - foreach (var filter in filters) - picker.FileTypeFilter.Add(filter); - WinRT.Interop.InitializeWithWindow.Initialize(picker, hwnd); - return picker; - } - - internal static FolderPicker CreateFolderPicker(nint hwnd, FolderPickerOptions options) - { - var picker = new FolderPicker - { - SuggestedStartLocation = options.SuggestedStartLocation, - CommitButtonText = options.CommitButtonText ?? string.Empty, - SettingsIdentifier = options.SettingsIdentifier ?? string.Empty, - }; - if (options.ViewMode is { } viewMode) - picker.ViewMode = viewMode; - var filters = options.FileTypeFilter is { Count: > 0 } ? options.FileTypeFilter : ["*"]; - foreach (var filter in filters) - picker.FileTypeFilter.Add(filter); - WinRT.Interop.InitializeWithWindow.Initialize(picker, hwnd); - return picker; - } - - internal static FileSavePicker CreateFileSavePicker(nint hwnd, SaveFilePickerOptions options) - { - ArgumentNullException.ThrowIfNull(options.FileTypeChoices); - if (options.FileTypeChoices.Count == 0) - throw new ArgumentException("At least one file type choice is required.", nameof(options)); - - var picker = new FileSavePicker - { - SuggestedStartLocation = options.SuggestedStartLocation, - SuggestedFileName = options.SuggestedFileName ?? string.Empty, - CommitButtonText = options.CommitButtonText ?? string.Empty, - SettingsIdentifier = options.SettingsIdentifier ?? string.Empty, - }; - foreach (var (label, extensions) in options.FileTypeChoices) - picker.FileTypeChoices.Add(label, new List(extensions)); - if (options.SuggestedSaveFile is { } saveFile) - picker.SuggestedSaveFile = saveFile; - WinRT.Interop.InitializeWithWindow.Initialize(picker, hwnd); - return picker; - } -} diff --git a/src/Reactor/Core/Pickers.cs b/src/Reactor/Core/Pickers.cs new file mode 100644 index 000000000..8322d5733 --- /dev/null +++ b/src/Reactor/Core/Pickers.cs @@ -0,0 +1,230 @@ +using Microsoft.UI; +using Microsoft.Windows.Storage.Pickers; + +namespace Microsoft.UI.Reactor.Core; + +/// A labelled group of file extensions offered by a file picker, such as ("Text", [".txt", ".md"]). +/// The display label shown in the picker's file-type list. +/// The extensions in the group, each including the leading dot. +public sealed record FileTypeChoice(string Label, IReadOnlyList Extensions); + +/// Options for and . +/// Extensions shown, such as ".txt"; null or empty shows all files. Ignored when is non-empty. +/// The initial location suggested to Windows. +/// The commit button label; empty uses the platform default. +public sealed record FilePickerOptions( + IReadOnlyList? FileTypeFilter = null, + PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, + string CommitButtonText = "") +{ + /// The native view mode; null preserves the platform default. + public PickerViewMode? ViewMode { get; init; } + + /// Identifies the picker state remembered by Windows. + public string SettingsIdentifier { get; init; } = ""; + + /// The dialog title; empty uses the platform default. + public string Title { get; init; } = ""; + + /// An absolute folder path that the dialog always opens in; empty lets Windows decide. + public string SuggestedFolder { get; init; } = ""; + + /// An absolute folder path used only until Windows remembers a location for this picker; empty lets Windows decide. + public string SuggestedStartFolder { get; init; } = ""; + + /// Ordered, labelled file-type groups. When non-empty they replace . + public IReadOnlyList? FileTypeChoices { get; init; } + + /// The zero-based file-type entry selected initially; null preserves the platform default. + public int? InitialFileTypeIndex { get; init; } +} + +/// Options for and . +/// The initial location suggested to Windows. +/// The commit button label; empty uses the platform default. +public sealed record FolderPickerOptions( + PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, + string CommitButtonText = "") +{ + /// The native view mode; null preserves the platform default. + public PickerViewMode? ViewMode { get; init; } + + /// Identifies the picker state remembered by Windows. + public string SettingsIdentifier { get; init; } = ""; + + /// The dialog title; empty uses the platform default. + public string Title { get; init; } = ""; + + /// An absolute folder path that the dialog always opens in; empty lets Windows decide. + public string SuggestedFolder { get; init; } = ""; + + /// An absolute folder path used only until Windows remembers a location for this picker; empty lets Windows decide. + public string SuggestedStartFolder { get; init; } = ""; +} + +/// Options for . +/// Ordered, labelled file-type groups. At least one choice is required. +/// The name suggested to the user, without requiring an extension. +/// The initial location suggested to Windows. +/// The commit button label; empty uses the platform default. +public sealed record SaveFilePickerOptions( + IReadOnlyList FileTypeChoices, + string SuggestedFileName = "", + PickerLocationId SuggestedStartLocation = PickerLocationId.DocumentsLibrary, + string CommitButtonText = "") +{ + /// Identifies the picker state remembered by Windows. + public string SettingsIdentifier { get; init; } = ""; + + /// The dialog title; empty uses the platform default. + public string Title { get; init; } = ""; + + /// An absolute folder path that the dialog always opens in; empty lets Windows decide. + public string SuggestedFolder { get; init; } = ""; + + /// An absolute folder path used only until Windows remembers a location for this picker; empty lets Windows decide. + public string SuggestedStartFolder { get; init; } = ""; + + /// The extension, such as ".txt", appended when the user types a name without one; empty uses the platform default. + public string DefaultFileExtension { get; init; } = ""; + + /// The zero-based file-type choice selected initially; null preserves the platform default. + public int? InitialFileTypeIndex { get; init; } + + /// Whether Windows asks before overwriting an existing file. Defaults to true. + public bool ShowOverwritePrompt { get; init; } = true; +} + +internal static class PickerOptionsValidation +{ + public static void Validate(FilePickerOptions options) + { + if (options.FileTypeChoices is { } choices) + ValidateChoices(choices); + } + + public static void Validate(SaveFilePickerOptions options) + { + ArgumentNullException.ThrowIfNull(options.FileTypeChoices, "options.FileTypeChoices"); + if (options.FileTypeChoices.Count == 0) + throw new ArgumentException("At least one file type choice is required.", nameof(options)); + ValidateChoices(options.FileTypeChoices); + } + + // The native map keys choices by label, so a duplicate would silently replace the + // earlier entry in place and change which group the dialog offers. + private static void ValidateChoices(IReadOnlyList choices) + { + var labels = new HashSet(StringComparer.Ordinal); + foreach (var choice in choices) + { + if (choice is null) + throw new ArgumentException("File type choices cannot contain null entries.", "options"); + if (string.IsNullOrWhiteSpace(choice.Label)) + throw new ArgumentException("Each file type choice requires a label.", "options"); + if (choice.Extensions is not { Count: > 0 }) + throw new ArgumentException($"File type choice '{choice.Label}' requires at least one extension.", "options"); + if (!labels.Add(choice.Label)) + throw new ArgumentException($"File type choice '{choice.Label}' is listed more than once.", "options"); + } + } +} + +internal interface IPickerService +{ + Task PickFileAsync(WindowId owner, FilePickerOptions options); + Task> PickMultipleFilesAsync(WindowId owner, FilePickerOptions options); + Task PickFolderAsync(WindowId owner, FolderPickerOptions options); + Task> PickMultipleFoldersAsync(WindowId owner, FolderPickerOptions options); + Task PickSaveFileAsync(WindowId owner, SaveFilePickerOptions options); +} + +internal sealed class DefaultPickerService : IPickerService +{ + public async Task PickFileAsync(WindowId owner, FilePickerOptions options) + => await CreateFileOpenPicker(owner, options).PickSingleFileAsync(); + + // Return the WinRT projection as-is instead of copying it into another collection, + // which would require extra ABI generation in AOT apps. + public async Task> PickMultipleFilesAsync(WindowId owner, FilePickerOptions options) + => await CreateFileOpenPicker(owner, options).PickMultipleFilesAsync() ?? []; + + public async Task PickFolderAsync(WindowId owner, FolderPickerOptions options) + => await CreateFolderPicker(owner, options).PickSingleFolderAsync(); + + public async Task> PickMultipleFoldersAsync(WindowId owner, FolderPickerOptions options) + => await CreateFolderPicker(owner, options).PickMultipleFoldersAsync() ?? []; + + public async Task PickSaveFileAsync(WindowId owner, SaveFilePickerOptions options) + => await CreateFileSavePicker(owner, options).PickSaveFileAsync(); + + internal static FileOpenPicker CreateFileOpenPicker(WindowId owner, FilePickerOptions options) + { + var picker = new FileOpenPicker(owner) + { + SuggestedStartLocation = options.SuggestedStartLocation, + CommitButtonText = options.CommitButtonText ?? string.Empty, + SettingsIdentifier = options.SettingsIdentifier ?? string.Empty, + Title = options.Title ?? string.Empty, + SuggestedFolder = options.SuggestedFolder ?? string.Empty, + SuggestedStartFolder = options.SuggestedStartFolder ?? string.Empty, + }; + if (options.ViewMode is { } viewMode) + picker.ViewMode = viewMode; + // An empty filter already means "All Files" in the native picker. + if (options.FileTypeFilter is { } filters) + { + foreach (var filter in filters) + picker.FileTypeFilter.Add(filter); + } + if (options.FileTypeChoices is { } choices) + AddChoices(picker.FileTypeChoices, choices); + if (options.InitialFileTypeIndex is { } index) + picker.InitialFileTypeIndex = index; + return picker; + } + + internal static FolderPicker CreateFolderPicker(WindowId owner, FolderPickerOptions options) + { + var picker = new FolderPicker(owner) + { + SuggestedStartLocation = options.SuggestedStartLocation, + CommitButtonText = options.CommitButtonText ?? string.Empty, + SettingsIdentifier = options.SettingsIdentifier ?? string.Empty, + Title = options.Title ?? string.Empty, + SuggestedFolder = options.SuggestedFolder ?? string.Empty, + SuggestedStartFolder = options.SuggestedStartFolder ?? string.Empty, + }; + if (options.ViewMode is { } viewMode) + picker.ViewMode = viewMode; + return picker; + } + + internal static FileSavePicker CreateFileSavePicker(WindowId owner, SaveFilePickerOptions options) + { + var picker = new FileSavePicker(owner) + { + SuggestedStartLocation = options.SuggestedStartLocation, + SuggestedFileName = options.SuggestedFileName ?? string.Empty, + CommitButtonText = options.CommitButtonText ?? string.Empty, + SettingsIdentifier = options.SettingsIdentifier ?? string.Empty, + Title = options.Title ?? string.Empty, + SuggestedFolder = options.SuggestedFolder ?? string.Empty, + SuggestedStartFolder = options.SuggestedStartFolder ?? string.Empty, + DefaultFileExtension = options.DefaultFileExtension ?? string.Empty, + ShowOverwritePrompt = options.ShowOverwritePrompt, + }; + AddChoices(picker.FileTypeChoices, options.FileTypeChoices); + if (options.InitialFileTypeIndex is { } index) + picker.InitialFileTypeIndex = index; + return picker; + } + + // The native map preserves insertion order, which sets the order of the dialog's + // file-type list and therefore its default entry. + private static void AddChoices(IDictionary> target, IReadOnlyList choices) + { + foreach (var choice in choices) + target.Add(choice.Label, new List(choice.Extensions)); + } +} \ No newline at end of file diff --git a/src/Reactor/Core/RenderContext.cs b/src/Reactor/Core/RenderContext.cs index 49722eb00..d34ba04c6 100644 --- a/src/Reactor/Core/RenderContext.cs +++ b/src/Reactor/Core/RenderContext.cs @@ -16,6 +16,11 @@ public sealed class RenderContext private ContextScope? _contextScope; private int _uiThreadId; + // The window whose host last rendered this context. Picker calls run from event + // handlers, outside render, where ReactorApp.ActiveHostInternal is whichever host + // rendered or was constructed most recently (or null after any host is disposed). + private Microsoft.UI.Reactor.ReactorWindow? _owningWindow; + /// /// Time source used by time-based hooks (currently 's /// window). Defaults to ; @@ -28,6 +33,7 @@ internal void BeginRender(Action requestRerender) _hookIndex = 0; _requestRerender = requestRerender; _uiThreadId = Environment.CurrentManagedThreadId; + _owningWindow = Microsoft.UI.Reactor.ReactorApp.ActiveHostInternal?.OwningWindow; } internal void BeginRender(Action requestRerender, ContextScope contextScope) @@ -36,6 +42,7 @@ internal void BeginRender(Action requestRerender, ContextScope contextScope) _requestRerender = requestRerender; _contextScope = contextScope; _uiThreadId = Environment.CurrentManagedThreadId; + _owningWindow = Microsoft.UI.Reactor.ReactorApp.ActiveHostInternal?.OwningWindow; } /// @@ -2037,58 +2044,79 @@ public Action UseWindowDragMove() internal static IPickerService PickerService { get; set; } = new DefaultPickerService(); - /// - /// Opens a single-file picker initialized with the owning window HWND. Must be called on the UI thread. - /// - /// Call from an event handler, not during Render. Cancellation returns null. - public Task UseFilePickerAsync(FilePickerOptions options) + /// Opens a single-file picker owned by this component's window. + /// + /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window + /// that last rendered this component. Cancellation returns null. + /// + public Task PickFileAsync(FilePickerOptions options) { ArgumentNullException.ThrowIfNull(options); - var hwnd = GetPickerWindowHandle(nameof(UseFilePickerAsync)); - return PickerService.PickFileAsync(hwnd, options); + PickerOptionsValidation.Validate(options); + var owner = GetPickerOwner(nameof(PickFileAsync)); + return PickerService.PickFileAsync(owner, options); } - /// Opens a multiple-file picker on the owning window's UI thread. - /// Call from an event handler, not during Render. Cancellation returns an empty list. - public Task> UseMultipleFilesPickerAsync(FilePickerOptions options) + /// Opens a multiple-file picker owned by this component's window. + /// + /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window + /// that last rendered this component. Cancellation returns an empty list. + /// + public Task> PickMultipleFilesAsync(FilePickerOptions options) { ArgumentNullException.ThrowIfNull(options); - var hwnd = GetPickerWindowHandle(nameof(UseMultipleFilesPickerAsync)); - return PickerService.PickMultipleFilesAsync(hwnd, options); + PickerOptionsValidation.Validate(options); + var owner = GetPickerOwner(nameof(PickMultipleFilesAsync)); + return PickerService.PickMultipleFilesAsync(owner, options); } - /// - /// Opens a folder picker initialized with the owning window HWND. Must be called on the UI thread. - /// - /// Call from an event handler, not during Render. Cancellation returns null. - public Task UseFolderPickerAsync(FolderPickerOptions options) + /// Opens a single-folder picker owned by this component's window. + /// + /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window + /// that last rendered this component. Cancellation returns null. + /// + public Task PickFolderAsync(FolderPickerOptions options) { ArgumentNullException.ThrowIfNull(options); - var hwnd = GetPickerWindowHandle(nameof(UseFolderPickerAsync)); - return PickerService.PickFolderAsync(hwnd, options); + var owner = GetPickerOwner(nameof(PickFolderAsync)); + return PickerService.PickFolderAsync(owner, options); } - /// Opens a save-file picker on the owning window's UI thread. - /// Call from an event handler, not during Render. Cancellation returns null. Writing to the returned file is the caller's responsibility. - public Task UseSaveFilePickerAsync(SaveFilePickerOptions options) + /// Opens a multiple-folder picker owned by this component's window. + /// + /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window + /// that last rendered this component. Cancellation returns an empty list. + /// + public Task> PickMultipleFoldersAsync(FolderPickerOptions options) { ArgumentNullException.ThrowIfNull(options); - var hwnd = GetPickerWindowHandle(nameof(UseSaveFilePickerAsync)); - return PickerService.PickSaveFileAsync(hwnd, options); + var owner = GetPickerOwner(nameof(PickMultipleFoldersAsync)); + return PickerService.PickMultipleFoldersAsync(owner, options); } - private static nint GetPickerWindowHandle(string hookName) + /// Opens a save-file picker owned by this component's window. + /// + /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window + /// that last rendered this component. Cancellation returns null. Selecting a target does not + /// create or write the file; that is the caller's responsibility. + /// + public Task PickSaveFileAsync(SaveFilePickerOptions options) { - var win = Microsoft.UI.Reactor.ReactorApp.ActiveHostInternal?.OwningWindow - ?? throw new InvalidOperationException($"{hookName} requires an owning ReactorWindow."); - EnsurePickerOnUiThread(win, hookName); - return win.Hwnd; + ArgumentNullException.ThrowIfNull(options); + PickerOptionsValidation.Validate(options); + var owner = GetPickerOwner(nameof(PickSaveFileAsync)); + return PickerService.PickSaveFileAsync(owner, options); } - private static void EnsurePickerOnUiThread(Microsoft.UI.Reactor.ReactorWindow win, string hookName) + private Microsoft.UI.WindowId GetPickerOwner(string operation) { + var win = _owningWindow + ?? throw new InvalidOperationException($"{operation} requires an owning ReactorWindow."); + if (win.IsDisposed) + throw new InvalidOperationException($"{operation} requires an open owning ReactorWindow."); if (!win.NativeWindow.DispatcherQueue.HasThreadAccess) - throw new InvalidOperationException($"{hookName} must be called on the owning window's UI thread."); + throw new InvalidOperationException($"{operation} must be called on the owning window's UI thread."); + return win.AppWindow.Id; } /// @@ -2541,6 +2569,7 @@ internal void DetachFromHost() ClearHookState(); _requestRerender = null; _contextScope = null; + _owningWindow = null; } /// diff --git a/src/Reactor/Hosting/ReactorWindow.cs b/src/Reactor/Hosting/ReactorWindow.cs index 7dfd0086f..a2f38f392 100644 --- a/src/Reactor/Hosting/ReactorWindow.cs +++ b/src/Reactor/Hosting/ReactorWindow.cs @@ -290,6 +290,8 @@ public sealed partial class ReactorWindow : IDisposable internal nint Hwnd => _hwnd; + internal bool IsDisposed => _disposed; + /// /// When true, this window is an auxiliary surface (e.g. a docking /// tear-off floating window) that must never be elected the application's diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/Phase7WindowingFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/Phase7WindowingFixtures.cs index 3c2174919..21113d644 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/Phase7WindowingFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/Phase7WindowingFixtures.cs @@ -4,13 +4,12 @@ using Microsoft.UI.Xaml.Automation.Peers; using Microsoft.UI.Xaml.Automation.Provider; using Microsoft.UI.Xaml.Controls; -using Windows.Storage; -using Windows.Storage.Pickers; +using Microsoft.Windows.Storage.Pickers; using static Microsoft.UI.Reactor.Factories; namespace Microsoft.UI.Reactor.AppTests.Host.SelfTest.Fixtures; -/// Spec 054 Phase 7 fixtures for title-bar inference, transparent backdrop, and picker HWND initialization. +/// Spec 054 Phase 7 fixtures for title-bar inference, transparent backdrop, and picker owner-window initialization. internal static class Phase7WindowingFixtures { private static void EnsureUIDispatcher() @@ -57,11 +56,11 @@ private sealed class TransparentBackdropComponent : Component private sealed class PickerComponent : Component { - public Func>? Pick { get; private set; } + public Func>? Pick { get; private set; } public override Element Render() { - Pick = () => UseFilePickerAsync(new FilePickerOptions()); + Pick = () => PickFileAsync(new FilePickerOptions()); return TextBlock("picker"); } } @@ -101,8 +100,8 @@ public override Element Render() return VStack( aspectMounted ? Component() : TextBlock("aspect unmounted"), TextBlock($"hooks {count}"), - Button("File", () => _ = UseFilePickerAsync(fileOptions)).OnMount(fe => FileButton = (Button)fe), - Button("Folder", () => _ = UseFolderPickerAsync(folderOptions)).OnMount(fe => FolderButton = (Button)fe)); + Button("File", () => _ = PickFileAsync(fileOptions)).OnMount(fe => FileButton = (Button)fe), + Button("Folder", () => _ = PickFolderAsync(folderOptions)).OnMount(fe => FolderButton = (Button)fe)); } } @@ -116,33 +115,36 @@ public override Element Render() } private sealed class StubPickerService : IPickerService { - public nint LastHwnd { get; private set; } + public Microsoft.UI.WindowId LastOwner { get; private set; } public int FileCalls { get; private set; } public int FolderCalls { get; private set; } public FilePickerOptions? LastFileOptions { get; private set; } public FolderPickerOptions? LastFolderOptions { get; private set; } - public Task PickFileAsync(nint hwnd, FilePickerOptions options) + public Task PickFileAsync(Microsoft.UI.WindowId owner, FilePickerOptions options) { - LastHwnd = hwnd; + LastOwner = owner; LastFileOptions = options; FileCalls++; - return Task.FromResult(null); + return Task.FromResult(null); } - public Task PickFolderAsync(nint hwnd, FolderPickerOptions options) + public Task PickFolderAsync(Microsoft.UI.WindowId owner, FolderPickerOptions options) { - LastHwnd = hwnd; + LastOwner = owner; LastFolderOptions = options; FolderCalls++; - return Task.FromResult(null); + return Task.FromResult(null); } - public Task> PickMultipleFilesAsync(nint hwnd, FilePickerOptions options) - => Task.FromResult>([]); + public Task> PickMultipleFilesAsync(Microsoft.UI.WindowId owner, FilePickerOptions options) + => Task.FromResult>([]); - public Task PickSaveFileAsync(nint hwnd, SaveFilePickerOptions options) - => Task.FromResult(null); + public Task> PickMultipleFoldersAsync(Microsoft.UI.WindowId owner, FolderPickerOptions options) + => Task.FromResult>([]); + + public Task PickSaveFileAsync(Microsoft.UI.WindowId owner, SaveFilePickerOptions options) + => Task.FromResult(null); } private static async Task OpenAndSettle(WindowSpec spec, Func root) @@ -445,8 +447,7 @@ public override async Task RunAsync() try { await component.Pick!(); - var expected = WinRT.Interop.WindowNative.GetWindowHandle(win.NativeWindow); - H.Check("FilePicker_InitializesWithWindow", service.FileCalls == 1 && service.LastHwnd == expected); + H.Check("FilePicker_InitializesWithWindow", service.FileCalls == 1 && service.LastOwner == win.AppWindow.Id); } finally { @@ -527,19 +528,19 @@ public override async Task RunAsync() H.Check("UseIsCovered_RerendersOnZOrderChange", coveredUpdated); Invoke(component.FileButton!); - var expected = WinRT.Interop.WindowNative.GetWindowHandle(win.NativeWindow); - H.Check("UseFilePickerAsync_RoutesThroughPickerService", service.FileCalls == 1 && service.LastHwnd == expected); + var expected = win.AppWindow.Id; + H.Check("PickFileAsync_RoutesThroughPickerService", service.FileCalls == 1 && service.LastOwner == expected); var fileOpts = service.LastFileOptions; - H.Check("UseFilePickerAsync_RoutesThroughPickerService_Options", + H.Check("PickFileAsync_RoutesThroughPickerService_Options", fileOpts is not null && fileOpts.FileTypeFilter?.FirstOrDefault() == ".txt" && fileOpts.SuggestedStartLocation == PickerLocationId.PicturesLibrary && fileOpts.CommitButtonText == "Open Test"); Invoke(component.FolderButton!); - H.Check("UseFolderPickerAsync_RoutesThroughPickerService", service.FolderCalls == 1 && service.LastHwnd == expected); + H.Check("PickFolderAsync_RoutesThroughPickerService", service.FolderCalls == 1 && service.LastOwner == expected); var folderOpts = service.LastFolderOptions; - H.Check("UseFolderPickerAsync_RoutesThroughPickerService_Options", + H.Check("PickFolderAsync_RoutesThroughPickerService_Options", folderOpts is not null && folderOpts.SuggestedStartLocation == PickerLocationId.Desktop && folderOpts.CommitButtonText == "Select Test"); diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickerHooksFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickerHooksFixtures.cs deleted file mode 100644 index c2f31a119..000000000 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickerHooksFixtures.cs +++ /dev/null @@ -1,177 +0,0 @@ -using Microsoft.UI.Dispatching; -using Microsoft.UI.Reactor.Core; -using Windows.Storage; -using Windows.Storage.Pickers; -using static Microsoft.UI.Reactor.Factories; - -namespace Microsoft.UI.Reactor.AppTests.Host.SelfTest.Fixtures; - -internal static class PickerHooksFixtures -{ - private sealed class PickerComponent : Component - { - public FilePickerOptions OpenOptions { get; } = new([".txt", ".md"], PickerLocationId.Downloads, "Read") - { - ViewMode = PickerViewMode.Thumbnail, - SettingsIdentifier = "reactor-open-test", - }; - public FolderPickerOptions FolderOptions { get; } = new(PickerLocationId.Desktop, "Select") - { - FileTypeFilter = [".txt"], - ViewMode = PickerViewMode.Thumbnail, - SettingsIdentifier = "reactor-folder-test", - }; - public SaveFilePickerOptions SaveOptions { get; } = new( - new Dictionary> { ["Text"] = [".txt", ".md"], ["JSON"] = [".json"] }, - "report", PickerLocationId.PicturesLibrary, "Export") - { - SettingsIdentifier = "reactor-save-test", - }; - public Func> PickSingle = null!; - public Func>> PickMultiple = null!; - public Func> PickFolder = null!; - public Func> PickSave = null!; - - public override Element Render() - { - PickSingle = () => UseFilePickerAsync(OpenOptions); - PickMultiple = () => UseMultipleFilesPickerAsync(OpenOptions); - // Exercise the RenderContext entry point as well as Component forwarding. - PickFolder = () => Context.UseFolderPickerAsync(FolderOptions); - PickSave = () => UseSaveFilePickerAsync(SaveOptions); - return TextBlock("picker hooks"); - } - } - - private sealed class StubPickerService : IPickerService - { - public nint Hwnd; - public object? Options; - public string? Operation; - public int Calls; - public StorageFile? File; - public StorageFolder? Folder; - public IReadOnlyList Files = []; - public Exception? Error; - - private Task Record(nint hwnd, object options, string operation, T result) - { - Hwnd = hwnd; - Options = options; - Operation = operation; - Calls++; - return Error is null ? Task.FromResult(result) : Task.FromException(Error); - } - - public Task PickFileAsync(nint hwnd, FilePickerOptions options) - => Record(hwnd, options, "single", File); - public Task> PickMultipleFilesAsync(nint hwnd, FilePickerOptions options) - => Record(hwnd, options, "multiple", Files); - public Task PickFolderAsync(nint hwnd, FolderPickerOptions options) - => Record(hwnd, options, "folder", Folder); - public Task PickSaveFileAsync(nint hwnd, SaveFilePickerOptions options) - => Record(hwnd, options, "save", File); - } - - internal sealed class Suite(Harness h) : SelfTestFixtureBase(h) - { - public override async Task RunAsync() - { - ReactorApp.UIDispatcher ??= DispatcherQueue.GetForCurrentThread(); - ReactorApp.ShutdownPolicy = ShutdownPolicy.Explicit; - var previous = RenderContext.PickerService; - var stub = new StubPickerService(); - var component = new PickerComponent(); - RenderContext.PickerService = stub; - ReactorWindow? window = null; - StorageFolder? directory = null; - try - { - window = ReactorApp.OpenWindow(new WindowSpec { Title = "Picker Hooks", Width = 320, Height = 220 }, () => component); - await window.Host.WaitForIdleAsync(); - await Harness.Render(100); - var hwnd = WinRT.Interop.WindowNative.GetWindowHandle(window.NativeWindow); - var temp = await StorageFolder.GetFolderFromPathAsync(global::System.IO.Path.GetTempPath()); - directory = await temp.CreateFolderAsync("ReactorPickerHooks-" + Guid.NewGuid().ToString("N")); - var first = await directory.CreateFileAsync("first.txt"); - var second = await directory.CreateFileAsync("second.md"); - stub.File = first; - stub.Folder = directory; - stub.Files = new List { first, second }; - - var single = await component.PickSingle(); - H.Check("PickerHooks_Single_Routes", stub.Operation == "single" && stub.Hwnd == hwnd && ReferenceEquals(stub.Options, component.OpenOptions)); - H.Check("PickerHooks_Single_Result", ReferenceEquals(single, first)); - var multiple = await component.PickMultiple(); - H.Check("PickerHooks_Multiple_Routes", stub.Operation == "multiple" && stub.Hwnd == hwnd && ReferenceEquals(stub.Options, component.OpenOptions)); - H.Check("PickerHooks_Multiple_PreservesCollection", ReferenceEquals(multiple, stub.Files) && multiple.Count == 2 && multiple[0].Path == first.Path && multiple[1].Path == second.Path); - var folder = await component.PickFolder(); - H.Check("PickerHooks_Folder_Routes", stub.Operation == "folder" && stub.Hwnd == hwnd && ReferenceEquals(stub.Options, component.FolderOptions)); - H.Check("PickerHooks_Folder_Result", ReferenceEquals(folder, directory)); - var save = await component.PickSave(); - H.Check("PickerHooks_Save_Routes", stub.Operation == "save" && stub.Hwnd == hwnd && ReferenceEquals(stub.Options, component.SaveOptions)); - H.Check("PickerHooks_Save_Result", ReferenceEquals(save, first)); - - stub.File = null; - stub.Folder = null; - stub.Files = []; - H.Check("PickerHooks_Single_Cancel", await component.PickSingle() is null); - H.Check("PickerHooks_Multiple_Cancel", (await component.PickMultiple()).Count == 0); - H.Check("PickerHooks_Folder_Cancel", await component.PickFolder() is null); - H.Check("PickerHooks_Save_Cancel", await component.PickSave() is null); - - Func[] operations = [() => component.PickSingle(), () => component.PickMultiple(), () => component.PickFolder(), () => component.PickSave()]; - for (int i = 0; i < operations.Length; i++) - { - var calls = stub.Calls; - bool rejected = false; - try { await Task.Run(operations[i]); } - catch (InvalidOperationException ex) { rejected = ex.Message.Contains("UI thread", StringComparison.Ordinal); } - H.Check($"PickerHooks_OffUiThread_{i}", rejected && stub.Calls == calls); - var error = new InvalidOperationException("picker failure"); - stub.Error = error; - Exception? actual = null; - try { await operations[i](); } - catch (Exception ex) { actual = ex; } - H.Check($"PickerHooks_Error_{i}", ReferenceEquals(actual, error)); - stub.Error = null; - } - - // Inspect the real pickers without opening OS dialogs. Both open - // operations use this factory, so options cannot drift between them. - var open = DefaultPickerService.CreateFileOpenPicker(hwnd, component.OpenOptions); - H.Check("PickerHooks_NativeOpenOptions", open.FileTypeFilter.SequenceEqual([".txt", ".md"]) - && open.SuggestedStartLocation == PickerLocationId.Downloads && open.CommitButtonText == "Read" - && open.ViewMode == PickerViewMode.Thumbnail && open.SettingsIdentifier == "reactor-open-test"); - var nativeFolder = DefaultPickerService.CreateFolderPicker(hwnd, component.FolderOptions); - H.Check("PickerHooks_NativeFolderOptions", nativeFolder.FileTypeFilter.SequenceEqual([".txt"]) - && nativeFolder.SuggestedStartLocation == PickerLocationId.Desktop && nativeFolder.CommitButtonText == "Select" - && nativeFolder.ViewMode == PickerViewMode.Thumbnail && nativeFolder.SettingsIdentifier == "reactor-folder-test"); - var nativeSave = DefaultPickerService.CreateFileSavePicker(hwnd, component.SaveOptions); - H.Check("PickerHooks_NativeSaveOptions", nativeSave.FileTypeChoices.Count == 2 - && nativeSave.FileTypeChoices["Text"].SequenceEqual([".txt", ".md"]) - && nativeSave.FileTypeChoices["JSON"].SequenceEqual([".json"]) - && nativeSave.SuggestedFileName == "report" && nativeSave.CommitButtonText == "Export" - && nativeSave.SuggestedStartLocation == PickerLocationId.PicturesLibrary && nativeSave.SettingsIdentifier == "reactor-save-test"); - var suggestedSave = DefaultPickerService.CreateFileSavePicker(hwnd, component.SaveOptions with { SuggestedSaveFile = first }); - H.Check("PickerHooks_NativeSuggestedSaveFile", suggestedSave.SuggestedSaveFile?.Path == first.Path); - var defaultOpen = DefaultPickerService.CreateFileOpenPicker(hwnd, new()); - var emptyOpen = DefaultPickerService.CreateFileOpenPicker(hwnd, new([])); - var defaultFolder = DefaultPickerService.CreateFolderPicker(hwnd, new()); - var emptyFolder = DefaultPickerService.CreateFolderPicker(hwnd, new() { FileTypeFilter = [] }); - H.Check("PickerHooks_NativeViewDefault", defaultOpen.ViewMode == PickerViewMode.List); - H.Check("PickerHooks_NativeFilterDefaults", defaultOpen.FileTypeFilter.SequenceEqual(["*"]) - && emptyOpen.FileTypeFilter.SequenceEqual(["*"]) - && defaultFolder.FileTypeFilter.SequenceEqual(["*"]) - && emptyFolder.FileTypeFilter.SequenceEqual(["*"])); - } - finally - { - RenderContext.PickerService = previous; - if (window is not null) window.Close(); - if (directory is not null) await directory.DeleteAsync(StorageDeleteOption.PermanentDelete); - await Harness.Render(100); - } - } - } -} diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs new file mode 100644 index 000000000..8f2d557db --- /dev/null +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs @@ -0,0 +1,276 @@ +using Microsoft.UI; +using Microsoft.UI.Dispatching; +using Microsoft.UI.Reactor.Core; +using Microsoft.Windows.Storage.Pickers; +using static Microsoft.UI.Reactor.Factories; + +namespace Microsoft.UI.Reactor.AppTests.Host.SelfTest.Fixtures; + +internal static class PickersFixtures +{ + private static readonly string[] OperationNames = ["single", "multiple", "folder", "folders", "save"]; + + private sealed class PickerComponent : Component + { + public FilePickerOptions OpenOptions { get; } = new([".txt", ".md"], PickerLocationId.Downloads, "Read") + { + ViewMode = PickerViewMode.Thumbnail, + SettingsIdentifier = "reactor-open-test", + Title = "Open a note", + SuggestedFolder = global::System.IO.Path.GetTempPath(), + // Deliberately not alphabetical, so a sorted native map would be visible. + FileTypeChoices = [new("Text", [".txt", ".md"]), new("JSON", [".json"]), new("Archive", [".zip"])], + InitialFileTypeIndex = 1, + }; + public FolderPickerOptions FolderOptions { get; } = new(PickerLocationId.Desktop, "Select") + { + ViewMode = PickerViewMode.Thumbnail, + SettingsIdentifier = "reactor-folder-test", + Title = "Choose a folder", + SuggestedStartFolder = global::System.IO.Path.GetTempPath(), + }; + public SaveFilePickerOptions SaveOptions { get; } = new( + [new("Text", [".txt", ".md"]), new("JSON", [".json"]), new("Archive", [".zip"])], + "report", PickerLocationId.PicturesLibrary, "Export") + { + SettingsIdentifier = "reactor-save-test", + Title = "Export the report", + SuggestedFolder = global::System.IO.Path.GetTempPath(), + DefaultFileExtension = ".txt", + InitialFileTypeIndex = 2, + ShowOverwritePrompt = false, + }; + + // One delegate per operation, in OperationNames order. + public Func[] Operations = []; + public Func> PickSingle = null!; + public Func>> PickMultiple = null!; + public Func> PickFolder = null!; + public Func>> PickFolders = null!; + public Func> PickSave = null!; + + public override Element Render() + { + // Component forwarding for files, the RenderContext entry point for folders. + PickSingle = () => PickFileAsync(OpenOptions); + PickMultiple = () => PickMultipleFilesAsync(OpenOptions); + PickFolder = () => Context.PickFolderAsync(FolderOptions); + PickFolders = () => Context.PickMultipleFoldersAsync(FolderOptions); + PickSave = () => PickSaveFileAsync(SaveOptions); + Operations = [() => PickSingle(), () => PickMultiple(), () => PickFolder(), () => PickFolders(), () => PickSave()]; + return TextBlock("pickers"); + } + } + + private sealed class StubPickerService : IPickerService + { + public WindowId Owner; + public object? Options; + public string? Operation; + public int Calls; + public IReadOnlyList Files = new List(); + public IReadOnlyList Folders = new List(); + public Exception? Error; + + private Task Record(WindowId owner, object options, string operation, T result) + { + Owner = owner; + Options = options; + Operation = operation; + Calls++; + return Error is null ? Task.FromResult(result) : Task.FromException(Error); + } + + // PickFileResult/PickFolderResult have no public constructor, so single picks can only + // be stubbed as cancelled; multiple picks return a distinct list instance to prove the + // collection is passed through rather than copied. + public Task PickFileAsync(WindowId owner, FilePickerOptions options) + => Record(owner, options, "single", null); + public Task> PickMultipleFilesAsync(WindowId owner, FilePickerOptions options) + => Record(owner, options, "multiple", Files); + public Task PickFolderAsync(WindowId owner, FolderPickerOptions options) + => Record(owner, options, "folder", null); + public Task> PickMultipleFoldersAsync(WindowId owner, FolderPickerOptions options) + => Record(owner, options, "folders", Folders); + public Task PickSaveFileAsync(WindowId owner, SaveFilePickerOptions options) + => Record(owner, options, "save", null); + } + + private static async Task Open(string title, Component component) + { + var window = ReactorApp.OpenWindow(new WindowSpec { Title = title, Width = 320, Height = 220 }, () => component); + await window.Host.WaitForIdleAsync(); + await Harness.Render(100); + return window; + } + + private static object ExpectedOptions(PickerComponent component, int operation) => operation switch + { + 0 or 1 => component.OpenOptions, + 2 or 3 => component.FolderOptions, + _ => component.SaveOptions, + }; + + internal sealed class Suite(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + ReactorApp.UIDispatcher ??= DispatcherQueue.GetForCurrentThread(); + ReactorApp.ShutdownPolicy = ShutdownPolicy.Explicit; + var previous = RenderContext.PickerService; + var stub = new StubPickerService(); + var component = new PickerComponent(); + RenderContext.PickerService = stub; + ReactorWindow? window = null; + try + { + window = await Open("Pickers", component); + var owner = window.AppWindow.Id; + + for (int i = 0; i < component.Operations.Length; i++) + { + await component.Operations[i](); + H.Check($"Pickers_Routes_{OperationNames[i]}", stub.Operation == OperationNames[i] + && stub.Owner == owner && ReferenceEquals(stub.Options, ExpectedOptions(component, i))); + } + + H.Check("Pickers_Single_Cancel", await component.PickSingle() is null); + H.Check("Pickers_Folder_Cancel", await component.PickFolder() is null); + H.Check("Pickers_Save_Cancel", await component.PickSave() is null); + var files = await component.PickMultiple(); + H.Check("Pickers_MultipleFiles_PassesCollectionThrough", ReferenceEquals(files, stub.Files) && files.Count == 0); + var folders = await component.PickFolders(); + H.Check("Pickers_MultipleFolders_PassesCollectionThrough", ReferenceEquals(folders, stub.Folders) && folders.Count == 0); + + for (int i = 0; i < component.Operations.Length; i++) + { + var calls = stub.Calls; + bool rejected = false; + try { await Task.Run(component.Operations[i]); } + catch (InvalidOperationException ex) { rejected = ex.Message.Contains("UI thread", StringComparison.Ordinal); } + H.Check($"Pickers_OffUiThread_{OperationNames[i]}", rejected && stub.Calls == calls); + var error = new InvalidOperationException("picker failure"); + stub.Error = error; + Exception? actual = null; + try { await component.Operations[i](); } + catch (Exception ex) { actual = ex; } + H.Check($"Pickers_Error_{OperationNames[i]}", ReferenceEquals(actual, error)); + stub.Error = null; + } + + // Inspect the real pickers without opening OS dialogs. Both open operations + // (and both folder operations) share one factory, so options cannot drift. + var temp = global::System.IO.Path.GetTempPath(); + var open = DefaultPickerService.CreateFileOpenPicker(owner, component.OpenOptions); + H.Check("Pickers_NativeOpenOptions", open.FileTypeFilter.SequenceEqual([".txt", ".md"]) + && open.SuggestedStartLocation == PickerLocationId.Downloads && open.CommitButtonText == "Read" + && open.ViewMode == PickerViewMode.Thumbnail && open.SettingsIdentifier == "reactor-open-test" + && open.Title == "Open a note" && open.SuggestedFolder == temp && open.SuggestedStartFolder == "" + && open.InitialFileTypeIndex == 1); + H.Check("Pickers_NativeOpenChoicesOrdered", + open.FileTypeChoices.Select(c => c.Key).SequenceEqual(["Text", "JSON", "Archive"]) + && open.FileTypeChoices["Text"].SequenceEqual([".txt", ".md"])); + var nativeFolder = DefaultPickerService.CreateFolderPicker(owner, component.FolderOptions); + H.Check("Pickers_NativeFolderOptions", nativeFolder.SuggestedStartLocation == PickerLocationId.Desktop + && nativeFolder.CommitButtonText == "Select" && nativeFolder.ViewMode == PickerViewMode.Thumbnail + && nativeFolder.SettingsIdentifier == "reactor-folder-test" && nativeFolder.Title == "Choose a folder" + && nativeFolder.SuggestedStartFolder == temp && nativeFolder.SuggestedFolder == ""); + var nativeSave = DefaultPickerService.CreateFileSavePicker(owner, component.SaveOptions); + H.Check("Pickers_NativeSaveOptions", nativeSave.SuggestedFileName == "report" + && nativeSave.CommitButtonText == "Export" && nativeSave.SuggestedStartLocation == PickerLocationId.PicturesLibrary + && nativeSave.SettingsIdentifier == "reactor-save-test" && nativeSave.Title == "Export the report" + && nativeSave.SuggestedFolder == temp && nativeSave.DefaultFileExtension == ".txt" + && nativeSave.InitialFileTypeIndex == 2 && !nativeSave.ShowOverwritePrompt); + H.Check("Pickers_NativeSaveChoicesOrdered", + nativeSave.FileTypeChoices.Select(c => c.Key).SequenceEqual(["Text", "JSON", "Archive"]) + && nativeSave.FileTypeChoices["Text"].SequenceEqual([".txt", ".md"]) + && nativeSave.FileTypeChoices["Archive"].SequenceEqual([".zip"])); + + var defaultOpen = DefaultPickerService.CreateFileOpenPicker(owner, new()); + var defaultSave = DefaultPickerService.CreateFileSavePicker(owner, new([new("Text", [".txt"])])); + H.Check("Pickers_NativeOpenDefaults", defaultOpen.FileTypeFilter.Count == 0 + && defaultOpen.FileTypeChoices.Count == 0 && defaultOpen.ViewMode == PickerViewMode.List + && defaultOpen.SuggestedStartLocation == PickerLocationId.DocumentsLibrary); + H.Check("Pickers_NativeSaveDefaults", defaultSave.ShowOverwritePrompt && defaultSave.DefaultFileExtension == ""); + } + finally + { + RenderContext.PickerService = previous; + window?.Close(); + await Harness.Render(100); + } + } + } + + /// + /// Issue #1374: the owner is the window that rendered the component, not whichever host + /// ReactorApp.ActiveHostInternal names when the handler runs. With A then B open, + /// that field names B; once B is disposed it is null. + /// + internal sealed class OwningWindow(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + ReactorApp.UIDispatcher ??= DispatcherQueue.GetForCurrentThread(); + ReactorApp.ShutdownPolicy = ShutdownPolicy.Explicit; + var previous = RenderContext.PickerService; + var stub = new StubPickerService(); + RenderContext.PickerService = stub; + var a = new PickerComponent(); + var b = new PickerComponent(); + ReactorWindow? windowA = null, windowB = null; + try + { + windowA = await Open("Pickers A", a); + windowB = await Open("Pickers B", b); + var ownerA = windowA.AppWindow.Id; + var ownerB = windowB.AppWindow.Id; + // Oracle guards: the two owners differ, and the process-wide host is B, so + // routing A's calls to A is not something a call-time lookup could produce. + H.Check("Pickers_OwningWindow_DistinctOwners", ownerA != ownerB); + H.Check("Pickers_OwningWindow_ActiveHostIsB", ReferenceEquals(ReactorApp.ActiveHostInternal?.OwningWindow, windowB)); + + for (int i = 0; i < a.Operations.Length; i++) + { + await a.Operations[i](); + H.Check($"Pickers_OwningWindow_BeforeClose_{OperationNames[i]}", stub.Operation == OperationNames[i] && stub.Owner == ownerA); + } + await b.Operations[0](); + H.Check("Pickers_OwningWindow_BRoutesToB", stub.Owner == ownerB); + + windowB.Close(); + await Harness.Render(100); + H.Check("Pickers_OwningWindow_ActiveHostClearedByClose", ReactorApp.ActiveHostInternal is null); + + for (int i = 0; i < a.Operations.Length; i++) + { + var calls = stub.Calls; + Exception? error = null; + try { await a.Operations[i](); } + catch (Exception ex) { error = ex; } + H.Check($"Pickers_OwningWindow_AfterClose_{OperationNames[i]}", + error is null && stub.Calls == calls + 1 && stub.Operation == OperationNames[i] && stub.Owner == ownerA); + } + + // B's component outlived its window: its pickers must refuse rather than + // parent a dialog to a closed (or unrelated) window. + for (int i = 0; i < b.Operations.Length; i++) + { + var calls = stub.Calls; + string? message = null; + try { await b.Operations[i](); } + catch (InvalidOperationException ex) { message = ex.Message; } + H.Check($"Pickers_OwningWindow_ClosedOwnerRejected_{OperationNames[i]}", + message is not null && message.Contains("owning ReactorWindow", StringComparison.Ordinal) && stub.Calls == calls); + } + } + finally + { + RenderContext.PickerService = previous; + windowB?.Close(); + windowA?.Close(); + await Harness.Render(100); + } + } + } +} diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index bcff429bf..7019b5223 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -1537,7 +1537,8 @@ internal static class SelfTestFixtureRegistry "BackdropTransparent_Apply", "FilePicker_InitializesWithWindow", "FilePicker_ThrowsOffUiThread", - "PickerHooks_Suite", + "Pickers_Suite", + "Pickers_OwningWindow", "AspectRatio_Suite_Math", "AspectRatio_Suite_Conform", "WindowStyle_ToolWindowExStyleRemoved", @@ -3621,7 +3622,8 @@ internal static string[] StaleTierDeclarations() => "BackdropTransparent_Apply" => new Phase7WindowingFixtures.BackdropTransparentApply(harness), "FilePicker_InitializesWithWindow" => new Phase7WindowingFixtures.FilePickerInitializesWithWindow(harness), "FilePicker_ThrowsOffUiThread" => new Phase7WindowingFixtures.FilePickerThrowsOffUiThread(harness), - "PickerHooks_Suite" => new PickerHooksFixtures.Suite(harness), + "Pickers_Suite" => new PickersFixtures.Suite(harness), + "Pickers_OwningWindow" => new PickersFixtures.OwningWindow(harness), "UseSpec054Hooks_Suite" => new Phase7WindowingFixtures.UseSpec054HooksSuite(harness), // Spec 036/045 — issue #647 multi-window teardown hardening regression. "Window647_PrimaryElectionExcludesAuxiliary" => new WindowLifecycleHardeningFixtures.PrimaryElectionExcludesAuxiliary(harness), diff --git a/tests/Reactor.Tests/AnalyzerTests/HookRulesAnalyzerTests.cs b/tests/Reactor.Tests/AnalyzerTests/HookRulesAnalyzerTests.cs index 431dbb42f..021241c79 100644 --- a/tests/Reactor.Tests/AnalyzerTests/HookRulesAnalyzerTests.cs +++ b/tests/Reactor.Tests/AnalyzerTests/HookRulesAnalyzerTests.cs @@ -1339,4 +1339,108 @@ public override Element Render() }; await analyzerTest.RunAsync(TestContext.Current.CancellationToken); } + + // ── Picker helpers (issue #1374) ────────────────────────────────── + // The pickers are not hooks: they reserve no slot and must run from event handlers. + // The Pick* names keep HOOKS_001/005 quiet there; the Use*-named control proves the same + // call shapes do reach the analyzer, so the clean result is not vacuous. + + private const string PickerStubs = @" +using System.Threading.Tasks; + +namespace Microsoft.UI.Reactor.Core +{ + public class RenderContext + { + public Task PickFileAsync(object options) => Task.FromResult(null); + public Task PickMultipleFilesAsync(object options) => Task.FromResult(null); + public Task PickFolderAsync(object options) => Task.FromResult(null); + public Task PickMultipleFoldersAsync(object options) => Task.FromResult(null); + public Task PickSaveFileAsync(object options) => Task.FromResult(null); + public Task UseFilePickerAsync(object options) => Task.FromResult(null); + } + + public abstract class Component + { + protected internal RenderContext Context { get; } = new RenderContext(); + public abstract string Render(); + protected Task PickFileAsync(object options) => Context.PickFileAsync(options); + protected Task PickMultipleFilesAsync(object options) => Context.PickMultipleFilesAsync(options); + protected Task PickFolderAsync(object options) => Context.PickFolderAsync(options); + protected Task PickMultipleFoldersAsync(object options) => Context.PickMultipleFoldersAsync(options); + protected Task PickSaveFileAsync(object options) => Context.PickSaveFileAsync(options); + protected Task UseFilePickerAsync(object options) => Context.UseFilePickerAsync(options); + } } +"; + + [Fact] + public async Task Pickers_Called_From_Handlers_Through_Component_Are_Not_Reported() + { + var test = PickerStubs + @" +class C : Microsoft.UI.Reactor.Core.Component +{ + static void Button(System.Func onClick) { } + + public override string Render() + { + Button(async () => { await PickFileAsync(null); await this.PickMultipleFilesAsync(null); }); + Button(async () => { if (System.DateTime.Now.Ticks > 0) await PickFolderAsync(null); }); + Button(async () => { await PickMultipleFoldersAsync(null); }); + Button(OnSave); + return """"; + } + + private async Task OnSave() => await PickSaveFileAsync(null); +}"; + var analyzerTest = new CSharpAnalyzerTest { TestCode = test }; + await analyzerTest.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Pickers_Called_From_Handlers_Through_RenderContext_Are_Not_Reported() + { + var test = PickerStubs + @" +class C : Microsoft.UI.Reactor.Core.Component +{ + static void Button(System.Func onClick) { } + + public override string Render() + { + var ctx = Context; + Button(async () => { await Context.PickFileAsync(null); await ctx.PickMultipleFilesAsync(null); }); + Button(async () => { if (System.DateTime.Now.Ticks > 0) await ctx.PickFolderAsync(null); }); + Button(async () => { await ctx.PickMultipleFoldersAsync(null); }); + Button(() => OnSave(ctx)); + return """"; + } + + private static async Task OnSave(Microsoft.UI.Reactor.Core.RenderContext ctx) => await ctx.PickSaveFileAsync(null); +}"; + var analyzerTest = new CSharpAnalyzerTest { TestCode = test }; + await analyzerTest.RunAsync(TestContext.Current.CancellationToken); + } + + [Fact] + public async Task Use_Named_Picker_In_The_Same_Handler_Shapes_Is_Reported() + { + var test = PickerStubs + @" +class C : Microsoft.UI.Reactor.Core.Component +{ + static void Button(System.Func onClick) { } + + public override string Render() + { + var ctx = Context; + Button(async () => { await {|REACTOR_HOOKS_001:UseFilePickerAsync(null)|}; }); + Button(async () => { await {|REACTOR_HOOKS_001:ctx.UseFilePickerAsync(null)|}; }); + Button(OnOpen); + return """"; + } + + private async Task OnOpen() => await {|REACTOR_HOOKS_005:Context.UseFilePickerAsync(null)|}; +}"; + var analyzerTest = new CSharpAnalyzerTest { TestCode = test }; + await analyzerTest.RunAsync(TestContext.Current.CancellationToken); + } +} \ No newline at end of file diff --git a/tests/Reactor.Tests/PickerHookTests.cs b/tests/Reactor.Tests/PickerHookTests.cs deleted file mode 100644 index 48d97d916..000000000 --- a/tests/Reactor.Tests/PickerHookTests.cs +++ /dev/null @@ -1,54 +0,0 @@ -using Microsoft.UI.Reactor.Core; -using Xunit; - -namespace Microsoft.UI.Reactor.Tests; - -public class PickerHookTests -{ - [Theory] - [InlineData("UseFilePickerAsync")] - [InlineData("UseMultipleFilesPickerAsync")] - [InlineData("UseFolderPickerAsync")] - [InlineData("UseSaveFilePickerAsync")] - public void NullOptionsAreRejectedBeforeWindowLookup(string method) - { - var context = new RenderContext(); - var error = Assert.Throws(() => Invoke(context, method, null)); - Assert.Equal("options", error.ParamName); - } - - [Theory] - [InlineData("UseFilePickerAsync")] - [InlineData("UseMultipleFilesPickerAsync")] - [InlineData("UseFolderPickerAsync")] - [InlineData("UseSaveFilePickerAsync")] - public void MissingWindowReportsTheRequestedOperation(string method) - { - var context = new RenderContext(); - var error = Assert.Throws(() => Invoke(context, method, new FilePickerOptions())); - Assert.Equal($"{method} requires an owning ReactorWindow.", error.Message); - } - - [Fact] - public void SavePickerRejectsMissingFileTypeChoices() - { - var missing = Assert.Throws(() => DefaultPickerService.CreateFileSavePicker(0, new(null!))); - Assert.Equal("options.FileTypeChoices", missing.ParamName); - var empty = Assert.Throws(() => DefaultPickerService.CreateFileSavePicker(0, new(new Dictionary>()))); - Assert.Equal("options", empty.ParamName); - Assert.StartsWith("At least one file type choice is required.", empty.Message); - } - - private static void Invoke(RenderContext context, string method, FilePickerOptions? options) - { - _ = method switch - { - "UseFilePickerAsync" => (Task)context.UseFilePickerAsync(options!), - "UseMultipleFilesPickerAsync" => context.UseMultipleFilesPickerAsync(options!), - "UseFolderPickerAsync" => context.UseFolderPickerAsync(options is null ? null! : new()), - "UseSaveFilePickerAsync" => context.UseSaveFilePickerAsync(options is null ? null! : new( - new Dictionary> { ["Text"] = [".txt"] })), - _ => throw new ArgumentOutOfRangeException(nameof(method)), - }; - } -} diff --git a/tests/Reactor.Tests/PickerTests.cs b/tests/Reactor.Tests/PickerTests.cs new file mode 100644 index 000000000..a86755607 --- /dev/null +++ b/tests/Reactor.Tests/PickerTests.cs @@ -0,0 +1,121 @@ +using Microsoft.UI.Reactor.Core; +using Xunit; + +namespace Microsoft.UI.Reactor.Tests; + +public class PickerTests +{ + private static readonly IReadOnlyList TextChoice = [new("Text", [".txt"])]; + + public static TheoryData Operations => new() + { + "PickFileAsync", "PickMultipleFilesAsync", "PickFolderAsync", "PickMultipleFoldersAsync", "PickSaveFileAsync", + }; + + [Theory] + [MemberData(nameof(Operations))] + public void NullOptionsAreRejectedBeforeWindowLookup(string operation) + { + var context = new RenderContext(); + var error = Assert.Throws(() => Invoke(context, operation, validOptions: false)); + Assert.Equal("options", error.ParamName); + } + + [Theory] + [MemberData(nameof(Operations))] + public void MissingWindowReportsTheRequestedOperation(string operation) + { + // A context that never rendered inside a ReactorWindow host has no owner. + var context = new RenderContext(); + var error = Assert.Throws(() => Invoke(context, operation, validOptions: true)); + Assert.Equal($"{operation} requires an owning ReactorWindow.", error.Message); + } + + [Fact] + public void SavePickerRejectsMissingFileTypeChoices() + { + var context = new RenderContext(); + var missing = Assert.Throws(() => { _ = context.PickSaveFileAsync(new(null!)); }); + Assert.Equal("options.FileTypeChoices", missing.ParamName); + var empty = Assert.Throws(() => { _ = context.PickSaveFileAsync(new([])); }); + Assert.Equal("options", empty.ParamName); + Assert.StartsWith("At least one file type choice is required.", empty.Message); + } + + public static TheoryData InvalidChoices => new() + { + { "File type choices cannot contain null entries.", [new("Text", [".txt"]), null!] }, + { "Each file type choice requires a label.", [new(" ", [".txt"])] }, + { "Each file type choice requires a label.", [new(null!, [".txt"])] }, + { "File type choice 'Text' requires at least one extension.", [new("Text", [])] }, + { "File type choice 'Text' requires at least one extension.", [new("Text", null!)] }, + { "File type choice 'Text' is listed more than once.", [new("Text", [".txt"]), new("JSON", [".json"]), new("Text", [".md"])] }, + }; + + // Validation runs before the owner lookup, so these reach the option checks headlessly. + [Theory] + [MemberData(nameof(InvalidChoices))] + public void SavePickerRejectsInvalidChoices(string message, FileTypeChoice[] choices) + { + var error = Assert.Throws(() => { _ = new RenderContext().PickSaveFileAsync(new(choices)); }); + Assert.Equal("options", error.ParamName); + Assert.StartsWith(message, error.Message); + } + + [Theory] + [MemberData(nameof(InvalidChoices))] + public void OpenPickersRejectInvalidChoices(string message, FileTypeChoice[] choices) + { + var options = new FilePickerOptions { FileTypeChoices = choices }; + var single = Assert.Throws(() => { _ = new RenderContext().PickFileAsync(options); }); + var multiple = Assert.Throws(() => { _ = new RenderContext().PickMultipleFilesAsync(options); }); + Assert.StartsWith(message, single.Message); + Assert.StartsWith(message, multiple.Message); + } + + [Fact] + public void OpenPickersAcceptMissingOrEmptyChoices() + { + // Choices are optional for open pickers, so these get past validation to the owner lookup. + foreach (var options in new FilePickerOptions[] { new(), new() { FileTypeChoices = [] } }) + { + var error = Assert.Throws(() => { _ = new RenderContext().PickFileAsync(options); }); + Assert.Equal("PickFileAsync requires an owning ReactorWindow.", error.Message); + } + } + + [Fact] + public void LabelsAreComparedOrdinally() + { + // "Text" and "text" are distinct native map keys, so they are not duplicates. + var options = new SaveFilePickerOptions([new("Text", [".txt"]), new("text", [".md"])]); + var error = Assert.Throws(() => { _ = new RenderContext().PickSaveFileAsync(options); }); + Assert.Equal("PickSaveFileAsync requires an owning ReactorWindow.", error.Message); + } + + [Fact] + public void SaveOptionsDefaultToShowingTheOverwritePrompt() + { + var options = new SaveFilePickerOptions(TextChoice); + Assert.True(options.ShowOverwritePrompt); + Assert.Null(options.InitialFileTypeIndex); + Assert.Equal("", options.SuggestedFolder); + Assert.Equal("", options.DefaultFileExtension); + } + + private static void Invoke(RenderContext context, string operation, bool validOptions) + { + FilePickerOptions file = validOptions ? new() : null!; + FolderPickerOptions folder = validOptions ? new() : null!; + SaveFilePickerOptions save = validOptions ? new(TextChoice) : null!; + _ = operation switch + { + "PickFileAsync" => (Task)context.PickFileAsync(file), + "PickMultipleFilesAsync" => context.PickMultipleFilesAsync(file), + "PickFolderAsync" => context.PickFolderAsync(folder), + "PickMultipleFoldersAsync" => context.PickMultipleFoldersAsync(folder), + "PickSaveFileAsync" => context.PickSaveFileAsync(save), + _ => throw new ArgumentOutOfRangeException(nameof(operation)), + }; + } +} From 6ecb7f2c5567c0a66b0496b2ab441efc428b98e7 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Tue, 20 Oct 2026 14:38:57 -0700 Subject: [PATCH 2/5] Address pr-review findings Supply the picker owner from each host's reconciler instead of the process-wide ReactorApp.ActiveHostInternal. A window opened mid-render (UseOpenWindow) repointed that field for the rest of the opener's pass, and virtualized rows render outside the host's pass, so both recorded the wrong owner. Reconciler.OwningWindow is set when a ReactorWindow claims its host and passed to every BeginRender. Tests: nested class/function components, a retired root, and a window opened mid-render (Pickers_OwnerSurvivesMidRenderOpen), each mutation-checked. Code-quality: readonly stub fields and narrowed catch clauses in PickersFixtures. Docs: CHANGELOG entry, superseded spec 054 task 7.3 checklist, and the ReactorHostControl caveat now states the call throws and how to build the native picker from XamlRoot.ContentIslandEnvironment.AppWindowId. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 890c9be9-295c-487f-a96e-93eaa64994b0 --- CHANGELOG.md | 15 ++ docs/_pipeline/templates/windows.md.dt | 15 +- docs/guide/windows.md | 15 +- docs/specs/054-windowing-evolution.md | 13 +- .../054-windowing-evolution-implementation.md | 57 +++---- src/Reactor/Core/Reconciler.Mount.cs | 6 +- src/Reactor/Core/Reconciler.cs | 14 +- src/Reactor/Core/RenderContext.cs | 18 +- src/Reactor/Hosting/ReactorHost.cs | 10 +- .../SelfTest/Fixtures/PickersFixtures.cs | 159 +++++++++++++++++- .../SelfTest/SelfTestFixtureRegistry.cs | 2 + 11 files changed, 254 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d7de8c3c..8f067dc6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -219,6 +219,21 @@ Conventions for contributors: ### Changed +- **Picker helpers moved to `Microsoft.Windows.Storage.Pickers` and renamed as non-hooks** + (issue #1374, spec 054 §8.3). Breaking: `UseFilePickerAsync` / `UseMultipleFilesPickerAsync` + / `UseFolderPickerAsync` / `UseSaveFilePickerAsync` are now `PickFileAsync` / + `PickMultipleFilesAsync` / `PickFolderAsync` / `PickSaveFileAsync` (plus the new + `PickMultipleFoldersAsync`) on `RenderContext` and `Component`, and return path-based + `PickFileResult` / `PickFolderResult` instead of `StorageFile` / `StorageFolder`. Call them + directly from event handlers; the hook-rules analyzer no longer flags them. Option enums + come from `Microsoft.Windows.Storage.Pickers`; `SaveFilePickerOptions.FileTypeChoices` is an + ordered `IReadOnlyList` (order sets the default entry); `Title`, + `SuggestedFolder`, `SuggestedStartFolder`, `InitialFileTypeIndex`, open-picker + `FileTypeChoices`, `DefaultFileExtension` and `ShowOverwritePrompt` are new; + `FolderPickerOptions.FileTypeFilter` and `SaveFilePickerOptions.SuggestedSaveFile` are + removed. The dialog is owned by the window whose host rendered the component (previously + whichever window was most recently active, which with several windows picked the wrong one + and threw after any window closed), and the pickers now work in elevated apps. - **The doc pipeline moved out of `mur`, and doc screenshots are captured with winapp's Windows Graphics Capture.** `mur docs` is now `dotnet run --project tools/Reactor.DocPipeline --` (`compile`, `check-tier`, `render-diagrams`, `new-diagram`; same options). It is a contributor diff --git a/docs/_pipeline/templates/windows.md.dt b/docs/_pipeline/templates/windows.md.dt index 681ff0a36..da7757199 100644 --- a/docs/_pipeline/templates/windows.md.dt +++ b/docs/_pipeline/templates/windows.md.dt @@ -600,15 +600,20 @@ Caveats: - These methods are not hooks: they reserve no hook slot and open a dialog when called. Call them from UI-thread event handlers, not during `Render()` or in `Task.Run`. Their `Pick*` names keep the hook-rules analyzer quiet in handlers. -- The owner is the window that last rendered the component, not whichever window - is active when the handler runs. If that window has closed, or the component - never rendered in a Reactor window, the call throws `InvalidOperationException`. +- The owner is the window whose host rendered the component (including nested and + virtualized-row components), not whichever window is active when the handler + runs. If that window has closed, or the component's root was replaced, the call + throws `InvalidOperationException`. - Option validation (missing or duplicate file-type choices) throws `ArgumentException` before any dialog opens. Windows validates extension syntax and folder paths. - Picker errors propagate to the caller; cancellation is not an error. - Framework tests use an internal picker-service seam instead of opening modal dialogs. -- Content hosted through `ReactorHostControl` has no owning Reactor window of its - own, so it cannot rely on these methods to pick the correct owner. +- Content hosted through `ReactorHostControl` (or a `ReactorHost` you construct + yourself) has no owning `ReactorWindow`, so these methods throw + `InvalidOperationException` there. Construct the native picker directly from the + hosting window instead, for example + `new Microsoft.Windows.Storage.Pickers.FileOpenPicker(xamlRoot.ContentIslandEnvironment.AppWindowId)`. + ## WPF / UWP migration map Coming from a pre-054 Reactor app instead? See diff --git a/docs/guide/windows.md b/docs/guide/windows.md index 13e5e1276..0ccf1eac8 100644 --- a/docs/guide/windows.md +++ b/docs/guide/windows.md @@ -933,15 +933,20 @@ Caveats: - These methods are not hooks: they reserve no hook slot and open a dialog when called. Call them from UI-thread event handlers, not during `Render()` or in `Task.Run`. Their `Pick*` names keep the hook-rules analyzer quiet in handlers. -- The owner is the window that last rendered the component, not whichever window - is active when the handler runs. If that window has closed, or the component - never rendered in a Reactor window, the call throws `InvalidOperationException`. +- The owner is the window whose host rendered the component (including nested and + virtualized-row components), not whichever window is active when the handler + runs. If that window has closed, or the component's root was replaced, the call + throws `InvalidOperationException`. - Option validation (missing or duplicate file-type choices) throws `ArgumentException` before any dialog opens. Windows validates extension syntax and folder paths. - Picker errors propagate to the caller; cancellation is not an error. - Framework tests use an internal picker-service seam instead of opening modal dialogs. -- Content hosted through `ReactorHostControl` has no owning Reactor window of its - own, so it cannot rely on these methods to pick the correct owner. +- Content hosted through `ReactorHostControl` (or a `ReactorHost` you construct + yourself) has no owning `ReactorWindow`, so these methods throw + `InvalidOperationException` there. Construct the native picker directly from the + hosting window instead, for example + `new Microsoft.Windows.Storage.Pickers.FileOpenPicker(xamlRoot.ContentIslandEnvironment.AppWindowId)`. + ## WPF / UWP migration map Coming from a pre-054 Reactor app instead? See diff --git a/docs/specs/054-windowing-evolution.md b/docs/specs/054-windowing-evolution.md index f63de9c62..73539be58 100644 --- a/docs/specs/054-windowing-evolution.md +++ b/docs/specs/054-windowing-evolution.md @@ -1137,11 +1137,14 @@ over the UWP `Windows.Storage.Pickers` + `InitializeWithWindow`, returning time. Pickers run from event handlers, outside render, where that field names the most recently rendered or constructed host (or null after any host is disposed). With two windows, A's picker parented to B, and every picker threw - once any window closed. `RenderContext` now captures the owning window in - `BeginRender` (where `ReactorHost.Render` scopes that field to itself), clears - it on detach, and refuses a closed owner. Content rendered through - `ReactorHostControl`, which does not scope the field, keeps the pre-existing - limitation shared by every window hook. + once any window closed. Each host's `Reconciler` now carries its + `ReactorWindow` (`Reconciler.OwningWindow`, set when the window claims the host), + and every `BeginRender` records it on the `RenderContext`. The process-wide field + is not consulted, so a window opened mid-render or a virtualized row rendered + outside the host's pass still records the right owner. The owner is cleared on + detach and a closed owner is refused. `ReactorHostControl` and directly + constructed hosts have no `ReactorWindow`, so the pickers throw there; callers + construct the native picker from `XamlRoot.ContentIslandEnvironment.AppWindowId`. - **Naming.** `HookRulesAnalyzer` treats any `Use[A-Z]*` method on `Component` / `RenderContext` as a hook, so calling the old names from a click lambda raised REACTOR_HOOKS_001 and from a named handler REACTOR_HOOKS_005. The diff --git a/docs/specs/tasks/054-windowing-evolution-implementation.md b/docs/specs/tasks/054-windowing-evolution-implementation.md index 1eba3ed89..364e76371 100644 --- a/docs/specs/tasks/054-windowing-evolution-implementation.md +++ b/docs/specs/tasks/054-windowing-evolution-implementation.md @@ -733,35 +733,19 @@ Purely additive. `Window.SystemBackdrop is TransparentBackdrop` on supported builds. -### 7.3 Picker helpers (spec §8.3) - -- [x] Add `FilePickerOptions` / `FolderPickerOptions` records (filters, - suggested-folder, etc. — minimum viable shape; pickers are out - of scope for windowing beyond the `InitializeWithWindow` helper). -- [x] Add `RenderContext.UseFilePickerAsync(FilePickerOptions)` → - `Task`. -- [x] Add `RenderContext.UseFolderPickerAsync(FolderPickerOptions)` → - `Task`. -- [x] Add `RenderContext.UseMultipleFilesPickerAsync(FilePickerOptions)` → - `Task>`, with an empty list on cancellation. -- [x] Add `RenderContext.UseSaveFilePickerAsync(SaveFilePickerOptions)` → - `Task`, with null on cancellation; the caller writes the file. -- [x] Expose all four helpers on `Component`; capture during Render and invoke - from UI-thread event handlers. -- [x] Implementation: `UseWindow().NativeWindow.GetWindowHandle()` → - `WinRT.Interop.InitializeWithWindow.Initialize(picker, hwnd)`. - **Must run on the UI thread**; throw if called off-thread (R12). - **Must use the owning window's HWND**; do not accept arbitrary - HWND parameters (R12). -- [x] Selftest `FilePicker_InitializesWithWindow` — use a mock / - `IPickerService` test double; assert `InitializeWithWindow` was - called with the owning HWND. -- [x] Note in the PR description: §8.3 is per spec a candidate to split - into its own spec (§11 Q5 in the source spec 053; not this spec — - 054 doesn't have an explicit Q on it, but the §10 phase wording - flags it). If scoping requires, lift this section out into a - follow-up spec; otherwise land here. - +### 7.3 Picker helpers (spec §8.3) — superseded by 7.3a + +The first cut shipped `Use{File,MultipleFiles,Folder,SaveFile}PickerAsync` over the +UWP `Windows.Storage.Pickers` + `InitializeWithWindow`, returning `StorageFile` / +`StorageFolder`, with owner resolution through `ReactorApp.ActiveHostInternal`. +Issue #1374 replaced all of that before 1.0; 7.3a is the current shape. The items +below record what that first cut delivered and stay checked for history only. + +- [x] Option records, the four helpers on `RenderContext` and `Component`, null / + empty-list cancellation, UI-thread-only calls, and no arbitrary-HWND parameter (R12). +- [x] Selftest `FilePicker_InitializesWithWindow` against an `IPickerService` test + double (now asserts the owner `WindowId`). +- [x] PR note that §8.3 could be split into its own spec; it stayed here. ### 7.3a Picker migration to `Microsoft.Windows.Storage.Pickers` (issue #1374) - [x] Rebuild the helpers on `Microsoft.Windows.Storage.Pickers` (`WindowId` @@ -778,13 +762,18 @@ Purely additive. ordered `IReadOnlyList`, validated before the owner lookup. Remove `FolderPickerOptions.FileTypeFilter` and `SaveFilePickerOptions.SuggestedSaveFile`. -- [x] Capture the owning window on `RenderContext` during render instead of - reading `ReactorApp.ActiveHostInternal` at call time; reject a closed owner. -- [x] Selftest `Pickers_OwningWindow`: with A then B open, A's helpers route to - A before and after B closes; B's helpers are rejected after B closes. - Mutation-checked against the call-time lookup. +- [x] Record the owning window on each `RenderContext` from its host's reconciler + (`Reconciler.OwningWindow`, set when a `ReactorWindow` claims its host) at every + `BeginRender`, instead of reading the process-wide `ReactorApp.ActiveHostInternal`. + Clear it on `DetachFromHost`; reject a closed owner. +- [x] Selftest `Pickers_OwningWindow`: with A then B open, A's helpers (root, nested + class component, function component) route to A before and after B closes; B's + helpers are rejected after B closes; a retired root is rejected. Mutation-checked. +- [x] Selftest `Pickers_OwnerSurvivesMidRenderOpen`: a window opened mid-render does + not become the owner of components that render after it in the opener's window. - [x] Selftest `Pickers_Suite`: native option read-back, including insertion-ordered `FileTypeChoices` on open and save pickers. + ### 7.4 Phase 7 exit gate - [x] Selftests registered in both places. diff --git a/src/Reactor/Core/Reconciler.Mount.cs b/src/Reactor/Core/Reconciler.Mount.cs index 607cc93d1..874d7101f 100644 --- a/src/Reactor/Core/Reconciler.Mount.cs +++ b/src/Reactor/Core/Reconciler.Mount.cs @@ -890,7 +890,7 @@ private UIElement MountComponent(ComponentElement compElement, Action requestRer bool inEffects = false; try { - component.Context.BeginRender(componentRerender, _contextScope); + component.Context.BeginRender(componentRerender, _contextScope, OwningWindow); using (ValidationRenderScope.Begin(ReadContext(ValidationContexts.Current))) { childElement = ValidationRenderScope.ApplyProvide(component.Render()); @@ -973,7 +973,7 @@ private UIElement MountFuncComponent(FuncElement funcElement, Action requestRere bool inEffects = false; try { - ctx.BeginRender(componentRerender, _contextScope); + ctx.BeginRender(componentRerender, _contextScope, OwningWindow); using (ValidationRenderScope.Begin(ReadContext(ValidationContexts.Current))) { childElement = ValidationRenderScope.ApplyProvide(funcElement.RenderFunc(ctx)); @@ -1055,7 +1055,7 @@ private UIElement MountMemoComponent(MemoElement memoElement, Action requestRere bool inEffects = false; try { - ctx.BeginRender(componentRerender, _contextScope); + ctx.BeginRender(componentRerender, _contextScope, OwningWindow); using (ValidationRenderScope.Begin(ReadContext(ValidationContexts.Current))) { childElement = ValidationRenderScope.ApplyProvide(memoElement.RenderFunc(ctx)); diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index 7c6b15d13..6cb242a07 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -51,6 +51,14 @@ public sealed partial class Reconciler : IDisposable internal readonly ILogger? _logger; private readonly List<(string Key, UIElement Target)> _pendingConnectedAnimationStarts = new(); private readonly ContextScope _contextScope = new(); + + /// + /// The whose host owns this reconciler, or + /// null for hosts without one (, + /// directly constructed hosts). Every component context it renders records it as its picker + /// owner, including renders outside the host's render pass (virtualized rows). + /// + internal Microsoft.UI.Reactor.ReactorWindow? OwningWindow { get; set; } // Ancestor-aware effective theme for the subtree currently being mounted/updated, // threaded top-down through Mount/Update (an element's own RequestedTheme wins, // otherwise the subtree inherits its ancestor's). ElementTheme.Default means "no @@ -2662,7 +2670,7 @@ private void ReconcileComponent(Element oldEl, Element newEl, UIElement control, receiver.SetProps(compEl.Props); } - node.Component.Context.BeginRender(componentRerender, _contextScope); + node.Component.Context.BeginRender(componentRerender, _contextScope, OwningWindow); using (ValidationRenderScope.Begin(ReadContext(ValidationContexts.Current))) { newChildElement = ValidationRenderScope.ApplyProvide(node.Component.Render()); @@ -2672,7 +2680,7 @@ private void ReconcileComponent(Element oldEl, Element newEl, UIElement control, } else if (node.Context is not null && newEl is FuncElement func) { - node.Context.BeginRender(componentRerender, _contextScope); + node.Context.BeginRender(componentRerender, _contextScope, OwningWindow); using (ValidationRenderScope.Begin(ReadContext(ValidationContexts.Current))) { newChildElement = ValidationRenderScope.ApplyProvide(func.RenderFunc(node.Context)); @@ -2682,7 +2690,7 @@ private void ReconcileComponent(Element oldEl, Element newEl, UIElement control, } else if (node.Context is not null && newEl is MemoElement memo) { - node.Context.BeginRender(componentRerender, _contextScope); + node.Context.BeginRender(componentRerender, _contextScope, OwningWindow); using (ValidationRenderScope.Begin(ReadContext(ValidationContexts.Current))) { newChildElement = ValidationRenderScope.ApplyProvide(memo.RenderFunc(node.Context)); diff --git a/src/Reactor/Core/RenderContext.cs b/src/Reactor/Core/RenderContext.cs index d34ba04c6..be2fd5acc 100644 --- a/src/Reactor/Core/RenderContext.cs +++ b/src/Reactor/Core/RenderContext.cs @@ -16,9 +16,11 @@ public sealed class RenderContext private ContextScope? _contextScope; private int _uiThreadId; - // The window whose host last rendered this context. Picker calls run from event - // handlers, outside render, where ReactorApp.ActiveHostInternal is whichever host - // rendered or was constructed most recently (or null after any host is disposed). + // The window whose host last rendered this context, supplied by that host's reconciler. + // Picker calls run from event handlers, outside render, so they cannot read the + // process-wide ReactorApp.ActiveHostInternal: it names whichever host rendered or was + // constructed most recently (a window opened mid-render overwrites it), is not scoped + // for virtualized-row renders, and is null after any host is disposed. private Microsoft.UI.Reactor.ReactorWindow? _owningWindow; /// @@ -29,20 +31,26 @@ public sealed class RenderContext internal global::System.TimeProvider TimeProvider { get; set; } = global::System.TimeProvider.System; internal void BeginRender(Action requestRerender) + => BeginRender(requestRerender, owningWindow: null); + + internal void BeginRender(Action requestRerender, Microsoft.UI.Reactor.ReactorWindow? owningWindow) { _hookIndex = 0; _requestRerender = requestRerender; _uiThreadId = Environment.CurrentManagedThreadId; - _owningWindow = Microsoft.UI.Reactor.ReactorApp.ActiveHostInternal?.OwningWindow; + _owningWindow = owningWindow; } internal void BeginRender(Action requestRerender, ContextScope contextScope) + => BeginRender(requestRerender, contextScope, owningWindow: null); + + internal void BeginRender(Action requestRerender, ContextScope contextScope, Microsoft.UI.Reactor.ReactorWindow? owningWindow) { _hookIndex = 0; _requestRerender = requestRerender; _contextScope = contextScope; _uiThreadId = Environment.CurrentManagedThreadId; - _owningWindow = Microsoft.UI.Reactor.ReactorApp.ActiveHostInternal?.OwningWindow; + _owningWindow = owningWindow; } /// diff --git a/src/Reactor/Hosting/ReactorHost.cs b/src/Reactor/Hosting/ReactorHost.cs index 6a0d75914..9adf40b33 100644 --- a/src/Reactor/Hosting/ReactorHost.cs +++ b/src/Reactor/Hosting/ReactorHost.cs @@ -181,7 +181,11 @@ public sealed class ReactorHost : IDisposable, Core.Diagnostics.IReactorDiagnost public ReactorWindow? OwningWindow { get => Volatile.Read(ref _owningWindow); - internal set => Volatile.Write(ref _owningWindow, value); + internal set + { + Volatile.Write(ref _owningWindow, value); + _reconciler.OwningWindow = value; + } } private ReactorWindow? _owningWindow; @@ -1003,7 +1007,7 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) // root and then throw, and the failure belongs to the component that threw. if (_rootComponent is { } renderingRoot) { - renderingRoot.Context.BeginRender(rerender); + renderingRoot.Context.BeginRender(rerender, OwningWindow); try { using (ValidationRenderScope.Begin(null)) @@ -1038,7 +1042,7 @@ void RecoverFromHookOrder(HookOrderException ex, RenderContext ctx, string mode) } else if (_rootRenderFunc is not null && _funcContext is not null) { - _funcContext.BeginRender(rerender); + _funcContext.BeginRender(rerender, OwningWindow); try { using (ValidationRenderScope.Begin(null)) diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs index 8f2d557db..4ce6ec491 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs @@ -49,6 +49,11 @@ private sealed class PickerComponent : Component public Func>> PickFolders = null!; public Func> PickSave = null!; + // Nested class and function components render through the reconciler's + // BeginRender overload, not the root one, so they carry their own owner. + public PickerProbe ChildProbe { get; } = new(); + public PickerProbe FuncProbe { get; } = new(); + public override Element Render() { // Component forwarding for files, the RenderContext entry point for folders. @@ -58,7 +63,67 @@ public override Element Render() PickFolders = () => Context.PickMultipleFoldersAsync(FolderOptions); PickSave = () => PickSaveFileAsync(SaveOptions); Operations = [() => PickSingle(), () => PickMultiple(), () => PickFolder(), () => PickFolders(), () => PickSave()]; - return TextBlock("pickers"); + var funcProbe = FuncProbe; + return VStack( + TextBlock("pickers"), + Component(ChildProbe), + RenderEachTime(ctx => + { + funcProbe.Pick = () => ctx.PickFileAsync(new FilePickerOptions()); + return TextBlock("function picker"); + })); + } + } + + internal sealed class PickerProbe + { + public Func? Pick; + public ReactorWindow? ActiveAtRender; + } + + private sealed class PickerChild : Component + { + public override Element Render() + { + Props.Pick = () => PickFileAsync(new FilePickerOptions()); + return TextBlock("child picker"); + } + } + + // Opens a window from inside its own render, which constructs another ReactorHost + // and so repoints ReactorApp.ActiveHostInternal in the middle of the opener's pass. + private sealed class MidRenderOpener : Component + { + public override Element Render() + { + UseOpenWindow(WindowKey.Of(MidRenderWindowKey), new WindowSpec { Title = "Pickers C", Width = 240, Height = 180 }, + () => new PickerComponent()); + return TextBlock("opener"); + } + } + + private const string MidRenderWindowKey = "pickers-mid-render"; + + private sealed class MidRenderRoot : Component + { + public PickerProbe SiblingProbe { get; } = new(); + public Action? OpenNow; + + public override Element Render() + { + var (open, setOpen) = UseState(false); + OpenNow = () => setOpen(true); + var probe = SiblingProbe; + return VStack( + open ? Component() : TextBlock("closed"), + // Renders on every pass, after the opener, so it records whatever owner + // is current once the other window exists. + RenderEachTime(ctx => + { + probe.Pick = () => ctx.PickFileAsync(new FilePickerOptions()); + probe.ActiveAtRender = ReactorApp.ActiveHostInternal?.OwningWindow; + return TextBlock("sibling picker"); + })); } } @@ -68,8 +133,8 @@ private sealed class StubPickerService : IPickerService public object? Options; public string? Operation; public int Calls; - public IReadOnlyList Files = new List(); - public IReadOnlyList Folders = new List(); + public readonly IReadOnlyList Files = new List(); + public readonly IReadOnlyList Folders = new List(); public Exception? Error; private Task Record(WindowId owner, object options, string operation, T result) @@ -151,9 +216,9 @@ public override async Task RunAsync() H.Check($"Pickers_OffUiThread_{OperationNames[i]}", rejected && stub.Calls == calls); var error = new InvalidOperationException("picker failure"); stub.Error = error; - Exception? actual = null; + InvalidOperationException? actual = null; try { await component.Operations[i](); } - catch (Exception ex) { actual = ex; } + catch (InvalidOperationException ex) { actual = ex; } H.Check($"Pickers_Error_{OperationNames[i]}", ReferenceEquals(actual, error)); stub.Error = null; } @@ -237,6 +302,8 @@ public override async Task RunAsync() } await b.Operations[0](); H.Check("Pickers_OwningWindow_BRoutesToB", stub.Owner == ownerB); + await CheckNested("BeforeClose", a, ownerA); + await CheckNested("BeforeCloseB", b, ownerB); windowB.Close(); await Harness.Render(100); @@ -245,12 +312,14 @@ public override async Task RunAsync() for (int i = 0; i < a.Operations.Length; i++) { var calls = stub.Calls; - Exception? error = null; + // The owner lookup reports a missing or closed owner as InvalidOperationException. + InvalidOperationException? error = null; try { await a.Operations[i](); } - catch (Exception ex) { error = ex; } + catch (InvalidOperationException ex) { error = ex; } H.Check($"Pickers_OwningWindow_AfterClose_{OperationNames[i]}", error is null && stub.Calls == calls + 1 && stub.Operation == OperationNames[i] && stub.Owner == ownerA); } + await CheckNested("AfterClose", a, ownerA); // B's component outlived its window: its pickers must refuse rather than // parent a dialog to a closed (or unrelated) window. @@ -263,6 +332,21 @@ public override async Task RunAsync() H.Check($"Pickers_OwningWindow_ClosedOwnerRejected_{OperationNames[i]}", message is not null && message.Contains("owning ReactorWindow", StringComparison.Ordinal) && stub.Calls == calls); } + + // Replacing A's root retires the old root's context (DetachFromHost): a retained + // instance must stop opening pickers over the window it no longer renders into. + windowA.Host.Mount(new PickerComponent()); + await windowA.Host.WaitForIdleAsync(); + await Harness.Render(100); + for (int i = 0; i < a.Operations.Length; i++) + { + var calls = stub.Calls; + string? message = null; + try { await a.Operations[i](); } + catch (InvalidOperationException ex) { message = ex.Message; } + H.Check($"Pickers_OwningWindow_RetiredRootRejected_{OperationNames[i]}", + message == $"{OperationName(i)} requires an owning ReactorWindow." && stub.Calls == calls); + } } finally { @@ -271,6 +355,67 @@ public override async Task RunAsync() windowA?.Close(); await Harness.Render(100); } + + async Task CheckNested(string phase, PickerComponent component, WindowId owner) + { + stub.Owner = default; + await component.ChildProbe.Pick!(); + H.Check($"Pickers_OwningWindow_{phase}_ChildComponent", stub.Operation == "single" && stub.Owner == owner); + stub.Owner = default; + await component.FuncProbe.Pick!(); + H.Check($"Pickers_OwningWindow_{phase}_FunctionComponent", stub.Operation == "single" && stub.Owner == owner); + } + } + + private static string OperationName(int i) => i switch + { + 0 => "PickFileAsync", + 1 => "PickMultipleFilesAsync", + 2 => "PickFolderAsync", + 3 => "PickMultipleFoldersAsync", + _ => "PickSaveFileAsync", + }; + } + + /// + /// Opening a window during another window's render constructs a host, which repoints + /// ReactorApp.ActiveHostInternal for the rest of the opener's pass. Components that + /// render after it in the opener's window must still record the opener's window as owner. + /// + internal sealed class OwnerSurvivesMidRenderOpen(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + ReactorApp.UIDispatcher ??= DispatcherQueue.GetForCurrentThread(); + ReactorApp.ShutdownPolicy = ShutdownPolicy.Explicit; + var previous = RenderContext.PickerService; + var stub = new StubPickerService(); + RenderContext.PickerService = stub; + var root = new MidRenderRoot(); + ReactorWindow? windowA = null; + try + { + windowA = await Open("Pickers mid-render", root); + root.OpenNow!(); + await windowA.Host.WaitForIdleAsync(); + await Harness.Render(100); + var windowC = ReactorApp.FindWindow(WindowKey.Of(MidRenderWindowKey)); + // Oracle guards: the other window exists, and while the sibling rendered the + // process-wide host named it rather than A, so recording A is not something a + // global lookup could produce. + H.Check("Pickers_MidRenderOpen_OtherWindowOpened", windowC is not null && !ReferenceEquals(windowC, windowA)); + H.Check("Pickers_MidRenderOpen_ActiveHostMovedDuringRender", windowC is not null && ReferenceEquals(root.SiblingProbe.ActiveAtRender, windowC)); + + await root.SiblingProbe.Pick!(); + H.Check("Pickers_MidRenderOpen_SiblingKeepsOwner", stub.Operation == "single" && stub.Owner == windowA.AppWindow.Id); + } + finally + { + RenderContext.PickerService = previous; + ReactorApp.FindWindow(WindowKey.Of(MidRenderWindowKey))?.Close(); + windowA?.Close(); + await Harness.Render(100); + } } } } diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 7019b5223..9e2334365 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -1539,6 +1539,7 @@ internal static class SelfTestFixtureRegistry "FilePicker_ThrowsOffUiThread", "Pickers_Suite", "Pickers_OwningWindow", + "Pickers_OwnerSurvivesMidRenderOpen", "AspectRatio_Suite_Math", "AspectRatio_Suite_Conform", "WindowStyle_ToolWindowExStyleRemoved", @@ -3624,6 +3625,7 @@ internal static string[] StaleTierDeclarations() => "FilePicker_ThrowsOffUiThread" => new Phase7WindowingFixtures.FilePickerThrowsOffUiThread(harness), "Pickers_Suite" => new PickersFixtures.Suite(harness), "Pickers_OwningWindow" => new PickersFixtures.OwningWindow(harness), + "Pickers_OwnerSurvivesMidRenderOpen" => new PickersFixtures.OwnerSurvivesMidRenderOpen(harness), "UseSpec054Hooks_Suite" => new Phase7WindowingFixtures.UseSpec054HooksSuite(harness), // Spec 036/045 — issue #647 multi-window teardown hardening regression. "Window647_PrimaryElectionExcludesAuxiliary" => new WindowLifecycleHardeningFixtures.PrimaryElectionExcludesAuxiliary(harness), From 73a595785a583bb1f46f342bb876048282386a69 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 21 Oct 2026 09:50:52 -0700 Subject: [PATCH 3/5] Address review round 1 Clear a context's picker owner when its cleanups run, so nested class, function and memo components that unmount through the reconciler (not DetachFromHost) stop opening pickers over a window they no longer render into. Cleared before the cleanups so a throwing cleanup cannot skip it. Pickers_OwningWindow now replaces A's root with an unrelated tree and checks retained child and function delegates are rejected. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 890c9be9-295c-487f-a96e-93eaa64994b0 --- src/Reactor/Core/RenderContext.cs | 6 ++++++ .../SelfTest/Fixtures/PickersFixtures.cs | 21 ++++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Reactor/Core/RenderContext.cs b/src/Reactor/Core/RenderContext.cs index be2fd5acc..fa65e6164 100644 --- a/src/Reactor/Core/RenderContext.cs +++ b/src/Reactor/Core/RenderContext.cs @@ -2483,6 +2483,12 @@ internal void FlushEffects() private void RunCleanupsCore(Action? onCleanupError) { + // Cleanups run when this context unmounts or is rolled back (nested components + // included), so it no longer renders into its window: a retained delegate must not + // open a picker over it. Cleared first so a throwing cleanup cannot skip it; a + // context that renders again (hot-reload reset) re-records it in BeginRender. + _owningWindow = null; + // Phase 1: Run effect cleanups. Drain BOTH the committed cleanup and any // staged-but-not-yet-flushed cleanup: when a render changes an effect's // deps it moves the old cleanup into PendingCleanup (to run at the next diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs index 4ce6ec491..0cb5c0afa 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs @@ -104,6 +104,13 @@ public override Element Render() private const string MidRenderWindowKey = "pickers-mid-render"; + // A replacement root whose tree shares nothing with PickerComponent's, so the old + // root's nested components unmount instead of being reused in place. + private sealed class ReplacementRoot : Component + { + public override Element Render() => TextBlock("replaced"); + } + private sealed class MidRenderRoot : Component { public PickerProbe SiblingProbe { get; } = new(); @@ -335,7 +342,7 @@ public override async Task RunAsync() // Replacing A's root retires the old root's context (DetachFromHost): a retained // instance must stop opening pickers over the window it no longer renders into. - windowA.Host.Mount(new PickerComponent()); + windowA.Host.Mount(new ReplacementRoot()); await windowA.Host.WaitForIdleAsync(); await Harness.Render(100); for (int i = 0; i < a.Operations.Length; i++) @@ -347,6 +354,18 @@ public override async Task RunAsync() H.Check($"Pickers_OwningWindow_RetiredRootRejected_{OperationNames[i]}", message == $"{OperationName(i)} requires an owning ReactorWindow." && stub.Calls == calls); } + // The replaced root's nested class and function components unmount through the + // reconciler's cleanup path, not DetachFromHost; their retained delegates must + // refuse too, even though window A is still open. + foreach (var (name, probe) in new[] { ("ChildComponent", a.ChildProbe), ("FunctionComponent", a.FuncProbe) }) + { + var calls = stub.Calls; + string? message = null; + try { await probe.Pick!(); } + catch (InvalidOperationException ex) { message = ex.Message; } + H.Check($"Pickers_OwningWindow_RetiredRoot{name}Rejected", + message == "PickFileAsync requires an owning ReactorWindow." && stub.Calls == calls); + } } finally { From 78f803ca23b52f2fb838040fac59b9302bb6d97b Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 21 Oct 2026 12:06:52 -0700 Subject: [PATCH 4/5] Address pr-review findings (second pass) - Picker owner errors now say why: a component never rendered in a ReactorWindow (ReactorHostControl content, with the workaround), one no longer mounted (owner released on unmount or root retirement), or one whose window closed. - Pickers_OwningWindow re-renders A while B is the active host, so class, function and memo children go through the reconciler's update path, adds a memo probe and a row realized by a LazyVStack outside the render pass, and asserts exact rejection messages. - New Pickers_ClosedWindowRejectedDuringCleanup reaches the disposed-window branch from a sibling's cleanup during ReactorWindow.Dispose(). - Skills and Pick*Async XML docs describe the owner as the window whose host renders the component (not the one that last rendered it); the guide's ReactorHostControl workaround leads with the host window's AppWindow.Id. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 890c9be9-295c-487f-a96e-93eaa64994b0 --- docs/_pipeline/templates/windows.md.dt | 13 +- docs/guide/windows.md | 13 +- .../reactor/skills/reactor-windowing/SKILL.md | 8 +- skills/windowing.md | 6 +- src/Reactor/Core/RenderContext.cs | 28 ++- .../SelfTest/Fixtures/PickersFixtures.cs | 166 ++++++++++++++++-- .../SelfTest/SelfTestFixtureRegistry.cs | 2 + tests/Reactor.Tests/PickerTests.cs | 10 +- 8 files changed, 207 insertions(+), 39 deletions(-) diff --git a/docs/_pipeline/templates/windows.md.dt b/docs/_pipeline/templates/windows.md.dt index da7757199..fac9e91ba 100644 --- a/docs/_pipeline/templates/windows.md.dt +++ b/docs/_pipeline/templates/windows.md.dt @@ -602,17 +602,20 @@ Caveats: `Task.Run`. Their `Pick*` names keep the hook-rules analyzer quiet in handlers. - The owner is the window whose host rendered the component (including nested and virtualized-row components), not whichever window is active when the handler - runs. If that window has closed, or the component's root was replaced, the call - throws `InvalidOperationException`. + runs. Once the component unmounts (including when its root is replaced) or its + window closes, the call throws `InvalidOperationException`. - Option validation (missing or duplicate file-type choices) throws `ArgumentException` before any dialog opens. Windows validates extension syntax and folder paths. - Picker errors propagate to the caller; cancellation is not an error. - Framework tests use an internal picker-service seam instead of opening modal dialogs. - Content hosted through `ReactorHostControl` (or a `ReactorHost` you construct yourself) has no owning `ReactorWindow`, so these methods throw - `InvalidOperationException` there. Construct the native picker directly from the - hosting window instead, for example - `new Microsoft.Windows.Storage.Pickers.FileOpenPicker(xamlRoot.ContentIslandEnvironment.AppWindowId)`. + `InvalidOperationException` there. Construct the native picker from the window + that hosts the control instead, for example + `new Microsoft.Windows.Storage.Pickers.FileOpenPicker(hostWindow.AppWindow.Id)`. + Code that only has an element can use its + `XamlRoot.ContentIslandEnvironment.AppWindowId` (for example from + `UseElementRef().Current?.XamlRoot`). ## WPF / UWP migration map diff --git a/docs/guide/windows.md b/docs/guide/windows.md index 0ccf1eac8..973b6feab 100644 --- a/docs/guide/windows.md +++ b/docs/guide/windows.md @@ -935,17 +935,20 @@ Caveats: `Task.Run`. Their `Pick*` names keep the hook-rules analyzer quiet in handlers. - The owner is the window whose host rendered the component (including nested and virtualized-row components), not whichever window is active when the handler - runs. If that window has closed, or the component's root was replaced, the call - throws `InvalidOperationException`. + runs. Once the component unmounts (including when its root is replaced) or its + window closes, the call throws `InvalidOperationException`. - Option validation (missing or duplicate file-type choices) throws `ArgumentException` before any dialog opens. Windows validates extension syntax and folder paths. - Picker errors propagate to the caller; cancellation is not an error. - Framework tests use an internal picker-service seam instead of opening modal dialogs. - Content hosted through `ReactorHostControl` (or a `ReactorHost` you construct yourself) has no owning `ReactorWindow`, so these methods throw - `InvalidOperationException` there. Construct the native picker directly from the - hosting window instead, for example - `new Microsoft.Windows.Storage.Pickers.FileOpenPicker(xamlRoot.ContentIslandEnvironment.AppWindowId)`. + `InvalidOperationException` there. Construct the native picker from the window + that hosts the control instead, for example + `new Microsoft.Windows.Storage.Pickers.FileOpenPicker(hostWindow.AppWindow.Id)`. + Code that only has an element can use its + `XamlRoot.ContentIslandEnvironment.AppWindowId` (for example from + `UseElementRef().Current?.XamlRoot`). ## WPF / UWP migration map diff --git a/plugins/reactor/skills/reactor-windowing/SKILL.md b/plugins/reactor/skills/reactor-windowing/SKILL.md index e30108e7b..a29e9367c 100644 --- a/plugins/reactor/skills/reactor-windowing/SKILL.md +++ b/plugins/reactor/skills/reactor-windowing/SKILL.md @@ -271,9 +271,11 @@ return VStack( `PickFileAsync` / `PickMultipleFilesAsync` / `PickFolderAsync` / `PickMultipleFoldersAsync` / `PickSaveFileAsync` return `PickFileResult` / `PickFolderResult` (`Microsoft.Windows.Storage.Pickers`, exposing `.Path`); cancellation -returns `null` or an empty list. The owner is the window that last rendered the component (not whichever window is -active), the dialog works in elevated apps, and the call throws `InvalidOperationException` if that window has -closed. `FileTypeChoices` is an ordered `IReadOnlyList` whose order sets the default entry; labels +returns `null` or an empty list. The owner is the window whose host renders the component (not whichever window is +active), the dialog works in elevated apps, and the call throws `InvalidOperationException` once the component has +unmounted or its window has closed. Inside `ReactorHostControl` there is no owning window, so the methods throw; build +the native picker from the hosting window's `AppWindow.Id` there. `FileTypeChoices` is an ordered +`IReadOnlyList` whose order sets the default entry; labels must be unique. Options also take `Title`, `SuggestedFolder`, `SuggestedStartFolder`, `InitialFileTypeIndex`, and (save) `DefaultFileExtension` / `ShowOverwritePrompt`. There is no arbitrary HWND parameter. diff --git a/skills/windowing.md b/skills/windowing.md index f6c8ce9f6..ca00ff37a 100644 --- a/skills/windowing.md +++ b/skills/windowing.md @@ -248,8 +248,10 @@ return VStack( ``` Results are `PickFileResult` / `PickFolderResult` (`Microsoft.Windows.Storage.Pickers`) exposing `.Path`; -cancellation returns `null` or an empty list. The dialog is owned by the window that last rendered the -component, works in elevated apps, and throws `InvalidOperationException` if that window has closed. +cancellation returns `null` or an empty list. The dialog is owned by the window whose host renders the +component, works in elevated apps, and throws `InvalidOperationException` once the component has unmounted +or its window has closed. Inside `ReactorHostControl` there is no owning window, so the methods throw; build +the native picker from the hosting window's `AppWindow.Id` there. `FileTypeChoices` is an ordered `IReadOnlyList`: order sets the default entry, labels must be unique. Options also take `Title`, `SuggestedFolder`, `SuggestedStartFolder`, `InitialFileTypeIndex`, and (save) `DefaultFileExtension` / `ShowOverwritePrompt`. Do not pass arbitrary HWNDs. diff --git a/src/Reactor/Core/RenderContext.cs b/src/Reactor/Core/RenderContext.cs index fa65e6164..b191332f9 100644 --- a/src/Reactor/Core/RenderContext.cs +++ b/src/Reactor/Core/RenderContext.cs @@ -16,12 +16,15 @@ public sealed class RenderContext private ContextScope? _contextScope; private int _uiThreadId; - // The window whose host last rendered this context, supplied by that host's reconciler. + // The window whose host renders this context, supplied by that host's reconciler. // Picker calls run from event handlers, outside render, so they cannot read the // process-wide ReactorApp.ActiveHostInternal: it names whichever host rendered or was // constructed most recently (a window opened mid-render overwrites it), is not scoped // for virtualized-row renders, and is null after any host is disposed. private Microsoft.UI.Reactor.ReactorWindow? _owningWindow; + // True once an owner was released by unmount or root retirement, so a picker call from a + // retained delegate can say so instead of pointing at hosting without a window. + private bool _owningWindowReleased; /// /// Time source used by time-based hooks (currently 's @@ -39,6 +42,7 @@ internal void BeginRender(Action requestRerender, Microsoft.UI.Reactor.ReactorWi _requestRerender = requestRerender; _uiThreadId = Environment.CurrentManagedThreadId; _owningWindow = owningWindow; + _owningWindowReleased = false; } internal void BeginRender(Action requestRerender, ContextScope contextScope) @@ -51,6 +55,7 @@ internal void BeginRender(Action requestRerender, ContextScope contextScope, Mic _contextScope = contextScope; _uiThreadId = Environment.CurrentManagedThreadId; _owningWindow = owningWindow; + _owningWindowReleased = false; } /// @@ -2055,7 +2060,7 @@ public Action UseWindowDragMove() /// Opens a single-file picker owned by this component's window. /// /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window - /// that last rendered this component. Cancellation returns null. + /// whose host renders this component; once the component unmounts, the call throws. Cancellation returns null. /// public Task PickFileAsync(FilePickerOptions options) { @@ -2068,7 +2073,7 @@ public Action UseWindowDragMove() /// Opens a multiple-file picker owned by this component's window. /// /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window - /// that last rendered this component. Cancellation returns an empty list. + /// whose host renders this component; once the component unmounts, the call throws. Cancellation returns an empty list. /// public Task> PickMultipleFilesAsync(FilePickerOptions options) { @@ -2081,7 +2086,7 @@ public Action UseWindowDragMove() /// Opens a single-folder picker owned by this component's window. /// /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window - /// that last rendered this component. Cancellation returns null. + /// whose host renders this component; once the component unmounts, the call throws. Cancellation returns null. /// public Task PickFolderAsync(FolderPickerOptions options) { @@ -2093,7 +2098,7 @@ public Action UseWindowDragMove() /// Opens a multiple-folder picker owned by this component's window. /// /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window - /// that last rendered this component. Cancellation returns an empty list. + /// whose host renders this component; once the component unmounts, the call throws. Cancellation returns an empty list. /// public Task> PickMultipleFoldersAsync(FolderPickerOptions options) { @@ -2105,7 +2110,7 @@ public Action UseWindowDragMove() /// Opens a save-file picker owned by this component's window. /// /// Not a hook: call from a UI-thread event handler, not during Render. The owner is the window - /// that last rendered this component. Cancellation returns null. Selecting a target does not + /// whose host renders this component; once the component unmounts, the call throws. Cancellation returns null. Selecting a target does not /// create or write the file; that is the caller's responsibility. /// public Task PickSaveFileAsync(SaveFilePickerOptions options) @@ -2118,10 +2123,13 @@ public Action UseWindowDragMove() private Microsoft.UI.WindowId GetPickerOwner(string operation) { - var win = _owningWindow - ?? throw new InvalidOperationException($"{operation} requires an owning ReactorWindow."); + var win = _owningWindow ?? throw new InvalidOperationException(_owningWindowReleased + ? $"{operation} was called from a component that is no longer mounted in a ReactorWindow." + : $"{operation} requires a component rendered in a ReactorWindow. Content hosted by ReactorHostControl, or by a ReactorHost you construct, has no owning window: construct the Microsoft.Windows.Storage.Pickers picker from the hosting window's AppWindow.Id instead."); + // Reachable when ReactorWindow.Dispose() runs component cleanups: it marks the window + // disposed before disposing the host, and contexts not yet cleaned up still hold it. if (win.IsDisposed) - throw new InvalidOperationException($"{operation} requires an open owning ReactorWindow."); + throw new InvalidOperationException($"{operation} was called after the component's ReactorWindow closed."); if (!win.NativeWindow.DispatcherQueue.HasThreadAccess) throw new InvalidOperationException($"{operation} must be called on the owning window's UI thread."); return win.AppWindow.Id; @@ -2487,6 +2495,7 @@ private void RunCleanupsCore(Action? onCleanupError) // included), so it no longer renders into its window: a retained delegate must not // open a picker over it. Cleared first so a throwing cleanup cannot skip it; a // context that renders again (hot-reload reset) re-records it in BeginRender. + _owningWindowReleased |= _owningWindow is not null; _owningWindow = null; // Phase 1: Run effect cleanups. Drain BOTH the committed cleanup and any @@ -2583,6 +2592,7 @@ internal void DetachFromHost() ClearHookState(); _requestRerender = null; _contextScope = null; + _owningWindowReleased |= _owningWindow is not null; _owningWindow = null; } diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs index 0cb5c0afa..9586d0689 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs @@ -49,13 +49,19 @@ private sealed class PickerComponent : Component public Func>> PickFolders = null!; public Func> PickSave = null!; - // Nested class and function components render through the reconciler's - // BeginRender overload, not the root one, so they carry their own owner. + // Nested class, function and memo components render through the reconciler's + // BeginRender overloads (mount and update), not the root one, so they carry their own + // owner. The row probe is realized by a virtualized list, outside the host's render pass. public PickerProbe ChildProbe { get; } = new(); public PickerProbe FuncProbe { get; } = new(); + public PickerProbe MemoProbe { get; } = new(); + public PickerProbe RowProbe { get; } = new(); + public Action Bump = null!; public override Element Render() { + var (revision, setRevision) = UseState(0); + Bump = () => setRevision(revision + 1); // Component forwarding for files, the RenderContext entry point for folders. PickSingle = () => PickFileAsync(OpenOptions); PickMultiple = () => PickMultipleFilesAsync(OpenOptions); @@ -64,14 +70,30 @@ public override Element Render() PickSave = () => PickSaveFileAsync(SaveOptions); Operations = [() => PickSingle(), () => PickMultiple(), () => PickFolder(), () => PickFolders(), () => PickSave()]; var funcProbe = FuncProbe; + var memoProbe = MemoProbe; + var rowProbe = RowProbe; + // The second row only exists after a bump, so it is realized by the list's layout + // after the window's render pass has restored the process-wide active host. + IReadOnlyList rows = revision == 0 ? ["row0"] : ["row0", "row1"]; return VStack( TextBlock("pickers"), - Component(ChildProbe), + // The revision is part of the props so a bump re-renders the child (update path). + Component(new(ChildProbe, revision)), RenderEachTime(ctx => { + funcProbe.Renders++; funcProbe.Pick = () => ctx.PickFileAsync(new FilePickerOptions()); return TextBlock("function picker"); - })); + }), + Memo(ctx => + { + memoProbe.Renders++; + memoProbe.Pick = () => ctx.PickFileAsync(new FilePickerOptions()); + return TextBlock("memo picker"); + }, revision), + LazyVStack(rows, x => x, (row, _) => row == "row1" + ? Component(new(rowProbe, 0)) + : TextBlock(row).Height(30)).Height(120)); } } @@ -79,17 +101,29 @@ internal sealed class PickerProbe { public Func? Pick; public ReactorWindow? ActiveAtRender; + public int Renders; } - private sealed class PickerChild : Component + private sealed record ChildProps(PickerProbe Probe, int Revision); + + private sealed class PickerChild : Component { public override Element Render() { - Props.Pick = () => PickFileAsync(new FilePickerOptions()); + var probe = Props.Probe; + probe.Renders++; + probe.ActiveAtRender = ReactorApp.ActiveHostInternal?.OwningWindow; + probe.Pick = () => PickFileAsync(new FilePickerOptions()); return TextBlock("child picker"); } } + internal static string ReleasedMessage(string operation) + => $"{operation} was called from a component that is no longer mounted in a ReactorWindow."; + + internal static string ClosedMessage(string operation) + => $"{operation} was called after the component's ReactorWindow closed."; + // Opens a window from inside its own render, which constructs another ReactorHost // and so repoints ReactorApp.ActiveHostInternal in the middle of the opener's pass. private sealed class MidRenderOpener : Component @@ -312,6 +346,25 @@ public override async Task RunAsync() await CheckNested("BeforeClose", a, ownerA); await CheckNested("BeforeCloseB", b, ownerB); + // Re-render A while B is the active host: the class, function and memo children + // now render through the reconciler's update path, and the list realizes a new + // row in layout, outside A's render pass. + var renders = (a.ChildProbe.Renders, a.FuncProbe.Renders, a.MemoProbe.Renders); + a.Bump(); + await windowA.Host.WaitForIdleAsync(); + var rowRealized = await Harness.WaitFor(() => a.RowProbe.Pick is not null, maxPasses: 20, perPassMs: 25); + H.Check("Pickers_OwningWindow_Rerendered", a.ChildProbe.Renders > renders.Item1 + && a.FuncProbe.Renders > renders.Item2 && a.MemoProbe.Renders > renders.Item3); + H.Check("Pickers_OwningWindow_ActiveHostStillB", ReferenceEquals(ReactorApp.ActiveHostInternal?.OwningWindow, windowB)); + await CheckNested("AfterRerender", a, ownerA); + H.Check("Pickers_OwningWindow_RowRealized", rowRealized); + if (rowRealized) + { + stub.Owner = default; + await a.RowProbe.Pick!(); + H.Check("Pickers_OwningWindow_VirtualizedRow", stub.Operation == "single" && stub.Owner == ownerA); + } + windowB.Close(); await Harness.Render(100); H.Check("Pickers_OwningWindow_ActiveHostClearedByClose", ReactorApp.ActiveHostInternal is null); @@ -336,8 +389,9 @@ public override async Task RunAsync() string? message = null; try { await b.Operations[i](); } catch (InvalidOperationException ex) { message = ex.Message; } + // Closing B unmounted its tree, so the owner was released before any call. H.Check($"Pickers_OwningWindow_ClosedOwnerRejected_{OperationNames[i]}", - message is not null && message.Contains("owning ReactorWindow", StringComparison.Ordinal) && stub.Calls == calls); + message == ReleasedMessage(OperationName(i)) && stub.Calls == calls); } // Replacing A's root retires the old root's context (DetachFromHost): a retained @@ -352,19 +406,19 @@ public override async Task RunAsync() try { await a.Operations[i](); } catch (InvalidOperationException ex) { message = ex.Message; } H.Check($"Pickers_OwningWindow_RetiredRootRejected_{OperationNames[i]}", - message == $"{OperationName(i)} requires an owning ReactorWindow." && stub.Calls == calls); + message == ReleasedMessage(OperationName(i)) && stub.Calls == calls); } // The replaced root's nested class and function components unmount through the // reconciler's cleanup path, not DetachFromHost; their retained delegates must // refuse too, even though window A is still open. - foreach (var (name, probe) in new[] { ("ChildComponent", a.ChildProbe), ("FunctionComponent", a.FuncProbe) }) + foreach (var (name, probe) in new[] { ("ChildComponent", a.ChildProbe), ("FunctionComponent", a.FuncProbe), ("MemoComponent", a.MemoProbe) }) { var calls = stub.Calls; string? message = null; try { await probe.Pick!(); } catch (InvalidOperationException ex) { message = ex.Message; } H.Check($"Pickers_OwningWindow_RetiredRoot{name}Rejected", - message == "PickFileAsync requires an owning ReactorWindow." && stub.Calls == calls); + message == ReleasedMessage("PickFileAsync") && stub.Calls == calls); } } finally @@ -383,10 +437,13 @@ async Task CheckNested(string phase, PickerComponent component, WindowId owner) stub.Owner = default; await component.FuncProbe.Pick!(); H.Check($"Pickers_OwningWindow_{phase}_FunctionComponent", stub.Operation == "single" && stub.Owner == owner); + stub.Owner = default; + await component.MemoProbe.Pick!(); + H.Check($"Pickers_OwningWindow_{phase}_MemoComponent", stub.Operation == "single" && stub.Owner == owner); } } - private static string OperationName(int i) => i switch + internal static string OperationName(int i) => i switch { 0 => "PickFileAsync", 1 => "PickMultipleFilesAsync", @@ -437,4 +494,89 @@ public override async Task RunAsync() } } } -} + + private sealed class CleanupPair + { + public readonly Func?[] Pick = new Func?[2]; + public readonly string?[] Messages = new string?[2]; + } + + private sealed record CleanupProps(CleanupPair Pair, int Index); + + // On unmount, calls its sibling's picker. Whichever sibling cleans up first still sees the + // other holding its owner, while the disposing window is already marked disposed. + private sealed class CleanupProbe : Component + { + public override Element Render() + { + var (pair, index) = (Props.Pair, Props.Index); + pair.Pick[index] = () => PickFileAsync(new FilePickerOptions()); + UseEffect(() => () => + { + + try { _ = pair.Pick[1 - index]?.Invoke(); } + catch (InvalidOperationException ex) { pair.Messages[index] = ex.Message; } + }, pair); + return TextBlock($"cleanup {index}"); + } + } + + private sealed class CleanupRoot(CleanupPair pair) : Component + { + public override Element Render() => VStack( + Component(new(pair, 0)), + Component(new(pair, 1))); + } + + /// + /// marks the window disposed before its host runs + /// component cleanups, so a cleanup can reach a sibling whose context still records that + /// window. The call must be refused. (A native close instead disposes the host from its own + /// Closed handler first, which releases every owner before the window is marked.) + /// + internal sealed class ClosedWindowRejectedDuringCleanup(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + ReactorApp.UIDispatcher ??= DispatcherQueue.GetForCurrentThread(); + ReactorApp.ShutdownPolicy = ShutdownPolicy.Explicit; + var previous = RenderContext.PickerService; + var stub = new StubPickerService(); + RenderContext.PickerService = stub; + var pair = new CleanupPair(); + ReactorWindow? window = null; + try + { + window = await Open("Pickers cleanup", new CleanupRoot(pair)); + + H.Check("Pickers_CleanupClose_Mounted", pair.Pick[0] is not null && pair.Pick[1] is not null); + var disposed = window; + window = null; + disposed.Dispose(); + await Harness.Render(100); + // Dispose() bypasses Window.Closed -> UnregisterWindow; clean up the registration + // and the still-live native window (same hygiene as TitleBar_DisposeNoClose). + ReactorApp.UnregisterWindow(disposed); + try { disposed.NativeWindow.Close(); } + catch (Exception ex) when (ex is ObjectDisposedException or InvalidOperationException or global::System.Runtime.InteropServices.COMException) + { + global::System.Diagnostics.Debug.WriteLine($"[Reactor] Pickers_ClosedWindowRejectedDuringCleanup cleanup close threw: {ex}"); + } + + var messages = pair.Messages.Where(m => m is not null).ToList(); + // One sibling reaches the disposed-window branch; the other, cleaned up second, + // finds its sibling's owner already released. Neither reaches the service. + H.Check("Pickers_CleanupClose_ClosedOwnerRejected", messages.Contains(ClosedMessage("PickFileAsync"))); + H.Check("Pickers_CleanupClose_BothRejected", messages.Count == 2 + && messages.All(m => m == ClosedMessage("PickFileAsync") || m == ReleasedMessage("PickFileAsync"))); + H.Check("Pickers_CleanupClose_NoPicker", stub.Calls == 0); + } + finally + { + RenderContext.PickerService = previous; + window?.Close(); + await Harness.Render(100); + } + } + } +} \ No newline at end of file diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 9e2334365..753d5e0e6 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -1540,6 +1540,7 @@ internal static class SelfTestFixtureRegistry "Pickers_Suite", "Pickers_OwningWindow", "Pickers_OwnerSurvivesMidRenderOpen", + "Pickers_ClosedWindowRejectedDuringCleanup", "AspectRatio_Suite_Math", "AspectRatio_Suite_Conform", "WindowStyle_ToolWindowExStyleRemoved", @@ -3626,6 +3627,7 @@ internal static string[] StaleTierDeclarations() => "Pickers_Suite" => new PickersFixtures.Suite(harness), "Pickers_OwningWindow" => new PickersFixtures.OwningWindow(harness), "Pickers_OwnerSurvivesMidRenderOpen" => new PickersFixtures.OwnerSurvivesMidRenderOpen(harness), + "Pickers_ClosedWindowRejectedDuringCleanup" => new PickersFixtures.ClosedWindowRejectedDuringCleanup(harness), "UseSpec054Hooks_Suite" => new Phase7WindowingFixtures.UseSpec054HooksSuite(harness), // Spec 036/045 — issue #647 multi-window teardown hardening regression. "Window647_PrimaryElectionExcludesAuxiliary" => new WindowLifecycleHardeningFixtures.PrimaryElectionExcludesAuxiliary(harness), diff --git a/tests/Reactor.Tests/PickerTests.cs b/tests/Reactor.Tests/PickerTests.cs index a86755607..4214be858 100644 --- a/tests/Reactor.Tests/PickerTests.cs +++ b/tests/Reactor.Tests/PickerTests.cs @@ -28,7 +28,7 @@ public void MissingWindowReportsTheRequestedOperation(string operation) // A context that never rendered inside a ReactorWindow host has no owner. var context = new RenderContext(); var error = Assert.Throws(() => Invoke(context, operation, validOptions: true)); - Assert.Equal($"{operation} requires an owning ReactorWindow.", error.Message); + Assert.Equal(NoOwnerMessage(operation), error.Message); } [Fact] @@ -80,7 +80,7 @@ public void OpenPickersAcceptMissingOrEmptyChoices() foreach (var options in new FilePickerOptions[] { new(), new() { FileTypeChoices = [] } }) { var error = Assert.Throws(() => { _ = new RenderContext().PickFileAsync(options); }); - Assert.Equal("PickFileAsync requires an owning ReactorWindow.", error.Message); + Assert.Equal(NoOwnerMessage("PickFileAsync"), error.Message); } } @@ -90,7 +90,7 @@ public void LabelsAreComparedOrdinally() // "Text" and "text" are distinct native map keys, so they are not duplicates. var options = new SaveFilePickerOptions([new("Text", [".txt"]), new("text", [".md"])]); var error = Assert.Throws(() => { _ = new RenderContext().PickSaveFileAsync(options); }); - Assert.Equal("PickSaveFileAsync requires an owning ReactorWindow.", error.Message); + Assert.Equal(NoOwnerMessage("PickSaveFileAsync"), error.Message); } [Fact] @@ -103,6 +103,10 @@ public void SaveOptionsDefaultToShowingTheOverwritePrompt() Assert.Equal("", options.DefaultFileExtension); } + // A context that never rendered in a ReactorWindow (ReactorHostControl content, or no host + // at all) gets the message that names the hosting limitation and its workaround. + private static string NoOwnerMessage(string operation) => + $"{operation} requires a component rendered in a ReactorWindow. Content hosted by ReactorHostControl, or by a ReactorHost you construct, has no owning window: construct the Microsoft.Windows.Storage.Pickers picker from the hosting window's AppWindow.Id instead."; private static void Invoke(RenderContext context, string operation, bool validOptions) { FilePickerOptions file = validOptions ? new() : null!; From 67fcd8399f2018d94483a6f1e03383d1ecf770eb Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Wed, 21 Oct 2026 12:44:32 -0700 Subject: [PATCH 5/5] Address review round 3 Refuse picker calls during either teardown order. A native close disposes the host from its own Window.Closed handler (running component cleanups) before ReactorWindow marks itself disposed, so a sibling's cleanup could still reach the picker service. ReactorWindow.IsClosingOrDisposed checks the window and its host. Pickers_ClosedWindowRejectedDuringCleanup now covers Close() as well as direct Dispose(). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 890c9be9-295c-487f-a96e-93eaa64994b0 --- src/Reactor/Core/RenderContext.cs | 7 +- src/Reactor/Hosting/ReactorHost.cs | 3 + src/Reactor/Hosting/ReactorWindow.cs | 5 +- .../SelfTest/Fixtures/PickersFixtures.cs | 64 +++++++++++-------- 4 files changed, 50 insertions(+), 29 deletions(-) diff --git a/src/Reactor/Core/RenderContext.cs b/src/Reactor/Core/RenderContext.cs index b191332f9..b84f12799 100644 --- a/src/Reactor/Core/RenderContext.cs +++ b/src/Reactor/Core/RenderContext.cs @@ -2126,9 +2126,10 @@ private Microsoft.UI.WindowId GetPickerOwner(string operation) var win = _owningWindow ?? throw new InvalidOperationException(_owningWindowReleased ? $"{operation} was called from a component that is no longer mounted in a ReactorWindow." : $"{operation} requires a component rendered in a ReactorWindow. Content hosted by ReactorHostControl, or by a ReactorHost you construct, has no owning window: construct the Microsoft.Windows.Storage.Pickers picker from the hosting window's AppWindow.Id instead."); - // Reachable when ReactorWindow.Dispose() runs component cleanups: it marks the window - // disposed before disposing the host, and contexts not yet cleaned up still hold it. - if (win.IsDisposed) + // Reachable while a closing window's host runs component cleanups: a sibling not yet + // cleaned up still holds the window. On a native close the host is disposed first, on + // a direct ReactorWindow.Dispose() the window is, so check both. + if (win.IsClosingOrDisposed) throw new InvalidOperationException($"{operation} was called after the component's ReactorWindow closed."); if (!win.NativeWindow.DispatcherQueue.HasThreadAccess) throw new InvalidOperationException($"{operation} must be called on the owning window's UI thread."); diff --git a/src/Reactor/Hosting/ReactorHost.cs b/src/Reactor/Hosting/ReactorHost.cs index 9adf40b33..3b2452cb3 100644 --- a/src/Reactor/Hosting/ReactorHost.cs +++ b/src/Reactor/Hosting/ReactorHost.cs @@ -60,6 +60,9 @@ public sealed class ReactorHost : IDisposable, Core.Diagnostics.IReactorDiagnost internal FrameworkElement? ThemeListenerElement => _themeListenerElement; private volatile bool _disposed; + // Set at the start of Dispose(), before component cleanups run. + internal bool IsDisposed => _disposed; + // Set when the owning window has raised Closed — i.e. the native window is // being torn down. Drives BackdropApplier.Reset so it does NOT write // Window.SystemBackdrop on an already-destroyed window, which AVs diff --git a/src/Reactor/Hosting/ReactorWindow.cs b/src/Reactor/Hosting/ReactorWindow.cs index a2f38f392..0b4a1dd9e 100644 --- a/src/Reactor/Hosting/ReactorWindow.cs +++ b/src/Reactor/Hosting/ReactorWindow.cs @@ -290,7 +290,10 @@ public sealed partial class ReactorWindow : IDisposable internal nint Hwnd => _hwnd; - internal bool IsDisposed => _disposed; + // True once teardown has started. The host is disposed first on a native close (its + // Window.Closed handler runs before this window's), and this window first on a direct + // Dispose(); component cleanups run during either, so both flags are needed. + internal bool IsClosingOrDisposed => _disposed || _host.IsDisposed; /// /// When true, this window is an auxiliary surface (e.g. a docking diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs index 9586d0689..805653f2f 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/PickersFixtures.cs @@ -529,10 +529,10 @@ public override Element Render() => VStack( } /// - /// marks the window disposed before its host runs - /// component cleanups, so a cleanup can reach a sibling whose context still records that - /// window. The call must be refused. (A native close instead disposes the host from its own - /// Closed handler first, which releases every owner before the window is marked.) + /// Component cleanups run while a window tears down, and a cleanup can reach a sibling whose + /// context still records that window. The call must be refused on both teardown paths: a + /// native close disposes the host (from its own Closed handler) before the window, and a + /// direct disposes the window before the host. /// internal sealed class ClosedWindowRejectedDuringCleanup(Harness h) : SelfTestFixtureBase(h) { @@ -543,40 +543,54 @@ public override async Task RunAsync() var previous = RenderContext.PickerService; var stub = new StubPickerService(); RenderContext.PickerService = stub; - var pair = new CleanupPair(); - ReactorWindow? window = null; try { - window = await Open("Pickers cleanup", new CleanupRoot(pair)); + await RunPath("Close", stub, w => w.Close()); + await RunPath("Dispose", stub, w => + { + w.Dispose(); + // Dispose() bypasses Window.Closed -> UnregisterWindow; clean up the + // registration and the still-live native window (as TitleBar_DisposeNoClose). + ReactorApp.UnregisterWindow(w); + try { w.NativeWindow.Close(); } + catch (Exception ex) when (ex is ObjectDisposedException or InvalidOperationException or global::System.Runtime.InteropServices.COMException) + { + global::System.Diagnostics.Debug.WriteLine($"[Reactor] Pickers_ClosedWindowRejectedDuringCleanup cleanup close threw: {ex}"); + } + }); + } + finally + { + RenderContext.PickerService = previous; + } + } - H.Check("Pickers_CleanupClose_Mounted", pair.Pick[0] is not null && pair.Pick[1] is not null); - var disposed = window; + private async Task RunPath(string path, StubPickerService stub, Action tearDown) + { + var pair = new CleanupPair(); + var calls = stub.Calls; + ReactorWindow? window = await Open($"Pickers cleanup {path}", new CleanupRoot(pair)); + try + { + H.Check($"Pickers_Cleanup{path}_Mounted", pair.Pick[0] is not null && pair.Pick[1] is not null); + var closing = window; window = null; - disposed.Dispose(); + tearDown(closing); await Harness.Render(100); - // Dispose() bypasses Window.Closed -> UnregisterWindow; clean up the registration - // and the still-live native window (same hygiene as TitleBar_DisposeNoClose). - ReactorApp.UnregisterWindow(disposed); - try { disposed.NativeWindow.Close(); } - catch (Exception ex) when (ex is ObjectDisposedException or InvalidOperationException or global::System.Runtime.InteropServices.COMException) - { - global::System.Diagnostics.Debug.WriteLine($"[Reactor] Pickers_ClosedWindowRejectedDuringCleanup cleanup close threw: {ex}"); - } var messages = pair.Messages.Where(m => m is not null).ToList(); - // One sibling reaches the disposed-window branch; the other, cleaned up second, - // finds its sibling's owner already released. Neither reaches the service. - H.Check("Pickers_CleanupClose_ClosedOwnerRejected", messages.Contains(ClosedMessage("PickFileAsync"))); - H.Check("Pickers_CleanupClose_BothRejected", messages.Count == 2 + // The sibling cleaned up first reaches the closing-window branch; the one cleaned + // up second finds its sibling's owner already released. Neither reaches the service. + H.Check($"Pickers_Cleanup{path}_ClosedOwnerRejected", messages.Contains(ClosedMessage("PickFileAsync"))); + H.Check($"Pickers_Cleanup{path}_BothRejected", messages.Count == 2 && messages.All(m => m == ClosedMessage("PickFileAsync") || m == ReleasedMessage("PickFileAsync"))); - H.Check("Pickers_CleanupClose_NoPicker", stub.Calls == 0); + H.Check($"Pickers_Cleanup{path}_NoPicker", stub.Calls == calls); } finally { - RenderContext.PickerService = previous; window?.Close(); await Harness.Render(100); } } } -} \ No newline at end of file +}