Merge pull request #3927 from element-hq/toger5/back-button-press-control-on-esc

Refactor of Escape keyboard event bahvior (allow mute/emoji shortcuts after closing a modal with escape)

src/Modal.tsx

@@ -92,6 +92,11 @@ export const Modal: FC<Props> = ({
     return (
       <Drawer.Root
         open={open}
+        // This autofocus is a custom vault property and not the
+        // standard HTML autofocus attribute.
+        // It makes the Drawer.Root behave like the `DialogRoot`
+        // eslint-disable-next-line jsx-a11y/no-autofocus
+        autoFocus
         onOpenChange={onOpenChange}
         dismissible={onDismiss !== undefined}
       >

src/room/InCallView.tsx

@@ -227,10 +227,7 @@ export const InCallView: FC<InCallViewProps> = ({
   const toggleVideo = useBehavior(muteStates.video.toggle$);
   const setAudioEnabled = useBehavior(muteStates.audio.setEnabled$);
 
-  // This function incorrectly assumes that there is a camera and microphone, which is not always the case.
-  // TODO: Make sure that this module is resilient when it comes to camera/microphone availability!
   useCallViewKeyboardShortcuts(
-    containerRef1,
     toggleAudio,
     toggleVideo,
     setAudioEnabled,

src/room/LobbyView.tsx

@@ -47,6 +47,7 @@ import { usePageTitle } from "../usePageTitle";
 import { getValue } from "../utils/observable";
 import { useBehavior } from "../useBehavior";
 import { CallFooter } from "../components/CallFooter";
+import { useCallViewKeyboardShortcuts } from "../useCallViewKeyboardShortcuts";
 
 interface Props {
   client: MatrixClient;
@@ -91,6 +92,11 @@ export const LobbyView: FC<Props> = ({
   const [settingsModalOpen, setSettingsModalOpen] = useState(false);
   const [settingsTab, setSettingsTab] = useState(defaultSettingsTab);
 
+  // This function incorrectly assumes that there is a camera and microphone, which is not always the case.
+  // TODO: Make sure that this module is resilient when it comes to camera/microphone availability!
+  // Next to the keyboard shortcuts, this is also responsible for catching escape key presses and forwarding the to mobile -> pip.
+  useCallViewKeyboardShortcuts(toggleAudio, toggleVideo, null, null, null);
+
   const openSettings = useCallback(
     () => setSettingsModalOpen(true),
     [setSettingsModalOpen],

src/useCallViewKeyboardShortcuts.test.tsx

@@ -6,7 +6,7 @@ Please see LICENSE in the repository root for full details.
 */
 
 import { render } from "@testing-library/react";
-import { type FC, useRef, useState } from "react";
+import { type FC, useState } from "react";
 import { expect, test, vi } from "vitest";
 import { Button } from "@vector-im/compound-web";
 import userEvent from "@testing-library/user-event";
@@ -39,9 +39,7 @@ const TestComponent: FC<TestComponentProps> = ({
   initialModalOpen = false,
 }) => {
   const [modalOpen, setModalOpen] = useState(initialModalOpen);
-  const ref = useRef<HTMLDivElement | null>(null);
   useCallViewKeyboardShortcuts(
-    ref,
     () => {},
     () => {},
     setAudioEnabled,
@@ -49,8 +47,11 @@ const TestComponent: FC<TestComponentProps> = ({
     toggleHandRaised,
   );
   return (
-    <div ref={ref}>
-      <Button onClick={onButtonClick}>TEST</Button>
+    <>
+      <div id={initialModalOpen ? "root" : undefined}>
+        <Button onClick={onButtonClick}>TEST</Button>
+      </div>
+      {/*// modal lives outside of the root*/}
       {modalOpen && (
         <dialog
           open
@@ -64,7 +65,7 @@ const TestComponent: FC<TestComponentProps> = ({
           <button>InModalButton</button>
         </dialog>
       )}
-    </div>
+    </>
   );
 };
 
@@ -164,12 +165,13 @@ test("unmuting happens in place of the default action", async () => {
   // container element that can be interactive and receive focus / keydown
   // events. <video> is kind of a weird choice, but it'll do the job.
   render(
-    <video
-      tabIndex={0}
-      onKeyDown={(e) => defaultPrevented(e.isDefaultPrevented())}
-    >
-      <TestComponent setAudioEnabled={() => {}} />
-    </video>,
+    <div id="root">
+      <video
+        tabIndex={0}
+        onKeyDown={(e) => defaultPrevented(e.isDefaultPrevented())}
+      />
+      <TestComponent setAudioEnabled={() => {}} />,
+    </div>,
   );
 
   await user.tab(); // Focus the <video>

src/useCallViewKeyboardShortcuts.ts

@@ -5,7 +5,8 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 Please see LICENSE in the repository root for full details.
 */
 
-import { type RefObject, useCallback, useMemo, useRef } from "react";
+import { useCallback, useMemo, useRef } from "react";
+import { logger } from "matrix-js-sdk/lib/logger";
 
 import { useEventTarget } from "./useEvents";
 import {
@@ -18,22 +19,61 @@ import {
  * Determines whether focus is in the same part of the tree as the given
  * element (specifically, if the element or an ancestor of it is focused).
  */
-const mayReceiveKeyEvents = (e: HTMLElement): boolean => {
-  const focusedElement = document.activeElement;
-  return focusedElement !== null && focusedElement.contains(e);
+const mayReceiveKeyEvents = (): boolean => {
+  const root = document.getElementById("root");
+  if (root === null) {
+    logger.warn(
+      "[mayReceiveKeyEvents] Root element not found, always allow keyboard shortcuts (m,v,esc...)",
+    );
+    return true;
+  }
+  const focusElement = document.activeElement;
+  const nothingInFocus = focusElement === null;
+  const focusOnBody = focusElement === document.body;
+  const noPrimaryFocus =
+    nothingInFocus || root.contains(focusElement) || focusOnBody;
+
+  logger.warn(
+    `[mayReceiveKeyEvents] nothingInFocus ${nothingInFocus}, focusOnBody ${focusOnBody}, noPrimaryFocus ${noPrimaryFocus}`,
+  );
+  // Only if we do not have a primary focus we allow keyboard shortcut events.
+  return noPrimaryFocus;
+};
+
+/**
+ * Only do push to talk behavior if the active element is not a button or button like.
+ */
+const mayReceiveSpaceKeyEvents = (): boolean => {
+  const activeElement = document.activeElement;
+  if (activeElement === null) return true;
+  return activeElement.tagName.toLowerCase() !== "button";
 };
 
 const KeyToReactionMap: Record<string, ReactionOption> = Object.fromEntries(
   ReactionSet.slice(0, ReactionsRowSize).map((r, i) => [(i + 1).toString(), r]),
 );
 
+/**
+ * This hook sets up gloabl keyboard shortcuts. It will filter for keyboard presses that should be ignored due to user
+ * currently focussing on a modal.
+ * This is achieved by using the fact, that all modal inputs are outside the #root element and use react portals to get rendered.
+ * The following shortcuts are auspported (optional):
+ * @param toggleAudio - triggered on (m)
+ * @param toggleVideo - triggered on (v)
+ * @param setAudioEnabled - push to talk behavior controlled via (space)
+ * @param sendReaction - triggered on (1,2,3,...)
+ * @param toggleHandRaised - triggered on (h)
+ * Additionally this method listens to the (escape) key to trigger the onBackButtonPressed callback, which is used to navigate to pip in the native app.
+ *
+ * Note: This function incorrectly assumes that there is a camera and microphone, which is not always the case.
+ */
+// TODO: Make sure that this module is resilient when it comes to camera/microphone availability!
 export function useCallViewKeyboardShortcuts(
-  focusElement: RefObject<HTMLElement | null>,
   toggleAudio: (() => void) | null,
   toggleVideo: (() => void) | null,
   setAudioEnabled: ((enabled: boolean) => void) | null,
-  sendReaction: (reaction: ReactionOption) => void,
-  toggleHandRaised: () => void,
+  sendReaction: ((reaction: ReactionOption) => void) | null,
+  toggleHandRaised: (() => void) | null,
 ): void {
   const spacebarHeld = useRef(false);
 
@@ -45,8 +85,8 @@ export function useCallViewKeyboardShortcuts(
     "keydown",
     useCallback(
       (event: KeyboardEvent) => {
-        if (focusElement.current === null) return;
-        if (!mayReceiveKeyEvents(focusElement.current)) return;
+        logger.info("Keydown event", event);
+        if (!mayReceiveKeyEvents()) return;
         if (event.altKey || event.ctrlKey || event.metaKey || event.shiftKey)
           return;
 
@@ -56,7 +96,7 @@ export function useCallViewKeyboardShortcuts(
         } else if (event.key === "v") {
           event.preventDefault();
           toggleVideo?.();
-        } else if (event.key === " ") {
+        } else if (event.key === " " && mayReceiveSpaceKeyEvents()) {
           event.preventDefault();
           if (!spacebarHeld.current) {
             spacebarHeld.current = true;
@@ -64,16 +104,16 @@ export function useCallViewKeyboardShortcuts(
           }
         } else if (event.key === "h") {
           event.preventDefault();
-          toggleHandRaised();
+          toggleHandRaised?.();
         } else if (KeyToReactionMap[event.key]) {
           event.preventDefault();
-          sendReaction(KeyToReactionMap[event.key]);
+          sendReaction?.(KeyToReactionMap[event.key]);
         } else if (event.key === "Escape") {
+          logger.info("Escape key pressed, triggering onBackButtonPressed");
           window.controls.onBackButtonPressed?.();
         }
       },
       [
-        focusElement,
         toggleVideo,
         toggleAudio,
         setAudioEnabled,
@@ -92,15 +132,13 @@ export function useCallViewKeyboardShortcuts(
     "keyup",
     useCallback(
       (event: KeyboardEvent) => {
-        if (focusElement.current === null) return;
-        if (!mayReceiveKeyEvents(focusElement.current)) return;
-
+        if (!mayReceiveKeyEvents() || !mayReceiveSpaceKeyEvents()) return;
         if (event.key === " ") {
           spacebarHeld.current = false;
           setAudioEnabled?.(false);
         }
       },
-      [focusElement, setAudioEnabled],
+      [setAudioEnabled],
     ),
   );