From c093829b4e454e040c951efed21177cfc3501fed Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Sat, 30 Oct 2021 22:14:07 -0700
Subject: [PATCH 01/11] Revert "Add arrow glyphs #151"

This reverts commit 57a6ee71867d5bb99fc2a59d9df8df0e64dae9ff.

It was not clear in issue #151 calling for a change in formatting of arrows that arrows should not be treated as modifier keys.
---
 packages/commands/src/index.ts | 56 +---------------------------------
 1 file changed, 1 insertion(+), 55 deletions(-)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index 3db49daed..81aad73ac 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1062,26 +1062,6 @@ export namespace CommandRegistry {
      */
     shift: boolean;
 
-    /**
-     * Whether `'ArrowLeft'` appears in the keystroke.
-     */
-    arrowLeft: boolean;
-
-    /**
-     * Whether `'ArrowUp'` appears in the keystroke.
-     */
-    arrowUp: boolean;
-
-    /**
-     * Whether `'ArrowRight'` appears in the keystroke.
-     */
-    arrowRight: boolean;
-
-    /**
-     * Whether `'ArrowDown'` appears in the keystroke.
-     */
-    arrowDown: boolean;
-
     /**
      * The primary key for the keystroke.
      */
@@ -1116,10 +1096,6 @@ export namespace CommandRegistry {
     let cmd = false;
     let ctrl = false;
     let shift = false;
-    let arrowLeft = false;
-    let arrowUp = false;
-    let arrowRight = false;
-    let arrowDown = false;
     for (let token of keystroke.split(/\s+/)) {
       if (token === 'Accel') {
         if (Platform.IS_MAC) {
@@ -1135,29 +1111,11 @@ export namespace CommandRegistry {
         ctrl = true;
       } else if (token === 'Shift') {
         shift = true;
-      } else if (token === 'ArrowLeft') {
-        arrowLeft = true;
-      } else if (token === 'ArrowUp') {
-        arrowUp = true;
-      } else if (token === 'ArrowRight') {
-        arrowRight = true;
-      } else if (token === 'ArrowDown') {
-        arrowDown = true;
       } else if (token.length > 0) {
         key = token;
       }
     }
-    return {
-      cmd,
-      ctrl,
-      alt,
-      shift,
-      key,
-      arrowLeft,
-      arrowUp,
-      arrowRight,
-      arrowDown
-    };
+    return { cmd, ctrl, alt, shift, key };
   }
 
   /**
@@ -1229,18 +1187,6 @@ export namespace CommandRegistry {
       if (parts.cmd) {
         mods += '\u2318 ';
       }
-      if (parts.arrowLeft) {
-        mods += '\u2190 ';
-      }
-      if (parts.arrowUp) {
-        mods += '\u2191 ';
-      }
-      if (parts.arrowRight) {
-        mods += '\u2192 ';
-      }
-      if (parts.arrowDown) {
-        mods += '\u2193 ';
-      }
     } else {
       if (parts.ctrl) {
         mods += 'Ctrl+';

From 0371ee54b1207e2e5bc5c43c2e64b761b8c7aa68 Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Sun, 31 Oct 2021 00:38:08 -0700
Subject: [PATCH 02/11] Format a key for display in the keyboard layout.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This puts the formatting of human-readable strings closer to where the keys are actually defined, and allows new keyboard layouts to extend this formatting for other keys the keyboard may have.

This is exploratory work. It seems that the formatting of specific keys is not standardized across applications, operating systems, or keyboards, so I’m not sure if this will make anyone happier. It does seem like the keyboard layout is the right place for this formatting logic to happen for individual keys in the layout.

I did verify that VoiceOver reads each of the macOS shortcuts correctly in menus (e.g., the curvy right arrow is read as “return” in a menu, etc.)
---
 packages/keyboard/package.json |  3 ++
 packages/keyboard/src/index.ts | 86 +++++++++++++++++++++++++++++++++-
 2 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/packages/keyboard/package.json b/packages/keyboard/package.json
index 4f84a2405..16e7cf9dd 100644
--- a/packages/keyboard/package.json
+++ b/packages/keyboard/package.json
@@ -43,6 +43,9 @@
     "test:ie": "cd tests && karma start --browsers=IE",
     "watch": "tsc --build --watch"
   },
+  "dependencies": {
+    "@lumino/domutils": "^1.8.0"
+  },
   "devDependencies": {
     "@microsoft/api-extractor": "^7.6.0",
     "@types/chai": "^3.4.35",
diff --git a/packages/keyboard/src/index.ts b/packages/keyboard/src/index.ts
index 3e3cd5d01..c906326f1 100644
--- a/packages/keyboard/src/index.ts
+++ b/packages/keyboard/src/index.ts
@@ -8,6 +8,8 @@
 | The full license is in the file LICENSE, distributed with this software.
 |----------------------------------------------------------------------------*/
 
+import { Platform } from '@lumino/domutils';
+
 /**
  * An object which represents an abstract keyboard layout.
  */
@@ -64,6 +66,15 @@ export interface IKeyboardLayout {
    *   does not represent a valid primary key.
    */
   keyForKeydownEvent(event: KeyboardEvent): string;
+
+  /**
+   * Get the formatted string for displaying a key in the user interface.
+   *
+   * @param key - The user provided key.
+   *
+   * @returns A unicode string representing the key in the interface.
+   */
+  formatKey(key: string): string;
 }
 
 /**
@@ -115,12 +126,14 @@ export class KeycodeLayout implements IKeyboardLayout {
   constructor(
     name: string,
     codes: KeycodeLayout.CodeMap,
-    modifierKeys: string[] = []
+    modifierKeys: string[] = [],
+    format: KeycodeLayout.KeyFormat = x => x
   ) {
     this.name = name;
     this._codes = codes;
     this._keys = KeycodeLayout.extractKeys(codes);
     this._modifierKeys = KeycodeLayout.convertToKeySet(modifierKeys);
+    this._format = format;
   }
 
   /**
@@ -171,9 +184,21 @@ export class KeycodeLayout implements IKeyboardLayout {
     return this._codes[event.keyCode] || '';
   }
 
+  /**
+   * Get the formatted string for displaying a key in the user interface.
+   *
+   * @param key - The user provided key.
+   *
+   * @returns A unicode string representing the key in the interface.
+   */
+  formatKey(key: string): string {
+    return this._format(key);
+  }
+
   private _keys: KeycodeLayout.KeySet;
   private _codes: KeycodeLayout.CodeMap;
   private _modifierKeys: KeycodeLayout.KeySet;
+  private _format: KeycodeLayout.KeyFormat;
 }
 
 /**
@@ -190,6 +215,11 @@ export namespace KeycodeLayout {
    */
   export type KeySet = { readonly [key: string]: boolean };
 
+  /**
+   * A type alias for a key format function.
+   */
+  export type KeyFormat = (key: string) => string;
+
   /**
    * Extract the set of keys from a code map.
    *
@@ -221,6 +251,57 @@ export namespace KeycodeLayout {
   }
 }
 
+export const MAC_DISPLAY: { [key: string]: string } = {
+  Backspace: '⌫',
+  Tab: '⇥',
+  Enter: '↩',
+  Shift: '⇧',
+  Ctrl: '⌃',
+  Alt: '⌥',
+  Escape: '⎋',
+  PageUp: '⇞',
+  PageDown: '⇟',
+  End: '↘',
+  Home: '↖',
+  ArrowLeft: '←',
+  ArrowUp: '↑',
+  ArrowRight: '→',
+  ArrowDown: '↓',
+  Delete: '⌦',
+  Meta: '⌘'
+};
+
+export const WIN_DISPLAY: { [key: string]: string } = {
+  // 'Backspace': '⌫',
+  // 'Tab': '⇥',
+  // 'Enter': 'Return',
+  // 'Shift': '⇧',
+  // 'Ctrl': 'Ctrl',
+  // 'Alt': '⌥',
+  // 'Pause': '',
+  Escape: 'Esc',
+  // 'Space': '␣',
+  PageUp: 'Page Up',
+  PageDown: 'Page Down',
+  // 'End': '↘',
+  // 'Home': '↖',
+  ArrowLeft: 'Left',
+  ArrowUp: 'Right',
+  ArrowRight: 'Up',
+  ArrowDown: 'Down',
+  // 'Insert': '',
+  Delete: 'Del'
+  // 'Meta': ''
+};
+
+export function formatKey(key: string): string {
+  if (Platform.IS_MAC) {
+    return MAC_DISPLAY.hasOwnProperty(key) ? MAC_DISPLAY[key] : key;
+  } else {
+    return WIN_DISPLAY.hasOwnProperty(key) ? WIN_DISPLAY[key] : key;
+  }
+}
+
 /**
  * A keycode-based keyboard layout for US English keyboards.
  *
@@ -345,7 +426,8 @@ export const EN_US: IKeyboardLayout = new KeycodeLayout(
     222: "'",
     224: 'Meta' // firefox
   },
-  ['Shift', 'Ctrl', 'Alt', 'Meta'] // modifier keys
+  ['Shift', 'Ctrl', 'Alt', 'Meta'], // modifier keys,
+  formatKey
 );
 
 /**

From 552152552ecf37b7656e55adb954e271a1292b0d Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 01:59:57 -0700
Subject: [PATCH 03/11] More work on using modifier keys from the layout
 instead of hardcoding them.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This adds a modifierKeys attribute to the keyboard layout so we can iterate over the modifier keys. Since we often want to our modifier keys displayed in a specific order, we need the modifier keys to be an ordered set, but Object.keys ordering is not guaranteed until ES2020, I believe. Therefore I converted the modifier keys to a Set, which does iterate in insertion order for sure.

Finally, we make a kind of awkward special case for Command/Meta treatment on macos so that things are backwards-compatible with the current behavior.

There are some blockers to further progress using getModifierState to determine modifier keys:

1. It does not appear that we can easily clone a KeyboardEvent and clone the getModifierState function, at least not as easily as we did for the standardized ctrl, shift, alt, etc. A way forward is that we could keep a reference to the original event and proxy getModifierState calls to the original event.
2. The current keyboard layout specifies the control key as “Ctrl”, not the standardized “Control” used in getModifierState. This is so that users can specify their keyboard shortcuts using “Ctrl” instead of “Control”. To make more progress here, we could use “Control” in the keyboard layout, and carefully trace where “Ctrl” was used and convert it to “Control” at the appropriate places.
---
 packages/commands/src/index.ts  | 67 ++++++++++++++-------------------
 packages/keyboard/src/index.ts  | 42 +++++++++++++++------
 packages/keyboard/tsconfig.json |  2 +-
 3 files changed, 61 insertions(+), 50 deletions(-)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index 81aad73ac..801dc91e1 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1172,33 +1172,25 @@ export namespace CommandRegistry {
    * Format a keystroke for display on the local system.
    */
   export function formatKeystroke(keystroke: string): string {
-    let mods = '';
     let parts = parseKeystroke(keystroke);
-    if (Platform.IS_MAC) {
-      if (parts.ctrl) {
-        mods += '\u2303 ';
-      }
-      if (parts.alt) {
-        mods += '\u2325 ';
-      }
-      if (parts.shift) {
-        mods += '\u21E7 ';
-      }
-      if (parts.cmd) {
-        mods += '\u2318 ';
-      }
-    } else {
-      if (parts.ctrl) {
-        mods += 'Ctrl+';
-      }
-      if (parts.alt) {
-        mods += 'Alt+';
-      }
-      if (parts.shift) {
-        mods += 'Shift+';
-      }
+    let layout = getKeyboardLayout();
+    let label = [];
+    let separator = Platform.IS_MAC ? '+' : ' ';
+    if (parts.ctrl) {
+      label.push('Ctrl');
     }
-    return mods + parts.key;
+    if (parts.alt) {
+      label.push('Alt');
+    }
+    if (parts.shift) {
+      label.push('Shift');
+    }
+    if (Platform.IS_MAC && parts.cmd) {
+      // Keyboard layouts label Command as Meta
+      label.push('Meta');
+    }
+    label.push(parts.key);
+    return label.map(layout.formatKey).join(separator);
   }
 
   /**
@@ -1228,20 +1220,19 @@ export namespace CommandRegistry {
     if (!key || layout.isModifierKey(key)) {
       return '';
     }
-    let mods = '';
-    if (event.ctrlKey) {
-      mods += 'Ctrl ';
-    }
-    if (event.altKey) {
-      mods += 'Alt ';
-    }
-    if (event.shiftKey) {
-      mods += 'Shift ';
-    }
-    if (event.metaKey && Platform.IS_MAC) {
-      mods += 'Cmd ';
+    // Loop through modifier keys in order to test them
+    let mods = [];
+    for (let mod in layout.modifierKeys()) {
+      // Special treatment for Meta (Cmd on macOS) for backwards compatibility
+      if (mod === 'Meta') {
+        if (Platform.IS_MAC && event.getModifierState(mod)) {
+          mods.push('Cmd');
+        }
+      } else if (event.getModifierState(mod)) {
+        mods.push(mod);
+      }
     }
-    return mods + key;
+    return mods.join(" ") + " " + key;
   }
 }
 
diff --git a/packages/keyboard/src/index.ts b/packages/keyboard/src/index.ts
index c906326f1..4648697cc 100644
--- a/packages/keyboard/src/index.ts
+++ b/packages/keyboard/src/index.ts
@@ -32,6 +32,17 @@ export interface IKeyboardLayout {
    */
   keys(): string[];
 
+  /**
+   * Get an arrow of all modifier key values supported by the layout.
+   *
+   * @returns A new array of the supported modifier key values.
+   *
+   * #### Notes
+   * This can be useful for authoring tools and debugging, when it's
+   * necessary to know which modifier keys are available for shortcut use.
+   */
+  modifierKeys(): string[];
+
   /**
    * Test whether the given key is a valid value for the layout.
    *
@@ -42,18 +53,17 @@ export interface IKeyboardLayout {
   isValidKey(key: string): boolean;
 
   /**
-   * Test whether the given key is a modifier key.
+   * Test whether the given key is a valid modifier key for the layout.
    *
-   * @param key - The user provided key.
+   * @param key - The user provided key to test for validity.
    *
    * @returns `true` if the key is a modifier key, `false` otherwise.
    *
    * #### Notes
-   * This is necessary so that we don't process modifier keys pressed
-   * in the middle of the key sequence.
-   * E.g. "Shift C Ctrl P" is actually 4 keydown events:
-   *   "Shift", "Shift P", "Ctrl", "Ctrl P",
-   * and events for "Shift" and "Ctrl" should be ignored.
+   * This may be useful to determine whether we should ignore a keypress
+   * event in the middle of a key sequence. For example, "Shift C Ctrl P" is
+   * actually four keydown events: "Shift, "Shift P", "Ctrl", "Ctrl P", but
+   * the keypress events for the modifier keys "Shift" and "Ctrl" are ignored.
    */
   isModifierKey(key: string): boolean;
 
@@ -132,7 +142,7 @@ export class KeycodeLayout implements IKeyboardLayout {
     this.name = name;
     this._codes = codes;
     this._keys = KeycodeLayout.extractKeys(codes);
-    this._modifierKeys = KeycodeLayout.convertToKeySet(modifierKeys);
+    this._modifierKeys = new Set<string>(modifierKeys);
     this._format = format;
   }
 
@@ -150,6 +160,15 @@ export class KeycodeLayout implements IKeyboardLayout {
     return Object.keys(this._keys);
   }
 
+  /**
+   * Get an arrow of all modifier key values supported by the layout.
+   *
+   * @returns A new array of the supported modifier key values.
+   */
+  modifierKeys(): string[] {
+    return Array.from(this._modifierKeys);
+  }
+
   /**
    * Test whether the given key is a valid value for the layout.
    *
@@ -169,7 +188,7 @@ export class KeycodeLayout implements IKeyboardLayout {
    * @returns `true` if the key is a modifier key, `false` otherwise.
    */
   isModifierKey(key: string): boolean {
-    return key in this._modifierKeys;
+    return this._modifierKeys.has(key);
   }
 
   /**
@@ -197,7 +216,7 @@ export class KeycodeLayout implements IKeyboardLayout {
 
   private _keys: KeycodeLayout.KeySet;
   private _codes: KeycodeLayout.CodeMap;
-  private _modifierKeys: KeycodeLayout.KeySet;
+  private _modifierKeys: Set<string>;
   private _format: KeycodeLayout.KeyFormat;
 }
 
@@ -426,7 +445,8 @@ export const EN_US: IKeyboardLayout = new KeycodeLayout(
     222: "'",
     224: 'Meta' // firefox
   },
-  ['Shift', 'Ctrl', 'Alt', 'Meta'], // modifier keys,
+  // The modifier is labeled "Control", but the key value is "Ctrl"?
+  ['Ctrl', 'Alt', 'Shift', 'Meta'], // modifier keys in display order
   formatKey
 );
 
diff --git a/packages/keyboard/tsconfig.json b/packages/keyboard/tsconfig.json
index 17f720517..6f983ed21 100644
--- a/packages/keyboard/tsconfig.json
+++ b/packages/keyboard/tsconfig.json
@@ -13,7 +13,7 @@
     "moduleResolution": "node",
     "target": "ES5",
     "outDir": "lib",
-    "lib": ["ES5", "DOM", "ES2015.Iterable"],
+    "lib": ["ES5", "DOM", "ES2015.Collection", "ES2015.Iterable"],
     "importHelpers": true,
     "types": [],
     "rootDir": "src"

From 03db23ef06aeb25a7d49a56d0c9fbeabd863d008 Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 02:02:54 -0700
Subject: [PATCH 04/11] Propagate the getModifierState method in keyboard event
 clones.

---
 packages/commands/src/index.ts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index 801dc91e1..e91cc813a 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1572,6 +1572,7 @@ namespace Private {
     clone.shiftKey = event.shiftKey || false;
     clone.metaKey = event.metaKey || false;
     clone.view = event.view || window;
+    clone.getModifierState = (key: string) => event.getModifierState(key);
     return clone as KeyboardEvent;
   }
 }

From bfdf92620a5822ee72fb7298858c58909ebfb0c5 Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 02:15:09 -0700
Subject: [PATCH 05/11] Change the Control modifier in the keyboard layout from
 Ctrl to Control.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We still keep the user-facing label as Ctrl for now for compatibility.

This may be considered a breaking change in the keyboard package, since the layout has been changed to reference ‘Control’.
---
 packages/commands/src/index.ts | 10 ++++++++--
 packages/keyboard/src/index.ts |  4 ++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index e91cc813a..eb6da7aac 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1177,7 +1177,8 @@ export namespace CommandRegistry {
     let label = [];
     let separator = Platform.IS_MAC ? '+' : ' ';
     if (parts.ctrl) {
-      label.push('Ctrl');
+      // Keyboard layouts label Ctrl as Control
+      label.push('Control');
     }
     if (parts.alt) {
       label.push('Alt');
@@ -1229,7 +1230,12 @@ export namespace CommandRegistry {
           mods.push('Cmd');
         }
       } else if (event.getModifierState(mod)) {
-        mods.push(mod);
+        if (mod === 'Control') {
+          // For backwards compatibility, we use Ctrl for the Control modifier
+          mods.push('Ctrl');
+        } else {
+          mods.push(mod);
+        }
       }
     }
     return mods.join(" ") + " " + key;
diff --git a/packages/keyboard/src/index.ts b/packages/keyboard/src/index.ts
index 4648697cc..731bb2f05 100644
--- a/packages/keyboard/src/index.ts
+++ b/packages/keyboard/src/index.ts
@@ -349,7 +349,7 @@ export const EN_US: IKeyboardLayout = new KeycodeLayout(
     9: 'Tab',
     13: 'Enter',
     16: 'Shift',
-    17: 'Ctrl',
+    17: 'Control',
     18: 'Alt',
     19: 'Pause',
     27: 'Escape',
@@ -446,7 +446,7 @@ export const EN_US: IKeyboardLayout = new KeycodeLayout(
     224: 'Meta' // firefox
   },
   // The modifier is labeled "Control", but the key value is "Ctrl"?
-  ['Ctrl', 'Alt', 'Shift', 'Meta'], // modifier keys in display order
+  ['Control', 'Alt', 'Shift', 'Meta'], // modifier keys in display order
   formatKey
 );
 

From f59ced404b0c97bcd437b96944b88e88efdffa81 Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 02:15:50 -0700
Subject: [PATCH 06/11] Revert "Change the Control modifier in the keyboard
 layout from Ctrl to Control."

This reverts commit bfdf92620a5822ee72fb7298858c58909ebfb0c5.
---
 packages/commands/src/index.ts | 10 ++--------
 packages/keyboard/src/index.ts |  4 ++--
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index eb6da7aac..e91cc813a 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1177,8 +1177,7 @@ export namespace CommandRegistry {
     let label = [];
     let separator = Platform.IS_MAC ? '+' : ' ';
     if (parts.ctrl) {
-      // Keyboard layouts label Ctrl as Control
-      label.push('Control');
+      label.push('Ctrl');
     }
     if (parts.alt) {
       label.push('Alt');
@@ -1230,12 +1229,7 @@ export namespace CommandRegistry {
           mods.push('Cmd');
         }
       } else if (event.getModifierState(mod)) {
-        if (mod === 'Control') {
-          // For backwards compatibility, we use Ctrl for the Control modifier
-          mods.push('Ctrl');
-        } else {
-          mods.push(mod);
-        }
+        mods.push(mod);
       }
     }
     return mods.join(" ") + " " + key;
diff --git a/packages/keyboard/src/index.ts b/packages/keyboard/src/index.ts
index 731bb2f05..4648697cc 100644
--- a/packages/keyboard/src/index.ts
+++ b/packages/keyboard/src/index.ts
@@ -349,7 +349,7 @@ export const EN_US: IKeyboardLayout = new KeycodeLayout(
     9: 'Tab',
     13: 'Enter',
     16: 'Shift',
-    17: 'Control',
+    17: 'Ctrl',
     18: 'Alt',
     19: 'Pause',
     27: 'Escape',
@@ -446,7 +446,7 @@ export const EN_US: IKeyboardLayout = new KeycodeLayout(
     224: 'Meta' // firefox
   },
   // The modifier is labeled "Control", but the key value is "Ctrl"?
-  ['Control', 'Alt', 'Shift', 'Meta'], // modifier keys in display order
+  ['Ctrl', 'Alt', 'Shift', 'Meta'], // modifier keys in display order
   formatKey
 );
 

From a8bf2250ace43dd1bfefa567a290d217d3d27012 Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 02:19:14 -0700
Subject: [PATCH 07/11] Convert Ctrl to Control when checking the modifier
 state.

This preserves backwards compatibility for the keyboard layout, while still switching to getModifierState for checking the modifier state.
---
 packages/commands/src/index.ts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index e91cc813a..1dbbb0615 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1228,6 +1228,11 @@ export namespace CommandRegistry {
         if (Platform.IS_MAC && event.getModifierState(mod)) {
           mods.push('Cmd');
         }
+      } else if (mod === 'Ctrl') {
+        // For backwards compatibility, our keyboard layout still uses Ctrl.
+        if (event.getModifierState('Control')) {
+          mods.push('Ctrl');
+        }
       } else if (event.getModifierState(mod)) {
         mods.push(mod);
       }

From d15b22ef93c41995df22f301cf42f7512db10dfb Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 03:32:48 -0700
Subject: [PATCH 08/11] Fix test generation and an array iteration typo

---
 packages/commands/src/index.ts            |  2 +-
 packages/commands/tests/src/index.spec.ts | 97 ++++++++++++++---------
 2 files changed, 62 insertions(+), 37 deletions(-)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index 1dbbb0615..f78ebc46a 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1222,7 +1222,7 @@ export namespace CommandRegistry {
     }
     // Loop through modifier keys in order to test them
     let mods = [];
-    for (let mod in layout.modifierKeys()) {
+    for (let mod of layout.modifierKeys()) {
       // Special treatment for Meta (Cmd on macOS) for backwards compatibility
       if (mod === 'Meta') {
         if (Platform.IS_MAC && event.getModifierState(mod)) {
diff --git a/packages/commands/tests/src/index.spec.ts b/packages/commands/tests/src/index.spec.ts
index fd8bd8235..35a8a5385 100644
--- a/packages/commands/tests/src/index.spec.ts
+++ b/packages/commands/tests/src/index.spec.ts
@@ -26,6 +26,30 @@ const NULL_COMMAND = {
   }
 };
 
+function generateKeydown(options: any) {
+  let event = generate('keydown', options);
+  (event as any).getModifierState = (key: string) => {
+    let state = false;
+    switch (key) {
+      case 'Alt':
+        state = options.altKey || false;
+        break;
+      case 'Control':
+        state = options.ctrlKey || false;
+        break;
+      case 'Shift':
+        state = options.shiftKey || false;
+        break;
+      case 'Meta':
+        state = options.metaKey || false;
+        break;
+    }
+    console.log(`modifier state ${key} ${state}`)
+    return state;
+  };
+  return event;
+}
+
 describe('@lumino/commands', () => {
   describe('CommandRegistry', () => {
     let registry: CommandRegistry = null!;
@@ -581,7 +605,7 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let event = generate('keydown', { keyCode: 59, ctrlKey: true });
+        let event = generateKeydown({ keyCode: 59, ctrlKey: true });
         elem.dispatchEvent(event);
         expect(called).to.equal(true);
       });
@@ -599,7 +623,7 @@ describe('@lumino/commands', () => {
           command: 'test'
         });
         binding.dispose();
-        let event = generate('keydown', { keyCode: 59, ctrlKey: true });
+        let event = generateKeydown({ keyCode: 59, ctrlKey: true });
         elem.dispatchEvent(event);
         expect(called).to.equal(false);
       });
@@ -641,7 +665,7 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let event = generate('keydown', { keyCode: 59, ctrlKey: true });
+        let event = generateKeydown({ keyCode: 59, ctrlKey: true });
         elem.dispatchEvent(event);
         expect(called).to.equal(true);
       });
@@ -659,7 +683,7 @@ describe('@lumino/commands', () => {
           command: 'test'
         });
         parent.setAttribute('data-lm-suppress-shortcuts', 'true');
-        let event = generate('keydown', { keyCode: 59, ctrlKey: true });
+        let event = generateKeydown({ keyCode: 59, ctrlKey: true });
         elem.dispatchEvent(event);
         expect(called).to.equal(false);
       });
@@ -676,7 +700,7 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let event = generate('keydown', { keyCode: 45, ctrlKey: true });
+        let event = generateKeydown({ keyCode: 45, ctrlKey: true });
         elem.dispatchEvent(event);
         expect(called).to.equal(false);
       });
@@ -693,8 +717,8 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let eventAlt = generate('keydown', { keyCode: 83, altKey: true });
-        let eventShift = generate('keydown', { keyCode: 83, shiftKey: true });
+        let eventAlt = generateKeydown({ keyCode: 83, altKey: true });
+        let eventShift = generateKeydown({ keyCode: 83, shiftKey: true });
         elem.dispatchEvent(eventAlt);
         expect(count).to.equal(0);
         elem.dispatchEvent(eventShift);
@@ -713,17 +737,17 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let eventK = generate('keydown', { keyCode: 75, ctrlKey: true });
-        let eventL = generate('keydown', { keyCode: 76, ctrlKey: true });
+        let eventK = generateKeydown({ keyCode: 75, ctrlKey: true });
+        let eventL = generateKeydown({ keyCode: 76, ctrlKey: true });
         elem.dispatchEvent(eventK);
         expect(count).to.equal(0);
         elem.dispatchEvent(eventL);
         expect(count).to.equal(1);
-        elem.dispatchEvent(generate('keydown', eventL)); // Don't reuse; clone.
+        elem.dispatchEvent(generateKeydown(eventL)); // Don't reuse; clone.
         expect(count).to.equal(1);
-        elem.dispatchEvent(generate('keydown', eventK)); // Don't reuse; clone.
+        elem.dispatchEvent(generateKeydown(eventK)); // Don't reuse; clone.
         expect(count).to.equal(1);
-        elem.dispatchEvent(generate('keydown', eventL)); // Don't reuse; clone.
+        elem.dispatchEvent(generateKeydown(eventL)); // Don't reuse; clone.
         expect(count).to.equal(2);
       });
 
@@ -739,7 +763,7 @@ describe('@lumino/commands', () => {
           selector: '.inaccessible-scope',
           command: 'test'
         });
-        let event = generate('keydown', { keyCode: 80, shiftKey: true });
+        let event = generateKeydown({ keyCode: 80, shiftKey: true });
         expect(count).to.equal(0);
         elem.dispatchEvent(event);
         expect(count).to.equal(0);
@@ -757,7 +781,7 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let event = generate('keydown', { keyCode: 17 });
+        let event = generateKeydown({ keyCode: 17 });
         expect(count).to.equal(0);
         elem.dispatchEvent(event);
         expect(count).to.equal(0);
@@ -786,8 +810,8 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test2'
         });
-        let event1 = generate('keydown', { keyCode: 83, ctrlKey: true });
-        let event2 = generate('keydown', { keyCode: 68, ctrlKey: true });
+        let event1 = generateKeydown({ keyCode: 83, ctrlKey: true });
+        let event2 = generateKeydown({ keyCode: 68, ctrlKey: true });
         expect(count1).to.equal(0);
         expect(count2).to.equal(0);
         elem.dispatchEvent(event1);
@@ -821,13 +845,13 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test2'
         });
-        let event1 = generate('keydown', {
+        let event1 = generateKeydown({
           keyCode: 84,
           ctrlKey: true,
           altKey: true,
           shiftKey: true
         });
-        let event2 = generate('keydown', {
+        let event2 = generateKeydown({
           keyCode: 81,
           ctrlKey: true,
           altKey: true,
@@ -858,9 +882,10 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let event1 = generate('keydown', { keyCode: 68 });
-        let event2 = generate('keydown', { keyCode: 69 });
+        let event1 = generateKeydown({ keyCode: 68 });
+        let event2 = generateKeydown({ keyCode: 69 });
         elem.dispatchEvent(event1);
+        console.log(codes);
         expect(codes.length).to.equal(0);
         elem.dispatchEvent(event2);
         expect(called).to.equal(false);
@@ -885,7 +910,7 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let event = generate('keydown', { keyCode: 68 });
+        let event = generateKeydown({ keyCode: 68 });
         elem.dispatchEvent(event);
         expect(codes.length).to.equal(0);
         setTimeout(() => {
@@ -919,7 +944,7 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test2'
         });
-        let event = generate('keydown', { keyCode: 68 });
+        let event = generateKeydown({ keyCode: 68 });
         elem.dispatchEvent(event);
         expect(called1).to.equal(false);
         expect(called2).to.equal(false);
@@ -954,7 +979,7 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test2'
         });
-        let event = generate('keydown', { keyCode: 59, ctrlKey: true });
+        let event = generateKeydown({ keyCode: 59, ctrlKey: true });
         elem.dispatchEvent(event);
         expect(called1).to.equal(false);
         expect(called2).to.equal(true);
@@ -977,7 +1002,7 @@ describe('@lumino/commands', () => {
           selector: '#baz',
           command: 'test'
         });
-        let event = generate('keydown', { keyCode: 68 });
+        let event = generateKeydown({ keyCode: 68 });
         elem.dispatchEvent(event);
         expect(codes).to.deep.equal([68]);
         expect(called).to.equal(false);
@@ -1001,7 +1026,7 @@ describe('@lumino/commands', () => {
           selector: '#baz',
           command: 'test'
         });
-        let event = generate('keydown', { keyCode: 68 });
+        let event = generateKeydown({ keyCode: 68 });
         elem.dispatchEvent(event);
         expect(codes).to.deep.equal([68]);
         expect(called).to.equal(false);
@@ -1020,9 +1045,9 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let eventK = generate('keydown', { keyCode: 75, ctrlKey: true });
-        let eventCtrl = generate('keydown', { keyCode: 17, ctrlKey: true });
-        let eventL = generate('keydown', { keyCode: 76, ctrlKey: true });
+        let eventK = generateKeydown({ keyCode: 75, ctrlKey: true });
+        let eventCtrl = generateKeydown({ keyCode: 17, ctrlKey: true });
+        let eventL = generateKeydown({ keyCode: 76, ctrlKey: true });
         elem.dispatchEvent(eventK);
         expect(count).to.equal(0);
         elem.dispatchEvent(eventCtrl); // user presses Ctrl again - this should not break the sequence
@@ -1043,10 +1068,10 @@ describe('@lumino/commands', () => {
           selector: `#${elem.id}`,
           command: 'test'
         });
-        let eventShift = generate('keydown', { keyCode: 16, shiftlKey: true });
-        let eventK = generate('keydown', { keyCode: 75, shiftKey: true });
-        let eventCtrl = generate('keydown', { keyCode: 17, ctrlKey: true });
-        let eventL = generate('keydown', { keyCode: 76, ctrlKey: true });
+        let eventShift = generateKeydown({ keyCode: 16, shiftlKey: true });
+        let eventK = generateKeydown({ keyCode: 75, shiftKey: true });
+        let eventCtrl = generateKeydown({ keyCode: 17, ctrlKey: true });
+        let eventL = generateKeydown({ keyCode: 76, ctrlKey: true });
         elem.dispatchEvent(eventShift);
         expect(count).to.equal(0);
         elem.dispatchEvent(eventK);
@@ -1107,7 +1132,7 @@ describe('@lumino/commands', () => {
 
     describe('.keystrokeForKeydownEvent()', () => {
       it('should create a normalized keystroke', () => {
-        let event = generate('keydown', { ctrlKey: true, keyCode: 83 });
+        let event = generateKeydown({ ctrlKey: true, keyCode: 83 });
         let keystroke = CommandRegistry.keystrokeForKeydownEvent(
           event as KeyboardEvent
         );
@@ -1115,7 +1140,7 @@ describe('@lumino/commands', () => {
       });
 
       it('should handle multiple modifiers', () => {
-        let event = generate('keydown', {
+        let event = generateKeydown({
           ctrlKey: true,
           altKey: true,
           shiftKey: true,
@@ -1128,7 +1153,7 @@ describe('@lumino/commands', () => {
       });
 
       it('should fail on an invalid shortcut', () => {
-        let event = generate('keydown', { keyCode: -1 });
+        let event = generateKeydown({ keyCode: -1 });
         let keystroke = CommandRegistry.keystrokeForKeydownEvent(
           event as KeyboardEvent
         );
@@ -1136,7 +1161,7 @@ describe('@lumino/commands', () => {
       });
 
       it('should return nothing for keys that are marked as modifier in keyboard layout', () => {
-        let event = generate('keydown', { keyCode: 17, ctrlKey: true });
+        let event = generateKeydown({ keyCode: 17, ctrlKey: true });
         let keystroke = CommandRegistry.keystrokeForKeydownEvent(
           event as KeyboardEvent
         );

From be5d3fff29bc8810fab7af51e423577dea2f22ea Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 03:49:15 -0700
Subject: [PATCH 09/11] Fix shortcut formatting.

---
 packages/commands/src/index.ts | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index f78ebc46a..3b3f6d672 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1190,7 +1190,7 @@ export namespace CommandRegistry {
       label.push('Meta');
     }
     label.push(parts.key);
-    return label.map(layout.formatKey).join(separator);
+    return label.map(k => layout.formatKey(k)).join(separator);
   }
 
   /**
@@ -1237,7 +1237,8 @@ export namespace CommandRegistry {
         mods.push(mod);
       }
     }
-    return mods.join(" ") + " " + key;
+    mods.push(key);
+    return mods.join(' ');
   }
 }
 

From 4acd80ed058521218d7bacd06add7a49ea500d68 Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 03:57:33 -0700
Subject: [PATCH 10/11] Fix platform separator for keyboard shortcuts.

---
 packages/commands/src/index.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
index 3b3f6d672..d0ee91a4a 100644
--- a/packages/commands/src/index.ts
+++ b/packages/commands/src/index.ts
@@ -1175,7 +1175,7 @@ export namespace CommandRegistry {
     let parts = parseKeystroke(keystroke);
     let layout = getKeyboardLayout();
     let label = [];
-    let separator = Platform.IS_MAC ? '+' : ' ';
+    let separator = Platform.IS_MAC ? ' ' : '+';
     if (parts.ctrl) {
       label.push('Ctrl');
     }

From 34c3d92374e02be10dbeb1129338d0232a1a68a6 Mon Sep 17 00:00:00 2001
From: Jason Grout <jgrout6@bloomberg.net>
Date: Tue, 2 Nov 2021 04:06:07 -0700
Subject: [PATCH 11/11] Delete debugging output

---
 packages/commands/tests/src/index.spec.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/packages/commands/tests/src/index.spec.ts b/packages/commands/tests/src/index.spec.ts
index 35a8a5385..4ae79bd66 100644
--- a/packages/commands/tests/src/index.spec.ts
+++ b/packages/commands/tests/src/index.spec.ts
@@ -44,7 +44,6 @@ function generateKeydown(options: any) {
         state = options.metaKey || false;
         break;
     }
-    console.log(`modifier state ${key} ${state}`)
     return state;
   };
   return event;