From 30d76a3a46459315fb94ea1996975de3bbf291ce Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:10:24 +0200 Subject: [PATCH] fix(search): keep in-flight typing when the page rewrites its query params (#1432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(search): keep in-flight typing when the page rewrites its query params `WorkspaceShellSearchSyncService` re-read `q` on every `NavigationEnd` and unconditionally called `setSearchState(...)`, which cancels the pending debounce and overwrites the search box. Any same-page navigation that carried no search intent — a downloads filter chip writing `?filter=…`, a refresh bump, or the router echoing back our own `q` — therefore ate whatever had been typed since the last applied term. While input is debouncing, ignore a navigation that stays on the same page and carries the term already applied. Real search intent (route change, back/forward, a different `q`) still syncs as before. This is the race behind the flaky `@downloads @electron keeps global and scoped libraries truthful …` e2e test: it clicks a filter chip and fills the searchbox with nothing awaited in between, so under CI load the chip's navigation lands after the keystroke, wipes the term, and `q` is never written. Reproduced deterministically by dispatching the chip click and the `input` event in the same page task; the searchbox value goes to `""` and `q` stays `null`. The new spec fails on the old code for the two regression cases and passes for the three guard cases on both. Co-Authored-By: Claude Opus 5 * fix(search): keep history authoritative and retire the debounce on Enter Two follow-ups from review of the same-page navigation guard. Greptile: the guard could not tell an app-initiated `q` echo from back/forward landing on a history entry that carries the same term. Key the exemption on `Navigation.trigger === 'imperative'` instead, so browser history always wins over in-flight typing. `lastSuccessfulNavigation` is set immediately before `NavigationEnd` is emitted, so it describes the navigation being handled. Codex: with the guard in place, an Enter commit no longer had its queued debounce cancelled as a side effect of the resulting `NavigationEnd`. Typing "Beta " and pressing Enter before the debounce expired applied the trimmed term, then the stale timeout reapplied the untrimmed one — leaving the box and URL on "Beta" while the provider store searched "Beta ". `applySearchQuery()` now cancels the pending debounce itself, which is the correct owner of that rule rather than relying on a navigation side effect. Both new tests were mutation-checked: dropping either sub-fix fails exactly its own test and no other. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- .../search-keeps-typing-during-navigation.md | 9 + docs/architecture/workspace-shell.md | 16 ++ .../helpers/workspace-shell-route-utils.ts | 12 +- ...orkspace-shell-search-sync.service.spec.ts | 243 ++++++++++++++++++ .../workspace-shell-search-sync.service.ts | 63 +++-- 5 files changed, 323 insertions(+), 20 deletions(-) create mode 100644 .changes/search-keeps-typing-during-navigation.md create mode 100644 libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.spec.ts diff --git a/.changes/search-keeps-typing-during-navigation.md b/.changes/search-keeps-typing-during-navigation.md new file mode 100644 index 000000000..00fa9dc8a --- /dev/null +++ b/.changes/search-keeps-typing-during-navigation.md @@ -0,0 +1,9 @@ +--- +type: fix +area: search +--- + +The search box no longer loses what you are typing when the page updates its +address at the same moment — switching a downloads filter and immediately +typing used to wipe the term, and fast typing could snap back to an earlier +word. diff --git a/docs/architecture/workspace-shell.md b/docs/architecture/workspace-shell.md index 5782803d7..6470d49e0 100644 --- a/docs/architecture/workspace-shell.md +++ b/docs/architecture/workspace-shell.md @@ -162,6 +162,22 @@ Search is shell-owned and route-aware: 7. Global search uses the header input as its primary input and writes the search phrase to the `q` query parameter, so history/back-forward behavior 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 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/helpers/workspace-shell-route-utils.ts b/libs/workspace/shell/feature/src/lib/workspace-shell/services/helpers/workspace-shell-route-utils.ts index 37caca7ec..9e7e103d7 100644 --- a/libs/workspace/shell/feature/src/lib/workspace-shell/services/helpers/workspace-shell-route-utils.ts +++ b/libs/workspace/shell/feature/src/lib/workspace-shell/services/helpers/workspace-shell-route-utils.ts @@ -29,6 +29,14 @@ export function getRouteQueryParam( return typeof value === 'string' ? value : ''; } +/** + * The page part of a router URL, without query params or fragment — i.e. the + * identity of "which page am I on", as opposed to how it is parameterised. + */ +export function getRoutePath(url: string): string { + return url.split('?')[0].split('#')[0]; +} + export function syncSearchQueryParam( router: Router, currentUrl: string, @@ -40,7 +48,7 @@ export function syncSearchQueryParam( return false; } - const routePath = currentUrl.split('?')[0]; + const routePath = getRoutePath(currentUrl); const queryParams = { ...router.parseUrl(currentUrl).queryParams, }; @@ -58,7 +66,7 @@ export function syncSearchQueryParam( } export function bumpRefreshQueryParam(router: Router, currentUrl: string): void { - const routePath = currentUrl.split('?')[0]; + const routePath = getRoutePath(currentUrl); const queryParams = { ...router.parseUrl(currentUrl).queryParams, refresh: Date.now().toString(), 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 new file mode 100644 index 000000000..67ba19b76 --- /dev/null +++ b/libs/workspace/shell/feature/src/lib/workspace-shell/services/workspace-shell-search-sync.service.spec.ts @@ -0,0 +1,243 @@ +import { signal } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { NavigationEnd, Router } from '@angular/router'; +import { Store } from '@ngrx/store'; +import { TranslateService } from '@ngx-translate/core'; +import { Subject, of } from 'rxjs'; +import { PlaylistContextFacade } from '@iptvnator/playlist/shared/util'; +import { StalkerStore } from '@iptvnator/portal/stalker/data-access'; +import { XtreamStore } from '@iptvnator/portal/xtream/data-access'; +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'; + +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. `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 = {}; + parsed.searchParams.forEach((value, key) => { + queryParams[key] = value; + }); + return { queryParams }; + }), + }; + + TestBed.configureTestingModule({ + providers: [ + WorkspaceShellRouteStateService, + WorkspaceShellSearchSyncService, + WorkspaceShellSearchService, + { provide: Router, useValue: router }, + { + provide: Store, + useValue: { + selectSignal: jest.fn().mockReturnValue(signal([])), + dispatch: jest.fn(), + }, + }, + { + provide: PlaylistContextFacade, + useValue: { activePlaylist: signal(null) }, + }, + { + provide: WorkspaceStartupPreferencesService, + useValue: { + getFirstAvailableWorkspacePath: jest.fn(() => '/'), + persistLastRestorablePath: jest.fn(), + showDashboard: jest.fn(() => true), + }, + }, + { + provide: TranslateService, + useValue: { + instant: jest.fn((key: string) => key), + onLangChange: of(null), + }, + }, + { + provide: RuntimeCapabilitiesService, + useValue: { + isElectron: true, + isMacOS: true, + supportsDownloads: true, + }, + }, + { + provide: XtreamStore, + useValue: { + setSearchTerm: jest.fn(), + setCategorySearchTerm: jest.fn(), + getSelectedCategory: signal(null), + }, + }, + { + provide: StalkerStore, + useValue: { + setSearchPhrase: jest.fn(), + getSelectedCategoryName: signal(''), + itvFullListActive: signal(false), + }, + }, + ], + }); + + routeState = TestBed.inject(WorkspaceShellRouteStateService); + service = TestBed.inject(WorkspaceShellSearchSyncService); + searchService = TestBed.inject(WorkspaceShellSearchService); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('keeps in-flight typing when the page writes an unrelated query param', () => { + // The downloads filter chips write `?filter=…` with replaceUrl. Under + // load that navigation can land after the first keystroke — it must + // not eat the search term the user is still typing. + service.onSearchInput('Beta Movie'); + navigateTo(DOWNLOADS_URL); + + expect(service.searchQuery()).toBe('Beta Movie'); + + jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS); + + 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', () => { + service.onSearchInput('Beta'); + jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS); + expect(service.appliedSearchQuery()).toBe('Beta'); + + // The user keeps typing while the applied term reaches the URL. + service.onSearchInput('Beta Movie'); + navigateTo(`${DOWNLOADS_URL}?q=Beta`); + + expect(service.searchQuery()).toBe('Beta Movie'); + + jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS); + + expect(service.appliedSearchQuery()).toBe('Beta Movie'); + }); + + it('clears in-flight typing when the navigation leaves the page', () => { + service.onSearchInput('Beta Movie'); + navigateTo('/workspace/sources'); + + expect(service.searchQuery()).toBe(''); + + jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS); + + expect(service.appliedSearchQuery()).toBe(''); + expect(service.searchQuery()).toBe(''); + }); + + it('adopts a same-page q that differs from the applied term', () => { + // Back/forward and the command palette change `q` in place; that is + // real search intent and wins over whatever is being typed. + service.onSearchInput('Beta Movie'); + navigateTo(`${DOWNLOADS_URL}?q=Alpha`); + + expect(service.searchQuery()).toBe('Alpha'); + + jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS); + + 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`); + + expect(service.searchQuery()).toBe('Gamma'); + expect(service.appliedSearchQuery()).toBe('Gamma'); + expect(routeState.currentUrl()).toBe(`${DOWNLOADS_URL}?q=Gamma`); + + navigateTo('/workspace/settings/general'); + + expect(service.searchQuery()).toBe(''); + expect(service.appliedSearchQuery()).toBe(''); + }); +}); 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 ced3009e6..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 @@ -7,6 +7,7 @@ import { XtreamStore } from '@iptvnator/portal/xtream/data-access'; import { parseWorkspaceShellRoute } from '@iptvnator/workspace/shell/util'; import { SEARCH_INPUT_DEBOUNCE_MS } from './helpers/workspace-shell-constants'; import { + getRoutePath, getRouteQueryParam, syncSearchQueryParam, } from './helpers/workspace-shell-route-utils'; @@ -22,17 +23,13 @@ export class WorkspaceShellSearchSyncService { private searchDebounceTimeoutId: ReturnType | null = null; + private lastSyncedUrl: string | null = null; readonly searchQuery = signal(''); 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( @@ -113,36 +110,66 @@ 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); } private syncSearchFromUrl(url: string): void { - if (parseWorkspaceShellRoute(url).usesQuerySearch) { - this.setSearchState(getRouteQueryParam(this.router, url, 'q')); + const previousUrl = this.lastSyncedUrl; + this.lastSyncedUrl = url; + + const nextTerm = parseWorkspaceShellRoute(url).usesQuerySearch + ? getRouteQueryParam(this.router, url, 'q') + : ''; + + // 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() && + this.router.lastSuccessfulNavigation()?.trigger === 'imperative' + ) { return; } - this.setSearchState(''); + this.setSearchState(nextTerm); } 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; + } + } }