diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts index 60a221c88..d001c9838 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.spec.ts @@ -194,6 +194,16 @@ describe('PlaylistSwitcherPanelComponent', () => { expect(closed).toHaveBeenCalled(); }); + it('moves through options whose IDs a selector would misread', () => { + const restored = { ...ALPHA, _id: 'odd:id.one', title: 'Aardvark' }; + playlists.set([restored, BRAVO]); + fixture.detectChanges(); + expect(() => press('ArrowDown')).not.toThrow(); + expect(input().getAttribute('aria-activedescendant')).toBe( + 'playlist-switcher-all-odd:id.one' + ); + }); + it('filters by name or host as you type and highlights the first match', () => { type('charlie.example'); expect(optionIds()).toEqual(['playlist-switcher-all-charlie']); diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts index 0bb26d390..f67f620de 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts +++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.ts @@ -27,6 +27,7 @@ import { orderFailingFirst, PlaylistContextFacade, PlaylistRecentHistoryService, + replacementAfterRemoval, SourceBucketOf, sourceSortName, SourcesTypeFilter, @@ -280,22 +281,21 @@ export class PlaylistSwitcherPanelComponent { * from Undo, which then takes the focus and returns it to the trigger. */ remove(playlist: PlaylistMeta): void { - const all = this.groups().all.map((option) => option.playlist); - const index = all.findIndex((item) => item._id === playlist._id); + const next = replacementAfterRemoval( + this.groups().all.map((option) => option.playlist), + playlist._id, + (row) => this.removal.progress.isDeleting(row._id) + ); this.close(); // Closing put the focus back on the trigger. const returnFocusTo = this.document.activeElement; - this.removal.remove( - playlist, - all[index + 1] ?? all[index - 1] ?? null, - { - returnFocusTo: - returnFocusTo instanceof HTMLElement && - returnFocusTo !== this.document.body - ? returnFocusTo - : null, - } - ); + this.removal.remove(playlist, next, { + returnFocusTo: + returnFocusTo instanceof HTMLElement && + returnFocusTo !== this.document.body + ? returnFocusTo + : null, + }); } /** A row menu gives the focus back to the search, unless closing. */ @@ -337,7 +337,13 @@ export class PlaylistSwitcherPanelComponent { private scrollHighlightIntoView(): void { const id = this.activeOptionId(); const list = this.list().nativeElement; - const option = id ? list.querySelector(`#${id}`) : null; + // By exact ID, not a selector: a restored playlist ID may hold "." + // or ":", which a selector would misread. + const option = id + ? this.rowElements() + .map((row) => row.nativeElement as HTMLElement) + .find((element) => element.id === id) + : undefined; if (!option) { return; } diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts index 2753485de..c6afca2b1 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.spec.ts @@ -206,6 +206,14 @@ describe('RecentPlaylistsComponent', () => { expect(ids()).toEqual(['file', 'm3u', 'xtream', 'url']); }); + it('hands over past a source whose delete is still running', () => { + ( + removal.progress as { isDeleting: (id: string) => boolean } + ).isDeleting = (id) => id === 'xtream'; + component.remove(M3U); + expect(removal.remove).toHaveBeenCalledWith(M3U, FILE); + }); + it('applies the type and status filters together', () => { viewState.setType('m3u'); expect(ids()).toEqual(['m3u', 'file']); diff --git a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts index bc602026e..d83cb582b 100644 --- a/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts +++ b/libs/playlist/shared/ui/src/lib/recent-playlists/recent-playlists.component.ts @@ -31,6 +31,7 @@ import { filterSources, orderFailingFirst, PlaylistContextFacade, + replacementAfterRemoval, SourceBucketOf, sourceSortName, SourcesViewStateService, @@ -201,9 +202,11 @@ export class RecentPlaylistsComponent { if (this.refresh.isRefreshing(playlist._id)) { return; } - const rows = this.visiblePlaylists(); - const index = rows.findIndex((item) => item._id === playlist._id); - const next = rows[index + 1] ?? rows[index - 1] ?? null; + const next = replacementAfterRemoval( + this.visiblePlaylists(), + playlist._id, + (row) => this.removal.progress.isDeleting(row._id) + ); this.removal.remove(playlist, next); } diff --git a/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts b/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts index 2f5b82f00..0b59a40fd 100644 --- a/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts +++ b/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.spec.ts @@ -209,6 +209,17 @@ describe('SourceRemovalService', () => { expect(context.activatePlaylist).toHaveBeenCalledWith('a'); }); + it('does not hand over to a source that is being deleted', () => { + resolvedPlaylistId.set('a'); + service.progress.track('b', { + operation: 'delete-playlist', + status: 'started', + }); + service.remove(playlist('a'), playlist('b')); + expect(context.selectPlaylist).not.toHaveBeenCalled(); + expect(toasts[0].message).toBe('WORKSPACE.SOURCES.REMOVED'); + }); + it('leaves a switch the user made after the delete alone on Undo', () => { resolvedPlaylistId.set('a'); service.remove(playlist('a'), playlist('b')); diff --git a/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts b/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts index 54839e20f..b24ff937e 100644 --- a/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts +++ b/libs/playlist/shared/ui/src/lib/source-removal/source-removal.service.ts @@ -105,7 +105,14 @@ export class SourceRemovalService { const wasOnItsRoute = this.playlistContext.routePlaylistId() === playlist._id; const previousUrl = this.router.url; - const replacement = wasActive ? next : null; + // A source that is leaving too cannot take over. + const replacement = + wasActive && + next && + !this.isPending(next._id) && + !this.progress.isDeleting(next._id) + ? next + : null; this.setPending(playlist._id, true); if (replacement) { diff --git a/libs/playlist/shared/util/src/lib/source-display/source-list.spec.ts b/libs/playlist/shared/util/src/lib/source-display/source-list.spec.ts index 9e5212239..861c41792 100644 --- a/libs/playlist/shared/util/src/lib/source-display/source-list.spec.ts +++ b/libs/playlist/shared/util/src/lib/source-display/source-list.spec.ts @@ -6,6 +6,7 @@ import { filterSources, matchesSourceQuery, orderFailingFirst, + replacementAfterRemoval, SourceBucketOf, } from './source-list'; import { @@ -98,6 +99,26 @@ describe('source list filters', () => { }); }); +describe('replacementAfterRemoval', () => { + const rows = ['a', 'b', 'c', 'd'].map((id) => ({ _id: id })); + const none = () => false; + + it('hands over to the next row, or the previous one at the end', () => { + expect(replacementAfterRemoval(rows, 'b', none)?._id).toBe('c'); + expect(replacementAfterRemoval(rows, 'd', none)?._id).toBe('c'); + expect(replacementAfterRemoval([{ _id: 'a' }], 'a', none)).toBeNull(); + }); + + it('skips rows that are leaving too', () => { + const deleting = (row: { _id: string }) => + row._id === 'c' || row._id === 'd'; + expect(replacementAfterRemoval(rows, 'b', deleting)?._id).toBe('a'); + expect( + replacementAfterRemoval(rows, 'a', (row) => row._id !== 'a') + ).toBeNull(); + }); +}); + describe('SourcesViewStateService', () => { const create = () => { TestBed.configureTestingModule({ diff --git a/libs/playlist/shared/util/src/lib/source-display/source-list.ts b/libs/playlist/shared/util/src/lib/source-display/source-list.ts index 32b19765e..d77ecbcc5 100644 --- a/libs/playlist/shared/util/src/lib/source-display/source-list.ts +++ b/libs/playlist/shared/util/src/lib/source-display/source-list.ts @@ -122,3 +122,30 @@ export function orderFailingFirst( } return [...failing, ...rest]; } + +/** + * The source that takes over when `removedId` leaves a list: the nearest + * row after it, else before it, skipping rows that are leaving too (a + * delete still running). Null when no other source can take over. + */ +export function replacementAfterRemoval( + rows: readonly T[], + removedId: string, + leaving: (row: T) => boolean +): T | null { + const index = rows.findIndex((row) => row._id === removedId); + if (index === -1) { + return null; + } + for (let next = index + 1; next < rows.length; next += 1) { + if (!leaving(rows[next])) { + return rows[next]; + } + } + for (let previous = index - 1; previous >= 0; previous -= 1) { + if (!leaving(rows[previous])) { + return rows[previous]; + } + } + return null; +} diff --git a/libs/portal/shared/data-access/src/lib/source-status.service.spec.ts b/libs/portal/shared/data-access/src/lib/source-status.service.spec.ts index cb5400f23..085319db3 100644 --- a/libs/portal/shared/data-access/src/lib/source-status.service.spec.ts +++ b/libs/portal/shared/data-access/src/lib/source-status.service.spec.ts @@ -232,6 +232,26 @@ describe('SourceStatusService', () => { }); }); + it("keeps an edited account's status from the old account's slow check", async () => { + let finishOld!: (status: string) => void; + portal.checkPortalStatus + .mockReturnValueOnce(new Promise((r) => (finishOld = r))) + .mockResolvedValueOnce('active'); + const service = create(false); + service.recheck(xtream); + + const edited = { ...xtream, password: 'changed' } as PlaylistMeta; + service.ensureChecked(edited, new AbortController().signal); + await Promise.resolve(); + await Promise.resolve(); + expect(service.statusOf(edited)?.reachability).toBe('ok'); + + finishOld('inactive'); + await Promise.resolve(); + await Promise.resolve(); + expect(service.statusOf(edited)?.reachability).toBe('ok'); + }); + it('drops a result that lands after the list closed', async () => { const service = create(false); const controller = new AbortController(); diff --git a/libs/portal/shared/data-access/src/lib/source-status.service.ts b/libs/portal/shared/data-access/src/lib/source-status.service.ts index 481628879..b6039757d 100644 --- a/libs/portal/shared/data-access/src/lib/source-status.service.ts +++ b/libs/portal/shared/data-access/src/lib/source-status.service.ts @@ -57,6 +57,11 @@ export class SourceStatusService { private readonly injector = inject(Injector); private readonly history = inject(SourceHealthHistoryService); private readonly portalStatus = inject(PortalStatusService); + /** + * Browser-build portal statuses by playlist and connection + * (`portalKey`): a slow check of an account the user has since edited + * lands under the old key and cannot overwrite the new account's. + */ private readonly portalStatuses = signal>( new Map() ); @@ -77,7 +82,7 @@ export class SourceStatusService { return null; } if (!this.runtime.supportsSourceHealth) { - const status = this.portalStatuses().get(playlist._id); + const status = this.portalStatuses().get(portalKey(playlist)); return status ? { reachability: sourceReachabilityFromPortalStatus(status), @@ -147,16 +152,17 @@ export class SourceStatusService { fresh: boolean, signal?: AbortSignal ): Promise { - const { _id, serverUrl, username, password } = playlist; + const { serverUrl, username, password } = playlist; + const key = portalKey(playlist); const cached = fresh ? null : this.portalStatus.getCachedStatus(serverUrl, username, password); if (cached) { - this.setPortalStatus(_id, cached); + this.setPortalStatus(key, cached); return; } - if (fresh || !this.portalStatuses().has(_id)) { - this.setPortalStatus(_id, 'checking'); + if (fresh || !this.portalStatuses().has(key)) { + this.setPortalStatus(key, 'checking'); } let status: PortalStatus; try { @@ -172,23 +178,28 @@ export class SourceStatusService { if (signal?.aborted) { // A slow portal must not land after the list has closed; drop a // placeholder so the next opening checks again. - if (this.portalStatuses().get(_id) === 'checking') { - this.setPortalStatus(_id, null); + if (this.portalStatuses().get(key) === 'checking') { + this.setPortalStatus(key, null); } return; } - this.setPortalStatus(_id, status); + this.setPortalStatus(key, status); } - private setPortalStatus(id: string, status: PortalStatus | null): void { + private setPortalStatus(key: string, status: PortalStatus | null): void { this.portalStatuses.update((current) => { const next = new Map(current); if (status) { - next.set(id, status); + next.set(key, status); } else { - next.delete(id); + next.delete(key); } return next; }); } } + +/** A playlist's portal status key: its ID and its connection. */ +function portalKey(playlist: PlaylistMeta): string { + return `${playlist._id}\u0000${sourceHealthFingerprint(playlist)}`; +}