From 48c06f848181ea1eecc508f85f273f9e7172fc73 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 10:08:51 -0600 Subject: [PATCH 01/10] Crude way of layering the waveform and seek bar Not intended for production. --- .../audio_messages/_PlaybackContainer.scss | 25 ++++++++++++++++--- .../audio_messages/RecordingPlayback.tsx | 22 +++++++++------- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/res/css/views/audio_messages/_PlaybackContainer.scss b/res/css/views/audio_messages/_PlaybackContainer.scss index 4999980beaf..079ebd3baae 100644 --- a/res/css/views/audio_messages/_PlaybackContainer.scss +++ b/res/css/views/audio_messages/_PlaybackContainer.scss @@ -55,9 +55,6 @@ limitations under the License. // For timeline-rendered playback, mirror the values for where the clock is in // the waveform version. .mx_SeekBar { - margin-left: 8px; - margin-right: 6px; - & + .mx_Clock { text-align: right; @@ -65,4 +62,26 @@ limitations under the License. padding: 0; } } + + .mx_RecordingPlayback_waveformAndSeek { + margin-left: 8px; + margin-right: 6px; + position: relative; + width: 100%; + height: 32px; + + .mx_Waveform { + position: absolute; + top: -1px; + left: 0; + width: 102%; + } + + .mx_SeekBar { + position: absolute; + top: 13px; + left: 0; + width: 100%; + } + } } diff --git a/src/components/views/audio_messages/RecordingPlayback.tsx b/src/components/views/audio_messages/RecordingPlayback.tsx index f9e84059588..b0cd1e66a60 100644 --- a/src/components/views/audio_messages/RecordingPlayback.tsx +++ b/src/components/views/audio_messages/RecordingPlayback.tsx @@ -26,14 +26,14 @@ interface IProps extends IAudioPlayerBaseProps { /** * When true, use a waveform instead of a seek bar */ - withWaveform?: boolean; + withWaveform?: boolean; // TODO: Refactor into look&feel enum } export default class RecordingPlayback extends AudioPlayerBase { // This component is rendered in two ways: the composer and timeline. They have different // rendering properties (specifically the difference of a waveform or not). - private renderWaveformLook(): ReactNode { + private renderComposerLook(): ReactNode { return <> @@ -42,12 +42,15 @@ export default class RecordingPlayback extends AudioPlayerBase { private renderSeekableLook(): ReactNode { return <> - +
+ + +
; } @@ -60,7 +63,8 @@ export default class RecordingPlayback extends AudioPlayerBase { playbackPhase={this.state.playbackPhase} ref={this.playPauseRef} /> - { this.props.withWaveform ? this.renderWaveformLook() : this.renderSeekableLook() } + { this.props.withWaveform ? this.renderComposerLook() : null } + { this.renderSeekableLook() } ); } From 470c1b805e3573100febe66b65e746c7ed2b3541 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 13:14:59 -0600 Subject: [PATCH 02/10] Use a layout prop instead of something less descriptive --- .../audio_messages/_PlaybackContainer.scss | 2 +- .../audio_messages/RecordingPlayback.tsx | 33 ++++++++++++---- .../views/rooms/VoiceRecordComposerTile.tsx | 7 +++- .../audio_messages/RecordingPlayback-test.tsx | 38 +++++++++++++------ 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/res/css/views/audio_messages/_PlaybackContainer.scss b/res/css/views/audio_messages/_PlaybackContainer.scss index 079ebd3baae..dcc297eca26 100644 --- a/res/css/views/audio_messages/_PlaybackContainer.scss +++ b/res/css/views/audio_messages/_PlaybackContainer.scss @@ -63,7 +63,7 @@ limitations under the License. } } - .mx_RecordingPlayback_waveformAndSeek { + .mx_RecordingPlayback_timelineLayoutMiddle { margin-left: 8px; margin-right: 6px; position: relative; diff --git a/src/components/views/audio_messages/RecordingPlayback.tsx b/src/components/views/audio_messages/RecordingPlayback.tsx index b0cd1e66a60..b14b3818d2e 100644 --- a/src/components/views/audio_messages/RecordingPlayback.tsx +++ b/src/components/views/audio_messages/RecordingPlayback.tsx @@ -22,11 +22,20 @@ import AudioPlayerBase, { IProps as IAudioPlayerBaseProps } from "./AudioPlayerB import SeekBar from "./SeekBar"; import PlaybackWaveform from "./PlaybackWaveform"; -interface IProps extends IAudioPlayerBaseProps { +export enum PlaybackLayout { + /** + * Clock on the left side of a waveform, without seek bar. + */ + Composer, + /** - * When true, use a waveform instead of a seek bar + * Clock on the right side of a waveform, with an added seek bar. */ - withWaveform?: boolean; // TODO: Refactor into look&feel enum + Timeline, +} + +interface IProps extends IAudioPlayerBaseProps { + layout?: PlaybackLayout; // Defaults to Timeline layout } export default class RecordingPlayback extends AudioPlayerBase { @@ -40,9 +49,9 @@ export default class RecordingPlayback extends AudioPlayerBase { ; } - private renderSeekableLook(): ReactNode { + private renderTimelineLook(): ReactNode { return <> -
+
{ } protected renderComponent(): ReactNode { + let body: ReactNode; + switch(this.props.layout) { + case PlaybackLayout.Composer: + body = this.renderComposerLook(); + break; + case PlaybackLayout.Timeline: // default is timeline, fall through. + default: + body = this.renderTimelineLook(); + break; + } + return (
{ playbackPhase={this.state.playbackPhase} ref={this.playPauseRef} /> - { this.props.withWaveform ? this.renderComposerLook() : null } - { this.renderSeekableLook() } + { body }
); } diff --git a/src/components/views/rooms/VoiceRecordComposerTile.tsx b/src/components/views/rooms/VoiceRecordComposerTile.tsx index 27201bedbab..9e3a20ea489 100644 --- a/src/components/views/rooms/VoiceRecordComposerTile.tsx +++ b/src/components/views/rooms/VoiceRecordComposerTile.tsx @@ -28,7 +28,7 @@ import LiveRecordingWaveform from "../audio_messages/LiveRecordingWaveform"; import LiveRecordingClock from "../audio_messages/LiveRecordingClock"; import { VoiceRecordingStore } from "../../../stores/VoiceRecordingStore"; import { UPDATE_EVENT } from "../../../stores/AsyncStore"; -import RecordingPlayback from "../audio_messages/RecordingPlayback"; +import RecordingPlayback, { PlaybackLayout } from "../audio_messages/RecordingPlayback"; import Modal from "../../../Modal"; import ErrorDialog from "../dialogs/ErrorDialog"; import MediaDeviceHandler, { MediaDeviceKindEnum } from "../../../MediaDeviceHandler"; @@ -231,7 +231,10 @@ export default class VoiceRecordComposerTile extends React.PureComponent; + return ; } // only other UI is the recording-in-progress UI diff --git a/test/components/views/audio_messages/RecordingPlayback-test.tsx b/test/components/views/audio_messages/RecordingPlayback-test.tsx index 931dca34d67..3dd577f7ff7 100644 --- a/test/components/views/audio_messages/RecordingPlayback-test.tsx +++ b/test/components/views/audio_messages/RecordingPlayback-test.tsx @@ -20,13 +20,14 @@ import { mocked } from 'jest-mock'; import { logger } from 'matrix-js-sdk/src/logger'; import { act } from 'react-dom/test-utils'; -import RecordingPlayback from '../../../../src/components/views/audio_messages/RecordingPlayback'; +import RecordingPlayback, { PlaybackLayout } from '../../../../src/components/views/audio_messages/RecordingPlayback'; import { Playback } from '../../../../src/audio/Playback'; import RoomContext, { TimelineRenderingType } from '../../../../src/contexts/RoomContext'; import { createAudioContext } from '../../../../src/audio/compat'; import { findByTestId, flushPromises } from '../../../test-utils'; import PlaybackWaveform from '../../../../src/components/views/audio_messages/PlaybackWaveform'; import SeekBar from "../../../../src/components/views/audio_messages/SeekBar"; +import PlaybackClock from "../../../../src/components/views/audio_messages/PlaybackClock"; jest.mock('../../../../src/audio/compat', () => ({ createAudioContext: jest.fn(), @@ -128,19 +129,34 @@ describe('', () => { expect(playback.toggle).toHaveBeenCalled(); }); - it('should render a seek bar by default', () => { - const playback = new Playback(new ArrayBuffer(8)); - const component = getComponent({ playback }); + describe('Composer Layout', () => { + it('should have a waveform, no seek bar, and clock', () => { + const playback = new Playback(new ArrayBuffer(8)); + const component = getComponent({ playback, layout: PlaybackLayout.Composer }); - expect(component.find(PlaybackWaveform).length).toBeFalsy(); - expect(component.find(SeekBar).length).toBeTruthy(); + expect(component.find(PlaybackClock).length).toBeTruthy(); + expect(component.find(PlaybackWaveform).length).toBeTruthy(); + expect(component.find(SeekBar).length).toBeFalsy(); + }); }); - it('should render a waveform when requested', () => { - const playback = new Playback(new ArrayBuffer(8)); - const component = getComponent({ playback, withWaveform: true }); + describe('Timeline Layout', () => { + it('should have a waveform, a seek bar, and clock', () => { + const playback = new Playback(new ArrayBuffer(8)); + const component = getComponent({ playback, layout: PlaybackLayout.Timeline }); - expect(component.find(PlaybackWaveform).length).toBeTruthy(); - expect(component.find(SeekBar).length).toBeFalsy(); + expect(component.find(PlaybackClock).length).toBeTruthy(); + expect(component.find(PlaybackWaveform).length).toBeTruthy(); + expect(component.find(SeekBar).length).toBeTruthy(); + }); + + it('should be the default', () => { + const playback = new Playback(new ArrayBuffer(8)); + const component = getComponent({ playback }); // no layout set for test + + expect(component.find(PlaybackClock).length).toBeTruthy(); + expect(component.find(PlaybackWaveform).length).toBeTruthy(); + expect(component.find(SeekBar).length).toBeTruthy(); + }); }); }); From e7fe1b91f057200990f8d525bb96c7420a569b56 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 13:39:23 -0600 Subject: [PATCH 03/10] Fix alignment properly, and play with styles --- .../audio_messages/_PlaybackContainer.scss | 53 +++++++++++++------ 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/res/css/views/audio_messages/_PlaybackContainer.scss b/res/css/views/audio_messages/_PlaybackContainer.scss index dcc297eca26..6930580d0c7 100644 --- a/res/css/views/audio_messages/_PlaybackContainer.scss +++ b/res/css/views/audio_messages/_PlaybackContainer.scss @@ -52,36 +52,55 @@ limitations under the License. padding-left: 8px; // isolate from recording circle / play control } - // For timeline-rendered playback, mirror the values for where the clock is in - // the waveform version. - .mx_SeekBar { - & + .mx_Clock { - text-align: right; - - // Take the padding off the clock because it's accounted for in the seek bar - padding: 0; - } - } - .mx_RecordingPlayback_timelineLayoutMiddle { margin-left: 8px; margin-right: 6px; position: relative; - width: 100%; - height: 32px; + display: inline-block; + flex: 1; + height: 30px; // same height as mx_Waveform, needed for automatic vertical centering .mx_Waveform { position: absolute; - top: -1px; left: 0; - width: 102%; + top: 0; } .mx_SeekBar { position: absolute; - top: 13px; left: 0; - width: 100%; + height: 30px; + top: -1px; // visually vertically centered + + // Hide the hairline progress bar. Need to have distinct rules because CSS is weird. + background: none; + &::before { + background: none; + } + &::-moz-range-progress { + background: none; + } + + // Make the thumb easier to see. Like the SeekBar original styles, these need to be + // distinct. We also convert it into a line instead of a ball. + &::-webkit-slider-thumb { + width: 2px; + height: 30px; // height of waveform + background-color: $accent; + } + &::-moz-range-thumb { + width: 2px; + height: 30px; // height of waveform + background-color: $accent; + } + } + + // For timeline-rendered playback, the clock is on the other side of the waveform. + & + .mx_Clock { + text-align: right; + + // Take the padding off the clock because it's accounted for by the `timelineLayoutMiddle` + padding: 0; } } } From eb118f5940ba217cc2e74e624e8e5e17d2d53d05 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 13:49:33 -0600 Subject: [PATCH 04/10] Convert back to a ball --- .../views/audio_messages/_PlaybackContainer.scss | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/res/css/views/audio_messages/_PlaybackContainer.scss b/res/css/views/audio_messages/_PlaybackContainer.scss index 6930580d0c7..224aba50932 100644 --- a/res/css/views/audio_messages/_PlaybackContainer.scss +++ b/res/css/views/audio_messages/_PlaybackContainer.scss @@ -64,13 +64,16 @@ limitations under the License. position: absolute; left: 0; top: 0; + + // drop the whole waveform into the background a bit so the seek thumb can be seen + opacity: 0.6; } .mx_SeekBar { position: absolute; left: 0; height: 30px; - top: -1px; // visually vertically centered + top: -2px; // visually vertically centered // Hide the hairline progress bar. Need to have distinct rules because CSS is weird. background: none; @@ -82,15 +85,15 @@ limitations under the License. } // Make the thumb easier to see. Like the SeekBar original styles, these need to be - // distinct. We also convert it into a line instead of a ball. + // distinct. We also make it a bit bigger. &::-webkit-slider-thumb { - width: 2px; - height: 30px; // height of waveform + width: 10px; + height: 10px; background-color: $accent; } &::-moz-range-thumb { - width: 2px; - height: 30px; // height of waveform + width: 10px; + height: 10px; background-color: $accent; } } From 86536ed211f54d9bd2c989b1a2fc54f89605e344 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 14:10:56 -0600 Subject: [PATCH 05/10] Use `transparent` which makes NVDA happy enough --- .../views/audio_messages/_PlaybackContainer.scss | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/res/css/views/audio_messages/_PlaybackContainer.scss b/res/css/views/audio_messages/_PlaybackContainer.scss index 224aba50932..1411134c394 100644 --- a/res/css/views/audio_messages/_PlaybackContainer.scss +++ b/res/css/views/audio_messages/_PlaybackContainer.scss @@ -64,9 +64,6 @@ limitations under the License. position: absolute; left: 0; top: 0; - - // drop the whole waveform into the background a bit so the seek thumb can be seen - opacity: 0.6; } .mx_SeekBar { @@ -75,7 +72,8 @@ limitations under the License. height: 30px; top: -2px; // visually vertically centered - // Hide the hairline progress bar. Need to have distinct rules because CSS is weird. + // Hide the hairline progress bar since we're at 100% height. Need to have distinct rules + // because CSS is weird. background: none; &::before { background: none; @@ -85,16 +83,18 @@ limitations under the License. } // Make the thumb easier to see. Like the SeekBar original styles, these need to be - // distinct. We also make it a bit bigger. + // distinct. We make it transparent so it doesn't show up on the UI, but also larger + // so it's easier to grab by mouse users in some browsers. Most browsers let the user + // move and drag the thumb regardless of hitting the thumb, however. &::-webkit-slider-thumb { width: 10px; height: 10px; - background-color: $accent; + background-color: transparent; } &::-moz-range-thumb { width: 10px; height: 10px; - background-color: $accent; + background-color: transparent; } } From 9dc36baa506a3cc28141f7ff748746d7fd11c8c9 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 14:19:58 -0600 Subject: [PATCH 06/10] Allow keyboards in the seek bar --- src/components/views/audio_messages/RecordingPlayback.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/audio_messages/RecordingPlayback.tsx b/src/components/views/audio_messages/RecordingPlayback.tsx index b14b3818d2e..eb0c6cb28d0 100644 --- a/src/components/views/audio_messages/RecordingPlayback.tsx +++ b/src/components/views/audio_messages/RecordingPlayback.tsx @@ -55,7 +55,7 @@ export default class RecordingPlayback extends AudioPlayerBase { From 99b5f5c455b62f61fe75c27ad0de168eed00ff82 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 14:31:40 -0600 Subject: [PATCH 07/10] Try to make the clock behave more correctly with screen readers MIDNIGHT --- src/components/views/audio_messages/Clock.tsx | 6 +++--- src/components/views/audio_messages/PlaybackClock.tsx | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/views/audio_messages/Clock.tsx b/src/components/views/audio_messages/Clock.tsx index f978d8837c9..996738dc9a9 100644 --- a/src/components/views/audio_messages/Clock.tsx +++ b/src/components/views/audio_messages/Clock.tsx @@ -18,7 +18,7 @@ import React, { HTMLProps } from "react"; import { formatSeconds } from "../../../DateUtils"; -export interface IProps extends Pick, "aria-live"> { +export interface IProps extends Pick, "aria-live" | "role"> { seconds: number; } @@ -31,14 +31,14 @@ export default class Clock extends React.Component { super(props); } - shouldComponentUpdate(nextProps: Readonly): boolean { + public shouldComponentUpdate(nextProps: Readonly): boolean { const currentFloor = Math.floor(this.props.seconds); const nextFloor = Math.floor(nextProps.seconds); return currentFloor !== nextFloor; } public render() { - return + return { formatSeconds(this.props.seconds) } ; } diff --git a/src/components/views/audio_messages/PlaybackClock.tsx b/src/components/views/audio_messages/PlaybackClock.tsx index 3f05ad0b895..968445d91af 100644 --- a/src/components/views/audio_messages/PlaybackClock.tsx +++ b/src/components/views/audio_messages/PlaybackClock.tsx @@ -76,6 +76,7 @@ export default class PlaybackClock extends React.PureComponent { } return ; } From af4b323841e1dd609b0f8e6c56f67c8f7610695c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 14:38:07 -0600 Subject: [PATCH 08/10] Remove legacy export --- src/components/views/audio_messages/Clock.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/audio_messages/Clock.tsx b/src/components/views/audio_messages/Clock.tsx index 996738dc9a9..c5e6919ec89 100644 --- a/src/components/views/audio_messages/Clock.tsx +++ b/src/components/views/audio_messages/Clock.tsx @@ -18,7 +18,7 @@ import React, { HTMLProps } from "react"; import { formatSeconds } from "../../../DateUtils"; -export interface IProps extends Pick, "aria-live" | "role"> { +interface IProps extends Pick, "aria-live" | "role"> { seconds: number; } From a059059adf8eafa6ebf646f40f2e924fa6fbef22 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 14:39:17 -0600 Subject: [PATCH 09/10] Remove redundant attr --- src/components/views/audio_messages/PlaybackClock.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/views/audio_messages/PlaybackClock.tsx b/src/components/views/audio_messages/PlaybackClock.tsx index 968445d91af..c9da949ea1f 100644 --- a/src/components/views/audio_messages/PlaybackClock.tsx +++ b/src/components/views/audio_messages/PlaybackClock.tsx @@ -77,7 +77,6 @@ export default class PlaybackClock extends React.PureComponent { return ; } } From 827c3244811a7e2ba8fc1b4256af2b7596eeed0f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 14:45:11 -0600 Subject: [PATCH 10/10] Appease the linter --- src/components/views/audio_messages/RecordingPlayback.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/audio_messages/RecordingPlayback.tsx b/src/components/views/audio_messages/RecordingPlayback.tsx index eb0c6cb28d0..a65fc6e6a84 100644 --- a/src/components/views/audio_messages/RecordingPlayback.tsx +++ b/src/components/views/audio_messages/RecordingPlayback.tsx @@ -66,7 +66,7 @@ export default class RecordingPlayback extends AudioPlayerBase { protected renderComponent(): ReactNode { let body: ReactNode; - switch(this.props.layout) { + switch (this.props.layout) { case PlaybackLayout.Composer: body = this.renderComposerLook(); break;