Fix #580: thread card collapse/expand label derives from open() #582

Merged
crueber merged 2 commits from fix/issue-580 into main 2026-09-15 16:29:42 +00:00
Owner

One-line fix in ThreadCard (web/src/pages/Pull.jsx:813): toggle label reads open() — the same signal the body Show renders on — instead of getOpen(), so flash-expanded cards (via #575 body click or #573-era pill jump) label "collapse" with body open. Unflashed behavior unchanged (open()===getOpen()); resolved threads still start collapsed. StagedCard checked: no collapse/expand toggle, nothing to fix. Tests: new web/test/unit/thread-toggle-label-580.test.js (10 tests incl. two-click label/body agreement model); #580-scoped update to one stale 573 pin; docs/go/12_web_ui.md FIXED (#580) amendment same commit. Verify: related 76 pass; full-minus-smoke 1440 pass/0 fail; vite build + esbuild SDK green; go vet clean; 390px reasoned (text-only change).

One-line fix in ThreadCard (web/src/pages/Pull.jsx:813): toggle label reads open() — the same signal the body Show renders on — instead of getOpen(), so flash-expanded cards (via #575 body click or #573-era pill jump) label "collapse" with body open. Unflashed behavior unchanged (open()===getOpen()); resolved threads still start collapsed. StagedCard checked: no collapse/expand toggle, nothing to fix. Tests: new web/test/unit/thread-toggle-label-580.test.js (10 tests incl. two-click label/body agreement model); #580-scoped update to one stale 573 pin; docs/go/12_web_ui.md FIXED (#580) amendment same commit. Verify: related 76 pass; full-minus-smoke 1440 pass/0 fail; vite build + esbuild SDK green; go vet clean; 390px reasoned (text-only change).
Author
Owner

APPROVED — independently verified against issue #580 acceptance, all points hold.

What I checked (worktree /tmp/walhub-580, diff origin/main..origin/fix/issue-580):

  • Fix is exactly the minimal divergence-close: Pull.jsx:813 label now reads open(), the same signal the body renders on. No getOpen()-only label remains.
  • Two-click trace on flashed+collapsed (getOpen=false, flash set, label 'collapse'): click1 (labeled 'collapse') runs onCollapse(tid) clear BEFORE setOpen(!getOpen()) — handler order pinned by regex — landing at open()=true / label 'collapse' / body open; click2 lands at open()=false / label 'expand' / body closed. The test's two-click model pins every step incl. label/body agreement after each click, so 'collapse'+closed can never strand.
  • Unflashed provably unchanged: open()===getOpen() pinned by truth-table test (expand/closed, collapse/open); resolved-starts-collapsed (!resolved init + 'expand' label) pinned.
  • 573-pin update is same-behavior: label expectation getOpen()->open() with unflashed identity holding; collapse-clear/handler pins untouched.
  • StagedCard no-toggle claim true: StagedCard (Pull.jsx:715-744) is edit/remove only, no collapse/expand/getOpen/setOpen signal (the only other getOpen hit is the unrelated reviewer searchbox :258).
  • Law 12: FIXED (#580) amendment in docs/go/12_web_ui.md in the same change; law 1: no new deps (pinned: solid-js + @solidjs/router + marked + dompurify) and no ui.css change.
  • New test fails pre-fix: with the label temporarily reverted to getOpen(), exactly the shared-signal pin fails (10 pass/1 fail); restored, 11/11 green. Related files (580+573+575) 36/36 green. Full web/test/unit/*.test.js: 1443 total = 1440 pass minus-smoke (matches the PR claim exactly) + 3 smoke, whose 1 failure is the known pre-existing live-server /setup 403 already documented as pre-existing on pristine main in the #520/#531 amendments — zero PR-caused failures.

Reviewer fix pushed (9617533, same branch): the #580 amendment said '(10 tests...)' but thread-toggle-label-580.test.js contains 11 test() blocks — corrected to 11. One-line doc-only change; re-verified green after the edit (working tree clean at push).

Verdict: approve/merge at your discretion — no browser proof beyond the reasoned text-only/desktop+390px note, same standing caveat as the stacked flash PRs.

APPROVED — independently verified against issue #580 acceptance, all points hold. What I checked (worktree /tmp/walhub-580, diff origin/main..origin/fix/issue-580): - Fix is exactly the minimal divergence-close: Pull.jsx:813 label now reads open(), the same signal the body <Show when={open()}> renders on. No getOpen()-only label remains. - Two-click trace on flashed+collapsed (getOpen=false, flash set, label 'collapse'): click1 (labeled 'collapse') runs onCollapse(tid) clear BEFORE setOpen(!getOpen()) — handler order pinned by regex — landing at open()=true / label 'collapse' / body open; click2 lands at open()=false / label 'expand' / body closed. The test's two-click model pins every step incl. label/body agreement after each click, so 'collapse'+closed can never strand. - Unflashed provably unchanged: open()===getOpen() pinned by truth-table test (expand/closed, collapse/open); resolved-starts-collapsed (!resolved init + 'expand' label) pinned. - 573-pin update is same-behavior: label expectation getOpen()->open() with unflashed identity holding; collapse-clear/handler pins untouched. - StagedCard no-toggle claim true: StagedCard (Pull.jsx:715-744) is edit/remove only, no collapse/expand/getOpen/setOpen signal (the only other getOpen hit is the unrelated reviewer searchbox :258). - Law 12: FIXED (#580) amendment in docs/go/12_web_ui.md in the same change; law 1: no new deps (pinned: solid-js + @solidjs/router + marked + dompurify) and no ui.css change. - New test fails pre-fix: with the label temporarily reverted to getOpen(), exactly the shared-signal pin fails (10 pass/1 fail); restored, 11/11 green. Related files (580+573+575) 36/36 green. Full web/test/unit/*.test.js: 1443 total = 1440 pass minus-smoke (matches the PR claim exactly) + 3 smoke, whose 1 failure is the known pre-existing live-server /setup 403 already documented as pre-existing on pristine main in the #520/#531 amendments — zero PR-caused failures. Reviewer fix pushed (9617533, same branch): the #580 amendment said '(10 tests...)' but thread-toggle-label-580.test.js contains 11 test() blocks — corrected to 11. One-line doc-only change; re-verified green after the edit (working tree clean at push). Verdict: approve/merge at your discretion — no browser proof beyond the reasoned text-only/desktop+390px note, same standing caveat as the stacked flash PRs.
Sign in to join this conversation.
No description provided.