fix(workspace): address hero review feedback

- Key hero TMDB extras by the TMDB language so a language change reloads
  the localized overview and genres (memo, requested keys and the loading
  effect all track it).
- Pin the first visible slide's id, so a live slide arriving ahead of it
  never replaces what the user is viewing; if the active slide goes away
  the one now at its position takes over.
- The "most recent" fallback can be a finished title: open its details
  instead of offering "Continue".
- Use a min-height flex layout with the slide in normal flow, so an
  unusually full slide grows the banner instead of clipping its top.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
4grayandClaude Opus 5.5 committed 2026-09-27 18:04:59 +02:00
1 parent 2b28b5ac84
commit 7d4213026e
7 files changed
+170 -17

No files matched your search

@@ -90,6 +90,7 @@ describe('DashboardHeroSlidesPresenter', () => {
let candidates: ReturnType<typeof signal<DashboardHeroLiveCandidate[]>>;
let liveDetails: jest.Mock;
let tmdbEnabled: ReturnType<typeof signal<boolean>>;
let tmdbLanguage: ReturnType<typeof signal<string>>;
let getExtras: jest.Mock;
const positions = new Map<string | number, PlaybackPositionData>([
[
@@ -167,7 +168,8 @@ describe('DashboardHeroSlidesPresenter', () => {
provide: DashboardHeroTmdbService,
useValue: {
isEnabled: () => tmdbEnabled(),
keyFor: (item: PortalActivityItem) => item.title,
keyFor: (item: PortalActivityItem) =>
`${tmdbLanguage()}//${item.title}`,
getExtras,
},
},
@@ -191,6 +193,7 @@ describe('DashboardHeroSlidesPresenter', () => {
candidates = signal([{ origin: 'favorite', item: channel }]);
liveDetails = jest.fn(() => onAir);
tmdbEnabled = signal(false);
tmdbLanguage = signal('en-US');
getExtras = jest.fn().mockResolvedValue(null);
});
@@ -318,6 +321,56 @@ describe('DashboardHeroSlidesPresenter', () => {
});
});
it('loads the extras again when the TMDB language changes', async () => {
tmdbEnabled.set(true);
getExtras.mockImplementation((item: PortalActivityItem) =>
Promise.resolve(
item.title === 'Big Pharma'
? {
backdropUrl: null,
rating: '7.5',
genres: [
tmdbLanguage() === 'en-US' ? 'Drama' : 'Драма',
],
overview: `plot in ${tmdbLanguage()}`,
year: 2024,
}
: null
)
);
const presenter = create();
TestBed.tick();
await Promise.resolve();
expect(presenter.slides()[0].description).toBe('plot in en-US');
tmdbLanguage.set('ru-RU');
TestBed.tick();
await Promise.resolve();
expect(getExtras).toHaveBeenCalledTimes(6);
expect(presenter.slides()[0]).toMatchObject({
description: 'plot in ru-RU',
genres: ['Драма'],
});
});
it('opens a finished fallback title on its details, never "Continue"', () => {
recentItems.set([watchedMovie]);
favorites.set([]);
addedItems.set([]);
candidates.set([]);
const [fallback] = create().slides();
expect(fallback).toMatchObject({
kind: 'recent',
primaryAction: {
labelKey: 'WORKSPACE.DASHBOARD.HERO_DETAILS',
state: { resume: false },
},
secondaryAction: null,
});
});
it('falls back to the generated stage when an image fails to load', () => {
const presenter = create();
presenter.markImageFailed('https://img/pharma-poster.jpg');
@@ -130,10 +130,17 @@ export class DashboardHeroSlidesPresenter {
if (!this.heroTmdb.isEnabled()) {
return;
}
const items = this.sources()
// Keys read here, tracked: they carry the TMDB language, so a
// language change loads the localized overview and genres.
const requests = this.sources()
.map((source) => source.item)
.filter((item) => item.type !== 'live');
untracked(() => items.forEach((item) => this.loadTmdbExtras(item)));
.filter((item) => item.type !== 'live')
.map((item) => ({ item, key: this.heroTmdb.keyFor(item) }));
untracked(() =>
requests.forEach(({ item, key }) =>
this.loadTmdbExtras(item, key)
)
);
});
}
@@ -143,8 +150,7 @@ export class DashboardHeroSlidesPresenter {
);
}
private loadTmdbExtras(item: PortalActivityItem): void {
const key = this.heroTmdb.keyFor(item);
private loadTmdbExtras(item: PortalActivityItem, key: string): void {
if (this.requestedTmdbKeys.has(key)) {
return;
}
@@ -247,7 +253,7 @@ export class DashboardHeroSlidesPresenter {
/**
* Resume slides keep the hero's resume handoff (a saved series episode
* auto-plays) and offer "Details" as the detail-only way in; discovery
* slides open the detail page; live slides open the channel.
* and fallback slides open the detail page; live slides open the channel.
*/
private actionsFor(
source: DashboardHeroSource,
@@ -299,6 +305,17 @@ export class DashboardHeroSlidesPresenter {
secondaryAction: null,
};
}
// The fallback row can be a finished title (the resume
// candidates skip those): nothing to continue, open details.
if (source.kind === 'recent') {
return {
primaryAction: detailsAction(
link,
this.data.getRecentItemDetailNavigationState(item)
),
secondaryAction: null,
};
}
const canResume =
this.data.getRecentItemResumeNavigation(item) !== null;
return {
@@ -56,6 +56,7 @@ describe('DashboardHeroTmdbService', () => {
} as unknown as DashboardHeroTmdbItem;
let isEnabled: jest.Mock;
let language: jest.Mock;
let enrichMovie: jest.Mock;
let enrichTv: jest.Mock;
@@ -64,7 +65,7 @@ describe('DashboardHeroTmdbService', () => {
providers: [
{
provide: TmdbEnrichmentService,
useValue: { isEnabled, enrichMovie, enrichTv },
useValue: { isEnabled, language, enrichMovie, enrichTv },
},
],
});
@@ -73,6 +74,7 @@ describe('DashboardHeroTmdbService', () => {
beforeEach(() => {
isEnabled = jest.fn().mockReturnValue(true);
language = jest.fn().mockReturnValue('en-US');
enrichMovie = jest.fn().mockResolvedValue(null);
enrichTv = jest.fn().mockResolvedValue(tvDetails);
});
@@ -115,6 +117,21 @@ describe('DashboardHeroTmdbService', () => {
});
});
it('loads the extras again for another TMDB language', async () => {
const service = createService();
const item: DashboardHeroTmdbItem = { title: 'Serial', type: 'series' };
await service.getExtras(item);
const englishKey = service.keyFor(item);
await service.getExtras(item);
expect(enrichTv).toHaveBeenCalledTimes(1);
language.mockReturnValue('ru-RU');
expect(service.keyFor(item)).not.toBe(englishKey);
await service.getExtras(item);
expect(enrichTv).toHaveBeenCalledTimes(2);
});
it('reports a missing overview and year as null', async () => {
const service = createService();
@@ -54,10 +54,13 @@ export class DashboardHeroTmdbService {
/**
* Identity of the lookup for an item — the memo key, and the staleness
* guard callers compare against while a request is in flight.
* guard callers compare against while a request is in flight. The TMDB
* language is part of it: the overview and genre names are localized,
* so a language change must load them again. Reactive when read inside
* a computed or effect (settings signal underneath).
*/
keyFor(item: DashboardHeroTmdbItem): string {
return dashboardTmdbLookupKey(item);
return `${this.enrichment.language()}//${dashboardTmdbLookupKey(item)}`;
}
getExtras(
@@ -43,7 +43,14 @@
);
position: relative;
height: clamp(320px, 42vh, 520px);
// A floor, not a fixed height: the slide sits in normal flow at the
// bottom, so an unusually full slide (two-line title, programme,
// synopsis, wrapped pills) grows the banner instead of losing its top.
display: flex;
flex-direction: column;
justify-content: flex-end;
box-sizing: border-box;
min-height: clamp(320px, 42vh, 520px);
// Flush with the content area's top and sides; the rails page's own
// top padding and gutters are cancelled. The negative bottom margin
// tucks the first rail into the fade (the gradient does the spacing).
@@ -189,10 +196,12 @@
// ── Slide content ──────────────────────────────────────────────────────────
.hero__content {
position: absolute;
left: var(--hero-inset);
bottom: 34px;
position: relative;
z-index: 1;
box-sizing: border-box;
// Top padding keeps clear of the title bar edge when the slide grows.
margin-left: var(--hero-inset);
padding: 72px 0 34px;
display: flex;
flex-direction: column;
align-items: flex-start;
@@ -621,8 +630,9 @@
// ── Narrow content area ────────────────────────────────────────────────────
@container dashboard (max-width: 720px) {
.hero__content {
right: var(--hero-inset);
bottom: 28px;
margin-right: var(--hero-inset);
// Clears the rotation controls, which move to the top here.
padding: 64px 0 28px;
max-width: none;
}
@@ -179,6 +179,32 @@ describe('DashboardHeroComponent', () => {
expect(dots()).toHaveLength(4);
});
it('keeps the first slide when a slide arrives ahead of it untouched', () => {
slides.set([slide('fav', 'Favourite'), slide('added', 'Import')]);
render();
expect(activeTitle()).toBe('Favourite');
slides.set([
slide('live', 'Live channel'),
slide('fav', 'Favourite'),
slide('added', 'Import'),
]);
fixture.detectChanges();
expect(activeTitle()).toBe('Favourite');
});
it('shows the slide now at the same position when the active one goes', () => {
render();
dots()[1].click();
fixture.detectChanges();
slides.set([slide('a', 'First'), slide('c', 'Third')]);
fixture.detectChanges();
expect(activeTitle()).toBe('Third');
});
it('never auto-rotates under reduced motion, but the dots still work', () => {
reducedMotion = true;
render();
@@ -4,6 +4,7 @@ import {
computed,
DestroyRef,
inject,
linkedSignal,
signal,
} from '@angular/core';
import { MatIcon } from '@angular/material/icon';
@@ -11,6 +12,7 @@ import { RouterLink } from '@angular/router';
import { TranslatePipe } from '@ngx-translate/core';
import { DashboardHeroSlidesPresenter } from './dashboard-hero-slides.presenter';
import { HERO_ROTATION_MS } from './dashboard-hero-slides.utils';
import type { DashboardHeroSlide } from './dashboard-hero.utils';
const REDUCED_MOTION_QUERY = '(prefers-reduced-motion: reduce)';
@@ -44,7 +46,32 @@ export class DashboardHeroComponent {
readonly slides = this.presenter.slides;
readonly rotationMs = HERO_ROTATION_MS;
private readonly activeId = signal<string | null>(null);
/**
* The slide on screen, by id. Pinned to the first slide as soon as one
* exists, so a late slide inserted ahead of it cannot take its place;
* if the active slide itself disappears, the one now at its position
* takes over.
*/
private readonly activeId = linkedSignal<
DashboardHeroSlide[],
string | null
>({
source: this.slides,
computation: (slides, previous) => {
const currentId = previous?.value ?? null;
if (currentId && slides.some((slide) => slide.id === currentId)) {
return currentId;
}
const previousIndex =
previous?.source.findIndex((slide) => slide.id === currentId) ??
-1;
const index = Math.min(
Math.max(previousIndex, 0),
slides.length - 1
);
return slides[index]?.id ?? null;
},
});
private readonly hovered = signal(false);
private readonly focusWithin = signal(false);
readonly userPaused = signal(false);