From 18fa1371d3461d9d968f4a223493b9670a10f6ef Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Wed, 19 Apr 2023 15:17:32 -0400 Subject: [PATCH 1/6] Use a ref for spacebarHeld because we can and it means fewer renders --- src/useCallViewKeyboardShortcuts.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/useCallViewKeyboardShortcuts.ts b/src/useCallViewKeyboardShortcuts.ts index 8a247d0..d1689dd 100644 --- a/src/useCallViewKeyboardShortcuts.ts +++ b/src/useCallViewKeyboardShortcuts.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { useCallback, useState } from "react"; +import { useCallback, useRef } from "react"; import { getSetting } from "./settings/useSetting"; import { useEventTarget } from "./useEvents"; @@ -25,7 +25,7 @@ export function useCallViewKeyboardShortcuts( toggleLocalVideoMuted: () => void, setMicrophoneMuted: (muted: boolean) => void ) { - const [spacebarHeld, setSpacebarHeld] = useState(false); + const spacebarHeld = useRef(false); useEventTarget( window, @@ -43,8 +43,8 @@ export function useCallViewKeyboardShortcuts( toggleMicrophoneMuted(); } else if (event.key == "v") { toggleLocalVideoMuted(); - } else if (event.key === " " && !spacebarHeld) { - setSpacebarHeld(true); + } else if (event.key === " " && !spacebarHeld.current) { + spacebarHeld.current = true; setMicrophoneMuted(false); } }, @@ -54,7 +54,6 @@ export function useCallViewKeyboardShortcuts( toggleLocalVideoMuted, toggleMicrophoneMuted, setMicrophoneMuted, - setSpacebarHeld, ] ) ); @@ -72,11 +71,11 @@ export function useCallViewKeyboardShortcuts( } if (event.key === " ") { - setSpacebarHeld(false); + spacebarHeld.current = false; setMicrophoneMuted(true); } }, - [enabled, setMicrophoneMuted, setSpacebarHeld] + [enabled, setMicrophoneMuted] ) ); @@ -85,9 +84,9 @@ export function useCallViewKeyboardShortcuts( "blur", useCallback(() => { if (spacebarHeld) { - setSpacebarHeld(false); + spacebarHeld.current = false; setMicrophoneMuted(true); } - }, [setMicrophoneMuted, setSpacebarHeld, spacebarHeld]) + }, [setMicrophoneMuted, spacebarHeld]) ); } From 56bd54a645e42a65de648053d77db78400badc73 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Wed, 19 Apr 2023 15:51:44 -0400 Subject: [PATCH 2/6] Disable keyboard shortcuts when focus is in a modal --- src/room/InCallView.tsx | 2 +- src/useCallViewKeyboardShortcuts.ts | 34 ++++++++++++++++++----------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index cf1e4dc..1202534 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -157,7 +157,7 @@ export function InCallView({ const { hideScreensharing } = useUrlParams(); useCallViewKeyboardShortcuts( - !feedbackModalState.isOpen, + containerRef1, toggleMicrophoneMuted, toggleLocalVideoMuted, setMicrophoneMuted diff --git a/src/useCallViewKeyboardShortcuts.ts b/src/useCallViewKeyboardShortcuts.ts index d1689dd..e753ea4 100644 --- a/src/useCallViewKeyboardShortcuts.ts +++ b/src/useCallViewKeyboardShortcuts.ts @@ -14,30 +14,41 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { useCallback, useRef } from "react"; +import { RefObject, useCallback, useRef } from "react"; import { getSetting } from "./settings/useSetting"; import { useEventTarget } from "./useEvents"; +/** + * Determines whether focus is in the same part of the tree as the given + * element (specifically, if an ancestor or descendant of it is focused). + */ +const mayReceiveKeyEvents = (e: HTMLElement): boolean => { + const focusedElement = document.activeElement + return focusedElement !== null && (focusedElement.contains(e) || e.contains(focusedElement)) +} + export function useCallViewKeyboardShortcuts( - enabled: boolean, + focusElement: RefObject, toggleMicrophoneMuted: () => void, toggleLocalVideoMuted: () => void, setMicrophoneMuted: (muted: boolean) => void ) { const spacebarHeld = useRef(false); + // These event handlers are set on the window because we want users to be able + // to trigger them without going to the trouble of focusing something + useEventTarget( window, "keydown", useCallback( (event: KeyboardEvent) => { - if (!enabled) return; + if (focusElement.current === null) return + if (!mayReceiveKeyEvents(focusElement.current)) return; // Check if keyboard shortcuts are enabled const keyboardShortcuts = getSetting("keyboard-shortcuts", true); - if (!keyboardShortcuts) { - return; - } + if (!keyboardShortcuts) return; if (event.key === "m") { toggleMicrophoneMuted(); @@ -49,8 +60,6 @@ export function useCallViewKeyboardShortcuts( } }, [ - enabled, - spacebarHeld, toggleLocalVideoMuted, toggleMicrophoneMuted, setMicrophoneMuted, @@ -63,19 +72,18 @@ export function useCallViewKeyboardShortcuts( "keyup", useCallback( (event: KeyboardEvent) => { - if (!enabled) return; + if (focusElement.current === null) return + if (!mayReceiveKeyEvents(focusElement.current)) return; // Check if keyboard shortcuts are enabled const keyboardShortcuts = getSetting("keyboard-shortcuts", true); - if (!keyboardShortcuts) { - return; - } + if (!keyboardShortcuts) return; if (event.key === " ") { spacebarHeld.current = false; setMicrophoneMuted(true); } }, - [enabled, setMicrophoneMuted] + [setMicrophoneMuted] ) ); From 1184b71396350af6a7fa3d370f6708b8ed3907c2 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Wed, 19 Apr 2023 15:54:39 -0400 Subject: [PATCH 3/6] Format with Prettier --- src/useCallViewKeyboardShortcuts.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/useCallViewKeyboardShortcuts.ts b/src/useCallViewKeyboardShortcuts.ts index e753ea4..5f9c457 100644 --- a/src/useCallViewKeyboardShortcuts.ts +++ b/src/useCallViewKeyboardShortcuts.ts @@ -24,9 +24,12 @@ import { useEventTarget } from "./useEvents"; * element (specifically, if an ancestor or descendant of it is focused). */ const mayReceiveKeyEvents = (e: HTMLElement): boolean => { - const focusedElement = document.activeElement - return focusedElement !== null && (focusedElement.contains(e) || e.contains(focusedElement)) -} + const focusedElement = document.activeElement; + return ( + focusedElement !== null && + (focusedElement.contains(e) || e.contains(focusedElement)) + ); +}; export function useCallViewKeyboardShortcuts( focusElement: RefObject, @@ -44,7 +47,7 @@ export function useCallViewKeyboardShortcuts( "keydown", useCallback( (event: KeyboardEvent) => { - if (focusElement.current === null) return + if (focusElement.current === null) return; if (!mayReceiveKeyEvents(focusElement.current)) return; // Check if keyboard shortcuts are enabled const keyboardShortcuts = getSetting("keyboard-shortcuts", true); @@ -59,11 +62,7 @@ export function useCallViewKeyboardShortcuts( setMicrophoneMuted(false); } }, - [ - toggleLocalVideoMuted, - toggleMicrophoneMuted, - setMicrophoneMuted, - ] + [toggleLocalVideoMuted, toggleMicrophoneMuted, setMicrophoneMuted] ) ); @@ -72,7 +71,7 @@ export function useCallViewKeyboardShortcuts( "keyup", useCallback( (event: KeyboardEvent) => { - if (focusElement.current === null) return + if (focusElement.current === null) return; if (!mayReceiveKeyEvents(focusElement.current)) return; // Check if keyboard shortcuts are enabled const keyboardShortcuts = getSetting("keyboard-shortcuts", true); From 6f1398981928343a4987a59fbc98e431c5bc61ac Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Wed, 19 Apr 2023 15:55:55 -0400 Subject: [PATCH 4/6] Fix lint errors --- src/useCallViewKeyboardShortcuts.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/useCallViewKeyboardShortcuts.ts b/src/useCallViewKeyboardShortcuts.ts index 5f9c457..e25cb04 100644 --- a/src/useCallViewKeyboardShortcuts.ts +++ b/src/useCallViewKeyboardShortcuts.ts @@ -62,7 +62,12 @@ export function useCallViewKeyboardShortcuts( setMicrophoneMuted(false); } }, - [toggleLocalVideoMuted, toggleMicrophoneMuted, setMicrophoneMuted] + [ + focusElement, + toggleLocalVideoMuted, + toggleMicrophoneMuted, + setMicrophoneMuted, + ] ) ); @@ -82,7 +87,7 @@ export function useCallViewKeyboardShortcuts( setMicrophoneMuted(true); } }, - [setMicrophoneMuted] + [focusElement, setMicrophoneMuted] ) ); From 4114622d44bf0568c9ce09be90cc1766af0d8f11 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Wed, 19 Apr 2023 16:15:38 -0400 Subject: [PATCH 5/6] Remove the keyboard shortcut setting --- src/settings/SettingsModal.tsx | 17 ----------------- src/settings/useSetting.ts | 3 --- src/useCallViewKeyboardShortcuts.ts | 7 ------- 3 files changed, 27 deletions(-) diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index 6e38b5f..b126bb8 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -28,7 +28,6 @@ import { ReactComponent as OverflowIcon } from "../icons/Overflow.svg"; import { SelectInput } from "../input/SelectInput"; import { useMediaHandler } from "./useMediaHandler"; import { - useKeyboardShortcuts, useSpatialAudio, useShowInspector, useOptInAnalytics, @@ -65,7 +64,6 @@ export const SettingsModal = (props: Props) => { const [optInAnalytics, setOptInAnalytics] = useOptInAnalytics(); const [developerSettingsTab, setDeveloperSettingsTab] = useDeveloperSettingsTab(); - const [keyboardShortcuts, setKeyboardShortcuts] = useKeyboardShortcuts(); const [newGrid, setNewGrid] = useNewGrid(); const downloadDebugLog = useDownloadDebugLog(); @@ -176,21 +174,6 @@ export const SettingsModal = (props: Props) => { } > -

Keyboard

- - ) => - setKeyboardShortcuts(event.target.checked) - } - /> -

Analytics

=> { return [false, null]; }; -export const useKeyboardShortcuts = () => - useSetting("keyboard-shortcuts", true); - export const useNewGrid = () => useSetting("new-grid", false); export const useDeveloperSettingsTab = () => diff --git a/src/useCallViewKeyboardShortcuts.ts b/src/useCallViewKeyboardShortcuts.ts index e25cb04..11ea8f0 100644 --- a/src/useCallViewKeyboardShortcuts.ts +++ b/src/useCallViewKeyboardShortcuts.ts @@ -16,7 +16,6 @@ limitations under the License. import { RefObject, useCallback, useRef } from "react"; -import { getSetting } from "./settings/useSetting"; import { useEventTarget } from "./useEvents"; /** @@ -49,9 +48,6 @@ export function useCallViewKeyboardShortcuts( (event: KeyboardEvent) => { if (focusElement.current === null) return; if (!mayReceiveKeyEvents(focusElement.current)) return; - // Check if keyboard shortcuts are enabled - const keyboardShortcuts = getSetting("keyboard-shortcuts", true); - if (!keyboardShortcuts) return; if (event.key === "m") { toggleMicrophoneMuted(); @@ -78,9 +74,6 @@ export function useCallViewKeyboardShortcuts( (event: KeyboardEvent) => { if (focusElement.current === null) return; if (!mayReceiveKeyEvents(focusElement.current)) return; - // Check if keyboard shortcuts are enabled - const keyboardShortcuts = getSetting("keyboard-shortcuts", true); - if (!keyboardShortcuts) return; if (event.key === " ") { spacebarHeld.current = false; From 28368da60a347ce5daf0120e5de34d21d3f4f0b1 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Wed, 19 Apr 2023 16:20:53 -0400 Subject: [PATCH 6/6] Update strings --- public/locales/en-GB/app.json | 2 -- 1 file changed, 2 deletions(-) diff --git a/public/locales/en-GB/app.json b/public/locales/en-GB/app.json index 389eeb7..6e91a53 100644 --- a/public/locales/en-GB/app.json +++ b/public/locales/en-GB/app.json @@ -106,7 +106,6 @@ "Show call inspector": "Show call inspector", "Sign in": "Sign in", "Sign out": "Sign out", - "Single-key keyboard shortcuts": "Single-key keyboard shortcuts", "Spatial audio": "Spatial audio", "Speaker": "Speaker", "Speaker {{n}}": "Speaker {{n}}", @@ -138,7 +137,6 @@ "Walkie-talkie call": "Walkie-talkie call", "Walkie-talkie call name": "Walkie-talkie call name", "WebRTC is not supported or is being blocked in this browser.": "WebRTC is not supported or is being blocked in this browser.", - "Whether to enable single-key keyboard shortcuts, e.g. 'm' to mute/unmute the mic.": "Whether to enable single-key keyboard shortcuts, e.g. 'm' to mute/unmute the mic.", "Yes, join call": "Yes, join call", "You can't talk at the same time": "You can't talk at the same time", "Your recent calls": "Your recent calls"