[settings] Show uneditable shortcut in CmdPal page #38060
Conversation
stefansjfw
changed the title
stefansjfw
added
the
Product-Command Palette
There was a problem hiding this comment.
Pull Request Overview
This PR shows updates to display the uneditable shortcut on the Command Palette page. It introduces a new CmdPalProperties class to handle the hotkey, implements a file watcher to monitor settings changes, and updates the view models and views accordingly.
- Added CmdPalProperties for handling hotkey settings.
- Introduced a file watcher utility in Helper.cs to monitor changes in the settings file.
- Updated DashboardViewModel and CmdPalViewModel to use the hotkey and refresh its value, and injected a DispatcherQueue into the CmdPalPage.
Reviewed Changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/settings-ui/Settings.UI.Library/CmdPalProperties.cs | Introduces a new class for managing Command Palette hotkey settings. |
| src/settings-ui/Settings.UI.Library/Utilities/Helper.cs | Adds a new method to get a file watcher for settings changes. |
| src/settings-ui/Settings.UI/ViewModels/DashboardViewModel.cs | Updates module item initialization to use the hotkey shortcut. |
| src/settings-ui/Settings.UI/ViewModels/CmdPalViewModel.cs | Implements hotkey handling and file watcher integration with UI update logic. |
| src/settings-ui/Settings.UI/SettingsXAML/Views/CmdPalPage.xaml.cs | Injects required DispatcherQueue to the view model for hotkey updates. |
Files not reviewed (2)
- src/settings-ui/Settings.UI/SettingsXAML/Views/CmdPalPage.xaml: Language not supported
- src/settings-ui/Settings.UI/Strings/en-us/Resources.resw: Language not supported
| } | ||
| } | ||
|
|
||
| public HotkeySettings Hotkey |
There was a problem hiding this comment.
The setter for the Hotkey property is empty and does not store any value, which can lead to unexpected behavior if the property is ever set. Consider either implementing the setter to update _hotkey or removing it to make the property read-only.
|
Sorry... I don't get why. I mean when cmdpal is "enabled", the hotkey should be editable. |
| { | ||
| var localAppDataDir = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData); | ||
|
|
||
| #if DEBUG |
There was a problem hiding this comment.
i mean big picture, this isn't exactly right, but it'll work this week
|
|
||
| #pragma warning disable SA1401 // Fields should be private | ||
| #pragma warning disable CA1051 // Do not declare visible instance fields | ||
| public HotkeySettings Hotkey; |
There was a problem hiding this comment.
okay this is my c# knowledge being limited, but is there a reason to do it this way vs just
public HotkeySettings Hotkey { get; private set; }
CmdPal's settings live totally outside of PowerToys' own settings. We could in the future have PT read the CmdPal settings JSON, edit the single property, and write it back out, but that sounds too delicate to toss in at the eleventh hour We probably also in the future should add a link to open the CmdPal settings from the PT Run settings page. That's probably even more important (but also not something for 0.1) |
* [settings] Show CmdPal shortcut * show in dashboard as well   As noted in microsoft#37908
* [settings] Show CmdPal shortcut * show in dashboard as well   As noted in microsoft#37908
Related to #37908