From d91f9f63eac930106145ea6c27dd189d0d7275e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 19 Feb 2022 10:07:10 +0100 Subject: [PATCH 1/8] Don't use strings as keys and properly handle call keybinds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- src/KeyBindingsDefaults.ts | 5 ++++ src/KeyBindingsManager.ts | 4 +++ src/accessibility/KeyboardShortcuts.ts | 40 +++++++++++++++++--------- src/components/views/voip/CallView.tsx | 33 ++++++++++----------- 4 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/KeyBindingsDefaults.ts b/src/KeyBindingsDefaults.ts index 76d55b42d3a..b9166700507 100644 --- a/src/KeyBindingsDefaults.ts +++ b/src/KeyBindingsDefaults.ts @@ -154,6 +154,10 @@ const navigationBindings = (): KeyBinding[] => { return getBindingsByCategory(CategoryName.NAVIGATION); }; +const callBindings = (): KeyBinding[] => { + return getBindingsByCategory(CategoryName.CALLS); +}; + const labsBindings = (): KeyBinding[] => { if (!SdkConfig.get()['showLabsSettings']) return []; @@ -166,5 +170,6 @@ export const defaultBindingsProvider: IKeyBindingsProvider = { getRoomListBindings: roomListBindings, getRoomBindings: roomBindings, getNavigationBindings: navigationBindings, + getCallBindings: callBindings, getLabsBindings: labsBindings, }; diff --git a/src/KeyBindingsManager.ts b/src/KeyBindingsManager.ts index 995ca3bf323..5f14fbaef16 100644 --- a/src/KeyBindingsManager.ts +++ b/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); } diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 85a9afbc461..5b04d4d4dc1 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -115,19 +115,29 @@ export enum KeyBindingAction { /** Select next room with unread messages */ SelectNextUnreadRoom = 'KeyBinding.nextUnreadRoom', + /** 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', } type IKeyboardShortcuts = { // TODO: We should figure out what to do with the keyboard shortcuts that are not handled by KeybindingManager - [k in (KeyBindingAction | string)]: ISetting; + [k in (KeyBindingAction)]?: ISetting; }; 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 { @@ -189,8 +199,8 @@ export const CATEGORIES: Record = { }, [CategoryName.CALLS]: { categoryLabel: _td("Calls"), settingNames: [ - "KeyBinding.toggleMicInCall", - "KeyBinding.toggleWebcamInCall", + KeyBindingAction.ToggleMicInCall, + KeyBindingAction.ToggleWebcamInCall, ], }, [CategoryName.ROOM]: { categoryLabel: _td("Room"), @@ -218,8 +228,8 @@ export const CATEGORIES: Record = { categoryLabel: _td("Navigation"), settingNames: [ KeyBindingAction.ToggleUserMenu, - "KeyBinding.closeDialogOrContextMenu", - "KeyBinding.activateSelectedButton", + KeyBindingAction.CloseDialogOrContextMenu, + KeyBindingAction.ActivateSelectedButton, KeyBindingAction.ToggleRoomSidePanel, KeyBindingAction.ToggleSpacePanel, KeyBindingAction.ShowKeyboardSettings, @@ -321,14 +331,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, @@ -567,13 +577,13 @@ 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, }, @@ -599,7 +609,7 @@ export const getCustomizableShortcuts = (): IKeyboardShortcuts => { }).reduce((o, key) => { o[key] = keyboardShortcuts[key]; return o; - }, {}); + }, {} as IKeyboardShortcuts); }; export const getKeyboardShortcuts = (): IKeyboardShortcuts => { @@ -611,10 +621,12 @@ export const getKeyboardShortcuts = (): IKeyboardShortcuts => { return entries.reduce((acc, [key, value]) => { acc[key] = value; return acc; - }, {}); + }, {} as IKeyboardShortcuts); }; -export const registerShortcut = (shortcutName: string, categoryName: CategoryName, shortcut: ISetting): void => { +export const registerShortcut = ( + shortcutName: KeyBindingAction | string, categoryName: CategoryName, shortcut: ISetting, +): void => { KEYBOARD_SHORTCUTS[shortcutName] = shortcut; - CATEGORIES[categoryName].settingNames.push(shortcutName); + CATEGORIES[categoryName].settingNames.push(shortcutName as KeyBindingAction); }; diff --git a/src/components/views/voip/CallView.tsx b/src/components/views/voip/CallView.tsx index e0a8220e1f7..a809adcf41e 100644 --- a/src/components/views/voip/CallView.tsx +++ b/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 { // CallHandler would probably be a better place for this private onNativeKeyDown = (ev): void => { let handled = false; - const ctrlCmdOnly = isOnlyCtrlOrCmdKeyEvent(ev); - - switch (ev.key) { - case Key.D: - if (ctrlCmdOnly) { - this.onMicMuteClick(); - // show the controls to give feedback - this.buttonsRef.current?.showControls(); - handled = true; - } + + const callAction = getKeyBindingsManager().getCallAction(ev); + switch (callAction) { + case KeyBindingAction.ToggleMicInCall: + this.onMicMuteClick(); + // show the controls to give feedback + this.buttonsRef.current?.showControls(); + handled = true; break; - case Key.E: - if (ctrlCmdOnly) { - this.onVidMuteClick(); - // show the controls to give feedback - this.buttonsRef.current?.showControls(); - handled = true; - } + case KeyBindingAction.ToggleWebcamInCall: + this.onVidMuteClick(); + // show the controls to give feedback + this.buttonsRef.current?.showControls(); + handled = true; break; } From 39a3b552e6333ed8c52740a47c9135f439fd8900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 19 Feb 2022 13:25:00 +0100 Subject: [PATCH 2/8] Move handling of desktop shortcuts into the react-sdk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- src/BasePlatform.ts | 6 ++ src/accessibility/KeyboardShortcuts.ts | 72 +++++++++++++++++++--- src/components/structures/LoggedInView.tsx | 32 +++++++--- 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/src/BasePlatform.ts b/src/BasePlatform.ts index 54fc6081c9e..40ac9d11996 100644 --- a/src/BasePlatform.ts +++ b/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 | null { return null; } diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 5b04d4d4dc1..d31838b0c61 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -20,6 +20,7 @@ import { isMac, Key } from "../Keyboard"; import { ISetting } 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,15 @@ 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 */ @@ -239,6 +249,10 @@ export const CATEGORIES: Record = { KeyBindingAction.SelectPrevUnreadRoom, KeyBindingAction.SelectNextRoom, KeyBindingAction.SelectPrevRoom, + KeyBindingAction.OpenUserSettings, + KeyBindingAction.SwitchToSpaceByNumber, + KeyBindingAction.PreviousVisitedRoomOrCommunity, + KeyBindingAction.NextVisitedRoomOrCommunity, ], }, [CategoryName.AUTOCOMPLETE]: { categoryLabel: _td("Autocomplete"), @@ -543,7 +557,7 @@ export const KEYBOARD_SHORTCUTS: IKeyboardShortcuts = { const getNonCustomizableShortcuts = (): IKeyboardShortcuts => { const ctrlEnterToSend = SettingsStore.getValue('MessageComposerInput.ctrlEnterToSend'); - return { + const keyboardShortcuts: IKeyboardShortcuts = { [KeyBindingAction.SendMessage]: { default: { key: Key.ENTER, @@ -590,6 +604,18 @@ const getNonCustomizableShortcuts = (): IKeyboardShortcuts => { 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 => { @@ -604,6 +630,43 @@ export const getCustomizableShortcuts = (): IKeyboardShortcuts => { displayName: _td("Redo edit"), }; + if (PlatformPeg.get().overrideBrowserShortcuts()) { + keyboardShortcuts[KeyBindingAction.PreviousVisitedRoomOrCommunity] = { + default: { + metaKey: isMac, + altKey: !isMac, + key: isMac ? Key.SQUARE_BRACKET_LEFT : Key.ARROW_LEFT, + }, + displayName: _td("Previous recently visited room or community"), + }; + keyboardShortcuts[KeyBindingAction.NextVisitedRoomOrCommunity] = { + default: { + metaKey: isMac, + altKey: !isMac, + key: isMac ? Key.SQUARE_BRACKET_RIGHT : Key.ARROW_RIGHT, + }, + displayName: _td("Next recently visited room or community"), + }; + + keyboardShortcuts[KeyBindingAction.SwitchToSpaceByNumber] = { + default: { + ctrlOrCmdKey: true, + key: DIGITS, + }, + displayName: _td("Switch to space by number"), + }; + + if (isMac) { + keyboardShortcuts[KeyBindingAction.OpenUserSettings] = { + default: { + metaKey: true, + key: Key.COMMA, + }, + displayName: _td("Open user settings"), + }; + } + } + return Object.keys(keyboardShortcuts).filter(k => { return !keyboardShortcuts[k].controller?.settingDisabled; }).reduce((o, key) => { @@ -623,10 +686,3 @@ export const getKeyboardShortcuts = (): IKeyboardShortcuts => { return acc; }, {} as IKeyboardShortcuts); }; - -export const registerShortcut = ( - shortcutName: KeyBindingAction | string, categoryName: CategoryName, shortcut: ISetting, -): void => { - KEYBOARD_SHORTCUTS[shortcutName] = shortcut; - CATEGORIES[categoryName].settingNames.push(shortcutName as KeyBindingAction); -}; diff --git a/src/components/structures/LoggedInView.tsx b/src/components/structures/LoggedInView.tsx index 667d59dd312..1a2c8733e07 100644 --- a/src/components/structures/LoggedInView.tsx +++ b/src/components/structures/LoggedInView.tsx @@ -22,7 +22,7 @@ import classNames from 'classnames'; import { ISyncStateData, SyncState } from 'matrix-js-sdk/src/sync'; import { IUsageLimit } from 'matrix-js-sdk/src/@types/partials'; -import { Key } from '../../Keyboard'; +import { isOnlyCtrlOrCmdKeyEvent, Key } from '../../Keyboard'; import PageTypes from '../../PageTypes'; import MediaDeviceHandler from '../../MediaDeviceHandler'; import { fixupColorFonts } from '../../utils/FontManager'; @@ -72,6 +72,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 { unread: true, }); break; - default: - // if we do not have a handler for it, pass it to the platform which might - handled = PlatformPeg.get().onKeyDown(ev); + case KeyBindingAction.PreviousVisitedRoomOrCommunity: + PlatformPeg.get().navigateForwardBack(true); + 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 { 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({ + action: Action.SwitchSpace, + num: ev.code.slice(5), // Cut off the first 5 characters - "Digit" + }); + handled = true; + } + if (handled) { ev.stopPropagation(); ev.preventDefault(); From f44b43d269d214c114f8c0c1d5fe4cb0cf4d02b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 19 Feb 2022 13:42:40 +0100 Subject: [PATCH 3/8] Be consistent about conditional shortcuts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- src/accessibility/KeyboardShortcuts.ts | 105 +++++++++++++------------ 1 file changed, 56 insertions(+), 49 deletions(-) diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index d31838b0c61..39705487fad 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -271,6 +271,17 @@ export const CATEGORIES: Record = { }, }; +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 @@ -551,6 +562,44 @@ 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 @@ -619,58 +668,16 @@ const getNonCustomizableShortcuts = (): IKeyboardShortcuts => { }; export const getCustomizableShortcuts = (): IKeyboardShortcuts => { - const keyboardShortcuts = Object.assign({}, KEYBOARD_SHORTCUTS); + const overrideBrowserShortcuts = PlatformPeg.get().overrideBrowserShortcuts(); - keyboardShortcuts[KeyBindingAction.EditRedo] = { - default: { - key: isMac ? Key.Z : Key.Y, - ctrlOrCmdKey: true, - shiftKey: isMac, - }, - displayName: _td("Redo edit"), - }; - - if (PlatformPeg.get().overrideBrowserShortcuts()) { - keyboardShortcuts[KeyBindingAction.PreviousVisitedRoomOrCommunity] = { - default: { - metaKey: isMac, - altKey: !isMac, - key: isMac ? Key.SQUARE_BRACKET_LEFT : Key.ARROW_LEFT, - }, - displayName: _td("Previous recently visited room or community"), - }; - keyboardShortcuts[KeyBindingAction.NextVisitedRoomOrCommunity] = { - default: { - metaKey: isMac, - altKey: !isMac, - key: isMac ? Key.SQUARE_BRACKET_RIGHT : Key.ARROW_RIGHT, - }, - displayName: _td("Next recently visited room or community"), - }; - - keyboardShortcuts[KeyBindingAction.SwitchToSpaceByNumber] = { - default: { - ctrlOrCmdKey: true, - key: DIGITS, - }, - displayName: _td("Switch to space by number"), - }; - - if (isMac) { - keyboardShortcuts[KeyBindingAction.OpenUserSettings] = { - default: { - metaKey: true, - key: Key.COMMA, - }, - displayName: _td("Open user settings"), - }; - } - } + return Object.keys(KEYBOARD_SHORTCUTS).filter((k: KeyBindingAction) => { + if (KEYBOARD_SHORTCUTS[k]?.controller?.settingDisabled) return false; + if (MAC_ONLY_SHORTCUTS.includes(k) && !isMac) return false; + if (DESKTOP_SHORTCUTS.includes(k) && !overrideBrowserShortcuts) return false; - return Object.keys(keyboardShortcuts).filter(k => { - return !keyboardShortcuts[k].controller?.settingDisabled; + return true; }).reduce((o, key) => { - o[key] = keyboardShortcuts[key]; + o[key] = KEYBOARD_SHORTCUTS[key]; return o; }, {} as IKeyboardShortcuts); }; From 48f3aa787db9aa77ca001a9477e41bcaf0c28331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sat, 19 Feb 2022 13:43:01 +0100 Subject: [PATCH 4/8] i18n MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- src/i18n/strings/en_EN.json | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index ca38c23b057..fe7012d3813 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3435,10 +3435,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", - "Redo edit": "Redo edit" + "Activate selected button": "Activate selected button" } From df852068889f20449d4cc5d1585298edde20c9ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sun, 20 Feb 2022 09:48:04 +0100 Subject: [PATCH 5/8] Fix existing tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- test/accessibility/KeyboardShortcuts-test.ts | 26 ++----------------- .../views/rooms/SendMessageComposer-test.tsx | 3 +++ 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/test/accessibility/KeyboardShortcuts-test.ts b/test/accessibility/KeyboardShortcuts-test.ts index 88be362af9c..ec05fe64ff0 100644 --- a/test/accessibility/KeyboardShortcuts-test.ts +++ b/test/accessibility/KeyboardShortcuts-test.ts @@ -15,18 +15,15 @@ limitations under the License. */ import { - CATEGORIES, - CategoryName, getCustomizableShortcuts, getKeyboardShortcuts, KEYBOARD_SHORTCUTS, - registerShortcut, } from "../../src/accessibility/KeyboardShortcuts"; -import { Key } from "../../src/Keyboard"; -import { ISetting } from "../../src/settings/Settings"; +import PlatformPeg from "../../src/PlatformPeg"; describe("KeyboardShortcuts", () => { it("doesn't change KEYBOARD_SHORTCUTS when getting shortcuts", () => { + PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => false }); const copyKeyboardShortcuts = Object.assign({}, KEYBOARD_SHORTCUTS); getCustomizableShortcuts(); @@ -34,23 +31,4 @@ describe("KeyboardShortcuts", () => { getKeyboardShortcuts(); expect(KEYBOARD_SHORTCUTS).toEqual(copyKeyboardShortcuts); }); - - describe("registerShortcut()", () => { - it("correctly registers shortcut", () => { - const shortcutName = "Keybinding.definitelyARealShortcut"; - const shortcutCategory = CategoryName.NAVIGATION; - const shortcut: ISetting = { - displayName: "A real shortcut", - default: { - ctrlKey: true, - key: Key.A, - }, - }; - - registerShortcut(shortcutName, shortcutCategory, shortcut); - - expect(getKeyboardShortcuts()[shortcutName]).toBe(shortcut); - expect(CATEGORIES[shortcutCategory].settingNames.includes(shortcutName)).toBeTruthy(); - }); - }); }); diff --git a/test/components/views/rooms/SendMessageComposer-test.tsx b/test/components/views/rooms/SendMessageComposer-test.tsx index 6b87c14f5c2..c2015032741 100644 --- a/test/components/views/rooms/SendMessageComposer-test.tsx +++ b/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('', () => { const roomContext = { @@ -255,6 +256,8 @@ describe('', () => { }); it("persists to session history upon sending", async () => { + PlatformPeg.get = () => ({ overrideBrowserShortcuts: () => false }); + const wrapper = mount( From 5111a4be63cce42a3103c98c7b66e86b22f2e397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Sun, 20 Feb 2022 10:14:06 +0100 Subject: [PATCH 6/8] Add more tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- src/accessibility/KeyboardShortcuts.ts | 10 +++++ test/accessibility/KeyboardShortcuts-test.ts | 39 +++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 39705487fad..a91493a5d3e 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -693,3 +693,13 @@ export const getKeyboardShortcuts = (): IKeyboardShortcuts => { return acc; }, {} as IKeyboardShortcuts); }; + +// For tests +export function mock({ keyboardShortcuts, macOnlyShortcuts, desktopShortcuts }): void { + Object.keys(KEYBOARD_SHORTCUTS).forEach((k) => delete KEYBOARD_SHORTCUTS[k]); + if (keyboardShortcuts) Object.assign(KEYBOARD_SHORTCUTS, keyboardShortcuts); + MAC_ONLY_SHORTCUTS.splice(0, MAC_ONLY_SHORTCUTS.length); + if (macOnlyShortcuts) macOnlyShortcuts.forEach((e) => MAC_ONLY_SHORTCUTS.push(e)); + DESKTOP_SHORTCUTS.splice(0, DESKTOP_SHORTCUTS.length); + if (desktopShortcuts) desktopShortcuts.forEach((e) => DESKTOP_SHORTCUTS.push(e)); +} diff --git a/test/accessibility/KeyboardShortcuts-test.ts b/test/accessibility/KeyboardShortcuts-test.ts index ec05fe64ff0..f0d45d79593 100644 --- a/test/accessibility/KeyboardShortcuts-test.ts +++ b/test/accessibility/KeyboardShortcuts-test.ts @@ -18,11 +18,20 @@ import { getCustomizableShortcuts, getKeyboardShortcuts, KEYBOARD_SHORTCUTS, + mock, } from "../../src/accessibility/KeyboardShortcuts"; import PlatformPeg from "../../src/PlatformPeg"; 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); @@ -31,4 +40,32 @@ describe("KeyboardShortcuts", () => { getKeyboardShortcuts(); expect(KEYBOARD_SHORTCUTS).toEqual(copyKeyboardShortcuts); }); + + it("correctly filters shortcuts", async () => { + mock({ + keyboardShortcuts: { + "Keybind1": {}, + "Keybind2": {}, + "Keybind3": { "controller": { settingDisabled: true } }, + "Keybind4": {}, + }, + macOnlyShortcuts: ["Keybind1"], + desktopShortcuts: ["Keybind2"], + + }); + 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(); + }); }); From a23e9b64f60cbe50a2058b666b03c2c89b76d365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Mon, 21 Feb 2022 20:06:17 +0100 Subject: [PATCH 7/8] Use enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- src/KeyBindingsDefaults.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/KeyBindingsDefaults.ts b/src/KeyBindingsDefaults.ts index b75b013b9dc..65b198d69e5 100644 --- a/src/KeyBindingsDefaults.ts +++ b/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, }, From 9066d420771e0ab40c8d170c34c35ff4532a1a37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Brandner?= Date: Tue, 22 Feb 2022 16:25:37 +0100 Subject: [PATCH 8/8] Fix types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Šimon Brandner --- src/accessibility/KeyboardShortcuts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/accessibility/KeyboardShortcuts.ts b/src/accessibility/KeyboardShortcuts.ts index 64d8b9e50d7..7db10078d93 100644 --- a/src/accessibility/KeyboardShortcuts.ts +++ b/src/accessibility/KeyboardShortcuts.ts @@ -152,7 +152,7 @@ type KeyboardShortcutSetting = IBaseSetting; type IKeyboardShortcuts = { // TODO: We should figure out what to do with the keyboard shortcuts that are not handled by KeybindingManager - [k in (KeyBindingAction)]?: ISetting; + [k in (KeyBindingAction)]?: KeyboardShortcutSetting; }; export interface ICategory {