mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-15 20:53:23 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Its UI is the operator's daily surface: task lists, boards, budgets, agent status — all built on shadcn components and Tailwind > - Visual values (colors, spacing, type sizes, radii) were hardcoded at ~1,600 call sites: the same "small gray label" was 9/10/11px depending on the file, charts disagreed with chips about status colors, two toggle-switch implementations coexisted in two greens, and there was no visual regression coverage > - This made the UI drift-prone and made any restyle a hundreds-of-files project, which discourages design iteration > - This pull request extracts visual values into a single token layer in `ui/src/index.css`, adds a Storybook visual regression suite backed by external immutable baseline archives, and then applies a deliberate retune reviewed change-by-change on screenshot diffs > - The benefit is that Paperclip's look becomes a config surface: retheming is a token edit reviewed as a snapshot diff, drift is blocked by a token gate, and future UI PRs can prove exactly what changed visually without committing hundreds of PNGs ## Linked Issues or Issue Description No existing public issue covers this work (searched "design tokens", "visual regression", "design system" across issues and PRs). Related in spirit: Refs #8982 (theming a hardcoded panel — a one-off instance of the same problem class this PR addresses systematically). **Problem (feature-request form):** UI visual values are hardcoded per call site with no source of truth and no regression coverage; consistency depends on reviewer memory, and restyling requires mass file edits. **Proposed solution (this PR):** a single token layer + enforcement gate + externally stored visual snapshot suite, then an intentional restyle on top of that foundation. ## What Changed - **Token extraction (zero visual change, machine-verified during development):** committed codemods (`scripts/codemod-*.mjs`) moved ~1,600 hardcoded color/type/spacing/radius/shadow/misc values into named tokens in a non-inline `:root` block of `ui/src/index.css`. - **Visual regression suite:** `pnpm test:storybook-visual` covers 255 stories × light/dark = 510 Playwright screenshots at `maxDiffPixels: 0`, plus new primitive-coverage stories and deterministic-render fixes. - **External visual baselines:** committed PNG snapshots were removed. `tests/storybook-visual/baseline-manifest.json` pins an immutable archive URL/hash/size/count, and `scripts/storybook-visual-baseline.mjs` handles `download`, `verify`, `pack`, and trusted maintainer `upload` flows. - **Opt-in visual CI artifacts:** added a `Storybook Visual` workflow that runs on manual dispatch or PRs labeled `storybook-visual`, downloads/verifies the baseline, runs Playwright, and uploads Playwright report/test-result artifacts for review. Normal PR runs do not mutate baseline objects. - **Token gate:** `pnpm check:token-gates` — zero hex literals, zero arbitrary bracket values, zero raw font-sizes in `ui/src/components/**` and `ui/src/pages/**`, with a documented inline allowlist for legitimate opt-outs. - **Theme retune (intentional, snapshot-reviewed):** new base theme values; radius ladder derived from a single `--radius` knob; micro-type cluster collapsed to a named ladder (`--text-nano/micro/compact` + Tailwind `text-xs`/`text-sm`); letter-spacing collapsed to named steps. - **One status-color vocabulary:** charts, quota/budget bar fills, RUNNING/live chips, and liveness indicators all use the canonical `--status-*` hues. Light-mode legibility fixes for red alert surfaces that used dark-tuned text classes. - **One switch:** `ToggleSwitch` restyled to the registry capsule form, second hand-rolled implementation removed, and all call sites unified. - **Docs:** `DESIGN.md` is the design contract; `doc/design/` holds audit reports, decision logs, and updated guidance for external baseline review/update workflows. - Dead code removed (`agentStatusBadge` duplicate map), byte-identical contrast constants consolidated, semantic renames (`--project-seed`/`--project-none`, `--liveness-blue`). ## Verification - `pnpm check:token-gates` — 3/3 gates CLEAN during the design-system run - `pnpm typecheck` && `pnpm --filter @paperclipai/ui build` — green during the design-system run - `node --test scripts/__tests__/storybook-visual-baseline.test.mjs` — pass after external-baseline rework - `pnpm exec tsc --noEmit --pretty false --module NodeNext --moduleResolution NodeNext --target ES2022 --types node,@playwright/test tests/storybook-visual/playwright.config.ts tests/storybook-visual/storybook-visual.spec.ts` — pass after external-baseline rework - `git diff --check origin/pr/9134..HEAD` — pass after external-baseline rework - `find tests/storybook-visual -type f -name '*.png' -print | wc -l` — `0` - `node scripts/storybook-visual-baseline.mjs verify` — intentionally fails closed until the first trusted maintainer publishes the baseline archive and updates `baseline-manifest.json` ## Risks - **Large but shallow:** the PR still touches many UI files due to mechanical token extraction and retune work, but committed PNG snapshot churn has been removed from the branch. - **Baseline publication required before the visual suite can pass in clean clones:** the manifest currently has placeholder archive metadata. A trusted maintainer must publish the first immutable archive, then update `baseline-manifest.json`. - **Rendering platform variance:** the external baseline should be captured in the documented Linux/Chromium environment. Future CI runs verify against the pinned archive and fail closed on checksum/count mismatch. - **Visual CI is opt-in while stabilizing:** add the `storybook-visual` label or dispatch the workflow manually to produce downloadable Playwright report/test-result artifacts. - **Scheduled follow-ups, deliberately out of scope:** Tailwind palette classes map to semantic tokens in a dedicated pass; card/pill component consolidation; ESLint ratchet. Tracked in `doc/design/DECISION-SHEET.md`. ## Model Used Claude Fable 5 (Anthropic, `claude-fable-5`, Mythos-class tier) with extended thinking, running in Claude Code with tool use; mechanical phases delegated to Claude Sonnet subagents. Follow-up external-baseline rework assisted by OpenAI Codex (`gpt-5` coding agent with repository, terminal, and GitHub tool use). All bulk rewrites executed via deterministic, idempotent scripts committed in `scripts/`; intentional visual changes were human-reviewed on screenshot contact sheets. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run targeted local verification and documented the intentional baseline-publication failure above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green *(pending new CI run after this rework)* - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups *(pending review)* - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) and OpenAI Codex --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Dotta <bippadotta@protonmail.com> Co-authored-by: Paperclip <noreply@paperclip.ing>
317 lines
11 KiB
TypeScript
317 lines
11 KiB
TypeScript
import { Check, ChevronsUpDown } from "lucide-react";
|
|
import { useMemo, useRef, useState, type ReactNode } from "react";
|
|
import { Button } from "@/components/ui/button";
|
|
import {
|
|
Command,
|
|
CommandEmpty,
|
|
CommandGroup,
|
|
CommandInput,
|
|
CommandItem,
|
|
CommandList,
|
|
} from "@/components/ui/command";
|
|
import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover";
|
|
import { fuzzyTextMatchesQuery, normalizeSearchText, scoreFuzzyTextFields } from "@/lib/searchable-select";
|
|
import { cn } from "@/lib/utils";
|
|
|
|
export interface SearchableSelectOption<TValue extends string = string> {
|
|
key: string;
|
|
value: TValue;
|
|
label: string;
|
|
title?: string;
|
|
searchText?: string;
|
|
disabled?: boolean;
|
|
}
|
|
|
|
export interface SearchableSelectGroup<TValue extends string = string, TOption extends SearchableSelectOption<TValue> = SearchableSelectOption<TValue>> {
|
|
id: string;
|
|
label?: string;
|
|
options: readonly TOption[];
|
|
}
|
|
|
|
interface SearchableSelectRenderState {
|
|
selected: boolean;
|
|
}
|
|
|
|
export interface SearchableSelectProps<
|
|
TValue extends string = string,
|
|
TOption extends SearchableSelectOption<TValue> = SearchableSelectOption<TValue>,
|
|
> {
|
|
value: TValue | "";
|
|
groups: readonly SearchableSelectGroup<TValue, TOption>[];
|
|
onValueChange: (value: TValue, option: TOption) => void | boolean | { close?: boolean };
|
|
placeholder: string;
|
|
searchPlaceholder?: string;
|
|
emptyMessage?: string;
|
|
loadingMessage?: string;
|
|
loading?: boolean;
|
|
disabled?: boolean;
|
|
className?: string;
|
|
triggerClassName?: string;
|
|
contentClassName?: string;
|
|
align?: "start" | "center" | "end";
|
|
contentWidth?: "trigger" | "auto";
|
|
renderValue?: (option: TOption | null) => ReactNode;
|
|
renderOption?: (option: TOption, state: SearchableSelectRenderState) => ReactNode;
|
|
deriveGroups?: (
|
|
query: string,
|
|
groups: readonly SearchableSelectGroup<TValue, TOption>[],
|
|
) => readonly SearchableSelectGroup<TValue, TOption>[];
|
|
filterOption?: (option: TOption, query: string) => boolean;
|
|
scoreOption?: (option: TOption, query: string) => number | null;
|
|
disablePortal?: boolean;
|
|
/**
|
|
* Optional pinned "creatable" item rendered at the bottom of the list,
|
|
* regardless of the query (used e.g. by the secret picker's
|
|
* `+ Create secret "<query>"…` affordance). `render` receives the live
|
|
* query so the label can echo it; `onSelect` fires with the query and the
|
|
* popover closes.
|
|
*/
|
|
createItem?: {
|
|
render: (query: string) => ReactNode;
|
|
onSelect: (query: string) => void;
|
|
};
|
|
}
|
|
|
|
function defaultFilterOption(option: SearchableSelectOption, query: string) {
|
|
return fuzzyTextMatchesQuery(`${option.label} ${option.searchText ?? ""}`, query);
|
|
}
|
|
|
|
function defaultScoreOption(option: SearchableSelectOption, query: string) {
|
|
return scoreFuzzyTextFields([
|
|
{ text: option.label, weight: 0 },
|
|
{ text: option.searchText, weight: 20 },
|
|
], query);
|
|
}
|
|
|
|
export function SearchableSelect<
|
|
TValue extends string = string,
|
|
TOption extends SearchableSelectOption<TValue> = SearchableSelectOption<TValue>,
|
|
>({
|
|
value,
|
|
groups,
|
|
onValueChange,
|
|
placeholder,
|
|
searchPlaceholder = "Search...",
|
|
emptyMessage = "No options found.",
|
|
loadingMessage = "Loading...",
|
|
loading = false,
|
|
disabled = false,
|
|
className,
|
|
triggerClassName,
|
|
contentClassName,
|
|
align = "start",
|
|
contentWidth = "trigger",
|
|
renderValue,
|
|
renderOption,
|
|
deriveGroups,
|
|
filterOption = defaultFilterOption,
|
|
scoreOption,
|
|
disablePortal,
|
|
createItem,
|
|
}: SearchableSelectProps<TValue, TOption>) {
|
|
const [open, setOpen] = useState(false);
|
|
const [query, setQuery] = useState("");
|
|
const pointerFocusRef = useRef(false);
|
|
const suppressNextTriggerFocusRef = useRef(false);
|
|
|
|
const selectedOption = useMemo(() => {
|
|
for (const group of groups) {
|
|
const option = group.options.find((candidate) => candidate.value === value);
|
|
if (option) return option;
|
|
}
|
|
return null;
|
|
}, [groups, value]);
|
|
|
|
const visibleGroups = useMemo(
|
|
() => (deriveGroups ? deriveGroups(query, groups) : groups),
|
|
[deriveGroups, groups, query],
|
|
);
|
|
|
|
const filteredGroups = useMemo(() => {
|
|
if (loading) return [];
|
|
const normalizedQuery = normalizeSearchText(query);
|
|
return visibleGroups
|
|
.map((group) => {
|
|
const options = group.options
|
|
.map((option, index) => {
|
|
if (!normalizedQuery) return { option, index, score: 0 };
|
|
|
|
if (!filterOption(option, query)) return null;
|
|
|
|
if (scoreOption) {
|
|
const score = scoreOption(option, query);
|
|
return score === null ? null : { option, index, score };
|
|
}
|
|
|
|
return {
|
|
option,
|
|
index,
|
|
score: defaultScoreOption(option, query) ?? Number.MAX_SAFE_INTEGER,
|
|
};
|
|
})
|
|
.filter((entry): entry is { option: TOption; index: number; score: number } => entry !== null);
|
|
|
|
if (normalizedQuery) {
|
|
options.sort((a, b) => a.score - b.score || a.index - b.index);
|
|
}
|
|
|
|
return {
|
|
...group,
|
|
options: options.map((entry) => entry.option),
|
|
};
|
|
})
|
|
.filter((group) => group.options.length > 0);
|
|
}, [filterOption, loading, query, scoreOption, visibleGroups]);
|
|
|
|
const hasOptions = filteredGroups.some((group) => group.options.length > 0);
|
|
|
|
function closePopover({ suppressTriggerFocus = false }: { suppressTriggerFocus?: boolean } = {}) {
|
|
if (suppressTriggerFocus) {
|
|
suppressNextTriggerFocusRef.current = true;
|
|
}
|
|
setOpen(false);
|
|
setQuery("");
|
|
}
|
|
|
|
function selectOption(option: TOption) {
|
|
if (option.disabled) return;
|
|
suppressNextTriggerFocusRef.current = true;
|
|
const result = onValueChange(option.value, option);
|
|
if (result === false || (typeof result === "object" && result?.close === false)) {
|
|
return;
|
|
}
|
|
closePopover();
|
|
}
|
|
|
|
return (
|
|
<Popover
|
|
open={open}
|
|
onOpenChange={(next) => {
|
|
setOpen(next);
|
|
if (!next) setQuery("");
|
|
}}
|
|
>
|
|
<PopoverTrigger asChild>
|
|
<Button
|
|
type="button"
|
|
variant="outline"
|
|
disabled={disabled}
|
|
onPointerDown={() => {
|
|
pointerFocusRef.current = true;
|
|
}}
|
|
onFocus={() => {
|
|
const shouldIgnoreFocus = pointerFocusRef.current || suppressNextTriggerFocusRef.current;
|
|
pointerFocusRef.current = false;
|
|
suppressNextTriggerFocusRef.current = false;
|
|
if (shouldIgnoreFocus) {
|
|
return;
|
|
}
|
|
setOpen(true);
|
|
}}
|
|
onKeyDown={(event) => {
|
|
if (event.key === "Escape" && open) {
|
|
event.preventDefault();
|
|
closePopover();
|
|
}
|
|
}}
|
|
aria-expanded={open}
|
|
role="combobox"
|
|
className={cn("w-full justify-between overflow-hidden", className, triggerClassName)}
|
|
>
|
|
<span className={cn("min-w-0 flex-1 truncate text-left", !selectedOption && "text-muted-foreground")}>
|
|
{renderValue ? renderValue(selectedOption) : selectedOption?.label ?? placeholder}
|
|
</span>
|
|
<ChevronsUpDown className="ml-2 size-4 opacity-50" />
|
|
</Button>
|
|
</PopoverTrigger>
|
|
<PopoverContent
|
|
align={align}
|
|
collisionPadding={16}
|
|
disablePortal={disablePortal}
|
|
className={cn(
|
|
"p-0",
|
|
contentWidth === "trigger"
|
|
? "w-(--radix-popover-trigger-width) min-w-56 max-w-(--sz-calc-23)"
|
|
: "w-72 max-w-(--sz-calc-23)",
|
|
contentClassName,
|
|
)}
|
|
onKeyDownCapture={(event) => {
|
|
if (event.key === "Escape") {
|
|
event.preventDefault();
|
|
event.stopPropagation();
|
|
closePopover({ suppressTriggerFocus: true });
|
|
}
|
|
}}
|
|
>
|
|
<Command shouldFilter={false}>
|
|
<CommandInput
|
|
value={query}
|
|
onValueChange={setQuery}
|
|
placeholder={searchPlaceholder}
|
|
/>
|
|
<CommandList
|
|
className="overscroll-contain touch-pan-y"
|
|
onWheelCapture={(event) => {
|
|
if (event.currentTarget.scrollHeight > event.currentTarget.clientHeight) {
|
|
event.stopPropagation();
|
|
}
|
|
}}
|
|
>
|
|
{loading ? (
|
|
<div className="px-3 py-6 text-center text-sm text-muted-foreground">{loadingMessage}</div>
|
|
) : (
|
|
<>
|
|
{!hasOptions && !createItem ? <CommandEmpty>{emptyMessage}</CommandEmpty> : null}
|
|
{!hasOptions && createItem ? (
|
|
<div className="px-3 py-3 text-center text-xs text-muted-foreground">{emptyMessage}</div>
|
|
) : null}
|
|
{filteredGroups.map((group) => (
|
|
<CommandGroup key={group.id} heading={group.label}>
|
|
{group.options.map((option) => {
|
|
const selected = option.value === value;
|
|
return (
|
|
<CommandItem
|
|
key={option.key}
|
|
value={option.key}
|
|
disabled={option.disabled}
|
|
onSelect={() => selectOption(option)}
|
|
>
|
|
{renderOption
|
|
? renderOption(option, { selected })
|
|
: <span className="min-w-0 truncate">{option.label}</span>}
|
|
<Check className={cn("ml-auto size-4", selected ? "opacity-100" : "opacity-0")} />
|
|
</CommandItem>
|
|
);
|
|
})}
|
|
</CommandGroup>
|
|
))}
|
|
{createItem ? (
|
|
<CommandGroup className="border-t border-border">
|
|
<CommandItem
|
|
value="__searchable_select_create__"
|
|
onSelect={() => {
|
|
const q = query;
|
|
suppressNextTriggerFocusRef.current = true;
|
|
closePopover();
|
|
// Defer to the next macrotask so this popover fully
|
|
// closes (and Radix returns focus to the trigger)
|
|
// before the consumer opens its own popover. Firing
|
|
// synchronously lets the close's focus-return land as a
|
|
// `focusOutside` on the newly-opened popover, which
|
|
// Radix would immediately dismiss (see PAP-12476).
|
|
window.setTimeout(() => createItem.onSelect(q), 0);
|
|
}}
|
|
>
|
|
{createItem.render(query)}
|
|
</CommandItem>
|
|
</CommandGroup>
|
|
) : null}
|
|
</>
|
|
)}
|
|
</CommandList>
|
|
</Command>
|
|
</PopoverContent>
|
|
</Popover>
|
|
);
|
|
}
|