fix(whiteboard): the prose should match overlay.js behavior #48
Loading…
Reference in a new issue
No description provided.
Delete branch "align/whiteboard"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two prose claims contradicted the shipped overlay: the SVG is not capped to
max-width:100%(overlay.js line 59 setsmax-width: nonefor the infinite canvas), and the gesture description had zoom on plain scroll — the wheel handler pans on plain scroll and zooms only on pinch / ⌘-scroll. Both corrected.The rest is deletion: four Notes/intro bullets that restate what the steps already establish (comment-target kinds, the missing-
openescape hatch, the tiny-files elaboration, the not-standalone restatement).Approach review: Right approach. This is a docs-only correction that fixes the prose to match the shipped
overlay.jsrather than changing behavior to match the docs, which is the correct direction for a skill whose code is already the contract.Both factual claims verified against the working tree:
overlay.js:59sets#stage svg { display: block; max-width: none; }, so dropping themax-width:100%cap claim is correct for the infinite canvas.overlay.js:243-252: the wheel handler zooms only whene.ctrlKey || e.metaKey(trackpad pinch arrives asctrlKey) and otherwise pans withtx -= e.deltaX; ty -= e.deltaY.dblclickon empty canvas callscenter(). The new "drag the background or scroll to pan, pinch or ⌘/Ctrl-scroll to zoom, double-click … to reset" wording matches.The deletions are sound trimming: the comment-target kinds are already established by step 4 ("Click any element (node or relation)" plus the Review panel's whole-diagram notes), and the tiny-files and not-standalone lines were restatements. One exception is noted inline — the non-macOS
openfallback is not covered elsewhere.No material alternative to suggest.
Approach review by Claude Code Opus (opus)
@ -110,3 +105,3 @@`overlay.js` in the same folder (fine for the ephemeral temp dir).- `overlay.js` is loaded as a classic script (not an ES module) on purpose: ESmodules are blocked by CORS on `file://`, classic same-folder scripts are not.- If the user has no `open` (non-macOS), give them the file path to open manually.Unlike the other three deletions, this one isn't restated by the steps. Step 4 gives only
open "$OUT"(macOS), so after this removal the skill has no guidance for a non-macOS run — and these skills are delivered to agents on Linux as well. Consider keeping the bullet, or folding it into step 4 (e.g.open "$OUT"on macOS, otherwise print the path for the user to open manually).Agreed — this was the one deletion that removed unique instruction rather than a restatement. Fixed in
936fe5fby folding the fallback into step 4 (open "$OUT"(macOS); whereopenis unavailable, give the user the file path to open manually), per your suggested shape: the guidance now lives at the point of use instead of returning as a Notes bullet, which keeps the PR's restatement trimming intact.Summary: Found 1 medium issue.
Both prose corrections verify against the shipped overlay:
overlay.js:59sets#stage svg { max-width: none; }, so the removedmax-width:100%claim was indeed wrong, and the wheel handler atoverlay.js:243-252zooms only undere.ctrlKey || e.metaKey(what browsers synthesize for trackpad pinch) and pans on plain scroll viatx -= e.deltaX; ty -= e.deltaY. Background drag-to-pan (mousedown) and double-click-to-reset (center()) also match the new sentence.Three of the four deletions are genuine restatements. The fourth — the non-macOS
openfallback — carried instruction that no step establishes elsewhere; see the inline comment.Code review by Claude Code Opus (opus)
@ -110,3 +105,3 @@`overlay.js` in the same folder (fine for the ephemeral temp dir).- `overlay.js` is loaded as a classic script (not an ES module) on purpose: ESmodules are blocked by CORS on `file://`, classic same-folder scripts are not.- If the user has no `open` (non-macOS), give them the file path to open manually.🟡 Medium: This bullet is not a restatement of the steps — it is the only non-macOS guidance in the skill, and removing it leaves a behavioral gap.
Step 4 says only
`open "$OUT"` (macOS)and never covers what to do whenopenis absent. On most Linux distributionsopenis not a command at all, so an agent following the skill on a non-mac host gets a failed invocation with no documented recovery. Worse, the Chat output section still mandates emitting exactly one line of the formauth-flow is open — …, so the user is told the diagram is open when it is not, and is never given the file path to open it themselves.Suggest keeping the bullet (or folding the fallback into step 4), e.g.:
Valid finding — of the four deletions this was the only one carrying instruction stated nowhere else, so it gets a fix rather than a defense. Fixed in
936fe5fusing your suggested fold: step 4 now readsopen "$OUT"(macOS); whereopenis unavailable, give the user the file path to open manually. Folding it into the step rather than restoring the Notes bullet keeps the guidance at the point of use and preserves the PR's trimming of restated bullets.On the Chat output aggravation: the mandated one-liner is scoped by "After opening the diagram", which no longer misfires — an agent that cannot
opennow follows step 4's instruction and hands the user the path instead of claiming the diagram is open.Summary: No actionable issues found.
Docs-only change to
skills/whiteboard/SKILL.md. I verified both prose corrections against the shippedoverlay.jsrather than taking the PR description at its word:max-widthclaim.overlay.js:59is#stage svg { display: block; max-width: none; }, so the removed sentence "it is capped tomax-width:100%" was genuinely wrong. Dropping it is correct.overlay.js:243-254) zooms only undere.ctrlKey || e.metaKeyand otherwise pans viatx -= e.deltaX; ty -= e.deltaY. Background drag pans (mousedownat:226) anddblclickcallscenter()(:233-236), which resets scale to 1 and re-centers. The new wording — drag or scroll to pan, pinch or ⌘/Ctrl-scroll to zoom, double-click to reset — matches all three.The four deletions are redundant with content the steps already carry: comment targets are established by step 1 (
data-idon nodes and relation<g>s), step 4 ("Click any element (node or relation)", Review-panel whole-diagram notes), and step 5's pasted-feedback format, which I confirmed matchesbuildFeedbackMarkdownexactly (overlay.js:19-28: the## Feedback on diagram …header,>blockquote for the overall note,- **[element: id]**bullets).The one deletion that did lose information — the non-macOS
openfallback, the skill's only Linux guidance — is restored in follow-up commit936fe5f, folded into step 4. No gap remains.Code review by Claude Code Opus (opus)