Merge pull request #1691 from bilawalsidhu/said/shortcuts-ignore-modifier-chords

fix(shortcuts): a shortcut key held with Cmd, Ctrl or Alt runs no shortcut

changelog.d/1691-shortcuts-ignore-modifier-chords.md

@@ -0,0 +1,5 @@
+- Leave browser and system chords alone: a shortcut key held with Cmd, Ctrl or
+  Alt no longer runs its single-key shortcut. Copying with Cmd+C or Ctrl+C used
+  to turn the CCTV layer on and open the nearest camera, Cmd+F opened the layers
+  panel and Cmd+1 to 6 changed the visual style. The Q/W/E/R/T keys of an
+  expanded city's points of interest follow the same rule (asturksever, #1691).

docs/CURRENT-STATE.md

@@ -2548,8 +2548,13 @@ Browser acceptance: `QA_BASE_URL=http://127.0.0.1:4173 node scripts/qa-segmented
 
 `ui/input` supplies the bubbling application shortcut listener and generated
 shader-parameter rows. Number/style keys, H/O/V/F/D/C actions, native form-control
-typing and Escape behavior retain their existing mappings. Capture-phase
-surfaces continue to arbitrate their own keyboard events first.
+typing and Escape behavior retain their existing mappings. A shortcut key held
+with Cmd, Ctrl or Alt runs no shortcut, so the browser's own chord (Cmd+C to
+copy, Cmd+F to find, Cmd+1 to switch tab) does not also toggle CCTV, the layers
+panel or a style; Shift does not count, and Escape dismisses search either way.
+The Q/W/E/R/T keys of an expanded city's points of interest follow the same
+rule.
+Capture-phase surfaces continue to arbitrate their own keyboard events first.
 
 The UI facade retains shader values, share-restore authority, render requests,
 search dismissal, visibility and Cockpit portal policy. Parameter rows preserve

src/ui/applicationShortcuts.js

@@ -10,7 +10,8 @@ const STYLE_KEYS = Object.freeze({
 /**
  * Bind the application's existing bubbling keyboard shortcuts.
  * Capture-phase surfaces keep first refusal. The caller owns each action and
- * persistence; form controls keep native typing except for Escape.
+ * persistence; form controls keep native typing except for Escape, and a key
+ * held with Cmd, Ctrl or Alt is left to the browser.
  * @param {object} options
  * @param {Document} options.documentRef Keyboard event target.
  * @param {HTMLElement} options.searchInput Additional editing target.
@@ -27,6 +28,13 @@ export function bindApplicationShortcuts({
       event.target?.matches?.('select, input, textarea') ||
       event.target === searchInput;
     if (isFormControl && event.key !== 'Escape') return;
+    // Cmd+C, Ctrl+F, Cmd+1 and the like belong to the browser and the system.
+    // Shift is not a chord here: Shift+C is still C.
+    if (
+      (event.metaKey || event.ctrlKey || event.altKey) &&
+      event.key !== 'Escape'
+    )
+      return;
 
     if (STYLE_KEYS[event.key]) actions.setStyle(STYLE_KEYS[event.key]);
     if (event.key === 'Escape') actions.dismissSearch();

src/ui/locationControls.js

@@ -88,6 +88,9 @@ export class LocationControls {
         event.target === elements.search
       )
         return;
+      // As for the application shortcuts: Cmd+R, Ctrl+W, Alt+E and the like
+      // belong to the browser and the system. Shift is not a chord.
+      if (event.metaKey || event.ctrlKey || event.altKey) return;
       const index = POI_KEYS.indexOf(event.key.toUpperCase());
       if (index !== -1 && index < (cities[cityId]?.pois.length || 0))
         onPoi(cityId, index);

src/ui/locationControls.test.mjs

@@ -191,6 +191,16 @@ test('location and POI keys route once while form controls retain typing', () =>
   f.controls.reset();
   assert.deepEqual(f.calls, [['poi', 'a', 1], ['reset']]);
 });
+test('POI keys held with Cmd, Ctrl or Alt run no POI; Shift still does', () => {
+  const f = fixture();
+  const target = { matches: () => false };
+  for (const modifier of ['metaKey', 'ctrlKey', 'altKey'])
+    for (const key of ['q', 'w', 'r', 'R'])
+      f.doc.fire('keydown', { key, target, [modifier]: true });
+  assert.deepEqual(f.calls, []);
+  f.doc.fire('keydown', { key: 'W', target, shiftKey: true });
+  assert.deepEqual(f.calls, [['poi', 'a', 1]]);
+});
 test('typing clears a failed search error and closing the emptied field drops it too', () => {
   const f = fixture();
   f.elements.search.value = 'Pariss';

src/ui/visualInput.test.mjs

@@ -117,10 +117,23 @@ test('the supplied search target retains editing even without form markup', () =
   assert.deepEqual(f.calls, [['dismissSearch']]);
 });
 
-test('shortcut extraction does not change repeat or modifier policy', () => {
+test('a held key still repeats its shortcut, and Shift is not a chord', () => {
   const f = shortcuts();
-  f.press('h', new Element(), { repeat: true, ctrlKey: true });
-  assert.deepEqual(f.calls, [['toggleHud']]);
+  f.press('h', new Element(), { repeat: true });
+  f.press('C', new Element(), { shiftKey: true });
+  assert.deepEqual(f.calls, [['toggleHud'], ['toggleCctv']]);
+});
+
+test('browser and system chords on a shortcut key run no shortcut', () => {
+  const f = shortcuts();
+  for (const modifier of ['metaKey', 'ctrlKey', 'altKey'])
+    for (const key of ['1', '6', 'h', 'o', 'v', 'f', 'd', 'c', 'C'])
+      f.press(key, new Element(), { [modifier]: true });
+  // Copying (Cmd+C, Ctrl+C) used to turn CCTV on and open the nearest camera.
+  assert.deepEqual(f.calls, []);
+  f.press('c');
+  f.press('Escape', new Element(), { ctrlKey: true });
+  assert.deepEqual(f.calls, [['toggleCctv'], ['dismissSearch']]);
 });
 
 test('destroy synchronously removes shortcuts and a replacement binds once', () => {