feat(frontend): in-sheet search + focus-steal fix in detail navigation

`useDetailNavigation` now owns a controllable local search (debounced,
ANDed with the URL `?q=` filter) and `DetailNavigationSheet` surfaces it
as a header input by default (`hasSearch={true}`). Sheet auto-focus no
longer steals focus from the search input when `filteredItems` shrinks
on a debounce flush — gate the option-focus effect on `document.activeElement`
so typing past the boundary keeps the caret in place. ArrowDown from the
input synchronously promotes focus into the listbox.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Sergey Kozyrenko
2026-05-20 18:38:49 +07:00
co-authored by Claude Opus 4.7
parent 4888568a7e
commit 5a26d800ee
8 changed files with 748 additions and 15 deletions
@@ -47,6 +47,7 @@ export function DetailNavigationButtons<T extends { id: string }>({
disabled={!controller.prevId}
onClick={controller.goToPrev}
size="icon"
type="button"
variant="outline"
>
<ChevronLeft />
@@ -64,6 +65,7 @@ export function DetailNavigationButtons<T extends { id: string }>({
)}
disabled={!controller.hasEntries}
onClick={controller.openSheet}
type="button"
variant="outline"
>
{controller.positionLabel}
@@ -79,6 +81,7 @@ export function DetailNavigationButtons<T extends { id: string }>({
disabled={!controller.nextId}
onClick={controller.goToNext}
size="icon"
type="button"
variant="outline"
>
<ChevronRight />
@@ -39,8 +39,11 @@ const LocationReadout = () => {
interface HarnessProps {
currentId?: null | string;
defaultSearchQuery?: string;
filter?: string;
hasSearch?: boolean;
items?: readonly Item[];
searchPlaceholder?: string;
}
/**
@@ -48,19 +51,31 @@ interface HarnessProps {
* focus / a11y interactions can be exercised without round-tripping through
* the toolbar's position button. Keeps each test focused on the leaf.
*/
const SheetHarness = ({ currentId = 'c', items = ITEMS }: HarnessProps) => {
const SheetHarness = ({
currentId = 'c',
defaultSearchQuery,
hasSearch,
items = ITEMS,
searchPlaceholder,
}: HarnessProps) => {
const nav = useDetailNavigation<Item>({
currentId,
defaultOpen: true,
defaultSearchQuery,
getHref,
getLabel,
getSearchableText,
items,
// Skip the debounce so each test sees the post-typing subset on the
// next paint instead of having to wait `>150ms` per keystroke.
searchDebounceMs: 0,
});
return (
<DetailNavigationSheet<Item>
controller={nav}
hasSearch={hasSearch}
searchPlaceholder={searchPlaceholder}
sheetTitle="Items"
/>
);
@@ -239,3 +254,174 @@ describe('DetailNavigationSheet — selection', () => {
});
});
});
describe('DetailNavigationSheet — search input', () => {
it('renders the search input by default (hasSearch=true)', async () => {
renderSheet({ currentId: 'c' });
const input = await screen.findByRole('textbox');
expect(input).toBeInTheDocument();
expect(input).toHaveValue('');
});
it('hides the search input when hasSearch is false (opt-out)', async () => {
renderSheet({ currentId: 'c', hasSearch: false });
await screen.findByRole('listbox');
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
});
it('uses the supplied placeholder', async () => {
renderSheet({ currentId: 'c', searchPlaceholder: 'Find item' });
const input = await screen.findByPlaceholderText('Find item');
expect(input).toBeInTheDocument();
});
it('typing narrows the listbox immediately when searchDebounceMs=0', async () => {
const user = userEvent.setup();
renderSheet({ currentId: 'a' });
const input = await screen.findByRole('textbox');
await user.type(input, 'cha');
const listbox = await screen.findByRole('listbox');
await waitFor(() => {
const labels = within(listbox)
.getAllByRole('option')
.map((option) => option.textContent ?? '');
expect(labels).toEqual(['Charlie']);
});
});
it('renders a clear button while the query is non-empty; clicking it resets', async () => {
const user = userEvent.setup();
renderSheet({ currentId: 'a', defaultSearchQuery: 'cha' });
const clearButton = await screen.findByRole('button', { name: 'Clear search' });
await user.click(clearButton);
const input = await screen.findByRole('textbox');
expect(input).toHaveValue('');
const listbox = await screen.findByRole('listbox');
await waitFor(() => {
const labels = within(listbox)
.getAllByRole('option')
.map((option) => option.textContent ?? '');
expect(labels).toEqual(['Alpha', 'Bravo', 'Charlie', 'Delta']);
});
});
it('Escape clears a non-empty query without closing the sheet', async () => {
const user = userEvent.setup();
renderSheet({ currentId: 'a', defaultSearchQuery: 'cha' });
const input = await screen.findByRole('textbox');
input.focus();
await user.keyboard('{Escape}');
expect(input).toHaveValue('');
// Sheet stayed mounted because the keydown was prevented + stopped.
expect(screen.queryByRole('listbox', { name: 'Items' })).toBeInTheDocument();
});
it('shows a query-specific empty state when nothing matches', async () => {
renderSheet({ currentId: 'a', defaultSearchQuery: 'zzzzz' });
await waitFor(() => {
expect(screen.getByText(/No items match "zzzzz"\./)).toBeInTheDocument();
});
expect(screen.queryByRole('listbox', { name: 'Items' })).not.toBeInTheDocument();
});
it('ANDs the in-sheet search with the URL ?q= filter', async () => {
// URL filter narrows to Alpha + Bravo + Charlie (substring "a"); the
// local "bra" then narrows further to just Bravo. Use a single
// `fireEvent.change` instead of `user.type` so the assertion does
// not race with React's per-keystroke re-renders.
renderSheet({ currentId: 'b', filter: 'a' });
const input = await screen.findByRole('textbox');
fireEvent.change(input, { target: { value: 'bra' } });
const listbox = await screen.findByRole('listbox', { name: 'Items' });
await waitFor(() => {
const labels = within(listbox)
.getAllByRole('option')
.map((option) => option.textContent ?? '');
expect(labels).toEqual(['Bravo']);
});
});
it('ArrowDown from the input moves roving focus into the listbox', async () => {
// Start at currentId 'b' so the initial roving focus is 'b'; that way
// we can prove ArrowDown from the *input* targeted the first item ('a')
// rather than just no-op'ing on already-focused 'a'.
renderSheet({ currentId: 'b' });
const input = await screen.findByRole('textbox');
fireEvent.keyDown(input, { key: 'ArrowDown' });
const listbox = await screen.findByRole('listbox');
await waitFor(() => {
const focused = within(listbox)
.getAllByRole('option')
.find((option) => option.getAttribute('tabindex') === '0');
expect(focused).toHaveAttribute('data-item-id', 'a');
});
// ArrowDown jumps DOM focus to the new option synchronously — the
// auto-focus effect skips when focus is on the search input, so this
// handler has to move focus itself. Without it, the user would have
// to click the option to interact with it after typing.
expect(document.activeElement).toHaveAttribute('data-item-id', 'a');
});
it('does not yank focus onto a listbox option when filteredItems shrinks (regression)', async () => {
// Reproduces the bug from the original implementation: typing a
// query that drops the current item from the subset triggered the
// auto-focus effect to grab focus onto the new `focusedId` (the
// first survivor) at the debounce boundary, interrupting typing.
//
// `currentId='d'` + query `'rav'` → Delta is gone, subset becomes
// [Bravo], render-phase reconciliation sets `focusedId='b'`. With
// the fix the effect observes the search input is focused and
// refuses to steal focus onto the option.
//
// (The exact post-shrink `activeElement` is sensitive to Radix's
// JSDOM-only focus-trap fallback behaviour — in a real browser
// focus stays on the input, but JSDOM may bounce it onto the
// dialog container if the previously-focused option unmounts. The
// bug is specifically about the listbox option stealing focus, so
// we assert against *that*, not against equality with the input.)
renderSheet({ currentId: 'd' });
const input = await screen.findByRole('textbox');
// Radix Dialog runs its own open-time focus trap. Wait for it to
// settle, then put focus on the input as the user would after
// clicking it.
await waitFor(() => {
input.focus();
expect(document.activeElement).toBe(input);
});
fireEvent.change(input, { target: { value: 'rav' } });
await waitFor(() => {
const labels = within(screen.getByRole('listbox'))
.getAllByRole('option')
.map((option) => option.textContent ?? '');
expect(labels).toEqual(['Bravo']);
});
// The exact failure mode of the original bug: focus jumps onto a
// `role="option"` button when the list shrinks. With the fix it
// never lands there until the user explicitly arrows into the list.
expect(document.activeElement?.getAttribute('role')).not.toBe('option');
});
});
@@ -1,6 +1,8 @@
import { Search, X } from 'lucide-react';
import { type KeyboardEvent, type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { Badge } from '@/components/ui/badge';
import { InputGroup, InputGroupAddon, InputGroupButton, InputGroupInput } from '@/components/ui/input-group';
import { ScrollArea } from '@/components/ui/scroll-area';
import { Sheet, SheetContent, SheetHeader, SheetTitle } from '@/components/ui/sheet';
import { cn } from '@/lib/utils';
@@ -9,7 +11,15 @@ import type { DetailNavigationController } from './use-detail-navigation';
interface DetailNavigationSheetProps<T extends { id: string }> {
controller: DetailNavigationController<T>;
/**
* Render a free-text search input in the sheet header that drives
* `controller.setSearchQuery`. Defaults to `true` — pass `false` to opt
* out for the rare consumer that wants the sheet to stay URL-filter-only.
*/
hasSearch?: boolean;
renderItem?: (item: T, isCurrent: boolean) => ReactNode;
/** Placeholder for the in-sheet search input. Defaults to "Search…". */
searchPlaceholder?: string;
sheetIcon?: ReactNode;
sheetTitle: string;
}
@@ -27,7 +37,9 @@ interface DetailNavigationSheetProps<T extends { id: string }> {
*/
export function DetailNavigationSheet<T extends { id: string }>({
controller,
hasSearch = true,
renderItem,
searchPlaceholder = 'Search…',
sheetIcon,
sheetTitle,
}: DetailNavigationSheetProps<T>) {
@@ -35,6 +47,7 @@ export function DetailNavigationSheet<T extends { id: string }>({
// read individual fields rather than the controller object — keeps the
// identity story the same as before the refactor.
const {
clearSearchQuery,
currentId,
currentIndex,
filteredItems: items,
@@ -42,15 +55,20 @@ export function DetailNavigationSheet<T extends { id: string }>({
getLabel,
handleItemSelect: onItemSelect,
isSheetOpen: open,
searchQuery,
setSearchQuery,
setSheetOpen: onOpenChange,
total,
} = controller;
const listRef = useRef<HTMLUListElement>(null);
const searchInputRef = useRef<HTMLInputElement>(null);
const buttonRefs = useRef(new Map<string, HTMLButtonElement>());
const [focusedId, setFocusedId] = useState<null | string>(null);
const hasEntries = items.length > 0;
const trimmedQuery = searchQuery.trim();
const hasClearButton = hasSearch && trimmedQuery.length > 0;
// Build an `id → index` map once per `items`/`getId` change so both the
// per-render membership check below and the keyboard handler's lookup
@@ -126,11 +144,35 @@ export function DetailNavigationSheet<T extends { id: string }>({
// After roving focus moves, push the focus into the DOM. `rAF` defers past
// Radix's own focus management so we don't fight its open-time focus trap.
//
// Gate: only auto-focus an option when the user's focus is already
// somewhere that *expects* roving (a list option) or has not yet landed
// (`document.activeElement === document.body` right after open, before
// Radix moves focus into the dialog). Crucially, we must NOT steal focus
// when the user is typing in the search input: every keystroke that
// flushes the debounce can re-target `focusedId` (when `currentId` falls
// out of the filtered subset, render-phase reconciliation snaps focus to
// the first survivor). Without this gate, focus jumps out of the input
// mid-type and the user can only enter a few characters before losing
// their place — exact repro: type "aes" with current flow 837 visible,
// focus moves to button 836 at the 150 ms debounce boundary.
//
// The fix is at the cause, not the symptom: a local input mirror (the
// `InputSearch` / `DataTableFilter` pattern) would keep keystrokes from
// being dropped during a state round-trip, but it would not stop this
// effect from yanking focus away. The two patterns solve different bugs.
useEffect(() => {
if (!open || focusedId === null) {
return;
}
const activeEl = document.activeElement;
const focusIsOnSearchInput = activeEl !== null && activeEl === searchInputRef.current;
if (focusIsOnSearchInput) {
return;
}
const id = requestAnimationFrame(() => {
const node = buttonRefs.current.get(focusedId);
@@ -206,6 +248,61 @@ export function DetailNavigationSheet<T extends { id: string }>({
[onItemSelect],
);
// ArrowDown from the input jumps focus into the listbox so keyboard
// users can flow `type → arrow down → enter` without hunting for the
// list. We move focus *here* synchronously (not in the auto-focus
// effect) because that effect now skips when focus is on the search
// input — otherwise it would yank focus mid-type. Escape is *not*
// handled here — Radix listens for it on a native document handler
// that React-level `stopPropagation` cannot reach. See
// `handleEscapeKeyDown` below, wired through `onEscapeKeyDown`.
const handleSearchKeyDown = useCallback(
(event: React.KeyboardEvent<HTMLInputElement>) => {
if (event.key === 'ArrowDown' && hasEntries) {
event.preventDefault();
const first = items[0];
if (!first) {
return;
}
const firstId = String(getId(first));
setFocusedId(firstId);
// Button refs for the current items are already mounted —
// this handler fires during a real user keystroke, so the
// listbox commit that wired them up has already happened.
buttonRefs.current.get(firstId)?.focus();
}
},
[getId, hasEntries, items],
);
// Intercept Esc at the Radix-Content level so we can clear a non-empty
// search before the dialog's built-in "close on Esc" fires. Once the
// query is empty, we let Radix close as usual — matches the two-step
// Esc affordance of `InputSearch` (clear, then dismiss).
const handleEscapeKeyDown = useCallback(
(event: KeyboardEvent) => {
if (hasSearch && trimmedQuery.length > 0) {
event.preventDefault();
clearSearchQuery();
}
},
[clearSearchQuery, hasSearch, trimmedQuery.length],
);
const handleSearchChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
setSearchQuery(event.target.value);
},
[setSearchQuery],
);
const handleSearchClear = useCallback(() => {
clearSearchQuery();
searchInputRef.current?.focus();
}, [clearSearchQuery]);
// One stable callback ref reused for every button. The previous shape —
// `setButtonRef(id) => (node) => …` — manufactured a new closure per id
// on every render, which made React re-attach refs (a `delete` + `set`
@@ -241,9 +338,10 @@ export function DetailNavigationSheet<T extends { id: string }>({
// listbox of items, the `SheetTitle` already describes it.
aria-describedby={undefined}
className="flex w-full max-w-sm flex-col gap-0 p-0 sm:max-w-sm"
onEscapeKeyDown={handleEscapeKeyDown}
side="right"
>
<SheetHeader className="border-b p-4">
<SheetHeader className="gap-3 border-b p-4">
<SheetTitle className="flex items-center gap-2 pr-8 text-base">
{sheetIcon}
<span>{sheetTitle}</span>
@@ -254,6 +352,39 @@ export function DetailNavigationSheet<T extends { id: string }>({
{total}
</Badge>
</SheetTitle>
{hasSearch ? (
<InputGroup className="h-9">
<InputGroupAddon align="inline-start">
<Search
aria-hidden="true"
className="text-muted-foreground"
/>
</InputGroupAddon>
<InputGroupInput
aria-label={searchPlaceholder}
className="h-9 py-0"
onChange={handleSearchChange}
onKeyDown={handleSearchKeyDown}
placeholder={searchPlaceholder}
ref={searchInputRef}
type="text"
value={searchQuery}
/>
{hasClearButton ? (
<InputGroupAddon align="inline-end">
<InputGroupButton
aria-label="Clear search"
onClick={handleSearchClear}
size="icon-sm"
type="button"
variant="ghost"
>
<X aria-hidden="true" />
</InputGroupButton>
</InputGroupAddon>
) : null}
</InputGroup>
) : null}
</SheetHeader>
{hasEntries ? (
<ScrollArea className="flex-1">
@@ -301,7 +432,9 @@ export function DetailNavigationSheet<T extends { id: string }>({
</ScrollArea>
) : (
<div className="text-muted-foreground flex flex-1 items-center justify-center px-4 text-center text-sm">
No items match the current filter.
{trimmedQuery.length > 0
? `No items match "${trimmedQuery}".`
: 'No items match the current filter.'}
</div>
)}
</SheetContent>
@@ -7,7 +7,14 @@ import { DetailNavigationSheet } from './detail-navigation-sheet';
export interface DetailNavigationToolbarProps<T extends { id: string }> {
controller: DetailNavigationController<T>;
/**
* Forwarded to `<DetailNavigationSheet>` controls the in-sheet search
* input. Defaults to `true`; pass `false` to opt out.
*/
hasSearch?: boolean;
renderItem?: (item: T, isCurrent: boolean) => ReactNode;
/** Forwarded placeholder for the in-sheet search input. */
searchPlaceholder?: string;
sheetIcon?: ReactNode;
sheetTitle: string;
}
@@ -24,7 +31,9 @@ export interface DetailNavigationToolbarProps<T extends { id: string }> {
*/
export function DetailNavigationToolbar<T extends { id: string }>({
controller,
hasSearch,
renderItem,
searchPlaceholder,
sheetIcon,
sheetTitle,
}: DetailNavigationToolbarProps<T>) {
@@ -40,7 +49,9 @@ export function DetailNavigationToolbar<T extends { id: string }>({
/>
<DetailNavigationSheet
controller={controller}
hasSearch={hasSearch}
renderItem={renderItem}
searchPlaceholder={searchPlaceholder}
sheetIcon={sheetIcon}
sheetTitle={sheetTitle}
/>
@@ -443,6 +443,227 @@ describe('useDetailNavigation — current item bookkeeping', () => {
});
});
describe('useDetailNavigation — local search (uncontrolled)', () => {
it('starts with an empty searchQuery and `isSearchActive=false` by default', () => {
const { result } = renderHook(
() =>
useDetailNavigation<Item>({
currentId: 'b',
getHref,
getLabel,
getSearchableText,
items: ITEMS,
}),
{ wrapper: renderInRoute(['/items/b']) },
);
expect(result.current.searchQuery).toBe('');
expect(result.current.debouncedSearchQuery).toBe('');
expect(result.current.isSearchActive).toBe(false);
// Without a query, the full subset is visible.
expect(result.current.filteredItems.map((item) => item.id)).toEqual(['a', 'b', 'c']);
});
it('honours defaultSearchQuery on first render and surfaces it as active', async () => {
const { result } = renderHook(
() =>
useDetailNavigation<Item>({
currentId: 'b',
defaultSearchQuery: 'bravo',
getHref,
getLabel,
getSearchableText,
items: ITEMS,
searchDebounceMs: 0,
}),
{ wrapper: renderInRoute(['/items/b']) },
);
expect(result.current.searchQuery).toBe('bravo');
await waitFor(() => {
expect(result.current.debouncedSearchQuery).toBe('bravo');
});
expect(result.current.isSearchActive).toBe(true);
expect(result.current.filteredItems.map((item) => item.id)).toEqual(['b']);
});
it('setSearchQuery + debounce narrows filteredItems', async () => {
const { result } = renderHook(
() =>
useDetailNavigation<Item>({
currentId: 'a',
getHref,
getLabel,
getSearchableText,
items: ITEMS,
searchDebounceMs: 0,
}),
{ wrapper: renderInRoute(['/items/a']) },
);
act(() => {
result.current.setSearchQuery('cha');
});
// searchQuery (raw) flips instantly for the input value.
expect(result.current.searchQuery).toBe('cha');
await waitFor(() => {
expect(result.current.filteredItems.map((item) => item.id)).toEqual(['c']);
});
// Current id 'a' is no longer in the subset → -1 / null neighbours.
expect(result.current.currentIndex).toBe(-1);
expect(result.current.prevId).toBeNull();
expect(result.current.nextId).toBeNull();
});
it('clearSearchQuery resets the value and restores the full subset', async () => {
const { result } = renderHook(
() =>
useDetailNavigation<Item>({
currentId: 'a',
defaultSearchQuery: 'pha',
getHref,
getLabel,
getSearchableText,
items: ITEMS,
searchDebounceMs: 0,
}),
{ wrapper: renderInRoute(['/items/a']) },
);
await waitFor(() => {
expect(result.current.filteredItems.map((item) => item.id)).toEqual(['a']);
});
act(() => {
result.current.clearSearchQuery();
});
expect(result.current.searchQuery).toBe('');
await waitFor(() => {
expect(result.current.filteredItems.map((item) => item.id)).toEqual(['a', 'b', 'c']);
});
});
it('combines URL filter and local search with AND semantics', async () => {
// URL ?q=a narrows to Alpha + Bravo + Charlie. Local "bra" then keeps
// only Bravo. Without AND we'd see Alpha (matches `a`) or Bravo+Charlie
// (matches local in isolation).
const { result } = renderHook(
() =>
useDetailNavigation<Item>({
currentId: 'b',
getHref,
getLabel,
getSearchableText,
items: ITEMS,
searchDebounceMs: 0,
}),
{ wrapper: renderInRoute(['/items/b?q=a']) },
);
await waitFor(() => {
expect(result.current.debouncedFilter).toBe('a');
});
act(() => {
result.current.setSearchQuery('bra');
});
await waitFor(() => {
expect(result.current.filteredItems.map((item) => item.id)).toEqual(['b']);
});
});
});
describe('useDetailNavigation — local search (controlled)', () => {
it('respects the controlled searchQuery and fires onSearchQueryChange', () => {
const onSearchQueryChange = vi.fn();
const { rerender, result } = renderHook(
({ searchQuery }: { searchQuery: string }) =>
useDetailNavigation<Item>({
currentId: 'b',
getHref,
getLabel,
getSearchableText,
items: ITEMS,
onSearchQueryChange,
searchDebounceMs: 0,
searchQuery,
}),
{
initialProps: { searchQuery: 'bravo' },
wrapper: renderInRoute(['/items/b']),
},
);
expect(result.current.searchQuery).toBe('bravo');
act(() => {
result.current.setSearchQuery('charlie');
});
// Parent owns the value — controller does not flip without a prop change.
expect(result.current.searchQuery).toBe('bravo');
// But the consumer is told to update.
expect(onSearchQueryChange).toHaveBeenLastCalledWith('charlie');
rerender({ searchQuery: 'charlie' });
expect(result.current.searchQuery).toBe('charlie');
});
it('fires onSearchQueryChange in uncontrolled mode too', () => {
const onSearchQueryChange = vi.fn();
const { result } = renderHook(
() =>
useDetailNavigation<Item>({
currentId: 'b',
getHref,
getLabel,
getSearchableText,
items: ITEMS,
onSearchQueryChange,
}),
{ wrapper: renderInRoute(['/items/b']) },
);
act(() => {
result.current.setSearchQuery('alpha');
});
expect(onSearchQueryChange).toHaveBeenLastCalledWith('alpha');
expect(result.current.searchQuery).toBe('alpha');
});
});
describe('useDetailNavigation — local search identity stability', () => {
it('keeps setSearchQuery / clearSearchQuery identity-stable across re-renders', () => {
const { rerender, result } = renderHook(
() =>
useDetailNavigation<Item>({
currentId: 'b',
getHref,
getLabel,
getSearchableText,
items: ITEMS,
}),
{ wrapper: renderInRoute(['/items/b']) },
);
const firstSet = result.current.setSearchQuery;
const firstClear = result.current.clearSearchQuery;
rerender();
expect(result.current.setSearchQuery).toBe(firstSet);
expect(result.current.clearSearchQuery).toBe(firstClear);
});
});
describe('useDetailNavigation — navigation back to list does not leak the filter', () => {
it('keeps the URL `?q=` intact after a navigate inside the same router', async () => {
const Harness = () => {
@@ -1,20 +1,32 @@
import { useCallback, useMemo, useState } from 'react';
import { useNavigate, useSearchParams } from 'react-router-dom';
import { useDebouncedValue } from '@/hooks/use-debounced-value';
import { useLatestRef } from '@/hooks/use-latest-ref';
import { useTableQueryFilterReader } from '@/hooks/use-table-query-filter';
import { mergeHrefWithSearchParams } from '@/lib/url-params';
import { useNavigation } from './use-navigation';
/**
* Default debounce for the in-sheet local search. Shorter than the URL filter's
* 200 ms (`useTableQueryFilterReader`) local search doesn't round-trip
* through `history` / re-render the route subtree, so we can afford to react
* faster while still coalescing burst typing.
*/
const DEFAULT_SEARCH_DEBOUNCE_MS = 150;
/**
* Headless controller for a detail page that walks a filtered list.
*
* Owns:
* - the filtered/sorted subset (pure `computeNavigation`),
* - the filtered/sorted subset (pure `computeNavigation`), narrowed by
* the URL filter AND an optional in-controller local search,
* - the resolved Prev / Next sibling ids and the pre-formatted position
* label (so leaf components don't recompute),
* - the sheet open state (controllable via `open` / `onOpenChange`),
* - the local search query state (controllable via `searchQuery` /
* `onSearchQueryChange`), with its own debounce independent of the URL,
* - navigation actions that thread the current `?<filter>=` into every
* prev / next / item-select destination.
*
@@ -24,6 +36,8 @@ import { useNavigation } from './use-navigation';
* controller can rely on referential equality for downstream memos.
*/
export interface DetailNavigationController<T extends { id: string }> {
/** Reset the local search to an empty string. Convenience for "X" buttons. */
clearSearchQuery: () => void;
closeSheet: () => void;
/** Active `currentId` coerced to a string, or `null` when absent. */
currentId: null | string;
@@ -33,6 +47,11 @@ export interface DetailNavigationController<T extends { id: string }> {
currentItem: null | T;
/** Debounced URL filter the controller is filtering against. */
debouncedFilter: string;
/**
* Debounced version of {@link searchQuery} this is what actually narrows
* `filteredItems`. Combined with `debouncedFilter` via AND.
*/
debouncedSearchQuery: string;
/** Sorted+filtered subset that drives prev / next / sheet listing. */
filteredItems: readonly T[];
/** Stable id accessor (defaults to `item.id` when not supplied). */
@@ -53,6 +72,8 @@ export interface DetailNavigationController<T extends { id: string }> {
handleItemSelect: (item: T) => void;
/** `true` iff `filteredItems.length > 0`. */
hasEntries: boolean;
/** `true` iff `debouncedSearchQuery` is non-empty (UI hints / clear button). */
isSearchActive: boolean;
isSheetOpen: boolean;
/**
@@ -69,6 +90,13 @@ export interface DetailNavigationController<T extends { id: string }> {
positionLabel: string;
/** ID of the previous filtered sibling, or `null` at the start / off-subset. */
prevId: null | string;
/**
* Current local search value (raw, **not** debounced) bind directly to
* an `<input value={...}>` for instant caret feedback. The debounced
* mirror is what filters the list, see {@link debouncedSearchQuery}.
*/
searchQuery: string;
setSearchQuery: (value: string) => void;
setSheetOpen: (open: boolean) => void;
/** Same as `filteredItems.length`, named explicitly for clarity. */
@@ -79,6 +107,8 @@ interface UseDetailNavigationOptions<T extends { id: string }> {
currentId: null | string | undefined;
/** Initial value for the uncontrolled case. Defaults to `false`. */
defaultOpen?: boolean;
/** Initial local search value for the uncontrolled case. Defaults to `''`. */
defaultSearchQuery?: string;
getHref: (item: T) => string;
getId?: (item: T) => string;
getLabel: (item: T) => string;
@@ -86,6 +116,13 @@ interface UseDetailNavigationOptions<T extends { id: string }> {
items: readonly T[];
onOpenChange?: (open: boolean) => void;
/**
* Fires on every `setSearchQuery` call useful when the consumer wants
* to mirror the value into URL params, localStorage, or analytics. Like
* `onOpenChange`, it fires in both controlled and uncontrolled mode.
*/
onSearchQueryChange?: (query: string) => void;
/**
* Controlled-mode opt-in for the sheet. When `open` is `undefined` the
* controller owns the state internally; when a value is provided the
@@ -96,6 +133,22 @@ interface UseDetailNavigationOptions<T extends { id: string }> {
* `@/components/ui/autocomplete.tsx`.
*/
open?: boolean;
/**
* Debounce (ms) applied between the raw `searchQuery` typed by the user
* and the `debouncedSearchQuery` that actually filters `filteredItems`.
* Defaults to {@link DEFAULT_SEARCH_DEBOUNCE_MS}. Pass `0` for instant
* filtering on small lists.
*/
searchDebounceMs?: number;
/**
* Controlled-mode opt-in for the local search query same `undefined`
* uncontrolled / value controlled convention as `open`. Use this
* when the consumer wants to surface the search outside the sheet (e.g.
* a page-level search box that also drives sibling navigation).
*/
searchQuery?: string;
sortFn?: (a: T, b: T) => number;
}
@@ -133,13 +186,17 @@ const defaultGetId = (item: { id: string }): string => item.id;
export function useDetailNavigation<T extends { id: string }>({
currentId,
defaultOpen,
defaultSearchQuery,
getHref,
getId,
getLabel,
getSearchableText,
items,
onOpenChange,
onSearchQueryChange,
open,
searchDebounceMs,
searchQuery,
sortFn,
}: UseDetailNavigationOptions<T>): DetailNavigationController<T> {
const navigate = useNavigate();
@@ -156,12 +213,43 @@ export function useDetailNavigation<T extends { id: string }>({
[getSearchableText, getLabel],
);
// Controllable local search (same pattern as the sheet's `open` below).
// Uncontrolled is the common case — most consumers want the controller to
// own the value so the in-sheet input "just works". Controlled callers
// (e.g. a page that mirrors the value to its own URL param) pass
// `searchQuery` and observe via `onSearchQueryChange`.
const onSearchQueryChangeRef = useLatestRef(onSearchQueryChange);
const [internalSearchQuery, setInternalSearchQuery] = useState(defaultSearchQuery ?? '');
const isSearchControlled = searchQuery !== undefined;
const activeSearchQuery = isSearchControlled ? searchQuery : internalSearchQuery;
const debouncedSearchQuery = useDebouncedValue(activeSearchQuery, searchDebounceMs ?? DEFAULT_SEARCH_DEBOUNCE_MS);
const setSearchQuery = useCallback(
(next: string) => {
if (!isSearchControlled) {
setInternalSearchQuery(next);
}
onSearchQueryChangeRef.current?.(next);
},
[isSearchControlled, onSearchQueryChangeRef],
);
const clearSearchQuery = useCallback(() => setSearchQuery(''), [setSearchQuery]);
// Stabilise the query tuple so `useNavigation`'s `useMemo` only refires
// when either source actually moved. Inline `[a, b]` per render would
// defeat the memo even when both strings are unchanged.
const queryTerms = useMemo(
() => [debouncedFilter, debouncedSearchQuery] as const,
[debouncedFilter, debouncedSearchQuery],
);
const { currentIndex, currentItem, filteredItems, nextId, prevId, total } = useNavigation<T>({
currentId,
getId: resolvedGetId,
getSearchableText: resolvedGetSearchableText,
items,
query: debouncedFilter,
query: queryTerms,
sortFn,
});
@@ -243,14 +331,17 @@ export function useDetailNavigation<T extends { id: string }>({
const normalizedCurrentId = currentId != null ? String(currentId) : null;
const hasEntries = filteredItems.length > 0;
const itemsEmpty = items.length === 0;
const isSearchActive = debouncedSearchQuery.length > 0;
return useMemo<DetailNavigationController<T>>(
() => ({
clearSearchQuery,
closeSheet,
currentId: normalizedCurrentId,
currentIndex,
currentItem,
debouncedFilter,
debouncedSearchQuery,
filteredItems,
getId: resolvedGetId,
getLabel,
@@ -259,21 +350,27 @@ export function useDetailNavigation<T extends { id: string }>({
goToPrev,
handleItemSelect,
hasEntries,
isSearchActive,
isSheetOpen,
itemsEmpty,
nextId,
openSheet,
positionLabel,
prevId,
searchQuery: activeSearchQuery,
setSearchQuery,
setSheetOpen,
total,
}),
[
activeSearchQuery,
clearSearchQuery,
closeSheet,
normalizedCurrentId,
currentIndex,
currentItem,
debouncedFilter,
debouncedSearchQuery,
filteredItems,
resolvedGetId,
getLabel,
@@ -282,12 +379,14 @@ export function useDetailNavigation<T extends { id: string }>({
goToPrev,
handleItemSelect,
hasEntries,
isSearchActive,
isSheetOpen,
itemsEmpty,
nextId,
openSheet,
positionLabel,
prevId,
setSearchQuery,
setSheetOpen,
total,
],
@@ -258,6 +258,63 @@ describe('computeNavigation', () => {
expect(result.currentIndex).toBe(0);
});
it('treats an empty array query as "no filter"', () => {
const result = computeNavigation({
currentId: 'c',
getId,
getSearchableText: getTitle,
items: ROWS,
query: [],
});
expect(result.filteredItems.map(getId)).toEqual(['a', 'b', 'c', 'd']);
expect(result.currentIndex).toBe(2);
});
it('ANDs every non-empty term when `query` is an array', () => {
// Both terms must match the haystack — substring "a" matches Alpha,
// Bravo, Charlie; substring "ph" then narrows to just Alpha.
const result = computeNavigation({
currentId: 'a',
getId,
getSearchableText: getTitle,
items: ROWS,
query: ['a', 'ph'],
});
expect(result.filteredItems.map(getId)).toEqual(['a']);
expect(result.currentIndex).toBe(0);
expect(result.prevId).toBeNull();
expect(result.nextId).toBeNull();
});
it('drops empty strings from an array query without throwing', () => {
// Callers can pass `[urlFilter, localSearch]` straight through; either
// one being empty must not turn the whole filter off — the other term
// is still expected to narrow.
const result = computeNavigation({
currentId: 'b',
getId,
getSearchableText: getTitle,
items: ROWS,
query: ['', 'bravo', ''],
});
expect(result.filteredItems.map(getId)).toEqual(['b']);
});
it('falls back to "no filter" when every array term is empty', () => {
const result = computeNavigation({
currentId: 'a',
getId,
getSearchableText: getTitle,
items: ROWS,
query: ['', '', ''],
});
expect(result.filteredItems.map(getId)).toEqual(['a', 'b', 'c', 'd']);
});
it('matches numeric item ids with string currentId', () => {
type NumRow = { id: number; title: string };
const rows: readonly NumRow[] = [
@@ -12,12 +12,17 @@ interface NavigationInput<T> {
getSearchableText?: (item: T) => null | string | undefined;
items: readonly T[];
/**
* Free-text filter to narrow the navigable subset. Empty / `undefined` is
* treated as "no filter" and every item passes. Matching reuses
* `createTextMatcher`, so behaviour matches the list page's column
* filter (substring, case + diacritic insensitive).
* Free-text filter(s) to narrow the navigable subset. Accepts:
* - `undefined` / empty string / empty array no filter, every item passes;
* - a single string substring match (case + diacritic insensitive);
* - a `readonly string[]` AND of every non-empty entry. Empty strings
* inside the array are ignored, so callers can pass
* `[urlFilter, localSearch]` without checking emptiness at the call site.
*
* Matching reuses `createTextMatcher`, so behaviour stays in lockstep with
* the list page's column filter regardless of how many sources combine here.
*/
query?: string;
query?: readonly string[] | string;
sortFn?: (a: T, b: T) => number;
}
@@ -48,10 +53,24 @@ export const computeNavigation = <T>({
query,
sortFn,
}: NavigationInput<T>): NavigationResult<T> => {
const hasQuery = query !== undefined && query.length > 0;
// Normalise `string | string[] | undefined` to a list of non-empty terms.
// Empties are dropped here once so the hot `items.filter` below never
// re-checks them — and callers can splice `[urlFilter, localSearch]`
// straight through without short-circuiting at the call site.
const queryTerms = query === undefined ? [] : Array.isArray(query) ? query : [query as string];
const activeTerms = queryTerms.filter((term): term is string => term.length > 0);
const hasQuery = activeTerms.length > 0;
// Build each matcher once at the top of the filter pass — `createTextMatcher`
// normalises the term, so doing it inside the per-item loop would burn
// O(items × terms) on the same work.
const matchers = hasQuery && getSearchableText ? activeTerms.map(createTextMatcher) : null;
const filtered =
hasQuery && getSearchableText
? items.filter((item) => createTextMatcher(query)(getSearchableText(item)))
matchers && getSearchableText
? items.filter((item) => {
const text = getSearchableText(item);
return matchers.every((match) => match(text));
})
: items;
const ordered = sortFn ? [...filtered].sort(sortFn) : filtered;
@@ -98,8 +117,12 @@ interface UseNavigationOptions<T> {
*/
getSearchableText?: (item: T) => null | string | undefined;
items: readonly T[];
/** Free-text filter. Empty / `undefined` → unfiltered. */
query?: string;
/**
* Free-text filter. Accepts a single string or an array (AND of every
* non-empty entry). Empty / `undefined` / `[]` unfiltered. See
* {@link NavigationInput.query} for details.
*/
query?: readonly string[] | string;
/**
* Optional comparator. When omitted, the input array order is preserved
* the navigation walks `items` exactly as the caller arranged them, which