Mirrors/PowerToys
1
0
Fork 0
mirror of https://github.com/microsoft/PowerToys.git synced 2025-12-16 03:37:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

[Settings] Fix Dashboard toggle glitches and sorting UI (#43626)

Browse Source 
## Summary of the Pull Request
Fixes two UI bugs in the Settings Dashboard: module list glitching when
toggling modules, and incorrect sort menu checkmarks.

<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist

- [x] Closes: #43624 
- [x] Closes: #43625
- [ ] **Communication:** I've discussed this with core contributors
already. If the work hasn't been agreed, this work might be rejected
- [ ] **Tests:** Added/updated and all pass
- [ ] **Localization:** All end-user-facing strings can be localized
- [ ] **Dev docs:** Added/updated
- [ ] **New binaries:** Added on the required places
- [ ] [JSON for
signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json)
for new binaries
- [ ] [WXS for
installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs)
for new binaries and localization folder
- [ ] [YML for CI
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml)
for new test projects
- [ ] [YML for signed
pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml)
- [ ] **Documentation updated:** If checked, please file a pull request
on [our docs
repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys)
and link it here: #xxx

<!-- Provide a more detailed description of the PR, other things fixed,
or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments

### User-Facing Fixes

#### 1. Module list glitching when toggling enabled state
When enabling or disabling a module from the "Utilities" list, the
entire list would flicker and redraw, causing other toggles to glitch.
This made it appear as if multiple modules were being affected by a
single change.

**Root cause**
The `AllModules` ObservableCollection was being completely cleared and
re-populated on every change, forcing the UI to destroy and recreate all
list items.

**Fix**
Refactored collection updates to modify items in-place:
- Introduced `_moduleItems` master list, built once during
initialization.
- `RefreshModuleList()` now updates properties without clearing
collections
- `SortModuleList()` uses `ObservableCollection.Move()` instead of
`Clear()`/`Add()`

#### 2. Incorrect sort menu checkmark behaviour
The checkmark in the "Sort by" menu would not update correctly when
changing sort order, sometimes showing the incorrect item checked, or
even both at once.

**Root cause**
The `IsChecked` prop on the `ToggleMenuFlyoutItem` is bound to
`DashboardSortOrder`, but the binding was not updating because the
ViewModel didn't raise a property change notification when the sort
order was changed.

**Fix**
Added `OnPropertyChanged(nameof(DashboardSortOrder))` in
`SortModuleList()`.

### Code quality improvements

1. Renamed `GetShortcutModules()` to `RefreshShortcutModules()`. The
original name implied a getter, but the routine actually affects state
by rebuilding the shortcut and action lists, violating the Command-Query
Separation principle.
2. Added an `_isUpdatingFromUI` flag as a defensive measure against
circular updates when a UI toggle is changed.
3. Separation of concerns for operations on the modules list. Building,
sorting and refreshing it are separated.
4. Added comments and XML doc headers for new methods. Included brief
description of GPO locking behaviour.

<!-- Describe how you validated the behavior. Add automated tests
wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
- Verified that toggling modules in the list no longer causes the list
to flicker or for other toggles to glitch.
- Confirmed that the sort order checkmarks update correctly and reflect
the current sort order.
- Tested GPO policy settings are still queried as before.
- Checked sort behaviour is unaffected.

## Videos

*Sorting UI*

https://github.com/user-attachments/assets/3484bf63-2946-4460-83a5-361fa7e41c82

*Toggle behaviour*

https://github.com/user-attachments/assets/1fae5429-6fa3-4431-80f3-0907dab4f326

---------

Co-authored-by: Gordon Lam (SH) <yeelam@microsoft.com>
This commit is contained in:
Dave Rayment
2025-11-19 08:49:40 +00:00
committed by GitHub
parent 46242b384e
commit 97d46efec2
1 changed files with 141 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

173
src/settings-ui/Settings.UI/ViewModels/DashboardViewModel.cs
View File

@@ -40,6 +40,12 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
public ObservableCollection<DashboardListItem> ActionModules { get; set; } = new ObservableCollection<DashboardListItem>();
// Master list of module items that is sorted and projected into AllModules.
private List<DashboardListItem> _moduleItems = new List<DashboardListItem>();
// Flag to prevent circular updates when a UI toggle triggers settings changes.
private bool _isUpdatingFromUI;
private AllHotkeyConflictsData _allHotkeyConflictsData = new AllHotkeyConflictsData();
public AllHotkeyConflictsData AllHotkeyConflictsData
@@ -74,7 +80,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
generalSettingsConfig.DashboardSortOrder = value;
OutGoingGeneralSettings outgoing = new OutGoingGeneralSettings(generalSettingsConfig);
SendConfigMSG(outgoing.ToString());
RefreshModuleList();
SortModuleList();
}
}
}
@@ -96,8 +102,9 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
// set the callback functions value to handle outgoing IPC message.
SendConfigMSG = ipcMSGCallBackFunc;
RefreshModuleList();
GetShortcutModules();
BuildModuleList();
SortModuleList();
RefreshShortcutModules();
}
protected override void OnConflictsUpdated(object sender, AllHotkeyConflictsEventArgs e)
@@ -129,11 +136,13 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
GlobalHotkeyConflictManager.Instance?.RequestAllConflicts();
}
private void RefreshModuleList()
/// <summary>
/// Builds the master list of module items. Called once during initialization.
/// Each module item contains its configuration, enabled state, and GPO lock status.
/// </summary>
private void BuildModuleList()
{
AllModules.Clear();
var moduleItems = new List<DashboardListItem>();
_moduleItems.Clear();
foreach (ModuleType moduleType in Enum.GetValues<ModuleType>())
{
@@ -149,47 +158,143 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
DashboardModuleItems = GetModuleItems(moduleType),
};
newItem.EnabledChangedCallback = EnabledChangedOnUI;
moduleItems.Add(newItem);
}
// Sort based on current sort order
var sortedItems = DashboardSortOrder switch
{
DashboardSortOrder.ByStatus => moduleItems.OrderByDescending(x => x.IsEnabled).ThenBy(x => x.Label),
_ => moduleItems.OrderBy(x => x.Label), // Default alphabetical
};
foreach (var item in sortedItems)
{
AllModules.Add(item);
_moduleItems.Add(newItem);
}
}
/// <summary>
/// Sorts the module list according to the current sort order and updates the AllModules collection.
/// On first call, populates AllModules. On subsequent calls, uses Move() to reorder items in-place
/// to avoid destroying and recreating UI elements.
/// </summary>
private void SortModuleList()
{
var sortedItems = (DashboardSortOrder switch
{
DashboardSortOrder.ByStatus => _moduleItems.OrderByDescending(x => x.IsEnabled).ThenBy(x => x.Label),
_ => _moduleItems.OrderBy(x => x.Label), // Default alphabetical
}).ToList();
// If AllModules is empty (first load), just populate it.
if (AllModules.Count == 0)
{
foreach (var item in sortedItems)
{
AllModules.Add(item);
}
return;
}
// Otherwise, update the collection in place using Move to avoid UI glitches.
for (int i = 0; i < sortedItems.Count; i++)
{
var currentItem = sortedItems[i];
var currentIndex = AllModules.IndexOf(currentItem);
if (currentIndex != -1 && currentIndex != i)
{
AllModules.Move(currentIndex, i);
}
}
// Notify that DashboardSortOrder changed so the menu updates its checked state.
OnPropertyChanged(nameof(DashboardSortOrder));
}
/// <summary>
/// Refreshes module enabled/locked states by re-reading GPO configuration. Only
/// updates properties that have actually changed to minimize UI notifications
/// then re-sorts the list according to the current sort order.
/// </summary>
private void RefreshModuleList()
{
foreach (var item in _moduleItems)
{
GpoRuleConfigured gpo = ModuleHelper.GetModuleGpoConfiguration(item.Tag);
// GPO can force-enable (Enabled) or force-disable (Disabled) a module.
// If Enabled: module is on and the user cannot disable it.
// If Disabled: module is off and the user cannot enable it.
// Otherwise, the setting is unlocked and the user can enable/disable it.
bool newEnabledState = gpo == GpoRuleConfigured.Enabled || (gpo != GpoRuleConfigured.Disabled && ModuleHelper.GetIsModuleEnabled(generalSettingsConfig, item.Tag));
// Lock the toggle when GPO is controlling the module.
bool newLockedState = gpo == GpoRuleConfigured.Enabled || gpo == GpoRuleConfigured.Disabled;
// Only update if there's an actual change to minimize UI notifications.
if (item.IsEnabled != newEnabledState)
{
item.IsEnabled = newEnabledState;
}
if (item.IsLocked != newLockedState)
{
item.IsLocked = newLockedState;
}
}
SortModuleList();
}
/// <summary>
/// Callback invoked when a user toggles a module's enabled state in the UI.
/// Sets the _isUpdatingFromUI flag to prevent circular updates, then updates
/// settings, re-sorts if needed, and refreshes dependent collections.
/// </summary>
private void EnabledChangedOnUI(DashboardListItem dashboardListItem)
{
Views.ShellPage.UpdateGeneralSettingsCallback(dashboardListItem.Tag, dashboardListItem.IsEnabled);
if (dashboardListItem.Tag == ModuleType.NewPlus && dashboardListItem.IsEnabled == true)
_isUpdatingFromUI = true;
try
{
var settingsUtils = new SettingsUtils();
var settings = NewPlusViewModel.LoadSettings(settingsUtils);
NewPlusViewModel.CopyTemplateExamples(settings.Properties.TemplateLocation.Value);
}
Views.ShellPage.UpdateGeneralSettingsCallback(dashboardListItem.Tag, dashboardListItem.IsEnabled);
// Request updated conflicts after module state change
RequestConflictData();
if (dashboardListItem.Tag == ModuleType.NewPlus && dashboardListItem.IsEnabled == true)
{
var settingsUtils = new SettingsUtils();
var settings = NewPlusViewModel.LoadSettings(settingsUtils);
NewPlusViewModel.CopyTemplateExamples(settings.Properties.TemplateLocation.Value);
}
// Re-sort only required if sorting by enabled status.
if (DashboardSortOrder == DashboardSortOrder.ByStatus)
{
SortModuleList();
}
// Always refresh shortcuts/actions to reflect enabled state changes.
RefreshShortcutModules();
// Request updated conflicts after module state change.
RequestConflictData();
}
finally
{
_isUpdatingFromUI = false;
}
}
/// <summary>
/// Callback invoked when module enabled state changes from other parts of the
/// settings UI. Ignores the notification if it was triggered by a UI toggle
/// we're already handling, to prevent circular updates.
/// </summary>
public void ModuleEnabledChangedOnSettingsPage()
{
// Ignore if this was triggered by a UI change that we're already handling.
if (_isUpdatingFromUI)
{
return;
}
try
{
RefreshModuleList();
GetShortcutModules();
RefreshShortcutModules();
OnPropertyChanged(nameof(ShortcutModules));
// Request updated conflicts after module state change
// Request updated conflicts after module state change.
RequestConflictData();
}
catch (Exception ex)
@@ -198,7 +303,11 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
}
}
private void GetShortcutModules()
/// <summary>
/// Rebuilds ShortcutModules and ActionModules collections by filtering AllModules
/// to only include enabled modules and their respective shortcut/action items.
/// </summary>
private void RefreshShortcutModules()
{
ShortcutModules.Clear();
ActionModules.Clear();