From 39f2bbaaf4381e23439f34da0c8161bafc49f4e8 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 Jun 2022 18:13:13 -0600 Subject: [PATCH] Bring back waveform for voice messages and retain seeking (#8843) * Crude way of layering the waveform and seek bar Not intended for production. * Use a layout prop instead of something less descriptive * Fix alignment properly, and play with styles * Convert back to a ball * Use `transparent` which makes NVDA happy enough * Allow keyboards in the seek bar * Try to make the clock behave more correctly with screen readers MIDNIGHT * Remove legacy export * Remove redundant attr * Appease the linter --- .../audio_messages/_PlaybackContainer.scss | 49 +++++++++++++++++-- src/components/views/audio_messages/Clock.tsx | 6 +-- .../views/audio_messages/PlaybackClock.tsx | 2 +- .../audio_messages/RecordingPlayback.tsx | 47 +++++++++++++----- .../views/rooms/VoiceRecordComposerTile.tsx | 7 ++- .../audio_messages/RecordingPlayback-test.tsx | 38 +++++++++----- 6 files changed, 116 insertions(+), 33 deletions(-) diff --git a/res/css/views/audio_messages/_PlaybackContainer.scss b/res/css/views/audio_messages/_PlaybackContainer.scss index 4999980beaf..1411134c394 100644 --- a/res/css/views/audio_messages/_PlaybackContainer.scss +++ b/res/css/views/audio_messages/_PlaybackContainer.scss @@ -52,16 +52,57 @@ 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_RecordingPlayback_timelineLayoutMiddle { margin-left: 8px; margin-right: 6px; + position: relative; + display: inline-block; + flex: 1; + height: 30px; // same height as mx_Waveform, needed for automatic vertical centering + .mx_Waveform { + position: absolute; + left: 0; + top: 0; + } + + .mx_SeekBar { + position: absolute; + left: 0; + height: 30px; + top: -2px; // visually vertically centered + + // 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; + } + &::-moz-range-progress { + background: none; + } + + // Make the thumb easier to see. Like the SeekBar original styles, these need to be + // 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: transparent; + } + &::-moz-range-thumb { + width: 10px; + height: 10px; + background-color: transparent; + } + } + + // 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 in the seek bar + // Take the padding off the clock because it's accounted for by the `timelineLayoutMiddle` padding: 0; } } diff --git a/src/components/views/audio_messages/Clock.tsx b/src/components/views/audio_messages/Clock.tsx index f978d8837c9..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"> { +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..c9da949ea1f 100644 --- a/src/components/views/audio_messages/PlaybackClock.tsx +++ b/src/components/views/audio_messages/PlaybackClock.tsx @@ -76,7 +76,7 @@ export default class PlaybackClock extends React.PureComponent { } return ; } } diff --git a/src/components/views/audio_messages/RecordingPlayback.tsx b/src/components/views/audio_messages/RecordingPlayback.tsx index f9e84059588..a65fc6e6a84 100644 --- a/src/components/views/audio_messages/RecordingPlayback.tsx +++ b/src/components/views/audio_messages/RecordingPlayback.tsx @@ -22,37 +22,60 @@ 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; + Timeline, +} + +interface IProps extends IAudioPlayerBaseProps { + layout?: PlaybackLayout; // Defaults to Timeline layout } 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 <> ; } - 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.renderWaveformLook() : this.renderSeekableLook() } + { body }
); } diff --git a/src/components/views/rooms/VoiceRecordComposerTile.tsx b/src/components/views/rooms/VoiceRecordComposerTile.tsx index 97001315f41..ec23e3c77fc 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(); + }); }); });