From 0ff79f1794304362189cbbe1d7a759af08ebc1e0 Mon Sep 17 00:00:00 2001
From: Olaf Sulich <olafsulich@gmail.com>
Date: Fri, 20 Dec 2024 10:25:27 +0100
Subject: [PATCH] fix: press space to unmute (#18544)

* fix: increase action delay for notification rendering

* fix(useKeyPress): don't call on release util the on press was called
---
 .../AppNotification/AppNotification.tsx       |  2 +-
 .../usePressSpaceToUnmute.ts                  |  5 +++-
 .../useKeyPressAndHold.test.ts                | 25 +++++++++++++++----
 .../useKeyPressAndHold/useKeyPressAndHold.ts  | 14 ++++++++---
 4 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/src/script/components/AppNotification/AppNotification.tsx b/src/script/components/AppNotification/AppNotification.tsx
index 7cc2d74cdd9..54e37e21ad4 100644
--- a/src/script/components/AppNotification/AppNotification.tsx
+++ b/src/script/components/AppNotification/AppNotification.tsx
@@ -52,7 +52,7 @@ const APP_NOTIFICATION_SELECTOR = '#app-notification';
 // Small delay to ensure rendering is complete.
 // In some cases (switching between windows) the notification is not displayed without this delay.
 // It's caused by the rendering behavior of the toast library (injecting the <Toaster/> into the DOM node).
-const ACTION_DELAY_MS = 1;
+const ACTION_DELAY_MS = 100;
 
 // Stores React roots for different windows (activeWindow).
 // Each window (identified by its 'name' property or 'default' if not available) gets its own root.
diff --git a/src/script/components/calling/CallingCell/usePressSpaceToUnmute/usePressSpaceToUnmute.ts b/src/script/components/calling/CallingCell/usePressSpaceToUnmute/usePressSpaceToUnmute.ts
index 189ada7574a..010806ca3f8 100644
--- a/src/script/components/calling/CallingCell/usePressSpaceToUnmute/usePressSpaceToUnmute.ts
+++ b/src/script/components/calling/CallingCell/usePressSpaceToUnmute/usePressSpaceToUnmute.ts
@@ -61,11 +61,14 @@ export const usePressSpaceToUnmute = ({callState, toggleMute, isMuted, enabled}:
     holdDelayMs: HOLD_DELAY_MS,
     onHold: () => {
       if (!isMuted()) {
-        return;
+        return false;
       }
+
       toggleMute(false);
       micOnNotification.show();
       amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.CALLING.PRESS_SPACE_TO_UNMUTE);
+
+      return true;
     },
     onRelease: () => {
       toggleMute(true);
diff --git a/src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.test.ts b/src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.test.ts
index 4f8bb1fc99b..02ff54347f5 100644
--- a/src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.test.ts
+++ b/src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.test.ts
@@ -26,7 +26,7 @@ import {useKeyPressAndHold} from './useKeyPressAndHold';
 jest.useFakeTimers();
 
 describe('useKeyPressAndHold', () => {
-  const mockOnHold = jest.fn();
+  const mockOnHold = jest.fn().mockReturnValue(true);
   const mockOnRelease = jest.fn();
   const defaultProps = {
     key: KEY.SPACE,
@@ -67,22 +67,37 @@ describe('useKeyPressAndHold', () => {
     expect(mockOnRelease).toHaveBeenCalledTimes(1);
   });
 
-  it("doesn't trigger onHold if key is released before holdDelay", () => {
+  it('calls onRelease only when onHold returns true', () => {
     renderHook(() => useKeyPressAndHold(defaultProps));
 
     act(() => {
-      window.dispatchEvent(new KeyboardEvent('keydown', {key: KEY.SPACE}));
+      window.dispatchEvent(new KeyboardEvent('keyup', {key: KEY.SPACE}));
     });
 
+    expect(mockOnRelease).not.toHaveBeenCalled();
+
     act(() => {
-      jest.advanceTimersByTime(100);
+      window.dispatchEvent(new KeyboardEvent('keydown', {key: KEY.SPACE}));
+      jest.advanceTimersByTime(200);
       window.dispatchEvent(new KeyboardEvent('keyup', {key: KEY.SPACE}));
     });
 
-    expect(mockOnHold).not.toHaveBeenCalled();
+    expect(mockOnHold).toHaveBeenCalledTimes(1);
     expect(mockOnRelease).toHaveBeenCalledTimes(1);
   });
 
+  it("doesn't call onRelease only when onHold returns false", () => {
+    renderHook(() => useKeyPressAndHold({...defaultProps, onHold: jest.fn().mockReturnValue(false)}));
+
+    act(() => {
+      window.dispatchEvent(new KeyboardEvent('keydown', {key: KEY.SPACE}));
+      jest.advanceTimersByTime(200);
+      window.dispatchEvent(new KeyboardEvent('keyup', {key: KEY.SPACE}));
+    });
+
+    expect(mockOnRelease).toHaveBeenCalledTimes(0);
+  });
+
   it("doesn't respond to key events when disabled", () => {
     renderHook(() => useKeyPressAndHold({...defaultProps, enabled: false}));
 
diff --git a/src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.ts b/src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.ts
index 24d225d1e4e..eba92b7d9fa 100644
--- a/src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.ts
+++ b/src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.ts
@@ -23,7 +23,7 @@ import {KEY} from 'Util/KeyboardUtil';
 
 interface KeyPressAndHoldParams {
   key: (typeof KEY)[keyof typeof KEY];
-  onHold: () => void;
+  onHold: () => boolean;
   onRelease: () => void;
   holdDelayMs: number;
   enabled?: boolean;
@@ -41,6 +41,7 @@ export const useKeyPressAndHold = ({
   const hasTriggeredRef = useRef(false);
   const holdTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
   const isKeyDownRef = useRef(false);
+  const isPressHandledRef = useRef(false);
 
   const handlePress = useCallback(() => {
     if (holdTimeoutRef.current) {
@@ -52,8 +53,9 @@ export const useKeyPressAndHold = ({
         return;
       }
 
-      onHold();
+      const called = onHold();
       hasTriggeredRef.current = true;
+      isPressHandledRef.current = called;
     }, holdDelayMs);
   }, [onHold, holdDelayMs]);
 
@@ -66,8 +68,14 @@ export const useKeyPressAndHold = ({
 
   const handleRelease = useCallback(() => {
     clearHoldTimeout();
-    onRelease();
     hasTriggeredRef.current = false;
+
+    if (!isPressHandledRef.current) {
+      return;
+    }
+
+    onRelease();
+    isPressHandledRef.current = false;
   }, [onRelease, clearHoldTimeout]);
 
   const handleKeyDown = useCallback(