fix: clean up shortcut help dialog (#688)

* chore: rearrange html to be a little nicer

* fix: dont show shortcuts with no keys

* fix: update shortcuts shown in dialog

* fix: i18n for keyboard help header

* fix: fix return type

Author: maribethb

src/constants.ts

@@ -35,6 +35,7 @@ export enum SHORTCUT_NAMES {
   LEFT = 'left',
   NEXT_STACK = 'next_stack',
   PREVIOUS_STACK = 'previous_stack',
+  // Unused.
   INSERT = 'insert',
   EDIT_OR_CONFIRM = 'edit_or_confirm',
   DISCONNECT = 'disconnect',
@@ -52,8 +53,16 @@ export enum SHORTCUT_NAMES {
   CREATE_WS_CURSOR = 'to_workspace',
   LIST_SHORTCUTS = 'list_shortcuts',
   CLEAN_UP = 'clean_up_workspace',
+  START_MOVE = 'start_move',
 }
 
+export const SHORTCUT_NAMES_TO_DISPLAY_TEXT: Record<string, string> = {
+  'keyboard_nav_copy': Msg['Copy'] || 'Copy',
+  'keyboard_nav_cut': Msg['Cut'] || 'Cut',
+  'keyboard_nav_paste': Msg['Paste'] || 'Paste',
+  'start_move': Msg['MOVE_BLOCK'] || 'Move',
+};
+
 /**
  * Types of possible messages passed into the loggingCallback in the Navigation
  * class.
@@ -73,7 +82,7 @@ export const SHORTCUT_CATEGORIES: Record<
   // Also allow undo/redo. Document the non-keyboard-nav versions of others for
   // better text because temporarily the name in the table is derived from
   // these id-like names.
-  Array<SHORTCUT_NAMES | 'undo' | 'redo' | 'cut' | 'copy' | 'paste' | 'delete'>
+  Array<SHORTCUT_NAMES | 'undo' | 'redo' | 'delete'>
 > = {};
 
 SHORTCUT_CATEGORIES[Msg['SHORTCUTS_GENERAL']] = [
@@ -86,12 +95,12 @@ SHORTCUT_CATEGORIES[Msg['SHORTCUTS_GENERAL']] = [
 ];
 
 SHORTCUT_CATEGORIES[Msg['SHORTCUTS_EDITING']] = [
-  SHORTCUT_NAMES.INSERT,
   'delete',
   SHORTCUT_NAMES.DISCONNECT,
-  'cut',
-  'copy',
-  'paste',
+  SHORTCUT_NAMES.START_MOVE,
+  SHORTCUT_NAMES.CUT,
+  SHORTCUT_NAMES.COPY,
+  SHORTCUT_NAMES.PASTE,
   SHORTCUT_NAMES.DUPLICATE,
   'undo',
   'redo',
@@ -104,4 +113,5 @@ SHORTCUT_CATEGORIES[Msg['SHORTCUTS_CODE_NAVIGATION']] = [
   SHORTCUT_NAMES.LEFT,
   SHORTCUT_NAMES.NEXT_STACK,
   SHORTCUT_NAMES.PREVIOUS_STACK,
+  SHORTCUT_NAMES.CREATE_WS_CURSOR,
 ];

src/shortcut_dialog.ts

@@ -52,19 +52,6 @@ export class ShortcutDialog {
     }
   }
 
-  /**
-   * Update the modifier key to the user's specific platform.
-   */
-  updatePlatformName() {
-    const platform = this.getPlatform();
-    const platformEl = this.outputDiv
-      ? this.outputDiv.querySelector('.platform')
-      : null;
-    if (platformEl) {
-      platformEl.textContent = platform;
-    }
-  }
-
   toggle(workspace: Blockly.WorkspaceSvg) {
     clearHelpHint(workspace);
     this.toggleInternal();
@@ -88,54 +75,56 @@ export class ShortcutDialog {
    * @returns A title case version of the name.
    */
   getReadableShortcutName(shortcutName: string) {
+    if (Constants.SHORTCUT_NAMES_TO_DISPLAY_TEXT[shortcutName]) {
+      return Constants.SHORTCUT_NAMES_TO_DISPLAY_TEXT[shortcutName];
+    }
     return upperCaseFirst(shortcutName.replace(/_/gi, ' '));
   }
 
   /**
    * List all currently registered shortcuts as a table.
    */
   createModalContent() {
-    let modalContents = `<div class="modal-container">
-      <dialog class="shortcut-modal">
-        <div class="shortcut-container" tabindex="0">
-          <div class="header">
-            <button class="close-modal">
-              <span class="material-symbols-outlined">close</span>
-            </button>
-            <h1>Keyboard shortcuts – <span class="platform">Windows</span></h1>
-          </div>
-          <div class="shortcut-tables">`;
+    let shortcutTables = ``;
 
     // Display shortcuts by their categories.
     for (const [key, categoryShortcuts] of Object.entries(
       Constants.SHORTCUT_CATEGORIES,
     )) {
-      modalContents += `
-        <table class="shortcut-table">
-          <tbody>
-          <tr class="category"><th colspan="3"><h2>${key}</h2></th></tr>
-          <tr>
-          `;
-
+      let shortcutTableRows = ``;
       for (const keyboardShortcut of categoryShortcuts) {
-        modalContents += `
-              <td>${this.getReadableShortcutName(keyboardShortcut)}</td>
-              <td>${this.actionShortcutsToHTML(keyboardShortcut)}</td>
-              </tr>`;
+        shortcutTableRows += this.getTableRowForShortcut(
+          keyboardShortcut as string,
+        );
       }
-      modalContents += '</tr></tbody></table>';
+      shortcutTables += `
+        <table class="shortcut-table">
+          <tbody>
+            <tr class="category"><th colspan="3"><h2>${key}</h2></th></tr>
+            ${shortcutTableRows}
+          </tbody>
+        </table>`;
     }
+
+    const modalContents = `<div class="modal-container">
+      <dialog class="shortcut-modal">
+        <div class="shortcut-container" tabindex="0">
+          <div class="header">
+            <button class="close-modal">
+              <span class="material-symbols-outlined">close</span>
+            </button>
+            <h1>${Msg['SHORTCUTS_HELP_HEADER'] || 'Keyboard shortcuts'} – <span class="platform">${this.getPlatform()}</span></h1>
+          </div>
+          <div class="shortcut-tables">
+          ${shortcutTables}
+          </div>
+        </dialog>
+      </div>`;
     if (this.outputDiv) {
-      this.outputDiv.innerHTML =
-        modalContents +
-        `</div>
-      </dialog>
-    </div>`;
+      this.outputDiv.innerHTML = modalContents;
       this.modalContainer = this.outputDiv.querySelector('.modal-container');
       this.shortcutDialog = this.outputDiv.querySelector('.shortcut-modal');
       this.closeButton = this.outputDiv.querySelector('.close-modal');
-      this.updatePlatformName();
-      // Can we also intercept the Esc key to dismiss.
       if (this.closeButton) {
         this.closeButton.addEventListener('click', (e) => {
           this.toggleInternal();
@@ -144,13 +133,25 @@ export class ShortcutDialog {
     }
   }
 
+  private getTableRowForShortcut(keyboardShortcut: string) {
+    const name = this.getReadableShortcutName(keyboardShortcut);
+    const keys = this.actionShortcutsToHTML(keyboardShortcut);
+    if (!name || !keys) return '';
+    return `
+              <tr>
+                <td>${name}</td>
+                <td>${keys}</td>
+              </tr>`;
+  }
+
   private actionShortcutsToHTML(action: string) {
     const shortcuts = getLongActionShortcutsAsKeys(action);
-    return shortcuts.map((keys) => this.actionShortcutToHTML(keys)).join(' / ');
+    return shortcuts.map((keys) => this.keysToHTML(keys)).join(' / ');
   }
 
-  private actionShortcutToHTML(keys: string[]) {
+  private keysToHTML(keys: string[]) {
     const separator = navigator.platform.startsWith('Mac') ? '' : ' + ';
+    if (!keys || !keys.length) return '';
     return [
       `<span class="shortcut-combo">`,
       ...keys.map((key, index) => {