CmdPal: Add slideshow background mode and rotation settings#46452
CmdPal: Add slideshow background mode and rotation settings#46452Error0229 wants to merge 5 commits intomicrosoft:mainfrom
Conversation
…t#45879) Add a dedicated Slideshow background mode in CmdPal appearance settings with folder-based image rotation, configurable interval, and shuffle. - Add ColorizationMode.Slideshow enum value - Add slideshow folder path, change interval, and shuffle settings - Add slideshow rotation engine in ThemeService (sequential/shuffle, interval-based, rotate-on-hide so next activation shows new image) - Add BackgroundImagePathResolver utility with unit tests (13 tests) - Add folder picker and slideshow controls to AppearancePage - Fix settings crash on empty preview image paths - Add localized strings for all new UI elements Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Single-button StackPanel wrappers around the image picker and folder picker buttons serve no purpose. Use the Button directly as the SettingsCard content. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eImageUri Move the relative-path-to-absolute-URI logic out of the view model ternary chains into a dedicated helper. Both AppearanceSettingsViewModel and DockAppearanceSettingsViewModel now read as a simple null-check chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@microsoft-github-policy-service agree |
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AppearanceSettingsViewModel.cs
Fixed
Show fixed
Hide fixed
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AppearanceSettingsViewModel.cs
Fixed
Show fixed
Hide fixed
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AppearanceSettingsViewModel.cs
Fixed
Show fixed
Hide fixed
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AppearanceSettingsViewModel.cs
Fixed
Show fixed
Hide fixed
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AppearanceSettingsViewModel.cs
Fixed
Show fixed
Hide fixed
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Properties/Resources.Designer.cs
Fixed
Show fixed
Hide fixed
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Properties/Resources.resx
Fixed
Show fixed
Hide fixed
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/IThemeService.cs
Fixed
Show fixed
Hide fixed
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f715528 to
1d6734e
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Adds a new Slideshow background mode to CmdPal’s appearance settings, including folder selection and rotation behavior, so users can rotate background images from a directory instead of a single file.
Changes:
- Introduces
ColorizationMode.Slideshowplus new persisted settings for slideshow folder, change interval, and shuffle. - Updates settings UI to support slideshow selection (folder picker + interval/shuffle controls).
- Adds
BackgroundImagePathResolverutility (with unit tests) and wires it into theme/image preview paths.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ColorizationMode.cs | Adds Slideshow mode to the colorization enum. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/SettingsModel.cs | Persists slideshow folder, interval minutes, and shuffle flag. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/IThemeService.cs | Adds RefreshThemeForActivation() hook for summon-time refresh behavior. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Services/ThemeService.cs | Implements slideshow rotation/selection logic and integrates slideshow into theme snapshot generation. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs | Triggers slideshow refresh before hiding to avoid visible swaps on next activation. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/AppearancePage.xaml | Adds Slideshow option and slideshow-specific settings cards/controls. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/AppearancePage.xaml.cs | Implements folder picker and forces mode switch when picking file/folder. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AppearanceSettingsViewModel.cs | Adds slideshow settings bindings and preview image resolution logic. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/DockAppearanceSettingsViewModel.cs | Uses resolver-based URI creation for safer image preview resolution. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/MainWindowViewModel.cs | Applies theme snapshots on UI thread without always dispatching. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/BackgroundImagePathResolver.cs | New helper to resolve folder-vs-file paths, supported images, and safe image URIs. |
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/BackgroundImagePathResolverTests.cs | Adds unit tests covering resolver behaviors (folder/file/URI cases). |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Strings/en-us/Resources.resw | Adds localized strings for slideshow UI and interval/shuffle labels. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Properties/Resources.resx | Adds localized string used by folder picker commit button. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Properties/Resources.Designer.cs | Regenerates strongly-typed accessor for the new resx string. |
| .github/actions/spell-check/expect.txt | Adds “slideshow” to spelling allow-list and removes an unused entry. |
Files not reviewed (1)
- src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Properties/Resources.Designer.cs: Language not supported
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AppearanceSettingsViewModel.cs
Show resolved
Hide resolved
…trings Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| public void TryGetLocalFolderPath_WithNull_ReturnsFalse() | ||
| { | ||
| var ok = BackgroundImagePathResolver.TryGetLocalFolderPath(null, out var resolved); | ||
|
|
||
| Assert.IsFalse(ok); |
There was a problem hiding this comment.
Add a unit test that passes an invalid/malformed path (illegal characters or malformed URI) into TryGetLocalFolderPath and asserts it returns false without throwing. This matters because the resolver is invoked from UI-bound properties and theme reload code paths.
| var localCandidate = Path.GetFullPath(candidate); | ||
| if (!Directory.Exists(localCandidate)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| folderPath = localCandidate; | ||
| return true; |
There was a problem hiding this comment.
TryGetLocalFolderPath uses Path.GetFullPath(candidate) without guarding for ArgumentException / NotSupportedException (e.g., settings.json contains an invalid path). That will throw and can crash theme reload/settings UI. Wrap GetFullPath in a try/catch and return false on invalid paths.
| var localCandidate = Path.GetFullPath(candidate); | |
| if (!Directory.Exists(localCandidate)) | |
| { | |
| return false; | |
| } | |
| folderPath = localCandidate; | |
| return true; | |
| try | |
| { | |
| var localCandidate = Path.GetFullPath(candidate); | |
| if (!Directory.Exists(localCandidate)) | |
| { | |
| return false; | |
| } | |
| folderPath = localCandidate; | |
| return true; | |
| } | |
| catch (ArgumentException) | |
| { | |
| return false; | |
| } | |
| catch (NotSupportedException) | |
| { | |
| return false; | |
| } |
| public static readonly HashSet<string> SupportedImageExtensions = new(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| ".png", |
There was a problem hiding this comment.
SupportedImageExtensions is exposed as a public mutable HashSet<string>, so external callers can add/remove extensions at runtime and break filtering. Consider exposing an immutable view (e.g., IReadOnlyCollection<string> / string[]) and keeping the HashSet private for membership checks.
| /// Requests an immediate refresh for summon-time visuals. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Used for activation-driven updates such as background slideshow rotation. |
There was a problem hiding this comment.
RefreshThemeForActivation() is a new public interface member but doesn’t document thread-affinity. Since the implementation calls Reload() (which touches XAML/BitmapImage), please document that it must be invoked on the UI thread (or enforce via an assert/dispatcher marshal) to prevent future off-thread callers from causing crashes.
| /// Used for activation-driven updates such as background slideshow rotation. | |
| /// Used for activation-driven updates such as background slideshow rotation. | |
| /// This method is UI-thread-affine and must be invoked on the UI thread. | |
| /// Callers are responsible for marshalling to the UI thread (for example, via a dispatcher) | |
| /// before invoking this method. |
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
New mode: Slideshow
ColorizationMode.Slideshowis added alongside the existingImagemode. Both modes share tint/opacity/blur/fit controls, but have separate path controls — single-image uses a file picker (BackgroundImagePath), slideshow uses a folder picker (BackgroundImageSlideshowFolderPath).Rotation engine
ThemeServicemaintains internal state to track the current slideshow image. Rotation triggers on: folder changed, current file deleted, or activation after the configured interval elapsed. Supports both shuffle and sequential (alphabetical wrap-around) ordering.Shuffle algorithm
Uses Fisher-Yates to pre-shuffle the full index array, then walks through it with a cursor. When the deck is exhausted, reshuffles — with the last-shown image swapped away from position 0 to avoid repeats at deck boundaries. This guarantees every image is shown exactly once before any image repeats.
Rotation timing
MainWindow.HideWindow()callsRefreshThemeForActivation()before hiding, so the image swaps while invisible — no flicker on next activation.Settings
New JSON keys (additive, no migration needed):
BackgroundImageSlideshowFolderPath(string)BackgroundImageChangeIntervalMinutes(int, default 0 = on activation only)BackgroundImageShuffle(bool, default true)Validation Steps Performed
src/modules/cmdpal/Microsoft.CmdPal.UIwithtools/build/build.cmd(Debug/x64) — 0 errorsBackgroundImagePathResolverTests(13/13 passed)