From 366745f16ddd01198baa4177519ef6c81aafad7c Mon Sep 17 00:00:00 2001 From: 4gray Date: Tue, 13 Oct 2026 09:51:33 +0200 Subject: [PATCH] fix(sources): checks behind filters, keyboard Undo in the switcher, check causes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local review round 1 (Codex + Greptile) on the redesign: - A saved Responding / Not responding filter hid every source without a result, and the hidden rows were what started the checks, so the list stayed empty. keepSourcesChecked (portal shared data-access) now keeps every library source checked while the Sources page lives, on desktop where a check is one shared job that rows join. - Removing from the switcher left its focus trap open around the search, outside of which the Undo toast sits: Tab could not reach Undo. The popover now closes first; Undo takes the focus and returns it to the trigger (SourceRemovalOptions.returnFocusTo). Every switcher action closes through one path, so a closing row menu no longer pulls the focus back into the popover. - A running desktop Xtream delete brought its source back into the switcher as an ordinary option that could be opened or removed again; the switcher now leaves it out until the delete ends (the Sources page shows its progress and Cancel). - Undo re-activated the removed source over a source the user had chosen since, when nothing had replaced it (it was the only row a filter showed). Undo now restores only while no other source is active. - The switcher highlight stays on its option, or in range, when the list changes under the same search, so Enter still opens something. - The status dot named every failure "Could not connect". SourceStatus now carries the check's cause (access denied, account disabled, timeout…, kept with the health history and validated on read), and M3U dots again say the check covers the playlist URL, not its channels. Tests: new specs for keepSourcesChecked, the saved-filter check, the Undo guard and focus return, the switcher close/highlight/deleting cases and the dot's cause; the filter and Undo specs fail without their fixes. The switcher keyboard E2E now deletes and undoes from the keyboard. Co-Authored-By: Claude Opus 5.5 --- .../src/playlist-switcher.e2e.ts | 21 ++++ docs/architecture/iptvnator-ui-guidelines.md | 23 +++-- .../playlist-switcher-panel.component.html | 2 +- .../playlist-switcher-panel.component.spec.ts | 55 ++++++++++- .../playlist-switcher-panel.component.ts | 99 ++++++++++++++++--- .../recent-playlists.component.spec.ts | 23 +++++ .../recent-playlists.component.ts | 11 ++- .../source-removal-toast.component.spec.ts | 39 ++++++-- .../source-removal-toast.component.ts | 19 +++- .../source-removal.service.spec.ts | 23 +++++ .../source-removal/source-removal.service.ts | 30 ++++-- .../source-row/source-row.component.spec.ts | 26 +++++ .../lib/source-row/source-tile.component.ts | 15 ++- libs/portal/shared/data-access/src/index.ts | 1 + .../lib/source-health-history.service.spec.ts | 16 ++- .../src/lib/source-health-history.service.ts | 33 ++++--- .../src/lib/source-status-checks.spec.ts | 98 ++++++++++++++++++ .../src/lib/source-status-checks.ts | 58 +++++++++++ .../src/lib/source-status.service.spec.ts | 23 ++++- .../src/lib/source-status.service.ts | 26 ++++- .../interfaces/src/lib/source-health.ts | 21 ++++ .../src/lib/source-reachability.spec.ts | 8 ++ .../interfaces/src/lib/source-reachability.ts | 13 ++- 23 files changed, 616 insertions(+), 67 deletions(-) create mode 100644 libs/portal/shared/data-access/src/lib/source-status-checks.spec.ts create mode 100644 libs/portal/shared/data-access/src/lib/source-status-checks.ts diff --git a/apps/electron-backend-e2e/src/playlist-switcher.e2e.ts b/apps/electron-backend-e2e/src/playlist-switcher.e2e.ts index 2bcaf00f3..4ace9dbce 100644 --- a/apps/electron-backend-e2e/src/playlist-switcher.e2e.ts +++ b/apps/electron-backend-e2e/src/playlist-switcher.e2e.ts @@ -565,6 +565,27 @@ test.describe('Electron Playlist Switcher', () => { await expect(panel).toHaveCount(0); if (viewport) await page.setViewportSize(viewport); + // A delete from the keyboard: the popover closes, since its + // focus trap would keep Tab from Undo; Undo takes the focus and + // gives it back to the trigger. + await page.keyboard.press('ControlOrMeta+p'); + await expect(search).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await page.keyboard.press('Shift+F10'); + const rowMenu = page.locator('.mat-mdc-menu-panel'); + await expect(rowMenu).toBeVisible(); + await page.keyboard.press('End'); + await expect( + rowMenu.getByRole('menuitem', { name: /Remove/ }) + ).toBeFocused(); + await page.keyboard.press('Enter'); + await expect(panel).toHaveCount(0); + const undo = page.locator('.source-removal-toast__undo'); + await expect(undo).toBeFocused(); + await page.keyboard.press('Enter'); + await expect(undo).toHaveCount(0); + await expect(trigger).toBeFocused(); + await openSources(page); await expect( page.locator('app-workspace-sources app-source-row') diff --git a/docs/architecture/iptvnator-ui-guidelines.md b/docs/architecture/iptvnator-ui-guidelines.md index fe2ed27c8..80458581b 100644 --- a/docs/architecture/iptvnator-ui-guidelines.md +++ b/docs/architecture/iptvnator-ui-guidelines.md @@ -1371,13 +1371,20 @@ in `@iptvnator/playlist/shared/util` (`source-display`, `source-format`, every list at once (`SourceRemovalService.pendingIds`) and the delete runs when the toast closes; its close button commits at once, and quitting within the four seconds keeps the source. Deleting the active source - switches to the next row. A desktop Xtream delete brings its row back as - "Removing…" with progress and Cancel. When the delete took the row and - with it the focus, the toast's Undo takes the focus. Bulk cleanup keeps - its confirmation dialog. + switches to the next row; Undo switches back only if no other source was + chosen since. A desktop Xtream delete brings its row back as "Removing…" + with progress and Cancel on the Sources page; the switcher leaves the + source out until the delete ends. When the delete took the row and with + it the focus, the toast's Undo takes the focus. A delete from the + switcher closes the popover first, since its focus trap would keep the + keyboard from Undo: Undo takes the focus and returns it to the trigger + (`SourceRemovalOptions.returnFocusTo`). Bulk cleanup keeps its + confirmation dialog. - **Side list.** Type and, on desktop, Status (Any / Responding / Not responding) combine and persist (`SourcesViewStateService`); each group - counts within the other's choice. + counts within the other's choice. The page checks every source, not only + the rows a filter shows (`keepSourcesChecked`): a saved status filter + would otherwise hide the unchecked sources it is waiting for. - **Switcher.** The trigger is a 40px secondary button; ⌘P / Ctrl+P opens it (⌘K stays the command palette). The popover is a CDK connected overlay, so row menus open above it, not inside its scrolling list. Focus stays in the @@ -1400,8 +1407,10 @@ files have no dot. On desktop the live `SourceHealthService` result wins; the last conclusive result per source is kept across restarts (`SourceHealthHistoryService`, no URL or credential stored), so a source that failed yesterday shows as failing before it is checked again, with "Not -responding · 3 days". The dot's tooltip and accessible name give the reason -and check time; M3U checks describe the source URL only. Retry and Check +responding · 3 days". The dot's tooltip and accessible name give the +check's cause (`SourceStatus.reason`: access denied, account disabled, +timeout…, kept with the history) and check time; M3U checks say they +cover the playlist URL, not its channels. Retry and Check again rerun the check; they are not a playlist refresh. The browser build keeps the Xtream portal status only. diff --git a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.html b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.html index adf30668a..dcd2baa47 100644 --- a/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.html +++ b/libs/playlist/shared/ui/src/lib/playlist-switcher/playlist-switcher-panel.component.html @@ -162,6 +162,6 @@ (playlistInfoRequested)="openInfo(option.playlist)" (accountInfoRequested)="openAccountInfo(option.playlist)" (removeRequested)="remove(option.playlist)" - (menuClosed)="searchInput.focus()" + (menuClosed)="onMenuClosed()" /> 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 2aef11c86..60a221c88 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 @@ -47,6 +47,7 @@ describe('PlaylistSwitcherPanelComponent', () => { let component: PlaylistSwitcherPanelComponent; let playlists: ReturnType>; let pending: ReturnType>>; + let deleting: ReturnType>>; let selectPlaylist: jest.Mock; let remove: jest.Mock; let navigate: jest.Mock; @@ -77,6 +78,7 @@ describe('PlaylistSwitcherPanelComponent', () => { localStorage.clear(); playlists = signal([ALPHA, BRAVO, CHARLIE]); pending = signal>(new Set()); + deleting = signal>(new Set()); selectPlaylist = jest.fn(); remove = jest.fn(); navigate = jest.fn(); @@ -121,7 +123,13 @@ describe('PlaylistSwitcherPanelComponent', () => { }, { provide: SourceRemovalService, - useValue: { pendingIds: pending, remove }, + useValue: { + pendingIds: pending, + remove, + progress: { + isDeleting: (id: string) => deleting().has(id), + }, + }, }, { provide: PlaylistRefreshActionService, @@ -218,13 +226,52 @@ describe('PlaylistSwitcherPanelComponent', () => { expect(closed).toHaveBeenCalled(); }); - it('leaves out a source whose delete is pending and hands over on delete', () => { - component.remove(BRAVO); - expect(remove).toHaveBeenCalledWith(BRAVO, CHARLIE); + it('closes before a delete, so Undo can take the focus and give it back to the trigger', () => { + const trigger = document.createElement('button'); + document.body.appendChild(trigger); + // The host closes the popover and focuses its trigger. + component.closeRequested.subscribe(() => trigger.focus()); + component.remove(BRAVO); + expect(remove).toHaveBeenCalledWith(BRAVO, CHARLIE, { + returnFocusTo: trigger, + }); + trigger.remove(); + }); + + it('leaves out a source whose delete is pending or running', () => { pending.set(new Set(['bravo'])); fixture.detectChanges(); expect(optionIds()).not.toContain('playlist-switcher-all-bravo'); + + pending.set(new Set()); + deleting.set(new Set(['alpha'])); + fixture.detectChanges(); + expect(optionIds()).toEqual([ + 'playlist-switcher-all-bravo', + 'playlist-switcher-all-charlie', + ]); + }); + + it('keeps the highlight on its option, or in range, when the list changes', () => { + press('ArrowUp'); + expect(input().getAttribute('aria-activedescendant')).toBe( + 'playlist-switcher-all-charlie' + ); + // An earlier option leaves: the highlight follows charlie. + playlists.set([BRAVO, CHARLIE]); + fixture.detectChanges(); + expect(input().getAttribute('aria-activedescendant')).toBe( + 'playlist-switcher-all-charlie' + ); + // The highlighted last option leaves: Enter opens the new last one. + playlists.set([BRAVO]); + fixture.detectChanges(); + expect(input().getAttribute('aria-activedescendant')).toBe( + 'playlist-switcher-all-bravo' + ); + press('Enter'); + expect(selectPlaylist).toHaveBeenCalledWith(BRAVO); }); it('goes to the Sources page from the footer', () => { 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 e5174ffcf..fdf23f8f7 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 @@ -1,5 +1,5 @@ import { A11yModule } from '@angular/cdk/a11y'; -import { NgTemplateOutlet } from '@angular/common'; +import { DOCUMENT, NgTemplateOutlet } from '@angular/common'; import { afterNextRender, ChangeDetectionStrategy, @@ -75,6 +75,7 @@ const KINDS: readonly PlaylistSourceKind[] = ['m3u', 'xtream', 'stalker']; }) export class PlaylistSwitcherPanelComponent { private readonly context = inject(PlaylistContextFacade); + private readonly document = inject(DOCUMENT); private readonly dialog = inject(MatDialog); private readonly recentHistory = inject(PlaylistRecentHistoryService); private readonly refreshAction = inject(PlaylistRefreshActionService); @@ -91,6 +92,7 @@ export class PlaylistSwitcherPanelComponent { }); readonly closeRequested = output(); + private closing = false; readonly playlistSelected = output(); readonly addPlaylistRequested = output(); readonly accountInfoRequested = output(); @@ -110,11 +112,20 @@ export class PlaylistSwitcherPanelComponent { return status ? sourceStatusBucket(status.reachability) : null; }; + /** + * Every source except one whose delete is pending or running: a running + * delete shows its progress and Cancel on the Sources page, and its + * source cannot be opened or removed again from here. + */ readonly playlists = computed(() => { const pending = this.removal.pendingIds(); return this.context .playlists() - .filter((playlist) => !pending.has(playlist._id)); + .filter( + (playlist) => + !pending.has(playlist._id) && + !this.removal.progress.isDeleting(playlist._id) + ); }); readonly typeCounts = computed(() => countSourcesByType(this.playlists(), 'all', this.bucketOf) @@ -145,13 +156,38 @@ export class PlaylistSwitcherPanelComponent { ...this.groups().recent, ...this.groups().all, ]); - /** A search highlights its first match, so Enter opens it. */ + /** + * A search highlights its first match, so Enter opens it. When the list + * changes under the same search (a source added or removed), the + * highlight stays on its option, or on the same place, clamped. + */ readonly highlight = linkedSignal< - readonly [string, SourcesTypeFilter], + { + query: string; + type: SourcesTypeFilter; + ids: readonly string[]; + }, number >({ - source: () => [this.query(), this.type()] as const, - computation: ([query]) => (query.trim() ? 0 : -1), + source: () => ({ + query: this.query(), + type: this.type(), + ids: this.options().map((option) => option.id), + }), + computation: (source, previous) => { + const sameSearch = + previous && + previous.source.query === source.query && + previous.source.type === source.type; + if (!sameSearch) { + return source.query.trim() ? 0 : -1; + } + const id = previous.source.ids[previous.value]; + const kept = id === undefined ? -1 : source.ids.indexOf(id); + return kept !== -1 + ? kept + : Math.min(previous.value, source.ids.length - 1); + }, }); readonly activeOptionId = computed( () => this.options()[this.highlight()]?.id ?? null @@ -199,7 +235,7 @@ export class PlaylistSwitcherPanelComponent { case 'Escape': event.preventDefault(); event.stopPropagation(); - this.closeRequested.emit(); + this.close(); break; case 'ContextMenu': case 'F10': @@ -214,7 +250,7 @@ export class PlaylistSwitcherPanelComponent { select(playlist: PlaylistMeta): void { this.context.selectPlaylist(playlist); this.playlistSelected.emit(playlist._id); - this.closeRequested.emit(); + this.close(); } canRefresh(playlist: PlaylistMeta): boolean { @@ -222,34 +258,67 @@ export class PlaylistSwitcherPanelComponent { } refresh(playlist: PlaylistMeta): void { - this.closeRequested.emit(); + this.close(); this.refreshAction.refresh(playlist); } openInfo(playlist: PlaylistMeta): void { - this.closeRequested.emit(); + this.close(); this.dialog.open(PlaylistInfoComponent, { data: playlist }); } openAccountInfo(playlist: PlaylistMeta): void { - this.closeRequested.emit(); + this.close(); this.accountInfoRequested.emit(playlist); } - /** Deletes with Undo; the active source hands over to its neighbour. */ + /** + * Deletes with Undo; the active source hands over to its neighbour. The + * popover closes first: its focus trap would keep the keyboard away + * 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); - this.removal.remove(playlist, all[index + 1] ?? all[index - 1] ?? null); + 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, + } + ); + } + + /** A row menu gives the focus back to the search, unless closing. */ + onMenuClosed(): void { + if (!this.closing) { + this.searchInput().nativeElement.focus(); + } + } + + /** + * Every action that leaves the popover closes it through here, so a row + * menu closing after its action does not pull the focus back into it. + */ + private close(): void { + this.closing = true; + this.closeRequested.emit(); } addPlaylist(): void { - this.closeRequested.emit(); + this.close(); this.addPlaylistRequested.emit(); } openAllSources(): void { - this.closeRequested.emit(); + this.close(); void this.router.navigate(['/workspace', 'sources']); } 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 4d4ea682d..ca4904d02 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 @@ -167,6 +167,29 @@ describe('RecentPlaylistsComponent', () => { ).toBe(3); }); + it('checks the sources a saved status filter hides', () => { + fixture.destroy(); + viewState.setStatus('ok'); + statuses.set('xtream', { reachability: 'unknown' }); + const ensureChecked = TestBed.inject(SourceStatusService) + .ensureChecked as jest.Mock; + ensureChecked.mockClear(); + + const restarted = TestBed.createComponent(RecentPlaylistsComponent); + restarted.detectChanges(); + TestBed.tick(); + + expect( + restarted.componentInstance.visiblePlaylists().map((p) => p._id) + ).toEqual(['m3u']); + // No row is on screen for it, yet its check runs, so it can join + // the list once it answers. + expect(ensureChecked.mock.calls.map(([p]) => p._id)).toContain( + 'xtream' + ); + restarted.destroy(); + }); + 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 451b0860b..5d7d8d04a 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 @@ -23,7 +23,10 @@ import { selectAllPlaylistsMeta, selectPlaylistsLoadingFlag, } from '@iptvnator/m3u-state'; -import { SourceStatusService } from '@iptvnator/portal/shared/data-access'; +import { + keepSourcesChecked, + SourceStatusService, +} from '@iptvnator/portal/shared/data-access'; import { filterSources, orderFailingFirst, @@ -131,6 +134,12 @@ export class RecentPlaylistsComponent { : sorted; }); + constructor() { + // A status filter hides the sources it has no result for, so the + // checks follow the library rather than the rows on screen. + keepSourcesChecked(() => this.playlists()); + } + resetFilters(): void { this.viewState.reset(); } diff --git a/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.spec.ts b/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.spec.ts index 827a0cb28..048f15c4c 100644 --- a/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.spec.ts @@ -8,11 +8,14 @@ import { SourceRemovalToastComponent } from './source-removal-toast.component'; describe('SourceRemovalToastComponent', () => { let opened: Subject; + let dismissed: Subject; let ref: { afterOpened: () => Subject; + afterDismissed: () => Subject; dismiss: jest.Mock; dismissWithAction: jest.Mock; }; + let data: Record; const render = () => { const fixture = TestBed.createComponent(SourceRemovalToastComponent); @@ -23,22 +26,22 @@ describe('SourceRemovalToastComponent', () => { beforeEach(() => { opened = new Subject(); + dismissed = new Subject(); ref = { afterOpened: () => opened, + afterDismissed: () => dismissed, dismiss: jest.fn(), dismissWithAction: jest.fn(), }; + data = { + message: 'Removed “Alpha”', + undoLabel: 'Undo', + closeLabel: 'Close', + }; TestBed.configureTestingModule({ imports: [SourceRemovalToastComponent], providers: [ - { - provide: MAT_SNACK_BAR_DATA, - useValue: { - message: 'Removed “Alpha”', - undoLabel: 'Undo', - closeLabel: 'Close', - }, - }, + { provide: MAT_SNACK_BAR_DATA, useFactory: () => data }, { provide: MatSnackBarRef, useValue: ref }, ], }); @@ -71,6 +74,26 @@ describe('SourceRemovalToastComponent', () => { ); }); + it('takes the focus from a closing surface and gives it back after', () => { + const trigger = document.createElement('button'); + document.body.appendChild(trigger); + trigger.focus(); + data['returnFocusTo'] = trigger; + const element = render(); + const undo = element.querySelector( + '.source-removal-toast__undo' + ); + + opened.next(); + expect(document.activeElement).toBe(undo); + + // The toast leaves the page and its focus with it. + element.remove(); + dismissed.next(); + expect(document.activeElement).toBe(trigger); + trigger.remove(); + }); + it('leaves the focus where the user put it', () => { render(); const elsewhere = document.createElement('button'); diff --git a/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.ts b/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.ts index 08cfc9d05..eb79a4224 100644 --- a/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.ts +++ b/libs/playlist/shared/ui/src/lib/source-removal/source-removal-toast.component.ts @@ -20,13 +20,17 @@ export interface SourceRemovalToastData { message: string; undoLabel: string; closeLabel: string; + /** Undo takes the focus, which returns here when the toast closes. */ + returnFocusTo?: HTMLElement | null; } /** * "Removed …" with Undo, and a close button that commits the delete at * once instead of waiting out the toast. A delete from a row's menu takes * the row, and with it the focus, off the page: Undo then takes the focus, - * so the keyboard can reach it. + * so the keyboard can reach it. A caller that closes on delete (the + * switcher) passes `returnFocusTo`: Undo takes the focus regardless, and + * the focus goes back there when the toast closes. */ @Component({ selector: 'app-source-removal-toast', @@ -75,11 +79,22 @@ export class SourceRemovalToastComponent { const document = inject(DOCUMENT); // After the opening animation: by then a closing menu has tried, and // failed, to give focus back to the removed row's ⋯ button. + const returnFocusTo = this.data.returnFocusTo ?? null; this.ref.afterOpened().subscribe(() => { const active = document.activeElement; - if (!active || active === document.body) { + if (returnFocusTo || !active || active === document.body) { (this.undo().nativeElement as HTMLElement).focus(); } }); + // The toast is gone by now; focus that left with it lands on body. + this.ref.afterDismissed().subscribe(() => { + const active = document.activeElement; + if ( + returnFocusTo?.isConnected && + (!active || active === document.body) + ) { + returnFocusTo.focus(); + } + }); } } 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 322bf6b95..8e143aaec 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 @@ -176,6 +176,29 @@ describe('SourceRemovalService', () => { expect(context.activatePlaylist).not.toHaveBeenCalled(); }); + it('leaves a later switch alone on Undo when nothing replaced the source', () => { + // The source was the only row a filter showed: no replacement. + resolvedPlaylistId.set('a'); + routePlaylistId.set('a'); + router.url = '/workspace/xtreams/a/vod'; + service.remove(playlist('a')); + resolvedPlaylistId.set('c'); + + toasts[0].action.next(); + expect(router.navigateByUrl).not.toHaveBeenCalled(); + expect(context.activatePlaylist).not.toHaveBeenCalled(); + }); + + it('passes the focus target of a closing surface to the toast', () => { + const trigger = document.createElement('button'); + service.remove(playlist('a'), null, { returnFocusTo: trigger }); + const openFromComponent = TestBed.inject(MatSnackBar) + .openFromComponent as jest.Mock; + expect(openFromComponent.mock.calls[0][1].data.returnFocusTo).toBe( + trigger + ); + }); + it('leaves the deleted source’s own page and returns on Undo', () => { resolvedPlaylistId.set('a'); routePlaylistId.set('a'); 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 6753766f1..c87954ff1 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 @@ -23,6 +23,15 @@ import { /** How long Undo stays available before the delete is committed. */ export const SOURCE_REMOVAL_UNDO_MS = 4000; +export interface SourceRemovalOptions { + /** + * For a surface that closes on delete, such as the switcher popover, + * whose focus trap the toast sits outside: Undo takes the focus at once, + * and it comes back here when the toast closes. + */ + returnFocusTo?: HTMLElement | null; +} + interface PendingRemoval { playlist: PlaylistMeta; snackBar: MatSnackBarRef; @@ -71,7 +80,11 @@ export class SourceRemovalService { * @param next The source to switch to when `playlist` is the active one: * the caller's next row, so the switch follows the list on screen. */ - remove(playlist: PlaylistMeta, next: PlaylistMeta | null = null): void { + remove( + playlist: PlaylistMeta, + next: PlaylistMeta | null = null, + options: SourceRemovalOptions = {} + ): void { if (this.isPending(playlist._id)) { return; } @@ -108,6 +121,7 @@ export class SourceRemovalService { 'PORTALS.MULTI_SOURCE.UNDO' ), closeLabel: this.translate.instant('CLOSE'), + returnFocusTo: options.returnFocusTo ?? null, }, duration: SOURCE_REMOVAL_UNDO_MS, panelClass: 'app-toast', @@ -142,11 +156,15 @@ export class SourceRemovalService { return; } // Only take the user back if they are still where the delete left - // them; a switch they made since then wins. - const stillReplaced = - this.playlistContext.resolvedPlaylistId() === - (removal.next?._id ?? null); - if (!stillReplaced && removal.next) { + // them: on its replacement, or with no other source chosen. A switch + // they made since then wins, also when nothing replaced the source + // (it was the only row a filter showed). + const current = this.playlistContext.resolvedPlaylistId(); + const untouched = + current === null || + current === removal.playlist._id || + current === removal.next?._id; + if (!untouched) { return; } if (removal.wasOnItsRoute) { diff --git a/libs/playlist/shared/ui/src/lib/source-row/source-row.component.spec.ts b/libs/playlist/shared/ui/src/lib/source-row/source-row.component.spec.ts index 163aa7f41..b784566db 100644 --- a/libs/playlist/shared/ui/src/lib/source-row/source-row.component.spec.ts +++ b/libs/playlist/shared/ui/src/lib/source-row/source-row.component.spec.ts @@ -106,6 +106,32 @@ describe('SourceRowComponent', () => { ).toBe('bad'); }); + it("names the check's cause on the dot, and an M3U check's scope", () => { + const dotLabel = (element: HTMLElement) => + element + .querySelector('.source-tile__dot') + ?.getAttribute('aria-label'); + statuses.set('m3u', { reachability: 'bad', reason: 'auth' }); + expect(dotLabel(render(M3U))).toBe( + 'SOURCE_HEALTH.REASON.auth · SOURCE_HEALTH.M3U_SCOPE' + ); + + // Without a kept cause the dot falls back to its colour's wording. + statuses.set('xtream', { reachability: 'bad' }); + expect( + dotLabel( + render({ + ...M3U, + _id: 'xtream', + url: undefined, + serverUrl: 'http://portal.test', + username: 'joe', + password: 'hunter2', + }) + ) + ).toBe('SOURCE_HEALTH.REASON.network'); + }); + it('opens on click and on Enter, but not while busy', () => { const element = render(M3U); const opened = jest.fn(); diff --git a/libs/playlist/shared/ui/src/lib/source-row/source-tile.component.ts b/libs/playlist/shared/ui/src/lib/source-row/source-tile.component.ts index 3878801a3..ff41dd2d5 100644 --- a/libs/playlist/shared/ui/src/lib/source-row/source-tile.component.ts +++ b/libs/playlist/shared/ui/src/lib/source-row/source-tile.component.ts @@ -13,11 +13,15 @@ import { injectTranslationTick } from '@iptvnator/services'; import { getPlaylistSourceIcon, PlaylistMeta, + resolvePlaylistSourceKind, SourceReachability, SourceStatus, } from '@iptvnator/shared/interfaces'; -/** The health reason shown for each reachability when no detail is kept. */ +/** + * The health reason shown for each reachability when the check left no + * cause, as in a browser build's "checking" status. + */ const REACHABILITY_REASON: Record = { ok: 'available', bad: 'network', @@ -83,8 +87,13 @@ export class SourceTileComponent { } const reason = status.pending ? 'checking' - : REACHABILITY_REASON[status.reachability]; + : (status.reason ?? REACHABILITY_REASON[status.reachability]); const label = this.translate.instant(`SOURCE_HEALTH.REASON.${reason}`); + // An M3U check reaches the playlist URL, not its channels. + const scope = + resolvePlaylistSourceKind(this.playlist()) === 'm3u' + ? this.translate.instant('SOURCE_HEALTH.M3U_SCOPE') + : ''; const time = status.checkedAt ? new Date(status.checkedAt).toLocaleString( normalizeDateLocale(this.translate.currentLang || 'en'), @@ -97,6 +106,6 @@ export class SourceTileComponent { } ) : ''; - return [label, time].filter(Boolean).join(' · '); + return [label, scope, time].filter(Boolean).join(' · '); }); } diff --git a/libs/portal/shared/data-access/src/index.ts b/libs/portal/shared/data-access/src/index.ts index cba065db7..dab20ee0e 100644 --- a/libs/portal/shared/data-access/src/index.ts +++ b/libs/portal/shared/data-access/src/index.ts @@ -6,6 +6,7 @@ export * from './lib/live-channel-playback-queue'; export * from './lib/source-health.service'; export * from './lib/source-health-history.service'; export * from './lib/source-status.service'; +export * from './lib/source-status-checks'; export * from './lib/source-cleanup.service'; diff --git a/libs/portal/shared/data-access/src/lib/source-health-history.service.spec.ts b/libs/portal/shared/data-access/src/lib/source-health-history.service.spec.ts index eb8eb280a..842fd9729 100644 --- a/libs/portal/shared/data-access/src/lib/source-health-history.service.spec.ts +++ b/libs/portal/shared/data-access/src/lib/source-health-history.service.spec.ts @@ -36,16 +36,30 @@ describe('SourceHealthHistoryService', () => { state: 'bad', checkedAt: 900, failingSince: 100, + reason: 'network', }); }); it('stores no URL or credential', () => { create().record(['a'], failed(100)); expect(localStorage.getItem(SOURCE_HEALTH_HISTORY_STORAGE_KEY)).toBe( - '{"a":{"state":"bad","checkedAt":100,"failingSince":100}}' + '{"a":{"state":"bad","checkedAt":100,"failingSince":100,"reason":"network"}}' ); }); + it('drops a stored cause this build cannot describe', () => { + localStorage.setItem( + SOURCE_HEALTH_HISTORY_STORAGE_KEY, + '{"a":{"state":"bad","checkedAt":1,"reason":"