feat: Render keyboard shortcuts nicely in menus. (#594)

Author: gonfunko

src/actions/clipboard.ts

@@ -14,7 +14,7 @@ import {
 } from 'blockly';
 import * as Constants from '../constants';
 import {Navigation} from '../navigation';
-import {getShortActionShortcut} from '../shortcut_formatting';
+import {getMenuItem} from '../shortcut_formatting';
 import {clearPasteHints, showCopiedHint, showCutHint} from '../hints';
 
 /**
@@ -104,10 +104,7 @@ export class Clipboard {
   private registerCutContextMenuAction() {
     const cutAction: ContextMenuRegistry.RegistryItem = {
       displayText: (scope) =>
-        Msg['CUT_SHORTCUT'].replace(
-          '%1',
-          getShortActionShortcut(Constants.SHORTCUT_NAMES.CUT),
-        ),
+        getMenuItem(Msg['CUT_SHORTCUT'], Constants.SHORTCUT_NAMES.CUT),
       preconditionFn: (scope) => this.cutPrecondition(scope),
       callback: (scope, menuOpenEvent) => {
         if (!isCopyable(scope.focusedNode)) return false;
@@ -250,10 +247,7 @@ export class Clipboard {
   private registerCopyContextMenuAction() {
     const copyAction: ContextMenuRegistry.RegistryItem = {
       displayText: (scope) =>
-        Msg['COPY_SHORTCUT'].replace(
-          '%1',
-          getShortActionShortcut(Constants.SHORTCUT_NAMES.COPY),
-        ),
+        getMenuItem(Msg['COPY_SHORTCUT'], Constants.SHORTCUT_NAMES.COPY),
       preconditionFn: (scope) => this.copyPrecondition(scope),
       callback: (scope, menuOpenEvent) => {
         if (!isCopyable(scope.focusedNode)) return false;
@@ -331,10 +325,7 @@ export class Clipboard {
   private registerPasteContextMenuAction() {
     const pasteAction: ContextMenuRegistry.RegistryItem = {
       displayText: (scope) =>
-        Msg['PASTE_SHORTCUT'].replace(
-          '%1',
-          getShortActionShortcut(Constants.SHORTCUT_NAMES.PASTE),
-        ),
+        getMenuItem(Msg['PASTE_SHORTCUT'], Constants.SHORTCUT_NAMES.PASTE),
       preconditionFn: (scope) => this.pastePrecondition(scope),
       callback: (scope: ContextMenuRegistry.Scope, menuOpenEvent: Event) => {
         const workspace = this.copyWorkspace;

src/actions/delete.ts

@@ -5,7 +5,7 @@
  */
 
 import {ContextMenuRegistry, Msg, ShortcutItems} from 'blockly';
-import {getShortActionShortcut} from '../shortcut_formatting';
+import {getMenuItem} from '../shortcut_formatting';
 
 /**
  * Action to delete the block the cursor is currently on.
@@ -58,17 +58,23 @@ export class DeleteAction {
     this.oldDisplayText = this.oldContextMenuItem.displayText;
 
     const displayText = (scope: ContextMenuRegistry.Scope) => {
-      const shortcut = getShortActionShortcut(ShortcutItems.names.DELETE);
-
+      let label: string;
       // Use the original item's text, which is dynamic based on the number
       // of blocks that will be deleted.
       if (typeof this.oldDisplayText === 'function') {
-        return this.oldDisplayText(scope) + ` (${shortcut})`;
+        const result = this.oldDisplayText(scope);
+        if (result instanceof HTMLElement) {
+          label = result.innerText;
+        } else {
+          label = result;
+        }
       } else if (typeof this.oldDisplayText === 'string') {
-        return this.oldDisplayText + ` (${shortcut})`;
+        label = this.oldDisplayText;
+      } else {
+        label = Msg['DELETE_BLOCK'];
       }
 
-      return Msg['DELETE_BLOCK'].replace('%1', shortcut);
+      return getMenuItem(label, ShortcutItems.names.DELETE);
     };
 
     this.oldContextMenuItem.displayText = displayText;

src/actions/edit.ts

@@ -11,7 +11,7 @@ import {
   keyboardNavigationController,
 } from 'blockly';
 import {Navigation} from 'src/navigation';
-import {getShortActionShortcut} from '../shortcut_formatting';
+import {getMenuItem} from '../shortcut_formatting';
 import * as Constants from '../constants';
 
 /**
@@ -57,9 +57,9 @@ export class EditAction {
    */
   private registerContextMenuAction() {
     const editAboveItem: ContextMenuRegistry.RegistryItem = {
-      displayText: Msg['EDIT_BLOCK_CONTENTS'].replace(
-        '%1',
-        getShortActionShortcut(Constants.SHORTCUT_NAMES.RIGHT),
+      displayText: getMenuItem(
+        Msg['EDIT_BLOCK_CONTENTS'],
+        Constants.SHORTCUT_NAMES.RIGHT,
       ),
       preconditionFn: (scope: ContextMenuRegistry.Scope, menuOpenEvent) => {
         if (menuOpenEvent instanceof PointerEvent) return 'hidden';

src/actions/move.ts

@@ -16,7 +16,7 @@ import {
 } from 'blockly';
 import {Direction} from '../drag_direction';
 import {Mover} from './mover';
-import {getShortActionShortcut} from '../shortcut_formatting';
+import {getMenuItem} from '../shortcut_formatting';
 
 const KeyCodes = utils.KeyCodes;
 const createSerializedKey = ShortcutRegistry.registry.createSerializedKey.bind(
@@ -156,10 +156,7 @@ export class MoveActions {
   private registerMenuItems() {
     const menuItems: ContextMenuRegistry.RegistryItem[] = [
       {
-        displayText: Msg['MOVE_BLOCK'].replace(
-          '%1',
-          getShortActionShortcut('start_move'),
-        ),
+        displayText: getMenuItem(Msg['MOVE_BLOCK'], 'start_move'),
         preconditionFn: (scope, menuOpenEvent) => {
           const workspace = scope.block?.workspace as WorkspaceSvg | null;
           if (!workspace || menuOpenEvent instanceof PointerEvent)

src/shortcut_formatting.ts

@@ -3,6 +3,31 @@ import {keyNames} from './keynames';
 
 const isMacPlatform = navigator.platform.startsWith('Mac');
 
+/**
+ * Returns an HTML menu item with a label and grey keyboard shortcut.
+ *
+ * @param labelText The text of the mneu item.
+ * @param action The identifier of an action to use the keyboard shortcut of.
+ * @returns A nicely formatted menu item.
+ */
+export function getMenuItem(labelText: string, action: string): HTMLElement {
+  // TODO: Once core is updated to remove the shortcut placeholders from the
+  // keyboard shortcut messages, remove this.
+  if (labelText.indexOf(')') === labelText.length - 1) {
+    labelText = labelText.split(' (')[0];
+  }
+  const container = document.createElement('div');
+  container.className = 'blocklyShortcutContainer';
+  const label = document.createElement('span');
+  label.textContent = labelText;
+  const shortcut = document.createElement('span');
+  shortcut.className = 'blocklyShortcut';
+  shortcut.textContent = getShortActionShortcut(action);
+  container.appendChild(label);
+  container.appendChild(shortcut);
+  return container;
+}
+
 /**
  * Find the primary shortcut for this platform and return it as single string
  * in a short user facing format.

test/index.html

@@ -161,6 +161,35 @@
         .blocklyToolboxCategoryContainer:focus-visible {
         outline: none;
       }
+
+      .blocklyRTL
+        .blocklyKeyboardNavigation
+        .blocklyMenuItemContent
+        .blocklyShortcutContainer {
+        flex-direction: row-reverse;
+      }
+      .blocklyKeyboardNavigation
+        .blocklyMenuItemContent
+        .blocklyShortcutContainer {
+        width: 100%;
+        display: flex;
+        justify-content: space-between;
+      }
+      .blocklyKeyboardNavigation
+        .blocklyMenuItemContent
+        .blocklyShortcutContainer
+        .blocklyShortcut {
+        color: #ccc;
+        margin-left: 16px;
+      }
+      .blocklyRTL
+        .blocklyKeyboardNavigation
+        .blocklyMenuItemContent
+        .blocklyShortcutContainer
+        .blocklyShortcut {
+        margin-left: 0;
+        margin-right: 16px;
+      }
     </style>
   </head>