Fix #574: inset the flashed-card ring so the hunk scroll wrapper never clips it #578

Merged
crueber merged 1 commit from fix/issue-574 into main 2026-09-15 14:57:45 +00:00
Owner

Closes #574.

The ThreadIndex jump flash on ThreadCard + StagedCard used outline outline-2 outline-emerald-500 at the default 0 offset (paints 2px OUTSIDE the border box) while every hunk renders inside overflow-x-auto — the ring clipped at the wrapper edge / added scroll extent on long-line hunks.

Both flash paths now compose outline-offset-[-2px] (Tailwind-only, byte-identical shared fragment): ring paints inside the border box hugging the rounded corners. Composer slot consistent (no divergent treatment); a11y :focus-visible untouched.

Verify: new web/test/unit/flash-ring-574.test.js (11 tests); related 573/567/554 green (51/51); full-minus-smoke 1417/1417; vite build green (bundle emits outline-offset:-2px); esbuild SDK ok; go vet clean. Rendered screenshot proof open per shared-daemon guard.

Closes #574. The ThreadIndex jump flash on ThreadCard + StagedCard used `outline outline-2 outline-emerald-500` at the default 0 offset (paints 2px OUTSIDE the border box) while every hunk renders inside `overflow-x-auto` — the ring clipped at the wrapper edge / added scroll extent on long-line hunks. Both flash paths now compose `outline-offset-[-2px]` (Tailwind-only, byte-identical shared fragment): ring paints inside the border box hugging the rounded corners. Composer slot consistent (no divergent treatment); a11y :focus-visible untouched. Verify: new web/test/unit/flash-ring-574.test.js (11 tests); related 573/567/554 green (51/51); full-minus-smoke 1417/1417; vite build green (bundle emits outline-offset:-2px); esbuild SDK ok; go vet clean. Rendered screenshot proof open per shared-daemon guard.
ThreadCard + StagedCard flash was outline-2 at 0 offset (paints outside the
border box); hunks render inside overflow-x-auto, so the ring clipped at the
wrapper edge. Both flash paths compose outline-offset-[-2px] (Tailwind-only,
byte-identical shared fragment). Focus-visible precedent untouched.
Author
Owner

Independent review — APPROVED (no code changes needed; no fix commit).

Verified against #574 acceptance, origin/main..origin/fix/issue-574 (1aaf082, 3 files):

  1. Ring geometry: both flash paths now compose the byte-identical fragment ' outline outline-2 outline-emerald-500 outline-offset-[-2px]' (Pull.jsx:696 StagedCard, :764 ThreadCard). outline 2px wide at -2px offset paints fully inside the border box, hugging rounded per CSS spec — cannot clip at the mb-3 overflow-x-auto hunk-wrapper edge (:524, unchanged) and cannot add scroll extent (outline is paint-only). Emerald-500 both themes, the #567 idiom.
  2. Tailwind compiles: fresh 'vite build' in the worktree emits '.outline-offset-[-2px]{outline-offset:-2px}' plus the unchanged 'outline-offset:1px' (focus-visible) in dist/assets CSS. Rendered check in headless Chromium (file repro, real bundle CSS): computed outlineOffset -2px / width 2px / solid / emerald on the card; wrapper scrollWidth identical (1200) with and without the fix; scrolled-right screenshot shows the inset ring fully visible at the card edge. (Caveat: this old headless build does not resolve rem in border-radius, so rounded corners do not render there — renderer limitation, pre-existing, unrelated; radius comes from the untouched 'rounded' class.)
  3. Shared treatment: flash fragments byte-identical across ThreadCard/StagedCard (pinned by test); draft composer card (:583) keeps exact slot classes 'ml-14 mt-1 rounded border border-zinc-200 p-2 dark:border-zinc-700' with no outline/ring of its own — no divergent treatment.
  4. No new scroll containers: only overflow sites are the pre-existing ref-list dropdown (:261) and hunk wrapper (:524); cards add none (pinned).
  5. focus-visible unchanged: ui.css:18-21 rule byte-identical (2px + 1px outside offset) and gutter-trigger 'focus-visible:outline...' classes (:547) byte-identical; ui.css not in the diff at all.
  6. Law 12: docs/go/12_web_ui.md carries the #574 decision entry in the same change.
  7. Law 1: no package.json change; test pins runtime deps exactly to solid-js + @solidjs/router + marked + dompurify; no new ui.css rule (Tailwind-only composition, no guideline extension needed).
  8. Tests fail pre-fix: new flash-ring-574.test.js 11/11 green on the branch; against origin/main Pull.jsx 3 fail (both inset-geometry pins + the JSX-wide no-bare-outside-outline sweep) — the bug reproduces, the fix resolves it. Full-minus-smoke 1417/1417 green (matches PR claim); related 573/567/554 pins green (35/35). The single smoke.test.js failure is environmental (a foreign server answers :8080 /healthz but 403s /setup; smoke requires a live app server) — same exclusion the PR description uses. vite build green; restored dist/.keep after the build (build deletes it; 'go:embed all:dist' needs it).
  9. Browser proof remains reasoned-only (shared-daemon loopback guard), consistent with #571/#572/#573 — acceptable here: single paint-only utility, zero layout delta pinned by test, plus the rendered geometry check above.

Verdict: APPROVED — meets all four acceptance criteria with the review checklist above. No follow-up commit (worktree clean apart from node_modules).

Independent review — APPROVED (no code changes needed; no fix commit). Verified against #574 acceptance, origin/main..origin/fix/issue-574 (1aaf082, 3 files): 1. Ring geometry: both flash paths now compose the byte-identical fragment ' outline outline-2 outline-emerald-500 outline-offset-[-2px]' (Pull.jsx:696 StagedCard, :764 ThreadCard). outline 2px wide at -2px offset paints fully inside the border box, hugging rounded per CSS spec — cannot clip at the mb-3 overflow-x-auto hunk-wrapper edge (:524, unchanged) and cannot add scroll extent (outline is paint-only). Emerald-500 both themes, the #567 idiom. 2. Tailwind compiles: fresh 'vite build' in the worktree emits '.outline-offset-\[-2px\]{outline-offset:-2px}' plus the unchanged 'outline-offset:1px' (focus-visible) in dist/assets CSS. Rendered check in headless Chromium (file repro, real bundle CSS): computed outlineOffset -2px / width 2px / solid / emerald on the card; wrapper scrollWidth identical (1200) with and without the fix; scrolled-right screenshot shows the inset ring fully visible at the card edge. (Caveat: this old headless build does not resolve rem in border-radius, so rounded corners do not render there — renderer limitation, pre-existing, unrelated; radius comes from the untouched 'rounded' class.) 3. Shared treatment: flash fragments byte-identical across ThreadCard/StagedCard (pinned by test); draft composer card (:583) keeps exact slot classes 'ml-14 mt-1 rounded border border-zinc-200 p-2 dark:border-zinc-700' with no outline/ring of its own — no divergent treatment. 4. No new scroll containers: only overflow sites are the pre-existing ref-list dropdown (:261) and hunk wrapper (:524); cards add none (pinned). 5. focus-visible unchanged: ui.css:18-21 rule byte-identical (2px + 1px outside offset) and gutter-trigger 'focus-visible:outline...' classes (:547) byte-identical; ui.css not in the diff at all. 6. Law 12: docs/go/12_web_ui.md carries the #574 decision entry in the same change. 7. Law 1: no package.json change; test pins runtime deps exactly to solid-js + @solidjs/router + marked + dompurify; no new ui.css rule (Tailwind-only composition, no guideline extension needed). 8. Tests fail pre-fix: new flash-ring-574.test.js 11/11 green on the branch; against origin/main Pull.jsx 3 fail (both inset-geometry pins + the JSX-wide no-bare-outside-outline sweep) — the bug reproduces, the fix resolves it. Full-minus-smoke 1417/1417 green (matches PR claim); related 573/567/554 pins green (35/35). The single smoke.test.js failure is environmental (a foreign server answers :8080 /healthz but 403s /setup; smoke requires a live app server) — same exclusion the PR description uses. vite build green; restored dist/.keep after the build (build deletes it; 'go:embed all:dist' needs it). 9. Browser proof remains reasoned-only (shared-daemon loopback guard), consistent with #571/#572/#573 — acceptable here: single paint-only utility, zero layout delta pinned by test, plus the rendered geometry check above. Verdict: APPROVED — meets all four acceptance criteria with the review checklist above. No follow-up commit (worktree clean apart from node_modules).
Sign in to join this conversation.
No description provided.