mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-08 17:06:15 -08:00
fix(workspace): lead first-entry Back to the parent without the Navigation API
Review follow-ups (Greptile): - Without the Navigation API (older Safari and Firefox) back() always called Location.back(), so a page that opened the session still left the app. The service now tracks the router's in-app history depth there (trackRouterHistoryDepth): first navigation 0, push +1, replacement keeps it, a traversal restores the depth recorded for its entry. Depth 0 opens the parent; an unknown depth (an entry from before a reload) keeps Location.back(). - Stalker's Discover (movie/tv section), actor and search pages now have tests that they hand the service the parent under the portal :id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e7e61da5f3
commit
0a4f218f56
5 files changed
+320
-18
No files matched your search
@@ -171,8 +171,12 @@ were reached by navigation.
|
||||
Back is history Back calls `WorkspaceBackNavigationService.back(resolveParent)`
|
||||
instead of `Location.back()`. It runs `Location.back()` while the previous
|
||||
entry is an in-app one, by the same Navigation API test as the history
|
||||
fallback, and also when that is unknown because the API is missing: there a
|
||||
page reached in the app must not jump to its parent. Otherwise the page opened
|
||||
fallback. Without the API (older Safari and Firefox) the router's history
|
||||
depth decides (`trackRouterHistoryDepth`): the document's first navigation is
|
||||
depth 0, a push adds one, a replacement keeps it and a traversal restores the
|
||||
depth recorded for its entry. A traversal to an entry from before a reload
|
||||
leaves the depth unknown and keeps `Location.back()`, which then has a
|
||||
previous entry. Otherwise the page opened
|
||||
the session (a deep link, a reload or a restored view), where
|
||||
`Location.back()` does nothing in Electron and leaves the app in a browser.
|
||||
The service then navigates to the page's parent with `replaceUrl`, so history
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
import type { DestroyRef } from '@angular/core';
|
||||
import {
|
||||
NavigationCancel,
|
||||
NavigationEnd,
|
||||
NavigationError,
|
||||
NavigationStart,
|
||||
type Router,
|
||||
} from '@angular/router';
|
||||
|
||||
/**
|
||||
* In-app history depth from router events, for runtimes without the
|
||||
* Navigation API (older Safari and Firefox), where the browser does not say
|
||||
* whether the previous entry belongs to this app session.
|
||||
*
|
||||
* The document's first navigation is depth 0, a push adds one, a replacement
|
||||
* or a navigation that skips the location keeps the depth, and a traversal
|
||||
* restores the depth recorded for the entry it returns to. Entries from
|
||||
* before a reload were recorded by another document, so a traversal to one
|
||||
* leaves the depth unknown (null).
|
||||
*/
|
||||
export function trackRouterHistoryDepth(
|
||||
router: Pick<Router, 'events' | 'currentNavigation'>,
|
||||
destroyRef: Pick<DestroyRef, 'onDestroy'>
|
||||
): () => number | null {
|
||||
let depth: number | null = null;
|
||||
let pending: number | null = null;
|
||||
let started = false;
|
||||
const depthByNavigationId = new Map<number, number>();
|
||||
// A Router without events (a partial test double) leaves the depth
|
||||
// unknown, which keeps browser history Back.
|
||||
if (!router.events) return () => null;
|
||||
|
||||
const subscription = router.events.subscribe((event) => {
|
||||
if (event instanceof NavigationStart) {
|
||||
if (!started) {
|
||||
pending = 0;
|
||||
} else if (event.navigationTrigger === 'popstate') {
|
||||
const restoredId = event.restoredState?.navigationId;
|
||||
pending =
|
||||
restoredId === undefined
|
||||
? null
|
||||
: (depthByNavigationId.get(restoredId) ?? null);
|
||||
} else {
|
||||
const extras = router.currentNavigation()?.extras;
|
||||
pending =
|
||||
extras?.replaceUrl || extras?.skipLocationChange
|
||||
? depth
|
||||
: (depth ?? 0) + 1;
|
||||
}
|
||||
started = true;
|
||||
} else if (event instanceof NavigationEnd) {
|
||||
depth = pending;
|
||||
if (depth !== null) depthByNavigationId.set(event.id, depth);
|
||||
} else if (
|
||||
event instanceof NavigationCancel ||
|
||||
event instanceof NavigationError
|
||||
) {
|
||||
pending = depth;
|
||||
}
|
||||
});
|
||||
destroyRef.onDestroy(() => subscription.unsubscribe());
|
||||
return () => depth;
|
||||
}
|
||||
@@ -1,7 +1,13 @@
|
||||
import { Location } from '@angular/common';
|
||||
import { signal } from '@angular/core';
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { Router } from '@angular/router';
|
||||
import {
|
||||
Event as RouterEvent,
|
||||
NavigationEnd,
|
||||
NavigationStart,
|
||||
Router,
|
||||
} from '@angular/router';
|
||||
import { Subject } from 'rxjs';
|
||||
import { WorkspaceBackTarget } from '@iptvnator/portal/shared/util';
|
||||
import {
|
||||
WORKSPACE_HISTORY_NAVIGATION,
|
||||
@@ -45,6 +51,35 @@ describe('WorkspaceBackNavigationService', () => {
|
||||
const back = jest.fn();
|
||||
const navigate = jest.fn().mockResolvedValue(true);
|
||||
const navigateByUrl = jest.fn().mockResolvedValue(true);
|
||||
let routerEvents = new Subject<RouterEvent>();
|
||||
const currentNavigation = signal<{
|
||||
extras: { replaceUrl?: boolean };
|
||||
} | null>(null);
|
||||
let navigationId = 0;
|
||||
|
||||
/** A router navigation as the history depth tracker sees it. */
|
||||
function routerNavigation(
|
||||
options: {
|
||||
popstateTo?: number;
|
||||
replaceUrl?: boolean;
|
||||
} = {}
|
||||
): number {
|
||||
const id = ++navigationId;
|
||||
currentNavigation.set({ extras: { replaceUrl: options.replaceUrl } });
|
||||
routerEvents.next(
|
||||
new NavigationStart(
|
||||
id,
|
||||
`/page-${id}`,
|
||||
options.popstateTo === undefined ? 'imperative' : 'popstate',
|
||||
options.popstateTo === undefined
|
||||
? null
|
||||
: { navigationId: options.popstateTo }
|
||||
)
|
||||
);
|
||||
routerEvents.next(new NavigationEnd(id, `/page-${id}`, `/page-${id}`));
|
||||
currentNavigation.set(null);
|
||||
return id;
|
||||
}
|
||||
|
||||
function createService(
|
||||
history: FakeHistory | null = null
|
||||
@@ -52,11 +87,21 @@ describe('WorkspaceBackNavigationService', () => {
|
||||
back.mockReset();
|
||||
navigate.mockClear();
|
||||
navigateByUrl.mockClear();
|
||||
routerEvents = new Subject<RouterEvent>();
|
||||
navigationId = 0;
|
||||
TestBed.resetTestingModule();
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
{ provide: Location, useValue: { back } },
|
||||
{ provide: Router, useValue: { navigate, navigateByUrl } },
|
||||
{
|
||||
provide: Router,
|
||||
useValue: {
|
||||
navigate,
|
||||
navigateByUrl,
|
||||
events: routerEvents,
|
||||
currentNavigation,
|
||||
},
|
||||
},
|
||||
{
|
||||
provide: WORKSPACE_HISTORY_NAVIGATION,
|
||||
useValue: history as unknown as WorkspaceHistoryNavigation,
|
||||
@@ -272,16 +317,75 @@ describe('WorkspaceBackNavigationService', () => {
|
||||
expect(navigateByUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps browser history without the Navigation API', () => {
|
||||
// The history is unknown there, so a reached page must not
|
||||
// jump to its parent.
|
||||
const service = createService(null);
|
||||
const parent = jest.fn(() => '/workspace/dashboard');
|
||||
describe('without the Navigation API', () => {
|
||||
// The router's history depth decides there (older Safari and
|
||||
// Firefox): a page that opened the session must not leave the app.
|
||||
it('opens the parent of the page that opened the session', async () => {
|
||||
const service = createService(null);
|
||||
routerNavigation();
|
||||
|
||||
service.back(parent);
|
||||
service.back(() => '/workspace/dashboard');
|
||||
await Promise.resolve();
|
||||
|
||||
expect(back).toHaveBeenCalledTimes(1);
|
||||
expect(parent).not.toHaveBeenCalled();
|
||||
expect(back).not.toHaveBeenCalled();
|
||||
expect(navigateByUrl).toHaveBeenCalledWith(
|
||||
'/workspace/dashboard',
|
||||
{ replaceUrl: true }
|
||||
);
|
||||
});
|
||||
|
||||
it('goes back in history after the router pushed a page', () => {
|
||||
const service = createService(null);
|
||||
routerNavigation();
|
||||
routerNavigation();
|
||||
const parent = jest.fn(() => '/workspace/dashboard');
|
||||
|
||||
service.back(parent);
|
||||
|
||||
expect(back).toHaveBeenCalledTimes(1);
|
||||
expect(parent).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('treats a replacement as the same entry', async () => {
|
||||
const service = createService(null);
|
||||
routerNavigation();
|
||||
routerNavigation({ replaceUrl: true });
|
||||
|
||||
service.back(() => ['/workspace', 'sources']);
|
||||
await Promise.resolve();
|
||||
|
||||
expect(back).not.toHaveBeenCalled();
|
||||
expect(navigate).toHaveBeenCalledWith(
|
||||
['/workspace', 'sources'],
|
||||
{ replaceUrl: true }
|
||||
);
|
||||
});
|
||||
|
||||
it('opens the parent again after Back returned to the first page', async () => {
|
||||
const service = createService(null);
|
||||
const first = routerNavigation();
|
||||
routerNavigation();
|
||||
routerNavigation({ popstateTo: first });
|
||||
|
||||
service.back(() => '/workspace/dashboard');
|
||||
await Promise.resolve();
|
||||
|
||||
expect(back).not.toHaveBeenCalled();
|
||||
expect(navigateByUrl).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('keeps browser history on an entry from before a reload', () => {
|
||||
const service = createService(null);
|
||||
routerNavigation();
|
||||
// Restores an entry recorded by the previous document.
|
||||
routerNavigation({ popstateTo: 42 });
|
||||
const parent = jest.fn(() => '/workspace/dashboard');
|
||||
|
||||
service.back(parent);
|
||||
|
||||
expect(back).toHaveBeenCalledTimes(1);
|
||||
expect(parent).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from '@angular/core';
|
||||
import { Router } from '@angular/router';
|
||||
import { WorkspaceBackTarget } from '@iptvnator/portal/shared/util';
|
||||
import { trackRouterHistoryDepth } from './router-history-depth';
|
||||
|
||||
/** The parts of the browser's Navigation API the history fallback reads. */
|
||||
export type WorkspaceHistoryNavigation = Pick<
|
||||
@@ -60,6 +61,10 @@ export class WorkspaceBackNavigationService {
|
||||
private readonly history = inject(WORKSPACE_HISTORY_NAVIGATION);
|
||||
private readonly targets = signal<readonly WorkspaceBackTarget[]>([]);
|
||||
private readonly canGoBackInApp = signal(false);
|
||||
/** In-app history depth where the Navigation API is missing. */
|
||||
private readonly routerDepth = this.history
|
||||
? () => null
|
||||
: trackRouterHistoryDepth(this.router, inject(DestroyRef));
|
||||
|
||||
/**
|
||||
* Generic Back to the previous page. It advertises no Escape (no page
|
||||
@@ -97,16 +102,21 @@ export class WorkspaceBackNavigationService {
|
||||
|
||||
/**
|
||||
* Back for a page with a parent route. Browser history while the previous
|
||||
* entry is an in-app one, and while that is unknown (no Navigation API).
|
||||
* Otherwise the page opened the session (deep link, reload, restored
|
||||
* view), where `Location.back()` would do nothing in Electron or leave
|
||||
* the app in a browser: the parent replaces the current entry, so
|
||||
* history Back cannot return to the page just left.
|
||||
* entry is an in-app one. Otherwise the page opened the session (deep
|
||||
* link, reload, restored view), where `Location.back()` would do nothing
|
||||
* in Electron or leave the app in a browser: the parent replaces the
|
||||
* current entry, so history Back cannot return to the page just left.
|
||||
* Without the Navigation API the router's history depth decides, and an
|
||||
* unknown depth (a traversal to an entry from before a reload) keeps
|
||||
* browser history, which then has a previous entry.
|
||||
*/
|
||||
back(
|
||||
resolveParent: () => WorkspaceBackParent | Promise<WorkspaceBackParent>
|
||||
): void {
|
||||
if (!this.history || this.canGoBackInApp()) {
|
||||
const inApp = this.history
|
||||
? this.canGoBackInApp()
|
||||
: (this.routerDepth() ?? 1) > 0;
|
||||
if (inApp) {
|
||||
this.location.back();
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
import { TestBed } from '@angular/core/testing';
|
||||
import { ActivatedRoute, provideRouter } from '@angular/router';
|
||||
import { of } from 'rxjs';
|
||||
import {
|
||||
WorkspaceBackNavigationService,
|
||||
WorkspaceBackParent,
|
||||
} from '@iptvnator/portal/shared/data-access';
|
||||
import {
|
||||
CatalogTitleMatchService,
|
||||
TmdbEnrichmentService,
|
||||
} from '@iptvnator/services';
|
||||
import { StalkerActorRouteComponent } from './stalker-actor-route.component';
|
||||
import { StalkerDiscoverRouteComponent } from './stalker-discover-route.component';
|
||||
import { StalkerSearchComponent } from './stalker-search/stalker-search.component';
|
||||
|
||||
/**
|
||||
* Stalker's Discover, actor and search pages lead Back to a parent route
|
||||
* when they opened the session. Their routes are Stalker's own, so the
|
||||
* Xtream tests do not cover this wiring: each page must hand the service
|
||||
* the parent under the portal `:id` of its ancestor route.
|
||||
*/
|
||||
describe('Stalker workspace Back parents', () => {
|
||||
const back = jest.fn();
|
||||
let queryParams: Record<string, string>;
|
||||
|
||||
function activatedRoute(): ActivatedRoute {
|
||||
const params = { id: 'stalker-1' };
|
||||
return {
|
||||
params: of(params),
|
||||
queryParams: of(queryParams),
|
||||
snapshot: { queryParams, params, pathFromRoot: [] },
|
||||
pathFromRoot: [],
|
||||
} as unknown as ActivatedRoute;
|
||||
}
|
||||
|
||||
function configure(): void {
|
||||
TestBed.configureTestingModule({
|
||||
providers: [
|
||||
provideRouter([]),
|
||||
{ provide: ActivatedRoute, useFactory: activatedRoute },
|
||||
{ provide: WorkspaceBackNavigationService, useValue: { back } },
|
||||
{
|
||||
provide: TmdbEnrichmentService,
|
||||
useValue: { discoverTitles: jest.fn() },
|
||||
},
|
||||
{
|
||||
provide: CatalogTitleMatchService,
|
||||
useValue: { isAvailable: false, matchTitles: jest.fn() },
|
||||
},
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
/** The parent the page handed to `back()` on its last call. */
|
||||
async function lastParent(): Promise<WorkspaceBackParent> {
|
||||
const resolveParent = back.mock.calls.at(-1)?.[0] as
|
||||
| (() => WorkspaceBackParent | Promise<WorkspaceBackParent>)
|
||||
| undefined;
|
||||
if (!resolveParent) throw new Error('Expected a back() call');
|
||||
return resolveParent();
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
back.mockReset();
|
||||
queryParams = {};
|
||||
configure();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['movie', 'vod'],
|
||||
['tv', 'series'],
|
||||
])('leads a %s Discover page to the %s list', async (type, section) => {
|
||||
queryParams = { type, year: '1990' };
|
||||
const page = TestBed.runInInjectionContext(
|
||||
() => new StalkerDiscoverRouteComponent()
|
||||
);
|
||||
|
||||
page.goBack();
|
||||
|
||||
expect(await lastParent()).toEqual([
|
||||
'/workspace',
|
||||
'stalker',
|
||||
'stalker-1',
|
||||
section,
|
||||
]);
|
||||
});
|
||||
|
||||
it('leads an actor page to the portal root, which redirects to its default section', async () => {
|
||||
const page = TestBed.runInInjectionContext(
|
||||
() => new StalkerActorRouteComponent()
|
||||
);
|
||||
|
||||
page.goBack();
|
||||
|
||||
expect(await lastParent()).toEqual([
|
||||
'/workspace',
|
||||
'stalker',
|
||||
'stalker-1',
|
||||
]);
|
||||
});
|
||||
|
||||
it('leads the search page to the portal root', async () => {
|
||||
// The search page needs the whole portal store to construct; Back
|
||||
// only uses its route and the back service.
|
||||
const page = Object.assign(
|
||||
Object.create(StalkerSearchComponent.prototype),
|
||||
{
|
||||
activatedRoute: activatedRoute(),
|
||||
backNavigation: { back },
|
||||
}
|
||||
) as StalkerSearchComponent;
|
||||
|
||||
page.goBack();
|
||||
|
||||
expect(await lastParent()).toEqual([
|
||||
'/workspace',
|
||||
'stalker',
|
||||
'stalker-1',
|
||||
]);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user