Improve recording button state management (#4180)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Danny Rorabaugh <imnasnainaec@gmail.com>

Author: Copilot

src/components/DataEntry/DataEntryTable/NewEntry/index.tsx

@@ -117,6 +117,10 @@ export default function NewEntry(props: NewEntryProps): ReactElement {
     [glossInput, vernInput]
   );
 
+  const focusGloss = useCallback((): void => {
+    focus(FocusTarget.Gloss);
+  }, [focus]);
+
   const resetState = useCallback((): void => {
     resetNewEntry();
     setSubmitting(false);
@@ -277,7 +281,7 @@ export default function NewEntry(props: NewEntryProps): ReactElement {
           <NoteButton
             buttonId={NewEntryId.ButtonNote}
             noteText={submitting ? "" : newNote}
-            onExited={() => focus(FocusTarget.Gloss)}
+            onExited={focusGloss}
             updateNote={setNewNote}
           />
         )}
@@ -289,7 +293,7 @@ export default function NewEntry(props: NewEntryProps): ReactElement {
           deleteAudio={delNewAudio}
           replaceAudio={repNewAudio}
           uploadAudio={addNewAudio}
-          onClick={() => focus(FocusTarget.Gloss)}
+          onClick={focusGloss}
         />
       </Grid2>
 

src/components/ProjectSettings/tests/ProjectName.test.tsx

@@ -1,15 +1,11 @@
 import "@testing-library/jest-dom";
 import { act, render, screen } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
+import { toast } from "react-toastify"; // mocked in setupTests.js
 
 import ProjectName from "components/ProjectSettings/ProjectName";
 import { randomProject } from "types/project";
 
-jest.mock("react-toastify", () => ({
-  toast: { error: () => mockToastError() },
-}));
-
-const mockToastError = jest.fn();
 const mockSetProject = jest.fn();
 
 const mockProject = randomProject();
@@ -31,7 +27,7 @@ describe("ProjectName", () => {
     mockSetProject.mockResolvedValueOnce({});
     await userEvent.click(saveButton);
     expect(mockSetProject).toHaveBeenCalledWith({ ...mockProject, name });
-    expect(mockToastError).not.toHaveBeenCalled();
+    expect(toast.error).not.toHaveBeenCalled();
   });
 
   it("toasts on error", async () => {
@@ -41,8 +37,8 @@ describe("ProjectName", () => {
     await userEvent.clear(textField);
     await userEvent.type(textField, "whatever");
     mockSetProject.mockRejectedValueOnce({});
-    expect(mockToastError).not.toHaveBeenCalled();
+    expect(toast.error).not.toHaveBeenCalled();
     await userEvent.click(saveButton);
-    expect(mockToastError).toHaveBeenCalledTimes(1);
+    expect(toast.error).toHaveBeenCalledTimes(1);
   });
 });

src/components/Pronunciations/AudioRecorder.tsx

@@ -1,4 +1,10 @@
-import { ReactElement, useContext, useEffect, useState } from "react";
+import {
+  ReactElement,
+  useCallback,
+  useContext,
+  useEffect,
+  useRef,
+} from "react";
 import { useTranslation } from "react-i18next";
 import { toast } from "react-toastify";
 
@@ -23,16 +29,57 @@ export default function AudioRecorder(props: RecorderProps): ReactElement {
     (state: StoreState) => state.currentProjectState.speaker?.id
   );
   const recorder = useContext(RecorderContext);
-  const [clicked, setClicked] = useState(false);
+  const clickedRef = useRef(false);
   const { t } = useTranslation();
 
   useEffect(() => {
     // Re-enable clicking when the word id has changed
-    setClicked(false);
+    clickedRef.current = false;
   }, [props.id]);
 
-  async function startRecording(): Promise<boolean> {
-    if (clicked) {
+  const uploadAudio = useCallback(
+    (file?: File): boolean => {
+      if (!file?.size) {
+        console.error("No audio file to upload");
+        toast.error(t("pronunciations.recordingError"));
+        return false;
+      }
+
+      try {
+        if (!props.noSpeaker) {
+          (file as FileWithSpeakerId).speakerId = speakerId;
+        }
+        props.uploadAudio(file);
+        return true;
+      } catch (err) {
+        console.error("Error uploading audio:", err);
+        toast.error(t("pronunciations.recordingError"));
+        return false;
+      }
+    },
+    [props.noSpeaker, props.uploadAudio, speakerId, t]
+  );
+
+  const stopRecording = useCallback(async (): Promise<void> => {
+    // Prevent triggering this function if no recording is active.
+    if (recorder.getRecordingId() === undefined) {
+      return;
+    }
+
+    props.onClick?.();
+
+    const uploadSuccess = uploadAudio(await recorder.stopRecording());
+
+    if (uploadSuccess && props.id) {
+      // The id will change when the frontend updates with the new audio,
+      // so we rely on a useEffect above to reset the clickedRef.
+    } else {
+      clickedRef.current = false;
+    }
+  }, [props.id, props.onClick, recorder, uploadAudio]);
+
+  const startRecording = useCallback(async (): Promise<boolean> => {
+    if (clickedRef.current) {
       // Prevent recording again before this word has updated.
       return false;
     }
@@ -43,48 +90,33 @@ export default function AudioRecorder(props: RecorderProps): ReactElement {
       return false;
     }
 
-    setClicked(true);
+    clickedRef.current = true;
 
-    // Prevent starting a recording before a previous one is finished.
-    await stopRecording();
+    try {
+      // Prevent starting a recording before a previous one is finished.
+      await stopRecording();
 
-    if (!recorder.startRecording(props.id)) {
-      let errorMessage = t("pronunciations.recordingError");
-      if (isBrowserFirefox()) {
-        errorMessage += ` ${t("pronunciations.recordingPermission")}`;
+      if (!recorder.startRecording(props.id)) {
+        let errorMessage = t("pronunciations.recordingError");
+        if (isBrowserFirefox()) {
+          errorMessage += ` ${t("pronunciations.recordingPermission")}`;
+        }
+        toast.error(errorMessage);
+        clickedRef.current = false;
+        return false;
       }
-      toast.error(errorMessage);
+    } catch (err) {
+      console.error("Error starting recording:", err);
+      toast.error(t("pronunciations.recordingError"));
+      clickedRef.current = false;
       return false;
     }
-    return true;
-  }
-
-  async function stopRecording(): Promise<void> {
-    // Prevent triggering this function if no recording is active.
-    if (recorder.getRecordingId() === undefined) {
-      return;
-    }
 
-    if (props.onClick) {
-      props.onClick();
-    }
-    const file = await recorder.stopRecording();
-    if (!file || !file.size) {
-      toast.error(t("pronunciations.recordingError"));
-      setClicked(false);
-      return;
-    }
-    if (!props.noSpeaker) {
-      (file as FileWithSpeakerId).speakerId = speakerId;
-    }
-    props.uploadAudio(file);
-    if (!props.id) {
-      // If recorder is on something with an id,
-      // that id will update after the upload is complete,
-      // so rely on the useEffect above to do this.
-      setClicked(false);
-    }
-  }
+    // Don't set clickedRef to false here;
+    // It will be reset when the id changes after a successful upload,
+    // or in the catch block if there's an error.
+    return true;
+  }, [props.id, recorder, stopRecording, t]);
 
   return (
     <RecorderIcon

src/components/Pronunciations/RecorderContext.ts

@@ -1,11 +1,13 @@
 import { enqueueSnackbar } from "notistack";
 import { createContext } from "react";
+import { getI18n } from "react-i18next";
 
 import Recorder from "components/Pronunciations/Recorder";
-import i18n from "i18n";
 
 const RecorderContext = createContext(
-  new Recorder((textId: string) => enqueueSnackbar(i18n.t(textId)))
+  new Recorder((textId: string) =>
+    enqueueSnackbar(getI18n()?.t(textId) ?? textId)
+  )
 );
 
 export default RecorderContext;

src/components/Pronunciations/RecorderIcon.tsx

@@ -1,6 +1,6 @@
 import { FiberManualRecord } from "@mui/icons-material";
 import { IconButton, Tooltip } from "@mui/material";
-import { ReactElement, useEffect, useState } from "react";
+import { ReactElement, useEffect, useRef, useState } from "react";
 import { useTranslation } from "react-i18next";
 
 import {
@@ -30,56 +30,46 @@ export default function RecorderIcon(props: RecorderIconProps): ReactElement {
   const recordingId = useAppSelector(
     (state: StoreState) => state.pronunciationsState.wordId
   );
-  const isRecordingThis = isRecording && recordingId === props.id;
 
   const dispatch = useAppDispatch();
   const [hasMic, setHasMic] = useState(false);
   const { t } = useTranslation();
 
-  useEffect(() => {
-    checkMicPermission().then(setHasMic);
-  }, []);
+  // Use refs to for the useEffect cleanup.
+  const stopRef = useRef<(() => void) | undefined>();
 
-  async function toggleIsRecordingToTrue(): Promise<void> {
-    if (!isRecording) {
-      // Only start a recording if there's not another on in progress.
-      if (await props.startRecording()) {
-        dispatch(recording(props.id));
-      }
-    } else {
-      // This triggers if user clicks-and-holds on one entry's record icon,
-      // drags the mouse outside that icon before releasing,
-      // then clicks-and-holds a different entry's record icon.
-      if (recordingId !== props.id) {
-        console.error(
-          "Tried to record for an entry before finishing a recording on another entry."
-        );
-      }
-    }
-  }
-  function toggleIsRecordingToFalse(): void {
+  const isRecordingThis = isRecording && recordingId === props.id;
+  const disabled =
+    props.disabled || !hasMic || (isRecording && !isRecordingThis);
+
+  const stop = (): void => {
     if (isRecordingThis) {
       props.stopRecording();
       dispatch(resetPronunciations());
     }
-  }
+  };
+  stopRef.current = stop;
 
-  function handleTouchStart(): void {
-    // Temporarily disable context menu since some browsers
-    // interpret a long-press touch as a right-click.
-    document.addEventListener("contextmenu", disableContextMenu, false);
-  }
-  function handleTouchEnd(): void {
-    enableContextMenu();
-  }
+  useEffect(() => {
+    checkMicPermission().then(setHasMic);
+
+    return () => {
+      // Reset recording state if this component unmounts while recording
+      // (e.g., navigating away from the page mid-recording).
+      stopRef.current?.();
+    };
+  }, []);
 
-  function disableContextMenu(event: any): void {
-    event.preventDefault();
-    enableContextMenu();
-  }
-  function enableContextMenu(): void {
-    document.removeEventListener("contextmenu", disableContextMenu, false);
-  }
+  const start = async (): Promise<void> => {
+    if (isRecording) {
+      return;
+    }
+
+    // Only start a recording if there's not another one in progress.
+    if (await props.startRecording()) {
+      dispatch(recording(props.id));
+    }
+  };
 
   const tooltipId = hasMic
     ? "pronunciations.recordTooltip"
@@ -89,26 +79,26 @@ export default function RecorderIcon(props: RecorderIconProps): ReactElement {
     <Tooltip
       disableTouchListener // Distracting when already recording with a long-press.
       placement="top"
-      title={!props.disabled && t(tooltipId)}
+      title={disabled ? undefined : t(tooltipId)}
     >
       <span>
         <IconButton
           aria-label="record"
           data-testid={recordButtonId}
-          disabled={props.disabled || !hasMic}
+          disabled={disabled}
           id={recordButtonId}
-          onBlur={toggleIsRecordingToFalse}
-          onPointerDown={toggleIsRecordingToTrue}
-          onPointerUp={toggleIsRecordingToFalse}
-          onTouchEnd={handleTouchEnd}
-          onTouchStart={handleTouchStart}
+          onBlur={stop}
+          onContextMenu={(e) => e.preventDefault()}
+          onPointerCancel={stop}
+          onPointerDown={start}
+          onPointerUp={stop}
           size="large"
           tabIndex={-1}
         >
           <FiberManualRecord
             sx={{
               color: (t) =>
-                props.disabled || !hasMic
+                disabled
                   ? t.palette.grey[400]
                   : isRecordingThis
                     ? themeColors.recordActive