Skip to content

Commit 0584d76

Browse files
committed
fix(composer): serialize sends and snapshot drafts across the composer
1 parent 84c5f57 commit 0584d76

15 files changed

Lines changed: 2769 additions & 520 deletions

src/app/components/upload-board/UploadBoard.tsx

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { MutableRefObject, ReactNode } from 'react';
2-
import { useEffect, useImperativeHandle, useRef } from 'react';
2+
import { useEffect, useImperativeHandle } from 'react';
33
import { Badge, Box, Chip, Header, Spinner, Text, as, percent } from 'folds';
44
import { CaretRight, CaretUp, X, sizedIcon } from '$components/icons/phosphor';
55
import classNames from 'classnames';
@@ -27,14 +27,15 @@ export const UploadBoard = as<'div', UploadBoardProps>(({ header, children, ...p
2727
</Box>
2828
));
2929

30-
export type UploadBoardImperativeHandlers = { handleSend: () => Promise<void> };
30+
// Progress ticks re-render this header, so the caller reads uploads on demand here
31+
// instead of subscribing to them itself.
32+
export type UploadBoardImperativeHandlers = { getSendableUploads: () => Upload[] };
3133

3234
type UploadBoardHeaderProps = {
3335
open: boolean;
3436
onToggle: () => void;
3537
uploadFamilyObserverAtom: TUploadFamilyObserverAtom;
3638
onCancel: (uploads: Upload[]) => void;
37-
onSend: (uploads: Upload[]) => Promise<void>;
3839
onBusyChange?: (busy: boolean) => void;
3940
imperativeHandlerRef: MutableRefObject<UploadBoardImperativeHandlers | undefined>;
4041
};
@@ -44,11 +45,9 @@ export function UploadBoardHeader({
4445
onToggle,
4546
uploadFamilyObserverAtom,
4647
onCancel,
47-
onSend,
4848
onBusyChange,
4949
imperativeHandlerRef,
5050
}: UploadBoardHeaderProps) {
51-
const sendingRef = useRef(false);
5251
const uploads = useAtomValue(uploadFamilyObserverAtom);
5352

5453
const isSuccess = uploads.every((upload) => upload.status === UploadStatus.Success);
@@ -72,23 +71,11 @@ export function UploadBoardHeader({
7271
{ loaded: 0, total: 0 }
7372
);
7473

75-
const handleSend = async () => {
76-
if (sendingRef.current) return;
77-
sendingRef.current = true;
78-
try {
79-
await onSend(
80-
uploads.filter(
81-
(upload) =>
82-
upload.status === UploadStatus.Success || upload.status === UploadStatus.Loading
83-
)
84-
);
85-
} finally {
86-
sendingRef.current = false;
87-
}
88-
};
89-
9074
useImperativeHandle(imperativeHandlerRef, () => ({
91-
handleSend,
75+
getSendableUploads: () =>
76+
uploads.filter(
77+
(upload) => upload.status === UploadStatus.Success || upload.status === UploadStatus.Loading
78+
),
9279
}));
9380
const handleCancel = () => onCancel(uploads);
9481

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/* oxlint-disable typescript/no-explicit-any, vitest/require-mock-type-parameters */
2+
3+
import { act, render } from '@testing-library/react';
4+
import { createRef } from 'react';
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
6+
import { AudioMessageRecorder, type AudioMessageRecorderHandle } from './AudioMessageRecorder';
7+
8+
const recorderState = vi.hoisted(() => ({
9+
handleStop: vi.fn(),
10+
handleDelete: vi.fn(),
11+
onStop: undefined as ((payload: any) => void) | undefined,
12+
onDelete: undefined as (() => void) | undefined,
13+
}));
14+
15+
vi.mock('$plugins/voice-recorder-kit', () => ({
16+
useVoiceRecorder: ({ onStop, onDelete }: any) => {
17+
recorderState.onStop = onStop;
18+
recorderState.onDelete = onDelete;
19+
return {
20+
levels: [],
21+
seconds: 0,
22+
error: undefined,
23+
handleStop: recorderState.handleStop,
24+
handleDelete: recorderState.handleDelete,
25+
};
26+
},
27+
}));
28+
vi.mock('$hooks/useElementSizeObserver', () => ({ useElementSizeObserver: () => {} }));
29+
vi.mock('folds', () => ({
30+
Box: ({ children }: any) => <div>{children}</div>,
31+
Text: ({ children }: any) => <span>{children}</span>,
32+
}));
33+
34+
const payload = {
35+
audioFile: new Blob(['audio'], { type: 'audio/ogg' }),
36+
waveform: [0.5],
37+
audioLength: 1,
38+
audioCodec: 'audio/ogg',
39+
};
40+
41+
function renderRecorder() {
42+
const ref = createRef<AudioMessageRecorderHandle>();
43+
const onRecordingComplete = vi.fn();
44+
const result = render(
45+
<AudioMessageRecorder
46+
ref={ref}
47+
onRecordingComplete={onRecordingComplete}
48+
onRequestClose={vi.fn()}
49+
onWaveformUpdate={vi.fn()}
50+
onAudioLengthUpdate={vi.fn()}
51+
/>
52+
);
53+
return { ref, onRecordingComplete, result };
54+
}
55+
56+
beforeEach(() => {
57+
vi.useFakeTimers();
58+
recorderState.handleStop.mockReset();
59+
recorderState.handleDelete.mockReset();
60+
recorderState.onStop = undefined;
61+
recorderState.onDelete = undefined;
62+
});
63+
64+
afterEach(() => {
65+
vi.useRealTimers();
66+
});
67+
68+
describe('AudioMessageRecorder lifecycle', () => {
69+
it('makes stop idempotent and surfaces one completion', () => {
70+
const { ref, onRecordingComplete } = renderRecorder();
71+
72+
act(() => {
73+
ref.current?.stop();
74+
ref.current?.stop();
75+
recorderState.onStop?.(payload);
76+
recorderState.onStop?.(payload);
77+
});
78+
79+
expect(recorderState.handleStop).toHaveBeenCalledOnce();
80+
expect(onRecordingComplete).toHaveBeenCalledOnce();
81+
});
82+
83+
it('makes delayed cancel idempotent and cancels its timer on unmount', () => {
84+
const { ref, result } = renderRecorder();
85+
86+
act(() => {
87+
ref.current?.cancel();
88+
ref.current?.cancel();
89+
vi.advanceTimersByTime(180);
90+
});
91+
expect(recorderState.handleDelete).toHaveBeenCalledOnce();
92+
93+
result.unmount();
94+
act(() => vi.runOnlyPendingTimers());
95+
expect(recorderState.handleDelete).toHaveBeenCalledOnce();
96+
});
97+
98+
it('does not delete after cancel is followed by unmount before the delay', () => {
99+
const { ref, result } = renderRecorder();
100+
101+
act(() => ref.current?.cancel());
102+
result.unmount();
103+
act(() => vi.advanceTimersByTime(180));
104+
105+
expect(recorderState.handleDelete).not.toHaveBeenCalled();
106+
});
107+
});

src/app/features/room/AudioMessageRecorder.tsx

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ export const AudioMessageRecorder = forwardRef<
4848
AudioMessageRecorderHandle,
4949
AudioMessageRecorderProps
5050
>(({ onRecordingComplete, onRequestClose, onWaveformUpdate, onAudioLengthUpdate }, ref) => {
51-
const isDismissedRef = useRef(false);
5251
const userRequestedStopRef = useRef(false);
52+
const actionRef = useRef<'active' | 'stopping' | 'canceling' | 'dismissed'>('active');
53+
const cancelTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
5354
const containerRef = useRef<HTMLDivElement>(null);
5455
const [isCanceling, setIsCanceling] = useState(false);
5556
const [announcedTime, setAnnouncedTime] = useState(0);
@@ -67,8 +68,8 @@ export const AudioMessageRecorder = forwardRef<
6768
const stableOnStop = useCallback((payload: VoiceRecorderStopPayload) => {
6869
// useVoiceRecorder also stops during cancel/teardown paths, so only surface a completed
6970
// recording after an explicit user stop.
70-
if (!userRequestedStopRef.current) return;
71-
if (isDismissedRef.current) return;
71+
if (!userRequestedStopRef.current || actionRef.current !== 'stopping') return;
72+
actionRef.current = 'dismissed';
7273
onRecordingCompleteRef.current({
7374
audioBlob: payload.audioFile,
7475
waveform: payload.waveform,
@@ -80,7 +81,11 @@ export const AudioMessageRecorder = forwardRef<
8081
}, []);
8182

8283
const stableOnDelete = useCallback(() => {
83-
isDismissedRef.current = true;
84+
if (cancelTimerRef.current !== null) {
85+
clearTimeout(cancelTimerRef.current);
86+
cancelTimerRef.current = null;
87+
}
88+
actionRef.current = 'dismissed';
8489
onRequestCloseRef.current();
8590
}, []);
8691

@@ -91,22 +96,35 @@ export const AudioMessageRecorder = forwardRef<
9196
});
9297

9398
const doStop = useCallback(() => {
94-
if (isDismissedRef.current) return;
99+
if (actionRef.current !== 'active') return;
100+
actionRef.current = 'stopping';
95101
userRequestedStopRef.current = true;
96102
handleStop();
97103
}, [handleStop]);
98104

99105
const doCancel = useCallback(() => {
100-
if (isDismissedRef.current) return;
106+
if (actionRef.current !== 'active') return;
107+
actionRef.current = 'canceling';
101108
setIsCanceling(true);
102-
setTimeout(() => {
103-
isDismissedRef.current = true;
109+
cancelTimerRef.current = setTimeout(() => {
110+
cancelTimerRef.current = null;
111+
if (actionRef.current !== 'canceling') return;
112+
actionRef.current = 'dismissed';
104113
handleDelete();
105114
}, 180);
106115
}, [handleDelete]);
107116

108117
useImperativeHandle(ref, () => ({ stop: doStop, cancel: doCancel }), [doStop, doCancel]);
109118

119+
useEffect(
120+
() => () => {
121+
if (cancelTimerRef.current !== null) clearTimeout(cancelTimerRef.current);
122+
cancelTimerRef.current = null;
123+
actionRef.current = 'dismissed';
124+
},
125+
[]
126+
);
127+
110128
useEffect(() => {
111129
if (seconds > 0 && seconds % 30 === 0 && seconds !== announcedTime) {
112130
setAnnouncedTime(seconds);

0 commit comments

Comments
 (0)