From ac46cdf1c3aabb154bd99241a218fd9838f9ab8e Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 19 Apr 2024 13:25:07 -0400 Subject: [PATCH 1/7] Try to implement long-press --- src/components/Pronunciations/AudioPlayer.tsx | 37 ++++++++++++----- src/utilities/useLongPress.tsx | 40 +++++++++++++++++++ 2 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 src/utilities/useLongPress.tsx diff --git a/src/components/Pronunciations/AudioPlayer.tsx b/src/components/Pronunciations/AudioPlayer.tsx index 74a2ff3a9f..a86e7c25e4 100644 --- a/src/components/Pronunciations/AudioPlayer.tsx +++ b/src/components/Pronunciations/AudioPlayer.tsx @@ -31,6 +31,7 @@ import { import { PronunciationsStatus } from "components/Pronunciations/Redux/PronunciationsReduxTypes"; import { StoreState } from "types"; import { useAppDispatch, useAppSelector } from "types/hooks"; +import useLongPress from "utilities/useLongPress"; interface PlayerProps { audio: Pronunciation; @@ -125,15 +126,18 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { /** If audio can be deleted or speaker changed, a touchscreen press should open an * options menu instead of the context menu. */ - function handleTouch(e: TouchEvent): void { + function handleTouch(e?: TouchEvent): void { + console.info(e); if (canChangeSpeaker || canDeleteAudio) { // Temporarily disable context menu since some browsers // interpret a long-press touch as a right-click. disableContextMenu(); - setAnchor(e.currentTarget); + setAnchor(e?.currentTarget); } } + const { onTouchStart, onTouchEnd } = useLongPress(handleTouch); + async function handleOnSelect(speaker?: Speaker): Promise { if (canChangeSpeaker) { await props.updateAudioSpeaker!(speaker?.id); @@ -143,7 +147,8 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { /** If speaker can be changed, a right click should open the speaker menu instead of * the context menu. */ - function handleOnAuxClick(): void { + function handleOnAuxClick(e?: MouseEvent): void { + console.info(e); if (canChangeSpeaker) { disableContextMenu(); setSpeakerDialog(true); @@ -152,8 +157,10 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { /** Catch a multi-finger mousepad tap as a right click. */ function handleOnMouseDown(e: MouseEvent): void { + console.info(e); + //e.stopPropagation(); if (e.buttons > 1) { - handleOnAuxClick(); + handleOnAuxClick(e); } } @@ -202,8 +209,16 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { onAuxClick={handleOnAuxClick} onClick={deleteOrTogglePlay} onMouseDown={handleOnMouseDown} - onTouchStart={handleTouch} - onTouchEnd={enableContextMenu} + onTouchStart={ + handleTouch /*(e) => { + console.info(e); + onTouchStart(e); + }*/ + } + onTouchEnd={(e) => { + onTouchEnd(e); + enableContextMenu(); + }} aria-label="play" disabled={props.disabled} id={`audio-${props.audio.fileName}`} @@ -223,7 +238,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { > { + onMouseDown={() => { togglePlay(); handleMenuOnClose(); }} @@ -233,7 +248,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { {canChangeSpeaker && ( { + onMouseDown={() => { setSpeakerDialog(true); handleMenuOnClose(); }} @@ -244,10 +259,14 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { {canDeleteAudio && ( { + onTouchStart={(e: any) => { + console.warn(e); setDeleteConf(true); handleMenuOnClose(); }} + onMouseDown={(e: any) => { + console.warn(e); + }} > diff --git a/src/utilities/useLongPress.tsx b/src/utilities/useLongPress.tsx new file mode 100644 index 0000000000..dee83cfc21 --- /dev/null +++ b/src/utilities/useLongPress.tsx @@ -0,0 +1,40 @@ +import { + type TouchEvent, + type TouchEventHandler, + useCallback, + useEffect, + useState, +} from "react"; + +interface UseLongPressReturn { + onTouchEnd: TouchEventHandler; + onTouchStart: TouchEventHandler; +} + +// Inspired by https://stackoverflow.com/a/55880860/10210583 +export default function useLongPress( + onLongPress: (e: TouchEvent) => void, + delay = 1000 +): UseLongPressReturn { + const [event, setEvent] = useState | undefined>(); + + useEffect(() => { + const timerId = event + ? setTimeout(() => onLongPress(event), delay) + : undefined; + console.info(`timerId: ${timerId}`); + return () => { + clearTimeout(timerId); + }; + }, [delay, event, onLongPress]); + + const onTouchStart = useCallback((e: TouchEvent) => { + setEvent(e); + }, []); + + const onTouchEnd = useCallback(() => { + setEvent(undefined); + }, []); + + return { onTouchEnd, onTouchStart }; +} From c7af9d2130151883740c4d3ce1bebdd2f42e5e24 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 19 Apr 2024 15:16:30 -0400 Subject: [PATCH 2/7] Embed long-press to be able to use event target --- src/components/Pronunciations/AudioPlayer.tsx | 59 ++++++++++--------- src/utilities/useLongPress.tsx | 17 +++--- 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/src/components/Pronunciations/AudioPlayer.tsx b/src/components/Pronunciations/AudioPlayer.tsx index a86e7c25e4..30e7d5b199 100644 --- a/src/components/Pronunciations/AudioPlayer.tsx +++ b/src/components/Pronunciations/AudioPlayer.tsx @@ -31,7 +31,9 @@ import { import { PronunciationsStatus } from "components/Pronunciations/Redux/PronunciationsReduxTypes"; import { StoreState } from "types"; import { useAppDispatch, useAppSelector } from "types/hooks"; -import useLongPress from "utilities/useLongPress"; + +/** Number of ms for a touchscreen press to be considered a long-press. */ +const LongPressDelay = 500; interface PlayerProps { audio: Pronunciation; @@ -56,6 +58,9 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { ); const [anchor, setAnchor] = useState(); const [deleteConf, setDeleteConf] = useState(false); + const [longPressTarget, setLongPressTarget] = useState< + (EventTarget & HTMLButtonElement) | undefined + >(); const [speaker, setSpeaker] = useState(); const [speakerDialog, setSpeakerDialog] = useState(false); @@ -87,6 +92,17 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { } }, [audio, dispatchReset, isPlaying]); + // When pressed, set a timer for a long-press. + // https://stackoverflow.com/questions/48048957/add-a-long-press-event-in-react + useEffect(() => { + const timerId = longPressTarget + ? setTimeout(() => setAnchor(longPressTarget), LongPressDelay) + : undefined; + return () => { + clearTimeout(timerId); + }; + }, [longPressTarget]); + function togglePlay(): void { if (!isPlaying) { dispatch(playing(props.audio.fileName)); @@ -126,17 +142,20 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { /** If audio can be deleted or speaker changed, a touchscreen press should open an * options menu instead of the context menu. */ - function handleTouch(e?: TouchEvent): void { - console.info(e); + function handleTouchStart(e: TouchEvent): void { if (canChangeSpeaker || canDeleteAudio) { // Temporarily disable context menu since some browsers // interpret a long-press touch as a right-click. disableContextMenu(); - setAnchor(e?.currentTarget); + setLongPressTarget(e.currentTarget); } } - const { onTouchStart, onTouchEnd } = useLongPress(handleTouch); + /** When a touch ends, restore the context menu and cancel the long-press timer. */ + function handleTouchEnd(): void { + enableContextMenu(); + setLongPressTarget(undefined); + } async function handleOnSelect(speaker?: Speaker): Promise { if (canChangeSpeaker) { @@ -147,8 +166,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { /** If speaker can be changed, a right click should open the speaker menu instead of * the context menu. */ - function handleOnAuxClick(e?: MouseEvent): void { - console.info(e); + function handleOnAuxClick(): void { if (canChangeSpeaker) { disableContextMenu(); setSpeakerDialog(true); @@ -157,10 +175,8 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { /** Catch a multi-finger mousepad tap as a right click. */ function handleOnMouseDown(e: MouseEvent): void { - console.info(e); - //e.stopPropagation(); if (e.buttons > 1) { - handleOnAuxClick(e); + handleOnAuxClick(); } } @@ -201,6 +217,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { return ( <> } placement="top" > @@ -209,16 +226,8 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { onAuxClick={handleOnAuxClick} onClick={deleteOrTogglePlay} onMouseDown={handleOnMouseDown} - onTouchStart={ - handleTouch /*(e) => { - console.info(e); - onTouchStart(e); - }*/ - } - onTouchEnd={(e) => { - onTouchEnd(e); - enableContextMenu(); - }} + onTouchStart={handleTouchStart} + onTouchEnd={handleTouchEnd} aria-label="play" disabled={props.disabled} id={`audio-${props.audio.fileName}`} @@ -238,7 +247,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { > { + onClick={() => { togglePlay(); handleMenuOnClose(); }} @@ -248,7 +257,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { {canChangeSpeaker && ( { + onClick={() => { setSpeakerDialog(true); handleMenuOnClose(); }} @@ -259,14 +268,10 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { {canDeleteAudio && ( { - console.warn(e); + onClick={() => { setDeleteConf(true); handleMenuOnClose(); }} - onMouseDown={(e: any) => { - console.warn(e); - }} > diff --git a/src/utilities/useLongPress.tsx b/src/utilities/useLongPress.tsx index dee83cfc21..cbe19e28a4 100644 --- a/src/utilities/useLongPress.tsx +++ b/src/utilities/useLongPress.tsx @@ -16,24 +16,23 @@ export default function useLongPress( onLongPress: (e: TouchEvent) => void, delay = 1000 ): UseLongPressReturn { - const [event, setEvent] = useState | undefined>(); + const [touchEvent, setTouchEvent] = useState | undefined>(); useEffect(() => { - const timerId = event - ? setTimeout(() => onLongPress(event), delay) + const timerId = touchEvent + ? setTimeout(() => onLongPress(touchEvent), delay) : undefined; - console.info(`timerId: ${timerId}`); return () => { clearTimeout(timerId); }; - }, [delay, event, onLongPress]); + }, [delay, onLongPress, touchEvent]); - const onTouchStart = useCallback((e: TouchEvent) => { - setEvent(e); + const onTouchEnd = useCallback(() => { + setTouchEvent(undefined); }, []); - const onTouchEnd = useCallback(() => { - setEvent(undefined); + const onTouchStart = useCallback((event: TouchEvent) => { + setTouchEvent(event); }, []); return { onTouchEnd, onTouchStart }; From 9f8fefb5cae7e9849f294c620d12aff765fb7139 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 19 Apr 2024 15:16:58 -0400 Subject: [PATCH 3/7] Remove unusable extracted long-press hook --- src/utilities/useLongPress.tsx | 39 ---------------------------------- 1 file changed, 39 deletions(-) delete mode 100644 src/utilities/useLongPress.tsx diff --git a/src/utilities/useLongPress.tsx b/src/utilities/useLongPress.tsx deleted file mode 100644 index cbe19e28a4..0000000000 --- a/src/utilities/useLongPress.tsx +++ /dev/null @@ -1,39 +0,0 @@ -import { - type TouchEvent, - type TouchEventHandler, - useCallback, - useEffect, - useState, -} from "react"; - -interface UseLongPressReturn { - onTouchEnd: TouchEventHandler; - onTouchStart: TouchEventHandler; -} - -// Inspired by https://stackoverflow.com/a/55880860/10210583 -export default function useLongPress( - onLongPress: (e: TouchEvent) => void, - delay = 1000 -): UseLongPressReturn { - const [touchEvent, setTouchEvent] = useState | undefined>(); - - useEffect(() => { - const timerId = touchEvent - ? setTimeout(() => onLongPress(touchEvent), delay) - : undefined; - return () => { - clearTimeout(timerId); - }; - }, [delay, onLongPress, touchEvent]); - - const onTouchEnd = useCallback(() => { - setTouchEvent(undefined); - }, []); - - const onTouchStart = useCallback((event: TouchEvent) => { - setTouchEvent(event); - }, []); - - return { onTouchEnd, onTouchStart }; -} From 88769903e5b6695a977d0198bed6a8d655405b4f Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Fri, 19 Apr 2024 16:15:40 -0400 Subject: [PATCH 4/7] Increase LongPressDelay --- src/components/Pronunciations/AudioPlayer.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/Pronunciations/AudioPlayer.tsx b/src/components/Pronunciations/AudioPlayer.tsx index 30e7d5b199..58305bab9a 100644 --- a/src/components/Pronunciations/AudioPlayer.tsx +++ b/src/components/Pronunciations/AudioPlayer.tsx @@ -32,8 +32,9 @@ import { PronunciationsStatus } from "components/Pronunciations/Redux/Pronunciat import { StoreState } from "types"; import { useAppDispatch, useAppSelector } from "types/hooks"; -/** Number of ms for a touchscreen press to be considered a long-press. */ -const LongPressDelay = 500; +/** Number of ms for a touchscreen press to be considered a long-press. + * 600 ms is too short: it can still register as a click. */ +const LongPressDelay = 700; interface PlayerProps { audio: Pronunciation; From 020f97e40391f8f163645fffeadc7f39a191467e Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 22 Apr 2024 14:02:18 -0400 Subject: [PATCH 5/7] [AudioPlayer] Add long-press unit tests --- src/components/Pronunciations/AudioPlayer.tsx | 11 +- .../Pronunciations/tests/AudioPlayer.test.tsx | 116 ++++++++++++++++++ 2 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 src/components/Pronunciations/tests/AudioPlayer.test.tsx diff --git a/src/components/Pronunciations/AudioPlayer.tsx b/src/components/Pronunciations/AudioPlayer.tsx index 58305bab9a..8c13def473 100644 --- a/src/components/Pronunciations/AudioPlayer.tsx +++ b/src/components/Pronunciations/AudioPlayer.tsx @@ -34,7 +34,10 @@ import { useAppDispatch, useAppSelector } from "types/hooks"; /** Number of ms for a touchscreen press to be considered a long-press. * 600 ms is too short: it can still register as a click. */ -const LongPressDelay = 700; +export const longPressDelay = 700; + +export const playButtonId = (fileName: string): string => `audio-${fileName}`; +export const playMenuId = "play-menu"; interface PlayerProps { audio: Pronunciation; @@ -97,7 +100,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { // https://stackoverflow.com/questions/48048957/add-a-long-press-event-in-react useEffect(() => { const timerId = longPressTarget - ? setTimeout(() => setAnchor(longPressTarget), LongPressDelay) + ? setTimeout(() => setAnchor(longPressTarget), longPressDelay) : undefined; return () => { clearTimeout(timerId); @@ -231,7 +234,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { onTouchEnd={handleTouchEnd} aria-label="play" disabled={props.disabled} - id={`audio-${props.audio.fileName}`} + id={playButtonId(props.audio.fileName)} size={props.size || "large"} > {icon} @@ -239,7 +242,7 @@ export default function AudioPlayer(props: PlayerProps): ReactElement { { + return { + ...jest.requireActual("@mui/material"), + Menu: (props: any) =>
, + }; +}); + +jest.mock("backend", () => ({ + getSpeaker: () => mockGetSpeaker(), +})); +jest.mock("types/hooks", () => { + return { + ...jest.requireActual("types/hooks"), + useAppDispatch: () => mockDispatch, + }; +}); + +const mockCanDeleteAudio = jest.fn(); +const mockDispatch = jest.fn((action: any) => action); +const mockGetSpeaker = jest.fn(); + +let testRenderer: ReactTestRenderer; + +const mockFileName = "speech.mp3"; +const mockId = playButtonId(mockFileName); +const mockPronunciation = newPronunciation(mockFileName); +const mockStore = configureMockStore()(mockPlayingState()); +const mockTouchEvent: Partial> = { + currentTarget: {} as HTMLButtonElement, +}; + +function mockPlayingState(fileName = ""): Partial { + return { + ...defaultState, + pronunciationsState: { + fileName, + status: PronunciationsStatus.Inactive, + wordId: "", + }, + }; +} + +function renderAudioPlayer(canDelete = false): void { + act(() => { + testRenderer = create( + + + + ); + }); +} + +beforeEach(() => { + jest.clearAllMocks(); + jest.clearAllTimers(); +}); + +describe("Pronunciations", () => { + it("dispatches on play", () => { + renderAudioPlayer(); + expect(mockDispatch).not.toHaveBeenCalled(); + const playButton = testRenderer.root.findByProps({ id: mockId }); + act(() => { + playButton.props.onClick(); + }); + expect(mockDispatch).toHaveBeenCalledTimes(1); + }); + + it("opens the menu on long-press", () => { + // Provide deleteAudio prop so that menu is available + renderAudioPlayer(true); + + const playMenu = testRenderer.root.findByProps({ id: playMenuId }); + expect(playMenu.props.open).toBeFalsy(); + + // Use a mock timer to control the length of the press + jest.useFakeTimers(); + + const playButton = testRenderer.root.findByProps({ id: mockId }); + act(() => { + playButton.props.onTouchStart(mockTouchEvent); + }); + + // Advance the timer just shy of the long-press time + act(() => { + jest.advanceTimersByTime(longPressDelay - 1); + }); + expect(playMenu.props.open).toBeFalsy(); + + // Advance the timer just past the long-press time + act(() => { + jest.advanceTimersByTime(2); + }); + expect(playMenu.props.open).toBeTruthy(); + expect(mockDispatch).not.toHaveBeenCalled(); + }); +}); From 97e90e2307b6a84cb27f8bd73640138a13557fc4 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 22 Apr 2024 14:19:55 -0400 Subject: [PATCH 6/7] [AudioPlayer] Add press-not-long-enough test --- .../Pronunciations/tests/AudioPlayer.test.tsx | 47 +++++++++++++++++-- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/src/components/Pronunciations/tests/AudioPlayer.test.tsx b/src/components/Pronunciations/tests/AudioPlayer.test.tsx index 92245a1b1f..6af228d0db 100644 --- a/src/components/Pronunciations/tests/AudioPlayer.test.tsx +++ b/src/components/Pronunciations/tests/AudioPlayer.test.tsx @@ -89,18 +89,18 @@ describe("Pronunciations", () => { // Provide deleteAudio prop so that menu is available renderAudioPlayer(true); - const playMenu = testRenderer.root.findByProps({ id: playMenuId }); - expect(playMenu.props.open).toBeFalsy(); - // Use a mock timer to control the length of the press jest.useFakeTimers(); const playButton = testRenderer.root.findByProps({ id: mockId }); + const playMenu = testRenderer.root.findByProps({ id: playMenuId }); + + // Start a press and advance the timer just shy of the long-press time + expect(playMenu.props.open).toBeFalsy(); act(() => { playButton.props.onTouchStart(mockTouchEvent); }); - - // Advance the timer just shy of the long-press time + expect(playMenu.props.open).toBeFalsy(); act(() => { jest.advanceTimersByTime(longPressDelay - 1); }); @@ -111,6 +111,43 @@ describe("Pronunciations", () => { jest.advanceTimersByTime(2); }); expect(playMenu.props.open).toBeTruthy(); + + // Make sure the menu stays open and no play is dispatched + act(() => { + playButton.props.onTouchEnd(); + jest.runAllTimers(); + }); + expect(playMenu.props.open).toBeTruthy(); expect(mockDispatch).not.toHaveBeenCalled(); }); + + it("doesn't open the menu on short-press", () => { + // Provide deleteAudio prop so that menu is available + renderAudioPlayer(true); + + // Use a mock timer to control the length of the press + jest.useFakeTimers(); + + const playButton = testRenderer.root.findByProps({ id: mockId }); + const playMenu = testRenderer.root.findByProps({ id: playMenuId }); + + // Press the button and advance the timer, but end press before the long-press time + expect(playMenu.props.open).toBeFalsy(); + act(() => { + playButton.props.onTouchStart(mockTouchEvent); + }); + expect(playMenu.props.open).toBeFalsy(); + act(() => { + jest.advanceTimersByTime(longPressDelay - 1); + }); + expect(playMenu.props.open).toBeFalsy(); + act(() => { + playButton.props.onTouchEnd(); + }); + expect(playMenu.props.open).toBeFalsy(); + act(() => { + jest.advanceTimersByTime(2); + }); + expect(playMenu.props.open).toBeFalsy(); + }); }); From 340ce036743038b9c0b0b38578bd0c8e70317b7a Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Wed, 24 Apr 2024 08:32:28 -0400 Subject: [PATCH 7/7] [RecorderIcon] Disable long-press tooltip --- src/components/Pronunciations/RecorderIcon.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/Pronunciations/RecorderIcon.tsx b/src/components/Pronunciations/RecorderIcon.tsx index 58526d3edb..f7ab26112e 100644 --- a/src/components/Pronunciations/RecorderIcon.tsx +++ b/src/components/Pronunciations/RecorderIcon.tsx @@ -59,7 +59,11 @@ export default function RecorderIcon(props: RecorderIconProps): ReactElement { } return ( - +