diff --git a/.claude/skills/feature-walkthrough/SKILL.md b/.claude/skills/feature-walkthrough/SKILL.md new file mode 100644 index 0000000000..f2a4bfdc8b --- /dev/null +++ b/.claude/skills/feature-walkthrough/SKILL.md @@ -0,0 +1,97 @@ +--- +name: feature-walkthrough +description: >- + Explain the full logic and process of the current branch end-to-end so someone + with no prior knowledge of the task can understand, review, and reproduce it. + Scopes the change from the branch diff, traces the flow across every layer it + touches (frontend tool/hook/component, Java controller/service/endpoint, Python + engine, config, i18n, tests), and produces a self-contained walkthrough document + with Mermaid diagrams (sequence/flow/architecture), annotated file map with + clickable references, before/after behavior, screenshots where a UI is involved, + a "try it locally" section, and edge cases/risks. Use when asked for a feature or + branch walkthrough, "explain what this branch does", a design/logic writeup, PR + reviewer onboarding, or a hand-off doc. Pass --html to also emit a rendered HTML + version; --no-screens to skip screenshots. +argument-hint: "[branch-or-area] [--html] [--no-screens]" +allowed-tools: Read, Write, Edit, Glob, Grep, Bash +--- + +# Feature / Branch Walkthrough + +Turn the current branch into a walkthrough a newcomer can follow. Audience: +**someone who has never seen this task**. Explain the *why*, the *flow*, and *how to +try it* - not just a diff summary. + +`$ARGUMENTS` may name a branch or area to focus on; default is the current branch +vs `main`. Flags: `--html` (also emit a rendered HTML twin), `--no-screens`. + +## Process + +### 1. Scope the change +- `git log --oneline main..HEAD` and `git diff --stat main...HEAD` for the shape. +- Read the PR description / commit messages for stated intent. Do **not** invent + history or motivation that isn't evidenced (state current behavior in present tense). +- Classify touched files by layer: + - **Frontend**: tools (`frontend/editor/src/core/components/tools/*` or `.../core/tools/*`), + hooks (`core/hooks/tools/*`, `useToolOperation`), contexts, routes, i18n + (`public/locales/en-US`). + - **Java backend**: controllers (`.../controller/api/...`), services, models, config. + - **Engine**: `engine/src/stirling/{agents,contracts,api,services}`. + - **Config / build / docker / tests.** + +### 2. Trace the flow end-to-end +Follow one real path from user action to result. For a typical PDF tool that's: +UI control → `useToolOperation` hook → `POST /api/v1/...` → Spring controller → +service (PDFBox / LibreOffice / engine call) → response → review panel → download. +Read the actual files so the narrative is true to the code, and collect the exact +file:line anchors you'll cite. + +### 3. Draw the diagrams (Mermaid) +Pick what fits; usually 2-3 of: +- **Sequence diagram** - request/response across frontend → backend → engine. +- **Flowchart** - the core decision/branching logic of the feature. +- **Architecture/component** - new pieces and how they wire to existing ones. +- **State** - if the feature has modes/steps. +Keep nodes labeled in plain language. Validate the Mermaid parses before shipping. + +### 4. Screenshots (unless --no-screens) +If a UI is involved, capture key states with the stubbed Playwright harness +(see the **ui-walkthrough** skill and `files-page-screenshots.spec.ts` for the +pattern) or, for before/after, capture `main` then the branch. Drop PNGs in +`walkthrough//` and reference them from the doc. For backend-only +changes, show request/response examples (curl + JSON) instead. + +### 5. Write the walkthrough +Create `walkthrough//FEATURE-WALKTHROUGH.md` with: +1. **TL;DR** - what the branch does and who it's for, in 3-4 sentences. +2. **Problem & approach** - what wasn't possible before; the chosen solution. +3. **Architecture diagram** + 1-paragraph orientation. +4. **End-to-end flow** - the sequence diagram + a numbered walk of each step, + each citing the real file (clickable `path:line`). +5. **Key files** - annotated map (path → one line on its role). +6. **Logic deep-dive** - the flowchart + prose for the non-obvious decisions. +7. **Behavior** - before vs after; screenshots or request/response examples. +8. **Try it locally** - exact steps (`task dev` / `task dev:all`, the route to + open or the curl to run, any env like `DOCKER_ENABLE_SECURITY` or a test + license key). Make it copy-pasteable. +9. **Edge cases, risks, follow-ups** - what's untested, known limits, gotchas. + +Markdown is the primary deliverable - it renders with diagrams in GitHub PRs and +IDEs, no build step, ideal for review. + +### 6. If `--html` +Also emit `walkthrough//walkthrough.html`: the same content with Mermaid +rendered via `mermaid.initialize({startOnLoad:true})` (script from CDN; note in +the file that rendering diagrams needs network, the `.md` is the offline copy) and +screenshots inline. Keep it self-contained otherwise. + +### 7. Deliver +Give the doc path and a short chat summary. Offer to `SendUserFile` it. + +## Principles +- **True to the code.** Every claim traces to a file you read; cite `path:line`. + No fabricated migration/version history. +- **Newcomer-first.** Define repo-specific terms (FileContext, `useToolOperation`, + the `@app/*` layer cascade, stubbed vs live tests) on first use. +- **Show, don't assert.** Prefer a diagram + a real example over adjectives. +- Don't commit the `walkthrough/` output unless asked. diff --git a/.claude/skills/ui-before-after/SKILL.md b/.claude/skills/ui-before-after/SKILL.md new file mode 100644 index 0000000000..07a8561baa --- /dev/null +++ b/.claude/skills/ui-before-after/SKILL.md @@ -0,0 +1,122 @@ +--- +name: ui-before-after +description: >- + Analyse a branch or PR and automatically capture before/after screenshots of + every UI surface its changes touch, then pixel-diff the pairs to surface what + actually changed and assemble PR-ready before/after montage images. Generic and + diff-driven: it derives the capture targets from the diff (changed tools/routes → + URLs) instead of hand-listing screens, captures "before" from the base branch and + "after" from the head, then keeps only the views that visually differ. Each + comparison is auto-cropped to the region that actually changed (the bounding box of + differing pixels), falling back to the full page only when the change spans most of + it. Use for before/after shots, a visual diff of a branch/PR, "screenshots for the + PR description", "show what changed in the UI", or a side-by-side of UI changes. + Takes a PR number/URL (resolved via gh) or a branch; defaults to the current branch + vs its base. Flags: --scope , --base , --theme + light|dark|both, --all (capture every route, not just changed), --no-autocrop, + --pagewide , --threshold . +argument-hint: "[PR# | PR-url | branch] [--scope ] [--base ] [--theme both] [--all] [--no-autocrop]" +allowed-tools: Read, Write, Edit, Glob, Grep, Bash +--- + +# UI Before / After (generic visual diff) + +Point it at a branch or PR; it figures out which UI changed, screenshots every +affected surface **before** (base) and **after** (head), pixel-diffs the pairs, and +montages the ones that actually changed into images for the PR description. + +`$ARGUMENTS`: a PR number/URL, a branch, or nothing (current branch vs base). +By default it captures the full viewport and auto-crops each comparison to the region +that changed. Flags: `--scope ` (narrow the *capture* to a container, e.g. +`[data-sidebar="tool-panel"]`, when you already know where the change is), +`--no-autocrop` (keep full frames), `--pagewide ` (above this share of the +page, skip cropping; default 0.6), `--base `, +`--theme light|dark|both`, `--all` (walk every route, not just changed), +`--threshold ` (diff sensitivity, default 0.001). + +Shares the capture harness with **ui-walkthrough** - read its SKILL.md for the +stubbed-Playwright setup, worktree node_modules + `generate-icons`, the +stale-`:5173` gotcha, and the dark-mode init-script. Bundled helpers: +[capture-spec.template.ts](capture-spec.template.ts), [diff-shots.mjs](diff-shots.mjs), +[montage-template.html](montage-template.html), [shoot-sections.mjs](shoot-sections.mjs). + +## Process + +### 1. Resolve target + base +``` +gh pr view --json number,title,headRefName,baseRefName,url,files # PR +# or branch: base = merge-base(main, HEAD); head = HEAD +gh pr diff --name-only # or: git diff --name-only ...HEAD +``` + +### 2. Derive capture targets from the diff (the "analyse" step - no hand-listing) +Map changed frontend files to URLs generically: +- **Tools**: a changed `components/tools//…` or `hooks/tools//…` → + toolId → URL via the repo's own rule `getToolUrlPath` in + [toolsTaxonomy.ts:200](frontend/editor/src/core/data/toolsTaxonomy.ts): `/` + the + id kebab-cased (`addPageNumbers` → `/add-page-numbers`). +- **Pages/routes**: changed `filesPage/*` → `/files`, etc. +- `--all`: enumerate every tool in the registry instead of just changed ones. +Write `frontend/editor/screenshots/ui-diff/targets.json` = +`[{ "id":"compress", "url":"/compress", "name":"Compress" }]`. This is what makes +it generic - the spec never names a tool. + +### 3. Capture AFTER (head) then BEFORE (base) +Copy [capture-spec.template.ts](capture-spec.template.ts) → +`src/core/tests/stubbed/ui-before-after.spec.ts` (it loops `targets.json`, seeds a +sample PDF so file-dependent panels render, navigates to each URL, and screenshots +the full viewport - or the `--scope` container if given). Ensure the harness is ready +(node_modules + icons). +``` +# after = current head +cd frontend/editor && PR_SHOT_SIDE=after PR_SHOT_THEME=light \ + npx playwright test --project=stubbed ui-before-after.spec.ts +# before = base, in an isolated worktree (copy the spec + targets.json in) +git worktree add ../ba-base origin/ # or the merge-base +# set up its frontend, copy spec + screenshots/ui-diff/targets.json across, then: +cd ../ba-base/frontend/editor && PR_SHOT_SIDE=before PR_SHOT_THEME=light \ + npx playwright test --project=stubbed ui-before-after.spec.ts +# copy its screenshots/ui-diff/before/ back next to after/. Repeat with +# PR_SHOT_THEME=dark if --theme includes dark. Remove worktree when done. +``` + +### 4. Auto-diff (surface what changed) +``` +cd frontend/editor && node /diff-shots.mjs \ + screenshots/ui-diff/before screenshots/ui-diff/after screenshots/ui-diff +``` +Produces `diff-report.json` classifying each view `unchanged | changed | added | +removed`. For each changed view it computes the bounding box of differing pixels and +writes cropped `__before_crop.png` / `__after_crop.png` / `__diff.png` to that region +(+ padding) - **unless** the change covers more than `--pagewide` of the frame, where +it keeps the full frame (`pageWide:true`). Drop `unchanged` - that's the noise the +user doesn't want. + +### 5. Montage the changes +Build the manifest from the non-unchanged entries (group by tab/tool; each becomes a +state row with before/after). For changed views use the cropped `cropBefore` / +`cropAfter` from `diff-report.json` (tight on the affected region; full frame when +`pageWide`); `added`/`removed` render the "not present" placeholder. Fill +[montage-template.html](montage-template.html) (replace the `window.__BA__` data +block; base64-inline the PNGs for portability), then render one PNG per section with +[shoot-sections.mjs](shoot-sections.mjs). Optionally include the `__diff.png` overlay +as a third column. + +### 6. Deliver +Output the `montage_.png` files + a short summary (N changed / added / removed, +M unchanged skipped) and a paste-ready Markdown block. GitHub has no PR-body image +API, so tell the user to drag the PNGs into the description. Do **not** post to the +PR. + +## Gotchas +- Two installs (base worktree + head); junction main's node_modules only if its deps + match that ref, else `npm ci` (see ui-walkthrough's stale-dep note). +- A view that errors on one side (refactored/removed) → that side is missing; the + diff marks it added/removed rather than failing the run. +- Pixel diff needs equal dimensions, so capture at a fixed viewport (the template + does); a view whose size changed is reported as "changed (dimensions differ)", + uncropped. +- Auto-crop uses a single bounding box, so two far-apart changes give one large crop + (or trip `--pagewide`); narrow with `--scope` if that happens. +- `getToolUrlPath` is the source of truth for tool URLs - use it, don't guess slugs. +- Don't commit `screenshots/`, the throwaway spec, or the base worktree. diff --git a/.claude/skills/ui-before-after/capture-spec.template.ts b/.claude/skills/ui-before-after/capture-spec.template.ts new file mode 100644 index 0000000000..fd8882915b --- /dev/null +++ b/.claude/skills/ui-before-after/capture-spec.template.ts @@ -0,0 +1,67 @@ +// Generic before/after capturer. NOT app-specific: it walks a targets.json that +// the ui-before-after skill generates from the branch/PR diff, so nothing here is +// hand-listed. Copy to src/core/tests/stubbed/ui-before-after.spec.ts, then run +// once per (side, theme): +// PR_SHOT_SIDE=after PR_SHOT_THEME=light \ +// npx playwright test --project=stubbed ui-before-after.spec.ts +// +// targets.json shape: [{ "id":"compress", "url":"/compress", "name":"Compress", +// "needsFile": true }] +import { test } from "@app/tests/helpers/stub-test-base"; +import type { Page } from "@playwright/test"; +import fs from "node:fs"; +import path from "node:path"; + +const SIDE = process.env.PR_SHOT_SIDE ?? "after"; +const THEME = process.env.PR_SHOT_THEME ?? "light"; +// Capture the full viewport by default so the affected region is in frame +// wherever it is; diff-shots.mjs crops each comparison to what actually changed. +// Set PR_SHOT_SCOPE to a selector to narrow the capture to one container. +const SCOPE = process.env.PR_SHOT_SCOPE ?? ""; +const ROOT = path.resolve(process.cwd(), "screenshots", "ui-diff"); +const OUT = path.join(ROOT, SIDE); +// A tiny sample PDF so file-dependent tool panels render. Point at a real fixture. +const SAMPLE_PDF = process.env.PR_SHOT_SAMPLE ?? "src/core/tests/test-fixtures/sample.pdf"; + +type Target = { id: string; url: string; name?: string; needsFile?: boolean }; +const targets: Target[] = JSON.parse(fs.readFileSync(path.join(ROOT, "targets.json"), "utf-8")); + +test.use({ autoGoto: false, viewport: { width: 1600, height: 900 }, seedJwt: true }); + +async function applyTheme(page: Page): Promise { + if (THEME !== "dark") return; + await page.addInitScript(() => { + localStorage.setItem("mantine-color-scheme", "dark"); + localStorage.setItem("mantine-color-scheme-value", "dark"); + }); + await page.emulateMedia({ colorScheme: "dark" }); +} + +async function seedFile(page: Page): Promise { + if (!fs.existsSync(SAMPLE_PDF)) return; + await page.goto("/", { waitUntil: "domcontentloaded" }); + await page.getByTestId("files-button").click().catch(() => {}); + await page.locator('[data-testid="file-input"]').setInputFiles(SAMPLE_PDF).catch(() => {}); + await page.locator(".file-sidebar-file-item").first().isVisible({ timeout: 8_000 }).catch(() => {}); +} + +for (const t of targets) { + // One test per target so a single failure doesn't drop the rest. + test(`${SIDE}/${THEME} ${t.id}`, async ({ page }) => { + fs.mkdirSync(OUT, { recursive: true }); + await applyTheme(page); + if (t.needsFile !== false) await seedFile(page); + await page.goto(t.url, { waitUntil: "domcontentloaded" }); + await page.waitForTimeout(400); // settle Mantine portals/transitions + const shot = path.join(OUT, `${t.id}__${THEME}.png`); + if (SCOPE) { + const scope = page.locator(SCOPE).first(); + if (await scope.isVisible({ timeout: 8_000 }).catch(() => false)) { + await scope.screenshot({ path: shot }); + return; + } + } + // Full viewport (fixed size → stable dimensions for pixel diffing). + await page.screenshot({ path: shot }); + }); +} diff --git a/.claude/skills/ui-before-after/diff-shots.mjs b/.claude/skills/ui-before-after/diff-shots.mjs new file mode 100644 index 0000000000..94ff5e2aad --- /dev/null +++ b/.claude/skills/ui-before-after/diff-shots.mjs @@ -0,0 +1,106 @@ +// Auto-diff before/ vs after/ screenshots, classify each as +// unchanged | changed | added | removed, and CROP each changed pair to the +// affected region (bounding box of differing pixels + padding) - unless the +// change spans most of the page, in which case the full frame is kept. +// Run from frontend/editor (so deps resolve): +// node /diff-shots.mjs [outDir] +// Env: +// DIFF_THRESHOLD min fraction of differing pixels to count as changed (default 0.001) +// DIFF_PAD padding px around the affected region (default 24) +// DIFF_PAGEWIDE if affected bbox area / image area exceeds this, keep full frame (default 0.6) +import fs from "node:fs"; +import path from "node:path"; +import { createRequire } from "node:module"; + +const require = createRequire(path.join(process.cwd(), "noop.js")); +const pm = require("pixelmatch"); +const pixelmatch = pm.default || pm; +const { PNG } = require("pngjs"); + +const beforeDir = path.resolve(process.argv[2]); +const afterDir = path.resolve(process.argv[3]); +const outDir = path.resolve(process.argv[4] || afterDir); +const THRESHOLD = Number(process.env.DIFF_THRESHOLD ?? "0.001"); +const PAD = Number(process.env.DIFF_PAD ?? "24"); +const PAGEWIDE = Number(process.env.DIFF_PAGEWIDE ?? "0.6"); + +const read = (p) => PNG.sync.read(fs.readFileSync(p)); +const isShot = (f) => f.endsWith(".png") && !/__(diff|before_crop|after_crop)\.png$/.test(f); +const list = (d) => (fs.existsSync(d) ? fs.readdirSync(d).filter(isShot) : []); +const names = [...new Set([...list(beforeDir), ...list(afterDir)])].sort(); +fs.mkdirSync(outDir, { recursive: true }); + +function cropPNG(src, x, y, w, h) { + const out = new PNG({ width: w, height: h }); + PNG.bitblt(src, out, x, y, w, h, 0, 0); + return out; +} +const writePNG = (p, png) => fs.writeFileSync(p, PNG.sync.write(png)); + +// Bounding box of differing pixels using a diff mask (alpha>0 where changed). +function changedBBox(before, after, w, h) { + const mask = new PNG({ width: w, height: h }); + pixelmatch(before.data, after.data, mask.data, w, h, { threshold: 0.1, diffMask: true }); + let minX = w, minY = h, maxX = -1, maxY = -1, count = 0; + for (let y = 0; y < h; y++) { + for (let x = 0; x < w; x++) { + if (mask.data[(y * w + x) * 4 + 3] > 0) { + count++; + if (x < minX) minX = x; if (x > maxX) maxX = x; + if (y < minY) minY = y; if (y > maxY) maxY = y; + } + } + } + return maxX < 0 ? null : { minX, minY, maxX, maxY, count }; +} + +const report = []; +for (const name of names) { + const id = name.replace(/\.png$/, ""); + const bp = path.join(beforeDir, name), ap = path.join(afterDir, name); + const hasB = fs.existsSync(bp), hasA = fs.existsSync(ap); + if (hasB && !hasA) { report.push({ id, status: "removed", before: bp }); continue; } + if (!hasB && hasA) { report.push({ id, status: "added", after: ap }); continue; } + + const before = read(bp), after = read(ap); + if (before.width !== after.width || before.height !== after.height) { + report.push({ id, status: "changed", note: "dimensions differ", before: bp, after: ap }); + continue; + } + const w = after.width, h = after.height; + const overlay = new PNG({ width: w, height: h }); + const px = pixelmatch(before.data, after.data, overlay.data, w, h, { threshold: 0.1 }); + const ratio = px / (w * h); + if (ratio <= THRESHOLD) { report.push({ id, status: "unchanged", ratio: Number(ratio.toFixed(5)), before: bp, after: ap }); continue; } + + const box = changedBBox(before, after, w, h); + // Pad + clamp the affected region. + const x = Math.max(0, box.minX - PAD), y = Math.max(0, box.minY - PAD); + const x2 = Math.min(w, box.maxX + 1 + PAD), y2 = Math.min(h, box.maxY + 1 + PAD); + const bw = x2 - x, bh = y2 - y; + const pageWide = (bw * bh) / (w * h) > PAGEWIDE; + + const entry = { id, status: "changed", ratio: Number(ratio.toFixed(5)), before: bp, after: ap, pageWide }; + if (pageWide) { + // Change spans most of the page - keep the full frame, full overlay. + const dp = path.join(outDir, `${id}__diff.png`); writePNG(dp, overlay); + entry.diff = dp; + } else { + entry.bbox = { x, y, w: bw, h: bh }; + const cb = path.join(outDir, `${id}__before_crop.png`); writePNG(cb, cropPNG(before, x, y, bw, bh)); + const ca = path.join(outDir, `${id}__after_crop.png`); writePNG(ca, cropPNG(after, x, y, bw, bh)); + const dp = path.join(outDir, `${id}__diff.png`); writePNG(dp, cropPNG(overlay, x, y, bw, bh)); + entry.cropBefore = cb; entry.cropAfter = ca; entry.diff = dp; + } + report.push(entry); +} + +fs.writeFileSync(path.join(outDir, "diff-report.json"), JSON.stringify(report, null, 2)); +const changed = report.filter((r) => r.status !== "unchanged"); +console.log(`diffed ${report.length} view(s): ${changed.length} changed/added/removed, ${report.length - changed.length} unchanged`); +for (const r of changed) { + const tail = r.status !== "changed" ? "" + : r.pageWide ? " (page-wide → full frame)" + : ` (${(r.ratio * 100).toFixed(2)}%, cropped to ${r.bbox.w}×${r.bbox.h})`; + console.log(` ${r.status.padEnd(9)} ${r.id}${tail}${r.note ? " - " + r.note : ""}`); +} diff --git a/.claude/skills/ui-before-after/make_example.py b/.claude/skills/ui-before-after/make_example.py new file mode 100644 index 0000000000..ec5eeee029 --- /dev/null +++ b/.claude/skills/ui-before-after/make_example.py @@ -0,0 +1,48 @@ +"""Build EXAMPLE.html from montage-template.html using REAL files-page shots as +stand-in before/after pairs (layout demo, not an actual PR diff). Inlines PNGs as +data URIs so the HTML is portable. Run: python make_example.py""" +import base64 +import json +import pathlib +import re + +HERE = pathlib.Path(__file__).parent +SHOTS = pathlib.Path( + r"C:\Users\systo\git\Stirling-PDFNew\.claude\worktrees\kind-faraday-522a30" + r"\frontend\editor\screenshots\files-page" +) + + +def uri(fname): + p = SHOTS / fname + return "data:image/png;base64," + base64.b64encode(p.read_bytes()).decode() if p.exists() else None + + +data = { + "pr": "DEMO", + "title": "EXAMPLE — before/after montage (layout demo, real Files-page shots; not a real PR diff)", + "base": "main", "head": "demo-branch", + "cropSelector": "[data-sidebar=\"tool-panel\"] (real runs crop to the side; these demo shots are full-page)", + "tabs": [ + {"id": "files", "title": "Files page", "ctx": "Each row = one flow state; left = base branch, right = this PR.", + "states": [ + {"name": "Empty folder", "before": uri("01_empty_state_ctas.png"), "after": uri("02_empty_state_storage_off.png")}, + {"name": "Files + details panel", "before": uri("03_subtoolbar_with_files.png"), "after": uri("06_details_panel_save_to_server.png")}, + {"name": "Delete folder confirm", "before": None, "after": uri("19_delete_folder_dialog.png"), "note": "New in this PR"}, + ]}, + {"id": "move", "title": "Move-to-folder dialog", + "states": [ + {"name": "Dialog opened", "before": uri("07_move_dialog_collapsed.png"), "after": uri("08_move_dialog_create_folder_expanded.png")}, + {"name": "After folder created", "before": None, "after": uri("08b_move_dialog_after_create_folder.png"), "note": "New flow"}, + ]}, + ], +} + +tpl = (HERE / "montage-template.html").read_text(encoding="utf-8") +out = re.sub( + r"/\*__DATA__\*/.*?/\*__END__\*/", + lambda _m: "/*__DATA__*/" + json.dumps(data) + "/*__END__*/", + tpl, count=1, flags=re.S, +) +(HERE / "EXAMPLE.html").write_text(out, encoding="utf-8") +print("wrote", HERE / "EXAMPLE.html", "(", (HERE / "EXAMPLE.html").stat().st_size // 1024, "KB )") diff --git a/.claude/skills/ui-before-after/montage-template.html b/.claude/skills/ui-before-after/montage-template.html new file mode 100644 index 0000000000..ca0f69d4cf --- /dev/null +++ b/.claude/skills/ui-before-after/montage-template.html @@ -0,0 +1,106 @@ + + + + + +Before / After + + + +
+ + + + + diff --git a/.claude/skills/ui-before-after/shoot-sections.mjs b/.claude/skills/ui-before-after/shoot-sections.mjs new file mode 100644 index 0000000000..504e531998 --- /dev/null +++ b/.claude/skills/ui-before-after/shoot-sections.mjs @@ -0,0 +1,25 @@ +// Render each .tab-section of a montage HTML into its own PNG (the PR-ready image). +// Run from frontend/editor (so @playwright/test resolves): +// node /shoot-sections.mjs +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { createRequire } from "node:module"; + +const require = createRequire(path.join(process.cwd(), "noop.js")); +const { chromium } = require("@playwright/test"); + +const htmlPath = path.resolve(process.argv[2]); +const outDir = path.resolve(process.argv[3] || path.dirname(htmlPath)); + +const browser = await chromium.launch(); +const page = await browser.newPage({ viewport: { width: 1200, height: 1200 }, deviceScaleFactor: 2 }); +await page.goto(pathToFileURL(htmlPath).href, { waitUntil: "load" }); +await page.waitForTimeout(250); // let images/fonts paint +const ids = await page.$$eval(".tab-section", (els) => els.map((e) => e.id)); +if (!ids.length) { console.error("no .tab-section found"); process.exit(1); } +for (const id of ids) { + const name = id.replace(/^section-/, ""); + await page.locator("#" + id).screenshot({ path: path.join(outDir, `montage_${name}.png`) }); + console.log("wrote montage_" + name + ".png"); +} +await browser.close(); diff --git a/.claude/skills/ui-walkthrough/SKILL.md b/.claude/skills/ui-walkthrough/SKILL.md new file mode 100644 index 0000000000..ef6d4b9dec --- /dev/null +++ b/.claude/skills/ui-walkthrough/SKILL.md @@ -0,0 +1,120 @@ +--- +name: ui-walkthrough +description: >- + Full UI investigation of the current branch's feature. Enumerates every view + and state (empty, populated, loading, error, each dialog/menu/panel, responsive + breakpoints, light + dark + RTL), captures them with the stubbed Playwright + harness, assembles a single-image HTML walkthrough with a global light/dark + toggle slider, then runs two review passes: visual/consistency (alignment, + spacing, professionalism, dark/light parity, contrast, truncation) and + UX/ease-of-use (flow, discoverability, affordances, empty/error states, + expectations). Use when asked for a UI walkthrough, screenshot review, design + or QA pass, "find anywhere to make it easier/better for users", or before + merging frontend work. Pass --fix to auto-apply safe frontend fixes and + re-capture; --theme to limit themes; --no-rtl to skip RTL. +argument-hint: "[feature/area] [--fix] [--theme light|dark|both] [--no-rtl] [--breakpoints]" +allowed-tools: Read, Write, Edit, Glob, Grep, Bash +--- + +# UI Walkthrough + +Produce a reviewable HTML walkthrough of a feature's UI in every state and theme, +then critique it. Optionally auto-fix and re-capture. + +`$ARGUMENTS` may name the feature/area to focus on. If empty, scope from the +current branch diff. Flags: `--fix`, `--theme light|dark|both` (default both), +`--no-rtl`, `--breakpoints` (also capture phone/narrow widths). + +## What this repo gives you (use it, don't reinvent) + +- **Stubbed Playwright project** = backend-free screenshots via `page.route()` mocks. + Reference implementation: `frontend/editor/src/core/tests/stubbed/files-page-screenshots.spec.ts`. + It already shows the light / **dark** / **RTL** passes, JWT seeding, IndexedDB + seeding, and dumping PNGs to a `screenshots//` folder. Copy its shape. +- Helpers: `frontend/editor/src/core/tests/helpers/ui-helpers.ts` + (`uploadFiles`, `openSettings`, `waitForModalOpen`, `dismissTourTooltip`, …) + and the `stub-test-base` fixtures (`autoGoto`, `seedJwt`, `viewport`). +- Config: `frontend/editor/playwright.config.ts` (run from `frontend/editor/`). +- Report template: [report-template.html](report-template.html) - self-contained, + one big image at a time, a global light/dark slider that flips every shot, + thumbnail rail, prev/next + arrow keys, and a Findings tab. + +## Process + +### 1. Scope the feature +- If `$ARGUMENTS` is empty: `git diff --name-only main...HEAD` and read the PR/commits. + Identify changed pages, tools (`core/components/tools/` or `core/tools/`), + dialogs, panels, and routes. +- Enumerate **every view and state** to capture, e.g.: + empty / populated / loading / error / disabled; each dialog, menu, popover, tooltip; + each tab or step; selection + multi-select; success/result panel; and (if relevant) + permission/role variants. Write the list down before capturing - it's the report's spine. + +### 2. Prepare the harness (worktree-safe) +Worktrees have no `node_modules` and no generated icons. From repo root: +``` +cd frontend && npm ci # or junction main's node_modules (see memory) +cd frontend/editor && node scripts/generate-icons.js +``` +Kill any stale dev server first (it serves old modules): +`Get-NetTCPConnection -LocalPort 5173 -State Listen | %{ Stop-Process -Id $_.OwningProcess -Force }` + +### 3. Write the capture spec +Create `frontend/editor/src/core/tests/stubbed/-walkthrough.spec.ts`, +modeled on `files-page-screenshots.spec.ts`. For each enumerated view: +- stub the APIs it needs, drive the UI to that state, wait on a real locator + (not a fixed sleep), `await settle(page)` for Mantine portals, then + `page.screenshot({ path: shotPath("NN_name_") })`. +- Capture each view in **light and dark** (and RTL unless `--no-rtl`). Reuse the + `enableDarkMode` / `enableRtl` init-script pattern from the reference spec + (`localStorage["mantine-color-scheme"]="dark"` + `emulateMedia({colorScheme:"dark"})`). +- Name shots `NN__.png` so light/dark pair up by suffix. +- Prefer **stable test-ids** over translated accessible names (RTL/i18n breaks text locators). + +Run it: `cd frontend/editor && npx playwright test --project=stubbed -walkthrough.spec.ts`. +Add `--project=stubbed-firefox`/`-webkit` only if cross-browser layout matters. + +### 4. Build the report +- Copy `report-template.html` to `screenshots//walkthrough.html` (so the + relative `screenshots/...` image paths resolve, or rewrite paths to sit beside it). +- Build the manifest and inject it: replace the JSON between the + `/*__DATA__*/` … `/*__END__*/` markers with one `views[]` entry per view + (`{id,title,light,dark,viewport,notes}`) and an empty `findings` object you'll + fill in step 5. Keep `light`/`dark` as relative paths. +- The toggle slider answers the "one big image + flip light/dark for all" request: + it shows a single large screenshot, and switching the slider re-themes every view. + +### 5. Review pass 1 - visual & consistency +Open each screenshot (Read the PNG) and judge against the others: +alignment & spacing rhythm, control placement, button hierarchy, typography, +**light/dark parity** (contrast, invisible borders, washed-out text, wrong tokens), +truncation/overflow, RTL mirroring, focus states, icon consistency, professional polish. +Record each issue as a finding `{severity:high|med|low, view, title, detail, fix}`. + +### 6. Review pass 2 - UX & ease of use +Walk the flow as a first-time user: discoverability, number of steps, affordance +clarity, empty-state guidance, error recovery, destructive-action confirmation, +defaults, loading feedback, mobile reachability, accessible names, and whether the +UI matches user expectations for this kind of tool. Record findings the same way. + +Write both finding lists into the report's `findings.visual` / `findings.ux`, +and add short per-view `notes`. Re-inject the manifest. + +### 7. If `--fix` +Only safe, self-contained frontend fixes (spacing, alignment, tokens, missing +dark-mode colors, labels, aria, obvious copy). For each: edit the component/CSS, +mark the finding `fixed:true` with what changed, then **re-run the spec** to +re-capture the affected shots and regenerate the report. Run `task frontend:check`. +Leave anything risky or ambiguous as a finding, not a change. + +### 8. Deliver +Tell the user the report path and give a tight chat summary: N views × +themes captured, top findings by severity, and (if `--fix`) what changed. +Optionally `SendUserFile` the `walkthrough.html`. + +## Gotchas +- Stale `:5173` server serves old bundles - kill it before capturing (see step 2). +- Missing `material-symbols-icons.json` → blank app → every shot times out. Run + `generate-icons.js` first. +- `await settle(page)` before shots or portals/transitions tear mid-capture. +- Don't commit the generated `screenshots/` or the throwaway spec unless asked. diff --git a/.claude/skills/ui-walkthrough/make_example.py b/.claude/skills/ui-walkthrough/make_example.py new file mode 100644 index 0000000000..b65c2a010a --- /dev/null +++ b/.claude/skills/ui-walkthrough/make_example.py @@ -0,0 +1,116 @@ +"""Build a self-contained EXAMPLE.html from report-template.html with mock +light/dark screenshots, so the viewer + global theme slider can be demoed +without a real capture run. Run: python make_example.py""" +import base64 +import json +import pathlib +import re + +HERE = pathlib.Path(__file__).parent + + +def svg(bg, fg, panel, accent, muted, label, kind): + """A simple fake 'screen' SVG: title bar, sidebar, content varies by kind.""" + parts = [ + f'', + f'', + # top bar + f'', + f'', + f'', + f'', + # left sidebar + f'', + ] + for i in range(6): + y = 100 + i * 56 + parts.append(f'') + if kind == "empty": + parts += [ + f'', + f'', + f'{label}', + ] + elif kind == "form": + for i in range(4): + y = 140 + i * 90 + parts.append(f'') + parts.append(f'') + parts.append(f'') + parts.append(f'{label}') + else: # dialog + parts += [ + f'', + f'', + f'', + f'', + f'', + f'', + f'', + f'{label}', + ] + parts.append("") + return "".join(parts) + + +def data_uri(s): + return "data:image/svg+xml;base64," + base64.b64encode(s.encode()).decode() + + +LIGHT = dict(bg="#ffffff", fg="#111418", panel="#f1f3f6", accent="#2f6fed", muted="#c2c8d0") +DARK = dict(bg="#16181c", fg="#000000", panel="#1f232a", accent="#5b8cff", muted="#3a414b") + + +def pair(kind, label): + return ( + data_uri(svg(LIGHT["bg"], LIGHT["fg"], LIGHT["panel"], LIGHT["accent"], LIGHT["muted"], label, kind)), + data_uri(svg(DARK["bg"], DARK["fg"], DARK["panel"], DARK["accent"], DARK["muted"], label, kind)), + ) + + +views = [] +for idx, (kind, title, label) in enumerate([ + ("empty", "Empty state", "Drop a PDF to start"), + ("form", "Tool options panel", "Compress options"), + ("dialog", "Confirm dialog", "Replace original file?"), +], start=1): + light, dark = pair(kind, label) + views.append({ + "id": f"{idx:02d}_{kind}", + "title": title, + "light": light, + "dark": dark, + "viewport": "1600x900", + "notes": ["This is mock data to demo the viewer."], + }) + +data = { + "feature": "EXAMPLE - Compress PDF (mock data)", + "branch": "demo", + "generated": "example", + "views": views, + "findings": { + "visual": [ + {"severity": "high", "view": "03_dialog", "title": "Dialog buttons too close", + "detail": "Cancel/Confirm have only 8px gap; easy to misclick.", + "fix": "Increase gap to var(--mantine-spacing-md)."}, + {"severity": "low", "view": "02_form", "title": "Field labels low contrast in dark mode", + "detail": "Muted token fails WCAG AA on the dark panel.", + "fix": "Use --mantine-color-dimmed instead of a hard-coded grey."}, + ], + "ux": [ + {"severity": "med", "view": "01_empty", "title": "Primary CTA below the dropzone", + "detail": "Users expect the action button adjacent to the dropzone.", + "fix": "Move the button directly under the dashed zone."}, + ], + }, +} + +tpl = (HERE / "report-template.html").read_text(encoding="utf-8") +out = re.sub( + r"/\*__DATA__\*/.*?/\*__END__\*/", + lambda _m: "/*__DATA__*/" + json.dumps(data) + "/*__END__*/", + tpl, count=1, flags=re.S, +) +(HERE / "EXAMPLE.html").write_text(out, encoding="utf-8") +print("wrote", (HERE / "EXAMPLE.html")) diff --git a/.claude/skills/ui-walkthrough/report-template.html b/.claude/skills/ui-walkthrough/report-template.html new file mode 100644 index 0000000000..3e96216a3b --- /dev/null +++ b/.claude/skills/ui-walkthrough/report-template.html @@ -0,0 +1,298 @@ + + + + + + +UI Walkthrough + + + +
+
+

UI Walkthrough

+
+
+
+
+ + +
+
+ +
+ +
+ +
+
+ + +
+ +
+
+

+
+
    +
    +
    +
    + +
    + + + + +