mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Operators scan task state constantly, so the task **status** vocabulary (backlog / todo / in progress / in review / done / blocked / cancelled) has to read instantly > - Those statuses render through one shared component, `StatusGlyph`, whose icons were hand-rolled SVG geometry lifted from an internal spec > - Hand-rolled glyphs are harder to reason about, drift from the rest of the UI (which uses Lucide everywhere else), and mix fill/stroke styles across statuses > - This pull request swaps the hand-rolled geometry for named Lucide icons — one clean, consistent icon family — with no change to colours, sizing, or accessibility > - The benefit is a status icon set that is consistent with the rest of the app's iconography, trivially adjustable (change a mapping, not SVG path math), and simpler to maintain ## Linked Issues or Issue Description No existing public GitHub issue. Describing the change in-PR (feature/polish): **Problem / motivation.** The task status icons in `StatusGlyph` were bespoke inline SVGs (a half-filled disc for *in progress*, a filled disc + knockout check for *done*, ring+bar for *blocked*, ring+slash for *cancelled*, etc.). The rest of the UI uses [Lucide](https://lucide.dev) icons, so the status set was the odd one out — and its mixed fill/stroke shapes were harder to scan and to tweak. **Proposed solution.** Map each status to a Lucide icon and render that instead: | Status | Lucide icon | | --- | --- | | backlog | `circle-dashed` | | todo | `circle` | | in_progress | `rotate-cw` | | in_review | `circle-dot` | | done | `circle-check` | | blocked | `circle-minus` | | cancelled | `ban` | | in_queue (covered-blocked) | `circle-minus`, recoloured blue | Colours (the `--status-task-icon-*` tokens), the `sm/md/lg` size scale, `currentColor` recolouring, and the `role="img"` / `aria-label` behaviour are all unchanged — only the shapes change. **Alternatives considered.** Keeping the bespoke geometry (rejected: inconsistent with the app and harder to maintain). **Related PRs** (linked for reviewer context, not dependencies): - Refs #8580 — the merged PR that established the current hand-rolled status glyphs this PR restyles. - Refs #8838 — open PR forwarding Radix trigger props through `StatusGlyph`; touches the same component (no overlap with this change). - Refs #1760 — open proposal to redesign the *cancelled* status icon specifically; this PR moves cancelled to Lucide `ban`. ## What Changed - `ui/src/components/StatusGlyph.tsx`: replaced the per-status hand-rolled SVG `glyphBody()` geometry with a `status → Lucide icon` map (`circle-dashed`, `circle`, `rotate-cw`, `circle-dot`, `circle-check`, `circle-minus`, `ban`). Kept the token-driven colour wiring, size scale, `currentColor` recolouring, a11y label handling, and the `in_queue` = blocked-icon-recoloured-blue behaviour. - `ui/src/components/StatusGlyph.test.tsx`: updated to lock the new icon mapping (per-status Lucide class, size scale, colour var, `in_queue`, a11y) instead of the old geometry. Net: two files, +74 / −138 (the component got smaller). Because every status surface (list, board, detail header, status picker, sub-task/blocked-by pills, chips) routes through `StatusGlyph`, this single-component edit covers them all. ## Verification - `pnpm check:token-gates` → **3/3 clean** (no hardcoded colour/spacing/font values introduced). - `pnpm typecheck` → clean across all packages. - `cd ui && pnpm vitest run` → **2509/2509 passing**, including the updated `StatusGlyph` test. - Manual: ran the worktree dev server and confirmed the new icons render everywhere (task list, task detail, related-task chips, and the status picker showing all seven). **Storybook visual-regression note:** this is an intentional visual change, so the status-icon stories will diff against the published baseline. The baseline snapshots need to be regenerated and republished by a maintainer (`pnpm test:storybook-visual:update` from a trusted environment) as part of accepting this change — the visual-regression CI check is expected to be red until then. No baseline is published in the environment this PR was authored in, so that step is left to a maintainer. ## Risks - **Low risk / cosmetic.** No logic, data, or API changes — only the rendered icon shapes. Colours, sizes, and accessibility labels are unchanged. - The most noticeable shifts are *in progress* (half-disc → rotating arrow), *done* (solid disc+check → outline circle+check), and *cancelled* (ring+slash → ban). These are deliberate. - The only CI check expected to fail is the Storybook visual-regression job, pending a maintainer baseline update (see Verification). ## Model Used Claude Opus 4.8 (`claude-opus-4-8`), run in Claude Code with extended thinking and tool use (file edits, local test runs, browser-driven visual verification). ## 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 tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
91 lines
3.9 KiB
TypeScript
91 lines
3.9 KiB
TypeScript
// @vitest-environment node
|
|
|
|
import { renderToStaticMarkup } from "react-dom/server";
|
|
import { describe, expect, it } from "vitest";
|
|
import { StatusGlyph } from "./StatusGlyph";
|
|
import { taskStatusIconVar } from "../lib/status-colors";
|
|
|
|
/**
|
|
* The unified status glyph renders every status from ONE Lucide icon per status
|
|
* (all on a shared `viewBox="0 0 24 24"`, at a `sm 14 / md 16 / lg 20` scale),
|
|
* coloured from the AA-tuned `--status-task-icon-*` vars. These tests lock the
|
|
* icon mapping, the size scale, the colour wiring and `in_queue`.
|
|
*/
|
|
|
|
/** Status → the Lucide class token its icon renders (`lucide-<kebab-name>`). */
|
|
const STATUS_ICON_CLASS: Record<string, string> = {
|
|
backlog: "lucide-circle-dashed",
|
|
todo: "lucide-circle",
|
|
in_progress: "lucide-rotate-cw",
|
|
in_review: "lucide-circle-dot",
|
|
done: "lucide-circle-check",
|
|
blocked: "lucide-circle-minus",
|
|
cancelled: "lucide-ban",
|
|
in_queue: "lucide-circle-minus",
|
|
};
|
|
|
|
describe("StatusGlyph", () => {
|
|
it("renders one 24-unit viewBox for every status (proportional scaling)", () => {
|
|
for (const status of Object.keys(taskStatusIconVar)) {
|
|
const html = renderToStaticMarkup(<StatusGlyph status={status} />);
|
|
expect(html).toContain('viewBox="0 0 24 24"');
|
|
expect(html).toContain("<svg");
|
|
}
|
|
});
|
|
|
|
it("maps sm/md/lg to 14/16/20 px", () => {
|
|
expect(renderToStaticMarkup(<StatusGlyph status="todo" size="sm" />)).toContain('width="14"');
|
|
expect(renderToStaticMarkup(<StatusGlyph status="todo" size="md" />)).toContain('width="16"');
|
|
expect(renderToStaticMarkup(<StatusGlyph status="todo" size="lg" />)).toContain('width="20"');
|
|
// Default size is md.
|
|
expect(renderToStaticMarkup(<StatusGlyph status="todo" />)).toContain('width="16"');
|
|
});
|
|
|
|
it("colours each status from its --status-task-icon-* var", () => {
|
|
for (const [status, cssVar] of Object.entries(taskStatusIconVar)) {
|
|
const html = renderToStaticMarkup(<StatusGlyph status={status} />);
|
|
expect(html).toContain(`var(${cssVar})`);
|
|
}
|
|
});
|
|
|
|
it("falls back to the backlog icon + var for unknown statuses", () => {
|
|
const html = renderToStaticMarkup(<StatusGlyph status="mystery" />);
|
|
expect(html).toContain("var(--status-task-icon-backlog)");
|
|
expect(html).toContain("lucide-circle-dashed");
|
|
});
|
|
|
|
it("maps each status to its Lucide icon", () => {
|
|
for (const [status, iconClass] of Object.entries(STATUS_ICON_CLASS)) {
|
|
const html = renderToStaticMarkup(<StatusGlyph status={status} />);
|
|
expect(html).toContain(iconClass);
|
|
}
|
|
});
|
|
|
|
it("gives todo the plain circle (not a compound circle icon)", () => {
|
|
const html = renderToStaticMarkup(<StatusGlyph status="todo" />);
|
|
expect(html).toContain("lucide-circle");
|
|
for (const compound of ["circle-dashed", "circle-dot", "circle-check", "circle-minus"]) {
|
|
expect(html).not.toContain(compound);
|
|
}
|
|
});
|
|
|
|
it("renders in_queue as the blocked icon recoloured blue (in_queue var)", () => {
|
|
const queue = renderToStaticMarkup(<StatusGlyph status="in_queue" />);
|
|
const blocked = renderToStaticMarkup(<StatusGlyph status="blocked" />);
|
|
// Same icon as blocked (circle-minus)…
|
|
expect(queue).toContain("lucide-circle-minus");
|
|
// …but coloured from the in_queue (blue) icon var, not blocked's red.
|
|
expect(queue).toContain("var(--status-task-icon-in_queue)");
|
|
expect(queue).not.toContain("var(--status-task-icon-blocked)");
|
|
expect(blocked).toContain("var(--status-task-icon-blocked)");
|
|
});
|
|
|
|
it("is decorative by default and labelled when given a title", () => {
|
|
expect(renderToStaticMarkup(<StatusGlyph status="todo" />)).toContain('aria-hidden="true"');
|
|
const labelled = renderToStaticMarkup(<StatusGlyph status="todo" title="Todo" />);
|
|
expect(labelled).toContain('role="img"');
|
|
expect(labelled).toContain('aria-label="Todo"');
|
|
expect(labelled).toContain("<title>Todo</title>");
|
|
});
|
|
});
|