Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent cross-entry recording interference #3157

Merged
merged 12 commits into from
Jul 8, 2024
23 changes: 11 additions & 12 deletions src/components/Pronunciations/AudioRecorder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import { ReactElement, useContext } from "react";
import { useTranslation } from "react-i18next";
import { toast } from "react-toastify";

import Recorder from "components/Pronunciations/Recorder";
import RecorderContext from "components/Pronunciations/RecorderContext";
import RecorderIcon from "components/Pronunciations/RecorderIcon";
import { getFileNameForWord } from "components/Pronunciations/utilities";
import { useAppSelector } from "rootRedux/hooks";
import { type StoreState } from "rootRedux/types";
import { FileWithSpeakerId } from "types/word";
Expand All @@ -26,31 +24,32 @@ export default function AudioRecorder(props: RecorderProps): ReactElement {
const { t } = useTranslation();

async function startRecording(): Promise<void> {
const recordingId = recorder.getRecordingId();
if (recordingId && recordingId !== props.id) {
// Prevent interfering with an active recording on a different entry.
return;
}

// Prevent starting a recording before a previous one is finished.
await stopRecording();

recorder.startRecording();
recorder.startRecording(props.id);
}

async function stopRecording(): Promise<void> {
async function stopRecording(): Promise<string | undefined> {
// Prevent triggering this function if no recording is active.
if (!recorder.isRecording()) {
if (recorder.getRecordingId() === undefined) {
return;
}

if (props.onClick) {
props.onClick();
}
const blob = await recorder.stopRecording();
if (!blob) {
const file = await recorder.stopRecording();
if (!file) {
toast.error(t("pronunciations.noMicAccess"));
return;
}
const fileName = getFileNameForWord(props.id);
const file = new File([blob], fileName, {
lastModified: Date.now(),
type: Recorder.blobType,
});
if (!props.noSpeaker) {
(file as FileWithSpeakerId).speakerId = speakerId;
}
Expand Down
30 changes: 24 additions & 6 deletions src/components/Pronunciations/Recorder.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import RecordRTC from "recordrtc";

import { getFileNameForWord } from "components/Pronunciations/utilities";

export default class Recorder {
private toast: (text: string) => void;
private recordRTC?: RecordRTC;
private id?: string;

static blobType: RecordRTC.Options["type"] = "audio";

Expand All @@ -14,21 +17,36 @@ export default class Recorder {
.catch((err) => this.onError(err));
}

isRecording(): boolean {
return this.recordRTC?.getState() === "recording";
/** Checks if the recorder state is `"recording"`.
* If so, returns the `id` used with `startRecording()`.
* If not, returns `undefined`. */
getRecordingId(): string | undefined {
return this.recordRTC?.getState() === "recording"
? this.id ?? ""
: undefined;
}

startRecording(): void {
startRecording(id: string): void {
this.recordRTC?.reset();
this.id = id;
this.recordRTC?.startRecording();
}

stopRecording(): Promise<Blob | undefined> {
return new Promise<Blob | undefined>((resolve) => {
stopRecording(): Promise<File | undefined> {
return new Promise<File | undefined>((resolve) => {
const rec = this.recordRTC;
if (rec) {
rec.stopRecording(() => resolve(rec.getBlob()));
rec.stopRecording(() => {
const fileName = getFileNameForWord(this.id ?? "");
const file = new File([rec.getBlob()], fileName, {
lastModified: Date.now(),
type: Recorder.blobType,
});
this.id = undefined;
resolve(file);
});
} else {
this.id = undefined;
resolve(undefined);
}
});
Expand Down
31 changes: 24 additions & 7 deletions src/components/Pronunciations/RecorderIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,37 @@ interface RecorderIconProps {
export default function RecorderIcon(props: RecorderIconProps): ReactElement {
const isRecording = useAppSelector(
(state: StoreState) =>
state.pronunciationsState.status === PronunciationsStatus.Recording &&
state.pronunciationsState.wordId === props.id
state.pronunciationsState.status === PronunciationsStatus.Recording
);
const recordingId = useAppSelector(
(state: StoreState) => state.pronunciationsState.wordId
);
const isRecordingThis = isRecording && recordingId === props.id;

const dispatch = useAppDispatch();
const { t } = useTranslation();

function toggleIsRecordingToTrue(): void {
dispatch(recording(props.id));
props.startRecording();
if (!isRecording) {
// Only start a recording if there's not another on in progress.
dispatch(recording(props.id));
props.startRecording();
} 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 {
props.stopRecording();
dispatch(resetPronunciations());
if (isRecordingThis) {
props.stopRecording();
dispatch(resetPronunciations());
}
}

function handleTouchStart(): void {
Expand Down Expand Up @@ -82,7 +99,7 @@ export default function RecorderIcon(props: RecorderIconProps): ReactElement {
color: (t) =>
props.disabled
? t.palette.grey[400]
: isRecording
: isRecordingThis
? themeColors.recordActive
: themeColors.recordIdle,
}}
Expand Down
53 changes: 4 additions & 49 deletions src/components/Pronunciations/tests/AudioRecorder.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ import configureMockStore from "redux-mock-store";

import { defaultState } from "components/App/DefaultState";
import AudioRecorder from "components/Pronunciations/AudioRecorder";
import RecorderIcon, {
recordButtonId,
recordIconId,
} from "components/Pronunciations/RecorderIcon";
import { recordIconId } from "components/Pronunciations/RecorderIcon";
import { PronunciationsStatus } from "components/Pronunciations/Redux/PronunciationsReduxTypes";
import { type StoreState } from "rootRedux/types";
import theme, { themeColors } from "types/theme";
Expand All @@ -27,50 +24,8 @@ function mockRecordingState(wordId: string): Partial<StoreState> {
};
}

beforeAll(() => {
act(() => {
testRenderer = create(
<StyledEngineProvider injectFirst>
<ThemeProvider theme={theme}>
<Provider store={mockStore}>
<AudioRecorder id="2" uploadAudio={jest.fn()} />
</Provider>
</ThemeProvider>
</StyledEngineProvider>
);
});
});

describe("Pronunciations", () => {
test("pointerDown and pointerUp", () => {
const mockStartRecording = jest.fn();
const mockStopRecording = jest.fn();
act(() => {
testRenderer = create(
<StyledEngineProvider injectFirst>
<ThemeProvider theme={theme}>
<Provider store={mockStore}>
<RecorderIcon
id={"mockId"}
startRecording={mockStartRecording}
stopRecording={mockStopRecording}
/>
</Provider>
</ThemeProvider>
</StyledEngineProvider>
);
});

expect(mockStartRecording).not.toHaveBeenCalled();
testRenderer.root.findByProps({ id: recordButtonId }).props.onPointerDown();
expect(mockStartRecording).toHaveBeenCalled();

expect(mockStopRecording).not.toHaveBeenCalled();
testRenderer.root.findByProps({ id: recordButtonId }).props.onPointerUp();
expect(mockStopRecording).toHaveBeenCalled();
});

test("default style is idle", () => {
describe("AudioRecorder", () => {
test("default icon style is idle", () => {
act(() => {
testRenderer = create(
<ThemeProvider theme={theme}>
Expand All @@ -86,7 +41,7 @@ describe("Pronunciations", () => {
expect(icon.props.sx.color({})).toEqual(themeColors.recordIdle);
});

test("style depends on pronunciations state", () => {
test("icon style depends on pronunciations state", () => {
const wordId = "1";
const mockStore2 = configureMockStore()(mockRecordingState(wordId));
act(() => {
Expand Down
99 changes: 99 additions & 0 deletions src/components/Pronunciations/tests/RecorderIcon.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { StyledEngineProvider, ThemeProvider } from "@mui/material/styles";
import { Provider } from "react-redux";
import {
ReactTestInstance,
ReactTestRenderer,
act,
create,
} from "react-test-renderer";
import configureMockStore from "redux-mock-store";

import { defaultState } from "components/App/DefaultState";
import RecorderIcon, {
recordButtonId,
} from "components/Pronunciations/RecorderIcon";
import { PronunciationsStatus } from "components/Pronunciations/Redux/PronunciationsReduxTypes";
import { type StoreState } from "rootRedux/types";
import theme from "types/theme";

let testRenderer: ReactTestRenderer;
let testButton: ReactTestInstance;

function mockRecordingState(wordId: string): Partial<StoreState> {
return {
...defaultState,
pronunciationsState: {
fileName: "",
status: PronunciationsStatus.Recording,
wordId,
},
};
}

const mockWordId = "1234567890";

const mockStartRecording = jest.fn();
const mockStopRecording = jest.fn();

const renderRecorderIcon = async (wordId = ""): Promise<void> => {
await act(async () => {
testRenderer = create(
<StyledEngineProvider injectFirst>
<ThemeProvider theme={theme}>
<Provider
store={configureMockStore()(
wordId ? mockRecordingState(wordId) : defaultState
)}
>
<RecorderIcon
id={mockWordId}
startRecording={mockStartRecording}
stopRecording={mockStopRecording}
/>
</Provider>
</ThemeProvider>
</StyledEngineProvider>
);
});
testButton = testRenderer.root.findByProps({ id: recordButtonId });
};

beforeEach(() => {
jest.resetAllMocks();
});

describe("RecorderIcon", () => {
test("pointerDown records if no recording active", async () => {
await renderRecorderIcon();
expect(mockStartRecording).not.toHaveBeenCalled();
await act(async () => {
testButton.props.onPointerDown();
});
expect(mockStartRecording).toHaveBeenCalled();
});

test("pointerUp stops recording", async () => {
await renderRecorderIcon(mockWordId);
expect(mockStopRecording).not.toHaveBeenCalled();
await act(async () => {
testButton.props.onPointerUp();
});
expect(mockStopRecording).toHaveBeenCalled();
});

test("pointerUp does nothing if no recording active", async () => {
await renderRecorderIcon();
await act(async () => {
testButton.props.onPointerUp();
});
expect(mockStopRecording).not.toHaveBeenCalled();
});

test("pointerUp does nothing if different word id", async () => {
await renderRecorderIcon("different-id");
await act(async () => {
testButton.props.onPointerUp();
});
expect(mockStopRecording).not.toHaveBeenCalled();
});
});
Loading