From 254815cbcf5998df1aa289c7824837e87be12001 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Wed, 7 Dec 2022 10:37:30 +0100 Subject: [PATCH 1/2] Tweak voice broadcast chunk decoding (#9713) --- .../models/VoiceBroadcastPlayback.ts | 63 +++++++++++-------- .../models/VoiceBroadcastPlayback-test.ts | 3 + 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/voice-broadcast/models/VoiceBroadcastPlayback.ts b/src/voice-broadcast/models/VoiceBroadcastPlayback.ts index d21ca49e33..62ad35628c 100644 --- a/src/voice-broadcast/models/VoiceBroadcastPlayback.ts +++ b/src/voice-broadcast/models/VoiceBroadcastPlayback.ts @@ -143,13 +143,14 @@ export class VoiceBroadcastPlayback return false; } + if (!event.getId() && !event.getTxnId()) { + // skip events without id and txn id + return false; + } + this.chunkEvents.addEvent(event); this.setDuration(this.chunkEvents.getLength()); - if (this.getState() !== VoiceBroadcastPlaybackState.Stopped) { - await this.enqueueChunk(event); - } - if (this.getState() === VoiceBroadcastPlaybackState.Buffering) { await this.start(); this.updateLiveness(); @@ -183,18 +184,7 @@ export class VoiceBroadcastPlayback } }; - private async enqueueChunks(): Promise { - const promises = this.chunkEvents.getEvents().reduce((promises, event: MatrixEvent) => { - if (!this.playbacks.has(event.getId() || "")) { - promises.push(this.enqueueChunk(event)); - } - return promises; - }, [] as Promise[]); - - await Promise.all(promises); - } - - private async enqueueChunk(chunkEvent: MatrixEvent): Promise { + private async loadPlayback(chunkEvent: MatrixEvent): Promise { const eventId = chunkEvent.getId(); if (!eventId) { @@ -215,6 +205,14 @@ export class VoiceBroadcastPlayback }); } + private unloadPlayback(event: MatrixEvent): void { + const playback = this.playbacks.get(event.getId()!); + if (!playback) return; + + playback.destroy(); + this.playbacks.delete(event.getId()!); + } + private onPlaybackPositionUpdate = ( event: MatrixEvent, position: number, @@ -261,6 +259,7 @@ export class VoiceBroadcastPlayback if (newState !== PlaybackState.Stopped) return; await this.playNext(); + this.unloadPlayback(event); }; private async playNext(): Promise { @@ -283,10 +282,11 @@ export class VoiceBroadcastPlayback private async playEvent(event: MatrixEvent): Promise { this.setState(VoiceBroadcastPlaybackState.Playing); this.currentlyPlaying = event; - await this.getPlaybackForEvent(event)?.play(); + const playback = await this.getOrLoadPlaybackForEvent(event); + playback?.play(); } - private getPlaybackForEvent(event: MatrixEvent): Playback | undefined { + private async getOrLoadPlaybackForEvent(event: MatrixEvent): Promise { const eventId = event.getId(); if (!eventId) { @@ -294,6 +294,10 @@ export class VoiceBroadcastPlayback return; } + if (!this.playbacks.has(eventId)) { + await this.loadPlayback(event); + } + const playback = this.playbacks.get(eventId); if (!playback) { @@ -301,9 +305,18 @@ export class VoiceBroadcastPlayback logger.warn("unable to find playback for event", event); } + // try to load the playback for the next event for a smooth(er) playback + const nextEvent = this.chunkEvents.getNext(event); + if (nextEvent) this.loadPlayback(nextEvent); + return playback; } + private getCurrentPlayback(): Playback | undefined { + if (!this.currentlyPlaying) return; + return this.playbacks.get(this.currentlyPlaying.getId()!); + } + public getLiveness(): VoiceBroadcastLiveness { return this.liveness; } @@ -365,11 +378,8 @@ export class VoiceBroadcastPlayback return; } - const currentPlayback = this.currentlyPlaying - ? this.getPlaybackForEvent(this.currentlyPlaying) - : null; - - const skipToPlayback = this.getPlaybackForEvent(event); + const currentPlayback = this.getCurrentPlayback(); + const skipToPlayback = await this.getOrLoadPlaybackForEvent(event); if (!skipToPlayback) { logger.warn("voice broadcast chunk to skip to not found", event); @@ -396,14 +406,13 @@ export class VoiceBroadcastPlayback } public async start(): Promise { - await this.enqueueChunks(); const chunkEvents = this.chunkEvents.getEvents(); const toPlay = this.getInfoState() === VoiceBroadcastInfoState.Stopped ? chunkEvents[0] // start at the beginning for an ended voice broadcast : chunkEvents[chunkEvents.length - 1]; // start at the current chunk for an ongoing voice broadcast - if (this.playbacks.has(toPlay?.getId() || "")) { + if (toPlay) { return this.playEvent(toPlay); } @@ -422,7 +431,7 @@ export class VoiceBroadcastPlayback this.setState(VoiceBroadcastPlaybackState.Paused); if (!this.currentlyPlaying) return; - this.getPlaybackForEvent(this.currentlyPlaying)?.pause(); + this.getCurrentPlayback()?.pause(); } public resume(): void { @@ -433,7 +442,7 @@ export class VoiceBroadcastPlayback } this.setState(VoiceBroadcastPlaybackState.Playing); - this.getPlaybackForEvent(this.currentlyPlaying)?.play(); + this.getCurrentPlayback()?.play(); } /** diff --git a/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts b/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts index 64b2362703..269ee1a3e7 100644 --- a/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts +++ b/test/voice-broadcast/models/VoiceBroadcastPlayback-test.ts @@ -387,6 +387,9 @@ describe("VoiceBroadcastPlayback", () => { }); it("should play until the end", () => { + // assert first chunk was unloaded + expect(chunk1Playback.destroy).toHaveBeenCalled(); + // assert that the second chunk is being played expect(chunk2Playback.play).toHaveBeenCalled(); From 7943f838581b79e21422ad77cdf5d39297f1c7b9 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 7 Dec 2022 12:13:35 +0100 Subject: [PATCH 2/2] Change formatting buttons behavior (#9715) Change formatting buttons behaviour --- .../components/_FormattingButtons.pcss | 2 ++ .../components/FormattingButtons.tsx | 5 ++++- .../components/FormattingButtons-test.tsx | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/res/css/views/rooms/wysiwyg_composer/components/_FormattingButtons.pcss b/res/css/views/rooms/wysiwyg_composer/components/_FormattingButtons.pcss index 76026ff938..342a40c606 100644 --- a/res/css/views/rooms/wysiwyg_composer/components/_FormattingButtons.pcss +++ b/res/css/views/rooms/wysiwyg_composer/components/_FormattingButtons.pcss @@ -53,7 +53,9 @@ limitations under the License. height: var(--size); border-radius: 5px; } + } + .mx_FormattingButtons_Button_hover { &:hover { &::after { background: rgba($secondary-content, 0.1); diff --git a/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx b/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx index 32b132cc6c..c9408c8f0f 100644 --- a/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx +++ b/src/components/views/rooms/wysiwyg_composer/components/FormattingButtons.tsx @@ -48,7 +48,10 @@ function Button({ label, keyCombo, onClick, isActive, className }: ButtonProps) onClick={onClick} title={label} className={ - classNames('mx_FormattingButtons_Button', className, { 'mx_FormattingButtons_active': isActive })} + classNames('mx_FormattingButtons_Button', className, { + 'mx_FormattingButtons_active': isActive, + 'mx_FormattingButtons_Button_hover': !isActive, + })} tooltip={keyCombo && } alignment={Alignment.Top} />; diff --git a/test/components/views/rooms/wysiwyg_composer/components/FormattingButtons-test.tsx b/test/components/views/rooms/wysiwyg_composer/components/FormattingButtons-test.tsx index 2447e2f076..f97b2c614f 100644 --- a/test/components/views/rooms/wysiwyg_composer/components/FormattingButtons-test.tsx +++ b/test/components/views/rooms/wysiwyg_composer/components/FormattingButtons-test.tsx @@ -75,4 +75,20 @@ describe('FormattingButtons', () => { // Then expect(await screen.findByText('Bold')).toBeTruthy(); }); + + it('Should not have hover style when active', async () => { + // When + const user = userEvent.setup(); + render(); + await user.hover(screen.getByLabelText('Bold')); + + // Then + expect(screen.getByLabelText('Bold')).not.toHaveClass('mx_FormattingButtons_Button_hover'); + + // When + await user.hover(screen.getByLabelText('Underline')); + + // Then + expect(screen.getByLabelText('Underline')).toHaveClass('mx_FormattingButtons_Button_hover'); + }); });