Thread card collapse/expand label goes stale when the card is flash-expanded (click or jump) #580

New issue

Closed

opened 2026-09-15 15:41:28 +00:00 by crueber · 1 comment

crueber commented

2026-09-15 15:41:28 +00:00

Owner

What's requested

The thread card's collapse/expand toggle shows the wrong label when the card is force-expanded by a flash (card-body click or jump-to-comments pill). Fix the label to track the card's actual expansion state.

Evidence

web/src/pages/Pull.jsx:766 — effective open state: const open = () => getOpen() || flashed();

web/src/pages/Pull.jsx:812-814 — the toggle label reads only getOpen():

<button type="button" class="link" onClick={(e) => { e.stopPropagation(); props.onCollapse?.(t().tid); setOpen(!getOpen()); }}>
  {getOpen() ? "collapse" : "expand"}
</button>

A flashed card (props.flashTid?.() === t().tid, line 765) is expanded via open() but the label still says "expand" — stale the moment the card flash-expands. Two triggers:
1. Card-body click flash (Forgejo #575): clicking anywhere on a collapsed card's body sets the page-level flash, expanding it while the label still reads "expand".
2. Jump-to-comments pill: scrolling to a collapsed card flash-expands it; same stale label.

Architecture notes

Expansion has two sources: user toggle (getOpen()) and flash (flashed()). The label and the <Show when={open()}> body (line 816) read different signals — that divergence is the bug.
The flash-clear mechanics from #573 mean the toggle's onClick already clears the page-level flash for this tid before flipping getOpen(). Implementer's call on the label signal: deriving the label from open() is the minimal fix, but check the post-click behavior on a flashed-and-collapsed card so a click labeled "collapse" never lands the card still open (the clearFlash + setOpen flip must compose so the resulting open() matches the label the user clicked).
Related tests: web/test/unit/card-click-flash-575.test.js, web/test/unit/thread-flash-clear-573.test.js — extend or add a sibling covering the label.

Acceptance criteria

A collapsed card click-flashed (card body click) shows the label "collapse", matching its expanded body.
A card reached via jump-to-comments shows "collapse" while flash-expanded.
Clicking the toggle on a flashed card produces a label/body pair that agree (never "expand" label with visible comments, nor "collapse" label with hidden comments) — including the #573 clearFlash path.
Non-flashed toggle behavior is unchanged (resolved threads still start collapsed, label "expand").
Unit test covers the label tracking the flash-driven expansion.

## What's requested The thread card's collapse/expand toggle shows the wrong label when the card is force-expanded by a flash (card-body click or jump-to-comments pill). Fix the label to track the card's actual expansion state. ## Evidence - `web/src/pages/Pull.jsx:766` — effective open state: `const open = () => getOpen() || flashed();` - `web/src/pages/Pull.jsx:812-814` — the toggle label reads only `getOpen()`: ```jsx <button type="button" class="link" onClick={(e) => { e.stopPropagation(); props.onCollapse?.(t().tid); setOpen(!getOpen()); }}> {getOpen() ? "collapse" : "expand"} </button> ``` - A flashed card (`props.flashTid?.() === t().tid`, line 765) is expanded via `open()` but the label still says "expand" — stale the moment the card flash-expands. Two triggers: 1. **Card-body click flash** (Forgejo #575): clicking anywhere on a collapsed card's body sets the page-level flash, expanding it while the label still reads "expand". 2. **Jump-to-comments pill**: scrolling to a collapsed card flash-expands it; same stale label. ## Architecture notes - Expansion has two sources: user toggle (`getOpen()`) and flash (`flashed()`). The label and the `<Show when={open()}>` body (line 816) read different signals — that divergence is the bug. - The flash-clear mechanics from #573 mean the toggle's onClick already clears the page-level flash for this tid before flipping `getOpen()`. Implementer's call on the label signal: deriving the label from `open()` is the minimal fix, but check the post-click behavior on a flashed-and-collapsed card so a click labeled "collapse" never lands the card still open (the clearFlash + setOpen flip must compose so the resulting `open()` matches the label the user clicked). - Related tests: `web/test/unit/card-click-flash-575.test.js`, `web/test/unit/thread-flash-clear-573.test.js` — extend or add a sibling covering the label. ## Acceptance criteria - [ ] A collapsed card click-flashed (card body click) shows the label "collapse", matching its expanded body. - [ ] A card reached via jump-to-comments shows "collapse" while flash-expanded. - [ ] Clicking the toggle on a flashed card produces a label/body pair that agree (never "expand" label with visible comments, nor "collapse" label with hidden comments) — including the #573 clearFlash path. - [ ] Non-flashed toggle behavior is unchanged (resolved threads still start collapsed, label "expand"). - [ ] Unit test covers the label tracking the flash-driven expansion.

crueber added the

bug

pull requests

labels

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

crueber added this to the v1 milestone

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

crueber added the

issues

label

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

crueber referenced this issue from a commit

2026-09-15 16:27:26 +00:00

FIXED (Forgejo #580) — thread card collapse/expand label derives from open()

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

2026-09-15 16:27:30 +00:00

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

crueber referenced this issue from a commit

2026-09-15 16:29:23 +00:00

REVIEW (Forgejo #580) — correct new-test count 10 -> 11 in the #580 amendment

crueber referenced this issue

2026-09-15 16:29:30 +00:00

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

crueber referenced this issue from a commit

2026-09-15 16:29:42 +00:00

Merge pull request 'Fix #580: thread card collapse/expand label derives from open()' (#582) from fix/issue-580 into main

crueber closed this issue

2026-09-15 16:29:43 +00:00

crueber commented

2026-09-15 16:29:43 +00:00

Author

Owner

Fixed by #582 (merged): ThreadCard toggle label now derives from open() — the same effective signal as the body — so flash-expanded cards read collapse; two-click label/body agreement pinned incl. the #573 clear path; unflashed behavior unchanged. Verified: 1440 unit tests green (smoke excluded, pre-existing), vite/esbuild green, independent review APPROVE.