Fix #575: clicking inside a thread/staged card flashes it like a pill jump #579

Merged
crueber merged 1 commit from fix/issue-575 into main 2026-09-15 15:04:28 +00:00
Owner

Closes #575.

Consumes the #573 flash lifecycle (no second mechanism): ThreadCard/StagedCard roots set the same page-level flashTid/flashStaged through one shared cardFlashClick guard — same emerald ring, same #574 inset geometry. Page passes raw setters, so card click overwrites like a pill jump but never scrolls.

Decisions: (1) trigger scope — any body click flashes; control clicks (button/a/input/textarea/select/form, closest check) + selection drags (getSelection) ignored; (2) single-flash model — same setter as pill jump, click overwrites; (3) StagedCard joins. Collapse interplay: collapse stopPropagates + guard ignores controls → ends collapsed + unhighlighted (#573 test pin updated same commit). Pill jumps byte-identical; anchor/hash untouched.

Tests: new card-click-flash-575.test.js (11); related 573/574/567/546 green (76 total); full-minus-smoke 1429/1429; vite + esbuild green; go vet clean. 390px reasoned, no layout change.

Closes #575. Consumes the #573 flash lifecycle (no second mechanism): ThreadCard/StagedCard roots set the same page-level flashTid/flashStaged through one shared cardFlashClick guard — same emerald ring, same #574 inset geometry. Page passes raw setters, so card click overwrites like a pill jump but never scrolls. Decisions: (1) trigger scope — any body click flashes; control clicks (button/a/input/textarea/select/form, closest check) + selection drags (getSelection) ignored; (2) single-flash model — same setter as pill jump, click overwrites; (3) StagedCard joins. Collapse interplay: collapse stopPropagates + guard ignores controls → ends collapsed + unhighlighted (#573 test pin updated same commit). Pill jumps byte-identical; anchor/hash untouched. Tests: new card-click-flash-575.test.js (11); related 573/574/567/546 green (76 total); full-minus-smoke 1429/1429; vite + esbuild green; go vet clean. 390px reasoned, no layout change.
Card roots (ThreadCard/StagedCard, inline + file-end) set the same
page-level flashTid/flashStaged through one shared cardFlashClick guard
— the #573 lifecycle consumed, not forked. Page passes raw setters, so
card click overwrites like a pill jump but never scrolls. Collapse
stopPropagates + guard ignores control clicks: collapse ends collapsed +
unhighlighted. Selection drags ignored via getSelection check.
Author
Owner

Independent review of #579 (fix/issue-575 @ 6e97ed3) against #575 — verdict: APPROVED, no fix commits needed.

Acceptance trace (all hold):

  • Card click flashes the same ring as a pill jump: both roots call the SAME page signals (onFlashTid=setFlashTid, onFlashStaged=setFlashStaged threaded through DiffFile to inline + file-end cards); the ring fragment is the reused #573 flashTid/flashStaged read with the #574 inset geometry. No second mechanism — cardFlashClick owns no state (asserted).
  • Pill-jump unchanged: jumpToThread/jumpToStaged set+scrollIntoView idioms byte-identical pre/post (verified against origin/main); anchor/drift-hash surface untouched (no anchorContextSha call sites, freshnessOf/sortThreadsForIndex intact).
  • Collapse-click ends collapsed+unhighlighted: traced — button handler stopPropagations AND the root guard's closest() ignores the button target AND onCollapse clears before setOpen toggles. Either layer alone suffices (stopPropagation covers bubbling; the guard covers delegated-dispatch edge cases), so there is no ordering hole; final state is cleared, never flashed. Pinned as a regression test.
  • Selection drags don't re-flash: window.getSelection non-empty guard; scope documented as the issue's decision point 1.
  • Both themes: reused #574 emerald fragment, no theme-specific delta; behavior-only (one onClick per root, zero layout utilities), desktop + 390px hold by construction like #573/#574.

Checklist:

  • Guard convention: closest(button,a,input,textarea,select,form) vs the DiffFile row handler's (..., [data-no-row-comment]). The delta is justified, not a defect — form covers reply-form gap clicks, [data-no-row-comment] has no card meaning, a covers rendered comment links. Control inventory fully covered: resolve/collapse/reply-input/reply-submit (ThreadCard), edit/remove (StagedCard).
  • Single-flash overwrite: raw setters, click overwrites exactly like pill jump; cross-kind independence matches pill paths.
  • StagedCard parity: same guard, same setter terms, same ring.
  • #573 handler-pin update (toggle now asserts stopPropagation + onCollapse + setOpen) is a strengthening, not a weakening — still pins the clear-before-toggle.
  • Law 12: doc amendment in same change notes all three decisions (trigger scope / single-flash / StagedCard) plus collapse interplay.
  • Law 1: runtime deps still exactly solid-js + @solidjs/router + marked + dompurify; no ui.css rule, no backend/SDK/API change.
  • Tests fail pre-fix: confirmed — origin/main Pull.jsx has no cardFlashClick, no root onClick, no stopPropagation, so the new wiring/guard/collapse pins fail there by construction.

Evidence run in /tmp/walhub-575: 575+573 files 25/25 green; related 575/573/574/567/546 76/76 green. No defects found, nothing fixed in the worktree.

Independent review of #579 (fix/issue-575 @ 6e97ed3) against #575 — verdict: APPROVED, no fix commits needed. Acceptance trace (all hold): - Card click flashes the same ring as a pill jump: both roots call the SAME page signals (onFlashTid=setFlashTid, onFlashStaged=setFlashStaged threaded through DiffFile to inline + file-end cards); the ring fragment is the reused #573 flashTid/flashStaged read with the #574 inset geometry. No second mechanism — cardFlashClick owns no state (asserted). - Pill-jump unchanged: jumpToThread/jumpToStaged set+scrollIntoView idioms byte-identical pre/post (verified against origin/main); anchor/drift-hash surface untouched (no anchorContextSha call sites, freshnessOf/sortThreadsForIndex intact). - Collapse-click ends collapsed+unhighlighted: traced — button handler stopPropagations AND the root guard's closest() ignores the button target AND onCollapse clears before setOpen toggles. Either layer alone suffices (stopPropagation covers bubbling; the guard covers delegated-dispatch edge cases), so there is no ordering hole; final state is cleared, never flashed. Pinned as a regression test. - Selection drags don't re-flash: window.getSelection non-empty guard; scope documented as the issue's decision point 1. - Both themes: reused #574 emerald fragment, no theme-specific delta; behavior-only (one onClick per root, zero layout utilities), desktop + 390px hold by construction like #573/#574. Checklist: - Guard convention: closest(button,a,input,textarea,select,form) vs the DiffFile row handler's (..., [data-no-row-comment]). The delta is justified, not a defect — form covers reply-form gap clicks, [data-no-row-comment] has no card meaning, a covers rendered comment links. Control inventory fully covered: resolve/collapse/reply-input/reply-submit (ThreadCard), edit/remove (StagedCard). - Single-flash overwrite: raw setters, click overwrites exactly like pill jump; cross-kind independence matches pill paths. - StagedCard parity: same guard, same setter terms, same ring. - #573 handler-pin update (toggle now asserts stopPropagation + onCollapse + setOpen) is a strengthening, not a weakening — still pins the clear-before-toggle. - Law 12: doc amendment in same change notes all three decisions (trigger scope / single-flash / StagedCard) plus collapse interplay. - Law 1: runtime deps still exactly solid-js + @solidjs/router + marked + dompurify; no ui.css rule, no backend/SDK/API change. - Tests fail pre-fix: confirmed — origin/main Pull.jsx has no cardFlashClick, no root onClick, no stopPropagation, so the new wiring/guard/collapse pins fail there by construction. Evidence run in /tmp/walhub-575: 575+573 files 25/25 green; related 575/573/574/567/546 76/76 green. No defects found, nothing fixed in the worktree.
Sign in to join this conversation.
No description provided.