Repository navigation
feat(tui): make an expanded accordion's whole region the click/hover target - #29
Merged
Merged
Conversation
There was a problem hiding this comment.
Collapsed tool blocks with compact previews now highlight non-clickable body rows when their header is hovered. The hover span needs to remain header-only until the block is expanded.
🤖 Bits Code Review · Commit 19e8845
…target When a block (e.g. reasoning) is expanded, every disclosed row becomes part of the click target that collapses it, and hover highlights the whole region with two tones: a stronger fill on the header, a softer accordionDetailHover fill (45% accordionHover over the background) on the disclosed rows below it. A collapsed block keeps only its header row as the target. - chat.List: HeaderAt renamed to AccordionAt and extended via accordionAtRow to cover every row of an expanded item; hoverFillRows computes the (possibly header-scrolled-off) row span renderSurface paints. - styles: add Accordion.DetailHoverBackground, wired from a new accordionDetailHover palette token (dark #1C1E26, light #D7DCE5). - update.go/model.go: HeaderAt -> AccordionAt at the mouse-down site; the pendingAccordionToggle comment now reflects the larger target. - The hand pointer shape (reconcilePointerShape) already derives from List.Hovered, so it follows the whole expanded region automatically. - Tests: rewrote list_test.go's HeaderAt coverage into TestAccordionAtCoversTheExpandedBlockAndNothingElse plus a header-scrolled- off case, added TestHoverPaintsTheWholeExpandedBlock, and re-aimed TestMouseWheelScrollRefreshesHoverUnderTheStationaryPointer at the real intent (hover retargets on scroll, rather than always clearing). Added TestMouseMotionSetsHandPointerAcrossTheWholeExpandedRegion covering the hand-pointer regression explicitly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hoverFillRows painted a collapsed block's whole compact body even though accordionAtRow only targets its header, so the hover misrepresented the click target for blocks with preview rows (exec_command, edit_file, write_file). Add a regression test using an edit_file diff fixture, since the existing fixture's collapsed body is a single row and hid the bug.
jinroh
force-pushed
the
will.su/accordion-click-collapse
branch
from
October 16, 2026 15:19
814ddf8 to
d6f4f90
Compare
Contributor
|
rebased on top of main. |
A collapsed block's compact preview (exec_command output, edit_file and write_file diffs) is now part of its click and hover target, like an expanded block's disclosed rows. The zone is simply every row a guttered block renders, which drops the expanded-state branches from accordionAtRow and hoverFillRows and supersedes the header-only restriction from 780b217. Consolidate the accordion tests around that single rule: - TestAccordionZoneIsTheWholeRenderedBlock table-tests targeting and hover painting for header-only, preview and expanded blocks, replacing three overlapping tests. - TestAccordionZoneSurvivesTheHeaderScrollingOff now also checks the visible rows keep the detail fill once the header scrolls off. - TestMouseMotionSetsPointerShapeOverTheAccordionZone merges the two pointer-shape tests and asserts real OSC 22 transitions.
jinroh
force-pushed
the
will.su/accordion-click-collapse
branch
from
October 16, 2026 15:23
d6f4f90 to
9d711d9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
When an accordion block (reasoning, tool results) is expanded, its entire region — the title row and every disclosed content row — is now the click target that collapses it, and hover highlights that whole region. Previously only the title row was clickable.
A collapsed block is unchanged: its header row stays the only target, since its compact body rows are its own content rather than a disclosure affordance.
QA
https://cleanshot.com/share/DpTzZhCG
Details
HeaderAtbecomesAccordionAt— the target is no longer only the header, so the name moved with it. Hit-testing stays inchat.List;components.Accordionremains a dumb glyph renderer.renderSurface, reusingPaintRowBackgroundso each line's own foreground colors survive the repaint. It never touchesDocument()or item heights.accordionHover, and disclosed rows get a newaccordionDetailHovertoken —accordionHoverblended at 45% over the background (#1C1E26dark,#D7DCE5light). The header stays the region's visual anchor.reconcilePointerShapederives fromList.Hovered, which now resolves throughAccordionAt.headerindex is left unclamped so the paint loop can tell whether row 0 is the header, while the row span itself is clamped to the viewport.Test plan
go build ./...,./scripts/test.sh,./scripts/lint.sh— all clean.TestAccordionAtCoversTheExpandedBlockAndNothingElse— expanded rows are targets, a collapsed block's body rows are not, nothing past the viewport is.TestAccordionAtKeepsExpandedDetailRowsAfterTheHeaderScrollsOff— the header is not the hit-test anchor.TestHoverPaintsTheWholeExpandedBlock— header fill on row 0, detail fill below, no leak past the item's last row.TestMouseMotionSetsHandPointerAcrossTheWholeExpandedRegion— hand across every disclosed row, default outside.TestMouseWheelScrollRefreshesHoverUnderTheStationaryPointerwas re-aimed rather than deleted: it previously asserted that scrolling cleared hover, which assumed header-only targets. It now asserts hover is recomputed to a different block under a stationary pointer, which is the behavior it was really guarding.Note for review
internal/tui/styles/palette.goshows a large diff because gofmt re-aligned both palette literal blocks when the new field widened the longest key. The only substantive changes there are the one new struct field and its two values.