Merge branch 'nfriend-make-keybindings-tree-shakeable' into 'master'

Make keybindings.js tree-shakeable

See merge request gitlab-org/gitlab!56173

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

-import { flatten } from 'lodash';
+import { memoize } from 'lodash';
 import AccessorUtilities from '~/lib/utils/accessor';
 import { s__ } from '~/locale';
-import { shouldDisableShortcuts } from './shortcuts_toggle';
 
 export const LOCAL_STORAGE_KEY = 'gl-keyboard-shortcuts-customizations';
 
-let parsedCustomizations = {};
-const localStorageIsSafe = AccessorUtilities.isLocalStorageAccessSafe();
+/**
+ * @returns { Object.<string, string[]> } A map of command ID => keys of all
+ * keyboard shortcuts that have been customized by the user. These
+ * customizations are fetched from `localStorage`. This function is memoized,
+ * so its return value will not reflect changes made to the `localStorage` data
+ * after it has been called once.
+ *
+ * @example
+ * { "globalShortcuts.togglePerformanceBar": ["p e r f"] }
+ */
+export const getCustomizations = memoize(() => {
+  let parsedCustomizations = {};
+  const localStorageIsSafe = AccessorUtilities.isLocalStorageAccessSafe();
 
-if (localStorageIsSafe) {
+  if (localStorageIsSafe) {
     try {
       parsedCustomizations = JSON.parse(localStorage.getItem(LOCAL_STORAGE_KEY) || '{}');
     } catch (e) {
       /* do nothing */
     }
-}
+  }
 
-/**
- * A map of command => keys of all keyboard shortcuts
- * that have been customized by the user.
- *
- * @example
- * { "globalShortcuts.togglePerformanceBar": ["p e r f"] }
- *
- * @type { Object.<string, string[]> }
- */
-export const customizations = parsedCustomizations;
+  return parsedCustomizations;
+});
 
 // All available commands
-export const TOGGLE_PERFORMANCE_BAR = 'globalShortcuts.togglePerformanceBar';
-export const TOGGLE_CANARY = 'globalShortcuts.toggleCanary';
-
-/** All keybindings, grouped and ordered with descriptions */
-export const keybindingGroups = [
-  {
-    groupId: 'globalShortcuts',
-    name: s__('KeyboardShortcuts|Global Shortcuts'),
-    keybindings: [
-      {
+export const TOGGLE_PERFORMANCE_BAR = {
+  id: 'globalShortcuts.togglePerformanceBar',
   description: s__('KeyboardShortcuts|Toggle the Performance Bar'),
-        command: TOGGLE_PERFORMANCE_BAR,
   // eslint-disable-next-line @gitlab/require-i18n-strings
   defaultKeys: ['p b'],
-      },
-      {
+};
+
+export const TOGGLE_CANARY = {
+  id: 'globalShortcuts.toggleCanary',
   description: s__('KeyboardShortcuts|Toggle GitLab Next'),
-        command: TOGGLE_CANARY,
   // eslint-disable-next-line @gitlab/require-i18n-strings
   defaultKeys: ['g x'],
-      },
-    ],
-  },
-]
+};
 
-  // For each keybinding object, add a `customKeys` property populated with the
-  // user's custom keybindings (if the command has been customized).
-  // `customKeys` will be `undefined` if the command hasn't been customized.
-  .map((group) => {
-    return {
-      ...group,
-      keybindings: group.keybindings.map((binding) => ({
-        ...binding,
-        customKeys: customizations[binding.command],
-      })),
-    };
-  });
+export const WEB_IDE_COMMIT = {
+  id: 'webIDE.commit',
+  description: s__('KeyboardShortcuts|Commit (when editing commit message)'),
+  defaultKeys: ['mod+enter'],
+  customizable: false,
+};
 
-/**
- * A simple map of command => keys. All user customizations are included in this map.
- * This mapping is used to simplify `keysFor` below.
- *
- * @example
- * { "globalShortcuts.togglePerformanceBar": ["p e r f"] }
- */
-const commandToKeys = flatten(keybindingGroups.map((group) => group.keybindings)).reduce(
-  (acc, binding) => {
-    acc[binding.command] = binding.customKeys || binding.defaultKeys;
-    return acc;
-  },
-  {},
-);
+// All keybinding groups
+export const GLOBAL_SHORTCUTS_GROUP = {
+  id: 'globalShortcuts',
+  name: s__('KeyboardShortcuts|Global Shortcuts'),
+  keybindings: [TOGGLE_PERFORMANCE_BAR, TOGGLE_CANARY],
+};
+
+export const WEB_IDE_GROUP = {
+  id: 'webIDE',
+  name: s__('KeyboardShortcuts|Web IDE'),
+  keybindings: [WEB_IDE_COMMIT],
+};
+
+/** All keybindings, grouped and ordered with descriptions */
+export const keybindingGroups = [GLOBAL_SHORTCUTS_GROUP, WEB_IDE_GROUP];
 
 /**
  * Gets keyboard shortcuts associated with a command
  *
- * @param {string} command The command string. All command
- * strings are available as imports from this file.
+ * @param {string} command The command object. All command
+ * objects are available as imports from this file.
  *
  * @returns {string[]} An array of keyboard shortcut strings bound to the command
  *
@@ -95,9 +81,11 @@ const commandToKeys = flatten(keybindingGroups.map((group) => group.keybindings)
  * Mousetrap.bind(keysFor(TOGGLE_PERFORMANCE_BAR), handler);
  */
 export const keysFor = (command) => {
-  if (shouldDisableShortcuts()) {
-    return [];
+  if (command.customizable === false) {
+    // if the command is defined with `customizable: false`,
+    // don't allow this command to be customized.
+    return command.defaultKeys;
   }
 
-  return commandToKeys[command];
+  return getCustomizations()[command.id] || command.defaultKeys;
 };

doc/development/fe_guide/keyboard_shortcuts.md

@@ -33,9 +33,10 @@ Mousetrap.bind(keysFor(TOGGLE_PERFORMANCE_BAR), togglePerformanceBar);
 ## Shortcut customization
 
 `keybindings.js` stores keyboard shortcut customizations as a JSON string in
-`localStorage`. When `keybindings.js` is first imported, it fetches any
-customizations from `localStorage` and merges these customizations into the
-default set of keybindings. There is no UI to edit these customizations.
+`localStorage`. When `keysFor` is called, it uses the provided command object's
+`id` to lookup any customizations found in `localStorage` and returns the custom
+keybindings, or the default keybindings if the command has not been customized.
+There is no UI to edit these customizations.
 
 ## Adding new shortcuts
 
@@ -44,27 +45,33 @@ developers are encouraged to build _lots_ of keyboard shortcuts into GitLab.
 Shortcuts that are less likely to be used should be
 [disabled](#disabling-shortcuts) by default.
 
-To add a new shortcut, define and export a new command string in
+To add a new shortcut, define and export a new command object in
 `keybindings.js`:
 
 ```javascript
-export const MAKE_COFFEE = 'foodAndBeverage.makeCoffee';
+export const MAKE_COFFEE = {
+  id: 'foodAndBeverage.makeCoffee',
+  description: s__('KeyboardShortcuts|Make coffee'),
+  defaultKeys: ['mod+shift+c'],
+};
 ```
 
-Next, add a new command definition under the appropriate group in the
-`keybindingGroups` array:
+Next, add a new command to the appropriate keybinding group object:
 
 ```javascript
-{
-  description: s__('KeyboardShortcuts|Make coffee'),
-  command: MAKE_COFFEE,
-  defaultKeys: ['mod+shift+c'],
-  customKeys: customizations[MAKE_COFFEE],
+const COFFEE_GROUP = {
+  id: 'foodAndBeverage',
+  name: s__('KeyboardShortcuts|Food and Beverage'),
+  keybindings: [
+    MAKE_ESPRESSO,
+    MAKE_LATTE,
+    MAKE_COFFEE
+  ];
 }
 ```
 
 Finally, in the application code, import the `keysFor` function and the new
-command and bind the shortcut to the handler using Mousetrap:
+command object and bind the shortcut to the handler using Mousetrap:
 
 ```javascript
 import { keysFor, MAKE_COFFEE } from '~/behaviors/shortcuts/keybindings'
@@ -81,16 +88,34 @@ shortcut to an empty array `[]`. For example, to introduce a new shortcut that
 is disabled by default, a command can be defined like this:
 
 ```javascript
-export const MAKE_MOCHA = 'foodAndBeverage.makeMocha';
-
-{
+export const MAKE_MOCHA = {
+  id: 'foodAndBeverage.makeMocha',
   description: s__('KeyboardShortcuts|Make a mocha'),
-  command: MAKE_MOCHA,
   defaultKeys: [],
-  customKeys: customizations[MAKE_MOCHA],
-}
+};
 ```
 
+## Making shortcuts non-customizable
+
+Occasionally, it's important that a keyboard shortcut _not_ be customizable
+(although this should be a rare occurrence).
+
+In this case, a shortcut can be defined with `customizable: false`, which
+disables customization of the keybinding:
+
+```javascript
+export const MAKE_AMERICANO = {
+  id: 'foodAndBeverage.makeAmericano',
+  description: s__('KeyboardShortcuts|Make an Americano'),
+  defaultKeys: ['mod+shift+a'],
+
+  // this disables customization of this shortcut
+  customizable: false
+};
+```
+
+This shortcut will always be bound to its `defaultKeys`.
+
 ## Make cross-platform shortcuts
 
 It's difficult to make shortcuts that work well in all platforms and browsers.

locale/gitlab.pot

@@ -17544,6 +17544,9 @@ msgstr ""
 msgid "KeyboardKey|Ctrl+"
 msgstr ""
 
+msgid "KeyboardShortcuts|Commit (when editing commit message)"
+msgstr ""
+
 msgid "KeyboardShortcuts|Global Shortcuts"
 msgstr ""
 
@@ -17553,6 +17556,9 @@ msgstr ""
 msgid "KeyboardShortcuts|Toggle the Performance Bar"
 msgstr ""
 
+msgid "KeyboardShortcuts|Web IDE"
+msgstr ""
+
 msgid "Keys"
 msgstr ""
 

spec/frontend/behaviors/shortcuts/keybindings_spec.js

+import { flatten } from 'lodash';
 import { useLocalStorageSpy } from 'helpers/local_storage_helper';
+import {
+  keysFor,
+  getCustomizations,
+  keybindingGroups,
+  TOGGLE_PERFORMANCE_BAR,
+  LOCAL_STORAGE_KEY,
+  WEB_IDE_COMMIT,
+} from '~/behaviors/shortcuts/keybindings';
 
-describe('~/behaviors/shortcuts/keybindings.js', () => {
-  let keysFor;
-  let TOGGLE_PERFORMANCE_BAR;
-  let LOCAL_STORAGE_KEY;
-
+describe('~/behaviors/shortcuts/keybindings', () => {
   beforeAll(() => {
     useLocalStorageSpy();
   });
 
-  const setupCustomizations = async (customizationsAsString) => {
+  const setupCustomizations = (customizationsAsString) => {
     localStorage.clear();
 
     if (customizationsAsString) {
       localStorage.setItem(LOCAL_STORAGE_KEY, customizationsAsString);
     }
 
-    jest.resetModules();
-    ({ keysFor, TOGGLE_PERFORMANCE_BAR, LOCAL_STORAGE_KEY } = await import(
-      '~/behaviors/shortcuts/keybindings'
-    ));
+    getCustomizations.cache.clear();
   };
 
+  describe('keybinding definition errors', () => {
+    beforeEach(() => {
+      setupCustomizations();
+    });
+
+    it('has no duplicate group IDs', () => {
+      const allGroupIds = keybindingGroups.map((group) => group.id);
+      expect(allGroupIds).toHaveLength(new Set(allGroupIds).size);
+    });
+
+    it('has no duplicate commands IDs', () => {
+      const allCommandIds = flatten(
+        keybindingGroups.map((group) => group.keybindings.map((kb) => kb.id)),
+      );
+      expect(allCommandIds).toHaveLength(new Set(allCommandIds).size);
+    });
+  });
+
   describe('when a command has not been customized', () => {
-    beforeEach(async () => {
-      await setupCustomizations('{}');
+    beforeEach(() => {
+      setupCustomizations('{}');
     });
 
-    it('returns the default keybinding for the command', () => {
+    it('returns the default keybindings for the command', () => {
       expect(keysFor(TOGGLE_PERFORMANCE_BAR)).toEqual(['p b']);
     });
   });
@@ -35,18 +55,30 @@ describe('~/behaviors/shortcuts/keybindings.js', () => {
   describe('when a command has been customized', () => {
     const customization = ['p b a r'];
 
-    beforeEach(async () => {
-      await setupCustomizations(JSON.stringify({ [TOGGLE_PERFORMANCE_BAR]: customization }));
+    beforeEach(() => {
+      setupCustomizations(JSON.stringify({ [TOGGLE_PERFORMANCE_BAR.id]: customization }));
     });
 
-    it('returns the default keybinding for the command', () => {
+    it('returns the custom keybindings for the command', () => {
       expect(keysFor(TOGGLE_PERFORMANCE_BAR)).toEqual(customization);
     });
   });
 
+  describe('when a command is marked as non-customizable', () => {
+    const customization = ['mod+shift+c'];
+
+    beforeEach(() => {
+      setupCustomizations(JSON.stringify({ [WEB_IDE_COMMIT.id]: customization }));
+    });
+
+    it('returns the default keybinding for the command', () => {
+      expect(keysFor(WEB_IDE_COMMIT)).toEqual(['mod+enter']);
+    });
+  });
+
   describe("when the localStorage entry isn't valid JSON", () => {
-    beforeEach(async () => {
-      await setupCustomizations('{');
+    beforeEach(() => {
+      setupCustomizations('{');
     });
 
     it('returns the default keybinding for the command', () => {
@@ -55,8 +87,8 @@ describe('~/behaviors/shortcuts/keybindings.js', () => {
   });
 
   describe(`when localStorage doesn't contain the ${LOCAL_STORAGE_KEY} key`, () => {
-    beforeEach(async () => {
-      await setupCustomizations();
+    beforeEach(() => {
+      setupCustomizations();
     });
 
     it('returns the default keybinding for the command', () => {