mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-04-05 10:46:33 +02:00
Optimize the module list logic (#44628)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This pull request refactors how module enable/disable state changes are handled in the settings UI, centralizing and simplifying the logic for updating and notifying these changes. The main improvements include unifying the callback mechanism for enable state changes, reducing code duplication, and making the update process more robust against circular updates. **Refactoring and Simplification of Enable State Handling:** * Introduced a unified `EnabledChangedCallback` and `UpdateStatus` method in the base `ModuleListItem` class to handle enable/disable state changes and notifications, replacing redundant logic in derived classes like `FlyoutMenuItem` and `DashboardListItem`. [[1]](diffhunk://#diff-23ab2cc13a1098a6071b3e12ce0919b7eba451d7683f6f62e5ec2cf661778a4cR21-R36) [[2]](diffhunk://#diff-23ab2cc13a1098a6071b3e12ce0919b7eba451d7683f6f62e5ec2cf661778a4cR99-R103) [[3]](diffhunk://#diff-5033dabc0e3ec7d01509b9d58878b9ee5745378d5e3a7fa92779bf9c111bcffcL25-L39) [[4]](diffhunk://#diff-9c93f68ee87a48d8affd140224601da95d7fe9642ad24350c7527d0f5773ec7dL29-L43) * Updated all usages to call `UpdateStatus` instead of directly setting `IsEnabled`, ensuring callbacks are managed consistently and preventing unnecessary notifications during programmatic updates. [[1]](diffhunk://#diff-734ba1b4b3044eb540bba08334bd141c968a113625be2d92c831f3cc3debc62fL108-R109) [[2]](diffhunk://#diff-aea3404667e7a3de2750bf9ab7ee8ff5e717892caa68ee1de86713cf8e21b44cL261-R260) **Improvements to Callback and Update Logic:** * Changed the signature of UI event handlers to use the base `ModuleListItem` type, improving code maintainability and reducing casting and duplication. [[1]](diffhunk://#diff-734ba1b4b3044eb540bba08334bd141c968a113625be2d92c831f3cc3debc62fL160-R161) [[2]](diffhunk://#diff-aea3404667e7a3de2750bf9ab7ee8ff5e717892caa68ee1de86713cf8e21b44cL278-R287) * Removed obsolete or redundant flags and logic for tracking update state, further simplifying the codebase. [[1]](diffhunk://#diff-aea3404667e7a3de2750bf9ab7ee8ff5e717892caa68ee1de86713cf8e21b44cL53) [[2]](diffhunk://#diff-aea3404667e7a3de2750bf9ab7ee8ff5e717892caa68ee1de86713cf8e21b44cL328) [[3]](diffhunk://#diff-aea3404667e7a3de2750bf9ab7ee8ff5e717892caa68ee1de86713cf8e21b44cL343-L346) **Project and Namespace Adjustments:** * Updated the project file to include all localized resource files recursively, improving localization support. * Added a missing namespace import to ensure proper type resolution. * Added a missing `using System;` directive for the `Action` delegate. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [ ] Closes: #xxx <!-- - [ ] Closes: #yyy (add separate lines for additional resolved issues) --> - [ ] **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 <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed
This commit is contained in:
@@ -10,6 +10,7 @@ using global::PowerToys.GPOWrapper;
|
|||||||
using ManagedCommon;
|
using ManagedCommon;
|
||||||
using Microsoft.PowerToys.QuickAccess.Helpers;
|
using Microsoft.PowerToys.QuickAccess.Helpers;
|
||||||
using Microsoft.PowerToys.QuickAccess.Services;
|
using Microsoft.PowerToys.QuickAccess.Services;
|
||||||
|
using Microsoft.PowerToys.Settings.UI.Controls;
|
||||||
using Microsoft.PowerToys.Settings.UI.Library;
|
using Microsoft.PowerToys.Settings.UI.Library;
|
||||||
using Microsoft.PowerToys.Settings.UI.Library.Helpers;
|
using Microsoft.PowerToys.Settings.UI.Library.Helpers;
|
||||||
using Microsoft.PowerToys.Settings.UI.Library.Interfaces;
|
using Microsoft.PowerToys.Settings.UI.Library.Interfaces;
|
||||||
@@ -26,6 +27,7 @@ public sealed class AllAppsViewModel : Observable
|
|||||||
private readonly SettingsUtils _settingsUtils;
|
private readonly SettingsUtils _settingsUtils;
|
||||||
private readonly ResourceLoader _resourceLoader;
|
private readonly ResourceLoader _resourceLoader;
|
||||||
private readonly DispatcherQueue _dispatcherQueue;
|
private readonly DispatcherQueue _dispatcherQueue;
|
||||||
|
private readonly List<FlyoutMenuItem> _allFlyoutMenuItems = new();
|
||||||
private GeneralSettings _generalSettings;
|
private GeneralSettings _generalSettings;
|
||||||
|
|
||||||
public ObservableCollection<FlyoutMenuItem> FlyoutMenuItems { get; }
|
public ObservableCollection<FlyoutMenuItem> FlyoutMenuItems { get; }
|
||||||
@@ -58,9 +60,28 @@ public sealed class AllAppsViewModel : Observable
|
|||||||
_resourceLoader = Helpers.ResourceLoaderInstance.ResourceLoader;
|
_resourceLoader = Helpers.ResourceLoaderInstance.ResourceLoader;
|
||||||
FlyoutMenuItems = new ObservableCollection<FlyoutMenuItem>();
|
FlyoutMenuItems = new ObservableCollection<FlyoutMenuItem>();
|
||||||
|
|
||||||
|
BuildFlyoutMenuItems();
|
||||||
RefreshFlyoutMenuItems();
|
RefreshFlyoutMenuItems();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void BuildFlyoutMenuItems()
|
||||||
|
{
|
||||||
|
_allFlyoutMenuItems.Clear();
|
||||||
|
foreach (ModuleType moduleType in Enum.GetValues<ModuleType>())
|
||||||
|
{
|
||||||
|
if (moduleType == ModuleType.GeneralSettings)
|
||||||
|
{
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
_allFlyoutMenuItems.Add(new FlyoutMenuItem
|
||||||
|
{
|
||||||
|
Tag = moduleType,
|
||||||
|
EnabledChangedCallback = EnabledChangedOnUI,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private void OnSettingsChanged(GeneralSettings newSettings)
|
private void OnSettingsChanged(GeneralSettings newSettings)
|
||||||
{
|
{
|
||||||
_dispatcherQueue.TryEnqueue(() =>
|
_dispatcherQueue.TryEnqueue(() =>
|
||||||
@@ -82,63 +103,37 @@ public sealed class AllAppsViewModel : Observable
|
|||||||
|
|
||||||
private void RefreshFlyoutMenuItems()
|
private void RefreshFlyoutMenuItems()
|
||||||
{
|
{
|
||||||
var desiredItems = new List<FlyoutMenuItem>();
|
foreach (var item in _allFlyoutMenuItems)
|
||||||
|
|
||||||
foreach (ModuleType moduleType in Enum.GetValues<ModuleType>())
|
|
||||||
{
|
{
|
||||||
if (moduleType == ModuleType.GeneralSettings)
|
var moduleType = item.Tag;
|
||||||
{
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
var gpo = Helpers.ModuleGpoHelper.GetModuleGpoConfiguration(moduleType);
|
var gpo = Helpers.ModuleGpoHelper.GetModuleGpoConfiguration(moduleType);
|
||||||
var isLocked = gpo is GpoRuleConfigured.Enabled or GpoRuleConfigured.Disabled;
|
var isLocked = gpo is GpoRuleConfigured.Enabled or GpoRuleConfigured.Disabled;
|
||||||
var isEnabled = gpo == GpoRuleConfigured.Enabled || (!isLocked && Microsoft.PowerToys.Settings.UI.Library.Helpers.ModuleHelper.GetIsModuleEnabled(_generalSettings, moduleType));
|
var isEnabled = gpo == GpoRuleConfigured.Enabled || (!isLocked && Microsoft.PowerToys.Settings.UI.Library.Helpers.ModuleHelper.GetIsModuleEnabled(_generalSettings, moduleType));
|
||||||
|
|
||||||
var existingItem = FlyoutMenuItems.FirstOrDefault(x => x.Tag == moduleType);
|
item.Label = _resourceLoader.GetString(Microsoft.PowerToys.Settings.UI.Library.Helpers.ModuleHelper.GetModuleLabelResourceName(moduleType));
|
||||||
|
item.IsLocked = isLocked;
|
||||||
|
item.Icon = Microsoft.PowerToys.Settings.UI.Library.Helpers.ModuleHelper.GetModuleTypeFluentIconName(moduleType);
|
||||||
|
|
||||||
if (existingItem != null)
|
if (item.IsEnabled != isEnabled)
|
||||||
{
|
{
|
||||||
existingItem.Label = _resourceLoader.GetString(Microsoft.PowerToys.Settings.UI.Library.Helpers.ModuleHelper.GetModuleLabelResourceName(moduleType));
|
item.UpdateStatus(isEnabled);
|
||||||
existingItem.IsLocked = isLocked;
|
|
||||||
existingItem.Icon = Microsoft.PowerToys.Settings.UI.Library.Helpers.ModuleHelper.GetModuleTypeFluentIconName(moduleType);
|
|
||||||
|
|
||||||
if (existingItem.IsEnabled != isEnabled)
|
|
||||||
{
|
|
||||||
var callback = existingItem.EnabledChangedCallback;
|
|
||||||
existingItem.EnabledChangedCallback = null;
|
|
||||||
existingItem.IsEnabled = isEnabled;
|
|
||||||
existingItem.EnabledChangedCallback = callback;
|
|
||||||
}
|
|
||||||
|
|
||||||
desiredItems.Add(existingItem);
|
|
||||||
}
|
|
||||||
else
|
|
||||||
{
|
|
||||||
desiredItems.Add(new FlyoutMenuItem
|
|
||||||
{
|
|
||||||
Label = _resourceLoader.GetString(Microsoft.PowerToys.Settings.UI.Library.Helpers.ModuleHelper.GetModuleLabelResourceName(moduleType)),
|
|
||||||
IsEnabled = isEnabled,
|
|
||||||
IsLocked = isLocked,
|
|
||||||
Tag = moduleType,
|
|
||||||
Icon = Microsoft.PowerToys.Settings.UI.Library.Helpers.ModuleHelper.GetModuleTypeFluentIconName(moduleType),
|
|
||||||
EnabledChangedCallback = EnabledChangedOnUI,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
var sortedItems = DashboardSortOrder switch
|
var sortedItems = DashboardSortOrder switch
|
||||||
{
|
{
|
||||||
DashboardSortOrder.ByStatus => desiredItems.OrderByDescending(x => x.IsEnabled).ThenBy(x => x.Label).ToList(),
|
DashboardSortOrder.ByStatus => _allFlyoutMenuItems.OrderByDescending(x => x.IsEnabled).ThenBy(x => x.Label).ToList(),
|
||||||
_ => desiredItems.OrderBy(x => x.Label).ToList(),
|
_ => _allFlyoutMenuItems.OrderBy(x => x.Label).ToList(),
|
||||||
};
|
};
|
||||||
|
|
||||||
for (int i = FlyoutMenuItems.Count - 1; i >= 0; i--)
|
if (FlyoutMenuItems.Count == 0)
|
||||||
{
|
{
|
||||||
if (!sortedItems.Contains(FlyoutMenuItems[i]))
|
foreach (var item in sortedItems)
|
||||||
{
|
{
|
||||||
FlyoutMenuItems.RemoveAt(i);
|
FlyoutMenuItems.Add(item);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
for (int i = 0; i < sortedItems.Count; i++)
|
for (int i = 0; i < sortedItems.Count; i++)
|
||||||
@@ -146,20 +141,17 @@ public sealed class AllAppsViewModel : Observable
|
|||||||
var item = sortedItems[i];
|
var item = sortedItems[i];
|
||||||
var oldIndex = FlyoutMenuItems.IndexOf(item);
|
var oldIndex = FlyoutMenuItems.IndexOf(item);
|
||||||
|
|
||||||
if (oldIndex < 0)
|
if (oldIndex != -1 && oldIndex != i)
|
||||||
{
|
|
||||||
FlyoutMenuItems.Insert(i, item);
|
|
||||||
}
|
|
||||||
else if (oldIndex != i)
|
|
||||||
{
|
{
|
||||||
FlyoutMenuItems.Move(oldIndex, i);
|
FlyoutMenuItems.Move(oldIndex, i);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private void EnabledChangedOnUI(FlyoutMenuItem item)
|
private void EnabledChangedOnUI(ModuleListItem item)
|
||||||
{
|
{
|
||||||
if (_coordinator.UpdateModuleEnabled(item.Tag, item.IsEnabled))
|
var flyoutItem = (FlyoutMenuItem)item;
|
||||||
|
if (_coordinator.UpdateModuleEnabled(flyoutItem.Tag, flyoutItem.IsEnabled))
|
||||||
{
|
{
|
||||||
_coordinator.NotifyUserSettingsInteraction();
|
_coordinator.NotifyUserSettingsInteraction();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -22,21 +22,6 @@ public sealed class FlyoutMenuItem : ModuleListItem
|
|||||||
set => base.Tag = value;
|
set => base.Tag = value;
|
||||||
}
|
}
|
||||||
|
|
||||||
public override bool IsEnabled
|
|
||||||
{
|
|
||||||
get => base.IsEnabled;
|
|
||||||
set
|
|
||||||
{
|
|
||||||
if (base.IsEnabled != value)
|
|
||||||
{
|
|
||||||
base.IsEnabled = value;
|
|
||||||
EnabledChangedCallback?.Invoke(this);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
public Action<FlyoutMenuItem>? EnabledChangedCallback { get; set; }
|
|
||||||
|
|
||||||
public bool Visible
|
public bool Visible
|
||||||
{
|
{
|
||||||
get => _visible;
|
get => _visible;
|
||||||
|
|||||||
@@ -2,6 +2,7 @@
|
|||||||
// The Microsoft Corporation licenses this file to you under the MIT license.
|
// The Microsoft Corporation licenses this file to you under the MIT license.
|
||||||
// See the LICENSE file in the project root for more information.
|
// See the LICENSE file in the project root for more information.
|
||||||
|
|
||||||
|
using System; // For Action
|
||||||
using System.ComponentModel;
|
using System.ComponentModel;
|
||||||
using System.Runtime.CompilerServices;
|
using System.Runtime.CompilerServices;
|
||||||
using System.Windows.Input;
|
using System.Windows.Input;
|
||||||
@@ -17,6 +18,22 @@ namespace Microsoft.PowerToys.Settings.UI.Controls
|
|||||||
private bool _isLocked;
|
private bool _isLocked;
|
||||||
private object? _tag;
|
private object? _tag;
|
||||||
private ICommand? _clickCommand;
|
private ICommand? _clickCommand;
|
||||||
|
private bool _isUpdating;
|
||||||
|
|
||||||
|
public Action<ModuleListItem>? EnabledChangedCallback { get; set; }
|
||||||
|
|
||||||
|
public void UpdateStatus(bool isEnabled)
|
||||||
|
{
|
||||||
|
_isUpdating = true;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
IsEnabled = isEnabled;
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
_isUpdating = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public virtual string Label
|
public virtual string Label
|
||||||
{
|
{
|
||||||
@@ -79,6 +96,11 @@ namespace Microsoft.PowerToys.Settings.UI.Controls
|
|||||||
{
|
{
|
||||||
_isEnabled = value;
|
_isEnabled = value;
|
||||||
OnPropertyChanged();
|
OnPropertyChanged();
|
||||||
|
|
||||||
|
if (!_isUpdating)
|
||||||
|
{
|
||||||
|
EnabledChangedCallback?.Invoke(this);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -26,21 +26,6 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
|
|||||||
set => base.Tag = value;
|
set => base.Tag = value;
|
||||||
}
|
}
|
||||||
|
|
||||||
public Action<DashboardListItem> EnabledChangedCallback { get; set; }
|
|
||||||
|
|
||||||
public override bool IsEnabled
|
|
||||||
{
|
|
||||||
get => base.IsEnabled;
|
|
||||||
set
|
|
||||||
{
|
|
||||||
if (base.IsEnabled != value)
|
|
||||||
{
|
|
||||||
base.IsEnabled = value;
|
|
||||||
EnabledChangedCallback?.Invoke(this);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
public bool Visible
|
public bool Visible
|
||||||
{
|
{
|
||||||
get => _visible;
|
get => _visible;
|
||||||
|
|||||||
@@ -50,7 +50,6 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
|
|||||||
|
|
||||||
// Flag to prevent circular updates when a UI toggle triggers settings changes.
|
// Flag to prevent circular updates when a UI toggle triggers settings changes.
|
||||||
private bool _isUpdatingFromUI;
|
private bool _isUpdatingFromUI;
|
||||||
private bool _isUpdatingFromSettings;
|
|
||||||
|
|
||||||
private AllHotkeyConflictsData _allHotkeyConflictsData = new AllHotkeyConflictsData();
|
private AllHotkeyConflictsData _allHotkeyConflictsData = new AllHotkeyConflictsData();
|
||||||
|
|
||||||
@@ -258,7 +257,7 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
|
|||||||
// Only update if there's an actual change to minimize UI notifications.
|
// Only update if there's an actual change to minimize UI notifications.
|
||||||
if (item.IsEnabled != newEnabledState)
|
if (item.IsEnabled != newEnabledState)
|
||||||
{
|
{
|
||||||
item.IsEnabled = newEnabledState;
|
item.UpdateStatus(newEnabledState);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (item.IsLocked != newLockedState)
|
if (item.IsLocked != newLockedState)
|
||||||
@@ -275,19 +274,17 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
|
|||||||
/// Sets the _isUpdatingFromUI flag to prevent circular updates, then updates
|
/// Sets the _isUpdatingFromUI flag to prevent circular updates, then updates
|
||||||
/// settings, re-sorts if needed, and refreshes dependent collections.
|
/// settings, re-sorts if needed, and refreshes dependent collections.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private void EnabledChangedOnUI(DashboardListItem dashboardListItem)
|
private void EnabledChangedOnUI(ModuleListItem item)
|
||||||
{
|
{
|
||||||
if (_isUpdatingFromSettings)
|
var dashboardListItem = (DashboardListItem)item;
|
||||||
{
|
var isEnabled = dashboardListItem.IsEnabled;
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
_isUpdatingFromUI = true;
|
_isUpdatingFromUI = true;
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
Views.ShellPage.UpdateGeneralSettingsCallback(dashboardListItem.Tag, dashboardListItem.IsEnabled);
|
Views.ShellPage.UpdateGeneralSettingsCallback(dashboardListItem.Tag, isEnabled);
|
||||||
|
|
||||||
if (dashboardListItem.Tag == ModuleType.NewPlus && dashboardListItem.IsEnabled == true)
|
if (dashboardListItem.Tag == ModuleType.NewPlus && isEnabled == true)
|
||||||
{
|
{
|
||||||
var settingsUtils = SettingsUtils.Default;
|
var settingsUtils = SettingsUtils.Default;
|
||||||
var settings = NewPlusViewModel.LoadSettings(settingsUtils);
|
var settings = NewPlusViewModel.LoadSettings(settingsUtils);
|
||||||
@@ -325,7 +322,6 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
_isUpdatingFromSettings = true;
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
RefreshModuleList();
|
RefreshModuleList();
|
||||||
@@ -340,10 +336,6 @@ namespace Microsoft.PowerToys.Settings.UI.ViewModels
|
|||||||
{
|
{
|
||||||
Logger.LogError($"Updating active/disabled modules list failed: {ex.Message}");
|
Logger.LogError($"Updating active/disabled modules list failed: {ex.Message}");
|
||||||
}
|
}
|
||||||
finally
|
|
||||||
{
|
|
||||||
_isUpdatingFromSettings = false;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
Reference in New Issue
Block a user