Commit-graph lane gutter breaks across rows (per-row margins/borders): margin-free rows + cell padding so the gutter connects

crueber commented

2026-09-14 12:15:29 +00:00

Owner

What's wrong

On the Commits tab with the graph toggle ON (shipped by #511 for #506), the lane gutter breaks visually between rows: the vertical lane lines stop and restart at every row boundary instead of running as one continuous line of development.

Root cause (static read, fresh tree — no local repro per standing rule)

Continuity of the gutter is an emergent property: each commit row draws its own rail segment (GraphRail in web/src/pages/Commits.jsx:60-130, an SVG absolutely filling only that row's box, top-half + bottom-half verticals meeting at GRAPH_ROW_MID), so the lanes only read as continuous if consecutive row boxes abut edge-to-edge — zero margins and zero inter-row borders across the graph column.

The current markup carries per-row spacers that insert visual gaps into the rail column:

web/src/pages/Commits.jsx:278 — the .commit-list card uses divide-y divide-zinc-100 dark:divide-zinc-800/60, i.e. a 1px top border on every row. In Forgejo/GitHub graph rendering the gutter column is border-free for exactly this reason.
Any future row-level margin (e.g. someone reaching for mb-* on .commit-row, web/src/pages/Commits.jsx:135) would do the same; the row currently uses py-2 padding, which is safe, and this must stay the convention.

Fix prescription

Rows stay margin-free; vertical spacing inside rows comes only from cell padding (py-* on .commit-row), never margins — so the rail column is a continuous strip.
While the graph is on, suppress the per-row divider in the rail column: the simplest correct shape is dropping divide-y when graph-on is set and restyling separation via row padding (or a border only on the columns right of the rail, e.g. box-shadow: inset Npx 0 0 / border on .commit-main, never on the row's left edge).
Whatever separation the non-graph list keeps between rows must not touch the first auto grid column (web/src/pages/Commits.jsx:135, .graph-on override at web/src/ui.css:368-372).
Same standard applies to the empty-row placeholder and any future rows added to .commit-list.

Acceptance criteria

With graph ON, a straight lane renders as one unbroken vertical through N consecutive rows (no 1px gaps at row boundaries, both light and dark themes).
With graph OFF, the commits list keeps its current per-row separation (divider look preserved).
.commit-row carries no margins anywhere; separation is padding/border only, and no border or margin lands in the rail column while graph-on.
Mobile (≤480px) fallback unchanged (rail hidden, 3-column row).

## What's wrong On the Commits tab with the graph toggle ON (shipped by #511 for #506), the lane gutter breaks visually between rows: the vertical lane lines stop and restart at every row boundary instead of running as one continuous line of development. ## Root cause (static read, fresh tree — no local repro per standing rule) Continuity of the gutter is an emergent property: each commit row draws its own rail segment (`GraphRail` in `web/src/pages/Commits.jsx:60-130`, an SVG absolutely filling only that row's box, top-half + bottom-half verticals meeting at `GRAPH_ROW_MID`), so the lanes only read as continuous if consecutive row boxes abut edge-to-edge — zero margins and zero inter-row borders across the graph column. The current markup carries per-row spacers that insert visual gaps into the rail column: 1. `web/src/pages/Commits.jsx:278` — the `.commit-list` card uses `divide-y divide-zinc-100 dark:divide-zinc-800/60`, i.e. a 1px top border on every row. In Forgejo/GitHub graph rendering the gutter column is border-free for exactly this reason. 2. Any future row-level margin (e.g. someone reaching for `mb-*` on `.commit-row`, `web/src/pages/Commits.jsx:135`) would do the same; the row currently uses `py-2` padding, which is safe, and this must stay the convention. ## Fix prescription - Rows stay **margin-free**; vertical spacing inside rows comes only from cell padding (`py-*` on `.commit-row`), never margins — so the rail column is a continuous strip. - While the graph is on, suppress the per-row divider in the rail column: the simplest correct shape is dropping `divide-y` when `graph-on` is set and restyling separation via row padding (or a border only on the columns *right of* the rail, e.g. `box-shadow: inset Npx 0 0` / border on `.commit-main`, never on the row's left edge). - Whatever separation the non-graph list keeps between rows must not touch the first `auto` grid column (`web/src/pages/Commits.jsx:135`, `.graph-on` override at `web/src/ui.css:368-372`). - Same standard applies to the empty-row placeholder and any future rows added to `.commit-list`. ## Acceptance criteria - [ ] With graph ON, a straight lane renders as one unbroken vertical through N consecutive rows (no 1px gaps at row boundaries, both light and dark themes). - [ ] With graph OFF, the commits list keeps its current per-row separation (divider look preserved). - [ ] `.commit-row` carries no margins anywhere; separation is padding/border only, and no border or margin lands in the rail column while `graph-on`. - [ ] Mobile (≤480px) fallback unchanged (rail hidden, 3-column row).

crueber added the

bug

label

2026-09-14 12:15:39 +00:00

crueber added this to the v1 milestone

2026-09-14 12:15:39 +00:00

crueber referenced this issue from a commit

2026-09-14 12:44:46 +00:00

Fix #512: graph gutter continuity (docs/go/12_web_ui.md decision)

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

2026-09-14 12:45:17 +00:00

Fix #512: graph gutter continuity #514

crueber commented

2026-09-14 12:45:34 +00:00

Author

Owner

Fix ready: #514 — margin-free rows, divide-y gated on graph-OFF, graph-on separation via .commit-main inset edge (never the rail column), mobile fallback untouched. Tests: 1150 total / 1149 pass / 1 pre-existing live-server smoke failure; vite+esbuild green.

Fix ready: https://git.packden.us/crueber/walhub/pulls/514 — margin-free rows, divide-y gated on graph-OFF, graph-on separation via .commit-main inset edge (never the rail column), mobile fallback untouched. Tests: 1150 total / 1149 pass / 1 pre-existing live-server smoke failure; vite+esbuild green.

crueber commented

2026-09-14 12:53:12 +00:00

Author

Owner

Review of PR #514 (fix/issue-512, 37c68e8) — verified in scratch worktree (removed afterward); main worktree untouched. No browser used (node tests + source/bundle reasoning only — note explicitly: browser proof still open, same as stated in the PR).

FINDINGS (all 4 acceptance criteria hold):

Dividers off under graph-on: web/src/pages/Commits.jsx — divide utilities moved from static class into classList gated on !graphOn() ("divide-y": !graphOn() etc.), "graph-on": graphOn() marker kept. Graph OFF recomputes to the same three utilities (class-attribute order differs, computed style identical) — divider look preserved.
Rail column untouched: web/src/ui.css — .graph-on .commit-row still sets only grid-template-columns; no border/margin/shadow on the row box or .commit-rail; separation is box-shadow: inset 0 1px on .commit-main only (verified .commit-main exists at Commits.jsx:142, second grid column right of the rail in the graph-on template). Colors match both themes: #f4f4f5 = zinc-100, rgb(39 39 42 / .6) = zinc-800/60. Selector .graph-on > .commit-row + .commit-row .commit-main correctly skips the first row (parity with divide-y semantics).
Rows margin-free: .commit-row keeps py-2, no m-/space- utilities; gap-x-2.5/px-3 are horizontal-only; ui.css pins .commit-row, .commit-empty { margin: 0 } against future reaches.
Empty placeholder same standard: new .commit-empty class on the No-commits
(Commits.jsx), covered by the margin guard.
Mobile fallback untouched: @480px block byte-identical (rail display:none, 3-column row). Note: graph-on mobile keeps the .commit-main inset edge for separation — sane, rail column still clean.
#506 intact: GraphRail/lanes/toggle untouched (one comment-only mention); commit-graph.js untouched; all 14 commit-graph-506 tests pass.
No backend change (4 files: Commits.jsx, ui.css, gutter test, 12_web_ui.md decision entry per law 12); package.json/pnpm-lock untouched, no new deps (law 1). No new routes/state — laws 7/8 unaffected.

TEST RESULTS (scratch worktree, node_modules symlinked from main):

commit-graph-gutter-512.test.js: 5/5 pass; commit-graph-506.test.js: 14/14 pass.
Full suite minus smoke: 1147/1147 pass.
smoke.test.js: fails (403 vs 200 on :8080) — file byte-identical to main, needs a live server serving the built SPA; something answers on :8080 but is not serving the app (left untouched per rules). Pre-existing/environmental, not PR-caused.
vite build green; compiled CSS contains .commit-row,.commit-empty{margin:0} + both light/dark inset-edge rules; esbuild SDK bundle green with zero graph leakage (UI-only change confirmed).

No fixes needed — nothing pushed.

MERGE RECOMMENDATION: ready to merge (browser proof remains open as declared in the PR; no blocking issue found).

Review of PR #514 (fix/issue-512, 37c68e8) — verified in scratch worktree (removed afterward); main worktree untouched. No browser used (node tests + source/bundle reasoning only — note explicitly: browser proof still open, same as stated in the PR). FINDINGS (all 4 acceptance criteria hold): 1. Dividers off under graph-on: web/src/pages/Commits.jsx — divide utilities moved from static class into classList gated on `!graphOn()` (`"divide-y": !graphOn()` etc.), `"graph-on": graphOn()` marker kept. Graph OFF recomputes to the same three utilities (class-attribute order differs, computed style identical) — divider look preserved. 2. Rail column untouched: web/src/ui.css — `.graph-on .commit-row` still sets only grid-template-columns; no border/margin/shadow on the row box or `.commit-rail`; separation is `box-shadow: inset 0 1px` on `.commit-main` only (verified .commit-main exists at Commits.jsx:142, second grid column right of the rail in the graph-on template). Colors match both themes: #f4f4f5 = zinc-100, rgb(39 39 42 / .6) = zinc-800/60. Selector `.graph-on > .commit-row + .commit-row .commit-main` correctly skips the first row (parity with divide-y semantics). 3. Rows margin-free: .commit-row keeps py-2, no m-*/space-* utilities; `gap-x-2.5`/px-3 are horizontal-only; ui.css pins `.commit-row, .commit-empty { margin: 0 }` against future reaches. 4. Empty placeholder same standard: new .commit-empty class on the No-commits <p> (Commits.jsx), covered by the margin guard. 5. Mobile fallback untouched: @480px block byte-identical (rail display:none, 3-column row). Note: graph-on mobile keeps the .commit-main inset edge for separation — sane, rail column still clean. 6. #506 intact: GraphRail/lanes/toggle untouched (one comment-only mention); commit-graph.js untouched; all 14 commit-graph-506 tests pass. 7. No backend change (4 files: Commits.jsx, ui.css, gutter test, 12_web_ui.md decision entry per law 12); package.json/pnpm-lock untouched, no new deps (law 1). No new routes/state — laws 7/8 unaffected. TEST RESULTS (scratch worktree, node_modules symlinked from main): - commit-graph-gutter-512.test.js: 5/5 pass; commit-graph-506.test.js: 14/14 pass. - Full suite minus smoke: 1147/1147 pass. - smoke.test.js: fails (403 vs 200 on :8080) — file byte-identical to main, needs a live server serving the built SPA; something answers on :8080 but is not serving the app (left untouched per rules). Pre-existing/environmental, not PR-caused. - vite build green; compiled CSS contains `.commit-row,.commit-empty{margin:0}` + both light/dark inset-edge rules; esbuild SDK bundle green with zero graph leakage (UI-only change confirmed). No fixes needed — nothing pushed. MERGE RECOMMENDATION: ready to merge (browser proof remains open as declared in the PR; no blocking issue found).

crueber closed this issue

2026-09-14 12:53:30 +00:00

crueber commented

2026-09-14 12:53:30 +00:00

Author

Owner

Fixed by PR #514 (review clean — all 4 criteria hold, gutter rules verified in compiled CSS), merged. Closing.

crueber referenced this issue from a commit

2026-09-14 12:53:31 +00:00

Merge pull request 'Fix #512: graph gutter continuity' (#514) from fix/issue-512 into main

crueber commented

2026-09-14 13:58:40 +00:00

Author

Owner

Reopening: still reproduces. The divide-gating reads correct in source; going to verify with a real rendered screenshot locally (scratch server + headless Chromium) instead of reasoning about it again.

crueber reopened this issue

2026-09-14 13:58:40 +00:00

crueber commented

2026-09-14 14:27:16 +00:00

Author

Owner

Reopening with screenshot evidence: straight lane segments visibly stop/restart at row joints (merge node to next-row dot has a dark gap; same at the lower joint). The divide-gating reads correct in source, so the gap source is elsewhere — going to reproduce rendered (scratch server + headless Chromium) and fix what I see.

crueber commented

2026-09-14 14:59:08 +00:00

Author

Owner

Verification update: production serves the fixed bundle (Bibhi10Z contains the divide-gating), and the emitted lane geometry for the exact screenshotted history is provably continuous (executed check on the real assignLanes/rowDiagonals/laneX: all joints match). Boxes/margins/borders verified abutting; classList reactivity verified in the solid runtime source. Remaining suspect is a stale tab bundle (SPA tabs never reload JS by themselves; any tab loaded before today's 12:53 deploy renders the old divide-always code) — or a render subtlety needing eyes. Next step: hard-refresh result.

crueber referenced this issue from a commit

2026-09-14 15:45:05 +00:00

Fix #512 (reopened): rail fills row box — move py-2 to content columns

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

2026-09-14 15:45:45 +00:00

Fix #512 (reopened): rail fills row box #519

crueber commented

2026-09-14 15:46:01 +00:00

Author

Owner

Follow-up fix PR: #519 (measured root cause + visual before/after).

Follow-up fix PR: https://git.packden.us/crueber/walhub/pulls/519 (measured root cause + visual before/after).

crueber referenced this issue from a commit

2026-09-14 16:06:11 +00:00

Review #519: fix wrong issue number in comments (#513 -> reopened #512)

crueber commented

2026-09-14 16:06:24 +00:00

Author

Owner

Review of PR #519 (fix/issue-512-rail, d185e31 + review fixup 0f44ec4) — verified in scratch worktree /tmp/pr519 (removed afterward); main worktree untouched and clean. No browser used by this reviewer: the fix was already verified with before/after screenshots by the author (rail 56px→72px, gutter continuous) — this review is node tests + source/bundle reasoning only. Browser open for myself explicitly: no private daemon per workspace rules, shared-daemon loopback guard.

FINDINGS (all 8 checks hold):

py-2 placement correct: web/src/pages/Commits.jsx — row (line 143) carries no vertical padding (commit-row grid grid-cols-[minmax(0,1fr)_auto_auto] items-center gap-x-2.5 px-3, px/gap-x horizontal-only); all three content columns carry their own py-2 (.commit-main L147, .commit-sha-col L167, .commit-check L177); rail (L67 commit-rail relative self-stretch shrink-0) unpadded so it fills the full box.
Total spacing preserved: the 16px vertical moved row→columns, so row height (tallest column, items-center) is unchanged vs base; graph-off heights identical too.
Graph-off dividers intact: classList divide-gating (L296-301) untouched — OFF recomputes the same three utilities.
Inset edge still on .commit-main (web/src/ui.css L385-386, both themes, selector skips first row): position sane — with no row padding it now sits exactly at the row joint, matching divide-y boundary semantics; nothing in the rail column.
Mobile fallback intact: @480px block (L387-390) unchanged — rail hidden, 3-column row; column py-2 keeps mobile row heights.
Test updates faithful (strictly stronger, not weakened): gutter test now asserts NO row vertical padding (py-/pt-/pb- token scan) + py-2 on each of the three columns + keeps the margin-free pins (row utilities + ui.css margin:0 guard). Targeted: gutter 5/5 + graph-506 14/14 = 19/19 pass.
Empty placeholder unaffected: .commit-empty (L317) untouched, still under the margin guard.
No backend change (4 files: Commits.jsx, ui.css, gutter test, 12_web_ui.md decision entry per law 12); package.json/pnpm-lock untouched, no new deps (law 1). Docs entry correctly references reopened #512.

ONE SMALL FIX PUSHED DIRECTLY (0f44ec4, comment-only): four code/test comments cited "#513 follow-up" / "hardened #513" — but #513 is the checks-tab/ETag issue. Renamed to "reopened #512" in Commits.jsx (2x), ui.css, gutter test (2x). Re-tested after the fixup.

TEST RESULTS (scratch worktree, node_modules symlinked from main):

Full: node --test web/test/unit/*.test.js → 1154 total / 1153 pass / 1 fail; the 1 failure is smoke.test.js (403 vs 200 on :8080, needs a live Go server) — file untouched by this PR, environmental, same as the PR description's 1153/1154 claim.
vite build green (637kB bundle, only the pre-existing chunk-size warning).
Note: vite build deletes tracked web/dist/.keep (emptyOutDir) — restored before committing; the fixup commit contains only the 3 intended files.

MERGE RECOMMENDATION: ready to merge.

Review of PR #519 (fix/issue-512-rail, d185e31 + review fixup 0f44ec4) — verified in scratch worktree /tmp/pr519 (removed afterward); main worktree untouched and clean. No browser used by this reviewer: the fix was already verified with before/after screenshots by the author (rail 56px→72px, gutter continuous) — this review is node tests + source/bundle reasoning only. Browser open for myself explicitly: no private daemon per workspace rules, shared-daemon loopback guard. FINDINGS (all 8 checks hold): 1. py-2 placement correct: web/src/pages/Commits.jsx — row (line 143) carries no vertical padding (`commit-row grid grid-cols-[minmax(0,1fr)_auto_auto] items-center gap-x-2.5 px-3`, px/gap-x horizontal-only); all three content columns carry their own py-2 (`.commit-main` L147, `.commit-sha-col` L167, `.commit-check` L177); rail (L67 `commit-rail relative self-stretch shrink-0`) unpadded so it fills the full box. 2. Total spacing preserved: the 16px vertical moved row→columns, so row height (tallest column, items-center) is unchanged vs base; graph-off heights identical too. 3. Graph-off dividers intact: classList divide-gating (L296-301) untouched — OFF recomputes the same three utilities. 4. Inset edge still on .commit-main (web/src/ui.css L385-386, both themes, selector skips first row): position sane — with no row padding it now sits exactly at the row joint, matching divide-y boundary semantics; nothing in the rail column. 5. Mobile fallback intact: @480px block (L387-390) unchanged — rail hidden, 3-column row; column py-2 keeps mobile row heights. 6. Test updates faithful (strictly stronger, not weakened): gutter test now asserts NO row vertical padding (py-/pt-/pb- token scan) + py-2 on each of the three columns + keeps the margin-free pins (row utilities + ui.css margin:0 guard). Targeted: gutter 5/5 + graph-506 14/14 = 19/19 pass. 7. Empty placeholder unaffected: `.commit-empty` (L317) untouched, still under the margin guard. 8. No backend change (4 files: Commits.jsx, ui.css, gutter test, 12_web_ui.md decision entry per law 12); package.json/pnpm-lock untouched, no new deps (law 1). Docs entry correctly references reopened #512. ONE SMALL FIX PUSHED DIRECTLY (0f44ec4, comment-only): four code/test comments cited "#513 follow-up" / "hardened #513" — but #513 is the checks-tab/ETag issue. Renamed to "reopened #512" in Commits.jsx (2x), ui.css, gutter test (2x). Re-tested after the fixup. TEST RESULTS (scratch worktree, node_modules symlinked from main): - Full: node --test web/test/unit/*.test.js → 1154 total / 1153 pass / 1 fail; the 1 failure is smoke.test.js (403 vs 200 on :8080, needs a live Go server) — file untouched by this PR, environmental, same as the PR description's 1153/1154 claim. - vite build green (637kB bundle, only the pre-existing chunk-size warning). - Note: vite build deletes tracked web/dist/.keep (emptyOutDir) — restored before committing; the fixup commit contains only the 3 intended files. MERGE RECOMMENDATION: ready to merge.

crueber referenced this issue from a commit

2026-09-14 16:07:38 +00:00

Merge pull request 'Fix #512 (reopened): rail fills row box' (#519) from fix/issue-512-rail into main

crueber commented

2026-09-14 16:07:39 +00:00

Author

Owner

Fixed by PR #519 (review clean; rail now fills the row box, verified with before/after screenshots — rail 56px→72px, gutter continuous), merged. Closing.

crueber closed this issue

2026-09-14 16:07:39 +00:00

crueber referenced this issue

2026-09-14 21:07:55 +00:00

Write docs/style-guideline.md: codify the UI design language with canonical references + AGENTS.md amendment making it mandatory for all UI work #537

Rows
Columns

Commit-graph lane gutter breaks across rows (per-row margins/borders): margin-free rows + cell padding so the gutter connects #512

What's wrong

Root cause (static read, fresh tree — no local repro per standing rule)

Fix prescription

Acceptance criteria