Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Fix clock line break in voice message player on monospace font #8815

Closed
wants to merge 2 commits into from
Closed

Fix clock line break in voice message player on monospace font #8815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eaa1ba0
Fix clock line break on voice message player in monospace font
luixxiul Jun 15, 2022
4e7a176
Merge branch 'develop' into Player
luixxiul Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions res/css/_common.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,3 +806,25 @@ legend {
mask-repeat: no-repeat;
mask-size: contain;
}

@define-mixin AudioPlayer {
.mx_PlayPauseButton {
margin-inline-end: $spacing-8;
}

// For audio player and voice message player
.mx_SeekBar,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be tightly scoped?

.mx_RecordingPlayback_timelineLayoutMiddle {
flex: 1;

+ .mx_Clock {
margin-inline-start: $spacing-8;
}
}

.mx_Clock {
min-width: $font-42px; // for flexbox
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which flexbox?

text-align: justify;
white-space: nowrap;
}
}
21 changes: 4 additions & 17 deletions res/css/views/audio_messages/_AudioPlayer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@ limitations under the License.
*/

.mx_MediaBody.mx_AudioPlayer_container {
padding: 16px 12px 12px 12px;
@mixin AudioPlayer;

padding-top: $spacing-16;

.mx_AudioPlayer_primaryContainer {
display: flex;

.mx_PlayPauseButton {
margin-right: 8px;
}

.mx_AudioPlayer_mediaInfo {
flex: 1;
overflow: hidden; // makes the ellipsis on the file name work
Expand All @@ -39,7 +37,7 @@ limitations under the License.
text-overflow: ellipsis;
white-space: nowrap;
overflow: hidden;
padding-bottom: 4px; // mimics the line-height differences in the Figma
padding-bottom: $spacing-4; // mimics the line-height differences in the Figma
}

.mx_AudioPlayer_byline {
Expand All @@ -52,16 +50,5 @@ limitations under the License.
.mx_AudioPlayer_seek {
display: flex;
align-items: center;

.mx_SeekBar {
flex: 1;
}

.mx_Clock {
min-width: $font-42px; // for flexbox
padding-left: $spacing-4; // isolate from seek bar
text-align: justify;
white-space: nowrap;
}
}
}
35 changes: 14 additions & 21 deletions res/css/views/audio_messages/_PlaybackContainer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,17 @@ limitations under the License.

// Container for live recording and playback controls
.mx_MediaBody.mx_VoiceMessagePrimaryContainer {
// The waveform (right) has a 1px padding on it that we want to account for, otherwise
// inherit from mx_MediaBody
padding-right: 11px;
@mixin AudioPlayer;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we should be using a class rather than a mixin.


// Cheat at alignment a bit
display: flex;
align-items: center;

contain: content;

// Waveforms are present in live recording only
.mx_Waveform {
flex: 1; // Keep space for waveforms

.mx_Waveform_bar {
background-color: $quaternary-content;
height: 100%;
Expand All @@ -45,18 +44,9 @@ limitations under the License.
}
}

.mx_Clock {
width: $font-42px; // we're not using a monospace font, so fake it
min-width: $font-42px; // force sensible layouts in awkward flexboxes (file panel, for example)
padding-right: 6px; // with the fixed width this ends up as a visual 8px most of the time, as intended.
padding-left: 8px; // isolate from recording circle / play control
}

.mx_RecordingPlayback_timelineLayoutMiddle {
margin-left: 8px;
margin-right: 6px;
position: relative;
display: inline-block;
display: flex;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nested flexboxes end up going poorly on Safari/Firefox, and I don't think we need it here.

flex: 1;
height: 30px; // same height as mx_Waveform, needed for automatic vertical centering

Expand All @@ -69,7 +59,6 @@ limitations under the License.
position: absolute;
left: 0;
height: 30px;
top: -2px; // visually vertically centered
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was moving the seek bar 2px up, which is still needed.


// Hide the hairline progress bar since we're at 100% height. Need to have distinct rules
// because CSS is weird.
Expand All @@ -96,13 +85,17 @@ limitations under the License.
background-color: transparent;
}
}
}
}

// For timeline-rendered playback, the clock is on the other side of the waveform.
& + .mx_Clock {
text-align: right;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text-align was fixing a layout issue that is introduced by this PR where the clock ends up not matching the designs:
image

(this PR)

There should be no padding or margin on the right side of the clock.

.mx_MessageComposer_controls {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't be targeting the composer here - we need to be targeting the PlaybackContainer classes itself.

.mx_Waveform {
margin-inline-start: $spacing-8;
}

// Take the padding off the clock because it's accounted for by the `timelineLayoutMiddle`
padding: 0;
}
.mx_VoiceMessagePrimaryContainer {
// The waveform (right) has a 1px padding on it that we want to account for, otherwise
// inherit from mx_MediaBody
padding-right: 11px;
}
}