fix(key-bindings): Handle overlapping key bindings correctly

- When multiple keys are pressed in quick succession,
it's possible for their keydown events to overlap before
the corresponding keyup events fire. This can lead to
incorrect binding matches if the logic doesn't account for
the current state of pressed keys.

The fix involves ensuring that the key event handling logic
correctly identifies which keys are currently pressed and
matches them against the defined bindings, even when overlaps occur.

- Additionally, the skip logic is improved to prevent keys from
getting stuck if the skip condition changes between keydown and keyup events.

Author: rkaraivanov

src/components/common/controllers/key-bindings.spec.ts

@@ -10,6 +10,7 @@ import { simulateKeyboard } from '../utils.spec.js';
 import {
   addKeybindings,
   altKey,
+  arrowDown,
   arrowUp,
   enterKey,
   shiftKey,
@@ -245,6 +246,69 @@ describe('Key bindings controller', () => {
       dispatch(repeatInstance, 'keydown', 'b', true);
       expect(repeatInstance.callCount).to.equal(1);
     });
+
+    it('should fire the correct binding when keys overlap (keyup not yet fired for previous key)', () => {
+      // Simulate pressing ArrowDown a couple of times, then pressing ArrowUp before
+      // ArrowDown's keyup fires — mimicking rapid navigation where keys overlap.
+      let repeatInstance2: LitElement & {
+        lastKey: string | undefined;
+        downCount: number;
+        upCount: number;
+      };
+
+      const repeatTag2 = defineCE(
+        class extends LitElement {
+          public lastKey: string | undefined = undefined;
+          public downCount = 0;
+          public upCount = 0;
+
+          constructor() {
+            super();
+            addKeybindings(this)
+              .set(
+                arrowDown,
+                () => {
+                  this.lastKey = arrowDown;
+                  this.downCount++;
+                },
+                { repeat: true }
+              )
+              .set(
+                arrowUp,
+                () => {
+                  this.lastKey = arrowUp;
+                  this.upCount++;
+                },
+                { repeat: true }
+              );
+          }
+        }
+      );
+
+      return fixture<typeof repeatInstance2>(
+        html`<${unsafeStatic(repeatTag2)}></${unsafeStatic(repeatTag2)}>`
+      ).then((el) => {
+        repeatInstance2 = el;
+
+        // Press ArrowDown twice (with repeat)
+        dispatch(repeatInstance2, 'keydown', arrowDown);
+        dispatch(repeatInstance2, 'keydown', arrowDown, true);
+
+        // Press ArrowUp while ArrowDown is still "held" (no keyup yet)
+        dispatch(repeatInstance2, 'keydown', arrowUp);
+
+        expect(repeatInstance2.upCount).to.equal(1);
+        expect(repeatInstance2.lastKey).to.equal(arrowUp);
+
+        // Release both keys
+        dispatch(repeatInstance2, 'keyup', arrowDown);
+        dispatch(repeatInstance2, 'keyup', arrowUp);
+
+        // Press ArrowDown again — should still work cleanly after the overlap
+        dispatch(repeatInstance2, 'keydown', arrowDown);
+        expect(repeatInstance2.downCount).to.equal(3);
+      });
+    });
   });
 });
 

src/components/common/controllers/key-bindings.ts

@@ -27,7 +27,7 @@ export const tabKey = 'Tab' as const;
 
 /* Modifiers */
 export const altKey = 'Alt' as const;
-export const ctrlKey = 'Ctrl' as const;
+export const ctrlKey = 'Control' as const;
 export const metaKey = 'Meta' as const;
 export const shiftKey = 'Shift' as const;
 
@@ -135,9 +135,20 @@ interface KeyBinding {
 
 //#region Internal functions and constants
 
-const MODIFIERS = new Set(['alt', 'ctrl', 'meta', 'shift']);
+const MODIFIERS = new Set(['alt', 'control', 'meta', 'shift']);
 const ALL_MODIFIER_VALUES = Array.from(MODIFIERS).sort();
 
+/**
+ * Maps internal modifier names to their corresponding `KeyboardEvent` boolean property names.
+ * Needed because `ctrlKey` in the DOM is the property for the `'control'` modifier (not `'controlKey'`).
+ */
+export const MODIFIER_EVENT_KEYS: Record<string, string> = {
+  alt: 'altKey',
+  control: 'ctrlKey',
+  meta: 'metaKey',
+  shift: 'shiftKey',
+};
+
 function normalizeKeys(keys: string | string[]): string[] {
   return asArray(keys).map((key) => key.toLowerCase());
 }
@@ -285,11 +296,9 @@ class KeyBindingController implements ReactiveController {
       return Boolean(getElementFromPath(this._skipSelector, event));
     }
 
-    if (isFunction(skip)) {
-      return skip.call(this._host, event.target as Element, event);
-    }
-
-    return false;
+    return isFunction(skip)
+      ? skip.call(this._host, event.target as Element, event)
+      : false;
   }
 
   //#endregion
@@ -335,6 +344,11 @@ class KeyBindingController implements ReactiveController {
    */
   private _handleKeyEvent(event: KeyboardEvent): void {
     if (this._shouldSkip(event)) {
+      // Always clean up on keyup regardless of whether the event is otherwise skipped.
+      const key = event.key.toLowerCase();
+      if (!MODIFIERS.has(key) && isKeyup(event)) {
+        this._pressedKeys.delete(key);
+      }
       return;
     }
 
@@ -345,14 +359,24 @@ class KeyBindingController implements ReactiveController {
     }
 
     const activeModifiers = ALL_MODIFIER_VALUES.filter(
-      (mod) => event[`${mod}Key` as keyof KeyboardEvent]
+      (mod) => event[MODIFIER_EVENT_KEYS[mod] as keyof KeyboardEvent]
     );
 
     const combination = createCombinationKey(
       Array.from(this._pressedKeys),
       activeModifiers
     );
-    const binding = this._bindings.get(combination);
+    let binding = this._bindings.get(combination);
+
+    // When multiple non-modifier keys are simultaneously in _pressedKeys (due to overlapping
+    // key presses, e.g. pressing ArrowUp before ArrowDown's keyup fires), the full combination
+    // won't match any single-key binding. Fall back to just the current key + modifiers so that
+    // single-key bindings continue to fire even when other keys are still "held".
+    if (!binding && this._pressedKeys.size > 1) {
+      binding = this._bindings.get(
+        createCombinationKey([key], activeModifiers)
+      );
+    }
 
     if (binding && this._bindingMatches(binding, event)) {
       this._applyEventModifiers(binding, event);
@@ -445,9 +469,11 @@ class KeyBindingController implements ReactiveController {
 
     return {
       unsubscribe: () => {
-        this._observedElement?.removeEventListener('keydown', this);
-        this._observedElement?.removeEventListener('keyup', this);
-        this._observedElement = undefined;
+        element.removeEventListener('keydown', this);
+        element.removeEventListener('keyup', this);
+        if (this._observedElement === element) {
+          this._observedElement = undefined;
+        }
       },
     };
   }

src/components/common/utils.spec.ts

@@ -7,7 +7,7 @@ import {
 } from '@open-wc/testing';
 import type { TemplateResult } from 'lit';
 import { type CalendarDay, toCalendarDay } from '../calendar/model.js';
-import { parseKeys } from './controllers/key-bindings.js';
+import { MODIFIER_EVENT_KEYS, parseKeys } from './controllers/key-bindings.js';
 import type { IgcFormControl } from './mixins/forms/types.js';
 import { toKebabCase } from './util.js';
 
@@ -348,7 +348,7 @@ export function simulateKeyboard(
   const eventOptions: Record<string, boolean> = {};
 
   for (const each of modifiers) {
-    eventOptions[`${each}Key`] = true;
+    eventOptions[MODIFIER_EVENT_KEYS[each]] = true;
   }
 
   for (const key of keys) {