Keybinding code unification #3 (#7850)

src/BasePlatform.ts

@@ -309,6 +309,12 @@ export default abstract class BasePlatform {
         return false;
     }
 
+    public overrideBrowserShortcuts(): boolean {
+        return false;
+    }
+
+    public navigateForwardBack(back: boolean): void {}
+
     getAvailableSpellCheckLanguages(): Promise<string[]> | null {
         return null;
     }

src/KeyBindingsDefaults.ts

@@ -152,7 +152,7 @@ const navigationBindings = (): KeyBinding[] => {
     const bindings = getBindingsByCategory(CategoryName.NAVIGATION);
 
     bindings.push({
-        action: "KeyBinding.closeDialogOrContextMenu" as KeyBindingAction,
+        action: KeyBindingAction.CloseDialogOrContextMenu,
         keyCombo: {
             key: Key.ESCAPE,
         },
@@ -161,6 +161,10 @@ const navigationBindings = (): KeyBinding[] => {
     return bindings;
 };
 
+const callBindings = (): KeyBinding[] => {
+    return getBindingsByCategory(CategoryName.CALLS);
+};
+
 const labsBindings = (): KeyBinding[] => {
     if (!SdkConfig.get()['showLabsSettings']) return [];
 
@@ -173,5 +177,6 @@ export const defaultBindingsProvider: IKeyBindingsProvider = {
     getRoomListBindings: roomListBindings,
     getRoomBindings: roomBindings,
     getNavigationBindings: navigationBindings,
+    getCallBindings: callBindings,
     getLabsBindings: labsBindings,
 };

src/KeyBindingsManager.ts

@@ -155,6 +155,10 @@ export class KeyBindingsManager {
         return this.getAction(this.bindingsProviders.map(it => it.getNavigationBindings), ev);
     }
 
+    getCallAction(ev: KeyboardEvent | React.KeyboardEvent): KeyBindingAction | undefined {
+        return this.getAction(this.bindingsProviders.map(it => it.getCallBindings), ev);
+    }
+
     getLabsAction(ev: KeyboardEvent | React.KeyboardEvent): KeyBindingAction | undefined {
         return this.getAction(this.bindingsProviders.map(it => it.getLabsBindings), ev);
     }

src/accessibility/KeyboardShortcuts.ts

@@ -20,6 +20,7 @@ import { isMac, Key } from "../Keyboard";
 import { IBaseSetting } from "../settings/Settings";
 import SettingsStore from "../settings/SettingsStore";
 import IncompatibleController from "../settings/controllers/IncompatibleController";
+import PlatformPeg from "../PlatformPeg";
 
 export enum KeyBindingAction {
     /** Send a message */
@@ -115,6 +116,25 @@ export enum KeyBindingAction {
     /** Select next room with unread messages */
     SelectNextUnreadRoom = 'KeyBinding.nextUnreadRoom',
 
+    /** Switches to a space by number */
+    SwitchToSpaceByNumber = "KeyBinding.switchToSpaceByNumber",
+    /** Opens user settings */
+    OpenUserSettings = "KeyBinding.openUserSettings",
+    /** Navigates backward */
+    PreviousVisitedRoomOrCommunity = "KeyBinding.previousVisitedRoomOrCommunity",
+    /** Navigates forward */
+    NextVisitedRoomOrCommunity = "KeyBinding.nextVisitedRoomOrCommunity",
+
+    /** Toggles microphone while on a call */
+    ToggleMicInCall = "KeyBinding.toggleMicInCall",
+    /** Toggles webcam while on a call */
+    ToggleWebcamInCall = "KeyBinding.toggleWebcamInCall",
+
+    /** Closes a dialog or a context menu */
+    CloseDialogOrContextMenu = "KeyBinding.closeDialogOrContextMenu",
+    /** Clicks the selected button */
+    ActivateSelectedButton = "KeyBinding.activateSelectedButton",
+
     /** Toggle visibility of hidden events */
     ToggleHiddenEventVisibility = 'KeyBinding.toggleHiddenEventVisibility',
 }
@@ -132,13 +152,13 @@ type KeyboardShortcutSetting = IBaseSetting<KeyBindingConfig>;
 
 type IKeyboardShortcuts = {
     // TODO: We should figure out what to do with the keyboard shortcuts that are not handled by KeybindingManager
-    [k in (KeyBindingAction | string)]: KeyboardShortcutSetting;
+    [k in (KeyBindingAction)]?: KeyboardShortcutSetting;
 };
 
 export interface ICategory {
     categoryLabel: string;
     // TODO: We should figure out what to do with the keyboard shortcuts that are not handled by KeybindingManager
-    settingNames: (KeyBindingAction | string)[];
+    settingNames: (KeyBindingAction)[];
 }
 
 export enum CategoryName {
@@ -200,8 +220,8 @@ export const CATEGORIES: Record<CategoryName, ICategory> = {
     }, [CategoryName.CALLS]: {
         categoryLabel: _td("Calls"),
         settingNames: [
-            "KeyBinding.toggleMicInCall",
+            KeyBindingAction.ToggleMicInCall,
-            "KeyBinding.toggleWebcamInCall",
+            KeyBindingAction.ToggleWebcamInCall,
         ],
     }, [CategoryName.ROOM]: {
         categoryLabel: _td("Room"),
@@ -229,8 +249,8 @@ export const CATEGORIES: Record<CategoryName, ICategory> = {
         categoryLabel: _td("Navigation"),
         settingNames: [
             KeyBindingAction.ToggleUserMenu,
-            "KeyBinding.closeDialogOrContextMenu",
+            KeyBindingAction.CloseDialogOrContextMenu,
-            "KeyBinding.activateSelectedButton",
+            KeyBindingAction.ActivateSelectedButton,
             KeyBindingAction.ToggleRoomSidePanel,
             KeyBindingAction.ToggleSpacePanel,
             KeyBindingAction.ShowKeyboardSettings,
@@ -240,6 +260,10 @@ export const CATEGORIES: Record<CategoryName, ICategory> = {
             KeyBindingAction.SelectPrevUnreadRoom,
             KeyBindingAction.SelectNextRoom,
             KeyBindingAction.SelectPrevRoom,
+            KeyBindingAction.OpenUserSettings,
+            KeyBindingAction.SwitchToSpaceByNumber,
+            KeyBindingAction.PreviousVisitedRoomOrCommunity,
+            KeyBindingAction.NextVisitedRoomOrCommunity,
         ],
     }, [CategoryName.AUTOCOMPLETE]: {
         categoryLabel: _td("Autocomplete"),
@@ -258,6 +282,17 @@ export const CATEGORIES: Record<CategoryName, ICategory> = {
     },
 };
 
+const DESKTOP_SHORTCUTS = [
+    KeyBindingAction.OpenUserSettings,
+    KeyBindingAction.SwitchToSpaceByNumber,
+    KeyBindingAction.PreviousVisitedRoomOrCommunity,
+    KeyBindingAction.NextVisitedRoomOrCommunity,
+];
+
+const MAC_ONLY_SHORTCUTS = [
+    KeyBindingAction.OpenUserSettings,
+];
+
 // This is very intentionally modelled after SETTINGS as it will make it easier
 // to implement customizable keyboard shortcuts
 // TODO: TravisR will fix this nightmare when the new version of the SettingsStore becomes a thing
@@ -332,14 +367,14 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = {
         },
         displayName: _td("Navigate to previous message in composer history"),
     },
-    "KeyBinding.toggleMicInCall": {
+    [KeyBindingAction.ToggleMicInCall]: {
         default: {
             ctrlOrCmdKey: true,
             key: Key.D,
         },
         displayName: _td("Toggle microphone mute"),
     },
-    "KeyBinding.toggleWebcamInCall": {
+    [KeyBindingAction.ToggleWebcamInCall]: {
         default: {
             ctrlOrCmdKey: true,
             key: Key.E,
@@ -538,13 +573,51 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = {
         },
         displayName: _td("Undo edit"),
     },
+    [KeyBindingAction.EditRedo]: {
+        default: {
+            key: isMac ? Key.Z : Key.Y,
+            ctrlOrCmdKey: true,
+            shiftKey: isMac,
+        },
+        displayName: _td("Redo edit"),
+    },
+    [KeyBindingAction.PreviousVisitedRoomOrCommunity]: {
+        default: {
+            metaKey: isMac,
+            altKey: !isMac,
+            key: isMac ? Key.SQUARE_BRACKET_LEFT : Key.ARROW_LEFT,
+        },
+        displayName: _td("Previous recently visited room or community"),
+    },
+    [KeyBindingAction.NextVisitedRoomOrCommunity]: {
+        default: {
+            metaKey: isMac,
+            altKey: !isMac,
+            key: isMac ? Key.SQUARE_BRACKET_RIGHT : Key.ARROW_RIGHT,
+        },
+        displayName: _td("Next recently visited room or community"),
+    },
+    [KeyBindingAction.SwitchToSpaceByNumber]: {
+        default: {
+            ctrlOrCmdKey: true,
+            key: DIGITS,
+        },
+        displayName: _td("Switch to space by number"),
+    },
+    [KeyBindingAction.OpenUserSettings]: {
+        default: {
+            metaKey: true,
+            key: Key.COMMA,
+        },
+        displayName: _td("Open user settings"),
+    },
 };
 
 // XXX: These have to be manually mirrored in KeyBindingDefaults
 const getNonCustomizableShortcuts = (): IKeyboardShortcuts => {
     const ctrlEnterToSend = SettingsStore.getValue('MessageComposerInput.ctrlEnterToSend');
 
-    return {
+    const keyboardShortcuts: IKeyboardShortcuts = {
         [KeyBindingAction.SendMessage]: {
             default: {
                 key: Key.ENTER,
@@ -578,39 +651,46 @@ const getNonCustomizableShortcuts = (): IKeyboardShortcuts => {
             },
             displayName: _td("Search (must be enabled)"),
         },
-        "KeyBinding.closeDialogOrContextMenu": {
+        [KeyBindingAction.CloseDialogOrContextMenu]: {
             default: {
                 key: Key.ESCAPE,
             },
             displayName: _td("Close dialog or context menu"),
         },
-        "KeyBinding.activateSelectedButton": {
+        [KeyBindingAction.ActivateSelectedButton]: {
             default: {
                 key: Key.ENTER,
             },
             displayName: _td("Activate selected button"),
         },
     };
+
+    if (PlatformPeg.get().overrideBrowserShortcuts()) {
+        keyboardShortcuts[KeyBindingAction.SwitchToSpaceByNumber] = {
+            default: {
+                ctrlOrCmdKey: true,
+                key: DIGITS,
+            },
+            displayName: _td("Switch to space by number"),
+        };
+    }
+
+    return keyboardShortcuts;
 };
 
 export const getCustomizableShortcuts = (): IKeyboardShortcuts => {
-    const keyboardShortcuts = Object.assign({}, KEYBOARD_SHORTCUTS);
+    const overrideBrowserShortcuts = PlatformPeg.get().overrideBrowserShortcuts();
 
-    keyboardShortcuts[KeyBindingAction.EditRedo] = {
+    return Object.keys(KEYBOARD_SHORTCUTS).filter((k: KeyBindingAction) => {
-        default: {
+        if (KEYBOARD_SHORTCUTS[k]?.controller?.settingDisabled) return false;
-            key: isMac ? Key.Z : Key.Y,
+        if (MAC_ONLY_SHORTCUTS.includes(k) && !isMac) return false;
-            ctrlOrCmdKey: true,
+        if (DESKTOP_SHORTCUTS.includes(k) && !overrideBrowserShortcuts) return false;
-            shiftKey: isMac,
-        },
-        displayName: _td("Redo edit"),
-    };
 
-    return Object.keys(keyboardShortcuts).filter(k => {
+        return true;
-        return !keyboardShortcuts[k].controller?.settingDisabled;
     }).reduce((o, key) => {
-        o[key] = keyboardShortcuts[key];
+        o[key] = KEYBOARD_SHORTCUTS[key];
         return o;
-    }, {});
+    }, {} as IKeyboardShortcuts);
 };
 
 export const getKeyboardShortcuts = (): IKeyboardShortcuts => {
@@ -622,14 +702,15 @@ export const getKeyboardShortcuts = (): IKeyboardShortcuts => {
     return entries.reduce((acc, [key, value]) => {
         acc[key] = value;
         return acc;
-    }, {});
+    }, {} as IKeyboardShortcuts);
 };
 
-export const registerShortcut = (
+// For tests
-    shortcutName: string,
+export function mock({ keyboardShortcuts, macOnlyShortcuts, desktopShortcuts }): void {
-    categoryName: CategoryName,
+    Object.keys(KEYBOARD_SHORTCUTS).forEach((k) => delete KEYBOARD_SHORTCUTS[k]);
-    shortcut: KeyboardShortcutSetting,
+    if (keyboardShortcuts) Object.assign(KEYBOARD_SHORTCUTS, keyboardShortcuts);
-): void => {
+    MAC_ONLY_SHORTCUTS.splice(0, MAC_ONLY_SHORTCUTS.length);
-    KEYBOARD_SHORTCUTS[shortcutName] = shortcut;
+    if (macOnlyShortcuts) macOnlyShortcuts.forEach((e) => MAC_ONLY_SHORTCUTS.push(e));
-    CATEGORIES[categoryName].settingNames.push(shortcutName);
+    DESKTOP_SHORTCUTS.splice(0, DESKTOP_SHORTCUTS.length);
-};
+    if (desktopShortcuts) desktopShortcuts.forEach((e) => DESKTOP_SHORTCUTS.push(e));
+}

src/components/structures/LoggedInView.tsx

@@ -23,7 +23,7 @@ import { ISyncStateData, SyncState } from 'matrix-js-sdk/src/sync';
 import { IUsageLimit } from 'matrix-js-sdk/src/@types/partials';
 import { RoomStateEvent } from "matrix-js-sdk/src/models/room-state";
 
-import { Key } from '../../Keyboard';
+import { isOnlyCtrlOrCmdKeyEvent, Key } from '../../Keyboard';
 import PageTypes from '../../PageTypes';
 import MediaDeviceHandler from '../../MediaDeviceHandler';
 import { fixupColorFonts } from '../../utils/FontManager';
@@ -73,6 +73,7 @@ import { OpenToTabPayload } from "../../dispatcher/payloads/OpenToTabPayload";
 import RightPanelStore from '../../stores/right-panel/RightPanelStore';
 import { TimelineRenderingType } from "../../contexts/RoomContext";
 import { KeyBindingAction } from "../../accessibility/KeyboardShortcuts";
+import { SwitchSpacePayload } from "../../dispatcher/payloads/SwitchSpacePayload";
 
 // We need to fetch each pinned message individually (if we don't already have it)
 // so each pinned message may trigger a request. Limit the number per room for sanity.
@@ -535,9 +536,14 @@ class LoggedInView extends React.Component<IProps, IState> {
                     unread: true,
                 });
                 break;
-            default:
+            case KeyBindingAction.PreviousVisitedRoomOrCommunity:
-                // if we do not have a handler for it, pass it to the platform which might
+                PlatformPeg.get().navigateForwardBack(true);
-                handled = PlatformPeg.get().onKeyDown(ev);
+                handled = true;
+                break;
+            case KeyBindingAction.NextVisitedRoomOrCommunity:
+                PlatformPeg.get().navigateForwardBack(false);
+                handled = true;
+                break;
         }
 
         // Handle labs actions here, as they apply within the same scope
@@ -560,12 +566,24 @@ class LoggedInView extends React.Component<IProps, IState> {
                     handled = true;
                     break;
                 }
-                default:
-                    // if we do not have a handler for it, pass it to the platform which might
-                    handled = PlatformPeg.get().onKeyDown(ev);
             }
         }
 
+        if (
+            !handled &&
+            PlatformPeg.get().overrideBrowserShortcuts() &&
+            SpaceStore.spacesEnabled &&
+            ev.code.startsWith("Digit") &&
+            ev.code !== "Digit0" && // this is the shortcut for reset zoom, don't override it
+            isOnlyCtrlOrCmdKeyEvent(ev)
+        ) {
+            dis.dispatch<SwitchSpacePayload>({
+                action: Action.SwitchSpace,
+                num: ev.code.slice(5), // Cut off the first 5 characters - "Digit"
+            });
+            handled = true;
+        }
+
         if (handled) {
             ev.stopPropagation();
             ev.preventDefault();

src/components/views/voip/CallView.tsx

@@ -29,7 +29,6 @@ import { _t, _td } from '../../../languageHandler';
 import VideoFeed from './VideoFeed';
 import RoomAvatar from "../avatars/RoomAvatar";
 import AccessibleButton from '../elements/AccessibleButton';
-import { isOnlyCtrlOrCmdKeyEvent, Key } from '../../../Keyboard';
 import { avatarUrlForMember } from '../../../Avatar';
 import { replaceableComponent } from "../../../utils/replaceableComponent";
 import DesktopCapturerSourcePicker from "../elements/DesktopCapturerSourcePicker";
@@ -38,6 +37,8 @@ import CallViewSidebar from './CallViewSidebar';
 import CallViewHeader from './CallView/CallViewHeader';
 import CallViewButtons from "./CallView/CallViewButtons";
 import PlatformPeg from "../../../PlatformPeg";
+import { getKeyBindingsManager } from "../../../KeyBindingsManager";
+import { KeyBindingAction } from "../../../accessibility/KeyboardShortcuts";
 
 interface IProps {
     // The call for us to display
@@ -293,25 +294,21 @@ export default class CallView extends React.Component<IProps, IState> {
     // CallHandler would probably be a better place for this
     private onNativeKeyDown = (ev): void => {
         let handled = false;
-        const ctrlCmdOnly = isOnlyCtrlOrCmdKeyEvent(ev);
 
-        switch (ev.key) {
+        const callAction = getKeyBindingsManager().getCallAction(ev);
-            case Key.D:
+        switch (callAction) {
-                if (ctrlCmdOnly) {
+            case KeyBindingAction.ToggleMicInCall:
                 this.onMicMuteClick();
                 // show the controls to give feedback
                 this.buttonsRef.current?.showControls();
                 handled = true;
-                }
                 break;
 
-            case Key.E:
+            case KeyBindingAction.ToggleWebcamInCall:
-                if (ctrlCmdOnly) {
                 this.onVidMuteClick();
                 // show the controls to give feedback
                 this.buttonsRef.current?.showControls();
                 handled = true;
-                }
                 break;
         }
 

src/i18n/strings/en_EN.json

@@ -3442,10 +3442,14 @@
     "Jump to first message": "Jump to first message",
     "Jump to last message": "Jump to last message",
     "Undo edit": "Undo edit",
+    "Redo edit": "Redo edit",
+    "Previous recently visited room or community": "Previous recently visited room or community",
+    "Next recently visited room or community": "Next recently visited room or community",
+    "Switch to space by number": "Switch to space by number",
+    "Open user settings": "Open user settings",
     "New line": "New line",
     "Force complete": "Force complete",
     "Search (must be enabled)": "Search (must be enabled)",
     "Close dialog or context menu": "Close dialog or context menu",
-    "Activate selected button": "Activate selected button",
+    "Activate selected button": "Activate selected button"
-    "Redo edit": "Redo edit"
 }

test/accessibility/KeyboardShortcuts-test.ts

@@ -15,18 +15,24 @@ limitations under the License.
 */
 
 import {
-    CATEGORIES,
-    CategoryName,
     getCustomizableShortcuts,
     getKeyboardShortcuts,
     KEYBOARD_SHORTCUTS,
-    registerShortcut,
+    mock,
 } from "../../src/accessibility/KeyboardShortcuts";
-import { Key } from "../../src/Keyboard";
+import PlatformPeg from "../../src/PlatformPeg";
-import { ISetting } from "../../src/settings/Settings";
 
 describe("KeyboardShortcuts", () => {
-    it("doesn't change KEYBOARD_SHORTCUTS when getting shortcuts", () => {
+    it("doesn't change KEYBOARD_SHORTCUTS when getting shortcuts", async () => {
+        mock({
+            keyboardShortcuts: {
+                "Keybind1": {},
+                "Keybind2": {},
+            },
+            macOnlyShortcuts: ["Keybind1"],
+            desktopShortcuts: ["Keybind2"],
+        });
+        PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => false });
         const copyKeyboardShortcuts = Object.assign({}, KEYBOARD_SHORTCUTS);
 
         getCustomizableShortcuts();
@@ -35,22 +41,31 @@ describe("KeyboardShortcuts", () => {
         expect(KEYBOARD_SHORTCUTS).toEqual(copyKeyboardShortcuts);
     });
 
-    describe("registerShortcut()", () => {
+    it("correctly filters shortcuts", async () => {
-        it("correctly registers shortcut", () => {
+        mock({
-            const shortcutName = "Keybinding.definitelyARealShortcut";
+            keyboardShortcuts: {
-            const shortcutCategory = CategoryName.NAVIGATION;
+                "Keybind1": {},
-            const shortcut: ISetting = {
+                "Keybind2": {},
-                displayName: "A real shortcut",
+                "Keybind3": { "controller": { settingDisabled: true } },
-                default: {
+                "Keybind4": {},
-                    ctrlKey: true,
-                    key: Key.A,
             },
-            };
+            macOnlyShortcuts: ["Keybind1"],
+            desktopShortcuts: ["Keybind2"],
 
-            registerShortcut(shortcutName, shortcutCategory, shortcut);
-
-            expect(getKeyboardShortcuts()[shortcutName]).toBe(shortcut);
-            expect(CATEGORIES[shortcutCategory].settingNames.includes(shortcutName)).toBeTruthy();
         });
+        PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => false });
+        expect(getCustomizableShortcuts()).toEqual({ "Keybind4": {} });
+
+        mock({
+            keyboardShortcuts: {
+                "Keybind1": {},
+                "Keybind2": {},
+            },
+            macOnlyShortcuts: undefined,
+            desktopShortcuts: ["Keybind2"],
+        });
+        PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => true });
+        expect(getCustomizableShortcuts()).toEqual({ "Keybind1": {}, "Keybind2": {} });
+        jest.resetModules();
     });
 });

test/components/views/rooms/SendMessageComposer-test.tsx

@@ -37,6 +37,7 @@ import MatrixToPermalinkConstructor from "../../../../src/utils/permalinks/Matri
 import defaultDispatcher from "../../../../src/dispatcher/dispatcher";
 import DocumentOffset from '../../../../src/editor/offset';
 import { Layout } from '../../../../src/settings/enums/Layout';
+import PlatformPeg from "../../../../src/PlatformPeg";
 
 describe('<SendMessageComposer/>', () => {
     const roomContext = {
@@ -253,6 +254,8 @@ describe('<SendMessageComposer/>', () => {
         });
 
         it("persists to session history upon sending", async () => {
+            PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => false });
+
             const wrapper = mount(<MatrixClientContext.Provider value={mockClient}>
                 <RoomContext.Provider value={roomContext}>