From b69b1e3d7474c9240d667bb876946951517cb4a7 Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 30 Sep 2026 20:17:51 +0200 Subject: [PATCH] fix(playback): address Up next card review feedback - Offer the Up next card setting for Embedded MPV only under the frame-copy engine; native view never mounts the shared controls. - Hovering pauses the collapse delay instead of restarting it. - Document that the card stays visible when the controls auto-hide. Co-Authored-By: Claude Opus 5.5 --- apps/web-e2e/src/xtream-series-playback.e2e.ts | 5 +---- .../settings-playback-section.component.html | 4 +++- .../settings-playback-section.component.spec.ts | 13 +++++++++++++ .../settings-playback-section.component.ts | 14 ++++++++++++++ docs/architecture/player-controls-contract.md | 5 +++-- .../player-up-next-card.component.scss | 6 +++--- .../player-up-next-card.component.spec.ts | 6 ++++-- .../player-up-next-card.component.ts | 17 +++++++++++++---- 8 files changed, 54 insertions(+), 16 deletions(-) diff --git a/apps/web-e2e/src/xtream-series-playback.e2e.ts b/apps/web-e2e/src/xtream-series-playback.e2e.ts index c3dd28a6e..80a5e7159 100644 --- a/apps/web-e2e/src/xtream-series-playback.e2e.ts +++ b/apps/web-e2e/src/xtream-series-playback.e2e.ts @@ -171,10 +171,7 @@ test.describe('@xtream inline series fullscreen', () => { }); // The corner button hides the card for this episode; bringing the - // controls back does not bring it back. The card hides with idle - // controls, so wake them first. - await playerView.hover(); - await expect(card).toBeVisible(); + // controls back does not bring it back. await playerView .locator('[data-test-id="player-controls-up-next-close"]') .click(); diff --git a/apps/web/src/app/settings/settings-playback-section.component.html b/apps/web/src/app/settings/settings-playback-section.component.html index 3df424143..c3f312fd3 100644 --- a/apps/web/src/app/settings/settings-playback-section.component.html +++ b/apps/web/src/app/settings/settings-playback-section.component.html @@ -137,7 +137,7 @@ } - @if (supportsFullscreenChannelPanel()) { + @if (supportsUpNextCard()) {
+ } + @if (supportsFullscreenChannelPanel()) {
{ expect( queryByTestId('fullscreen-channel-panel-setting') ).not.toBeNull(); + }); + + it('offers the up next card toggle for Embedded MPV only under the frame-copy engine', () => { + // The native-view engine never mounts the shared controls. + fixture.componentRef.setInput( + 'form', + createForm(VideoPlayer.EmbeddedMpv) + ); + fixture.detectChanges(); + expect(queryByTestId('player-up-next-card-setting')).toBeNull(); + + fixture.componentRef.setInput('frameCopyActive', true); + fixture.detectChanges(); expect(queryByTestId('player-up-next-card-setting')).not.toBeNull(); }); diff --git a/apps/web/src/app/settings/settings-playback-section.component.ts b/apps/web/src/app/settings/settings-playback-section.component.ts index c6d980340..6b0afd01f 100644 --- a/apps/web/src/app/settings/settings-playback-section.component.ts +++ b/apps/web/src/app/settings/settings-playback-section.component.ts @@ -89,6 +89,20 @@ export class SettingsPlaybackSectionComponent { ); } + /** + * The Up next card is part of the shared controls. Embedded MPV mounts + * them only under the frame-copy engine; the native-view engine keeps + * its own dock, where the toggle would control nothing. + */ + supportsUpNextCard(): boolean { + const value = this.form().value; + return ( + (this.isWebPlayerSelected() && + value.webPlayerSharedControls !== false) || + (value.player === VideoPlayer.EmbeddedMpv && this.frameCopyActive()) + ); + } + isExternalPlayerSelected(): boolean { const player = this.form().value.player; return player === VideoPlayer.MPV || player === VideoPlayer.VLC; diff --git a/docs/architecture/player-controls-contract.md b/docs/architecture/player-controls-contract.md index f127bc783..e90ef224f 100644 --- a/docs/architecture/player-controls-contract.md +++ b/docs/architecture/player-controls-contract.md @@ -424,8 +424,9 @@ routes such a request through the Up Next rail selection (`upNextEpisodeSelected`) whenever `seriesNavigation.canNext` is false, which plays the next season's first episode; either path keeps fullscreen exactly like the transport button. -The card is a glass surface that shows and hides with the controls; the -compact dock uses a smaller variant without the trailing icon. After +The card is a glass surface that stays visible when the controls auto-hide +(`showControls` is the host's input, not the idle state); the compact dock +uses a smaller variant without the trailing icon. After `UP_NEXT_COLLAPSE_DELAY_MS` (10 s, paused while hovered) the card asks to collapse and `ControlsUpNext.collapse()` turns it into a one-line pill (label and countdown) for the rest of the episode. A corner close button diff --git a/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.scss b/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.scss index 638303e60..61d18bded 100644 --- a/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.scss +++ b/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.scss @@ -1,7 +1,7 @@ // Glass card, bottom-right above the dock, inside the controls host (the -// `player-controls` container) whose `--pc-*` palette it reads. It shows and -// hides with the controls, collapses to a pill once seen, and the corner -// close button dismisses it for the episode. +// `player-controls` container) whose `--pc-*` palette it reads. It does not +// fade with idle controls; instead it collapses to a pill once seen, and the +// corner close button dismisses it for the episode. :host { position: absolute; right: 32px; diff --git a/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.spec.ts b/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.spec.ts index ddce54ef3..31461cf0b 100644 --- a/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.spec.ts +++ b/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.spec.ts @@ -152,7 +152,7 @@ describe('PlayerUpNextCardComponent', () => { expect(documentListener).not.toHaveBeenCalled(); }); - it('asks to collapse after a while, but not while hovered', () => { + it('asks to collapse after a while, pausing the count while hovered', () => { jest.useFakeTimers(); fixture.destroy(); fixture = TestBed.createComponent(PlayerUpNextCardComponent); @@ -167,14 +167,16 @@ describe('PlayerUpNextCardComponent', () => { const collapse = jest.fn(); fixture.componentInstance.collapseRequested.subscribe(collapse); + jest.advanceTimersByTime(UP_NEXT_COLLAPSE_DELAY_MS - 1000); fixture.nativeElement.dispatchEvent(new MouseEvent('mouseenter')); fixture.detectChanges(); jest.advanceTimersByTime(UP_NEXT_COLLAPSE_DELAY_MS * 2); expect(collapse).not.toHaveBeenCalled(); + // Leaving resumes the remaining second instead of a fresh delay. fixture.nativeElement.dispatchEvent(new MouseEvent('mouseleave')); fixture.detectChanges(); - jest.advanceTimersByTime(UP_NEXT_COLLAPSE_DELAY_MS - 1); + jest.advanceTimersByTime(999); expect(collapse).not.toHaveBeenCalled(); jest.advanceTimersByTime(1); expect(collapse).toHaveBeenCalledTimes(1); diff --git a/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.ts b/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.ts index 21425ad0a..b729cc6b6 100644 --- a/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.ts +++ b/libs/ui/playback/src/lib/player-controls/player-up-next-card.component.ts @@ -18,8 +18,8 @@ export const UP_NEXT_COLLAPSE_DELAY_MS = 10_000; * The "Up next" card in the player's bottom-right corner: thumbnail, the * time left, the next episode's label and title. One click plays it through * the host's ordinary next-episode path, so fullscreen survives. After a few - * seconds it asks to collapse into a pill (never while hovered), and the - * close button or Escape dismisses it for this episode. + * seconds of being seen (hovering pauses the count) it asks to collapse into + * a pill, and the close button or Escape dismisses it for this episode. */ @Component({ selector: 'app-player-up-next-card', @@ -46,6 +46,8 @@ export class PlayerUpNextCardComponent { readonly collapseRequested = output(); readonly hovered = signal(false); + /** Collapse delay still owed; hovering pauses it rather than resetting. */ + private collapseRemainingMs = UP_NEXT_COLLAPSE_DELAY_MS; /** Whole seconds under a minute, otherwise whole minutes (at least 1). */ readonly countdown = computed(() => { @@ -73,11 +75,18 @@ export class PlayerUpNextCardComponent { if (this.collapsed() || this.hovered()) { return; } + const startedAt = Date.now(); const timer = setTimeout( () => this.collapseRequested.emit(), - UP_NEXT_COLLAPSE_DELAY_MS + this.collapseRemainingMs ); - onCleanup(() => clearTimeout(timer)); + onCleanup(() => { + clearTimeout(timer); + this.collapseRemainingMs = Math.max( + 0, + this.collapseRemainingMs - (Date.now() - startedAt) + ); + }); }); }