diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index f5754da9ae..59d9a11596 100644 --- a/src/components/views/elements/Field.tsx +++ b/src/components/views/elements/Field.tsx @@ -262,7 +262,7 @@ export default class Field extends React.PureComponent { tooltipClassName={classNames("mx_Field_tooltip", tooltipClassName)} visible={(this.state.focused && this.props.forceTooltipVisible) || this.state.feedbackVisible} label={tooltipContent || this.state.feedback} - forceOnRight + alignment={Tooltip.Alignment.Right} />; } diff --git a/src/components/views/elements/InfoTooltip.tsx b/src/components/views/elements/InfoTooltip.tsx index 8f7f1ea53f..d49090dbae 100644 --- a/src/components/views/elements/InfoTooltip.tsx +++ b/src/components/views/elements/InfoTooltip.tsx @@ -18,8 +18,8 @@ limitations under the License. import React from 'react'; import classNames from 'classnames'; -import Tooltip from './Tooltip'; -import { _t } from "../../../languageHandler"; +import Tooltip, {Alignment} from './Tooltip'; +import {_t} from "../../../languageHandler"; import {replaceableComponent} from "../../../utils/replaceableComponent"; interface ITooltipProps { @@ -61,7 +61,7 @@ export default class InfoTooltip extends React.PureComponent :
; return (
diff --git a/src/components/views/elements/Tooltip.tsx b/src/components/views/elements/Tooltip.tsx index b2dd00de18..062d26c852 100644 --- a/src/components/views/elements/Tooltip.tsx +++ b/src/components/views/elements/Tooltip.tsx @@ -25,6 +25,14 @@ import {replaceableComponent} from "../../../utils/replaceableComponent"; const MIN_TOOLTIP_HEIGHT = 25; +export enum Alignment { + Natural, // Pick left or right + Left, + Right, + Top, // Centered + Bottom, // Centered +} + interface IProps { // Class applied to the element used to position the tooltip className?: string; @@ -36,7 +44,7 @@ interface IProps { visible?: boolean; // the react element to put into the tooltip label: React.ReactNode; - forceOnRight?: boolean; + alignment?: Alignment; // defaults to Natural yOffset?: number; } @@ -46,10 +54,14 @@ export default class Tooltip extends React.Component { private tooltip: void | Element | Component; private parent: Element; + // XXX: This is because some components (Field) are unable to `import` the Tooltip class, + // so we expose the Alignment options off of us statically. + public static readonly Alignment = Alignment; public static readonly defaultProps = { visible: true, yOffset: 0, + alignment: Alignment.Natural, }; // Create a wrapper for the tooltip outside the parent and attach it to the body element @@ -86,11 +98,35 @@ export default class Tooltip extends React.Component { offset = Math.floor(parentBox.height - MIN_TOOLTIP_HEIGHT); } - style.top = (parentBox.top - 2 + this.props.yOffset) + window.pageYOffset + offset; - if (!this.props.forceOnRight && parentBox.right > window.innerWidth / 2) { - style.right = window.innerWidth - parentBox.right - window.pageXOffset - 16; - } else { - style.left = parentBox.right + window.pageXOffset + 6; + const baseTop = (parentBox.top - 2 + this.props.yOffset) + window.pageYOffset; + const top = baseTop + offset; + const right = window.innerWidth - parentBox.right - window.pageXOffset - 16; + const left = parentBox.right + window.pageXOffset + 6; + const horizontalCenter = parentBox.right - window.pageXOffset - (parentBox.width / 2); + switch (this.props.alignment) { + case Alignment.Natural: + if (parentBox.right > window.innerWidth / 2) { + style.right = right; + style.top = top; + break; + } + // fall through to Right + case Alignment.Right: + style.left = left; + style.top = top; + break; + case Alignment.Left: + style.right = right; + style.top = top; + break; + case Alignment.Top: + style.top = baseTop - 16; + style.left = horizontalCenter; + break; + case Alignment.Bottom: + style.top = baseTop + parentBox.height; + style.left = horizontalCenter; + break; } return style; diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index 8910068d03..5178d52305 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -33,6 +33,8 @@ import {UPDATE_EVENT} from "../../../stores/AsyncStore"; import {replaceableComponent} from "../../../utils/replaceableComponent"; import VoiceRecordComposerTile from "./VoiceRecordComposerTile"; import {VoiceRecordingStore} from "../../../stores/VoiceRecordingStore"; +import {RecordingState} from "../../../voice/VoiceRecording"; +import Tooltip, {Alignment} from "../elements/Tooltip"; function ComposerAvatar(props) { const MemberStatusMessageAvatar = sdk.getComponent('avatars.MemberStatusMessageAvatar'); @@ -185,6 +187,7 @@ export default class MessageComposer extends React.Component { canSendMessages: this.props.room.maySendMessage(), isComposerEmpty: true, haveRecording: false, + recordingTimeLeftSeconds: null, // when set to a number, shows a toast }; } @@ -315,7 +318,17 @@ export default class MessageComposer extends React.Component { } _onVoiceStoreUpdate = () => { - this.setState({haveRecording: !!VoiceRecordingStore.instance.activeRecording}); + const recording = VoiceRecordingStore.instance.activeRecording; + this.setState({haveRecording: !!recording}); + if (recording) { + // We show a little heads up that the recording is about to automatically end soon. The 3s + // display time is completely arbitrary. Note that we don't need to deregister the listener + // because the recording instance will clean that up for us. + recording.on(RecordingState.EndingSoon, ({secondsLeft}) => { + this.setState({recordingTimeLeftSeconds: secondsLeft}); + setTimeout(() => this.setState({recordingTimeLeftSeconds: null}), 3000); + }); + } }; render() { @@ -396,8 +409,18 @@ export default class MessageComposer extends React.Component { ); } + let recordingTooltip; + const secondsLeft = Math.round(this.state.recordingTimeLeftSeconds); + if (secondsLeft) { + recordingTooltip = ; + } + return (
+ {recordingTooltip}
diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 3ca9efac11..ed5eef8b5a 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1474,6 +1474,7 @@ "The conversation continues here.": "The conversation continues here.", "This room has been replaced and is no longer active.": "This room has been replaced and is no longer active.", "You do not have permission to post to this room": "You do not have permission to post to this room", + "%(seconds)ss left": "%(seconds)ss left", "Bold": "Bold", "Italics": "Italics", "Strikethrough": "Strikethrough", diff --git a/src/stores/VoiceRecordingStore.ts b/src/stores/VoiceRecordingStore.ts index e1685de529..cc999f23f8 100644 --- a/src/stores/VoiceRecordingStore.ts +++ b/src/stores/VoiceRecordingStore.ts @@ -73,9 +73,7 @@ export class VoiceRecordingStore extends AsyncStoreWithClient { */ public disposeRecording(): Promise { if (this.state.recording) { - // Stop for good measure, but completely async because we're not concerned with this - // passing or failing. - this.state.recording.stop().catch(e => console.error("Error stopping recording", e)); + this.state.recording.destroy(); // stops internally } return this.updateState({recording: null}); } diff --git a/src/utils/Singleflight.ts b/src/utils/Singleflight.ts new file mode 100644 index 0000000000..c2a564ea3e --- /dev/null +++ b/src/utils/Singleflight.ts @@ -0,0 +1,126 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import {EnhancedMap} from "./maps"; + +// Inspired by https://pkg.go.dev/golang.org/x/sync/singleflight + +const keyMap = new EnhancedMap>(); + +/** + * Access class to get a singleflight context. Singleflights execute a + * function exactly once, unless instructed to forget about a result. + * + * Typically this is used to de-duplicate an action, such as a save button + * being pressed, without having to track state internally for an operation + * already being in progress. This doesn't expose a flag which can be used + * to disable a button, however it would be capable of returning a Promise + * from the first call. + * + * The result of the function call is cached indefinitely, just in case a + * second call comes through late. There are various functions named "forget" + * to have the cache be cleared of a result. + * + * Singleflights in our usecase are tied to an instance of something, combined + * with a string key to differentiate between multiple possible actions. This + * means that a "save" key will be scoped to the instance which defined it and + * not leak between other instances. This is done to avoid having to concatenate + * variables to strings to essentially namespace the field, for most cases. + */ +export class Singleflight { + private constructor() { + } + + /** + * A void marker to help with returning a value in a singleflight context. + * If your code doesn't return anything, return this instead. + */ + public static Void = Symbol("void"); + + /** + * Acquire a singleflight context. + * @param {Object} instance An instance to associate the context with. Can be any object. + * @param {string} key A string key relevant to that instance to namespace under. + * @returns {SingleflightContext} Returns the context to execute the function. + */ + public static for(instance: Object, key: string): SingleflightContext { + if (!instance || !key) throw new Error("An instance and key must be supplied"); + return new SingleflightContext(instance, key); + } + + /** + * Forgets all results for a given instance. + * @param {Object} instance The instance to forget about. + */ + public static forgetAllFor(instance: Object) { + keyMap.delete(instance); + } + + /** + * Forgets all cached results for all instances. Intended for use by tests. + */ + public static forgetAll() { + for (const k of keyMap.keys()) { + keyMap.remove(k); + } + } +} + +class SingleflightContext { + public constructor(private instance: Object, private key: string) { + } + + /** + * Forget this particular instance and key combination, discarding the result. + */ + public forget() { + const map = keyMap.get(this.instance); + if (!map) return; + map.remove(this.key); + if (!map.size) keyMap.remove(this.instance); + } + + /** + * Execute a function. If a result is already known, that will be returned instead + * of executing the provided function. However, if no result is known then the function + * will be called, with its return value cached. The function must return a value + * other than `undefined` - take a look at Singleflight.Void if you don't have a return + * to make. + * + * Note that this technically allows the caller to provide a different function each time: + * this is largely considered a bad idea and should not be done. Singleflights work off the + * premise that something needs to happen once, so duplicate executions will be ignored. + * + * For ideal performance and behaviour, functions which return promises are preferred. If + * a function is not returning a promise, it should return as soon as possible to avoid a + * second call potentially racing it. The promise returned by this function will be that + * of the first execution of the function, even on duplicate calls. + * @param {Function} fn The function to execute. + * @returns The recorded value. + */ + public do(fn: () => T): T { + const map = keyMap.getOrCreate(this.instance, new EnhancedMap()); + + // We have to manually getOrCreate() because we need to execute the fn + let val = map.get(this.key); + if (val === undefined) { + val = fn(); + map.set(this.key, val); + } + + return val; + } +} diff --git a/src/voice/VoiceRecording.ts b/src/voice/VoiceRecording.ts index 77c182fc54..55775ff786 100644 --- a/src/voice/VoiceRecording.ts +++ b/src/voice/VoiceRecording.ts @@ -20,17 +20,30 @@ import {MatrixClient} from "matrix-js-sdk/src/client"; import CallMediaHandler from "../CallMediaHandler"; import {SimpleObservable} from "matrix-widget-api"; import {clamp} from "../utils/numbers"; +import EventEmitter from "events"; +import {IDestroyable} from "../utils/IDestroyable"; +import {Singleflight} from "../utils/Singleflight"; const CHANNELS = 1; // stereo isn't important const SAMPLE_RATE = 48000; // 48khz is what WebRTC uses. 12khz is where we lose quality. const BITRATE = 24000; // 24kbps is pretty high quality for our use case in opus. +const TARGET_MAX_LENGTH = 120; // 2 minutes in seconds. Somewhat arbitrary, though longer == larger files. +const TARGET_WARN_TIME_LEFT = 10; // 10 seconds, also somewhat arbitrary. export interface IRecordingUpdate { waveform: number[]; // floating points between 0 (low) and 1 (high). timeSeconds: number; // float } -export class VoiceRecording { +export enum RecordingState { + Started = "started", + EndingSoon = "ending_soon", // emits an object with a single numerical value: secondsLeft + Ended = "ended", + Uploading = "uploading", + Uploaded = "uploaded", +} + +export class VoiceRecording extends EventEmitter implements IDestroyable { private recorder: Recorder; private recorderContext: AudioContext; private recorderSource: MediaStreamAudioSourceNode; @@ -43,6 +56,7 @@ export class VoiceRecording { private observable: SimpleObservable; public constructor(private client: MatrixClient) { + super(); } private async makeRecorder() { @@ -124,7 +138,7 @@ export class VoiceRecording { return this.mxc; } - private tryUpdateLiveData = (ev: AudioProcessingEvent) => { + private processAudioUpdate = (ev: AudioProcessingEvent) => { if (!this.recording) return; // The time domain is the input to the FFT, which means we use an array of the same @@ -150,6 +164,19 @@ export class VoiceRecording { waveform: translatedData, timeSeconds: ev.playbackTime, }); + + // Now that we've updated the data/waveform, let's do a time check. We don't want to + // go horribly over the limit. We also emit a warning state if needed. + const secondsLeft = TARGET_MAX_LENGTH - ev.playbackTime; + if (secondsLeft <= 0) { + // noinspection JSIgnoredPromiseFromCall - we aren't concerned with it overlapping + this.stop(); + } else if (secondsLeft <= TARGET_WARN_TIME_LEFT) { + Singleflight.for(this, "ending_soon").do(() => { + this.emit(RecordingState.EndingSoon, {secondsLeft}); + return Singleflight.Void; + }); + } }; public async start(): Promise { @@ -164,33 +191,44 @@ export class VoiceRecording { } this.observable = new SimpleObservable(); await this.makeRecorder(); - this.recorderProcessor.addEventListener("audioprocess", this.tryUpdateLiveData); + this.recorderProcessor.addEventListener("audioprocess", this.processAudioUpdate); await this.recorder.start(); this.recording = true; + this.emit(RecordingState.Started); } public async stop(): Promise { - if (!this.recording) { - throw new Error("No recording to stop"); - } + return Singleflight.for(this, "stop").do(async () => { + if (!this.recording) { + throw new Error("No recording to stop"); + } - // Disconnect the source early to start shutting down resources - this.recorderSource.disconnect(); - await this.recorder.stop(); + // Disconnect the source early to start shutting down resources + this.recorderSource.disconnect(); + await this.recorder.stop(); - // close the context after the recorder so the recorder doesn't try to - // connect anything to the context (this would generate a warning) - await this.recorderContext.close(); + // close the context after the recorder so the recorder doesn't try to + // connect anything to the context (this would generate a warning) + await this.recorderContext.close(); - // Now stop all the media tracks so we can release them back to the user/OS - this.recorderStream.getTracks().forEach(t => t.stop()); + // Now stop all the media tracks so we can release them back to the user/OS + this.recorderStream.getTracks().forEach(t => t.stop()); - // Finally do our post-processing and clean up - this.recording = false; - this.recorderProcessor.removeEventListener("audioprocess", this.tryUpdateLiveData); - await this.recorder.close(); + // Finally do our post-processing and clean up + this.recording = false; + this.recorderProcessor.removeEventListener("audioprocess", this.processAudioUpdate); + await this.recorder.close(); + this.emit(RecordingState.Ended); - return this.buffer; + return this.buffer; + }); + } + + public destroy() { + // noinspection JSIgnoredPromiseFromCall - not concerned about stop() being called async here + this.stop(); + this.removeAllListeners(); + Singleflight.forgetAllFor(this); } public async upload(): Promise { @@ -200,11 +238,13 @@ export class VoiceRecording { if (this.mxc) return this.mxc; + this.emit(RecordingState.Uploading); this.mxc = await this.client.uploadContent(new Blob([this.buffer], { type: "audio/ogg", }), { onlyContentUri: false, // to stop the warnings in the console }).then(r => r['content_uri']); + this.emit(RecordingState.Uploaded); return this.mxc; } } diff --git a/test/Singleflight-test.ts b/test/Singleflight-test.ts new file mode 100644 index 0000000000..4f0c6e0da3 --- /dev/null +++ b/test/Singleflight-test.ts @@ -0,0 +1,115 @@ +/* +Copyright 2021 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import {Singleflight} from "../src/utils/Singleflight"; + +describe('Singleflight', () => { + afterEach(() => { + Singleflight.forgetAll(); + }); + + it('should throw for bad context variables', () => { + const permutations: [Object, string][] = [ + [null, null], + [{}, null], + [null, "test"], + ]; + for (const p of permutations) { + try { + Singleflight.for(p[0], p[1]); + // noinspection ExceptionCaughtLocallyJS + throw new Error("failed to fail: " + JSON.stringify(p)); + } catch (e) { + expect(e.message).toBe("An instance and key must be supplied"); + } + } + }); + + it('should execute the function once', () => { + const instance = {}; + const key = "test"; + const val = {}; // unique object for reference check + const fn = jest.fn().mockReturnValue(val); + const sf = Singleflight.for(instance, key); + const r1 = sf.do(fn); + expect(r1).toBe(val); + expect(fn.mock.calls.length).toBe(1); + const r2 = sf.do(fn); + expect(r2).toBe(val); + expect(fn.mock.calls.length).toBe(1); + }); + + it('should execute the function once, even with new contexts', () => { + const instance = {}; + const key = "test"; + const val = {}; // unique object for reference check + const fn = jest.fn().mockReturnValue(val); + let sf = Singleflight.for(instance, key); + const r1 = sf.do(fn); + expect(r1).toBe(val); + expect(fn.mock.calls.length).toBe(1); + sf = Singleflight.for(instance, key); // RESET FOR TEST + const r2 = sf.do(fn); + expect(r2).toBe(val); + expect(fn.mock.calls.length).toBe(1); + }); + + it('should execute the function twice if the result was forgotten', () => { + const instance = {}; + const key = "test"; + const val = {}; // unique object for reference check + const fn = jest.fn().mockReturnValue(val); + const sf = Singleflight.for(instance, key); + const r1 = sf.do(fn); + expect(r1).toBe(val); + expect(fn.mock.calls.length).toBe(1); + sf.forget(); + const r2 = sf.do(fn); + expect(r2).toBe(val); + expect(fn.mock.calls.length).toBe(2); + }); + + it('should execute the function twice if the instance was forgotten', () => { + const instance = {}; + const key = "test"; + const val = {}; // unique object for reference check + const fn = jest.fn().mockReturnValue(val); + const sf = Singleflight.for(instance, key); + const r1 = sf.do(fn); + expect(r1).toBe(val); + expect(fn.mock.calls.length).toBe(1); + Singleflight.forgetAllFor(instance); + const r2 = sf.do(fn); + expect(r2).toBe(val); + expect(fn.mock.calls.length).toBe(2); + }); + + it('should execute the function twice if everything was forgotten', () => { + const instance = {}; + const key = "test"; + const val = {}; // unique object for reference check + const fn = jest.fn().mockReturnValue(val); + const sf = Singleflight.for(instance, key); + const r1 = sf.do(fn); + expect(r1).toBe(val); + expect(fn.mock.calls.length).toBe(1); + Singleflight.forgetAll(); + const r2 = sf.do(fn); + expect(r2).toBe(val); + expect(fn.mock.calls.length).toBe(2); + }); +}); +