Skip to content

fix(tui): keep mouse hit-testing in sync when the layout shifts without a resize#3798

Merged
dgageot merged 1 commit into
docker:mainfrom
dgageot:fix/session-load-click-hit-testing
Jul 23, 2026
Merged

fix(tui): keep mouse hit-testing in sync when the layout shifts without a resize#3798
dgageot merged 1 commit into
docker:mainfrom
dgageot:fix/session-load-click-hit-testing

Conversation

@dgageot

@dgageot dgageot commented Jul 23, 2026

Copy link
Copy Markdown
Member

After opening a past session with /session, the hover copy and edit buttons on messages did not respond to clicks until the terminal window was resized. Labels still appeared on hover (because multi-line messages tolerate a small offset), but clicks on their single-line hit targets always missed.

The root cause is that App.ReplaceSession re-emits startup info (agents, tools) asynchronously after the load. In the collapsed sidebar band layout — terminals narrower than 120 columns, or with the sidebar positioned top/bottom — those events grow the band by one line, visually shifting the message list down. Child component positions were only applied inside chatPage.SetSize, so mouse hit-testing stayed offset by the delta until a window resize re-ran SetSize.

The fix records the sidebar layout geometry that was last applied in SetSize. After every Update, the chat page compares the live computed layout against the recorded one and reapplies geometry when it has drifted. This is cheap because mouse handlers already compute the layout on each event. A new appModel-level test in pkg/tui/session_load_click_test.go loads a past session via LoadSessionMsg and clicks the hover "edit" label, covering the in-place path, the new-tab path, and the narrow band layout with post-load async sidebar growth; it failed reliably before the fix.

…ut a resize

After loading a past session via /session, App.ReplaceSession re-emits startup
info asynchronously; in the collapsed sidebar band layout (terminal < 120 cols
or top/bottom sidebar) this grows the band and shifts the message list down, but
child positions were only applied in SetSize, so copy/edit label clicks missed
until the window was resized. The chat page now compares the live computed layout
against the last applied one after each Update and reapplies the geometry when
it drifted.

Assisted-By: claude-sonnet-4-5
@dgageot
dgageot requested a review from a team as a code owner July 23, 2026 09:48

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The fix is correct and well-structured. The relayoutIfNeeded approach correctly detects layout drift after async sidebar updates, calls SetSize synchronously to apply the updated geometry, and the equality comparison on the sidebarLayout struct (all comparable value types) is safe. The WindowSizeMsg path is properly guarded — since update() calls SetSize (which records appliedLayout) before relayoutIfNeeded runs, the subsequent comparison finds no drift and returns nil, avoiding any double-apply. The new test covers in-place load, new-tab load, and the narrow band layout scenario that originally triggered the bug.

@aheritier aheritier added area/tui For features/issues/fixes related to the TUI kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Jul 23, 2026

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

The fix is well-scoped and correct. relayoutIfNeeded() is called after every Update, compares the live computeSidebarLayout() result against p.appliedLayout (recorded at the end of each SetSize), and calls SetSize synchronously when they differ — applying updated child positions before the next mouse event arrives. The width <= 0 || height <= 0 guard prevents the zero-value appliedLayout from triggering a spurious relayout before the first WindowSizeMsg. The approach is cheap (layout computation was already done per-event in mouse handlers) and bounded (the sidebar state stabilizes after the async startup info is fully consumed, so the drift disappears on the next update cycle). The new test correctly simulates the scenario end-to-end at the appModel level, using a manual TeamInfoEvent injection instead of the real async goroutine, which is cleaner and deterministic.

@dgageot
dgageot merged commit 42b231d into docker:main Jul 23, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/tui For features/issues/fixes related to the TUI kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants