Skip to content

Commit

Permalink
Handle duplicate requests to start recording a voice note
Browse files Browse the repository at this point in the history 
Co-authored-by: Scott Nonnenberg <scott@signal.org>
  • Loading branch information
@automated-signal @scottnonnenberg-signal
automated-signal and scottnonnenberg-signal committed Nov 12, 2021
1 parent a8eb371 commit 2ee5526
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 43 deletions.
9 changes: 7 additions & 2 deletions ts/components/CompositionArea.stories.tsx
View file
Expand Up @@ -5,7 +5,7 @@ import * as React from 'react';

import { storiesOf } from '@storybook/react';
import { action } from '@storybook/addon-actions';
import { boolean } from '@storybook/addon-knobs';
import { boolean, select } from '@storybook/addon-knobs';

import { IMAGE_JPEG } from '../types/MIME';
import type { Props } from './CompositionArea';
Expand All @@ -16,6 +16,7 @@ import enMessages from '../../_locales/en/messages.json';
import { fakeAttachment } from '../test-both/helpers/fakeAttachment';
import { landscapeGreenUrl } from '../storybook/Fixtures';
import { ThemeType } from '../types/Util';
import { RecordingState } from '../state/ducks/audioRecorder';

const i18n = setupI18n('en', enMessages);

Expand All @@ -42,7 +43,11 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
cancelRecording: action('cancelRecording'),
completeRecording: action('completeRecording'),
errorRecording: action('errorRecording'),
isRecording: Boolean(overrideProps.isRecording),
recordingState: select(
'recordingState',
RecordingState,
overrideProps.recordingState || RecordingState.Idle
),
startRecording: action('startRecording'),
// StagedLinkPreview
linkPreviewLoading: Boolean(overrideProps.linkPreviewLoading),
Expand Down
11 changes: 7 additions & 4 deletions ts/components/CompositionArea.tsx
View file
Expand Up @@ -11,7 +11,10 @@ import type {
LocalizerType,
ThemeType,
} from '../types/Util';
import type { ErrorDialogAudioRecorderType } from '../state/ducks/audioRecorder';
import type {
ErrorDialogAudioRecorderType,
RecordingState,
} from '../state/ducks/audioRecorder';
import type { HandleAttachmentsProcessingArgsType } from '../util/handleAttachmentsProcessing';
import { Spinner } from './Spinner';
import type { Props as EmojiButtonProps } from './emoji/EmojiButton';
Expand Down Expand Up @@ -90,7 +93,7 @@ export type OwnProps = Readonly<{
isFetchingUUID?: boolean;
isGroupV1AndDisabled?: boolean;
isMissingMandatoryProfileSharing?: boolean;
isRecording: boolean;
recordingState: RecordingState;
isSMSOnly?: boolean;
left?: boolean;
linkPreviewLoading: boolean;
Expand Down Expand Up @@ -176,7 +179,7 @@ export const CompositionArea = ({
completeRecording,
errorDialogAudioRecorderType,
errorRecording,
isRecording,
recordingState,
startRecording,
// StagedLinkPreview
linkPreviewLoading,
Expand Down Expand Up @@ -371,7 +374,7 @@ export const CompositionArea = ({
errorDialogAudioRecorderType={errorDialogAudioRecorderType}
errorRecording={errorRecording}
i18n={i18n}
isRecording={isRecording}
recordingState={recordingState}
onSendAudioRecording={(voiceNoteAttachment: AttachmentType) => {
onSendMessage({ voiceNoteAttachment });
}}
Expand Down
29 changes: 23 additions & 6 deletions ts/components/conversation/AudioCapture.stories.tsx
View file
Expand Up @@ -5,9 +5,12 @@ import * as React from 'react';

import { action } from '@storybook/addon-actions';
import { storiesOf } from '@storybook/react';
import { boolean } from '@storybook/addon-knobs';
import { select } from '@storybook/addon-knobs';

import { ErrorDialogAudioRecorderType } from '../../state/ducks/audioRecorder';
import {
ErrorDialogAudioRecorderType,
RecordingState,
} from '../../state/ducks/audioRecorder';
import type { PropsType } from './AudioCapture';
import { AudioCapture } from './AudioCapture';
import { setupI18n } from '../../util/setupI18n';
Expand All @@ -25,7 +28,11 @@ const createProps = (overrideProps: Partial<PropsType> = {}): PropsType => ({
errorDialogAudioRecorderType: overrideProps.errorDialogAudioRecorderType,
errorRecording: action('errorRecording'),
i18n,
isRecording: boolean('isRecording', overrideProps.isRecording || false),
recordingState: select(
'recordingState',
RecordingState,
overrideProps.recordingState || RecordingState.Idle
),
onSendAudioRecording: action('onSendAudioRecording'),
startRecording: action('startRecording'),
});
Expand All @@ -34,11 +41,21 @@ story.add('Default', () => {
return <AudioCapture {...createProps()} />;
});

story.add('Initializing', () => {
return (
<AudioCapture
{...createProps({
recordingState: RecordingState.Initializing,
})}
/>
);
});

story.add('Recording', () => {
return (
<AudioCapture
{...createProps({
isRecording: true,
recordingState: RecordingState.Recording,
})}
/>
);
Expand All @@ -49,7 +66,7 @@ story.add('Voice Limit', () => {
<AudioCapture
{...createProps({
errorDialogAudioRecorderType: ErrorDialogAudioRecorderType.Timeout,
isRecording: true,
recordingState: RecordingState.Recording,
})}
/>
);
Expand All @@ -60,7 +77,7 @@ story.add('Switched Apps', () => {
<AudioCapture
{...createProps({
errorDialogAudioRecorderType: ErrorDialogAudioRecorderType.Blur,
isRecording: true,
recordingState: RecordingState.Recording,
})}
/>
);
Expand Down
29 changes: 14 additions & 15 deletions ts/components/conversation/AudioCapture.tsx
View file
Expand Up @@ -8,7 +8,10 @@ import { noop } from 'lodash';
import type { AttachmentType } from '../../types/Attachment';
import { ConfirmationDialog } from '../ConfirmationDialog';
import type { LocalizerType } from '../../types/Util';
import { ErrorDialogAudioRecorderType } from '../../state/ducks/audioRecorder';
import {
ErrorDialogAudioRecorderType,
RecordingState,
} from '../../state/ducks/audioRecorder';
import { ToastVoiceNoteLimit } from '../ToastVoiceNoteLimit';
import { ToastVoiceNoteMustBeOnlyAttachment } from '../ToastVoiceNoteMustBeOnlyAttachment';
import { useEscapeHandling } from '../../hooks/useEscapeHandling';
Expand All @@ -30,7 +33,7 @@ export type PropsType = {
errorDialogAudioRecorderType?: ErrorDialogAudioRecorderType;
errorRecording: (e: ErrorDialogAudioRecorderType) => unknown;
i18n: LocalizerType;
isRecording: boolean;
recordingState: RecordingState;
onSendAudioRecording: OnSendAudioRecordingType;
startRecording: () => unknown;
};
Expand All @@ -50,7 +53,7 @@ export const AudioCapture = ({
errorDialogAudioRecorderType,
errorRecording,
i18n,
isRecording,
recordingState,
onSendAudioRecording,
startRecording,
}: PropsType): JSX.Element => {
Expand All @@ -59,18 +62,14 @@ export const AudioCapture = ({

// Cancel recording if we switch away from this conversation, unmounting
useEffect(() => {
if (!isRecording) {
return;
}

return () => {
cancelRecording();
};
}, [cancelRecording, isRecording]);
}, [cancelRecording]);

// Stop recording and show confirmation if user switches away from this app
useEffect(() => {
if (!isRecording) {
if (recordingState !== RecordingState.Recording) {
return;
}

Expand All @@ -82,15 +81,15 @@ export const AudioCapture = ({
return () => {
window.removeEventListener('blur', handler);
};
}, [isRecording, completeRecording, errorRecording]);
}, [recordingState, completeRecording, errorRecording]);

const escapeRecording = useCallback(() => {
if (!isRecording) {
if (recordingState !== RecordingState.Recording) {
return;
}

cancelRecording();
}, [cancelRecording, isRecording]);
}, [cancelRecording, recordingState]);

useEscapeHandling(escapeRecording);

Expand All @@ -103,7 +102,7 @@ export const AudioCapture = ({

// Update timestamp regularly, then timeout if recording goes over five minutes
useEffect(() => {
if (!isRecording) {
if (recordingState !== RecordingState.Recording) {
return;
}

Expand Down Expand Up @@ -133,7 +132,7 @@ export const AudioCapture = ({
closeToast,
completeRecording,
errorRecording,
isRecording,
recordingState,
setDurationText,
]);

Expand Down Expand Up @@ -197,7 +196,7 @@ export const AudioCapture = ({
);
}

if (isRecording && !confirmationDialog) {
if (recordingState === RecordingState.Recording && !confirmationDialog) {
return (
<>
<div className="AudioCapture">
Expand Down
60 changes: 45 additions & 15 deletions ts/state/ducks/audioRecorder.ts
View file
Expand Up @@ -20,8 +20,14 @@ export enum ErrorDialogAudioRecorderType {

// State

export enum RecordingState {
Recording = 'recording',
Initializing = 'initializing',
Idle = 'idle',
}

export type AudioPlayerStateType = {
readonly isRecording: boolean;
readonly recordingState: RecordingState;
readonly errorDialogAudioRecorderType?: ErrorDialogAudioRecorderType;
};

Expand All @@ -30,6 +36,7 @@ export type AudioPlayerStateType = {
const CANCEL_RECORDING = 'audioRecorder/CANCEL_RECORDING';
const COMPLETE_RECORDING = 'audioRecorder/COMPLETE_RECORDING';
const ERROR_RECORDING = 'audioRecorder/ERROR_RECORDING';
const NOW_RECORDING = 'audioRecorder/NOW_RECORDING';
const START_RECORDING = 'audioRecorder/START_RECORDING';

type CancelRecordingAction = {
Expand All @@ -48,11 +55,16 @@ type StartRecordingAction = {
type: typeof START_RECORDING;
payload: undefined;
};
type NowRecordingAction = {
type: typeof NOW_RECORDING;
payload: undefined;
};

type AudioPlayerActionType =
| CancelRecordingAction
| CompleteRecordingAction
| ErrorRecordingAction
| NowRecordingAction
| StartRecordingAction;

// Action Creators
Expand All @@ -70,30 +82,40 @@ function startRecording(): ThunkAction<
void,
RootStateType,
unknown,
StartRecordingAction | ErrorRecordingAction
StartRecordingAction | NowRecordingAction | ErrorRecordingAction
> {
return async (dispatch, getState) => {
if (getState().composer.attachments.length) {
return;
}
if (getState().audioRecorder.recordingState !== RecordingState.Idle) {
return;
}

let recordingStarted = false;
dispatch({
type: START_RECORDING,
payload: undefined,
});

try {
recordingStarted = await recorder.start();
const started = await recorder.start();

if (started) {
dispatch({
type: NOW_RECORDING,
payload: undefined,
});
} else {
dispatch({
type: ERROR_RECORDING,
payload: ErrorDialogAudioRecorderType.ErrorRecording,
});
}
} catch (err) {
dispatch({
type: ERROR_RECORDING,
payload: ErrorDialogAudioRecorderType.ErrorRecording,
});
return;
}

if (recordingStarted) {
dispatch({
type: START_RECORDING,
payload: undefined,
});
}
};
}
Expand Down Expand Up @@ -184,7 +206,7 @@ function errorRecording(

function getEmptyState(): AudioPlayerStateType {
return {
isRecording: false,
recordingState: RecordingState.Idle,
};
}

Expand All @@ -196,15 +218,23 @@ export function reducer(
return {
...state,
errorDialogAudioRecorderType: undefined,
isRecording: true,
recordingState: RecordingState.Initializing,
};
}

if (action.type === NOW_RECORDING) {
return {
...state,
errorDialogAudioRecorderType: undefined,
recordingState: RecordingState.Recording,
};
}

if (action.type === CANCEL_RECORDING || action.type === COMPLETE_RECORDING) {
return {
...state,
errorDialogAudioRecorderType: undefined,
isRecording: false,
recordingState: RecordingState.Idle,
};
}

Expand Down
2 changes: 1 addition & 1 deletion ts/state/smart/CompositionArea.tsx
View file
Expand Up @@ -87,7 +87,7 @@ const mapStateToProps = (state: StateType, props: ExternalProps) => {
// AudioCapture
errorDialogAudioRecorderType:
state.audioRecorder.errorDialogAudioRecorderType,
isRecording: state.audioRecorder.isRecording,
recordingState: state.audioRecorder.recordingState,
// AttachmentsList
draftAttachments,
// MediaQualitySelector
Expand Down

0 comments on commit 2ee5526

Please sign in to comment.