fix(control-ui): make long /btw side results scrollable#101169
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 7:23 PM ET / 23:23 UTC. Summary PR surface: Source +9, Tests +51. Total +60 across 2 files. Reproducibility: yes. source-reproducible. Current main renders arbitrary sanitized Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused CSS and responsive-test fix after normal merge-result refresh, keeping the side-result body as the desktop scroll owner and the mobile card as the mobile scroll owner. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible. Current main renders arbitrary sanitized Is this the best way to solve the issue? Yes. The PR fixes the presentation owner directly, preserves the existing mobile scroll model, and uses the established guarded responsive browser suite rather than a duplicate Playwright lifecycle. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a89fe705b8b8. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +9, Tests +51. Total +60 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
|
Proof that the fix works. Local Playwright run (1366x900 viewport, Chromium headless, installed via
Screenshot after scrolling the BTW side-result body to the bottom: I also added a standalone lightweight browser test ( Verification artifacts (script + screenshot + metrics) are on the throw-away branch |
62e8f2c to
f046898
Compare
|
Maintainer fixup and verification complete at exact head
The PR body now contains the required problem, rationale, impact, and evidence sections. Land-ready from review. |
The BTW side-result card in Control UI expanded to the full height of its content on desktop, so answers taller than the viewport couldn't be read. Constrain the result body with a max-height and overflow:auto so it scrolls in place, while keeping the existing mobile overlay behavior intact. - Add max-height/min(55vh,480px), overflow:auto, and overscroll-behavior:contain to .chat-side-result__body for non-mobile viewports. - Reset those rules inside the mobile media query so the whole card still scrolls as a fixed overlay. - Add a lightweight browser test that verifies the body is scrollable on desktop and the mobile overlay still scrolls as a whole.
0316d2f to
032e996
Compare
|
Merged via squash.
|
|
Land-ready proof for exact head
Known gap: no screenshot was published because the interaction assertions exercise the real rendered browser fixture directly and no image upload was needed. |
|
Merged via squash.
|

What Problem This Solves
Long
/btwside-result cards could grow beyond the desktop viewport, leaving later content inaccessible. The original CSS fix was sound, but its standalone browser test duplicated the established responsive fixture and initially bypassed the repository's Chromium capability guard.Why This Change Was Made
chat-responsive.browser.test.tsand the existing typed full-chat fixture.scrollTopon the desktop body and mobile card, rather than checking CSS declarations alone.User Impact
Desktop users can read the full result without the card escaping the viewport. Mobile users keep the existing full-card scrolling behavior. There are no configuration, protocol, or persisted-state changes.
Evidence
9d560b57d39ee70ffeba5974b96d9ee6b365cd42cbx_a861210854a8, public networking, no Tailscale, no instance profilepnpm test ui/src/pages/chat/chat-responsive.browser.test.tsscrollTop; no horizontal overflowContributor: @SnoutFirst