diff --git a/DESIGN.md b/DESIGN.md index cc423674..d4af2c12 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -97,16 +97,21 @@ as a document, and these are working surfaces. ## Search -There is exactly one search surface in the product: the command-palette -scope. It is reachable two ways that resolve to the same UI — cmd+K -anywhere, or clicking the magnifier in the top nav, which morphs in place -into an inline search bar over about 200ms with the in-place morph easing -(see Motion). Esc collapses it back to the magnifier, with focus returning -to the magnifier itself. There is no page-local -search input that duplicates palette scope; a page that needs scoped -filtering builds it as a filter control, not a second "search." See -`docs/command-palette.md` for the palette's scoring and result-group -contract — this section only fixes how it's invoked from chrome. +Two separate surfaces, never merged, and neither opens the other (a +decision re-litigated more than once — see `docs/DECISIONS.md` → Search): + +- **The magnifier in the stage top bar is a per-page filter.** It scopes to + whatever page it's on — Files filters files, Skills filters skills — and + never leaves that page. Clicking it morphs it in place into an inline + input over about 200ms with the in-place morph easing (see Motion); Esc + collapses it back, with focus returning to the magnifier. Where a page + already has its own filter, the magnifier drives that filter directly + rather than the page adding a second input. A page with nothing to filter + renders no magnifier at all. +- **`Cmd+K` opens the global command palette**, reachable from anywhere + (including a route with no stage top bar of its own) and rendered as its + own surface, never anchored to the magnifier. See `docs/command-palette.md` + for the palette's scoring and result-group contract. ## Color, Type & Icons diff --git a/apps/web/src/app.css b/apps/web/src/app.css index 2e00c37c..36878933 100644 --- a/apps/web/src/app.css +++ b/apps/web/src/app.css @@ -719,15 +719,18 @@ select:disabled, margin-left: auto; } -/* The one search entry point: a magnifier that morphs in place into the - palette's inline bar. Width is the animated property, so both states have - to be the same element — never a swap between two boxes. The transition is - authored here rather than as Tailwind utilities: react-ui ships a prebuilt - stylesheet, and `duration-standard`/`ease-*` compile to classes only in - react-ui's own build, so a utility class would be inert here. `--ease-in-out` - is react-ui's documented curve for something growing in place — a spring's - overshoot would jitter the whole top bar. Reduced motion is already handled - by that stylesheet's global transition-duration collapse. */ +/* A page's per-page filter (DECISIONS.md → Search): a magnifier that morphs + in place into a plain text input scoped to that page. Width is the + animated property, so both states have to be the same element — never a + swap between two boxes. The transition is authored here rather than as + Tailwind utilities: react-ui ships a prebuilt stylesheet, and + `duration-standard`/`ease-*` compile to classes only in react-ui's own + build, so a utility class would be inert here. `--ease-in-out` is + react-ui's documented curve for something growing in place — a spring's + overshoot would jitter the whole top bar. Reduced motion is already + handled by that stylesheet's global transition-duration collapse. This is + never the global command palette — see `command-palette-provider.tsx`, + mounted on its own. */ .stage-search { display: flex; flex-shrink: 0; @@ -770,22 +773,7 @@ select:disabled, color: var(--foreground); } -.stage-search [data-slot="command-palette-inline"] { - display: flex; - flex: 1; - min-width: 0; - align-items: center; -} - -.stage-search [data-slot="command-palette-inline-field"] { - display: flex; - flex: 1; - min-width: 0; - align-items: center; - gap: 0.35rem; -} - -.stage-search [data-slot="command-palette-input"] { +.stage-search-input { min-width: 0; flex: 1; border: 0; @@ -794,7 +782,7 @@ select:disabled, color: var(--foreground); } -.stage-search [data-slot="command-palette-input"]::placeholder { +.stage-search-input::placeholder { color: var(--muted-foreground); } diff --git a/apps/web/src/command-palette-open-store.ts b/apps/web/src/command-palette-open-store.ts index 719a10c5..634e7913 100644 --- a/apps/web/src/command-palette-open-store.ts +++ b/apps/web/src/command-palette-open-store.ts @@ -1,10 +1,10 @@ -// The state of the product's single search surface (DESIGN.md → Search), -// held outside the React tree because the surfaces that read and write it are -// siblings, not ancestors: `CommandPaletteProvider` renders the palette, -// `StageTopBar`'s magnifier morphs into it, and a context menu item opens it, -// and app.tsx's Shell mounts the first two side by side. One store, so the -// morph and the palette can never disagree about whether search is open, and -// so cmd+K, the magnifier, and a menu item all drive the same surface. +// The state of the global command palette (DECISIONS.md → Search) — a +// separate surface from the stage top bar's per-page filter, which owns no +// state here at all. Held outside the React tree because the things that +// open it are siblings, not ancestors: `CommandPaletteProvider` renders the +// palette itself, `Cmd+K` opens it from anywhere via `useCommandShortcut`, +// and a context menu item opens it too. One store, so all three ways in can +// never disagree about whether the palette is open. // // Module state outlives a React remount, so search is scoped explicitly: // `CommandPaletteProvider` closes it on a route change (a Back out of a diff --git a/apps/web/src/command-palette-provider.tsx b/apps/web/src/command-palette-provider.tsx index c32ac079..d834945e 100644 --- a/apps/web/src/command-palette-provider.tsx +++ b/apps/web/src/command-palette-provider.tsx @@ -1,5 +1,6 @@ import { artifactKindLabel, + CommandPalette, useCommandShortcut, useTheme, } from "@corbits/react-ui"; @@ -24,9 +25,7 @@ import { } from "@corbits/command-palette"; import { useQueryClient } from "@tanstack/react-query"; import { - createContext, useCallback, - useContext, useEffect, useMemo, useRef, @@ -43,6 +42,7 @@ import { import { openCommandPalette, setCommandPaletteOpen, + setCommandPaletteQuery, useCommandPaletteOpen, useCommandPaletteQuery, } from "./command-palette-open-store"; @@ -71,44 +71,12 @@ const STATIC_COMMANDS = buildStaticCommands( ); /** - * The data `StageSearch` renders — everything react-ui's `CommandPaletteInline` - * needs, computed once here rather than re-derived at the one place it is - * consumed. `StageSearch` owns the surface (the morphing bar, the input, - * the dropdown); this provider owns what fills it. - */ -export type CommandPaletteRenderProps = { - readonly groups: readonly CommandPaletteGroup[]; - readonly onSelect: (id: string) => void; - readonly loading: boolean; - readonly error?: string; - readonly hasMore: boolean; - readonly onLoadMore?: () => void; - readonly footer: string; -}; - -/** `StageTopBar` mounts in isolation across the page test suite (no - * workbench, no query client, nothing search needs) — an inert palette - * that renders a magnifier with no results is the honest fallback there. - * The real app always mounts `CommandPaletteProvider` above `AppShell` - * (`app.tsx`), so production code never sees this default. */ -const INERT_RENDER_PROPS: CommandPaletteRenderProps = { - groups: [], - onSelect: () => undefined, - loading: false, - hasMore: false, - footer: "", -}; - -const CommandPaletteRenderContext = - createContext(INERT_RENDER_PROPS); - -/** Read by `StageSearch`, mounted anywhere under `CommandPaletteProvider`. */ -export function useCommandPaletteRender(): CommandPaletteRenderProps { - return useContext(CommandPaletteRenderContext); -} - -/** - * Wires the data-driven react-ui command palette into the app shell. + * Wires the data-driven react-ui command palette into the app shell, and + * renders it — the global surface `Cmd+K` (and a context-menu item) opens, + * as its own modal dialog (`CommandPalette`), never anchored to the stage + * top bar's per-page filter magnifier. Mounted once in `app.tsx`'s `Shell`, + * above `AppShell`, so it works from every route — including one that + * matches no page and renders no stage top bar of its own. * * Grouping, `#`/`@`/`>`/`/` scope parsing, and the Recents rule live in * `@corbits/command-palette` (`buildCommandPaletteGroups`) — this file only @@ -118,11 +86,6 @@ export function useCommandPaletteRender(): CommandPaletteRenderProps { * `useEntitySearch` paging this provider already used; routines, skills and * library artifacts are small per-bench catalogs fetched once and filtered * client-side, the same way the static route list already is. - * - * This provider computes the data and hands it down through context; it - * renders no search surface itself. `StageSearch` (the top bar's magnifier) - * is the one place that data becomes UI — react-ui's `CommandPaletteInline`, - * anchored in place, never a centered dialog. */ export function CommandPaletteProvider({ path, @@ -136,8 +99,8 @@ export function CommandPaletteProvider({ const { memberships, selectedTenantId, selectTenant } = useBench(); const queryClient = useQueryClient(); // Open state and query live in the shared store, not in this component: - // the top nav's magnifier morphs into this very surface and has to read - // the same state (`command-palette-open-store`). + // Cmd+K and a context-menu item both open this surface from outside the + // React tree (`command-palette-open-store`). const open = useCommandPaletteOpen(); const query = useCommandPaletteQuery(); const [recents, setRecents] = useState([]); @@ -645,24 +608,23 @@ export function CommandPaletteProvider({ ], ); - const renderProps = useMemo( - () => ({ - groups, - onSelect: handleSelect, - loading, - // `exactOptionalPropertyTypes` distinguishes an absent key from an - // explicit `undefined`, so the key only appears when there is an error. - ...(error ? { error: "Search failed. Try again." } : {}), - hasMore, - onLoadMore: loadMore, - footer: "# workbenches · @ people · > actions · / pages", - }), - [groups, handleSelect, loading, error, hasMore, loadMore], - ); - return ( - + <> {children} - + + ); } diff --git a/apps/web/src/pages/library-page.tsx b/apps/web/src/pages/library-page.tsx index 14e26c6b..dcb6d47d 100644 --- a/apps/web/src/pages/library-page.tsx +++ b/apps/web/src/pages/library-page.tsx @@ -1,7 +1,6 @@ import { BulkActionBar, Button, - LibrarySearchInput, Menu, MenuContent, MenuItem, @@ -316,9 +315,10 @@ function PreviewPane({ * Every control the page owns — the workbench lens, the name filter, sort, * the rows/grid toggle, Upload — lives in `StageTopBar`'s action slot * (DESIGN.md → Pages & Routing: the top nav owns the page's actions, and a - * page body never floats its own). The name filter is a filter control, not - * a second search: the product has exactly one search surface and it is the - * palette the top bar already carries (DESIGN.md → Search). + * page body never floats its own). The name filter drives the stage top + * bar's own magnifier (`filter` prop) rather than a second input — the + * magnifier IS this page's filter, never the global palette (DECISIONS.md + * → Search). */ export function LibraryPage({ artifacts, @@ -440,6 +440,12 @@ export function LibraryPage({ ? `${artifacts.length} files` : artifactKindLabel(selectedSummary.kind) } + filter={{ + label: "Filter files", + placeholder: "Filter by name", + value: activeQuery, + onChange: setActiveQuery, + }} actions={ <> {selectedSummary !== null ? ( @@ -473,12 +479,6 @@ export function LibraryPage({ ) : null} - - ) : null} - + activeTab === "skills" ? ( + + ) : null } /> diff --git a/apps/web/src/pages/skills-page.tsx b/apps/web/src/pages/skills-page.tsx index 2a9ce96f..3e215e1d 100644 --- a/apps/web/src/pages/skills-page.tsx +++ b/apps/web/src/pages/skills-page.tsx @@ -20,7 +20,6 @@ import { Badge, Button, EmptyState, - LibrarySearchInput, RichEmptyState, Table, TableBody, @@ -123,10 +122,23 @@ export function SkillsPage({ const crumbs = [{ label: "Skills" }]; - function stage(actions: ReactNode, body: ReactNode) { + function stage( + actions: ReactNode, + body: ReactNode, + filter?: { + readonly value: string; + readonly onChange: (value: string) => void; + }, + ) { return (
- +
{body} @@ -194,14 +206,7 @@ export function SkillsPage({ ); return stage( - <> - - {newSkillButton} - , + newSkillButton,
{filtered.length === 0 ? ( , + { value: query, onChange: setQuery }, ); } diff --git a/apps/web/src/shell/stage-search.tsx b/apps/web/src/shell/stage-search.tsx index 9535f590..414860f1 100644 --- a/apps/web/src/shell/stage-search.tsx +++ b/apps/web/src/shell/stage-search.tsx @@ -1,47 +1,49 @@ -// The one way into search from chrome (DESIGN.md → Search): a magnifier that -// morphs in place into an inline bar and hands the query to the command -// palette — the product's single search surface. There is no second search -// implementation behind this control; it opens the same palette cmd+K does, -// and its expanded/collapsed state IS the palette's open state -// (`command-palette-open-store`), so the two can never disagree. +// The stage top bar's per-page filter (DECISIONS.md → Search): a magnifier +// that morphs in place into a plain text input scoped to whatever the page +// is showing — Files filters files, Skills filters skills. It never reaches +// the global command palette; `Cmd+K` is a separate surface entirely +// (`command-palette-provider.tsx`), mounted on its own rather than out of +// this control. // -// The palette itself is react-ui's non-modal `CommandPaletteInline`: the -// field it renders IS the real, focusable search input, anchored to this -// control, with its results hanging directly beneath — never a centered -// dialog the magnifier merely opens. `leading` carries the magnifier button -// itself, so the collapsed control and the expanded bar are one continuous -// element rather than a button and a separate window. +// A page hands in the filter it already owns (`value`/`onChange`); this +// component only supplies the chrome — the button, the morph, and the input +// that drives that state directly. `StageTopBar` renders it only when a page +// passes a filter, so a page with nothing to filter shows no magnifier. // // Motion is the width transition authored on `.stage-search` in app.css -// (react-ui's `--duration-standard` and `--ease-in-out`, the curve its -// theme documents for something growing in place). Reduced motion needs -// nothing here: react-ui's stylesheet already collapses every transition -// duration under `prefers-reduced-motion`, which makes the swap instant. +// (react-ui's `--duration-standard` and `--ease-in-out`). Reduced motion +// needs nothing here: react-ui's stylesheet already collapses every +// transition duration under `prefers-reduced-motion`. import { MagnifyingGlass } from "@corbits/icons"; -import { CommandPaletteInline } from "@corbits/react-ui"; -import { useEffect, useRef } from "react"; +import { useEffect, useRef, useState } from "react"; -import { useCommandPaletteRender } from "../command-palette-provider"; -import { - openCommandPalette, - setCommandPaletteOpen, - setCommandPaletteQuery, - useCommandPaletteOpen, - useCommandPaletteQuery, -} from "../command-palette-open-store"; +export type StageSearchProps = { + /** Accessible name for both the button and the input, and the default + * placeholder — e.g. "Filter files". Never "Search …": this is a filter, + * not the product's search surface. */ + readonly label: string; + readonly value: string; + readonly onChange: (value: string) => void; + readonly placeholder?: string; +}; -export function StageSearch() { - const open = useCommandPaletteOpen(); - const query = useCommandPaletteQuery(); - const render = useCommandPaletteRender(); +export function StageSearch({ + label, + value, + onChange, + placeholder, +}: StageSearchProps) { + const [open, setOpen] = useState(value.length > 0); + const wasOpen = useRef(open); const buttonRef = useRef(null); - const wasOpen = useRef(false); + const inputRef = useRef(null); + // A query the page already carries in (a prefilled filter) keeps the bar + // expanded even before anyone has focused it. + const expanded = open || value.length > 0; - // Whichever way the palette closed — Escape, an outside click, the - // store — focus comes back to the control the morph came out of, instead - // of being dropped on the document. useEffect(() => { + if (open) inputRef.current?.focus(); if (wasOpen.current && !open) buttonRef.current?.focus(); wasOpen.current = open; }, [open]); @@ -50,37 +52,38 @@ export function StageSearch() {
- -
); } diff --git a/apps/web/src/shell/stage-top-bar.tsx b/apps/web/src/shell/stage-top-bar.tsx index 3ac1a982..9f50a166 100644 --- a/apps/web/src/shell/stage-top-bar.tsx +++ b/apps/web/src/shell/stage-top-bar.tsx @@ -8,10 +8,12 @@ // carries an `href`, so the trail is deep-linkable and a plain click // navigates through the app's own `Link` instead of reloading the shell. // -// The bar also carries the product's one search entry point (`StageSearch`, -// DESIGN.md → Search) ahead of the page's own controls. It is shell chrome, -// not a page action: no page passes it, and no page can opt out — that is -// what makes "exactly one search surface" true of every route at once. +// `filter` is a page's own per-page filter (DECISIONS.md → Search), rendered +// through `StageSearch` ahead of `actions` when a page passes one. It is not +// shell chrome the way it used to be: a page with nothing to filter passes +// none and gets no magnifier at all. The global command palette (`Cmd+K`) +// is a separate surface entirely — see `command-palette-provider.tsx` — +// mounted on its own rather than out of this bar. // // `@corbits/react-ui`'s `TopBarBreadcrumbs` renders bare ``, which // would drop the SPA out from under the click, so the trail lives here @@ -21,7 +23,7 @@ import { Fragment, type ReactNode } from "react"; import { Link } from "../navigation"; import { Chip, type ChipTone } from "./chip"; -import { StageSearch } from "./stage-search"; +import { StageSearch, type StageSearchProps } from "./stage-search"; export type StageCrumb = { readonly label: string; @@ -34,6 +36,7 @@ export function StageTopBar({ crumbs, subtitle, chip, + filter, actions, }: { /** The page's title trail: parents first, the page itself last. */ @@ -42,6 +45,10 @@ export function StageTopBar({ /** A quiet status pill (mock's `.chip[data-tone]`), rendered first among * the right-aligned actions — ambient state, not a button. */ readonly chip?: { readonly tone: ChipTone; readonly label: ReactNode }; + /** This page's own filter, if it has one — rendered as the magnifier that + * morphs into an input (`StageSearch`), driving the page's own filter + * state directly. Omitted entirely on a page with nothing to filter. */ + readonly filter?: StageSearchProps; /** The primary-action slot: the buttons and inputs this page owns. */ readonly actions?: ReactNode; }) { @@ -61,7 +68,7 @@ export function StageTopBar({ className="stage-top-bar-actions" data-testid="stage-top-bar-actions" > - + {filter !== undefined ? : null} {chip !== undefined ? {chip.label} : null} {actions}
diff --git a/apps/web/test/global-command-palette.test.tsx b/apps/web/test/global-command-palette.test.tsx new file mode 100644 index 00000000..bdb428bb --- /dev/null +++ b/apps/web/test/global-command-palette.test.tsx @@ -0,0 +1,245 @@ +// DECISIONS.md → Search: `Cmd+K` opens the global command palette — a +// separate surface from the stage top bar's per-page filter magnifier +// (`stage-search-filter.test.tsx`). It has to work from anywhere, including +// a route that renders no stage top bar of its own (an unmatched route), +// which is exactly what PR #246 broke: the palette used to live inside +// `StageSearch`, so no `StageTopBar` meant no palette. `CommandPaletteProvider` +// now renders the palette itself, mounted once above `AppShell`, independent +// of whatever the route renders. + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { ThemeProvider } from "@corbits/react-ui"; + +import { BenchProvider } from "../src/bench-context"; +import { CommandPaletteProvider } from "../src/command-palette-provider"; +import { setCommandPaletteOpen } from "../src/command-palette-open-store"; +import { NavigationProvider } from "../src/navigation"; +import { StageTopBar } from "../src/shell/stage-top-bar"; +import { TestQueryProvider } from "./test-query-provider"; + +const noop = () => undefined; +const realFetch = globalThis.fetch; +const realMatchMedia = window.matchMedia; + +const TENANT = "tnt_1"; + +function stubMatchMedia(): void { + window.matchMedia = ((media: string) => + ({ + media, + matches: false, + addEventListener: noop, + removeEventListener: noop, + }) as unknown as MediaQueryList) as typeof window.matchMedia; +} + +const json = (body: unknown, status = 200) => + new Response(JSON.stringify(body), { + status, + headers: { "content-type": "application/json" }, + }); + +const slugHandled = { + id: "wfd_1", + tenantId: TENANT, + name: "research-analyst", + description: "Answers research questions", + currentVersion: "1", + status: "deployed" as const, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", +}; + +function stubShellFetch(): void { + globalThis.fetch = ((input: RequestInfo | URL) => { + const path = String(input); + if (path.includes("/api/me/principals")) + return Promise.resolve( + json({ + data: [ + { + principalId: "prn_1", + tenantId: TENANT, + tenantName: "Corbits Bench", + tenantSlug: "corbits-bench", + kind: "user", + status: "active", + roles: [], + }, + ], + nextCursor: null, + }), + ); + if (path.includes("/api/workbench-tenancies/kinds")) + return Promise.resolve(json({ workbenchTenantIds: [] })); + if (path.includes("/workflows/definitions")) + return Promise.resolve(json({ data: [slugHandled], nextCursor: null })); + if (path.includes("/mcp-servers")) + return Promise.resolve(json({ data: [] })); + if (path.includes("/skills")) return Promise.resolve(json({ skills: [] })); + if (path.includes("/routines")) return Promise.resolve(json({ data: [] })); + return Promise.resolve(json({ data: [], nextCursor: null })); + }) as typeof fetch; +} + +let container: HTMLDivElement; +let root: Root; + +beforeEach(() => { + stubMatchMedia(); + stubShellFetch(); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + setCommandPaletteOpen(false); + globalThis.fetch = realFetch; + window.matchMedia = realMatchMedia; +}); + +async function settle(): Promise { + for (let i = 0; i < 25; i++) { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + }); + } +} + +async function render(node: React.ReactElement): Promise { + await act(async () => { + root.render(node); + }); + await settle(); +} + +function paletteInputs(): readonly HTMLInputElement[] { + return [...document.querySelectorAll('[role="combobox"]')]; +} + +function paletteInput(): HTMLInputElement { + const input = paletteInputs()[0]; + if (input === undefined) throw new Error("the palette rendered no input"); + return input; +} + +async function pressCmdK(): Promise { + await act(async () => { + document.dispatchEvent( + new KeyboardEvent("keydown", { key: "k", metaKey: true, bubbles: true }), + ); + }); + await settle(); +} + +async function typeInPalette(value: string): Promise { + const setValue = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + "value", + )?.set; + if (setValue === undefined) throw new Error("no native value setter"); + const input = paletteInput(); + await act(async () => { + setValue.call(input, value); + input.dispatchEvent(new Event("input", { bubbles: true })); + }); + await settle(); +} + +function resultRow(text: string): HTMLElement { + const row = [ + ...document.querySelectorAll('[role="option"]'), + ].find((option) => option.textContent?.includes(text)); + if (row === undefined) throw new Error(`no result row for ${text}`); + return row; +} + +/** `withStageTopBar={false}` stands in for an unmatched route: no + * `StageTopBar`, no magnifier, nothing the old, palette-inside-the-magnifier + * wiring needed to mount the overlay. */ +function Harness({ + navigate = noop, + path = "/agents", + withStageTopBar = true, +}: { + readonly navigate?: (to: string) => void; + readonly path?: string; + readonly withStageTopBar?: boolean; +}) { + return ( + + + + + + {withStageTopBar ? ( + + ) : ( +
Page not found
+ )} +
+
+
+
+
+ ); +} + +describe("Cmd+K opens the global command palette", () => { + test("from an ordinary route", async () => { + await render(); + expect(paletteInputs()).toHaveLength(0); + + await pressCmdK(); + + expect(paletteInputs()).toHaveLength(1); + }); + + test("from an unmatched route with no stage top bar of its own", async () => { + await render(); + expect(container.textContent).toContain("Page not found"); + expect(paletteInputs()).toHaveLength(0); + + await pressCmdK(); + + expect(paletteInputs()).toHaveLength(1); + }); + + test("the palette is not reachable by clicking anything in the stage top bar — there is no magnifier door into it", async () => { + await render(); + const stageSearch = container.querySelector('[data-testid="stage-search"]'); + // Agents has nothing to filter, so it carries no magnifier at all. + expect(stageSearch).toBeNull(); + }); + + test("a route change closes the palette, so Back never leaves it standing", async () => { + await render(); + await pressCmdK(); + expect(paletteInputs()).toHaveLength(1); + + await render(); + + expect(paletteInputs()).toHaveLength(0); + }); + + test("selecting a result navigates to that entity's own slug detail route", async () => { + const navigated: string[] = []; + await render( navigated.push(to)} />); + + await pressCmdK(); + await typeInPalette("@"); + + await act(async () => { + resultRow("research-analyst").dispatchEvent( + new MouseEvent("click", { bubbles: true }), + ); + }); + + expect(navigated).toContain("/agents/research-analyst"); + }); +}); diff --git a/apps/web/test/global-search-morph.test.tsx b/apps/web/test/global-search-morph.test.tsx deleted file mode 100644 index a6d8de07..00000000 --- a/apps/web/test/global-search-morph.test.tsx +++ /dev/null @@ -1,417 +0,0 @@ -// CL-6410 / CL-6487: the product's one search surface. DESIGN.md's Search -// section fixes how it is invoked from chrome: the top-nav magnifier morphs -// in place into an inline bar, Esc collapses it, and cmd+K reaches the -// identical palette — never a second search implementation, and never a -// centered dialog. The field the morph reveals IS the real, focusable -// search input (react-ui's `CommandPaletteInline`) — not a span mirroring a -// query typed somewhere else. -// -// The motion assertions deliberately check the authored stylesheet and the -// tokens it consumes, not class names on the element: react-ui ships a -// prebuilt stylesheet, so a Tailwind motion utility (`duration-standard`, -// `ease-spring`) compiles to nothing here and a className assertion would -// green-light a morph that never runs. - -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; -import { act } from "react"; -import { createRoot, type Root } from "react-dom/client"; -import { ThemeProvider } from "@corbits/react-ui"; - -import { BenchProvider } from "../src/bench-context"; -import { CommandPaletteProvider } from "../src/command-palette-provider"; -import { setCommandPaletteOpen } from "../src/command-palette-open-store"; -import { NavigationProvider } from "../src/navigation"; -import { StageTopBar } from "../src/shell/stage-top-bar"; -import { TestQueryProvider } from "./test-query-provider"; - -const noop = () => undefined; -const realFetch = globalThis.fetch; -const realMatchMedia = window.matchMedia; - -const TENANT = "tnt_1"; - -const appCss = readFileSync(new URL("../src/app.css", import.meta.url), "utf8"); -const reactUiCss = readFileSync( - new URL("../node_modules/@corbits/react-ui/dist/styles.css", import.meta.url), - "utf8", -); - -/** The declaration block of the rule that names `className`. */ -function ruleFor(css: string, className: string): string { - const selector = new RegExp(`\\.${className}\\s*[,{]`); - const block = css.split("}").find((candidate) => selector.test(candidate)); - if (block === undefined) throw new Error(`no rule for .${className}`); - return block.slice(block.indexOf("{")); -} - -function stubMatchMedia(matching: Record): void { - window.matchMedia = ((media: string) => - ({ - media, - matches: matching[media] ?? false, - addEventListener: noop, - removeEventListener: noop, - }) as unknown as MediaQueryList) as typeof window.matchMedia; -} - -const json = (body: unknown, status = 200) => - new Response(JSON.stringify(body), { - status, - headers: { "content-type": "application/json" }, - }); - -const slugHandled = { - id: "wfd_1", - tenantId: TENANT, - name: "research-analyst", - description: "Answers research questions", - currentVersion: "1", - status: "deployed" as const, - createdAt: "2026-01-01T00:00:00.000Z", - updatedAt: "2026-01-01T00:00:00.000Z", -}; - -/** A handle that is not a slug — minted before the rule tightened, or - * imported. Its detail route cannot be guessed at. */ -const unsluggedHandle = { - ...slugHandled, - id: "wfd_2", - name: "Café Crème Bot", -}; - -function stubShellFetch(): void { - globalThis.fetch = ((input: RequestInfo | URL) => { - const path = String(input); - if (path.includes("/api/me/principals")) - return Promise.resolve( - json({ - data: [ - { - principalId: "prn_1", - tenantId: TENANT, - tenantName: "Corbits Bench", - tenantSlug: "corbits-bench", - kind: "user", - status: "active", - roles: [], - }, - ], - nextCursor: null, - }), - ); - if (path.includes("/api/workbench-tenancies/kinds")) - return Promise.resolve(json({ workbenchTenantIds: [] })); - if (path.includes("/workflows/definitions")) - return Promise.resolve( - json({ data: [slugHandled, unsluggedHandle], nextCursor: null }), - ); - if (path.includes("/mcp-servers")) - return Promise.resolve(json({ data: [] })); - if (path.includes("/skills")) return Promise.resolve(json({ skills: [] })); - if (path.includes("/routines")) return Promise.resolve(json({ data: [] })); - return Promise.resolve(json({ data: [], nextCursor: null })); - }) as typeof fetch; -} - -let container: HTMLDivElement; -let root: Root; - -beforeEach(() => { - stubMatchMedia({}); - stubShellFetch(); - container = document.createElement("div"); - document.body.appendChild(container); - root = createRoot(container); -}); - -afterEach(() => { - act(() => root.unmount()); - container.remove(); - setCommandPaletteOpen(false); - globalThis.fetch = realFetch; - window.matchMedia = realMatchMedia; -}); - -async function settle(): Promise { - for (let i = 0; i < 25; i++) { - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 10)); - }); - } -} - -async function render(node: React.ReactElement): Promise { - await act(async () => { - root.render(node); - }); - await settle(); -} - -function searchShell(): HTMLElement { - const shell = container.querySelector( - '[data-testid="stage-search"]', - ); - if (shell === null) throw new Error("the top nav renders no search control"); - return shell; -} - -function magnifier(): HTMLButtonElement { - const button = searchShell().querySelector( - 'button[aria-label="Search"]', - ); - if (button === null) throw new Error("no magnifier in the top nav"); - return button; -} - -function paletteInputs(): readonly HTMLInputElement[] { - return [...document.querySelectorAll('[role="combobox"]')]; -} - -function paletteInput(): HTMLInputElement { - const input = paletteInputs()[0]; - if (input === undefined) throw new Error("the palette rendered no input"); - return input; -} - -async function typeInPalette(value: string): Promise { - const setValue = Object.getOwnPropertyDescriptor( - window.HTMLInputElement.prototype, - "value", - )?.set; - if (setValue === undefined) throw new Error("no native value setter"); - const input = paletteInput(); - await act(async () => { - setValue.call(input, value); - input.dispatchEvent(new Event("input", { bubbles: true })); - }); - await settle(); -} - -async function pressEscapeInPalette(): Promise { - await act(async () => { - paletteInput().dispatchEvent( - new KeyboardEvent("keydown", { - key: "Escape", - bubbles: true, - cancelable: true, - }), - ); - }); - await settle(); -} - -function resultRow(text: string): HTMLElement { - const row = [ - ...document.querySelectorAll('[role="option"]'), - ].find((option) => option.textContent?.includes(text)); - if (row === undefined) throw new Error(`no result row for ${text}`); - return row; -} - -function Harness({ - navigate = noop, - path = "/agents", -}: { - readonly navigate?: (to: string) => void; - readonly path?: string; -}) { - return ( - - - - - - - - - - - - ); -} - -describe("the top-nav search morph", () => { - test("the collapsed control is a magnifier and nothing else", async () => { - await render(); - expect(magnifier().getAttribute("aria-expanded")).toBe("false"); - expect(paletteInputs()).toHaveLength(0); - }); - - test("clicking the magnifier morphs it in place into the inline bar", async () => { - await render(); - await act(async () => { - magnifier().click(); - }); - await settle(); - - expect(paletteInputs()).toHaveLength(1); - expect(magnifier().getAttribute("aria-expanded")).toBe("true"); - expect(searchShell().dataset.expanded).toBe("true"); - }); - - test("the morph is a real transition: authored on the element, on tokens the shipped stylesheet defines", () => { - const rule = ruleFor(appCss, "stage-search"); - // One element whose width animates — a swap between two boxes could not - // transition at all. - expect(rule).toContain("transition: width var(--duration-standard)"); - // react-ui's documented curve for something growing in place; a spring's - // overshoot would jitter the whole top bar. - expect(rule).toContain("var(--ease-in-out)"); - // Both tokens have to exist in the prebuilt sheet the app actually - // imports, or the declaration silently resolves to nothing. - expect(reactUiCss).toContain("--duration-standard:"); - expect(reactUiCss).toContain("--ease-in-out:"); - }); - - test("the morph carries no Tailwind motion utility, which would be inert against the prebuilt stylesheet", async () => { - await render(); - await act(async () => { - magnifier().click(); - }); - expect(searchShell().className).not.toContain("duration-"); - expect(searchShell().className).not.toContain("ease-"); - expect(searchShell().className).not.toContain("transition-"); - }); - - test("reduced motion needs no per-element handling: the shipped stylesheet collapses every transition", () => { - const reducedMotionBlock = reactUiCss.slice( - reactUiCss.lastIndexOf("prefers-reduced-motion: reduce"), - ); - expect(reducedMotionBlock).toContain("transition-duration: 0.01ms"); - }); - - test("the inline bar's field is the real search input, not a second surface mirroring one", async () => { - await render(); - await act(async () => { - magnifier().click(); - }); - await settle(); - await typeInPalette("resea"); - - // Exactly one editable field in the whole surface, and it lives inside - // the top-bar morph itself — never a separate dialog elsewhere. - expect(paletteInputs()).toHaveLength(1); - expect(searchShell().contains(paletteInput())).toBe(true); - expect(paletteInput().value).toBe("resea"); - }); -}); - -describe("collapsing back to the magnifier", () => { - test("Escape inside the palette collapses the morph and returns focus to the magnifier", async () => { - await render(); - await act(async () => { - magnifier().click(); - }); - await settle(); - expect(paletteInputs()).toHaveLength(1); - - await pressEscapeInPalette(); - - expect(paletteInputs()).toHaveLength(0); - expect(magnifier().getAttribute("aria-expanded")).toBe("false"); - expect(document.activeElement).toBe(magnifier()); - }); - - test("focus lands on the magnifier even when the palette was opened by cmd+K, which never focused it", async () => { - await render(); - await act(async () => { - document.dispatchEvent( - new KeyboardEvent("keydown", { - key: "k", - metaKey: true, - bubbles: true, - }), - ); - }); - await settle(); - expect(document.activeElement).not.toBe(magnifier()); - - await pressEscapeInPalette(); - - expect(document.activeElement).toBe(magnifier()); - }); - - test("a route change closes the surface, so Back never leaves it standing", async () => { - await render(); - await act(async () => { - magnifier().click(); - }); - await settle(); - expect(paletteInputs()).toHaveLength(1); - - await render(); - - expect(paletteInputs()).toHaveLength(0); - }); -}); - -describe("one search surface, two doors", () => { - test("cmd+K and the magnifier open the identical palette", async () => { - await render(); - expect(paletteInputs()).toHaveLength(0); - - await act(async () => { - document.dispatchEvent( - new KeyboardEvent("keydown", { - key: "k", - metaKey: true, - bubbles: true, - }), - ); - }); - await settle(); - expect(paletteInputs()).toHaveLength(1); - const fromShortcut = paletteInput().getAttribute("aria-label"); - expect(searchShell().dataset.expanded).toBe("true"); - - await pressEscapeInPalette(); - expect(paletteInputs()).toHaveLength(0); - - await act(async () => { - magnifier().click(); - }); - await settle(); - expect(paletteInputs()).toHaveLength(1); - expect(paletteInput().getAttribute("aria-label")).toBe(fromShortcut); - }); - - test("selecting a result navigates to that entity's own slug detail route", async () => { - const navigated: string[] = []; - await render( navigated.push(to)} />); - - await act(async () => { - magnifier().click(); - }); - await settle(); - await typeInPalette("@"); - - await act(async () => { - resultRow("research-analyst").dispatchEvent( - new MouseEvent("click", { bubbles: true }), - ); - }); - - expect(navigated).toContain("/agents/research-analyst"); - }); - - test("an entity whose handle is not a slug keeps its id deep link, never a guessed slug", async () => { - const navigated: string[] = []; - await render( navigated.push(to)} />); - - await act(async () => { - magnifier().click(); - }); - await settle(); - await typeInPalette("@"); - - await act(async () => { - resultRow("Café Crème Bot").dispatchEvent( - new MouseEvent("click", { bubbles: true }), - ); - }); - - expect(navigated).toContain("/agents/wfd_2"); - expect(navigated).not.toContain("/agents/cafe-creme-bot"); - }); -}); diff --git a/apps/web/test/plugins-page.test.tsx b/apps/web/test/plugins-page.test.tsx index e3e4f293..5460ce18 100644 --- a/apps/web/test/plugins-page.test.tsx +++ b/apps/web/test/plugins-page.test.tsx @@ -192,7 +192,7 @@ describe("PluginsRoute", () => { const el = await mount(); expect( - el.querySelector('input[aria-label="Search plugins"]'), + el.querySelector('button[aria-label="Filter plugins"]'), ).not.toBeNull(); expect(el.textContent).not.toContain("New skill"); @@ -205,7 +205,7 @@ describe("PluginsRoute", () => { }); expect( - el.querySelector('input[aria-label="Search skills"]'), + el.querySelector('button[aria-label="Filter skills"]'), ).not.toBeNull(); expect(el.textContent).toContain("New skill"); expect(el.textContent).toContain("weekly-digest"); diff --git a/apps/web/test/stage-search-filter.test.tsx b/apps/web/test/stage-search-filter.test.tsx new file mode 100644 index 00000000..e67bd4f9 --- /dev/null +++ b/apps/web/test/stage-search-filter.test.tsx @@ -0,0 +1,174 @@ +// DECISIONS.md → Search: the stage top bar's magnifier filters the page it +// is on — it is not a door into the global command palette, and it never +// was meant to be one after CL-6487/CL-6410 conflated the two (PR #246). +// This suite covers the per-page filter surface in isolation: a page hands +// in its own `value`/`onChange`, and the magnifier morphs into a plain +// input that drives that state directly, never the palette's open store. + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { useState } from "react"; + +import { + setCommandPaletteOpen, + useCommandPaletteOpen, +} from "../src/command-palette-open-store"; +import { StageTopBar } from "../src/shell/stage-top-bar"; + +const appCss = readFileSync(new URL("../src/app.css", import.meta.url), "utf8"); + +function ruleFor(css: string, className: string): string { + const selector = new RegExp(`\\.${className}\\s*[,{]`); + const block = css.split("}").find((candidate) => selector.test(candidate)); + if (block === undefined) throw new Error(`no rule for .${className}`); + return block.slice(block.indexOf("{")); +} + +let container: HTMLDivElement; +let root: Root; + +beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); + setCommandPaletteOpen(false); +}); + +function render(node: React.ReactElement): void { + act(() => { + root.render(node); + }); +} + +function magnifier(): HTMLButtonElement { + const button = container.querySelector( + '[data-testid="stage-search"] button', + ); + if (button === null) throw new Error("no magnifier rendered"); + return button; +} + +function filterInput(): HTMLInputElement | null { + return container.querySelector( + '[data-testid="stage-search"] input', + ); +} + +function Harness({ filterable = true }: { readonly filterable?: boolean }) { + const [value, setValue] = useState(""); + // Reads the global palette's open state alongside the filter, so a test + // can assert typing in the page filter never touches it. + const globalOpen = useCommandPaletteOpen(); + return ( + <> + {String(globalOpen)} + + + ); +} + +describe("the stage top bar's per-page filter", () => { + test("a page with a filter shows a magnifier that morphs into an input", () => { + render(); + expect(filterInput()).toBeNull(); + + act(() => magnifier().click()); + + const input = filterInput(); + expect(input).not.toBeNull(); + expect(input?.getAttribute("aria-label")).toBe("Filter files"); + expect(input?.getAttribute("placeholder")).toBe("Filter by name"); + }); + + test("typing filters the page directly and never opens the global palette", () => { + render(); + act(() => magnifier().click()); + + const input = filterInput(); + if (input === null) throw new Error("no filter input"); + const setValue = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + "value", + )?.set; + if (setValue === undefined) throw new Error("no native value setter"); + act(() => { + setValue.call(input, "invoice"); + input.dispatchEvent(new Event("input", { bubbles: true })); + }); + + expect(filterInput()?.value).toBe("invoice"); + expect( + container.querySelector('[data-testid="global-open"]')?.textContent, + ).toBe("false"); + }); + + test("Escape clears the query first, then collapses back to the magnifier", () => { + render(); + act(() => magnifier().click()); + + const setValue = Object.getOwnPropertyDescriptor( + window.HTMLInputElement.prototype, + "value", + )?.set; + const input = filterInput(); + if (input === null || setValue === undefined) + throw new Error("setup failed"); + act(() => { + setValue.call(input, "invoice"); + input.dispatchEvent(new Event("input", { bubbles: true })); + }); + + act(() => { + filterInput()?.dispatchEvent( + new KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + cancelable: true, + }), + ); + }); + expect(filterInput()?.value).toBe(""); + + act(() => { + filterInput()?.dispatchEvent( + new KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + cancelable: true, + }), + ); + }); + expect(filterInput()).toBeNull(); + expect(document.activeElement).toBe(magnifier()); + }); + + test("a page with nothing to filter renders no magnifier at all", () => { + render(); + expect(container.querySelector('[data-testid="stage-search"]')).toBeNull(); + }); + + test("the magnifier keeps its 40px minimum hit target", () => { + const rule = ruleFor(appCss, "stage-search-button"); + expect(rule).toContain("width: 2.75rem"); + expect(rule).toContain("height: 2.75rem"); + }); +}); diff --git a/docs/command-palette.md b/docs/command-palette.md index 2ad8abf8..f7adfff0 100644 --- a/docs/command-palette.md +++ b/docs/command-palette.md @@ -7,33 +7,24 @@ navigation. ## How it's invoked -This palette is the product's only search surface (DESIGN.md → Search), and -it has exactly two doors: Cmd/Ctrl-K anywhere, and the magnifier the shell's -top bar carries on every route (`StageSearch` in -`apps/web/src/shell/stage-search.tsx`). Clicking the magnifier morphs it in -place into an inline bar — a width transition authored in `app.css` on -react-ui's `--duration-standard` and `--ease-in-out` (the curve its theme -documents for something growing in place; the app imports react-ui's -prebuilt stylesheet, where Tailwind motion utilities do not exist) — and -opens this same overlay. Escape collapses the bar back to the magnifier and -returns focus to it. Reduced motion needs nothing here: that stylesheet -already collapses every transition duration under -`prefers-reduced-motion`. - -Both doors read and write one state, `command-palette-open-store.ts`: an -external store rather than component state, because the palette provider and -the top bar are siblings in `app.tsx`'s Shell, and a context-menu item opens -the palette too. That state outlives a remount, so it is scoped explicitly — -the provider closes search on a route change (a Back out of a result never -leaves the overlay standing) and on a bench switch. cmd+K opens and does not -toggle: react-ui's shortcut yields to text fields, and an open palette holds -focus in its own input, so Escape and the overlay are the ways back out. - -Because react-ui's `CommandPalette` is a modal dialog that owns the editable -input once open, the morphed bar _shows_ the live query as text rather than -rendering a second input a click could land in — one editable search field in -the product, with the morph showing where the overlay came from. An anchored, -non-modal palette in react-ui would let that bar be the input itself. +This palette is a separate surface from the stage top bar's per-page filter +magnifier (DESIGN.md → Search, `docs/DECISIONS.md` → Search) — the two are +never merged, and neither opens the other. The palette has exactly two doors: +Cmd/Ctrl-K anywhere, and a context-menu item. Both call +`openCommandPalette()` from `command-palette-open-store.ts`, an external +store rather than component state because those doors and +`CommandPaletteProvider` (which renders the palette itself, mounted once in +`app.tsx`'s Shell above `AppShell`) are siblings, not ancestor/descendant. +That state outlives a remount, so it is scoped explicitly — the provider +closes search on a route change (a Back out of a result never leaves the +overlay standing) and on a bench switch. cmd+K opens and does not toggle: +react-ui's shortcut yields to text fields, and an open palette holds focus in +its own input, so Escape and the overlay are the ways back out. + +The palette renders as react-ui's `CommandPalette`, a centered modal dialog — +its own surface, independent of any page's stage top bar, which is exactly +what makes it reachable from a route that renders no stage top bar of its +own (an unmatched route included). ## Where it lives @@ -68,9 +59,10 @@ with three responsibilities: as small per-bench catalogs (filtered client-side, the same way the static route list already is), lists the bench's connected MCP servers as Plugins, builds `@corbits/command-palette`'s static commands -from `apps/web/src/routes.tsx`, and hands the assembled groups to react-ui's -data-driven palette. No new endpoint beyond the ones the Routines, Skills, -and Library pages already use, no domain logic in the app. +from `apps/web/src/routes.tsx`, and renders the assembled groups through +react-ui's data-driven `CommandPalette` itself. No new endpoint beyond the +ones the Routines, Skills, and Library pages already use, no domain logic in +the app. ## Groups, in display order