Skip to content

Commit

Permalink
fix: press space to unmute (#18544)
Browse files Browse the repository at this point in the history
* fix: increase action delay for notification rendering

* fix(useKeyPress): don't call on release util the on press was called
  • Loading branch information
olafsulich authored Dec 20, 2024
1 parent ac5d126 commit 0ff79f1
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/script/components/AppNotification/AppNotification.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
25 changes: 20 additions & 5 deletions src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}));

Expand Down
14 changes: 11 additions & 3 deletions src/script/hooks/useKeyPressAndHold/useKeyPressAndHold.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -52,8 +53,9 @@ export const useKeyPressAndHold = ({
return;
}

onHold();
const called = onHold();
hasTriggeredRef.current = true;
isPressHandledRef.current = called;
}, holdDelayMs);
}, [onHold, holdDelayMs]);

Expand All @@ -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(
Expand Down

0 comments on commit 0ff79f1

Please sign in to comment.