diff --git a/docs/architecture/workspace-shell.md b/docs/architecture/workspace-shell.md index a647010e5..6470d49e0 100644 --- a/docs/architecture/workspace-shell.md +++ b/docs/architecture/workspace-shell.md @@ -164,13 +164,20 @@ Search is shell-owned and route-aware: matches the rest of the workspace. 8. The URL is authoritative for the search box only when it carries search intent. `WorkspaceShellSearchSyncService` re-reads `q` on every - `NavigationEnd`, but a navigation that stays on the same page and carries - the term already applied is ignored while input is still debouncing — - otherwise a page writing an unrelated query param (a downloads filter chip, - a refresh bump) or the router echoing back our own `q` would cancel the - pending debounce and reset the box, eating everything typed since. Pages are - free to write their own query params while the user types; they must not - assume the shell will re-apply the search afterwards. + `NavigationEnd`, but an **app-initiated** navigation that stays on the same + page and carries the term already applied is ignored while input is still + debouncing — otherwise a page writing an unrelated query param (a downloads + filter chip, a refresh bump) or the router echoing back our own `q` would + cancel the pending debounce and reset the box, eating everything typed + since. Pages are free to write their own query params while the user types; + they must not assume the shell will re-apply the search afterwards. +9. Browser history overrides that guard. The exemption is keyed on + `Navigation.trigger === 'imperative'`, so back/forward always re-applies + what the history entry carries, even mid-typing. +10. Applying a term explicitly supersedes a queued one. `applySearchQuery()` + cancels any pending debounce, so the Enter key committing a trimmed term + cannot be overwritten a moment later by the untrimmed keystroke still + waiting behind it. Rail navigation is also shell-owned: diff --git a/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.spec.ts b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.spec.ts index 5f09942f4..67ba19b76 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.spec.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.spec.ts @@ -11,6 +11,7 @@ import { RuntimeCapabilitiesService } from '@iptvnator/services'; import { WorkspaceStartupPreferencesService } from '@iptvnator/workspace/shell/util'; import { SEARCH_INPUT_DEBOUNCE_MS } from './helpers/workspace-shell-constants'; import { WorkspaceShellRouteStateService } from './workspace-shell-route-state.service'; +import { WorkspaceShellSearchService } from './workspace-shell-search.service'; import { WorkspaceShellSearchSyncService } from './workspace-shell-search-sync.service'; const DOWNLOADS_URL = '/workspace/downloads'; @@ -19,28 +20,41 @@ describe('WorkspaceShellSearchSyncService', () => { let service: WorkspaceShellSearchSyncService; let routeState: WorkspaceShellRouteStateService; let events: Subject; + let searchService: WorkspaceShellSearchService; + let trigger: 'imperative' | 'popstate'; let router: { url: string; events: Subject; navigate: jest.Mock; navigateByUrl: jest.Mock; parseUrl: jest.Mock; + lastSuccessfulNavigation: () => { trigger: string }; }; - /** Emits the NavigationEnd both shell services listen for. */ - function navigateTo(url: string): void { + /** + * Emits the NavigationEnd both shell services listen for. `origin` mirrors + * `Navigation.trigger`: 'imperative' is the app navigating, 'popstate' is + * the user moving through browser history. + */ + function navigateTo( + url: string, + origin: 'imperative' | 'popstate' = 'imperative' + ): void { router.url = url; + trigger = origin; events.next(new NavigationEnd(1, url, url)); } beforeEach(() => { jest.useFakeTimers(); events = new Subject(); + trigger = 'imperative'; router = { url: DOWNLOADS_URL, events, navigate: jest.fn().mockResolvedValue(true), navigateByUrl: jest.fn().mockResolvedValue(true), + lastSuccessfulNavigation: () => ({ trigger }), parseUrl: jest.fn((url: string) => { const parsed = new URL(url, 'http://localhost'); const queryParams: Record = {}; @@ -55,6 +69,7 @@ describe('WorkspaceShellSearchSyncService', () => { providers: [ WorkspaceShellRouteStateService, WorkspaceShellSearchSyncService, + WorkspaceShellSearchService, { provide: Router, useValue: router }, { provide: Store, @@ -95,17 +110,23 @@ describe('WorkspaceShellSearchSyncService', () => { useValue: { setSearchTerm: jest.fn(), setCategorySearchTerm: jest.fn(), + getSelectedCategory: signal(null), }, }, { provide: StalkerStore, - useValue: { setSearchPhrase: jest.fn() }, + useValue: { + setSearchPhrase: jest.fn(), + getSelectedCategoryName: signal(''), + itvFullListActive: signal(false), + }, }, ], }); routeState = TestBed.inject(WorkspaceShellRouteStateService); service = TestBed.inject(WorkspaceShellSearchSyncService); + searchService = TestBed.inject(WorkspaceShellSearchService); }); afterEach(() => { @@ -125,6 +146,13 @@ describe('WorkspaceShellSearchSyncService', () => { expect(service.appliedSearchQuery()).toBe('Beta Movie'); expect(service.searchQuery()).toBe('Beta Movie'); + + // The term has to reach the URL — that is what the page reads back. + TestBed.flushEffects(); + expect(router.navigateByUrl).toHaveBeenCalledWith( + `${DOWNLOADS_URL}?q=Beta+Movie`, + { replaceUrl: true } + ); }); it('does not roll typing back to the term its own q navigation echoes', () => { @@ -168,6 +196,38 @@ describe('WorkspaceShellSearchSyncService', () => { expect(service.appliedSearchQuery()).toBe('Alpha'); }); + it('lets browser history win over in-flight typing', () => { + // Back/forward is authoritative even when the entry restores the term + // already applied — only app-initiated navigations are treated as + // echoes worth ignoring. + service.onSearchInput('Beta'); + jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS); + service.onSearchInput('Beta Movie'); + + navigateTo(`${DOWNLOADS_URL}?q=Beta`, 'popstate'); + + expect(service.searchQuery()).toBe('Beta'); + + jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS); + + expect(service.appliedSearchQuery()).toBe('Beta'); + }); + + it('does not let a pending keystroke reapply behind an Enter commit', () => { + // Enter commits the trimmed term immediately; the debounce still + // holding the untrimmed keystroke must not overwrite it afterwards. + service.onSearchInput('Beta Movie '); + searchService.onSearchEnter('Beta Movie '); + + expect(service.appliedSearchQuery()).toBe('Beta Movie'); + + navigateTo(`${DOWNLOADS_URL}?q=Beta+Movie`); + jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS); + + expect(service.appliedSearchQuery()).toBe('Beta Movie'); + expect(service.searchQuery()).toBe('Beta Movie'); + }); + it('syncs the search box from the url when nothing is being typed', () => { navigateTo(`${DOWNLOADS_URL}?q=Gamma`); diff --git a/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.ts b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.ts index 2a4135d11..c75b48d1e 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.ts @@ -29,12 +29,7 @@ export class WorkspaceShellSearchSyncService { readonly appliedSearchQuery = signal(''); constructor() { - this.destroyRef.onDestroy(() => { - if (this.searchDebounceTimeoutId !== null) { - clearTimeout(this.searchDebounceTimeoutId); - this.searchDebounceTimeoutId = null; - } - }); + this.destroyRef.onDestroy(() => this.cancelPendingSearchApply()); this.router.events .pipe( @@ -115,16 +110,18 @@ export class WorkspaceShellSearchSyncService { } setSearchState(value: string): void { - if (this.searchDebounceTimeoutId !== null) { - clearTimeout(this.searchDebounceTimeoutId); - this.searchDebounceTimeoutId = null; - } - + this.cancelPendingSearchApply(); this.searchQuery.set(value); this.appliedSearchQuery.set(value); } + /** + * Applies a term now. This supersedes a queued debounce — pressing Enter + * commits the trimmed term, and the keystroke that is still waiting must + * not reapply the untrimmed one behind it. + */ applySearchQuery(value: string): void { + this.cancelPendingSearchApply(); this.appliedSearchQuery.set(value); } @@ -136,18 +133,24 @@ export class WorkspaceShellSearchSyncService { ? getRouteQueryParam(this.router, url, 'q') : ''; - // A navigation that stays on the same page and carries the term we - // already applied brings no search intent of its own: it is either an - // unrelated query param the page wrote (a filter chip, a refresh bump) - // or the router echoing back our own `q`. Syncing anyway would cancel - // the pending debounce and reset the box to the applied term, silently - // eating everything typed since — including the whole word, when the - // first keystroke has not been applied yet. + // An app-initiated navigation that stays on the same page and carries + // the term we already applied brings no search intent of its own: it is + // either an unrelated query param the page wrote (a filter chip, a + // refresh bump) or the router echoing back our own `q`. Syncing anyway + // would cancel the pending debounce and reset the box to the applied + // term, silently eating everything typed since — including the whole + // word, when the first keystroke has not been applied yet. + // + // The trigger check keeps that narrow: history is always authoritative, + // so back/forward re-applies what the entry carries even mid-typing. + // `lastSuccessfulNavigation` is set immediately before `NavigationEnd` + // is emitted, so it describes the navigation being handled here. if ( this.searchDebounceTimeoutId !== null && previousUrl !== null && getRoutePath(url) === getRoutePath(previousUrl) && - nextTerm === this.appliedSearchQuery() + nextTerm === this.appliedSearchQuery() && + this.router.lastSuccessfulNavigation()?.trigger === 'imperative' ) { return; } @@ -156,13 +159,17 @@ export class WorkspaceShellSearchSyncService { } private scheduleSearchApply(value: string): void { - if (this.searchDebounceTimeoutId !== null) { - clearTimeout(this.searchDebounceTimeoutId); - } - + this.cancelPendingSearchApply(); this.searchDebounceTimeoutId = setTimeout(() => { this.searchDebounceTimeoutId = null; this.applySearchQuery(value); }, SEARCH_INPUT_DEBOUNCE_MS); } + + private cancelPendingSearchApply(): void { + if (this.searchDebounceTimeoutId !== null) { + clearTimeout(this.searchDebounceTimeoutId); + this.searchDebounceTimeoutId = null; + } + } }