fix(playback): address review findings on the Hybrid player controls

- Wide dock overflow: chips and the side settings panel now need 960px of
  player width (ControlsLayout.roomy). Between 720px and 960px the dock
  stays wide but folds the chips into `tune` with state dots and opens
  settings as the bottom sheet. The control row's side columns are
  minmax(min-content, 1fr), so the transport shifts instead of the actions
  overlapping it or leaving the player; chip labels cap at 132px.
- Up next: no countdown once playback has ended (autoplay off schedules
  no switch).
- Up next card: a still that fails to load falls back to the label tile;
  a new URL gets its own attempt.
- Timeline segments are positioned absolutely at their time positions,
  so drawn boundaries match the linear seek input and hover label.
- Type fixes found by a strict type-check: the gap-fill list type, and a
  field initializer that read a constructor parameter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5.5 committed 2026-09-26 23:40:55 +02:00
1 parent d0c1d49aee
commit ea25c739f5
17 files changed
+263 -50

No files matched your search

@@ -109,6 +109,8 @@ test('@web @playback settings panel opens from the speed chip and applies in pla
page,
}) => {
test.setTimeout(90_000);
// Chips and the side panel need a player of at least 960px.
await page.setViewportSize({ width: 1600, height: 1000 });
await serveClip(page);
await selectHtml5Player(page);
await importPlaylist(page);
+22 -5
View File
@@ -343,7 +343,11 @@ app's `--app-selection-color` is a different blue that would fight the video.
The track is drawn as a row of segments, one flex item per segment with
`flex-grow` equal to its share of the duration and its own accent fill, so
a film's chapters or a catch-up recording's programmes read directly off
the bar. The optional `timelineSegments` input
the bar. Each segment is placed absolutely at its time position (`left` =
start percent, `width` = share minus the 3px gap every segment but the
last keeps), so a drawn boundary sits exactly where the linear seek input
and the hover label change segment; a segment shorter than the gap
collapses instead of pushing its neighbours. The optional `timelineSegments` input
(`PlayerTimelineSegment { startSeconds, endSeconds, title }`) supplies
them; `normalizeTimelineSegments` (`controls-timeline-segments.ts`) clamps
to the duration, orders, drops empty and reversed entries, cuts overlaps at
@@ -366,8 +370,10 @@ watched, "Up next · in 7 min" and the title. The host supplies the item
through the optional `upNext` input (`PlayerUpNextItem { label, title,
thumbnailUrl, progressPercent }`); `ControlsUpNext` decides when it shows —
`seriesNavigation` capability, `canNextEpisode`, a finite duration with at
most `UP_NEXT_THRESHOLD_SECONDS` (8 min) left, not live, controls shown,
most `UP_NEXT_THRESHOLD_SECONDS` (8 min) left, not live, not `ended` (with
autoplay off nothing is scheduled, so no countdown), controls shown,
settings panel closed — and how many minutes remain (never below one). A
still that fails to load falls back to the label tile. A
click emits `nextEpisodeRequested`, the same output the hosts already
handle, so the switch keeps fullscreen exactly like the transport button.
The card is a glass surface that does not fade with the controls; the
@@ -393,7 +399,7 @@ and owns open/toggle/close; the `tune` button and the panel render only
while at least one group exists, and the availability reconciliation closes
the panel the moment the last group disappears.
- **Wide dock**: two **value chips** precede `tune` — subtitles
- **Roomy wide dock** (≥ 960px): two **value chips** precede `tune` — subtitles
(`closed_caption` + the selected track's label, or "Off") and speed
(`speed` + `1.25×`). Audio and aspect ratio have no chip: they are
panel-only. A chip click opens the panel **focused on its group**
@@ -405,7 +411,7 @@ the panel the moment the last group disappears.
corner shift left by the panel's width (`--panel-open` modifiers,
`right: 370px`), the chips and the picture-in-picture / recording buttons
fold away, `tune` fills in the accent color, and fullscreen stays.
- **Compact dock**: no chips; `tune` carries **state dots** (5px, cyan when
- **Compact dock, and wide docks below 960px**: no chips; `tune` carries **state dots** (5px, cyan when
subtitles are on or a non-default audio track is selected, violet when
speed, aspect or manual quality differ from their default) and the panel
opens as a **bottom sheet** (`--sheet` modifier: grip, two-column rows,
@@ -1038,8 +1044,19 @@ places that must agree: the `@container player-controls (max-width: 719px)`
block in the stylesheet sizes the compact dock (14px gutters, 32px buttons,
36px play circle, 5px track), and `ControlsLayout`
(`COMPACT_LAYOUT_MAX_WIDTH`, a `ResizeObserver` on the host) drives the
template branches CSS cannot express — today the inline volume slider versus
template branches CSS cannot express — the inline volume slider versus
its popover. Without `ResizeObserver` (unit tests) the mode stays `wide`.
A second threshold, `ROOMY_LAYOUT_MIN_WIDTH` (960px, `ControlsLayout.roomy`),
gates the wide dock's extras: the subtitle/speed chips and the settings
panel beside the video. Between 720px and 960px the dock stays wide (full
button sizes, inline volume) but folds the chips into `tune` with state
dots and opens settings as the bottom sheet, because the widest action row
(volume, series transport, two chips, tune/record/PiP/fullscreen) and the
dock beside a 370px panel do not fit there. The control row's side columns
are `minmax(min-content, 1fr)`, so if the actions still need more than half
of what the transport leaves, the transport slides off-centre instead of
the actions overlapping it or leaving the player.
Episode navigation stays in the compact transport: the series hosts rely on
those buttons, and the inline series player is often narrower than 720px.
@@ -1,4 +1,8 @@
import { COMPACT_LAYOUT_MAX_WIDTH, ControlsLayout } from './controls-layout';
import {
COMPACT_LAYOUT_MAX_WIDTH,
ControlsLayout,
ROOMY_LAYOUT_MIN_WIDTH,
} from './controls-layout';
type ResizeCallback = (entries: ResizeObserverEntry[]) => void;
@@ -61,6 +65,23 @@ describe('ControlsLayout', () => {
expect(layout.mode()).toBe('wide');
});
it('reports room for chips and the side panel only from the roomy width', () => {
const layout = new ControlsLayout();
layout.attach(document.createElement('div'));
expect(layout.roomy()).toBe(true);
callbacks[0]([entry(ROOMY_LAYOUT_MIN_WIDTH - 1)]);
expect(layout.mode()).toBe('wide');
expect(layout.roomy()).toBe(false);
callbacks[0]([entry(ROOMY_LAYOUT_MIN_WIDTH)]);
expect(layout.roomy()).toBe(true);
callbacks[0]([entry(400)]);
expect(layout.mode()).toBe('compact');
expect(layout.roomy()).toBe(false);
});
it('uses the last entry of a batch and ignores zero widths', () => {
const layout = new ControlsLayout();
layout.attach(document.createElement('div'));
@@ -10,6 +10,17 @@ import { signal } from '@angular/core';
*/
export const COMPACT_LAYOUT_MAX_WIDTH = 719;
/**
* Container width (px) from which the wide dock has room for its extras:
* the subtitle/speed value chips beside the action buttons, and the
* settings panel beside the video (the panel takes 370px of the dock).
* Between the compact breakpoint and this width the dock stays wide but
* folds the chips into `tune` and opens settings as a bottom sheet.
* Sized from the widest action row: volume + series transport + two chips
* + tune/record/PiP/fullscreen, and dock + panel with only tune/fullscreen.
*/
export const ROOMY_LAYOUT_MIN_WIDTH = 960;
export type ControlsLayoutMode = 'compact' | 'wide';
/**
@@ -23,6 +34,8 @@ export type ControlsLayoutMode = 'compact' | 'wide';
*/
export class ControlsLayout {
readonly mode = signal<ControlsLayoutMode>('wide');
/** Room for chips and the side panel; see {@link ROOMY_LAYOUT_MIN_WIDTH}. */
readonly roomy = signal(true);
private observer: ResizeObserver | null = null;
attach(host: HTMLElement): void {
@@ -53,6 +66,10 @@ export class ControlsLayout {
if (this.mode() !== next) {
this.mode.set(next);
}
const roomy = width >= ROOMY_LAYOUT_MIN_WIDTH;
if (this.roomy() !== roomy) {
this.roomy.set(roomy);
}
}
detach(): void {
@@ -53,7 +53,7 @@ export class ControlsSettings {
() => this.available() && this.deps.menus.settingsOpen()
);
readonly focusGroup = this.deps.menus.settingsFocus;
readonly focusGroup = computed(() => this.deps.menus.settingsFocus());
readonly subtitlesOn = computed(
() => this.groups().subtitles && this.deps.state().subtitlesEnabled
@@ -7,7 +7,12 @@ import {
describe('normalizeTimelineSegments', () => {
it('renders one untitled segment without input or duration', () => {
expect(normalizeTimelineSegments(null, 600)).toEqual([
{ startSeconds: 0, endSeconds: 600, title: null, share: 1 },
expect.objectContaining({
startSeconds: 0,
endSeconds: 600,
title: null,
share: 1,
}),
]);
expect(normalizeTimelineSegments([], 600)).toHaveLength(1);
expect(
@@ -15,7 +20,9 @@ describe('normalizeTimelineSegments', () => {
[{ startSeconds: 0, endSeconds: 10, title: 'Intro' }],
0
)
).toEqual([{ startSeconds: 0, endSeconds: 0, title: null, share: 1 }]);
).toEqual([
expect.objectContaining({ startSeconds: 0, title: null, share: 1 }),
]);
expect(
normalizeTimelineSegments([], Number.POSITIVE_INFINITY)
).toHaveLength(1);
@@ -31,26 +38,52 @@ describe('normalizeTimelineSegments', () => {
);
expect(segments).toEqual([
{ startSeconds: 0, endSeconds: 60, title: null, share: 0.1 },
{
expect.objectContaining({
startSeconds: 0,
endSeconds: 60,
title: null,
share: 0.1,
}),
expect.objectContaining({
startSeconds: 60,
endSeconds: 300,
title: 'Chapter 1',
share: 0.4,
},
{
}),
expect.objectContaining({
startSeconds: 300,
endSeconds: 450,
title: 'Chapter 2',
share: 0.25,
},
{ startSeconds: 450, endSeconds: 600, title: null, share: 0.25 },
}),
expect.objectContaining({
startSeconds: 450,
endSeconds: 600,
title: null,
share: 0.25,
}),
]);
expect(
segments.reduce((total, segment) => total + segment.share, 0)
).toBeCloseTo(1);
});
it('places segments at their time positions with a gap after all but the last', () => {
const segments = normalizeTimelineSegments(
[
{ startSeconds: 0, endSeconds: 150, title: 'A' },
{ startSeconds: 150, endSeconds: 600, title: 'B' },
],
600
);
expect(segments.map((s) => s.startPercent)).toEqual([0, 25]);
expect(segments.map((s) => s.width)).toEqual([
'max(0px, calc(25% - 3px))',
'75%',
]);
});
it('clamps to the duration, cuts overlaps and drops empty segments', () => {
const segments = normalizeTimelineSegments(
[
@@ -5,15 +5,24 @@ export interface TimelineSegmentView {
startSeconds: number;
endSeconds: number;
title: string | null;
/** Share of the whole duration, 0..1; drives the segment's flex-grow. */
/** Share of the whole duration, 0..1. */
share: number;
/** Left edge on the track, 0..100 — the same mapping the seek input uses. */
startPercent: number;
/** CSS width: the share, minus the gap every segment but the last keeps. */
width: string;
}
/** Visual gap (px) after every segment except the last. */
export const TIMELINE_SEGMENT_GAP_PX = 3;
const WHOLE_TIMELINE: TimelineSegmentView = {
startSeconds: 0,
endSeconds: 0,
title: null,
share: 1,
startPercent: 0,
width: '100%',
};
/**
@@ -46,7 +55,10 @@ export function normalizeTimelineSegments(
.filter((segment) => segment.endSeconds > segment.startSeconds)
.sort((a, b) => a.startSeconds - b.startSeconds);
const cover: Omit<TimelineSegmentView, 'share'>[] = [];
const cover: Pick<
TimelineSegmentView,
'startSeconds' | 'endSeconds' | 'title'
>[] = [];
let cursor = 0;
for (const segment of ordered) {
const startSeconds = Math.max(segment.startSeconds, cursor);
@@ -70,10 +82,19 @@ export function normalizeTimelineSegments(
title: null,
});
}
return cover.map((segment) => ({
...segment,
share: (segment.endSeconds - segment.startSeconds) / durationSeconds,
}));
return cover.map((segment, index) => {
const share =
(segment.endSeconds - segment.startSeconds) / durationSeconds;
const last = index === cover.length - 1;
return {
...segment,
share,
startPercent: (segment.startSeconds / durationSeconds) * 100,
width: last
? `${share * 100}%`
: `max(0px, calc(${share * 100}% - ${TIMELINE_SEGMENT_GAP_PX}px))`,
};
});
}
/** How much of one segment the position has played through, 0..100. */
@@ -117,6 +117,16 @@ describe('ControlsUpNext', () => {
expect(upNext.visible()).toBe(false);
});
it('does not count down once the episode has ended', () => {
setState({
status: 'ended',
canNextEpisode: true,
durationSeconds: 1200,
positionSeconds: 1200,
});
expect(upNext.visible()).toBe(false);
});
it('yields to hidden controls and to the open settings panel', () => {
showControls.set(false);
expect(upNext.visible()).toBe(false);
@@ -47,6 +47,9 @@ export class ControlsUpNext {
remaining === null ||
remaining > UP_NEXT_THRESHOLD_SECONDS ||
state.isLive ||
// An ended episode with autoplay off schedules no switch: a
// countdown would promise one. Autoplay replaces the playback.
state.status === 'ended' ||
!state.canNextEpisode ||
!this.deps.capabilities().seriesNavigation ||
!this.deps.showControls() ||
@@ -317,11 +317,13 @@ describe('PlayerControlsComponent dock', () => {
expect(
segments.map((s) => s.dataset['segmentTitle'] ?? null)
).toEqual(['Chapter 1', 'Chapter 2', null]);
expect(segments.map((s) => s.style.flexGrow)).toEqual([
'0.25',
'0.5',
'0.25',
// Positioned by time, so boundaries match the linear seek input.
expect(segments.map((s) => s.style.left)).toEqual([
'0%',
'25%',
'75%',
]);
expect(segments.at(-1)?.style.width).toBe('25%');
expect(
segments.map(
(s) =>
@@ -23,7 +23,7 @@
class="player-controls__title"
[class.player-controls__title--visible]="controlsAreVisible()"
[class.player-controls__title--panel-open]="
settings.isOpen() && !isCompact()
settings.isOpen() && layout.roomy()
"
data-test-id="player-controls-media-title"
>
@@ -43,7 +43,7 @@
class="player-controls__corner"
[class.player-controls__corner--visible]="controlsAreVisible()"
[class.player-controls__corner--panel-open]="
settings.isOpen() && !isCompact()
settings.isOpen() && layout.roomy()
"
(pointerenter)="chrome.onPointerEnter()"
(pointerleave)="chrome.onPointerLeave()"
@@ -123,10 +123,10 @@
[class.player-controls__bar--visible]="controlsAreVisible()"
[class.player-controls__bar--compact]="isCompact()"
[class.player-controls__bar--panel-open]="
settings.isOpen() && !isCompact()
settings.isOpen() && layout.roomy()
"
[class.player-controls__bar--sheet-open]="
settings.isOpen() && isCompact()
settings.isOpen() && !layout.roomy()
"
(pointerenter)="chrome.onPointerEnter()"
(pointerleave)="chrome.onPointerLeave()"
@@ -160,7 +160,8 @@
[class.player-controls__timeline-segment--titled]="
segment.title !== null
"
[style.flex-grow]="segment.share"
[style.left.%]="segment.startPercent"
[style.width]="segment.width"
[attr.data-segment-title]="segment.title"
>
<div
@@ -466,7 +467,7 @@
<div class="player-controls__actions">
@if (
settings.available() && !isCompact() && !settings.isOpen()
settings.available() && layout.roomy() && !settings.isOpen()
) {
@if (settings.groups().subtitles) {
<button
@@ -548,7 +549,7 @@
>
<mat-icon>tune</mat-icon>
@if (
isCompact() &&
!layout.roomy() &&
!settings.isOpen() &&
settings.hasDots()
) {
@@ -572,7 +573,7 @@
</button>
}
@if (canRecord() && !(settings.isOpen() && !isCompact())) {
@if (canRecord() && !(settings.isOpen() && layout.roomy())) {
<button
mat-icon-button
type="button"
@@ -607,7 +608,7 @@
@if (
capabilities().pictureInPicture &&
!(settings.isOpen() && !isCompact())
!(settings.isOpen() && layout.roomy())
) {
<button
mat-icon-button
@@ -671,13 +672,13 @@
@if (settings.isOpen()) {
<app-player-settings-panel
class="player-controls__settings"
[class.player-controls__settings--sheet]="isCompact()"
[class.player-controls__settings--sheet]="!layout.roomy()"
data-test-id="player-controls-settings-panel"
[controller]="controller()"
[settings]="settings"
[selection]="menuSelection"
[subtitleSettings]="subtitleSettings"
[mode]="isCompact() ? 'sheet' : 'panel'"
[mode]="layout.roomy() ? 'panel' : 'sheet'"
[title]="'EMBEDDED_MPV.PLAYER.SETTINGS' | translate"
(closeRequested)="settings.close()"
(pointerenter)="chrome.onPointerEnter()"
@@ -377,19 +377,18 @@
top: 50%;
right: 0;
left: 0;
display: flex;
gap: 3px;
height: 6px;
transform: translateY(-50%);
}
// One flex item per segment; `flex-grow` carries the segment's share of the
// duration, so widths are proportional to time. A tiny segment still gets a
// visible sliver rather than vanishing between the 3px gaps.
// Segments sit at their exact time positions (`left` = start, `width` =
// share, minus the 3px gap at each segment's end), so a drawn boundary is
// where the linear seek input and the hover label change segment. A segment
// shorter than the gap simply collapses rather than shifting its neighbours.
.player-controls__timeline-segment {
position: relative;
flex: 1 1 0;
min-width: 4px;
position: absolute;
top: 0;
bottom: 0;
overflow: hidden;
border-radius: 3px;
background: var(--pc-track);
@@ -512,9 +511,16 @@
// --- Control row ------------------------------------------------------------
// Each side column keeps at least its own content width: when the action
// cluster needs more than half of what the transport leaves, the transport
// slides off-centre instead of the actions overlapping it or leaving the
// player. Which extras exist at all is decided by `ControlsLayout`.
.player-controls__row {
display: grid;
grid-template-columns: minmax(0, 1fr) auto minmax(0, 1fr);
grid-template-columns: minmax(min-content, 1fr) auto minmax(
min-content,
1fr
);
gap: 8px;
align-items: center;
}
@@ -563,7 +569,7 @@
align-items: center;
gap: 6px;
height: 32px;
max-width: 160px;
max-width: 132px;
padding: 0 11px;
color: var(--pc-text);
background: transparent;
@@ -1,7 +1,10 @@
import { WritableSignal, signal } from '@angular/core';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { TranslateModule, TranslateService } from '@ngx-translate/core';
import { COMPACT_LAYOUT_MAX_WIDTH } from './controls-layout';
import {
COMPACT_LAYOUT_MAX_WIDTH,
ROOMY_LAYOUT_MIN_WIDTH,
} from './controls-layout';
import {
DEFAULT_PLAYER_CAPABILITIES,
createEmptyControlsState,
@@ -314,6 +317,49 @@ describe('PlayerControlsComponent settings panel', () => {
});
});
describe('wide dock without room for extras', () => {
beforeEach(() => {
withEverything();
resizeTo(ROOMY_LAYOUT_MIN_WIDTH - 1);
});
it('stays wide but folds the chips into tune and opens a sheet', () => {
expect(query('.player-controls__bar')?.classList).not.toContain(
'player-controls__bar--compact'
);
expect(query('.player-controls__slider--inline')).not.toBeNull();
expect(
query('[data-test-id="player-controls-speed-chip"]')
).toBeNull();
expect(
query('[data-test-id="player-controls-subtitle-chip"]')
).toBeNull();
setState({
audioTracks: fake.state().audioTracks,
subtitleTracks: [{ id: 5, label: 'Russian', selected: true }],
subtitlesEnabled: true,
});
fixture.detectChanges();
expect(
query('[data-test-id="player-controls-settings-dots"]')
).not.toBeNull();
query('[data-test-id="player-controls-settings-button"]')?.click();
fixture.detectChanges();
expect(
query('[data-test-id="player-controls-settings-panel"]')
?.classList
).toContain('player-controls__settings--sheet');
expect(query('.player-controls__bar')?.classList).toContain(
'player-controls__bar--sheet-open'
);
expect(query('.player-controls__bar')?.classList).not.toContain(
'player-controls__bar--panel-open'
);
});
});
describe('compact dock', () => {
beforeEach(() => {
withEverything();
@@ -89,7 +89,7 @@ export class PlayerControlsComponent implements OnDestroy {
readonly feedback = new ControlsFeedback();
readonly anyMenuOpen = this.menus.anyOpen;
private readonly shortcuts = new ControlsShortcuts();
private readonly layout = new ControlsLayout();
readonly layout = new ControlsLayout();
/** Compact dock: narrow inline players and phone-sized viewports. */
readonly isCompact = computed(() => this.layout.mode() === 'compact');
private readonly visibility = new ControlsVisibility(() => this.canHide());
@@ -11,12 +11,13 @@
"
>
<span class="player-up-next__thumb" aria-hidden="true">
@if (item().thumbnailUrl) {
@if (thumbnail(); as src) {
<img
class="player-up-next__image"
[src]="item().thumbnailUrl"
[src]="src"
alt=""
loading="lazy"
(error)="onThumbnailError()"
/>
} @else {
<span class="player-up-next__placeholder">{{ item().label }}</span>
@@ -78,6 +78,26 @@ describe('PlayerUpNextCardComponent', () => {
);
});
it('falls back to the label tile when the still fails, and retries a new URL', () => {
query('.player-up-next__image')?.dispatchEvent(new Event('error'));
fixture.detectChanges();
expect(query('.player-up-next__image')).toBeNull();
expect(query('.player-up-next__placeholder')?.textContent?.trim()).toBe(
'S01E03'
);
fixture.componentRef.setInput('item', {
label: 'S01E04',
title: 'Four',
thumbnailUrl: 'https://img.example/other.jpg',
progressPercent: null,
});
fixture.detectChanges();
expect(query('.player-up-next__image')?.getAttribute('src')).toBe(
'https://img.example/other.jpg'
);
});
it('emits on click', () => {
const selected = jest.fn();
fixture.componentInstance.selected.subscribe(selected);
@@ -1,8 +1,10 @@
import {
ChangeDetectionStrategy,
Component,
computed,
input,
output,
signal,
} from '@angular/core';
import { MatIconModule } from '@angular/material/icon';
import { TranslatePipe } from '@ngx-translate/core';
@@ -29,4 +31,15 @@ export class PlayerUpNextCardComponent {
readonly minutesLeft = input.required<number>();
readonly compact = input(false);
readonly selected = output<void>();
/** The still that failed to load; a new URL gets its own attempt. */
private readonly failedThumbnail = signal<string | null>(null);
readonly thumbnail = computed(() => {
const url = this.item().thumbnailUrl;
return url && url !== this.failedThumbnail() ? url : null;
});
onThumbnailError(): void {
this.failedThumbnail.set(this.item().thumbnailUrl);
}
}