mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-08 17:06:15 -08:00
fix(settings): let search reveals win over pending input and gate embedded MPV rows
- A reveal (result click, command palette, Enter) now cancels a search keystroke still waiting for its debounce, so its q navigation can no longer supersede the reveal and leave the results open. - Embedded MPV extra options and auto-reconnect require a lazily probed embedded MPV capability; frame copy also needs frameCopyAvailable, so search never offers a row the settings page cannot render. - Keyboard users keep a focus-visible ring on the revealed row after the highlight fades. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
3b3357713f
commit
b78b3a75e7
13 files changed
+200
-30
No files matched your search
@@ -66,6 +66,8 @@ export class SettingsSearchFacade {
|
||||
});
|
||||
|
||||
constructor() {
|
||||
void this.settingsSearch.ensureEmbeddedMpvSupportLoaded();
|
||||
|
||||
effect(() => {
|
||||
if (!this.isSearching()) {
|
||||
this.settingsCtx.setMatchCounts(null);
|
||||
|
||||
@@ -115,8 +115,9 @@
|
||||
}
|
||||
|
||||
// A row opened from settings search or the command palette. It receives
|
||||
// programmatic focus (tabindex -1); the highlight is its focus indicator, so
|
||||
// the default outline is dropped. The wash fills the row's own padding box
|
||||
// programmatic focus (tabindex -1). Keyboard users keep a focus ring after
|
||||
// the temporary highlight fades; pointer users rely on the highlight alone,
|
||||
// as with any mouse-driven focus. The wash fills the row's own padding box
|
||||
// and two offset shadows widen it past the flush-left row edges without
|
||||
// shifting layout or painting over the separators above and below.
|
||||
.setting-item[data-setting-id] {
|
||||
@@ -125,6 +126,13 @@
|
||||
&:focus {
|
||||
outline: none;
|
||||
}
|
||||
|
||||
&:focus-visible {
|
||||
outline: 2px solid
|
||||
color-mix(in srgb, var(--app-selection-color) 60%, transparent);
|
||||
outline-offset: 6px;
|
||||
border-radius: 4px;
|
||||
}
|
||||
}
|
||||
|
||||
.setting-item--revealed {
|
||||
|
||||
@@ -270,6 +270,10 @@ and from the command palette. Both use the same index and ranking.
|
||||
description of the current language plus the keywords; every query token
|
||||
must match (AND), and a label prefix outranks a word start, which outranks
|
||||
an inner match. Rows whose `requires` the runtime lacks are never returned.
|
||||
Embedded MPV rows depend on a lazy support probe
|
||||
(`ensureEmbeddedMpvSupportLoaded()`), run when the settings page or the
|
||||
command palette opens, never from shell bootstrap; frame copy also needs
|
||||
`frameCopyAvailable`, matching the settings page gate.
|
||||
5. Settings routes use `local-filter` search mode, so the term lives in `q`.
|
||||
While `q` is set, the settings page shows ranked results in place of the
|
||||
section page and the settings context panel shows per-section match
|
||||
@@ -280,9 +284,12 @@ and from the command palette. Both use the same index and ranking.
|
||||
to the section page without `q` (which clears the box) and the page
|
||||
scrolls to, focuses, and briefly highlights the row once the form is
|
||||
hydrated. A row hidden by the current form state falls back to its
|
||||
`fallbackId`, the control that makes it appear. Enter deliberately does
|
||||
not apply the term first, because the `q` sync navigation would supersede
|
||||
the reveal navigation.
|
||||
`fallbackId`, the control that makes it appear. A reveal must win over the
|
||||
typed term: `WorkspaceShellSearchSyncService` drops a keystroke still
|
||||
waiting for its debounce through `onReveal()`, and Enter does not apply
|
||||
the term first, because either `q` sync navigation would supersede the
|
||||
reveal navigation. Keyboard users keep a `:focus-visible` ring on the row
|
||||
after the highlight fades.
|
||||
7. `Ctrl/Cmd+F` on settings focuses the header search instead of opening
|
||||
global search.
|
||||
|
||||
|
||||
+6
-4
@@ -52,11 +52,13 @@ export class WorkspaceShellCommandPaletteService {
|
||||
return;
|
||||
}
|
||||
|
||||
const embeddedMpvSupportLoad =
|
||||
this.playerCommands.ensureEmbeddedMpvSupportLoaded();
|
||||
if (embeddedMpvSupportLoad) {
|
||||
const embeddedMpvSupportLoads = [
|
||||
this.playerCommands.ensureEmbeddedMpvSupportLoaded(),
|
||||
this.settingsSearch.ensureEmbeddedMpvSupportLoaded(),
|
||||
].filter((load): load is Promise<void> => load !== undefined);
|
||||
if (embeddedMpvSupportLoads.length > 0) {
|
||||
this.commandPaletteOpening = true;
|
||||
void embeddedMpvSupportLoad.finally(() => {
|
||||
void Promise.allSettled(embeddedMpvSupportLoads).finally(() => {
|
||||
this.commandPaletteOpening = false;
|
||||
this.openResolvedCommandPalette(ctx, initialQuery);
|
||||
});
|
||||
|
||||
+16
@@ -9,6 +9,7 @@ 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 { SettingsSearchService } from '@iptvnator/workspace/shell/util/settings-search';
|
||||
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';
|
||||
@@ -133,6 +134,21 @@ describe('WorkspaceShellSearchSyncService', () => {
|
||||
jest.useRealTimers();
|
||||
});
|
||||
|
||||
it('drops a still-debouncing keystroke when a settings result is opened', () => {
|
||||
// Typing one more letter and clicking a visible settings result
|
||||
// within the debounce window: applying that keystroke afterwards
|
||||
// would start a `q` navigation that supersedes the reveal.
|
||||
service.onSearchInput('them');
|
||||
TestBed.inject(SettingsSearchService).reveal({
|
||||
id: 'theme',
|
||||
section: 'general',
|
||||
labelKey: 'SETTINGS.THEME',
|
||||
});
|
||||
jest.advanceTimersByTime(SEARCH_INPUT_DEBOUNCE_MS);
|
||||
|
||||
expect(service.appliedSearchQuery()).toBe('');
|
||||
});
|
||||
|
||||
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
|
||||
|
||||
+8
-8
@@ -5,6 +5,7 @@ import { filter } from 'rxjs';
|
||||
import { StalkerStore } from '@iptvnator/portal/stalker/data-access';
|
||||
import { XtreamStore } from '@iptvnator/portal/xtream/data-access';
|
||||
import { parseWorkspaceShellRoute } from '@iptvnator/workspace/shell/util';
|
||||
import { SettingsSearchService } from '@iptvnator/workspace/shell/util/settings-search';
|
||||
import { SEARCH_INPUT_DEBOUNCE_MS } from './helpers/workspace-shell-constants';
|
||||
import {
|
||||
getRoutePath,
|
||||
@@ -30,6 +31,13 @@ export class WorkspaceShellSearchSyncService {
|
||||
|
||||
constructor() {
|
||||
this.destroyRef.onDestroy(() => this.cancelPendingSearchApply());
|
||||
// Opening a settings search result navigates away from the typed
|
||||
// term; a keystroke still debouncing must not apply afterwards.
|
||||
this.destroyRef.onDestroy(
|
||||
inject(SettingsSearchService).onReveal(() =>
|
||||
this.cancelPendingSearchApply()
|
||||
)
|
||||
);
|
||||
|
||||
this.router.events
|
||||
.pipe(
|
||||
@@ -139,14 +147,6 @@ export class WorkspaceShellSearchSyncService {
|
||||
this.appliedSearchQuery.set(value.trim());
|
||||
}
|
||||
|
||||
/**
|
||||
* Drops a keystroke still waiting for the debounce without applying it,
|
||||
* for callers that are about to navigate away from the typed term.
|
||||
*/
|
||||
discardPendingInput(): void {
|
||||
this.cancelPendingSearchApply();
|
||||
}
|
||||
|
||||
private syncSearchFromUrl(url: string): void {
|
||||
const previousUrl = this.lastSyncedUrl;
|
||||
this.lastSyncedUrl = url;
|
||||
|
||||
+2
-2
@@ -189,11 +189,11 @@ export class WorkspaceShellSearchService {
|
||||
// Enter on settings jumps to the best match, like picking the first
|
||||
// result. The reveal navigation drops `q`, which clears the box. The
|
||||
// term is deliberately not applied: applying would make the `q` sync
|
||||
// start its own navigation, superseding the reveal navigation.
|
||||
// start its own navigation, superseding the reveal navigation (the
|
||||
// reveal also cancels a keystroke still waiting for its debounce).
|
||||
if (this.routeState.currentRoute().kind === 'settings') {
|
||||
const [bestMatch] = this.settingsSearch.search(trimmedValue, 1);
|
||||
if (bestMatch) {
|
||||
this.searchSync.discardPendingInput();
|
||||
this.settingsSearch.reveal(bestMatch.entry);
|
||||
} else {
|
||||
this.searchSync.applySearchQuery(trimmedValue);
|
||||
|
||||
-3
@@ -31,7 +31,6 @@ describe('WorkspaceShellSearchService on settings routes', () => {
|
||||
searchQuery: ReturnType<typeof signal<string>>;
|
||||
appliedSearchQuery: ReturnType<typeof signal<string>>;
|
||||
applySearchQuery: jest.Mock;
|
||||
discardPendingInput: jest.Mock;
|
||||
};
|
||||
let settingsSearch: { search: jest.Mock; reveal: jest.Mock };
|
||||
|
||||
@@ -43,7 +42,6 @@ describe('WorkspaceShellSearchService on settings routes', () => {
|
||||
searchQuery: signal(''),
|
||||
appliedSearchQuery: signal(''),
|
||||
applySearchQuery: jest.fn(),
|
||||
discardPendingInput: jest.fn(),
|
||||
};
|
||||
settingsSearch = {
|
||||
search: jest.fn((query: string) =>
|
||||
@@ -112,7 +110,6 @@ describe('WorkspaceShellSearchService on settings routes', () => {
|
||||
|
||||
expect(settingsSearch.search).toHaveBeenCalledWith('theme', 1);
|
||||
expect(settingsSearch.reveal).toHaveBeenCalledWith(THEME);
|
||||
expect(searchSync.discardPendingInput).toHaveBeenCalled();
|
||||
// Applying would start the `q` sync navigation and supersede the
|
||||
// reveal navigation.
|
||||
expect(searchSync.applySearchQuery).not.toHaveBeenCalled();
|
||||
|
||||
+1
-1
@@ -1152,7 +1152,7 @@ describe('WorkspaceShellFacade', () => {
|
||||
expect(dialog.open).not.toHaveBeenCalled();
|
||||
|
||||
resolveSupport();
|
||||
await Promise.resolve();
|
||||
await new Promise((resolve) => setTimeout(resolve));
|
||||
|
||||
expect(dialog.open).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
+3
-4
@@ -93,8 +93,7 @@ export const PLAYBACK_SETTINGS_SEARCH_ENTRIES: readonly SettingsSearchEntry[] =
|
||||
labelKey: 'SETTINGS.EMBEDDED_MPV_FRAME_COPY',
|
||||
descriptionKey: 'SETTINGS.EMBEDDED_MPV_FRAME_COPY_DESCRIPTION',
|
||||
keywords: ['mpv', 'gpu', 'rendering', 'compatibility'],
|
||||
requires: ['desktop'],
|
||||
fallbackId: 'video-player',
|
||||
requires: ['embedded-mpv-frame-copy'],
|
||||
},
|
||||
{
|
||||
id: 'embedded-mpv-extra-options',
|
||||
@@ -102,7 +101,7 @@ export const PLAYBACK_SETTINGS_SEARCH_ENTRIES: readonly SettingsSearchEntry[] =
|
||||
labelKey: 'SETTINGS.EMBEDDED_MPV_EXTRA_OPTIONS',
|
||||
descriptionKey: 'SETTINGS.EMBEDDED_MPV_EXTRA_OPTIONS_DESCRIPTION',
|
||||
keywords: ['mpv', 'options', 'config', 'hwdec', 'arguments'],
|
||||
requires: ['desktop'],
|
||||
requires: ['embedded-mpv'],
|
||||
fallbackId: 'video-player',
|
||||
},
|
||||
{
|
||||
@@ -111,7 +110,7 @@ export const PLAYBACK_SETTINGS_SEARCH_ENTRIES: readonly SettingsSearchEntry[] =
|
||||
labelKey: 'SETTINGS.EMBEDDED_MPV_AUTO_RECONNECT',
|
||||
descriptionKey: 'SETTINGS.EMBEDDED_MPV_AUTO_RECONNECT_DESCRIPTION',
|
||||
keywords: ['mpv', 'reconnect', 'retry', 'buffering'],
|
||||
requires: ['desktop'],
|
||||
requires: ['embedded-mpv'],
|
||||
fallbackId: 'video-player',
|
||||
},
|
||||
{
|
||||
|
||||
@@ -27,6 +27,7 @@ interface RuntimeStub {
|
||||
supportsPortalConnectivityGuard: boolean;
|
||||
supportsManagedExternalPlayers: boolean;
|
||||
supportsExternalPlayerPathSettings: boolean;
|
||||
supportsEmbeddedMpv: boolean;
|
||||
}
|
||||
|
||||
function setup(runtime: Partial<RuntimeStub> = {}) {
|
||||
@@ -44,6 +45,7 @@ function setup(runtime: Partial<RuntimeStub> = {}) {
|
||||
supportsPortalConnectivityGuard: false,
|
||||
supportsManagedExternalPlayers: false,
|
||||
supportsExternalPlayerPathSettings: false,
|
||||
supportsEmbeddedMpv: false,
|
||||
...runtime,
|
||||
},
|
||||
},
|
||||
@@ -116,7 +118,7 @@ describe('SettingsSearchService', () => {
|
||||
expect(service.search('guide')).toEqual([]);
|
||||
});
|
||||
|
||||
it('offers every row on a fully capable desktop runtime except gated VOD failover', () => {
|
||||
it('offers the desktop rows that need no probe on a capable runtime', () => {
|
||||
const { service } = setup(DESKTOP);
|
||||
|
||||
const entries = service.visibleEntries().map(({ id }) => id);
|
||||
@@ -125,7 +127,72 @@ describe('SettingsSearchService', () => {
|
||||
expect(entries).toContain('remote-control-port');
|
||||
expect(entries).toContain('mpv-player-path');
|
||||
expect(entries).not.toContain('vod-auto-failover');
|
||||
expect(entries.length).toBe(SETTINGS_SEARCH_ENTRIES.length - 1);
|
||||
// Embedded MPV rows wait for the support probe.
|
||||
expect(entries).not.toContain('embedded-mpv-extra-options');
|
||||
expect(entries).not.toContain('embedded-mpv-frame-copy');
|
||||
expect(entries.length).toBe(SETTINGS_SEARCH_ENTRIES.length - 4);
|
||||
});
|
||||
|
||||
describe('embedded MPV probe', () => {
|
||||
const originalElectron = window.electron;
|
||||
|
||||
afterEach(() => {
|
||||
window.electron = originalElectron;
|
||||
});
|
||||
|
||||
function probeWith(support: {
|
||||
supported: boolean;
|
||||
frameCopyAvailable?: boolean;
|
||||
}) {
|
||||
const getEmbeddedMpvSupport = jest
|
||||
.fn()
|
||||
.mockResolvedValue({ platform: 'darwin', ...support });
|
||||
window.electron = {
|
||||
getEmbeddedMpvSupport,
|
||||
} as unknown as typeof window.electron;
|
||||
return getEmbeddedMpvSupport;
|
||||
}
|
||||
|
||||
it('reveals embedded MPV rows once support resolves, frame copy only when available', async () => {
|
||||
const getEmbeddedMpvSupport = probeWith({ supported: true });
|
||||
const { service } = setup({
|
||||
...DESKTOP,
|
||||
supportsEmbeddedMpv: true,
|
||||
});
|
||||
|
||||
await service.ensureEmbeddedMpvSupportLoaded();
|
||||
const entries = service.visibleEntries().map(({ id }) => id);
|
||||
|
||||
expect(entries).toContain('embedded-mpv-extra-options');
|
||||
expect(entries).toContain('embedded-mpv-auto-reconnect');
|
||||
// Frame copy is not available on this machine, so the row the
|
||||
// settings page never renders must not be offered either.
|
||||
expect(entries).not.toContain('embedded-mpv-frame-copy');
|
||||
expect(service.ensureEmbeddedMpvSupportLoaded()).toBeUndefined();
|
||||
expect(getEmbeddedMpvSupport).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('offers frame copy where the machine can run it', async () => {
|
||||
probeWith({ supported: true, frameCopyAvailable: true });
|
||||
const { service } = setup({
|
||||
...DESKTOP,
|
||||
supportsEmbeddedMpv: true,
|
||||
});
|
||||
|
||||
await service.ensureEmbeddedMpvSupportLoaded();
|
||||
|
||||
expect(service.visibleEntries().map(({ id }) => id)).toContain(
|
||||
'embedded-mpv-frame-copy'
|
||||
);
|
||||
});
|
||||
|
||||
it('does not probe where the runtime has no embedded MPV bridge', () => {
|
||||
const getEmbeddedMpvSupport = probeWith({ supported: true });
|
||||
const { service } = setup(DESKTOP);
|
||||
|
||||
expect(service.ensureEmbeddedMpvSupportLoaded()).toBeUndefined();
|
||||
expect(getEmbeddedMpvSupport).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it('returns translated, ranked results with their section', () => {
|
||||
@@ -187,6 +254,22 @@ describe('SettingsSearchService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('notifies reveal listeners before navigating, until unsubscribed', () => {
|
||||
const { service, router } = setup();
|
||||
const calls: string[] = [];
|
||||
router.navigate.mockImplementation(() => {
|
||||
calls.push('navigate');
|
||||
return Promise.resolve(true);
|
||||
});
|
||||
const stop = service.onReveal(() => calls.push('listener'));
|
||||
|
||||
service.reveal(SETTINGS_SEARCH_ENTRIES[0]);
|
||||
stop();
|
||||
service.reveal(SETTINGS_SEARCH_ENTRIES[1]);
|
||||
|
||||
expect(calls).toEqual(['listener', 'navigate', 'navigate']);
|
||||
});
|
||||
|
||||
it('completes only the reveal request that is still pending', () => {
|
||||
const { service } = setup();
|
||||
const [first, second] = SETTINGS_SEARCH_ENTRIES;
|
||||
|
||||
@@ -3,6 +3,7 @@ import { Router } from '@angular/router';
|
||||
import { TranslateService } from '@ngx-translate/core';
|
||||
import { VodSourceDiscoveryService } from '@iptvnator/portal/shared/data-access';
|
||||
import { RuntimeCapabilitiesService } from '@iptvnator/services';
|
||||
import { EmbeddedMpvSupport } from '@iptvnator/shared/interfaces';
|
||||
import { SETTINGS_SEARCH_ENTRIES } from './settings-search-entries';
|
||||
import { rankSearchMatch, tokenizeSearchQuery } from './settings-search-rank';
|
||||
import {
|
||||
@@ -32,6 +33,12 @@ export class SettingsSearchService {
|
||||
|
||||
private readonly revealRequest = signal<SettingsRevealRequest | null>(null);
|
||||
private revealNonce = 0;
|
||||
private readonly revealListeners = new Set<() => void>();
|
||||
private readonly embeddedMpvSupport = signal<EmbeddedMpvSupport | null>(
|
||||
null
|
||||
);
|
||||
private embeddedMpvSupportChecked = false;
|
||||
private embeddedMpvSupportLoad: Promise<void> | undefined;
|
||||
|
||||
/** Row the settings page should scroll to and highlight next. */
|
||||
readonly pendingReveal = this.revealRequest.asReadonly();
|
||||
@@ -48,9 +55,54 @@ export class SettingsSearchService {
|
||||
'managed-external-players': runtime.supportsManagedExternalPlayers,
|
||||
'external-player-paths': runtime.supportsExternalPlayerPathSettings,
|
||||
'vod-multi-source': this.vodSourceDiscovery.isAvailable,
|
||||
'embedded-mpv': !!this.embeddedMpvSupport()?.supported,
|
||||
'embedded-mpv-frame-copy':
|
||||
!!this.embeddedMpvSupport()?.frameCopyAvailable,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Probes embedded MPV support once so rows that need it become
|
||||
* searchable. Returns the pending probe, or `undefined` when there is
|
||||
* nothing to wait for. Call it lazily (palette open, settings page),
|
||||
* never from shell bootstrap: supported desktop builds may load the
|
||||
* native addon while answering.
|
||||
*/
|
||||
ensureEmbeddedMpvSupportLoaded(): Promise<void> | undefined {
|
||||
if (this.embeddedMpvSupportChecked) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const electron =
|
||||
typeof window === 'undefined' ? undefined : window.electron;
|
||||
if (
|
||||
!this.runtime.supportsEmbeddedMpv ||
|
||||
typeof electron?.getEmbeddedMpvSupport !== 'function'
|
||||
) {
|
||||
this.embeddedMpvSupportChecked = true;
|
||||
return undefined;
|
||||
}
|
||||
|
||||
this.embeddedMpvSupportLoad ??= electron
|
||||
.getEmbeddedMpvSupport()
|
||||
.then((support) => this.embeddedMpvSupport.set(support))
|
||||
.catch(() => this.embeddedMpvSupport.set(null))
|
||||
.finally(() => {
|
||||
this.embeddedMpvSupportChecked = true;
|
||||
});
|
||||
return this.embeddedMpvSupportLoad;
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs `listener` synchronously before every reveal navigation, so the
|
||||
* shell can drop a search keystroke still waiting for its debounce: if
|
||||
* it applied later, its `q` navigation would supersede the reveal.
|
||||
*/
|
||||
onReveal(listener: () => void): () => void {
|
||||
this.revealListeners.add(listener);
|
||||
return () => this.revealListeners.delete(listener);
|
||||
}
|
||||
|
||||
visibleSections(): SettingsSectionDefinition[] {
|
||||
const capabilities = this.capabilities();
|
||||
return SETTINGS_SECTION_DEFINITIONS.filter((section) =>
|
||||
@@ -121,6 +173,7 @@ export class SettingsSearchService {
|
||||
|
||||
/** Opens the row's section page and asks it to highlight the row. */
|
||||
reveal(entry: SettingsSearchEntry): void {
|
||||
this.revealListeners.forEach((listener) => listener());
|
||||
this.revealNonce += 1;
|
||||
this.revealRequest.set({
|
||||
id: entry.id,
|
||||
|
||||
@@ -11,7 +11,10 @@ export type SettingsSearchRequirement =
|
||||
| 'portal-connectivity-guard'
|
||||
| 'managed-external-players'
|
||||
| 'external-player-paths'
|
||||
| 'vod-multi-source';
|
||||
| 'vod-multi-source'
|
||||
/** Probed lazily; false until `ensureEmbeddedMpvSupportLoaded` resolves. */
|
||||
| 'embedded-mpv'
|
||||
| 'embedded-mpv-frame-copy';
|
||||
|
||||
export type SettingsSearchCapabilities = Readonly<
|
||||
Record<SettingsSearchRequirement, boolean>
|
||||
|
||||
Reference in new issue
Block a user