From 22219e0e802cc58eb3a6d507a2221ec9fc4ad64f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 14 Apr 2021 21:13:09 -0600 Subject: [PATCH 1/6] Adapt to use an Alignment enum instead --- src/components/views/elements/Field.tsx | 2 +- src/components/views/elements/InfoTooltip.tsx | 6 +-- src/components/views/elements/Tooltip.tsx | 48 ++++++++++++++++--- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/components/views/elements/Field.tsx b/src/components/views/elements/Field.tsx index f5754da9ae5..59d9a115961 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 8f7f1ea53f0..d49090dbae3 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 b2dd00de182..116b226b8f5 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; From 0677cf866cf694d6dae371bce8d3ae91d0c944e7 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 14 Apr 2021 21:15:06 -0600 Subject: [PATCH 2/6] Cap recording length, and warn at 10s remaining See diff for details. Note that this introduces an "Uploading" state which is not currently used. At the moment, if a user hits the maximum time then their recording will be broken. This is expected to be fixed in a future PR. --- src/components/views/rooms/MessageComposer.js | 25 +++++++++- src/i18n/strings/en_EN.json | 1 + src/stores/VoiceRecordingStore.ts | 4 +- src/voice/VoiceRecording.ts | 47 +++++++++++++++++-- 4 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index 283b11a437e..6c227458aa6 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -35,6 +35,8 @@ import ActiveWidgetStore from "../../../stores/ActiveWidgetStore"; 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'); @@ -191,6 +193,7 @@ export default class MessageComposer extends React.Component { joinedConference: WidgetStore.instance.isJoinedToConferenceIn(this.props.room), isComposerEmpty: true, haveRecording: false, + recordingTimeLeftSeconds: null, // when set to a number, shows a toast }; } @@ -331,7 +334,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 head's 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() { @@ -412,8 +425,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 02236f9997b..e8839369b1f 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1473,6 +1473,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 e1685de5297..cc999f23f87 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/voice/VoiceRecording.ts b/src/voice/VoiceRecording.ts index 77c182fc54b..41bce9d6981 100644 --- a/src/voice/VoiceRecording.ts +++ b/src/voice/VoiceRecording.ts @@ -20,17 +20,29 @@ 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"; 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; @@ -40,9 +52,12 @@ export class VoiceRecording { private buffer = new Uint8Array(0); private mxc: string; private recording = false; + private stopping = false; + private haveWarned = false; // whether or not EndingSoon has been fired private observable: SimpleObservable; public constructor(private client: MatrixClient) { + super(); } private async makeRecorder() { @@ -124,7 +139,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 +165,17 @@ 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 && !this.haveWarned) { + this.emit(RecordingState.EndingSoon, {secondsLeft}); + this.haveWarned = true; + } }; public async start(): Promise { @@ -164,9 +190,10 @@ 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 { @@ -174,6 +201,9 @@ export class VoiceRecording { throw new Error("No recording to stop"); } + if (this.stopping) return; + this.stopping = true; + // Disconnect the source early to start shutting down resources this.recorderSource.disconnect(); await this.recorder.stop(); @@ -187,12 +217,19 @@ export class VoiceRecording { // Finally do our post-processing and clean up this.recording = false; - this.recorderProcessor.removeEventListener("audioprocess", this.tryUpdateLiveData); + this.recorderProcessor.removeEventListener("audioprocess", this.processAudioUpdate); await this.recorder.close(); + this.emit(RecordingState.Ended); return this.buffer; } + public destroy() { + // noinspection JSIgnoredPromiseFromCall - not concerned about stop() being called async here + this.stop(); + this.removeAllListeners(); + } + public async upload(): Promise { if (!this.hasRecording) { throw new Error("No recording available to upload"); @@ -200,11 +237,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; } } From 22233a8745ee0f8b043b6955762a5bfbb6e87666 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 14 Apr 2021 21:47:30 -0600 Subject: [PATCH 3/6] Add a concept of a singleflight to avoid repeated calls to stop/ending This makes it easier to keep track of which pieces the client will have already dispatched or been executed, reducing the amount of class members needed. Critically, this makes it so the 'stop' button (which is currently a send button) actually works even after the automatic stop has happened. UI is still pending for stopping recording early. This is not covered by this change. --- src/utils/Singleflight.ts | 101 +++++++++++++++++++++++++++++++ src/voice/VoiceRecording.ts | 51 ++++++++-------- test/Singleflight-test.ts | 115 ++++++++++++++++++++++++++++++++++++ 3 files changed, 242 insertions(+), 25 deletions(-) create mode 100644 src/utils/Singleflight.ts create mode 100644 test/Singleflight-test.ts diff --git a/src/utils/Singleflight.ts b/src/utils/Singleflight.ts new file mode 100644 index 00000000000..c8d303bcac2 --- /dev/null +++ b/src/utils/Singleflight.ts @@ -0,0 +1,101 @@ +/* +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. + */ +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. + * @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 41bce9d6981..55775ff7868 100644 --- a/src/voice/VoiceRecording.ts +++ b/src/voice/VoiceRecording.ts @@ -22,6 +22,7 @@ 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. @@ -52,8 +53,6 @@ export class VoiceRecording extends EventEmitter implements IDestroyable { private buffer = new Uint8Array(0); private mxc: string; private recording = false; - private stopping = false; - private haveWarned = false; // whether or not EndingSoon has been fired private observable: SimpleObservable; public constructor(private client: MatrixClient) { @@ -172,9 +171,11 @@ export class VoiceRecording extends EventEmitter implements IDestroyable { if (secondsLeft <= 0) { // noinspection JSIgnoredPromiseFromCall - we aren't concerned with it overlapping this.stop(); - } else if (secondsLeft <= TARGET_WARN_TIME_LEFT && !this.haveWarned) { - this.emit(RecordingState.EndingSoon, {secondsLeft}); - this.haveWarned = true; + } else if (secondsLeft <= TARGET_WARN_TIME_LEFT) { + Singleflight.for(this, "ending_soon").do(() => { + this.emit(RecordingState.EndingSoon, {secondsLeft}); + return Singleflight.Void; + }); } }; @@ -197,37 +198,37 @@ export class VoiceRecording extends EventEmitter implements IDestroyable { } public async stop(): Promise { - if (!this.recording) { - throw new Error("No recording to stop"); - } - - if (this.stopping) return; - this.stopping = true; + 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.processAudioUpdate); - await this.recorder.close(); - this.emit(RecordingState.Ended); + // 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 { diff --git a/test/Singleflight-test.ts b/test/Singleflight-test.ts new file mode 100644 index 00000000000..4f0c6e0da34 --- /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); + }); +}); + From 1aeb9a5fb2dd120b85062bea5c5c236f63ea0ba5 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 14 Apr 2021 22:04:07 -0600 Subject: [PATCH 4/6] Appease the linter --- src/components/views/elements/Tooltip.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/elements/Tooltip.tsx b/src/components/views/elements/Tooltip.tsx index 116b226b8f5..062d26c8521 100644 --- a/src/components/views/elements/Tooltip.tsx +++ b/src/components/views/elements/Tooltip.tsx @@ -103,7 +103,7 @@ export default class Tooltip extends React.Component { 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) { + switch (this.props.alignment) { case Alignment.Natural: if (parentBox.right > window.innerWidth / 2) { style.right = right; From d23f66bb47f90f40feacbdd9418ca0cc0890fd28 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 16 Apr 2021 10:11:04 -0600 Subject: [PATCH 5/6] Update documentation --- src/components/views/rooms/MessageComposer.js | 2 +- src/utils/Singleflight.ts | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/MessageComposer.js b/src/components/views/rooms/MessageComposer.js index 6c227458aa6..7750a2f2011 100644 --- a/src/components/views/rooms/MessageComposer.js +++ b/src/components/views/rooms/MessageComposer.js @@ -337,7 +337,7 @@ export default class MessageComposer extends React.Component { const recording = VoiceRecordingStore.instance.activeRecording; this.setState({haveRecording: !!recording}); if (recording) { - // We show a little head's up that the recording is about to automatically end soon. The 3s + // 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}) => { diff --git a/src/utils/Singleflight.ts b/src/utils/Singleflight.ts index c8d303bcac2..1c2af4278dd 100644 --- a/src/utils/Singleflight.ts +++ b/src/utils/Singleflight.ts @@ -23,6 +23,22 @@ 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 are 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() { @@ -83,6 +99,15 @@ class SingleflightContext { * 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. */ From 20586e52bc1d4e3598dc96660a7b987920124837 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 19 Apr 2021 10:13:22 -0600 Subject: [PATCH 6/6] Update src/utils/Singleflight.ts to support English Co-authored-by: J. Ryan Stinnett --- src/utils/Singleflight.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/Singleflight.ts b/src/utils/Singleflight.ts index 1c2af4278dd..c2a564ea3eb 100644 --- a/src/utils/Singleflight.ts +++ b/src/utils/Singleflight.ts @@ -30,7 +30,7 @@ const keyMap = new EnhancedMap>(); * to disable a button, however it would be capable of returning a Promise * from the first call. * - * The result of the function call are cached indefinitely, just in case a + * 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. *