#463 - Repo header action strip: uneven gaps between Star/Watch/Fork/Clone pills — one spacing mechanism for the row - crueber/walhub

crueber commented

2026-09-13 15:03:35 +00:00

Owner

What's requested

The Star/Watch/Fork/Clone action strip in the repo header renders with uneven gaps between its pills. One spacing mechanism for the whole row should own the gaps; no element in the row should add invisible layout.

Evidence (static read of `web/src/pages/Repo.jsx` on main, `06c9743`)

The row is Repo.jsx:615: <div class="ml-auto flex items-center gap-2"> — gap-2 is the intended single spacing mechanism.
Repo.jsx:618 places <TasksOverlay repo={repoClient} /> between WatchToggle and the summary-gated Fork/Clone pair. TasksOverlay's returned tree is an unconditional wrapper <div class="tasks-indicator relative" ref={root}> (Repo.jsx ~line 434) whose inner <Show when={getRunning().length || getDone().length}> guards only the pill button.
With no tasks running (the common case) that wrapper is an empty 0-width flex item: flex gap applies on both sides of it, so the Star-to-Watch gap renders as 2× gap-2 (16px) while Watch-to-Fork and Fork-to-Clone get the normal 8px. That is the uneven gap.
Secondary composition wart: Star/Watch are <button class="btn px-2 py-1 text-sm">, Fork is <A class="btn px-2 py-1 text-sm"> (636), Clone is <summary class="btn ..."> inside <details class="clone-menu relative"> (99) — the <details> element (block, .clone-menu { position: relative } in web/css/repo.css:90) is the flex item, which matches metrics today but means nothing in the row itself enforces consistent spacing if any one child is absent or wrapped (exactly the TasksOverlay failure).

Architecture notes

The #447 comment block (Repo.jsx:622-635) already declares the ONE idiom for pill shape/metrics; it is silent on the row's spacing mechanism. This ticket extends that contract to spacing.
Minimal fix, and the preferred one: make TasksOverlay render nothing when idle — move the outer <Show> above the wrapper div (or return null / wrap at the call site) so the flex row contains only real pills; gap-2 on the row then owns all gaps by itself. The popover positioning (relative wrapper, absolute right-0 drop) must keep working when tasks ARE running — the wrapper only exists when the pill exists.
Alternative if a persistent anchor is ever wanted: keep the wrapper but take it out of flow participation when idle (hidden / display: contents when the inner Show is false). Either way, the rule is: no idle element contributes a flex item to this row.
Acceptance nuance: the row also changes membership reactively (Star/Watch appear after their fetches resolve, Fork/Clone after the summary lands) — with the wrapper gone, every present/absent transition just re-runs the same gap-2 distribution, so no per-pair spacing rules are needed or allowed.

Acceptance criteria

The Star/Watch/Fork/Clone strip uses a single spacing mechanism (the row's gap); no child element adds, removes, or doubles spacing.
TasksOverlay contributes no flex item (zero layout footprint) when no tasks are running, while its popover still opens/positions correctly when tasks run.
Spacing between every adjacent visible pill is identical at all of: initial load pre-summary, loaded, task running, task finished.
Narrow widths: the strip still behaves with the #274 tab-row treatment (no page-level push) and gains no new overflow from the change.

## What's requested The Star/Watch/Fork/Clone action strip in the repo header renders with uneven gaps between its pills. One spacing mechanism for the whole row should own the gaps; no element in the row should add invisible layout. ## Evidence (static read of `web/src/pages/Repo.jsx` on main, 06c9743) - The row is `Repo.jsx:615`: `<div class="ml-auto flex items-center gap-2">` — `gap-2` is the intended single spacing mechanism. - `Repo.jsx:618` places `<TasksOverlay repo={repoClient} />` between WatchToggle and the summary-gated Fork/Clone pair. TasksOverlay's returned tree is an **unconditional** wrapper `<div class="tasks-indicator relative" ref={root}>` (`Repo.jsx` ~line 434) whose inner `<Show when={getRunning().length || getDone().length}>` guards only the pill button. - With no tasks running (the common case) that wrapper is an **empty 0-width flex item**: flex `gap` applies on both sides of it, so the Star-to-Watch gap renders as 2× gap-2 (16px) while Watch-to-Fork and Fork-to-Clone get the normal 8px. That is the uneven gap. - Secondary composition wart: Star/Watch are `<button class="btn px-2 py-1 text-sm">`, Fork is `<A class="btn px-2 py-1 text-sm">` (636), Clone is `<summary class="btn ...">` inside `<details class="clone-menu relative">` (99) — the `<details>` element (block, `.clone-menu { position: relative }` in `web/css/repo.css:90`) is the flex item, which matches metrics today but means nothing in the row itself enforces consistent spacing if any one child is absent or wrapped (exactly the TasksOverlay failure). ## Architecture notes - The #447 comment block (Repo.jsx:622-635) already declares the ONE idiom for pill shape/metrics; it is silent on the row's spacing mechanism. This ticket extends that contract to spacing. - Minimal fix, and the preferred one: make TasksOverlay render nothing when idle — move the outer `<Show>` above the wrapper `div` (or `return null` / wrap at the call site) so the flex row contains only real pills; `gap-2` on the row then owns all gaps by itself. The popover positioning (`relative` wrapper, `absolute right-0` drop) must keep working when tasks ARE running — the wrapper only exists when the pill exists. - Alternative if a persistent anchor is ever wanted: keep the wrapper but take it out of flow participation when idle (`hidden` / `display: contents` when the inner Show is false). Either way, the rule is: no idle element contributes a flex item to this row. - Acceptance nuance: the row also changes membership reactively (Star/Watch appear after their fetches resolve, Fork/Clone after the summary lands) — with the wrapper gone, every present/absent transition just re-runs the same `gap-2` distribution, so no per-pair spacing rules are needed or allowed. ## Acceptance criteria - [ ] The Star/Watch/Fork/Clone strip uses a single spacing mechanism (the row's `gap`); no child element adds, removes, or doubles spacing. - [ ] TasksOverlay contributes no flex item (zero layout footprint) when no tasks are running, while its popover still opens/positions correctly when tasks run. - [ ] Spacing between every adjacent visible pill is identical at all of: initial load pre-summary, loaded, task running, task finished. - [ ] Narrow widths: the strip still behaves with the #274 tab-row treatment (no page-level push) and gains no new overflow from the change.

crueber added this to the v1 milestone

2026-09-13 15:03:35 +00:00

crueber added the

issues

label

2026-09-13 15:03:35 +00:00

crueber referenced this issue

2026-09-13 15:27:24 +00:00

Shared SVG icon mechanism + embed the provided icon set across Watch/Star/Fork/Clone/bell/theme controls #465

crueber referenced this issue from a commit

2026-09-13 17:58:15 +00:00

Fix #463: TasksOverlay renders nothing when idle, gap-2 owns row spacing

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

2026-09-13 17:58:16 +00:00

Fix #463: pill spacing #473

crueber commented

2026-09-13 17:58:20 +00:00

Author

Owner

Fixed by #473 — TasksOverlay renders nothing when idle (guard Show above the wrapper div), gap-2 owns all spacing, #447 contract extended to spacing. New headless test header-gap-463.test.js (5/5 green); targeted suites 55/55; full web 980/982 (2 smoke failures pre-existing on baseline); vite build clean.

Fixed by https://git.packden.us/crueber/walhub/pulls/473 — TasksOverlay renders nothing when idle (guard Show above the wrapper div), gap-2 owns all spacing, #447 contract extended to spacing. New headless test header-gap-463.test.js (5/5 green); targeted suites 55/55; full web 980/982 (2 smoke failures pre-existing on baseline); vite build clean.

crueber commented

2026-09-13 18:00:29 +00:00

Author

Owner

Review of PR #473 (fix/issue-463, faae6b7) — verified in a scratch worktree (created /tmp/pr473, removed afterward with --force; main worktree left clean, no checkouts/edits there; no live instance, docker, or volume touched; no browser — node tests + source reasoning, as instructed).

Scope matches expectations exactly: web/src/pages/Repo.jsx (TasksOverlay Show hoist + #447 comment extension) + new web/test/unit/header-gap-463.test.js. No backend/Go change, no new deps (package.json/go.mod untouched), no docs/infra change.

Findings by checklist (all pass, no fix-ups needed):

Idle renders nothing — PASS. web/src/pages/Repo.jsx:433-438: the <Show when={getRunning().length || getDone().length}> now opens the return before any element; the tasks-indicator relative wrapper (now :447) exists only inside the guarded branch. No
precedes the guard, and the branch uses absence (not hidden/display:contents) — zero flex items when idle.
Active states intact — PASS. Pill button (pill cursor-pointer, amber-busy/red-failed borders, pulse vs static dot, kind label, +n others, pct), the tasks-drop card absolute right-0 w-96 popover with running+done rows, outside-click dismiss (onDoc), LINGER_MS linger and BUSY_MS/IDLE_MS cadence all byte-identical inside the moved branch; relative wrapper still anchors the popover. Only delta is the dropped dead (list) render-prop param, which was never referenced.
gap-2 sole spacing owner — PASS. Row stays ml-auto flex items-center gap-2 (:618); no space-x/ml-/mr-/mx-/ms-/me- on any row child (test asserts); the summary-gated Fork/Clone ride Solid /<></> which contribute no DOM element.
Sibling audit — PASS. Star/Watch return
#447 contract extension accurate — PASS. The appended paragraph describes exactly the diff (guard placement, anchor preservation, sibling audit) — no over/under-claim.
Tasks behavior unchanged — PASS. Diff is confined to the return tree; effects, polling, linger, dismiss untouched.
Laws 1/7/8/12 — PASS. No new deps; no silent-waiting change; no new capability/seam; the in-code #447 contract comment + test record the decision in the same change (matches #447 precedent for this surface).

Tests (scratch worktree, node_modules symlinked from main): header-gap-463 + header-pills-447 + header-narrow = 19/19 pass; full suite minus server-smoke = 979/979 pass; vite build succeeds in ~2s (only the pre-existing >500kB chunk-size warning). The 2 smoke.test.js failures are environmental, not the PR: something already listens on 127.0.0.1:8080 returning 401 for / (the healthz-ok probe un-skips the tests, then / asserts 200) — a pure-frontend diff cannot cause that, and I left the live process alone per instructions.

MERGE RECOMMENDATION: ready to merge (not merging per instructions).

Review of PR #473 (fix/issue-463, faae6b7) — verified in a scratch worktree (created /tmp/pr473, removed afterward with --force; main worktree left clean, no checkouts/edits there; no live instance, docker, or volume touched; no browser — node tests + source reasoning, as instructed). Scope matches expectations exactly: web/src/pages/Repo.jsx (TasksOverlay Show hoist + #447 comment extension) + new web/test/unit/header-gap-463.test.js. No backend/Go change, no new deps (package.json/go.mod untouched), no docs/infra change. Findings by checklist (all pass, no fix-ups needed): 1. Idle renders nothing — PASS. web/src/pages/Repo.jsx:433-438: the <Show when={getRunning().length || getDone().length}> now opens the return before any element; the tasks-indicator relative wrapper (now :447) exists only inside the guarded branch. No <div> precedes the guard, and the branch uses absence (not hidden/display:contents) — zero flex items when idle. 2. Active states intact — PASS. Pill button (pill cursor-pointer, amber-busy/red-failed borders, pulse vs static dot, kind label, +n others, pct), the tasks-drop card absolute right-0 w-96 popover with running+done rows, outside-click dismiss (onDoc), LINGER_MS linger and BUSY_MS/IDLE_MS cadence all byte-identical inside the moved branch; relative wrapper still anchors the popover. Only delta is the dropped dead (list) render-prop param, which was never referenced. 3. gap-2 sole spacing owner — PASS. Row stays ml-auto flex items-center gap-2 (:618); no space-x/ml-/mr-/mx-/ms-/me- on any row child (test asserts); the summary-gated Fork/Clone ride Solid <Show>/<></> which contribute no DOM element. 4. Sibling audit — PASS. Star/Watch return <button> straight out of <Show> (no wrapper div); Fork is a bare <A>; Clone's <details class="clone-menu relative"> IS the pill with nothing wrapping it; Clone details-metrics unchanged. 5. #447 contract extension accurate — PASS. The appended paragraph describes exactly the diff (guard placement, anchor preservation, sibling audit) — no over/under-claim. 6. Tasks behavior unchanged — PASS. Diff is confined to the return tree; effects, polling, linger, dismiss untouched. 7. Laws 1/7/8/12 — PASS. No new deps; no silent-waiting change; no new capability/seam; the in-code #447 contract comment + test record the decision in the same change (matches #447 precedent for this surface). Tests (scratch worktree, node_modules symlinked from main): header-gap-463 + header-pills-447 + header-narrow = 19/19 pass; full suite minus server-smoke = 979/979 pass; vite build succeeds in ~2s (only the pre-existing >500kB chunk-size warning). The 2 smoke.test.js failures are environmental, not the PR: something already listens on 127.0.0.1:8080 returning 401 for / (the healthz-ok probe un-skips the tests, then / asserts 200) — a pure-frontend diff cannot cause that, and I left the live process alone per instructions. MERGE RECOMMENDATION: ready to merge (not merging per instructions).

crueber closed this issue

2026-09-13 18:00:42 +00:00

crueber referenced this issue from a commit

2026-09-13 18:00:42 +00:00

Merge pull request 'Fix #463: pill spacing' (#473) from fix/issue-463 into main

crueber commented

2026-09-13 18:00:43 +00:00

Author

Owner

Fixed by PR #473 (review clean — all 7 checks pass, active states byte-identical), merged. Closing.

crueber referenced this issue

2026-09-13 18:10:27 +00:00