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

New issue

Closed

opened 2026-09-15 11:28:47 +00:00 by crueber · 1 comment

crueber commented

2026-09-15 11:28:47 +00:00

Owner

What's requested

Refine the PR inline review composer interaction (built for #546, extended by #555) on the conversation page's diff so commenting feels like per-line affordance rather than a single staged draft:

Per-click "+" buttons outside the diff (left gutter). Move the per-line comment trigger out of the diff row body into the left gutter — one discrete "+" button per commentable line, always rendered there, so the trigger is a stable target that doesn't shift row content or depend on hover.
Click-anywhere-on-line opens the composer below that line. Clicking anywhere on a diff line (the row itself, not just the button) instantiates a composer immediately below that line in the diff.
Unlimited simultaneous composers per file. A reviewer can have as many open draft composers as they want in one file's diff at the same time — opening one never closes or replaces another.

Current state (static read of the current tree)

web/src/pages/Pull.jsx, DiffFile:

The "+" button is rendered inside each diff row, after the line text (<button ... class="ml-2 hidden shrink-0 px-1 text-emerald-600 group-hover:inline hover:text-emerald-700 focus-visible:inline pointer-coarse:inline ..."> in the row <div class="group flex font-mono text-xs ...">). It is hover/keyboard/coarse-pointer conditional, not a permanent gutter element.
Only the button opens the composer; clicking elsewhere on the row does nothing (row text is intentionally handler-free per the #555 note).
Draft state is a single signal per file: const [getDraft, setDraft] = createSignal(null) — stageLine overwrites setDraft({ anchor }), so opening a second comment discards/aborts the first, and only one composer exists at a time.
The composer renders at the file end (<Show when={getDraft()}> block after the hunks <For>), not below the line being commented on — the reviewer has to scroll to see what they're writing about on long files.

Everything else stays as landed: stageLine → buildAnchor (lib/review-anchor.js) → shared CommentComposer → props.onStage into the finish-review modal; Shift+click range extension; #502 anonymous-viewer gate; drifted-thread placement.

Architecture notes

Draft state changes shape: from one getDraft signal per file to a keyed collection (e.g. a signal holding a Map/array of drafts keyed by hunk+row or anchor start), so multiple composers coexist. Row rendering inserts the composer inline in the row <For> immediately below the matching row, instead of one <Show> at file end.
Keep stageLine's anchor-building logic intact — buildAnchor inputs must stay byte-identical (anchorContextSha pinned vectors / DriftHash twin in internal/review/model.go are unaffected; this is presentation only). Shift+click range extension should keep working with the gutter buttons.
Row click must not fight the row's other interactive content (inline thread cards below the row, existing selection semantics): the click target is the diff row itself, and clicks on text inside an open composer or thread card must not spawn another composer.
The gutter "+" placement should mirror the Files-tab DiffTable gutter conventions (line-number gutter cells, gutterLink accessibility labels) so the two diff surfaces read consistently.
#502 gate stays: no triggers, no composers, no row-click handler for anonymous viewers (props.canComment !== false guards all three entry points).
Mobile: the hover-only problem #555 fixed must not come back — permanent gutter buttons are inherently touch-visible, but keep focus-visible affordances and ensure tapping a row doesn't require precise hover.

Acceptance criteria

"+" comment buttons render in the left gutter, one per commentable line, outside the diff row content, always visible (no hover dependency)
Clicking anywhere on a diff line opens a composer immediately below that line (not at file end)
Multiple composers can be open simultaneously in one file; opening a new one leaves existing drafts intact with their text preserved
Each composer submits independently: any subset can be staged into the finish-review modal in any order
Shift+click on the gutter buttons still builds range anchors as today
Clicks inside an open composer or thread card do not spawn another composer
#502 gate holds: anonymous viewers get no gutter buttons, no row-click behavior, no composers
buildAnchor inputs and anchorContextSha pinned vector tests stay byte-identical (presentation-only change)
Existing thread placement, resolve/unresolve, and drifted-anchor behavior unchanged

## What's requested Refine the PR inline review composer interaction (built for #546, extended by #555) on the conversation page's diff so commenting feels like per-line affordance rather than a single staged draft: 1. **Per-click "+" buttons outside the diff (left gutter).** Move the per-line comment trigger out of the diff row body into the left gutter — one discrete "+" button per commentable line, always rendered there, so the trigger is a stable target that doesn't shift row content or depend on hover. 2. **Click-anywhere-on-line opens the composer below that line.** Clicking anywhere on a diff line (the row itself, not just the button) instantiates a composer immediately below that line in the diff. 3. **Unlimited simultaneous composers per file.** A reviewer can have as many open draft composers as they want in one file's diff at the same time — opening one never closes or replaces another. ## Current state (static read of the current tree) `web/src/pages/Pull.jsx`, `DiffFile`: - The "+" button is rendered **inside** each diff row, after the line text (`<button ... class="ml-2 hidden shrink-0 px-1 text-emerald-600 group-hover:inline hover:text-emerald-700 focus-visible:inline pointer-coarse:inline ...">` in the row `<div class="group flex font-mono text-xs ...">`). It is hover/keyboard/coarse-pointer conditional, not a permanent gutter element. - Only the button opens the composer; clicking elsewhere on the row does nothing (row text is intentionally handler-free per the #555 note). - Draft state is a **single signal per file**: `const [getDraft, setDraft] = createSignal(null)` — `stageLine` overwrites `setDraft({ anchor })`, so opening a second comment discards/aborts the first, and only one composer exists at a time. - The composer renders at the **file end** (`<Show when={getDraft()}>` block after the hunks `<For>`), not below the line being commented on — the reviewer has to scroll to see what they're writing about on long files. Everything else stays as landed: `stageLine` → `buildAnchor` (lib/review-anchor.js) → shared `CommentComposer` → `props.onStage` into the finish-review modal; Shift+click range extension; `#502` anonymous-viewer gate; drifted-thread placement. ## Architecture notes - Draft state changes shape: from one `getDraft` signal per file to a keyed collection (e.g. a signal holding a Map/array of drafts keyed by hunk+row or anchor start), so multiple composers coexist. Row rendering inserts the composer inline in the row `<For>` immediately below the matching row, instead of one `<Show>` at file end. - Keep `stageLine`'s anchor-building logic intact — `buildAnchor` inputs must stay byte-identical (`anchorContextSha` pinned vectors / `DriftHash` twin in `internal/review/model.go` are unaffected; this is presentation only). Shift+click range extension should keep working with the gutter buttons. - Row click must not fight the row's other interactive content (inline thread cards below the row, existing selection semantics): the click target is the diff row itself, and clicks on text inside an open composer or thread card must not spawn another composer. - The gutter "+" placement should mirror the Files-tab `DiffTable` gutter conventions (line-number gutter cells, `gutterLink` accessibility labels) so the two diff surfaces read consistently. - `#502` gate stays: no triggers, no composers, no row-click handler for anonymous viewers (`props.canComment !== false` guards all three entry points). - Mobile: the hover-only problem #555 fixed must not come back — permanent gutter buttons are inherently touch-visible, but keep `focus-visible` affordances and ensure tapping a row doesn't require precise hover. ## Acceptance criteria - [ ] "+" comment buttons render in the left gutter, one per commentable line, outside the diff row content, always visible (no hover dependency) - [ ] Clicking anywhere on a diff line opens a composer immediately below that line (not at file end) - [ ] Multiple composers can be open simultaneously in one file; opening a new one leaves existing drafts intact with their text preserved - [ ] Each composer submits independently: any subset can be staged into the finish-review modal in any order - [ ] Shift+click on the gutter buttons still builds range anchors as today - [ ] Clicks inside an open composer or thread card do not spawn another composer - [ ] `#502` gate holds: anonymous viewers get no gutter buttons, no row-click behavior, no composers - [ ] `buildAnchor` inputs and `anchorContextSha` pinned vector tests stay byte-identical (presentation-only change) - [ ] Existing thread placement, resolve/unresolve, and drifted-anchor behavior unchanged

crueber added the

enhancement

pull requests

labels

2026-09-15 11:28:55 +00:00

crueber added this to the v1 milestone

2026-09-15 11:28:55 +00:00

crueber added the

issues

label

2026-09-15 11:30:04 +00:00

crueber referenced this issue from a commit

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

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

crueber referenced this issue from a pull request that will close it,

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

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

crueber referenced this issue

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

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

crueber closed this issue

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

crueber referenced this issue 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 commented

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

Author

Owner

Fixed by #562 (merged): gutter '+' triggers per line (always visible), click-anywhere-on-row opens composer below the line, keyed multi-draft Map for unlimited simultaneous composers with independent submit. Verified: 1322 unit tests green (smoke excluded, pre-existing live-server failure only), vite+esbuild green, go vet clean, independent review APPROVE (all 9 acceptance criteria hold, presentation-only).

crueber referenced this issue

2026-09-15 13:07:45 +00:00

Staged inline review comments vanish from the diff — only the staged count increments; they must render in-thread at their line #567

crueber referenced this issue

2026-09-15 21:56:57 +00:00

Diff gutter rework: sign-first rows, hover affordance (Fix #598) #606