[cmdpal] Implement GotoPage #38025
Open
[cmdpal] Implement GotoPage #38025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…l object so even if we create a new wrapper we'll compare it against what it actually contains.
Just some minor clean-up/perf things I was trying, doesn't actually fix anything though, unfortunately (outside of maybe making startup a bit faster)... Does NOT fix zadjii-msft#213 We were effectively getting called to load images twice in a few spots, and we were trying to load them everywhere at startup. This minimized that in a few places, but we can't use x:Phase in the DataTemplate for the List Items (as it needs a UIElement), so that caused some issues. I thought I had a fix for all the duplicate items loading on the home page, but alas no. Have to continue digging into that, but got side-tracked by the above.
This allows extension objects to write error messages specifically to the page that owns them. We don't need the `ShowExceptionMessage` anymore, because that just tossed the error at whatever page was open (even if it wasn't the page that had an error). I also made this a `IPageContext`, rather than `IErrorContext`, so we could pass the page's task scheduler to that object too. That lets us have one base `UpdateProperties` implementation for all extension objects. I think this sneakily also adds support for `Page.Title` (separate from `Page.Name`) Closes #203
…t rebuild every launch
…w an exception, that makes sense
Replaces PR #218 IconBox is a custom control that's a ContentControl, it's generic (toolkitable) and should be able to be styled and templated. It knows how to take an IconSource and create the underlying IconElement as its content. It can also take any general value as a `SourceKey` and via an implementation of the SourceRequested event, translate a bound general object into the `IconSource` required. This is how caching can be provided by an application as well, for instance (like we'll do here). This uses the deferred events pattern to await the call to the `SourceRequested` event which may need to load data asynchronously We create a static x:Bind helper `IconCacheProvider` to encapsulate our shared logic for our eventual Icon cache. Renamed IconCacheService.xaml.cs -> IconCacheService.cs Removed old broken behavior (believe ultimate issue was due to instability in loaded/unloaded events, i.e. issue microsoft/microsoft-ui-xaml#1900) XAML Styler also did its thing...
Fixes #213 Replaces PR #218 FYI @Ryken100 (thanks for the info and assist in debugging the issue and discussing possible avenues of resolution) Thanks @zadjii-msft for validating the end path in #218 Before: ```xml <Border x:Name="IconBorder" Grid.Column="0" Width="16" Height="16" Margin="0,0,0,0"> <!-- LoadIconBehavior will magically fill this border up with an icon --> <Interactivity:Interaction.Behaviors> <cmdpalUI:LoadIconBehavior Source="{x:Bind Icon, Mode=OneWay}"/> </Interactivity:Interaction.Behaviors> </Border> ``` After: ```xml <cpcontrols:IconBox Grid.Column="0" Width="16" Height="16" Margin="0,0,0,0" SourceKey="{x:Bind Icon, Mode=OneWay}" SourceRequested="{x:Bind help:IconCacheProvider.SourceRequested}" /> ``` The IconCacheProvider is the translation layer between having a light-weight control and our specific app's logic/desire for an icon cache, using the deferred event pattern: ```cs public static partial class IconCacheProvider { private static readonly IconCacheService IconService = new(Microsoft.UI.Dispatching.DispatcherQueue.GetForCurrentThread()); public static async void SourceRequested(IconBox sender, SourceRequestedEventArgs args) { if (args.Key == null) { return; } if (args.Key is IconDataType iconData) { var deferral = args.GetDeferral(); args.Value = await IconService.GetIconSource(iconData); deferral.Complete(); } } } ``` ## Details `IconBox` is a custom control that's a ContentControl, its generic (toolkitable) and should be able to be styled and templated (haven't tested, but no reason it shouldn't as a XAML `ContentControl`, should help @niels9001 a ton). It knows how to take an `IconSource` and create the underlying `IconElement` as its content. It can also take any general value as a `SourceKey` and via an implementation of the `SourceRequested` event, translate a bound general object into the `IconSource` required. This is how caching can be provided by an application as well, for instance (like we'll do here). This uses the deferred events pattern to await the call to the `SourceRequested` event which may need to load data asynchronously We create a static x:Bind helper `IconCacheProvider` to encapsulate our shared logic for our eventual Icon cache. Also: - Renamed IconCacheService.xaml.cs -> IconCacheService.cs - Removed old broken behavior (believe ultimate issue was due to instability in loaded/unloaded events, i.e. issue microsoft/microsoft-ui-xaml#1900) - XAML Styler also did its thing... (some conflict here in config from PowerToys to resolve later, imagine this is also a consequence of us not having CI setup in fork...)
Migrate Window Walker from PT Run
Get the other two types of pages back in. Form pages and Markdown pages.  When forms fail to parse, we'll display the exception by replacing the card with one of our one authoring  Markdown pages support multiple bodies, and possibly details:   Ref #73
Update the idl to allow us to specify separate FG and BG colors. I think that's better honestly.
There was a problem hiding this comment.
Pull Request Overview
This PR partially implements the GoToPage feature without supporting "Push" navigation mode, including view model setup and sample pages integration.
- Added GoToPageArgsViewModel for managing page arguments.
- Introduced HandleGoToPageArgs that synchronously initializes the view model and navigates based on mode.
- Extended the sample pages with a new SampleGoToPage to demonstrate navigation commands.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/GoToPageArgsViewModel.cs | Introduces view model for GoToPage arguments. |
| src/modules/cmdpal/Exts/SamplePagesExtension/Pages/SampleGoToPage.cs | Implements a sample page with multiple navigation commands. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml.cs | Adds handling for GoToPage arguments and modifies navigation behavior. |
| src/modules/cmdpal/Exts/SamplePagesExtension/SamplesListPage.cs | Integrates the new sample page into the samples list. |
src/modules/cmdpal/Exts/SamplePagesExtension/Pages/SampleGoToPage.cs
Outdated
Show resolved
Hide resolved
moooyo
added
Don't merge - Hold for release
|
@zadjii |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
moooyo
changed the title
zadjii-msft
requested changes
Mar 24, 2025
moooyo
removed
the
Don't merge - Hold for release
moooyo
removed
the
Needs-Review
moooyo
added
the
Needs-Review
zadjii-msft
removed
the
Needs-Review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
Implement the GotoPage to make extension can control their work flow.
demo here (sorry for misunderstanding before, I recreate a new one to meet the docs definition):
Video.Project.10.mp4
limitation:
If we want to support all pages in the future, we need to add a location to make extensions register their page which they want to jump to.
If not, we may trap in recursion (such as the SampleGotoPage in this PR).
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed