diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md index b27122f..b9eb064 100644 --- a/THIRD_PARTY_NOTICES.md +++ b/THIRD_PARTY_NOTICES.md @@ -17,6 +17,10 @@ - Lucide, Lucide contributors, ISC: https://github.com/lucide-icons/lucide - React Markdown and remark-gfm, unified contributors, MIT: https://github.com/remarkjs/react-markdown and https://github.com/remarkjs/remark-gfm +- Highlighters (`@highlighters/core`), Jace Attard, MIT: + https://github.com/JaceThings/highlighters. The pencil-case dock in + `ui/src/review` is our own; its interaction language is modelled on + highlighte.rs. - DM Sans and IBM Plex Mono font packages, SIL Open Font License 1.1: https://fontsource.org/fonts/dm-sans and https://fontsource.org/fonts/ibm-plex-mono diff --git a/docs/design/review-tools.md b/docs/design/review-tools.md new file mode 100644 index 0000000..efe0917 --- /dev/null +++ b/docs/design/review-tools.md @@ -0,0 +1,71 @@ +# Review tools: the pencil case + +## What we learned from highlighte.rs + +[highlighte.rs](https://highlighte.rs) is a library for realistic highlighter +marks, but the thing worth taking is how its site feels to use. The tools are +physical: pens stand in a tray with their tips showing, rise when you hover, +lift right out when you pick one, and a soft outline travels between them on +a spring. Ink colour crossfades. Marks are drawn on, not switched on. There is +a squeak when you pick a marker. Everything shares one quint curve +(`cubic-bezier(0.2, 0, 0, 1)`), text sits on cream paper in warm brown ink, +and nothing is decorated that does not also do something. + +Their answer to "what do you build with this?" is a highlighter. Ours is code +review, because that is what maintainers do all day in AXP. + +## What was built + +A pencil case: a capsule tray that floats at the bottom of a contribution +page for anyone who can post (`ui/src/review`). Three tools, four inks, four +stamps, an eraser, and an opt-in sound toggle. + +- **Highlighter.** Drag over any prose (an agent turn, a maintainer prompt, a + comment) and a real highlighter mark is laid down with `@highlighters/core` + (MIT): chisel tip, word snapping, a short draw-on animation. A chip appears + under the mark: _Quote in discussion_. It quotes the passage into the + composer with an attribution and switches to the discussion. Quoted comments + render with the same mark over the quote, so the trail is visible. +- **Highlighter and Note on the diff.** With either held, Pierre's line + selection is enabled; dragging down the gutter selects lines (tinted with + the current ink), and a sticky note appears under the last line with the + reference (`src/parser.ts:L2–L4`). Pin it and it posts as a comment anchored + to the checkpoint and file, with the reference in code. +- **Stamp.** Pick LGTM, Needs work, Question or Nice, click anywhere, and the + stamp lands with a spring and a rubber-stamp ink texture. The verdict is + posted to the discussion as `**LGTM** — on bbbbbbb`, which the discussion + renders as a stamp again. Landed stamps are local decoration; the comment + is the record. +- **Paper.** The discussion sits on ruled paper (24px rhythm, 6% ink). + +No protocol changes. Every action is an ordinary `_axp/comment`, which is +why the tools work for contributors and verifiers as well as maintainers, and +why nothing needs a migration. + +## The interaction language, as rules + +These are the rules the pencil case follows, written down so the rest of the +app can follow them too: + +1. Tools are objects. They have a resting place, they rise when you reach for + them, and they lift when you pick them up. Selected ≠ highlighted. +2. One curve. `cubic-bezier(0.2, 0, 0, 1)` for state changes; a `linear()` + spring only for things that arrive (the tray, a stamp, a note). +3. Ink is a colour property, so changing it crossfades rather than re-renders. +4. Marks are drawn, over the text, never in it: selection, copy and find + keep working. +5. Sound is opt-in, synthesised, and remembered per browser. +6. Keyboard first: the tray is a toolbar, tools are toggle buttons, inks and + stamps are radio groups, Escape puts the tool down. +7. Decoration never replaces the record. A stamp is a comment. + +## Not done yet + +- A vendored handwriting face for notes and stamps; the note uses a cursive + system stack for now (see `typography.md` for candidates). +- Marks and landed stamps do not persist across reloads. If they should, the + comment already carries enough to redraw them (file, lines, quote). +- Stamps could drive review flow: LGTM opening the approval dialog for a + maintainer who can sign is the obvious next step. +- Underline and strike-through pens (highlighters supports both) for + suggesting deletions in prose. diff --git a/package-lock.json b/package-lock.json index fb6270b..28e6de7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,6 +24,7 @@ "@eslint/js": "^10.0.0", "@fontsource-variable/dm-sans": "5.3.0", "@fontsource/ibm-plex-mono": "5.3.0", + "@highlighters/core": "^2.0.0", "@pierre/diffs": "1.4.1", "@pierre/trees": "1.0.0-beta.6", "@playwright/test": "1.63.0", @@ -696,6 +697,16 @@ "url": "https://github.com/sponsors/ayuhito" } }, + "node_modules/@highlighters/core": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@highlighters/core/-/core-2.0.0.tgz", + "integrity": "sha512-OxuorhZZeqDIExovmOBdYLJLLqHsDXTU/YzsO42BPbNTSXqA579dWjoZHJI4I7Wg9jOt7EVLp60aIrNTQTyiqQ==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + } + }, "node_modules/@humanfs/core": { "version": "0.19.2", "resolved": "https://registry.npmjs.org/@humanfs/core/-/core-0.19.2.tgz", diff --git a/package.json b/package.json index 880335d..c84e412 100644 --- a/package.json +++ b/package.json @@ -74,6 +74,7 @@ "@eslint/js": "^10.0.0", "@fontsource-variable/dm-sans": "5.3.0", "@fontsource/ibm-plex-mono": "5.3.0", + "@highlighters/core": "^2.0.0", "@pierre/diffs": "1.4.1", "@pierre/trees": "1.0.0-beta.6", "@playwright/test": "1.63.0", diff --git a/scripts/ui-notices.mjs b/scripts/ui-notices.mjs index cef32e4..bc44477 100644 --- a/scripts/ui-notices.mjs +++ b/scripts/ui-notices.mjs @@ -12,6 +12,7 @@ const roots = [ "remark-gfm", "@fontsource-variable/dm-sans", "@fontsource/ibm-plex-mono", + "@highlighters/core", ]; const packages = new Map(); async function visit(name, parent = process.cwd()) { diff --git a/ui/src/Contribution.tsx b/ui/src/Contribution.tsx index dec4b90..45515b3 100644 --- a/ui/src/Contribution.tsx +++ b/ui/src/Contribution.tsx @@ -38,6 +38,16 @@ import { import { TabGroup } from "./vendor/huabu/TabGroup.js"; import { useCommand, useDraft } from "./api.js"; import { Transcript } from "./Transcript.js"; +import { + PencilCase, + StampLayer, + stampBody, + stampOf, + useReviewTools, +} from "./review/PencilCase.js"; +import type { Landed } from "./review/PencilCase.js"; +import { thunk } from "./review/sound.js"; +import { QuotedProse } from "./review/QuotedProse.js"; const DiffPanel = lazy(() => import("./DiffPanel.js")); @@ -137,6 +147,77 @@ export function ContributionPage({ const writable = workspace.principal.role !== "observer"; const disabled = offline || command.busy; const checkpoint = exchange.checkpoint; + const review = useReviewTools(); + const [quote, setQuote] = useState<{ + text: string; + author: string; + x: number; + y: number; + } | null>(null); + const [landed, setLanded] = useState([]); + useEffect(() => setQuote(null), [tab, review.tool]); + /** Highlighter: a drag over prose paints a mark and offers to quote it. */ + const paintSelection = (panel: HTMLElement) => { + if (review.tool !== "highlighter") return; + const selection = document.getSelection(); + if (!selection || selection.isCollapsed || !selection.rangeCount) return; + const range = selection.getRangeAt(0); + const node = range.commonAncestorContainer; + const element = node instanceof Element ? node : node.parentElement; + const prose = element?.closest(".prose"); + if (!prose || !panel.contains(prose)) return; + const text = selection.toString().trim(); + if (!text) return; + const author = prose.closest(".turn-response") + ? "the agent" + : prose.closest(".turn-prompt") + ? "the maintainer" + : (prose.closest(".comment")?.querySelector("strong")?.textContent ?? + "the discussion"); + const rect = range.getBoundingClientRect(); + const box = panel.getBoundingClientRect(); + review.mark(range); + selection.removeAllRanges(); + setQuote({ + text, + author, + x: rect.left - box.left + rect.width / 2, + y: rect.bottom - box.top + 8, + }); + }; + /** Stamp: lands where you click and posts the verdict. */ + const dropStamp = (event: React.MouseEvent) => { + if (review.tool !== "stamp" || disabled) return; + if ( + (event.target as HTMLElement).closest( + "button, a, input, textarea, select", + ) + ) + return; + const box = event.currentTarget.getBoundingClientRect(); + setLanded((all) => [ + ...all, + { + id: crypto.randomUUID(), + stamp: review.stamp, + where: tab, + x: event.clientX - box.left, + y: event.clientY - box.top, + rot: -12 + Math.random() * 10, + }, + ]); + thunk(); + const onChanges = tab === "changes" && checkpoint; + void command.send(contribution.id, { + kind: "comment", + body: stampBody( + review.stamp, + onChanges ? `on ${checkpoint.headCommit.slice(0, 7)}` : undefined, + ), + checkpoint: onChanges ? checkpoint.headCommit : null, + path: null, + }); + }; const reviewKey = `${checkpoint?.headCommit ?? ""}:${exchange.review?.contributor.signature ?? ""}`; const manifest = loadedReview?.key === reviewKey ? loadedReview.digest : null; const ready = useCallback( @@ -149,7 +230,12 @@ export function ContributionPage({ setTab("discussion"); }; return ( -
+
+ {writable && !offline && exchange.status !== "closed" && ( + + )} @@ -199,7 +285,36 @@ export function ContributionPage({ ? "Changes" : "Discussion" } + onPointerUp={(event) => paintSelection(event.currentTarget)} + onClick={dropStamp} > + stamp.where === tab)} + /> + {quote && ( + + )} {tab === "conversation" && ( <> { + void command.send(contribution.id, { + kind: "comment", + body: `${text}\n\n\`${reference}\``, + checkpoint: checkpoint.headCommit, + path, + }); + }, + }} /> ) : ( @@ -335,7 +462,7 @@ export function ContributionPage({ ))} {tab === "discussion" && ( -
+
@@ -369,7 +496,31 @@ export function ContributionPage({ )}
)} - + {(() => { + const stamp = stampOf(item.body); + if (stamp) { + const rest = item.body + .trim() + .slice(stamp.label.length + 4) + .replace(/^\s*—\s*/, ""); + return ( + <> + + {stamp.label} + + {rest &&

{rest}

} + + ); + } + return item.body.trimStart().startsWith("> ") ? ( + + ) : ( + + ); + })()}
))} diff --git a/ui/src/DiffPanel.tsx b/ui/src/DiffPanel.tsx index b94eb66..6be769e 100644 --- a/ui/src/DiffPanel.tsx +++ b/ui/src/DiffPanel.tsx @@ -9,6 +9,16 @@ import { FileTree, useFileTree } from "@pierre/trees/react"; import { Columns2, Rows3, MessageCircle, FileCode2 } from "lucide-react"; import { api } from "./api.js"; import { Empty, Loading } from "./components.js"; +import { StickyNote } from "./review/PencilCase.js"; +import type { Tool } from "./review/Pen.js"; + +export interface DiffReview { + tool: Tool | null; + /** current highlighter ink, used to tint selected lines */ + ink: string; + /** Post a note pinned to `reference` ("path:L3–L7"). */ + onNote: (text: string, path: string, reference: string) => void; +} // Keep comments readable on diff addition/removal backgrounds as well as white. registerCustomTheme("axp-light", async () => { @@ -33,11 +43,13 @@ export default function DiffPanel({ checkpoint, discuss, ready, + review, }: { session: string; checkpoint: string; discuss: (path: string) => void; ready: (manifestDigest: string | null) => void; + review?: DiffReview; }) { const [patch, setPatch] = useState(); const [error, setError] = useState(); @@ -71,16 +83,24 @@ export default function DiffPanel({
); if (patch === undefined) return Loading checkpoint…; - return ; + return ; } function Patch({ patch, discuss, + review, }: { patch: string; discuss: (path: string) => void; + review?: DiffReview; }) { + // Lines the reviewer has marked with the highlighter or note tool; a sticky + // note renders under the last one until it is pinned or discarded. + const [pending, setPending] = useState<{ start: number; end: number } | null>( + null, + ); + const selecting = review?.tool === "highlighter" || review?.tool === "note"; const parsed = useMemo(() => { try { return { @@ -118,6 +138,7 @@ function Patch({ }); const file = parsed.files.find((file) => file.name === selected) ?? parsed.files[0]; + useEffect(() => setPending(null), [selected, review?.tool]); if (parsed.error) return (
@@ -172,14 +193,54 @@ function Patch({
{ + const range = annotation.metadata as { start: number; end: number }; + const reference = + range.start === range.end + ? `${file.name}:L${range.start}` + : `${file.name}:L${range.start}–L${range.end}`; + return ( + setPending(null)} + onPost={(text) => { + review?.onNote(text, file.name, reference); + setPending(null); + }} + /> + ); + }} options={{ theme: "axp-light", themeType: "light", diffStyle: split ? "split" : "unified", overflow: "scroll", disableFileHeader: true, - unsafeCSS: - ":host { --diffs-font-family: 'IBM Plex Mono', monospace; --diffs-font-size: 12px; --diffs-line-height: 22px; }", + enableLineSelection: selecting, + controlledSelection: true, + onLineSelectionEnd: (range) => { + if (!selecting || !range) return; + const [start, end] = + range.start <= range.end + ? [range.start, range.end] + : [range.end, range.start]; + setPending({ start, end }); + }, + unsafeCSS: `:host { --diffs-font-family: 'IBM Plex Mono', monospace; --diffs-font-size: 12px; --diffs-line-height: 22px; --diffs-bg-selection-override: color-mix(in srgb, ${review?.ink ?? "#f3d43a"} 42%, transparent); --diffs-bg-selection-number-override: color-mix(in srgb, ${review?.ink ?? "#f3d43a"} 70%, transparent); }`, }} />
diff --git a/ui/src/review/Pen.tsx b/ui/src/review/Pen.tsx new file mode 100644 index 0000000..5bb4483 --- /dev/null +++ b/ui/src/review/Pen.tsx @@ -0,0 +1,112 @@ +/* The tools in the pencil case, drawn as simple SVG. Each sits in a 40×120 + * frame with its tip at the top so the tray can hide the lower body and let + * the tool rise when hovered or selected. Colours come in as CSS variables so + * ink changes crossfade without re-rendering. */ + +export type Tool = "highlighter" | "stamp" | "note"; + +export function Highlighter({ ink }: { ink: string }) { + return ( + + ); +} + +export function Stamp({ ink }: { ink: string }) { + return ( + + ); +} + +export function NotePen({ ink }: { ink: string }) { + return ( + + ); +} + +/** Shared gradient so every tool has the same soft cylinder shading. */ +export function PenDefs() { + return ( + + ); +} diff --git a/ui/src/review/PencilCase.tsx b/ui/src/review/PencilCase.tsx new file mode 100644 index 0000000..b3db2bf --- /dev/null +++ b/ui/src/review/PencilCase.tsx @@ -0,0 +1,328 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import type { ReactNode } from "react"; +import { Eraser, Volume2, VolumeX } from "lucide-react"; +import { highlight } from "@highlighters/core"; +import type { MarkHandle } from "@highlighters/core"; +import { Highlighter, NotePen, PenDefs, Stamp } from "./Pen.js"; +import type { Tool } from "./Pen.js"; +import { setSoundEnabled, soundEnabled, squeak, thunk } from "./sound.js"; +import "./pencil-case.css"; + +/* Review tools for a contribution, modelled on a real pencil case. + * + * Highlighter drag over prose to lay down a real highlighter mark and get + * a "Quote in discussion" chip; on the diff, select lines to + * pin a sticky note to them + * Stamp pick a stamp, click anywhere: it lands with a thunk and the + * verdict is posted to the discussion + * Note click a diff line for a sticky note + * + * Marks and stamps are local decoration; the discussion comment is the + * record. Nothing here changes the protocol. */ + +export const SWATCHES = [ + { id: "yellow", label: "Yellow", ink: "#f3d43a" }, + { id: "green", label: "Green", ink: "#8fd67a" }, + { id: "pink", label: "Pink", ink: "#f4a0c6" }, + { id: "blue", label: "Blue", ink: "#8cc5f2" }, +] as const; +export type Swatch = (typeof SWATCHES)[number]["id"]; + +export const STAMPS = [ + { id: "lgtm", label: "LGTM", ink: "#2f7d4a" }, + { id: "needs-work", label: "Needs work", ink: "#b23a2f" }, + { id: "question", label: "Question", ink: "#8b5e1a" }, + { id: "nice", label: "Nice", ink: "#4a5f9e" }, +] as const; +export type StampId = (typeof STAMPS)[number]["id"]; + +export interface ReviewState { + tool: Tool | null; + swatch: Swatch; + ink: string; + stamp: StampId; + setTool: (tool: Tool | null) => void; + setSwatch: (swatch: Swatch) => void; + setStamp: (stamp: StampId) => void; + /** Paint a highlighter mark over a range; returns its handle. */ + mark: (range: Range) => MarkHandle | null; + clearMarks: () => void; + marks: number; +} + +export function useReviewTools(): ReviewState { + const [tool, setToolState] = useState(null); + const [swatch, setSwatch] = useState("yellow"); + const [stamp, setStamp] = useState("lgtm"); + const handles = useRef([]); + const [marks, setMarks] = useState(0); + const setTool = useCallback((next: Tool | null) => { + setToolState((current) => { + const value = current === next ? null : next; + if (value) squeak(value === "stamp" ? 0.7 : 1); + return value; + }); + }, []); + const mark = useCallback( + (range: Range) => { + try { + const handle = highlight(range, { + color: { palette: "mild", swatch }, + tip: { type: "chisel" }, + snap: "word", + animation: { draw: true, duration: 420 }, + }); + handles.current.push(handle); + setMarks(handles.current.length); + squeak(1.3); + return handle; + } catch { + return null; + } + }, + [swatch], + ); + const clearMarks = useCallback(() => { + for (const handle of handles.current) handle.remove(); + handles.current = []; + setMarks(0); + }, []); + useEffect(() => () => clearMarks(), [clearMarks]); + useEffect(() => { + if (!tool) return; + const escape = (event: KeyboardEvent) => { + if (event.key === "Escape") setToolState(null); + }; + window.addEventListener("keydown", escape); + return () => window.removeEventListener("keydown", escape); + }, [tool]); + const ink = SWATCHES.find((s) => s.id === swatch)!.ink; + return { + tool, + swatch, + ink, + stamp, + setTool, + setSwatch, + setStamp, + mark, + clearMarks, + marks, + }; +} + +const TOOLS: { id: Tool; label: string; art: (ink: string) => ReactNode }[] = [ + { + id: "highlighter", + label: "Highlighter", + art: (ink) => , + }, + { id: "stamp", label: "Stamp", art: (ink) => }, + { id: "note", label: "Note", art: (ink) => }, +]; + +export function PencilCase({ review }: { review: ReviewState }) { + const [sound, setSound] = useState(soundEnabled); + const [focused, setFocused] = useState(null); + const ink = review.ink; + const stampInk = STAMPS.find((s) => s.id === review.stamp)!.ink; + const outlineIndex = focused ?? TOOLS.findIndex((t) => t.id === review.tool); + return ( +
+ +
+ {TOOLS.map((tool, index) => ( + + ))} +
+ + {review.tool === "stamp" ? ( +
+ {STAMPS.map((stamp) => ( + + ))} +
+ ) : ( +
+ {SWATCHES.map((swatch) => ( +
+ )} + +
+ + +
+
+ ); +} + +/** A stamp that has landed on the page. */ +export interface Landed { + id: string; + stamp: StampId; + /** which tab panel it landed on */ + where: string; + x: number; + y: number; + rot: number; +} +export function StampLayer({ stamps }: { stamps: Landed[] }) { + return ( + + ); +} + +/** Stamps are recorded as comments beginning with the verdict in bold. */ +export function stampBody(stamp: StampId, where?: string): string { + const def = STAMPS.find((s) => s.id === stamp)!; + return `**${def.label}**${where ? ` — ${where}` : ""}`; +} +export function stampOf(body: string): (typeof STAMPS)[number] | null { + const match = /^\*\*([^*]+)\*\*/.exec(body.trim()); + if (!match) return null; + return STAMPS.find((s) => s.label === match[1]) ?? null; +} + +/** Sticky note pinned to diff lines. */ +export function StickyNote({ + reference, + onPost, + onCancel, +}: { + reference: string; + onPost: (text: string) => void; + onCancel: () => void; +}) { + const [text, setText] = useState(""); + const area = useRef(null); + useEffect(() => { + area.current?.focus(); + }, []); + return ( +
+
{reference}
+