fix(downloads): close offline detail edge cases

This commit is contained in:
4gray committed 2026-08-01 16:29:50 +02:00
1 parent 211ef784fc
commit 942435cc98
8 files changed
+220 -39

No files matched your search

@@ -226,7 +226,14 @@
}
"
>
<div class="offline-detail__season-tabs" role="tablist">
<div
class="offline-detail__season-tabs"
role="tablist"
[attr.aria-label]="
'DOWNLOADS.OFFLINE_DETAIL.DOWNLOADED_EPISODES'
| translate: { count: count() }
"
>
@for (
season of offlineDetail.seasons;
track seasonTestId(season);
@@ -406,6 +406,11 @@ describe('DownloadOfflineDetailComponent', () => {
expect(seasons[0].getAttribute('aria-controls')).toBe(
'offline-season-1-panel'
);
expect(
(fixture.nativeElement as HTMLElement)
.querySelector('[role="tablist"]')
?.getAttribute('aria-label')
).toBe('2 downloaded episodes');
expect(seasons[0].textContent).toContain('2 downloaded episodes');
const firstSeasonEpisodes = Array.from(
(fixture.nativeElement as HTMLElement).querySelectorAll(
@@ -503,12 +508,15 @@ describe('DownloadOfflineDetailComponent', () => {
await render([first, second]);
button('offline-season-2').click();
await fixture.whenStable();
button('offline-season-2').focus();
expect(document.activeElement).toBe(button('offline-season-2'));
downloads.downloads.set([first]);
await fixture.whenStable();
expect(button('offline-season-1').getAttribute('aria-selected')).toBe(
'true'
);
expect(document.activeElement).toBe(button('offline-season-1'));
downloads.downloads.set([first, second]);
await fixture.whenStable();
@@ -659,23 +667,70 @@ describe('DownloadOfflineDetailComponent', () => {
}
);
it('does not redirect when a failed file action coincides with a missing-row emission', async () => {
const operation = deferred<DownloadActionResult>();
actions.run.mockReturnValueOnce(operation.promise);
await render([download(17)]);
it('runs different episode file actions independently while deduplicating the same item', async () => {
const first = deferred<DownloadActionResult>();
const second = deferred<DownloadActionResult>();
actions.run
.mockReturnValueOnce(first.promise)
.mockReturnValueOnce(second.promise);
const episode = (id: number, episodeNumber: number) =>
download(id, {
contentType: 'episode',
episodeNumber,
seasonNumber: 1,
seriesXtreamId: 77,
title: `Northwind - S01E0${episodeNumber}`,
});
await render([episode(17, 1), episode(18, 2)]);
button('offline-play').click();
downloads.downloads.set([
download(17, { fileAvailability: 'missing' }),
]);
await fixture.whenStable();
expect(router.navigate).not.toHaveBeenCalled();
button('episode-play-17').click();
button('episode-play-17').click();
button('episode-play-18').click();
operation.resolve('failed');
expect(actions.run).toHaveBeenCalledTimes(2);
expect(
actions.run.mock.calls.map(([action]) => action.item.id)
).toEqual([17, 18]);
second.resolve('success');
first.resolve('success');
await fixture.whenStable();
expect(router.navigate).not.toHaveBeenCalled();
});
it.each(['failed', 'success'] as const)(
'shows the missing-file transition when an action ends with %s after its row disappears',
async (result) => {
const operation = deferred<DownloadActionResult>();
actions.run.mockReturnValueOnce(operation.promise);
router.navigate.mockResolvedValueOnce(false);
await render([download(17)]);
button('offline-play').click();
downloads.downloads.set([
download(17, { fileAvailability: 'missing' }),
]);
await fixture.whenStable();
expect(router.navigate).not.toHaveBeenCalled();
operation.resolve(result);
await fixture.whenStable();
fixture.detectChanges();
await Promise.resolve();
await fixture.whenStable();
fixture.detectChanges();
expect(router.navigate).toHaveBeenCalledWith(['..'], {
relativeTo: expect.anything(),
queryParamsHandling: 'preserve',
replaceUrl: true,
});
expect(text()).toContain(
'This downloaded file is no longer available on disk.'
);
expect(button('redirect-retry')).toBeTruthy();
}
);
it('does not let an older episode action redirect a reused route in the same series', async () => {
const operation = deferred<DownloadActionResult>();
actions.run.mockReturnValueOnce(operation.promise);
@@ -33,9 +33,9 @@ export class DownloadOfflineFileCoordinatorService {
private readonly actions = inject(DownloadManagerActionsService);
private readonly navigation = inject(DownloadOfflineRouteNavigationService);
private readonly injector = inject(Injector);
private readonly activeAction = signal<ActiveFileAction | undefined>(
undefined
);
private readonly activeActions = signal<
ReadonlyMap<number, ActiveFileAction>
>(new Map());
private readonly ownedRouteGeneration = signal<number | undefined>(
undefined
);
@@ -50,11 +50,13 @@ export class DownloadOfflineFileCoordinatorService {
const route = inputs.route();
const row = inputs.selectedRow();
const detail = inputs.detail();
const activeRouteGeneration =
this.activeAction()?.routeGeneration;
const routeHasActiveAction = this.hasActiveAction(
this.activeActions(),
route.generation
);
if (
detail &&
activeRouteGeneration !== route.generation &&
!routeHasActiveAction &&
this.ownedRouteGeneration() === route.generation
) {
this.ownedRouteGeneration.set(undefined);
@@ -67,7 +69,7 @@ export class DownloadOfflineFileCoordinatorService {
!detail;
if (
!unavailable ||
activeRouteGeneration === route.generation ||
routeHasActiveAction ||
this.ownedRouteGeneration() === route.generation ||
this.redirectState()?.routeGeneration === route.generation
) {
@@ -99,7 +101,7 @@ export class DownloadOfflineFileCoordinatorService {
): Promise<void> {
const route = currentRoute();
if (
this.activeAction()?.routeGeneration === route.generation ||
this.activeActions().has(item.id) ||
route.downloadId === undefined
) {
return;
@@ -108,14 +110,29 @@ export class DownloadOfflineFileCoordinatorService {
actionGeneration: ++this.actionGeneration,
routeGeneration: route.generation,
};
this.activeAction.set(active);
this.activeActions.update((actions) =>
new Map(actions).set(item.id, active)
);
this.ownedRouteGeneration.set(route.generation);
const result = await this.actions.run({
type,
item: item as DownloadItem,
});
const stillOwnsAction = this.activeAction() === active;
if (stillOwnsAction) this.activeAction.set(undefined);
const stillOwnsAction = this.activeActions().get(item.id) === active;
if (stillOwnsAction) {
this.activeActions.update((actions) => {
const next = new Map(actions);
next.delete(item.id);
return next;
});
if (
result !== 'file-missing' &&
!this.hasActiveAction(this.activeActions(), route.generation) &&
this.ownedRouteGeneration() === route.generation
) {
this.ownedRouteGeneration.set(undefined);
}
}
if (
!stillOwnsAction ||
currentRoute().generation !== route.generation ||
@@ -126,6 +143,15 @@ export class DownloadOfflineFileCoordinatorService {
await this.redirect(route);
}
private hasActiveAction(
actions: ReadonlyMap<number, ActiveFileAction>,
routeGeneration: number
): boolean {
return Array.from(actions.values()).some(
(action) => action.routeGeneration === routeGeneration
);
}
private async redirect(route: OfflineDetailRouteContext): Promise<void> {
const state = this.redirectState();
if (
@@ -1,3 +1,4 @@
import { DOCUMENT } from '@angular/common';
import { effect, inject, Injectable, Injector, signal } from '@angular/core';
import type { DownloadOfflineSeason } from './download-offline-detail.viewmodel';
import { offlineSeasonKey } from './download-offline-detail.presentation';
@@ -10,6 +11,7 @@ interface SeasonSelection {
@Injectable()
export class DownloadOfflineSeasonSelectionService {
private readonly document = inject(DOCUMENT);
private readonly injector = inject(Injector);
private readonly selection = signal<SeasonSelection | undefined>(undefined);
@@ -35,10 +37,16 @@ export class DownloadOfflineSeasonSelectionService {
) {
return;
}
this.selection.set({
const shouldMoveFocus =
current?.routeGeneration === currentRoute.generation &&
this.document.activeElement?.id ===
this.tabIdForKey(current.key);
const normalized = {
key: offlineSeasonKey(available[0]),
routeGeneration: currentRoute.generation,
});
};
this.selection.set(normalized);
if (shouldMoveFocus) this.focusAfterRender(normalized);
},
{ injector: this.injector }
);
@@ -68,7 +76,7 @@ export class DownloadOfflineSeasonSelectionService {
}
tabId(season: DownloadOfflineSeason): string {
return `offline-${offlineSeasonKey(season)}-tab`;
return this.tabIdForKey(offlineSeasonKey(season));
}
handleKeydown(
@@ -104,4 +112,17 @@ export class DownloadOfflineSeasonSelectionService {
}
return undefined;
}
private focusAfterRender(selection: SeasonSelection): void {
queueMicrotask(() => {
if (this.selection() !== selection) return;
this.document
.getElementById(this.tabIdForKey(selection.key))
?.focus();
});
}
private tabIdForKey(key: string): string {
return `offline-${key}-tab`;
}
}
@@ -96,17 +96,21 @@
<!-- Backdrop Image -->
<div
class="hero__backdrop"
[class.hero__backdrop--blurred]="
!backdropUrl() && posterUrl() && !posterError()
"
[class.hero__backdrop--blurred]="usesPosterBackdrop()"
[style.backgroundImage]="
backdropUrl() && !posterError()
? 'url(' + backdropUrl() + ')'
: posterUrl() && !posterError()
? 'url(' + posterUrl() + ')'
: fallbackBackdropBackground()
backdropImageUrl() ? null : fallbackBackdropBackground()
"
></div>
>
@if (backdropImageUrl(); as backdropImage) {
<img
class="hero__backdrop-image"
[src]="backdropImage"
(error)="onBackdropError()"
alt=""
aria-hidden="true"
/>
}
</div>
<!-- Gradient Vignette Overlay -->
<div class="hero__vignette"></div>
@@ -104,6 +104,14 @@
z-index: 0;
overflow: hidden;
&-image {
display: block;
width: 100%;
height: 100%;
object-fit: cover;
object-position: center 20%;
}
// Fallback blur effect when using poster as backdrop
&--blurred {
filter: blur(40px) saturate(1.3);
@@ -66,13 +66,13 @@ describe('ContentHeroComponent', () => {
fixture.componentRef.setInput('posterUrl', 'broken.jpg');
fixture.detectChanges();
const first = (fixture.nativeElement as HTMLElement).querySelector(
'img[src="broken.jpg"]'
'.poster img[src="broken.jpg"]'
) as HTMLImageElement;
first.dispatchEvent(new Event('error'));
fixture.detectChanges();
expect(
(fixture.nativeElement as HTMLElement).querySelector(
'img[src="broken.jpg"]'
'.poster img[src="broken.jpg"]'
)
).toBeNull();
@@ -80,8 +80,50 @@ describe('ContentHeroComponent', () => {
fixture.detectChanges();
expect(
(fixture.nativeElement as HTMLElement).querySelector(
'img[src="replacement.jpg"]'
'.poster img[src="replacement.jpg"]'
)
).toBeTruthy();
});
it('keeps an explicit backdrop visible when the poster image fails', () => {
fixture.componentRef.setInput('posterUrl', 'broken-poster.jpg');
fixture.componentRef.setInput('backdropUrl', 'working-backdrop.jpg');
fixture.detectChanges();
const poster = (fixture.nativeElement as HTMLElement).querySelector(
'img[src="broken-poster.jpg"]'
) as HTMLImageElement;
poster.dispatchEvent(new Event('error'));
fixture.detectChanges();
expect(
(fixture.nativeElement as HTMLElement).querySelector(
'img.hero__backdrop-image[src="working-backdrop.jpg"]'
)
).toBeTruthy();
});
it('falls back the backdrop independently without hiding a valid poster', () => {
fixture.componentRef.setInput('posterUrl', 'working-poster.jpg');
fixture.componentRef.setInput('backdropUrl', 'broken-backdrop.jpg');
fixture.detectChanges();
const backdrop = (fixture.nativeElement as HTMLElement).querySelector(
'img.hero__backdrop-image'
) as HTMLImageElement | null;
expect(backdrop).toBeTruthy();
backdrop?.dispatchEvent(new Event('error'));
fixture.detectChanges();
expect(
(fixture.nativeElement as HTMLElement).querySelector(
'.poster img[src="working-poster.jpg"]'
)
).toBeTruthy();
expect(
(fixture.nativeElement as HTMLElement).querySelector(
'img.hero__backdrop-image'
)
).toBeNull();
});
});
@@ -41,6 +41,20 @@ export class ContentHeroComponent {
readonly backClicked = output<void>();
readonly posterError = signal(false);
private readonly failedBackdropUrl = signal<string | undefined>(undefined);
readonly backdropSourceUrl = computed(
() => this.backdropUrl() || this.posterUrl()
);
readonly backdropError = computed(() => {
const source = this.backdropSourceUrl();
return !!source && this.failedBackdropUrl() === source;
});
readonly backdropImageUrl = computed(() =>
this.backdropError() ? undefined : this.backdropSourceUrl()
);
readonly usesPosterBackdrop = computed(
() => !this.backdropUrl() && !!this.posterUrl() && !this.backdropError()
);
readonly descriptionEl =
viewChild<ElementRef<HTMLElement>>('descriptionEl');
@@ -76,6 +90,10 @@ export class ContentHeroComponent {
this.posterError.set(true);
}
onBackdropError(): void {
this.failedBackdropUrl.set(this.backdropSourceUrl());
}
readonly formattedTitle = computed(() => {
const t = this.title();
if (!t) return '';