feat: Add dynamic keycode for primary modifier key (#9571)

* feat: Add dynamic keycode for primary modifier key

* fix: Don't duplicate constants.

packages/blockly/core/shortcut_items.ts

@@ -134,10 +134,7 @@ function isCuttable(focused: IFocusableNode): boolean {
  */
 export function registerCopy() {
   const ctrlC = ShortcutRegistry.registry.createSerializedKey(KeyCodes.C, [
-    KeyCodes.CTRL,
-  ]);
-  const metaC = ShortcutRegistry.registry.createSerializedKey(KeyCodes.C, [
-    KeyCodes.META,
+    KeyCodes.CTRL_CMD,
   ]);
 
   const copyShortcut: KeyboardShortcut = {
@@ -179,7 +176,7 @@ export function registerCopy() {
           : undefined;
       return !!clipboard.copy(focused, copyCoords);
     },
-    keyCodes: [ctrlC, metaC],
+    keyCodes: [ctrlC],
   };
   ShortcutRegistry.registry.register(copyShortcut);
 }
@@ -189,10 +186,7 @@ export function registerCopy() {
  */
 export function registerCut() {
   const ctrlX = ShortcutRegistry.registry.createSerializedKey(KeyCodes.X, [
-    KeyCodes.CTRL,
-  ]);
-  const metaX = ShortcutRegistry.registry.createSerializedKey(KeyCodes.X, [
-    KeyCodes.META,
+    KeyCodes.CTRL_CMD,
   ]);
 
   const cutShortcut: KeyboardShortcut = {
@@ -224,7 +218,7 @@ export function registerCut() {
       }
       return !!copyData;
     },
-    keyCodes: [ctrlX, metaX],
+    keyCodes: [ctrlX],
   };
 
   ShortcutRegistry.registry.register(cutShortcut);
@@ -235,10 +229,7 @@ export function registerCut() {
  */
 export function registerPaste() {
   const ctrlV = ShortcutRegistry.registry.createSerializedKey(KeyCodes.V, [
-    KeyCodes.CTRL,
-  ]);
-  const metaV = ShortcutRegistry.registry.createSerializedKey(KeyCodes.V, [
-    KeyCodes.META,
+    KeyCodes.CTRL_CMD,
   ]);
 
   const pasteShortcut: KeyboardShortcut = {
@@ -309,7 +300,7 @@ export function registerPaste() {
       const centerCoords = new Coordinate(left + width / 2, top + height / 2);
       return !!clipboard.paste(copyData, targetWorkspace, centerCoords);
     },
-    keyCodes: [ctrlV, metaV],
+    keyCodes: [ctrlV],
   };
 
   ShortcutRegistry.registry.register(pasteShortcut);
@@ -320,10 +311,7 @@ export function registerPaste() {
  */
 export function registerUndo() {
   const ctrlZ = ShortcutRegistry.registry.createSerializedKey(KeyCodes.Z, [
-    KeyCodes.CTRL,
-  ]);
-  const metaZ = ShortcutRegistry.registry.createSerializedKey(KeyCodes.Z, [
-    KeyCodes.META,
+    KeyCodes.CTRL_CMD,
   ]);
 
   const undoShortcut: KeyboardShortcut = {
@@ -342,7 +330,7 @@ export function registerUndo() {
       e.preventDefault();
       return true;
     },
-    keyCodes: [ctrlZ, metaZ],
+    keyCodes: [ctrlZ],
   };
   ShortcutRegistry.registry.register(undoShortcut);
 }
@@ -353,13 +341,10 @@ export function registerUndo() {
  */
 export function registerRedo() {
   const ctrlShiftZ = ShortcutRegistry.registry.createSerializedKey(KeyCodes.Z, [
-    KeyCodes.CTRL,
-    KeyCodes.SHIFT,
-  ]);
-  const metaShiftZ = ShortcutRegistry.registry.createSerializedKey(KeyCodes.Z, [
-    KeyCodes.META,
+    KeyCodes.CTRL_CMD,
     KeyCodes.SHIFT,
   ]);
+
   // Ctrl-y is redo in Windows.  Command-y is never valid on Macs.
   const ctrlY = ShortcutRegistry.registry.createSerializedKey(KeyCodes.Y, [
     KeyCodes.CTRL,
@@ -381,7 +366,7 @@ export function registerRedo() {
       e.preventDefault();
       return true;
     },
-    keyCodes: [ctrlShiftZ, metaShiftZ, ctrlY],
+    keyCodes: [ctrlShiftZ, ctrlY],
   };
   ShortcutRegistry.registry.register(redoShortcut);
 }

packages/blockly/core/utils/keycodes.ts

@@ -4,6 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import * as userAgent from '../utils/useragent.js';
+
 // Former goog.module ID: Blockly.utils.KeyCodes
 
 /* eslint-disable @typescript-eslint/no-duplicate-enum-values */
@@ -151,4 +153,10 @@ export enum KeyCodes {
   // indicates a hardware/bios problem.
   // http://en.community.dell.com/support-forums/laptop/f/3518/p/19285957/19523128.aspx
   PHANTOM = 255,
+
+  // The primary modifier key on the current platform, i.e. Command on Apple
+  // platforms and Control elsewhere.
+  CTRL_CMD = userAgent.MAC || userAgent.IPHONE || userAgent.IPAD
+    ? MAC_WK_CMD_LEFT
+    : CTRL,
 }

packages/blockly/tests/mocha/shortcut_items_test.js

@@ -158,87 +158,42 @@ suite('Keyboard Shortcut Items', function () {
         'hideChaff',
       );
     });
-    const testCases = [
-      [
-        'Control C',
-        createKeyDownEvent(Blockly.utils.KeyCodes.C, [
-          Blockly.utils.KeyCodes.CTRL,
-        ]),
-      ],
-      [
-        'Meta C',
-        createKeyDownEvent(Blockly.utils.KeyCodes.C, [
-          Blockly.utils.KeyCodes.META,
-        ]),
-      ],
-    ];
+    const keyEvent = createKeyDownEvent(Blockly.utils.KeyCodes.C, [
+      Blockly.utils.KeyCodes.CTRL_CMD,
+    ]);
     // Copy a block.
-    suite('Simple', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.calledOnce(this.copySpy);
-          sinon.assert.calledOnce(this.hideChaffSpy);
-        });
-      });
+    test('Simple', function () {
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.calledOnce(this.copySpy);
+      sinon.assert.calledOnce(this.hideChaffSpy);
     });
     // Allow copying a block if a workspace is in readonly mode.
-    suite('Called when readOnly is true', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          this.workspace.setIsReadOnly(true);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.calledOnce(this.copySpy);
-          sinon.assert.calledOnce(this.hideChaffSpy);
-        });
-      });
+    test('Called when readOnly is true', function () {
+      this.workspace.setIsReadOnly(true);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.calledOnce(this.copySpy);
+      sinon.assert.calledOnce(this.hideChaffSpy);
     });
     // Do not copy a block if a drag is in progress.
-    suite('Drag in progress', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          sinon.stub(this.workspace, 'isDragging').returns(true);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.copySpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Drag in progress', function () {
+      sinon.stub(this.workspace, 'isDragging').returns(true);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.copySpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     // Do not copy a block if is is not deletable.
-    suite('Block is not deletable', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          sinon
-            .stub(Blockly.common.getSelected(), 'isOwnDeletable')
-            .returns(false);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.copySpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Block is not deletable', function () {
+      sinon.stub(Blockly.common.getSelected(), 'isOwnDeletable').returns(false);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.copySpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     // Do not copy a block if it is not movable.
-    suite('Block is not movable', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          sinon
-            .stub(Blockly.common.getSelected(), 'isOwnMovable')
-            .returns(false);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.copySpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Block is not movable', function () {
+      sinon.stub(Blockly.common.getSelected(), 'isOwnMovable').returns(false);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.copySpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     test('Not called when connection is focused', function () {
       // Restore the stub behavior called during setup
@@ -254,19 +209,13 @@ suite('Keyboard Shortcut Items', function () {
     });
     // Copy a comment.
     test('Workspace comment', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          Blockly.getFocusManager().getFocusedNode.restore();
-          this.comment = setSelectedComment(this.workspace);
-          this.copySpy = sinon.spy(this.comment, 'toCopyData');
+      Blockly.getFocusManager().getFocusedNode.restore();
+      this.comment = setSelectedComment(this.workspace);
+      this.copySpy = sinon.spy(this.comment, 'toCopyData');
 
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.calledOnce(this.copySpy);
-          sinon.assert.calledOnce(this.hideChaffSpy);
-        });
-      });
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.calledOnce(this.copySpy);
+      sinon.assert.calledOnce(this.hideChaffSpy);
     });
   });
 
@@ -280,92 +229,47 @@ suite('Keyboard Shortcut Items', function () {
         'hideChaff',
       );
     });
-    const testCases = [
-      [
-        'Control X',
-        createKeyDownEvent(Blockly.utils.KeyCodes.X, [
-          Blockly.utils.KeyCodes.CTRL,
-        ]),
-      ],
-      [
-        'Meta X',
-        createKeyDownEvent(Blockly.utils.KeyCodes.X, [
-          Blockly.utils.KeyCodes.META,
-        ]),
-      ],
-    ];
+    const keyEvent = createKeyDownEvent(Blockly.utils.KeyCodes.X, [
+      Blockly.utils.KeyCodes.CTRL_CMD,
+    ]);
     // Cut a block.
-    suite('Simple', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.calledOnce(this.copySpy);
-          sinon.assert.calledOnce(this.disposeSpy);
-          sinon.assert.calledOnce(this.hideChaffSpy);
-        });
-      });
+    test('Simple', function () {
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.calledOnce(this.copySpy);
+      sinon.assert.calledOnce(this.disposeSpy);
+      sinon.assert.calledOnce(this.hideChaffSpy);
     });
     // Do not cut a block if a workspace is in readonly mode.
-    suite('Not called when readOnly is true', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          this.workspace.setIsReadOnly(true);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.copySpy);
-          sinon.assert.notCalled(this.disposeSpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Not called when readOnly is true', function () {
+      this.workspace.setIsReadOnly(true);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.copySpy);
+      sinon.assert.notCalled(this.disposeSpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     // Do not cut a block if a drag is in progress.
-    suite('Drag in progress', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          sinon.stub(this.workspace, 'isDragging').returns(true);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.copySpy);
-          sinon.assert.notCalled(this.disposeSpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Drag in progress', function () {
+      sinon.stub(this.workspace, 'isDragging').returns(true);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.copySpy);
+      sinon.assert.notCalled(this.disposeSpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     // Do not cut a block if is is not deletable.
-    suite('Block is not deletable', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          sinon
-            .stub(Blockly.common.getSelected(), 'isOwnDeletable')
-            .returns(false);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.copySpy);
-          sinon.assert.notCalled(this.disposeSpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Block is not deletable', function () {
+      sinon.stub(Blockly.common.getSelected(), 'isOwnDeletable').returns(false);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.copySpy);
+      sinon.assert.notCalled(this.disposeSpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     // Do not cut a block if it is not movable.
-    suite('Block is not movable', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          sinon
-            .stub(Blockly.common.getSelected(), 'isOwnMovable')
-            .returns(false);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.copySpy);
-          sinon.assert.notCalled(this.disposeSpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Block is not movable', function () {
+      sinon.stub(Blockly.common.getSelected(), 'isOwnMovable').returns(false);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.copySpy);
+      sinon.assert.notCalled(this.disposeSpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     test('Not called when connection is focused', function () {
       // Restore the stub behavior called during setup
@@ -382,21 +286,15 @@ suite('Keyboard Shortcut Items', function () {
     });
 
     // Cut a comment.
-    suite('Workspace comment', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          Blockly.getFocusManager().getFocusedNode.restore();
-          this.comment = setSelectedComment(this.workspace);
-          this.copySpy = sinon.spy(this.comment, 'toCopyData');
-          this.disposeSpy = sinon.spy(this.comment, 'dispose');
+    test('Workspace comment', function () {
+      Blockly.getFocusManager().getFocusedNode.restore();
+      this.comment = setSelectedComment(this.workspace);
+      this.copySpy = sinon.spy(this.comment, 'toCopyData');
+      this.disposeSpy = sinon.spy(this.comment, 'dispose');
 
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.calledOnce(this.copySpy);
-          sinon.assert.calledOnce(this.disposeSpy);
-        });
-      });
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.calledOnce(this.copySpy);
+      sinon.assert.calledOnce(this.disposeSpy);
     });
   });
 
@@ -421,53 +319,26 @@ suite('Keyboard Shortcut Items', function () {
         'hideChaff',
       );
     });
-    const testCases = [
-      [
-        'Control Z',
-        createKeyDownEvent(Blockly.utils.KeyCodes.Z, [
-          Blockly.utils.KeyCodes.CTRL,
-        ]),
-      ],
-      [
-        'Meta Z',
-        createKeyDownEvent(Blockly.utils.KeyCodes.Z, [
-          Blockly.utils.KeyCodes.META,
-        ]),
-      ],
-    ];
+    const keyEvent = createKeyDownEvent(Blockly.utils.KeyCodes.Z, [
+      Blockly.utils.KeyCodes.CTRL_CMD,
+    ]);
     // Undo.
-    suite('Simple', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.calledOnce(this.undoSpy);
-          sinon.assert.calledWith(this.undoSpy, false);
-          sinon.assert.calledOnce(this.hideChaffSpy);
-        });
-      });
+    test('Simple', function () {
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.calledOnce(this.undoSpy);
+      sinon.assert.calledWith(this.undoSpy, false);
+      sinon.assert.calledOnce(this.hideChaffSpy);
     });
     // Do not undo if a drag is in progress.
-    suite('Drag in progress', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          sinon.stub(this.workspace, 'isDragging').returns(true);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.undoSpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Drag in progress', function () {
+      sinon.stub(this.workspace, 'isDragging').returns(true);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.undoSpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     // Do not undo if the workspace is in readOnly mode.
-    suite('Not called when readOnly is true', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        runReadOnlyTest(keyEvent, testCaseName);
-      });
+    test('Not called when readOnly is true', function () {
+      runReadOnlyTest(keyEvent);
     });
   });
 
@@ -479,55 +350,27 @@ suite('Keyboard Shortcut Items', function () {
         'hideChaff',
       );
     });
-    const testCases = [
-      [
-        'Control Shift Z',
-        createKeyDownEvent(Blockly.utils.KeyCodes.Z, [
-          Blockly.utils.KeyCodes.CTRL,
-          Blockly.utils.KeyCodes.SHIFT,
-        ]),
-      ],
-      [
-        'Meta Shift Z',
-        createKeyDownEvent(Blockly.utils.KeyCodes.Z, [
-          Blockly.utils.KeyCodes.META,
-          Blockly.utils.KeyCodes.SHIFT,
-        ]),
-      ],
-    ];
+    const keyEvent = createKeyDownEvent(Blockly.utils.KeyCodes.Z, [
+      Blockly.utils.KeyCodes.CTRL_CMD,
+      Blockly.utils.KeyCodes.SHIFT,
+    ]);
     // Undo.
-    suite('Simple', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.calledOnce(this.redoSpy);
-          sinon.assert.calledWith(this.redoSpy, true);
-          sinon.assert.calledOnce(this.hideChaffSpy);
-        });
-      });
+    test('Simple', function () {
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.calledOnce(this.redoSpy);
+      sinon.assert.calledWith(this.redoSpy, true);
+      sinon.assert.calledOnce(this.hideChaffSpy);
     });
     // Do not redo if a drag is in progress.
-    suite('Drag in progress', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        test(testCaseName, function () {
-          sinon.stub(this.workspace, 'isDragging').returns(true);
-          this.injectionDiv.dispatchEvent(keyEvent);
-          sinon.assert.notCalled(this.redoSpy);
-          sinon.assert.notCalled(this.hideChaffSpy);
-        });
-      });
+    test('Drag in progress', function () {
+      sinon.stub(this.workspace, 'isDragging').returns(true);
+      this.injectionDiv.dispatchEvent(keyEvent);
+      sinon.assert.notCalled(this.redoSpy);
+      sinon.assert.notCalled(this.hideChaffSpy);
     });
     // Do not undo if the workspace is in readOnly mode.
-    suite('Not called when readOnly is true', function () {
-      testCases.forEach(function (testCase) {
-        const testCaseName = testCase[0];
-        const keyEvent = testCase[1];
-        runReadOnlyTest(keyEvent, testCaseName);
-      });
+    test('Not called when readOnly is true', function () {
+      runReadOnlyTest(keyEvent);
     });
   });