Improved the KB Hook in Color Picker (#6175)

* Improved the KB Hook in Color Picker

* Update KeyboardMonitor.cs

* Further simplify a method

* Using var

* Update KeyboardMonitor.cs

Add return;

src/modules/colorPicker/ColorPickerUI/Keyboard/KeyboardMonitor.cs

@@ -11,6 +11,7 @@ using ColorPicker.Settings;
 using ColorPicker.Telemetry;
 using Microsoft.PowerToys.Settings.UI.Lib.Utilities;
 using Microsoft.PowerToys.Telemetry;
+using static ColorPicker.Win32Apis;
 
 namespace ColorPicker.Keyboard
 {
@@ -20,7 +21,6 @@ namespace ColorPicker.Keyboard
         private readonly AppStateHandler _appStateHandler;
         private readonly IUserSettings _userSettings;
 
-        private List<string> _currentlyPressedKeys = new List<string>();
         private List<string> _activationKeys = new List<string>();
         private GlobalKeyboardHook _keyboardHook;
 
@@ -62,45 +62,36 @@ namespace ColorPicker.Keyboard
 
         private void Hook_KeyboardPressed(object sender, GlobalKeyboardHookEventArgs e)
         {
+            var currentlyPressedKeys = new List<string>();
             var virtualCode = e.KeyboardData.VirtualCode;
 
             // ESC pressed
             if (virtualCode == KeyInterop.VirtualKeyFromKey(Key.Escape))
             {
-                _currentlyPressedKeys.Clear();
                 _appStateHandler.HideColorPicker();
                 PowerToysTelemetry.Log.WriteEvent(new ColorPickerCancelledEvent());
+                return;
             }
 
             var name = Helper.GetKeyName((uint)virtualCode);
 
-            // we got modifier with additional info such as "Ctrl (left)" - get rid of parenthesess
+            // If the last key pressed is a modifier key, then currentlyPressedKeys cannot possibly match with _activationKeys
-            if (name.IndexOf("(", StringComparison.OrdinalIgnoreCase) > 0 && name.Length > 1)
+            // because _activationKeys contains exactly 1 non-modifier key. Hence, there's no need to check if `name` is a
-            {
+            // modifier key or to do any additional processing on it.
-                name = name.Substring(0, name.IndexOf("(", StringComparison.OrdinalIgnoreCase)).Trim();
+
-            }
+            // Check pressed modifier keys.
+            AddModifierKeys(currentlyPressedKeys);
 
             if (e.KeyboardState == GlobalKeyboardHook.KeyboardState.KeyDown || e.KeyboardState == GlobalKeyboardHook.KeyboardState.SysKeyDown)
             {
-                if (!_currentlyPressedKeys.Contains(name))
+                currentlyPressedKeys.Add(name);
-                {
-                    _currentlyPressedKeys.Add(name);
-                }
-            }
-            else if (e.KeyboardState == GlobalKeyboardHook.KeyboardState.KeyUp || e.KeyboardState == GlobalKeyboardHook.KeyboardState.SysKeyUp)
-            {
-                if (_currentlyPressedKeys.Contains(name))
-                {
-                    _currentlyPressedKeys.Remove(name);
-                }
             }
 
-            _currentlyPressedKeys.Sort();
+            currentlyPressedKeys.Sort();
 
-            if (ArraysAreSame(_currentlyPressedKeys, _activationKeys))
+            if (ArraysAreSame(currentlyPressedKeys, _activationKeys))
             {
                 _appStateHandler.ShowColorPicker();
-                _currentlyPressedKeys.Clear();
             }
         }
 
@@ -121,5 +112,28 @@ namespace ColorPicker.Keyboard
 
             return true;
         }
+
+        private static void AddModifierKeys(List<string> currentlyPressedKeys)
+        {
+            if ((GetAsyncKeyState(VK_SHIFT) & 0x8000) != 0)
+            {
+                currentlyPressedKeys.Add("Shift");
+            }
+
+            if ((GetAsyncKeyState(VK_CONTROL) & 0x8000) != 0)
+            {
+                currentlyPressedKeys.Add("Ctrl");
+            }
+
+            if ((GetAsyncKeyState(VK_MENU) & 0x8000) != 0)
+            {
+                currentlyPressedKeys.Add("Alt");
+            }
+
+            if ((GetAsyncKeyState(VK_LWIN) & 0x8000) != 0 || (GetAsyncKeyState(VK_RWIN) & 0x8000) != 0)
+            {
+                currentlyPressedKeys.Add("Win");
+            }
+        }
     }
 }

src/modules/colorPicker/ColorPickerUI/Win32Apis.cs

@@ -17,6 +17,17 @@ namespace ColorPicker
         public const int LlkhfAltdown = KfAltdown >> 8;
         public const int MonitorinfofPrimary = 0x00000001;
 
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
+        public const int VK_SHIFT = 0x10;
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
+        public const int VK_CONTROL = 0x11;
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
+        public const int VK_MENU = 0x12;
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
+        public const int VK_LWIN = 0x5B;
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("StyleCop.CSharp.NamingRules", "SA1310:Field names should not contain underscore", Justification = "Interop")]
+        public const int VK_RWIN = 0x5C;
+
         public delegate bool MonitorEnumProc(
             IntPtr monitor, IntPtr hdc, IntPtr lprcMonitor, IntPtr lParam);
 
@@ -53,6 +64,9 @@ namespace ColorPicker
         [DllImport("user32.dll", CharSet = CharSet.Auto, CallingConvention = CallingConvention.StdCall, SetLastError = true)]
         internal static extern IntPtr CallNextHookEx(IntPtr idHook, int nCode, IntPtr wParam, IntPtr lParam);
 
+        [DllImport("user32.dll", CharSet = CharSet.Auto, CallingConvention = CallingConvention.StdCall, SetLastError = true)]
+        internal static extern short GetAsyncKeyState(int vKey);
+
         [DllImport("user32.dll", EntryPoint = "SystemParametersInfo")]
         internal static extern bool SystemParametersInfo(int uiAction, int uiParam, IntPtr pvParam, int fWinIni);