Repository navigation
Simplify Taskbar Widget options and refactor some settings pages - #556
Conversation
There was a problem hiding this comment.
Pull request overview
This PR simplifies the Taskbar Widget settings experience by removing rarely-changed options and wiring the widget behavior to always allow click-to-toggle, while also cleaning up some settings-page labeling and messaging.
Changes:
- Removed
TaskbarWidgetClickable/TaskbarWidgetCloseableFlyoutsettings and corresponding UI toggles; widget click now always toggles the Media Flyout. - Updated Next Up settings label to remove the “(Experimental)” marker (en-US).
- Downgraded the Taskbar Widget visual-issues InfoBar severity from Warning to Informational.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| FluentFlyoutWPF/ViewModels/UserSettings.cs | Removes the two Taskbar Widget boolean settings and their change handlers/defaults. |
| FluentFlyoutWPF/Resources/Localization/Dictionary-en-US.xaml | Removes strings for the deleted options; updates Next Up title text. |
| FluentFlyoutWPF/Pages/TaskbarWidgetPage.xaml | Removes the two toggle cards from the UI and changes InfoBar severity. |
| FluentFlyoutWPF/Controls/TaskbarWidgetControl.xaml.cs | Removes gating by the deleted settings; click now always toggles the flyout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private void Grid_MouseEnter(object sender, MouseEventArgs e) | ||
| { | ||
| if (!SettingsManager.Current.TaskbarWidgetClickable || String.IsNullOrEmpty(SongTitle.Text + SongArtist.Text)) return; | ||
| if (string.IsNullOrEmpty(SongTitle.Text + SongArtist.Text)) return; |
There was a problem hiding this comment.
string.IsNullOrEmpty(SongTitle.Text + SongArtist.Text) allocates a new concatenated string on every mouse enter, which can be relatively hot. Prefer checking the two Text values separately (or tracking a boolean like HasMedia set in UpdateUi) to avoid per-event allocations.
| if (string.IsNullOrEmpty(SongTitle.Text + SongArtist.Text)) return; | |
| if (string.IsNullOrEmpty(SongTitle.Text) && string.IsNullOrEmpty(SongArtist.Text)) return; |
| private void Grid_MouseLeave(object sender, MouseEventArgs e) | ||
| { | ||
| if (!SettingsManager.Current.TaskbarWidgetClickable || String.IsNullOrEmpty(SongTitle.Text + SongArtist.Text)) return; | ||
| if (string.IsNullOrEmpty(SongTitle.Text + SongArtist.Text)) return; |
There was a problem hiding this comment.
string.IsNullOrEmpty(SongTitle.Text + SongArtist.Text) allocates a new concatenated string on every mouse leave. Consider checking the two Text values separately (or using a cached boolean) to avoid unnecessary allocations during frequent pointer movement.
trueas most users will leave these on by default.(Experimental)label from Next Up settings page as this feature has been extensively tested and matured.