Fix #560: refine inline review composer (gutter triggers, row-click, multi-drafts) #562

Merged

crueber merged 1 commit from fix/issue-560 into main

2026-09-15 11:38:35 +00:00

crueber commented

2026-09-15 11:34:12 +00:00

Owner

Refines the conversation-page DiffFile inline review composer (web/src/pages/Pull.jsx), presentation only — review-anchor.js untouched.

Gutter triggers: per-line + moves from the row body into a left w-6 gutter cell, always visible (no hover dependency), Files-tab gutter conventions (gutter cell + line-labelling a11y).
Row click stages a composer immediately below that line (reuses stageLine/buildAnchor, incl. Shift+click ranges).
Keyed multi-drafts: single createSignal(null) + file-end Show becomes a Map keyed by hunk+row rendered inline in the row For — unlimited coexisting composers, independent submit/cancel in any order.
Preserved: stageLine to buildAnchor to CommentComposer to onStage; byte-identical anchor inputs; shift-range clamp; click isolation (stopPropagation + closest guard); #502 anon gate on all three entry points; thread placement/resolve/drift; no new deps; Tailwind-only, no new CSS.

Tests: new web/test/unit/inline-composer-560.test.js (19 tests); #560-scoped updates to two stale pins (tap-comment-555, diff-row-544). Full unit suite minus smoke: 1322 pass / 0 fail. vite build + esbuild bundle green; go vet ./internal/... clean. Docs: FIXED (#560) amendment in docs/go/12_web_ui.md + style-guideline §8 bullet update, same commit.

Refines the conversation-page DiffFile inline review composer (web/src/pages/Pull.jsx), presentation only — review-anchor.js untouched. - Gutter triggers: per-line + moves from the row body into a left w-6 gutter cell, always visible (no hover dependency), Files-tab gutter conventions (gutter cell + line-labelling a11y). - Row click stages a composer immediately below that line (reuses stageLine/buildAnchor, incl. Shift+click ranges). - Keyed multi-drafts: single createSignal(null) + file-end Show becomes a Map keyed by hunk+row rendered inline in the row For — unlimited coexisting composers, independent submit/cancel in any order. - Preserved: stageLine to buildAnchor to CommentComposer to onStage; byte-identical anchor inputs; shift-range clamp; click isolation (stopPropagation + closest guard); #502 anon gate on all three entry points; thread placement/resolve/drift; no new deps; Tailwind-only, no new CSS. Tests: new web/test/unit/inline-composer-560.test.js (19 tests); #560-scoped updates to two stale pins (tap-comment-555, diff-row-544). Full unit suite minus smoke: 1322 pass / 0 fail. vite build + esbuild bundle green; go vet ./internal/... clean. Docs: FIXED (#560) amendment in docs/go/12_web_ui.md + style-guideline §8 bullet update, same commit.

crueber added 1 commit

2026-09-15 11:34:12 +00:00

Fix #560 : gutter triggers, row-click staging, keyed multi-drafts for the inline review composer 71b3497328

crueber commented

2026-09-15 11:38:19 +00:00

Author

Owner

APPROVE — reviewed origin/main..origin/fix/issue-560 (71b3497, 6 files +388/-72) against issue #560, verified in /tmp/walhub-560.

All 9 acceptance criteria hold (presentation-only, review-anchor.js untouched):
(1) Gutter +: per-line button in w-6 gutter cell ahead of line numbers, always rendered, no hidden/group-hover/pointer-coarse indirection; old ml-2 hidden trigger gone, row body holds text only.
(2) Row click: row div onClick -> onRowClick -> stageLine (same buildAnchor path), composer renders inline below its row (draft Show inside row For, above thread cards); no file-end Show remains.
(3) Keyed drafts: createSignal(new Map()) keyed hunkIdx:rowIdx; staging is new Map(prev).set (siblings kept); each CommentComposer stays mounted per row so getBody text survives sibling adds; same-key re-stage updates anchor label without wiping text.
(4) Independent submit/cancel: per-key closeDraft, submit stages {anchor, body} into onStage then closes only its key; both paths pinned (>=2 closeDraft call sites).
(5) Shift+click: same file+side+hunk clamp, Math.min/max range, setLast only on plain path — gutter and row both forward ev.
(6) Isolation: gutter stopPropagation + row closest(button, a, input, textarea, select, [data-no-row-comment]); composers/thread cards are siblings BELOW the row so their clicks never reach the row handler; no touch handlers (tap arrives as click).
(7) #502 gate: Show on gutter + Show on composer + early return in onRowClick (3 gates, canComment plumbing unchanged).
(8) buildAnchor byte-identical: single-line NEW/OLD shapes, hash parity, pinned Go-twin vector all pass; DiffFile never hashes directly, no window.prompt.
(9) Placement/resolve/drift untouched: placement()/byKey/unplaced/freshness/unresolved-first sort + shared CommentComposer all pinned.

Also checked: a11y labels (gutter Comment on line N in + range title + F2 emerald tokens both themes; draft aria-label; focus-visible:outline ring; keyboard path via button so row div needs no role); Tailwind-only (no ui.css changes, no

APPROVE — reviewed origin/main..origin/fix/issue-560 (71b3497, 6 files +388/-72) against issue #560, verified in /tmp/walhub-560. All 9 acceptance criteria hold (presentation-only, review-anchor.js untouched): (1) Gutter +: per-line button in w-6 gutter cell ahead of line numbers, always rendered, no hidden/group-hover/pointer-coarse indirection; old ml-2 hidden trigger gone, row body holds text only. (2) Row click: row div onClick -> onRowClick -> stageLine (same buildAnchor path), composer renders inline below its row (draft Show inside row For, above thread cards); no file-end Show remains. (3) Keyed drafts: createSignal(new Map()) keyed hunkIdx:rowIdx; staging is new Map(prev).set (siblings kept); each CommentComposer stays mounted per row so getBody text survives sibling adds; same-key re-stage updates anchor label without wiping text. (4) Independent submit/cancel: per-key closeDraft, submit stages {anchor, body} into onStage then closes only its key; both paths pinned (>=2 closeDraft call sites). (5) Shift+click: same file+side+hunk clamp, Math.min/max range, setLast only on plain path — gutter and row both forward ev. (6) Isolation: gutter stopPropagation + row closest(button, a, input, textarea, select, [data-no-row-comment]); composers/thread cards are siblings BELOW the row so their clicks never reach the row handler; no touch handlers (tap arrives as click). (7) #502 gate: Show on gutter + Show on composer + early return in onRowClick (3 gates, canComment plumbing unchanged). (8) buildAnchor byte-identical: single-line NEW/OLD shapes, hash parity, pinned Go-twin vector all pass; DiffFile never hashes directly, no window.prompt. (9) Placement/resolve/drift untouched: placement()/byKey/unplaced/freshness/unresolved-first sort + shared CommentComposer all pinned. Also checked: a11y labels (gutter Comment on line N in <path> + range title + F2 emerald tokens both themes; draft aria-label; focus-visible:outline ring; keyboard path via button so row div needs no role); Tailwind-only (no ui.css changes, no <style>, no style=); docs law 11/12 (12_web_ui.md FIXED entry + style-guideline §8 update, same commit); no new deps (package.json untouched, dep list pinned); stale-pin updates minimal and justified (diff-row-544 row-div regex +onClick; tap-comment-555 visibility test rewritten to gutter model, full cover moved to inline-composer-560.test.js 19 tests). Verification: targeted 3 files 41/41 green; full unit suite minus smoke 1322 pass / 0 fail (matches PR claim exactly); vite build green (2.65s, exit 0); diff spot-checked for import/class errors — none (no new imports, all classes valid Tailwind). Nits (non-blocking, no fix pushed): cancel button still wears pre-existing undefined `link` class (predates #560; #555 only cleaned the +); row div has no cursor-pointer/role (keyboard covered by gutter button); [data-no-row-comment] guard has no producer yet (harmless future hook). Left as-is to keep the change minimal. Verdict: APPROVE. No fix commits pushed.

crueber merged commit 907b9a77ee into main

2026-09-15 11:38:35 +00:00

crueber referenced this pull request from a commit

2026-09-15 11:38:37 +00:00

Merge pull request 'Fix #560: refine inline review composer (gutter triggers, row-click, multi-drafts)' (#562) from fix/issue-560 into main

crueber referenced this pull request

2026-09-15 11:38:43 +00:00

Refine inline review composer: gutter + buttons, click-anywhere-on-line composer below the line, unlimited simultaneous composers #560