mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-09-01 19:51:34 +02:00
[Keyboard Manager] Fix shortcut modifier display order in the new editor (#49707)
<!-- 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 Fix shortcut key display order in the new Keyboard Manager editor (C# WinUI). When recording a shortcut, modifier keys are now always displayed in the standard canonical order (Win → Ctrl → Alt → Shift → Action key), regardless of the order the user physically pressed them. This matches the existing behavior of the old C++ editor's `GetKeyVector` function. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #48943 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **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 **Bug:** In the new C# KBM editor (`KeyboardManagerEditorUI`), the `GetFormattedKeyList()` method in `KeyboardHookHelper.cs` displayed modifier keys in the order the user pressed them rather than the standard display order. For example, pressing Shift before Win would show `Shift + Win + S` instead of `Win + Shift + S`. **Root cause:** The `modifierKeys` list was populated by iterating `_keyPressOrder` (which preserves temporal press order), and was then rendered directly without sorting. **Fix:** Added a sort step before the display loop that sorts modifier keys using the existing `KeyboardManagerInterop.GetKeyType()` P/Invoke, which returns the `KeyType` enum value (Win=0, Ctrl=1, Alt=2, Shift=3). This enforces the canonical order **Win → Ctrl → Alt → Shift → Action key**, matching the old C++ `EditorHelpers::GetKeyVector()` behavior. **Scope:** Single-line change in `KeyboardHookHelper.GetFormattedKeyList()`. This is a display-only fix — it does not affect the internal key tracking (`_keyPressOrder`), save/load logic, or hook behavior. **Changed file:** - `src/modules/keyboardmanager/KeyboardManagerEditorUI/Helpers/KeyboardHookHelper.cs` <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed 1. Open the new Keyboard Manager editor 2. Click the shortcut trigger button to start recording 3. Press modifier keys in non-standard order (e.g., press Shift first, then Win, then S) 4. **Before fix:** UI shows `Shift + Win + S` 5. **After fix:** UI shows `Win + Shift + S` (correct canonical order) 6. Verified standard-order input (e.g., Win → Shift → S) still displays correctly 7. Verified single modifier + action key shortcuts (e.g., Ctrl+C) display correctly 8. Verified all four modifiers (Win+Ctrl+Alt+Shift+Key) display in correct order regardless of press sequence 9. Verified saving and loading remappings is unaffected by the display change ````
This commit is contained in:
@@ -532,6 +532,10 @@
|
||||
</Project>
|
||||
<Project Path="src/modules/keyboardmanager/KeyboardManagerEngine/KeyboardManagerEngine.vcxproj" Id="ba661f5b-1d5a-4ffc-9bf1-fc39df280bdd" />
|
||||
<Project Path="src/modules/keyboardmanager/KeyboardManagerEngineLibrary/KeyboardManagerEngineLibrary.vcxproj" Id="e496b7fc-1e99-4bab-849b-0e8367040b02" />
|
||||
<Project Path="src/modules/keyboardmanager/KeyboardManagerEditorUI.UnitTests/KeyboardManagerEditorUI.UnitTests.csproj">
|
||||
<Platform Solution="*|ARM64" Project="ARM64" />
|
||||
<Platform Solution="*|x64" Project="x64" />
|
||||
</Project>
|
||||
</Folder>
|
||||
<Folder Name="/modules/MouseUtils/">
|
||||
<Project Path="src/modules/MouseUtils/CursorWrap/CursorWrap.vcxproj" Id="48a1db8c-5df8-4fb3-9e14-2b67f3f2d8b5" />
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
<Project Sdk="Microsoft.NET.Sdk">
|
||||
<!-- Look at Directory.Build.props in root for common stuff as well -->
|
||||
<Import Project="$(RepoRoot)src\Common.Dotnet.CsWinRT.props" />
|
||||
|
||||
<PropertyGroup>
|
||||
<SelfContained>true</SelfContained>
|
||||
<RuntimeIdentifier Condition="'$(Platform)' == 'x64'">win-x64</RuntimeIdentifier>
|
||||
<RuntimeIdentifier Condition="'$(Platform)' == 'ARM64'">win-arm64</RuntimeIdentifier>
|
||||
<IsPackable>false</IsPackable>
|
||||
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
|
||||
<AppendRuntimeIdentifierToOutputPath>false</AppendRuntimeIdentifierToOutputPath>
|
||||
<OutputPath>$(SolutionDir)$(Platform)\$(Configuration)\tests\KeyboardManagerEditorUI.UnitTests\</OutputPath>
|
||||
<OutputType>Exe</OutputType>
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="MSTest" />
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<ProjectReference Include="..\KeyboardManagerEditorUI\KeyboardManagerEditorUI.csproj" />
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
||||
@@ -0,0 +1,182 @@
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// The Microsoft Corporation licenses this file to you under the MIT license.
|
||||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
using System.Collections.Generic;
|
||||
using KeyboardManagerEditorUI.Helpers;
|
||||
using Microsoft.VisualStudio.TestTools.UnitTesting;
|
||||
using Windows.System;
|
||||
|
||||
namespace KeyboardManagerEditorUI.UnitTests
|
||||
{
|
||||
[TestClass]
|
||||
public class RemappingHelperTests
|
||||
{
|
||||
// Test that modifier keys pressed in reverse order (Shift, Alt, Ctrl, Win)
|
||||
// are sorted into the standard display order (Win, Ctrl, Alt, Shift)
|
||||
[TestMethod]
|
||||
public void SortModifierKeys_ShouldSortInCanonicalOrder_WhenPressedInReverseOrder()
|
||||
{
|
||||
// Arrange - keys in reverse of standard order
|
||||
var keys = new List<VirtualKey>
|
||||
{
|
||||
VirtualKey.Shift,
|
||||
VirtualKey.Menu, // Alt
|
||||
VirtualKey.Control,
|
||||
VirtualKey.LeftWindows,
|
||||
};
|
||||
|
||||
// Act
|
||||
RemappingHelper.SortModifierKeys(keys);
|
||||
|
||||
// Assert - should be Win, Ctrl, Alt, Shift
|
||||
Assert.AreEqual(VirtualKey.LeftWindows, keys[0]);
|
||||
Assert.AreEqual(VirtualKey.Control, keys[1]);
|
||||
Assert.AreEqual(VirtualKey.Menu, keys[2]);
|
||||
Assert.AreEqual(VirtualKey.Shift, keys[3]);
|
||||
}
|
||||
|
||||
// Test the specific bug scenario: Win+Shift+S where Shift was pressed before Win
|
||||
[TestMethod]
|
||||
public void SortModifierKeys_ShouldShowWinBeforeShift_WhenShiftPressedFirst()
|
||||
{
|
||||
// Arrange - Shift pressed before Win (the reported bug scenario)
|
||||
var keys = new List<VirtualKey>
|
||||
{
|
||||
VirtualKey.Shift,
|
||||
VirtualKey.LeftWindows,
|
||||
};
|
||||
|
||||
// Act
|
||||
RemappingHelper.SortModifierKeys(keys);
|
||||
|
||||
// Assert - Win should come before Shift
|
||||
Assert.AreEqual(VirtualKey.LeftWindows, keys[0]);
|
||||
Assert.AreEqual(VirtualKey.Shift, keys[1]);
|
||||
}
|
||||
|
||||
// Test that keys already in the correct order remain unchanged
|
||||
[TestMethod]
|
||||
public void SortModifierKeys_ShouldPreserveOrder_WhenAlreadyInCanonicalOrder()
|
||||
{
|
||||
// Arrange - already in correct order
|
||||
var keys = new List<VirtualKey>
|
||||
{
|
||||
VirtualKey.LeftWindows,
|
||||
VirtualKey.Control,
|
||||
VirtualKey.Menu,
|
||||
VirtualKey.Shift,
|
||||
};
|
||||
|
||||
// Act
|
||||
RemappingHelper.SortModifierKeys(keys);
|
||||
|
||||
// Assert - order should be unchanged
|
||||
Assert.AreEqual(VirtualKey.LeftWindows, keys[0]);
|
||||
Assert.AreEqual(VirtualKey.Control, keys[1]);
|
||||
Assert.AreEqual(VirtualKey.Menu, keys[2]);
|
||||
Assert.AreEqual(VirtualKey.Shift, keys[3]);
|
||||
}
|
||||
|
||||
// Test that left/right variants of the same modifier maintain their relative
|
||||
// position but are grouped correctly relative to other modifier types
|
||||
[TestMethod]
|
||||
public void SortModifierKeys_ShouldSortCorrectly_WithLeftRightVariants()
|
||||
{
|
||||
// Arrange - right shift before left control
|
||||
var keys = new List<VirtualKey>
|
||||
{
|
||||
VirtualKey.RightShift,
|
||||
VirtualKey.LeftControl,
|
||||
};
|
||||
|
||||
// Act
|
||||
RemappingHelper.SortModifierKeys(keys);
|
||||
|
||||
// Assert - Ctrl should come before Shift
|
||||
Assert.AreEqual(VirtualKey.LeftControl, keys[0]);
|
||||
Assert.AreEqual(VirtualKey.RightShift, keys[1]);
|
||||
}
|
||||
|
||||
// Test with a single modifier key (should not throw)
|
||||
[TestMethod]
|
||||
public void SortModifierKeys_ShouldHandleSingleModifier()
|
||||
{
|
||||
// Arrange
|
||||
var keys = new List<VirtualKey> { VirtualKey.Control };
|
||||
|
||||
// Act
|
||||
RemappingHelper.SortModifierKeys(keys);
|
||||
|
||||
// Assert
|
||||
Assert.AreEqual(1, keys.Count);
|
||||
Assert.AreEqual(VirtualKey.Control, keys[0]);
|
||||
}
|
||||
|
||||
// Test with an empty list (should not throw)
|
||||
[TestMethod]
|
||||
public void SortModifierKeys_ShouldHandleEmptyList()
|
||||
{
|
||||
// Arrange
|
||||
var keys = new List<VirtualKey>();
|
||||
|
||||
// Act
|
||||
RemappingHelper.SortModifierKeys(keys);
|
||||
|
||||
// Assert
|
||||
Assert.AreEqual(0, keys.Count);
|
||||
}
|
||||
|
||||
// Test GetModifierSortOrder returns correct values
|
||||
[TestMethod]
|
||||
public void GetModifierSortOrder_ShouldReturnCorrectOrder_ForAllModifierTypes()
|
||||
{
|
||||
// Win keys should return 0
|
||||
Assert.AreEqual(0, RemappingHelper.GetModifierSortOrder(VirtualKey.LeftWindows));
|
||||
Assert.AreEqual(0, RemappingHelper.GetModifierSortOrder(VirtualKey.RightWindows));
|
||||
|
||||
// Ctrl keys should return 1
|
||||
Assert.AreEqual(1, RemappingHelper.GetModifierSortOrder(VirtualKey.Control));
|
||||
Assert.AreEqual(1, RemappingHelper.GetModifierSortOrder(VirtualKey.LeftControl));
|
||||
Assert.AreEqual(1, RemappingHelper.GetModifierSortOrder(VirtualKey.RightControl));
|
||||
|
||||
// Alt keys should return 2
|
||||
Assert.AreEqual(2, RemappingHelper.GetModifierSortOrder(VirtualKey.Menu));
|
||||
Assert.AreEqual(2, RemappingHelper.GetModifierSortOrder(VirtualKey.LeftMenu));
|
||||
Assert.AreEqual(2, RemappingHelper.GetModifierSortOrder(VirtualKey.RightMenu));
|
||||
|
||||
// Shift keys should return 3
|
||||
Assert.AreEqual(3, RemappingHelper.GetModifierSortOrder(VirtualKey.Shift));
|
||||
Assert.AreEqual(3, RemappingHelper.GetModifierSortOrder(VirtualKey.LeftShift));
|
||||
Assert.AreEqual(3, RemappingHelper.GetModifierSortOrder(VirtualKey.RightShift));
|
||||
}
|
||||
|
||||
// Test that non-modifier keys get the highest sort order (4)
|
||||
[TestMethod]
|
||||
public void GetModifierSortOrder_ShouldReturnFour_ForNonModifierKeys()
|
||||
{
|
||||
Assert.AreEqual(4, RemappingHelper.GetModifierSortOrder(VirtualKey.A));
|
||||
Assert.AreEqual(4, RemappingHelper.GetModifierSortOrder(VirtualKey.Space));
|
||||
Assert.AreEqual(4, RemappingHelper.GetModifierSortOrder(VirtualKey.Enter));
|
||||
}
|
||||
|
||||
// Test that two modifiers pressed out of order (Alt then Ctrl) get corrected
|
||||
[TestMethod]
|
||||
public void SortModifierKeys_ShouldSortCorrectly_WhenAltPressedBeforeCtrl()
|
||||
{
|
||||
// Arrange
|
||||
var keys = new List<VirtualKey>
|
||||
{
|
||||
VirtualKey.Menu, // Alt
|
||||
VirtualKey.Control,
|
||||
};
|
||||
|
||||
// Act
|
||||
RemappingHelper.SortModifierKeys(keys);
|
||||
|
||||
// Assert - Ctrl should come before Alt
|
||||
Assert.AreEqual(VirtualKey.Control, keys[0]);
|
||||
Assert.AreEqual(VirtualKey.Menu, keys[1]);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// The Microsoft Corporation licenses this file to you under the MIT license.
|
||||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
@@ -212,6 +212,11 @@ namespace KeyboardManagerEditorUI.Helpers
|
||||
}
|
||||
}
|
||||
|
||||
// Sort modifier keys by the standard display order: Win → Ctrl → Alt → Shift
|
||||
// This matches the old C++ GetKeyVector behavior where keys are always shown in
|
||||
// canonical order regardless of the order the user pressed them.
|
||||
RemappingHelper.SortModifierKeys(modifierKeys);
|
||||
|
||||
foreach (var key in modifierKeys)
|
||||
{
|
||||
keyList.Add(_mappingService.GetKeyDisplayName((int)key));
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// Copyright (c) Microsoft Corporation
|
||||
// The Microsoft Corporation licenses this file to you under the MIT license.
|
||||
// See the LICENSE file in the project root for more information.
|
||||
|
||||
@@ -176,5 +176,32 @@ namespace KeyboardManagerEditorUI.Helpers
|
||||
|| key == VirtualKey.LeftWindows
|
||||
|| key == VirtualKey.RightWindows;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns the sort order for a modifier key in the standard display order:
|
||||
/// Win(0) → Ctrl(1) → Alt(2) → Shift(3).
|
||||
/// Non-modifier (action) keys return 4.
|
||||
/// This matches the old C++ GetKeyVector behavior.
|
||||
/// </summary>
|
||||
public static int GetModifierSortOrder(VirtualKey key)
|
||||
{
|
||||
return key switch
|
||||
{
|
||||
VirtualKey.LeftWindows or VirtualKey.RightWindows => 0,
|
||||
VirtualKey.Control or VirtualKey.LeftControl or VirtualKey.RightControl => 1,
|
||||
VirtualKey.Menu or VirtualKey.LeftMenu or VirtualKey.RightMenu => 2,
|
||||
VirtualKey.Shift or VirtualKey.LeftShift or VirtualKey.RightShift => 3,
|
||||
_ => 4,
|
||||
};
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Sorts a list of modifier keys in the standard display order:
|
||||
/// Win → Ctrl → Alt → Shift.
|
||||
/// </summary>
|
||||
public static void SortModifierKeys(List<VirtualKey> modifierKeys)
|
||||
{
|
||||
modifierKeys.Sort((a, b) => GetModifierSortOrder(a).CompareTo(GetModifierSortOrder(b)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user