diff --git a/docs/features/keyboardShortcuts.md b/docs/features/keyboardShortcuts.md new file mode 100644 index 0000000000..7181402354 --- /dev/null +++ b/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`. diff --git a/src/KeyBindingsDefaults.ts b/src/KeyBindingsDefaults.ts index a41680848d..5870f6451e 100644 --- a/src/KeyBindingsDefaults.ts +++ b/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, diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 9ac4552ace..c7f4ef68bb 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/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]) => { diff --git a/src/components/views/settings/tabs/user/KeyboardUserSettingsTab.tsx b/src/components/views/settings/tabs/user/KeyboardUserSettingsTab.tsx index d05e226c1a..95495118f8 100644 --- a/src/components/views/settings/tabs/user/KeyboardUserSettingsTab.tsx +++ b/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); }; diff --git a/test/accessibility/KeyboardShortcuts-test.ts b/test/accessibility/KeyboardShortcuts-test.ts index f0d45d7959..b4da655f6f 100644 --- a/test/accessibility/KeyboardShortcuts-test.ts +++ b/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(); }); }); diff --git a/test/components/views/settings/tabs/user/KeyboardUserSettingsTab-test.tsx b/test/components/views/settings/tabs/user/KeyboardUserSettingsTab-test.tsx index a3e367fd75..2a94da0eb4 100644 --- a/test/components/views/settings/tabs/user/KeyboardUserSettingsTab-test.tsx +++ b/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,