Document keyboard shortcuts (#7908)

docs/features/keyboardShortcuts.md

@@ -0,0 +1,59 @@
+# Keyboard shortcuts
+
+## Using the `KeyBindingManger`
+
+The `KeyBindingManager` (accessible using `getKeyBindingManager()`) is a class
+with several methods that allow you to get a `KeyBindingAction` based on a
+`KeyboardEvent | React.KeyboardEvent`.
+
+The event passed to the `KeyBindingManager` gets compared to the list of
+shortcuts that are retrieved from the `IKeyBindingsProvider`s. The
+`IKeyBindingsProvider` is in `KeyBindingDefaults`.
+
+### Examples
+
+Let's say we want to close a menu when the correct keys were pressed:
+
+```ts
+const onKeyDown = (ev: KeyboardEvent): void => {
+    let handled = true;
+    const action = getKeyBindingManager().getAccessibilityAction(ev)
+    switch (action) {
+        case KeyBindingAction.Escape:
+            closeMenu();
+            break;
+        default:
+            handled = false;
+            break;
+    }
+
+    if (handled) {
+        ev.preventDefault();
+        ev.stopPropagation();
+    }
+}
+```
+
+## Managing keyboard shortcuts
+
+There are a few things at play when it comes to keyboard shortcuts. The
+`KeyBindingManager` gets `IKeyBindingsProvider`s one of which is
+`defaultBindingsProvider` defined in `KeyBindingDefaults`. In
+`KeyBindingDefaults` a `getBindingsByCategory()` method is used to create
+`KeyBinding`s based on `KeyboardShortcutSetting`s defined in
+`KeyboardShortcuts`.
+
+### Adding keyboard shortcuts
+
+To add a keyboard shortcut there are two files we have to look at:
+`KeyboardShortcuts.ts` and `KeyBindingDefaults.ts`. In most cases we only need
+to edit `KeyboardShortcuts.ts`: add a `KeyBindingAction` and add the
+`KeyBindingAction` to the `KEYBOARD_SHORTCUTS` object.
+
+Though, to make matters worse, sometimes we want to add a shortcut that has
+multiple keybindings associated with. This keyboard shortcut won't be
+customizable as it would be rather difficult to manage both from the point of
+the settings and the UI. To do this, we have to add a `KeyBindingAction` and add
+the UI representation of that keyboard shortcut to the `getUIOnlyShortcuts()`
+method. Then, we also need to add the keybinding to the correct method in
+`KeyBindingDefaults`.

src/KeyBindingsDefaults.ts

@@ -26,13 +26,13 @@ import {
 import {
     CATEGORIES,
     CategoryName,
-    getCustomizableShortcuts,
+    getKeyboardShortcuts,
     KeyBindingAction,
 } from "./accessibility/KeyboardShortcuts";
 
 export const getBindingsByCategory = (category: CategoryName): KeyBinding[] => {
     return CATEGORIES[category].settingNames.reduce((bindings, name) => {
-        const value = getCustomizableShortcuts()[name]?.default;
+        const value = getKeyboardShortcuts()[name]?.default;
         if (value) {
             bindings.push({
                 action: name as KeyBindingAction,

src/accessibility/KeyboardShortcuts.ts

@@ -700,8 +700,12 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = {
     },
 };
 
-// XXX: These have to be manually mirrored in KeyBindingDefaults
-const getNonCustomizableShortcuts = (): IKeyboardShortcuts => {
+/**
+ * This function gets the keyboard shortcuts that should be presented in the UI
+ * but they shouldn't be consumed by KeyBindingDefaults. That means that these
+ * have to be manually mirrored in KeyBindingDefaults.
+ */
+const getUIOnlyShortcuts = (): IKeyboardShortcuts => {
     const ctrlEnterToSend = SettingsStore.getValue('MessageComposerInput.ctrlEnterToSend');
 
     const keyboardShortcuts: IKeyboardShortcuts = {
@@ -741,6 +745,9 @@ const getNonCustomizableShortcuts = (): IKeyboardShortcuts => {
     };
 
     if (PlatformPeg.get().overrideBrowserShortcuts()) {
+        // XXX: This keyboard shortcut isn't manually added to
+        // KeyBindingDefaults as it can't be easily handled by the
+        // KeyBindingManager
         keyboardShortcuts[KeyBindingAction.SwitchToSpaceByNumber] = {
             default: {
                 ctrlOrCmdKey: true,
@@ -753,7 +760,10 @@ const getNonCustomizableShortcuts = (): IKeyboardShortcuts => {
     return keyboardShortcuts;
 };
 
-export const getCustomizableShortcuts = (): IKeyboardShortcuts => {
+/**
+ * This function gets keyboard shortcuts that can be consumed by the KeyBindingDefaults.
+ */
+export const getKeyboardShortcuts = (): IKeyboardShortcuts => {
     const overrideBrowserShortcuts = PlatformPeg.get().overrideBrowserShortcuts();
 
     return Object.keys(KEYBOARD_SHORTCUTS).filter((k: KeyBindingAction) => {
@@ -768,10 +778,13 @@ export const getCustomizableShortcuts = (): IKeyboardShortcuts => {
     }, {} as IKeyboardShortcuts);
 };
 
-export const getKeyboardShortcuts = (): IKeyboardShortcuts => {
+/**
+ * Gets keyboard shortcuts that should be presented to the user in the UI.
+ */
+export const getKeyboardShortcutsForUI = (): IKeyboardShortcuts => {
     const entries = [
-        ...Object.entries(getNonCustomizableShortcuts()),
-        ...Object.entries(getCustomizableShortcuts()),
+        ...Object.entries(getUIOnlyShortcuts()),
+        ...Object.entries(getKeyboardShortcuts()),
     ];
 
     return entries.reduce((acc, [key, value]) => {

src/components/views/settings/tabs/user/KeyboardUserSettingsTab.tsx

@@ -18,7 +18,7 @@ limitations under the License.
 import React from "react";
 
 import {
-    getKeyboardShortcuts,
+    getKeyboardShortcutsForUI,
     ALTERNATE_KEY_NAME,
     KEY_ICON,
     ICategory,
@@ -32,11 +32,11 @@ import { _t } from "../../../../../languageHandler";
 
 // TODO: This should return KeyCombo but it has ctrlOrCmd instead of ctrlOrCmdKey
 const getKeyboardShortcutValue = (name: string): KeyBindingConfig => {
-    return getKeyboardShortcuts()[name]?.default;
+    return getKeyboardShortcutsForUI()[name]?.default;
 };
 
 const getKeyboardShortcutDisplayName = (name: string): string | null => {
-    const keyboardShortcutDisplayName = getKeyboardShortcuts()[name]?.displayName;
+    const keyboardShortcutDisplayName = getKeyboardShortcutsForUI()[name]?.displayName;
     return keyboardShortcutDisplayName && _t(keyboardShortcutDisplayName);
 };
 

test/accessibility/KeyboardShortcuts-test.ts

@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import {
-    getCustomizableShortcuts,
+    getKeyboardShortcutsForUI,
     getKeyboardShortcuts,
     KEYBOARD_SHORTCUTS,
     mock,
@@ -35,10 +35,10 @@ describe("KeyboardShortcuts", () => {
         PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => false });
         const copyKeyboardShortcuts = Object.assign({}, KEYBOARD_SHORTCUTS);
 
-        getCustomizableShortcuts();
-        expect(KEYBOARD_SHORTCUTS).toEqual(copyKeyboardShortcuts);
         getKeyboardShortcuts();
         expect(KEYBOARD_SHORTCUTS).toEqual(copyKeyboardShortcuts);
+        getKeyboardShortcutsForUI();
+        expect(KEYBOARD_SHORTCUTS).toEqual(copyKeyboardShortcuts);
     });
 
     it("correctly filters shortcuts", async () => {
@@ -54,7 +54,7 @@ describe("KeyboardShortcuts", () => {
 
         });
         PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => false });
-        expect(getCustomizableShortcuts()).toEqual({ "Keybind4": {} });
+        expect(getKeyboardShortcuts()).toEqual({ "Keybind4": {} });
 
         mock({
             keyboardShortcuts: {
@@ -65,7 +65,7 @@ describe("KeyboardShortcuts", () => {
             desktopShortcuts: ["Keybind2"],
         });
         PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => true });
-        expect(getCustomizableShortcuts()).toEqual({ "Keybind1": {}, "Keybind2": {} });
+        expect(getKeyboardShortcuts()).toEqual({ "Keybind1": {}, "Keybind2": {} });
         jest.resetModules();
     });
 });

test/components/views/settings/tabs/user/KeyboardUserSettingsTab-test.tsx

@@ -60,7 +60,7 @@ describe("KeyboardUserSettingsTab", () => {
 
     it("doesn't render same modifier twice", async () => {
         mockKeyboardShortcuts({
-            "getKeyboardShortcuts": () => ({
+            "getKeyboardShortcutsForUI": () => ({
                 "keybind1": {
                     default: {
                         key: Key.A,
@@ -76,7 +76,7 @@ describe("KeyboardUserSettingsTab", () => {
         jest.resetModules();
 
         mockKeyboardShortcuts({
-            "getKeyboardShortcuts": () => ({
+            "getKeyboardShortcutsForUI": () => ({
                 "keybind1": {
                     default: {
                         key: Key.A,
@@ -94,7 +94,7 @@ describe("KeyboardUserSettingsTab", () => {
 
     it("renders list of keyboard shortcuts", async () => {
         mockKeyboardShortcuts({
-            "getKeyboardShortcuts": () => ({
+            "getKeyboardShortcutsForUI": () => ({
                 "keybind1": {
                     default: {
                         key: Key.A,