Merge branch 'leipert-dynamic-kbd-help' into 'master'

Make keyboard shortcut help dynamic

See merge request gitlab-org/gitlab!56400

app/assets/javascripts/behaviors/shortcuts/keybindings.js

@@ -375,7 +375,7 @@ export const MR_PREVIOUS_FILE_IN_DIFF = {
 export const MR_GO_TO_FILE = {
   id: 'mergeRequests.goToFile',
   description: __('Go to file'),
-  defaultKeys: ['t', 'mod+p'],
+  defaultKeys: ['mod+p', 't'],
   customizable: false,
 };
 

app/assets/javascripts/behaviors/shortcuts/shortcut.vue

+<script>
+import { __, s__ } from '~/locale';
+
+// Map some keys to their proper representation depending on the system
+// See also: https://craig.is/killing/mice#keys
+const getKeyMap = () => {
+  const keyMap = {
+    up: '↑',
+    down: '↓',
+    left: '←',
+    right: '→',
+    ctrl: s__('KeyboardKey|Ctrl'),
+    shift: s__('KeyboardKey|Shift'),
+    enter: s__('KeyboardKey|Enter'),
+    esc: s__('KeyboardKey|Esc'),
+    command: '⌘',
+    option: window.gl?.client?.isMac ? '⌥' : s__('KeyboardKey|Alt'),
+  };
+
+  // Meta and alt are aliases
+  keyMap.meta = keyMap.command;
+  keyMap.alt = keyMap.option;
+
+  // Mod is Command on Mac, and Ctrl on Windows/Linux
+  keyMap.mod = window.gl?.client?.isMac ? keyMap.command : keyMap.ctrl;
+
+  return keyMap;
+};
+
+export default {
+  functional: true,
+  props: {
+    shortcuts: {
+      type: Array,
+      required: true,
+    },
+  },
+
+  render(createElement, context) {
+    const keyMap = getKeyMap();
+
+    const { staticClass } = context.data;
+
+    const shortcuts = context.props.shortcuts.reduce((acc, shortcut, i) => {
+      if (
+        !window.gl?.client?.isMac &&
+        (shortcut.includes('command') || shortcut.includes('meta'))
+      ) {
+        return acc;
+      }
+      const keys = shortcut.split(/([ +])/);
+
+      if (i !== 0 && acc.length) {
+        acc.push(` ${__('or')} `);
+        // If there are multiple alternative shortcuts,
+        // we keep them on the same line if they are single-key, e.g. `]` or `j`
+        // but if they consist of multiple keys, we insert a line break, e.g.:
+        // `shift` + `]` <br> or `shift` + `j`
+        if (keys.length > 1) {
+          acc.push(createElement('br'));
+        }
+      }
+
+      keys.forEach((key) => {
+        if (key === '+') {
+          acc.push(' + ');
+        } else if (key === ' ') {
+          acc.push(` ${__('then')} `);
+        } else {
+          acc.push(createElement('kbd', {}, [keyMap[key] ?? key]));
+        }
+      });
+
+      return acc;
+    }, []);
+
+    return createElement('div', { staticClass }, shortcuts);
+  },
+};
+</script>

app/assets/javascripts/behaviors/shortcuts/shortcuts_toggle.vue

@@ -6,7 +6,7 @@ import { disableShortcuts, enableShortcuts, shouldDisableShortcuts } from './sho
 
 export default {
   i18n: {
-    toggleLabel: __('Keyboard shortcuts'),
+    toggleLabel: __('Toggle shortcuts'),
   },
   components: {
     GlToggle,
@@ -31,14 +31,12 @@ export default {
 </script>
 
 <template>
-  <div v-if="localStorageUsable" class="d-inline-flex align-items-center js-toggle-shortcuts">
+  <div v-if="localStorageUsable" class="js-toggle-shortcuts">
     <gl-toggle
       v-model="shortcutsEnabled"
-      aria-describedby="shortcutsToggle"
       :label="$options.i18n.toggleLabel"
       label-position="left"
       @change="onChange"
     />
-    <div id="shortcutsToggle" class="sr-only">{{ __('Enable or disable keyboard shortcuts') }}</div>
   </div>
 </template>

app/assets/stylesheets/bootstrap_migration.scss

@@ -105,10 +105,6 @@ hr {
   }
 }
 
-kbd {
-  display: inline-block;
-}
-
 code {
   padding: 2px 4px;
   color: $code-color;

app/assets/stylesheets/framework.scss

@@ -22,6 +22,7 @@
 @import 'framework/flash';
 @import 'framework/forms';
 @import 'framework/gfm';
+@import 'framework/kbd';
 @import 'framework/header';
 @import 'framework/highlight';
 @import 'framework/issue_box';

app/assets/stylesheets/framework/dropdowns.scss

@@ -266,15 +266,6 @@
     }
   }
 
-  .shortcut-mappings {
-    display: none;
-  }
-
-  &.shortcuts .shortcut-mappings {
-    display: inline-block;
-    margin-right: 5px;
-  }
-
   ul {
     margin: 0;
     padding: 0;

app/assets/stylesheets/framework/kbd.scss

+kbd {
+  display: inline-block;
+  padding: 3px 5px;
+  font-size: $gl-font-size-monospace-sm;
+  line-height: 10px;
+  color: var(--gray-700, $gray-700);
+  vertical-align: middle;
+  background-color: var(--gray-10, $gray-10);
+  border-width: 1px;
+  border-style: solid;
+  border-color: var(--gray-100, $gray-100) var(--gray-100, $gray-100) var(--gray-200, $gray-200);
+  border-image: none;
+  border-radius: 3px;
+  box-shadow: 0 -1px 0 var(--gray-200, $gray-200) inset;
+}

app/assets/stylesheets/framework/typography.scss

@@ -81,22 +81,6 @@
     word-break: keep-all;
   }
 
-  kbd {
-    display: inline-block;
-    padding: 3px 5px;
-    font-size: 11px;
-    line-height: 10px;
-    color: $gray-700;
-    vertical-align: middle;
-    background-color: $gray-10;
-    border-width: 1px;
-    border-style: solid;
-    border-color: $gray-100 $gray-100 $gray-200;
-    border-image: none;
-    border-radius: 3px;
-    box-shadow: 0 -1px 0 $gray-200 inset;
-  }
-
   h1 {
     font-size: 1.75em;
     font-weight: $gl-font-weight-bold;

app/assets/stylesheets/pages/help.scss

-.shortcut-mappings {
-  font-size: 12px;
-  color: $gray-700;
-
-  tbody:first-child tr:first-child {
-    padding-top: 0;
+.shortcut-help {
+  &-body {
+    height: 80vh;
+    overflow-y: scroll;
   }
 
-  th {
-    padding-top: 15px;
-    line-height: 1.5;
-    color: $help-shortcut-header-color;
-    text-align: left;
+  &-container {
+    column-count: 1;
+    @include media-breakpoint-up(md) {
+      column-count: 2;
+    }
+    column-gap: 1rem;
   }
 
-  td {
-    padding-top: 3px;
-    padding-bottom: 3px;
-    vertical-align: top;
-    line-height: 20px;
-  }
+  &-mapping {
+    overflow: hidden;
+    break-inside: avoid;
+
+    &-title {
+      margin-left: 40%;
+    }
 
-  .shortcut {
-    padding-right: 10px;
-    color: $gray-300;
-    text-align: right;
-    white-space: nowrap;
+    kbd {
+      margin: 0.1rem 0;
+      line-height: unset;
+      font-size: unset;
+    }
   }
 }
 

changelogs/unreleased/leipert-dynamic-kbd-help.yml

+---
+title: "Update Keyboard shortcut help: adding search, update styling"
+merge_request: 56400
+author:
+type: changed

locale/gitlab.pot

@@ -12185,9 +12185,6 @@ msgstr ""
 msgid "Enable or disable Seat Link."
 msgstr ""
 
-msgid "Enable or disable keyboard shortcuts"
-msgstr ""
-
 msgid "Enable or disable the Pseudonymizer data collection."
 msgstr ""
 
@@ -18816,9 +18813,30 @@ msgstr ""
 msgid "Keyboard shortcuts"
 msgstr ""
 
+msgid "KeyboardKey|Alt"
+msgstr ""
+
+msgid "KeyboardKey|Ctrl"
+msgstr ""
+
 msgid "KeyboardKey|Ctrl+"
 msgstr ""
 
+msgid "KeyboardKey|Enter"
+msgstr ""
+
+msgid "KeyboardKey|Esc"
+msgstr ""
+
+msgid "KeyboardKey|Shift"
+msgstr ""
+
+msgid "KeyboardShortcuts|No shortcuts matched your search"
+msgstr ""
+
+msgid "KeyboardShortcuts|Search keyboard shortcuts"
+msgstr ""
+
 msgid "Keys"
 msgstr ""
 
@@ -33783,13 +33801,13 @@ msgstr ""
 msgid "Toggle project select"
 msgstr ""
 
-msgid "Toggle sidebar"
+msgid "Toggle shortcuts"
 msgstr ""
 
-msgid "Toggle the Performance Bar"
+msgid "Toggle sidebar"
 msgstr ""
 
-msgid "Toggle this dialog"
+msgid "Toggle the Performance Bar"
 msgstr ""
 
 msgid "Toggle thread"
@@ -38955,6 +38973,9 @@ msgstr ""
 msgid "the wiki"
 msgstr ""
 
+msgid "then"
+msgstr ""
+
 msgid "this document"
 msgstr ""
 

spec/frontend/behaviors/shortcuts/shortcut_spec.js

+import { shallowMount } from '@vue/test-utils';
+import Shortcut from '~/behaviors/shortcuts/shortcut.vue';
+
+describe('Shortcut Vue Component', () => {
+  const render = (shortcuts) => shallowMount(Shortcut, { propsData: { shortcuts } }).html();
+
+  afterEach(() => {
+    delete window.gl.client;
+  });
+
+  describe.each([true, false])('With browser env isMac: %p', (isMac) => {
+    beforeEach(() => {
+      window.gl = { client: { isMac } };
+    });
+
+    it.each([
+      ['up', '<kbd>↑</kbd>'],
+      ['down', '<kbd>↓</kbd>'],
+      ['left', '<kbd>←</kbd>'],
+      ['right', '<kbd>→</kbd>'],
+      ['ctrl', '<kbd>Ctrl</kbd>'],
+      ['shift', '<kbd>Shift</kbd>'],
+      ['enter', '<kbd>Enter</kbd>'],
+      ['esc', '<kbd>Esc</kbd>'],
+      // Some normal ascii letter
+      ['a', '<kbd>a</kbd>'],
+      // An umlaut letter
+      ['ø', '<kbd>ø</kbd>'],
+      // A number
+      ['5', '<kbd>5</kbd>'],
+    ])('renders platform agnostic key %p as: %p', (key, rendered) => {
+      expect(render([key])).toEqual(`<div>${rendered}</div>`);
+    });
+
+    it('renders keys combined with plus ("+") correctly', () => {
+      expect(render(['shift+a+b+c'])).toEqual(
+        `<div><kbd>Shift</kbd> + <kbd>a</kbd> + <kbd>b</kbd> + <kbd>c</kbd></div>`,
+      );
+    });
+
+    it('renders keys combined with space (" ") correctly', () => {
+      expect(render(['shift a b c'])).toEqual(
+        `<div><kbd>Shift</kbd> then <kbd>a</kbd> then <kbd>b</kbd> then <kbd>c</kbd></div>`,
+      );
+    });
+
+    it('renders multiple shortcuts correctly', () => {
+      expect(render(['shift+[', 'shift+k'])).toEqual(
+        `<div><kbd>Shift</kbd> + <kbd>[</kbd> or <br><kbd>Shift</kbd> + <kbd>k</kbd></div>`,
+      );
+      expect(render(['[', 'k'])).toEqual(`<div><kbd>[</kbd> or <kbd>k</kbd></div>`);
+    });
+  });
+
+  describe('With browser env isMac: true', () => {
+    beforeEach(() => {
+      window.gl = { client: { isMac: true } };
+    });
+
+    it.each([
+      ['mod', '<kbd>⌘</kbd>'],
+      ['command', '<kbd>⌘</kbd>'],
+      ['meta', '<kbd>⌘</kbd>'],
+      ['option', '<kbd>⌥</kbd>'],
+      ['alt', '<kbd>⌥</kbd>'],
+    ])('renders platform specific key %p as: %p', (key, rendered) => {
+      expect(render([key])).toEqual(`<div>${rendered}</div>`);
+    });
+
+    it('does render Mac specific shortcuts', () => {
+      expect(render(['command+[', 'ctrl+k'])).toEqual(
+        `<div><kbd>⌘</kbd> + <kbd>[</kbd> or <br><kbd>Ctrl</kbd> + <kbd>k</kbd></div>`,
+      );
+    });
+  });
+
+  describe('With browser env isMac: false', () => {
+    beforeEach(() => {
+      window.gl = { client: { isMac: false } };
+    });
+
+    it.each([
+      ['mod', '<kbd>Ctrl</kbd>'],
+      ['command', ''],
+      ['meta', ''],
+      ['option', '<kbd>Alt</kbd>'],
+      ['alt', '<kbd>Alt</kbd>'],
+    ])('renders platform specific key %p as: %p', (key, rendered) => {
+      expect(render([key])).toEqual(`<div>${rendered}</div>`);
+    });
+
+    it('does not render Mac specific shortcuts', () => {
+      expect(render(['command+[', 'ctrl+k'])).toEqual(`<div><kbd>Ctrl</kbd> + <kbd>k</kbd></div>`);
+    });
+  });
+});