From 942435cc9838abf3cf2b45ba50638bf9528b5964 Mon Sep 17 00:00:00 2001 From: 4gray Date: Fri, 31 Jul 2026 15:56:31 +0200 Subject: [PATCH] fix(downloads): close offline detail edge cases --- .../download-offline-detail.component.html | 9 ++- .../download-offline-detail.component.spec.ts | 79 ++++++++++++++++--- ...wnload-offline-file-coordinator.service.ts | 48 ++++++++--- ...wnload-offline-season-selection.service.ts | 27 ++++++- .../content-hero/content-hero.component.html | 22 +++--- .../content-hero/content-hero.component.scss | 8 ++ .../content-hero.component.spec.ts | 48 ++++++++++- .../content-hero/content-hero.component.ts | 18 +++++ 8 files changed, 220 insertions(+), 39 deletions(-) diff --git a/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-detail.component.html b/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-detail.component.html index 102e1eb21..b5caee4b4 100644 --- a/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-detail.component.html +++ b/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-detail.component.html @@ -226,7 +226,14 @@ } " > -
+
@for ( season of offlineDetail.seasons; track seasonTestId(season); diff --git a/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-detail.component.spec.ts b/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-detail.component.spec.ts index a794c5cb3..93d436f21 100644 --- a/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-detail.component.spec.ts +++ b/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-detail.component.spec.ts @@ -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(); - 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(); + const second = deferred(); + 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(); + 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(); actions.run.mockReturnValueOnce(operation.promise); diff --git a/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-file-coordinator.service.ts b/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-file-coordinator.service.ts index a82dd8eb6..36e4956cf 100644 --- a/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-file-coordinator.service.ts +++ b/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-file-coordinator.service.ts @@ -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( - undefined - ); + private readonly activeActions = signal< + ReadonlyMap + >(new Map()); private readonly ownedRouteGeneration = signal( 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 { 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, + routeGeneration: number + ): boolean { + return Array.from(actions.values()).some( + (action) => action.routeGeneration === routeGeneration + ); + } + private async redirect(route: OfflineDetailRouteContext): Promise { const state = this.redirectState(); if ( diff --git a/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-season-selection.service.ts b/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-season-selection.service.ts index 6a14c38b9..7d548bd21 100644 --- a/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-season-selection.service.ts +++ b/libs/portal/downloads/feature/src/lib/offline-detail/download-offline-season-selection.service.ts @@ -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(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`; + } } diff --git a/libs/ui/components/src/lib/content-hero/content-hero.component.html b/libs/ui/components/src/lib/content-hero/content-hero.component.html index 3503d772c..44b5ea4b7 100644 --- a/libs/ui/components/src/lib/content-hero/content-hero.component.html +++ b/libs/ui/components/src/lib/content-hero/content-hero.component.html @@ -96,17 +96,21 @@
+ > + @if (backdropImageUrl(); as backdropImage) { + + } +
diff --git a/libs/ui/components/src/lib/content-hero/content-hero.component.scss b/libs/ui/components/src/lib/content-hero/content-hero.component.scss index a44db8110..73ca7f5c8 100644 --- a/libs/ui/components/src/lib/content-hero/content-hero.component.scss +++ b/libs/ui/components/src/lib/content-hero/content-hero.component.scss @@ -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); diff --git a/libs/ui/components/src/lib/content-hero/content-hero.component.spec.ts b/libs/ui/components/src/lib/content-hero/content-hero.component.spec.ts index 1beb6a5ab..8c7aa1203 100644 --- a/libs/ui/components/src/lib/content-hero/content-hero.component.spec.ts +++ b/libs/ui/components/src/lib/content-hero/content-hero.component.spec.ts @@ -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(); + }); }); diff --git a/libs/ui/components/src/lib/content-hero/content-hero.component.ts b/libs/ui/components/src/lib/content-hero/content-hero.component.ts index e8bd1e76e..a3448e145 100644 --- a/libs/ui/components/src/lib/content-hero/content-hero.component.ts +++ b/libs/ui/components/src/lib/content-hero/content-hero.component.ts @@ -41,6 +41,20 @@ export class ContentHeroComponent { readonly backClicked = output(); readonly posterError = signal(false); + private readonly failedBackdropUrl = signal(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>('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 '';