Diff gutter: drop the hover-plus, lead with the +/- sign column, make row hover the affordance #598

New issue

Closed

opened 2026-09-15 21:16:21 +00:00 by crueber · 1 comment

crueber commented

2026-09-15 21:16:21 +00:00

Owner

Diff gutter: drop the hover-plus, lead with the +/- sign column, make row hover the affordance

What's requested

Rework the diff row anatomy so the interactivity affordance is the row itself, not a floating +:

Remove the left hover-plus — the + comment trigger that sits in the left gutter of diff rows goes away entirely.
Move the +/- sign column to the far left — the sign (+/-/context space) becomes the FIRST column of every diff row, ahead of the line numbers.
Row hover-highlight becomes the interactivity affordance — hovering a diff row highlights the whole row (the existing line-hl treatment: amber tint + emerald gutter edge, ui.css .diff-row.line-hl rules ~361-363), telling the user the row is clickable; clicking the row stages a line comment where commenting is gated on.

Evidence (current state, both diff renderers)

Conversation diff — web/src/pages/Pull.jsx DiffFile (~line 530-570): every row renders [gutter "+" span (w-6, always visible)] [oldNo] [newNo] [sign w-4] [text]. The plus column sits LEFT of the numbers and the sign column is buried 4th, between the numbers and the code — inverted from every mainstream diff UI (GitHub/GitLab put the sign first).
Files tab / commit diff — web/src/components/DiffTable.jsx lineTap() (~line 216-228): a hover-revealed + (group-hover:inline) is appended to the END of each code cell. It is the only hover-dependent affordance in the app's diff UI and is invisible until hover, easy to miss on both fine and coarse pointers.
Neither renderer currently shows the sign at the row start; DiffTable unified rows show no sign character at all (add/del state is color-only via lineClass(), DiffTable.jsx line 66), which reads poorly for color-blind users and in screenshots.

Architecture notes

The two renderers are separate by design (DiffTable = selection/hash model per #244/#546/#555; DiffFile = composer-per-row model per #560/#566/#567). The sign-column-first layout should be applied to BOTH so the Files tab, commit page, and conversation diff share one row anatomy.
DiffTable scope decision (implementer's call, note it): removing DiffTable's lineTap takes away its single-line staging path — DiffTable code cells deliberately carry no handlers (text selection must keep working) and single-line staging currently rides ONLY on the tap-plus (the range path rides on gutter-number drag + the "comment on selection" bar). Either keep lineTap's staging but relocate it into the new leftmost sign column (sign cell as the tap target), or add row-click staging mirroring DiffFile's onRowClick isolation check (closest("button, a, input, textarea, select, [data-no-row-comment]")). Pick one and note it — do not silently drop single-line commenting on the Files tab/commit diff.
The pinned hash twins (anchorContextSha in web/src/lib/diff.js ↔ DriftHash in internal/review/model.go) and the vectors in diff-review.test.js are untouched: this is pure presentation/anatomy — no anchor construction, no hash input bytes change.
DiffTable's drag/shift-click gutter-number selection (#244) stays as-is; the sign column is select-none and carries no drag handlers unless it becomes the tap target per the scope decision above.
ml-14 composer/thread indentation in Pull.jsx is derived from the current row layout (plus column + two number columns); re-derive the indent so composers still visually align under their row.
Standalone Tailwind utilities only; reuse the existing line-hl row-highlight tokens rather than new colors.

Acceptance criteria

No + button renders in the left gutter of any diff row (conversation diff, Files tab, commit page) in any state — hover, focus, or touch.
The +/-/context sign renders as the first cell of every diff row on all three surfaces, themed like the row (add green / del red / context muted).
Unified DiffTable rows now show a visible sign character (not color-only).
Hovering a diff row highlights the whole row with the existing line-hl treatment in both themes; the highlight is the affordance signaling the row is interactive.
Single-line comment staging still exists on ALL gated surfaces after the plus removal (path chosen and noted).
Range staging (drag / shift-click) still works on DiffTable gutters; row-click staging on the conversation diff still ignores clicks on buttons/links/inputs and inside open composers.
Inline composers and staged/thread cards still align under their anchor row after the re-layout.
Pinned diff-review hash vectors stay green (diff-review.test.js).
Render-verified before close: headless DOM assertions on the row structure (sign cell first, no gutter plus) and/or screenshots of Files tab, commit page, and conversation diff in both themes.

# Diff gutter: drop the hover-plus, lead with the +/- sign column, make row hover the affordance ## What's requested Rework the diff row anatomy so the interactivity affordance is the row itself, not a floating `+`: 1. **Remove the left hover-plus** — the `+` comment trigger that sits in the left gutter of diff rows goes away entirely. 2. **Move the +/- sign column to the far left** — the sign (`+`/`-`/context space) becomes the FIRST column of every diff row, ahead of the line numbers. 3. **Row hover-highlight becomes the interactivity affordance** — hovering a diff row highlights the whole row (the existing `line-hl` treatment: amber tint + emerald gutter edge, `ui.css` `.diff-row.line-hl` rules ~361-363), telling the user the row is clickable; clicking the row stages a line comment where commenting is gated on. ## Evidence (current state, both diff renderers) - **Conversation diff — `web/src/pages/Pull.jsx` DiffFile (~line 530-570):** every row renders `[gutter "+" span (w-6, always visible)] [oldNo] [newNo] [sign w-4] [text]`. The plus column sits LEFT of the numbers and the sign column is buried 4th, between the numbers and the code — inverted from every mainstream diff UI (GitHub/GitLab put the sign first). - **Files tab / commit diff — `web/src/components/DiffTable.jsx` `lineTap()` (~line 216-228):** a hover-revealed `+` (`group-hover:inline`) is appended to the END of each code cell. It is the only hover-dependent affordance in the app's diff UI and is invisible until hover, easy to miss on both fine and coarse pointers. - Neither renderer currently shows the sign at the row start; DiffTable unified rows show no sign character at all (add/del state is color-only via `lineClass()`, DiffTable.jsx line 66), which reads poorly for color-blind users and in screenshots. ## Architecture notes - The two renderers are separate by design (DiffTable = selection/hash model per #244/#546/#555; DiffFile = composer-per-row model per #560/#566/#567). The sign-column-first layout should be applied to BOTH so the Files tab, commit page, and conversation diff share one row anatomy. - **DiffTable scope decision (implementer's call, note it):** removing DiffTable's `lineTap` takes away its single-line staging path — DiffTable code cells deliberately carry no handlers (text selection must keep working) and single-line staging currently rides ONLY on the tap-plus (the range path rides on gutter-number drag + the "comment on selection" bar). Either keep `lineTap`'s staging but relocate it into the new leftmost sign column (sign cell as the tap target), or add row-click staging mirroring DiffFile's `onRowClick` isolation check (`closest("button, a, input, textarea, select, [data-no-row-comment]")`). Pick one and note it — do not silently drop single-line commenting on the Files tab/commit diff. - The pinned hash twins (`anchorContextSha` in `web/src/lib/diff.js` ↔ `DriftHash` in `internal/review/model.go`) and the vectors in `diff-review.test.js` are untouched: this is pure presentation/anatomy — no anchor construction, no hash input bytes change. - DiffTable's drag/shift-click gutter-number selection (#244) stays as-is; the sign column is `select-none` and carries no drag handlers unless it becomes the tap target per the scope decision above. - `ml-14` composer/thread indentation in Pull.jsx is derived from the current row layout (plus column + two number columns); re-derive the indent so composers still visually align under their row. - Standalone Tailwind utilities only; reuse the existing `line-hl` row-highlight tokens rather than new colors. ## Acceptance criteria - [ ] No `+` button renders in the left gutter of any diff row (conversation diff, Files tab, commit page) in any state — hover, focus, or touch. - [ ] The +/-/context sign renders as the first cell of every diff row on all three surfaces, themed like the row (add green / del red / context muted). - [ ] Unified DiffTable rows now show a visible sign character (not color-only). - [ ] Hovering a diff row highlights the whole row with the existing `line-hl` treatment in both themes; the highlight is the affordance signaling the row is interactive. - [ ] Single-line comment staging still exists on ALL gated surfaces after the plus removal (path chosen and noted). - [ ] Range staging (drag / shift-click) still works on DiffTable gutters; row-click staging on the conversation diff still ignores clicks on buttons/links/inputs and inside open composers. - [ ] Inline composers and staged/thread cards still align under their anchor row after the re-layout. - [ ] Pinned diff-review hash vectors stay green (`diff-review.test.js`). - [ ] Render-verified before close: headless DOM assertions on the row structure (sign cell first, no gutter plus) and/or screenshots of Files tab, commit page, and conversation diff in both themes.

crueber added the

enhancement

issues

labels

2026-09-15 21:16:31 +00:00

crueber added this to the v1 milestone

2026-09-15 21:16:31 +00:00

crueber added

bug

pull requests

and removed

enhancement

labels

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

crueber referenced this issue from a commit

2026-09-15 21:53:08 +00:00

Diff gutter rework: sign-first rows, hover affordance, no per-line plus (Fix #598)

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

2026-09-15 21:53:13 +00:00

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

crueber referenced this issue

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

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

crueber closed this issue

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

crueber referenced this issue from a commit

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

Merge pull request 'Diff gutter rework: sign-first rows, hover affordance (Fix #598)' (#606) from fix/issue-598 into main

crueber commented

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

Author

Owner

Fixed by #606 (merged): gutter plus removed everywhere; sign column first on all three surfaces (unified rows show visible signs); line-hl row hover is the affordance; DiffTable single-line staging relocates to the sign cell (row-click rejected to protect #555 text-selection); cards realigned ml-8; hashes byte-identical. Review fixed one defect pre-merge (cursor-pointer now gated on commentable rows). Verified: 1536 unit green (smoke excluded, pre-existing), vite/esbuild green, independent review APPROVE.