Skip to content

fix(studio): keep the reader put when a disclosure expands, and copy clean selections - #2319

Merged
kantord merged 3 commits into
mainfrom
studio-followups/09-transcript-scroll-copy
Oct 15, 2026
Merged

kantord merged 3 commits into
mainfrom
studio-followups/09-transcript-scroll-copy

Conversation

@kantord

@kantord kantord commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Summary

Expanding Activity, a reasoning disclosure, "Show all N lines" or "Show more" in the last message let the content ResizeObserver pull the reader to the bottom, sometimes leaving the toggle above the viewport. Transcript selections also carried screen-reader-only text.

Part of #2303 ("Tiny follow-up fixes"); it does not close it. This is PR 9 of 14 in a stack of draft PRs, reviewed and merged in order.

What changed

  • Transcript <section>: onClickCapture on an [aria-expanded="false"] target stops following. The ref is cleared synchronously because the observer fires before the next render, and setAtTranscriptBottom(false) keeps it false. Following resumes through the existing paths (scroll back within 80px, or the scroll-to-bottom button). The prototype has no observer and follows only when messages change, so expansion never scrolls there.
  • selectedTranscriptText: hides the .sr-only spans the selection crosses for one synchronous read and restores them in finally; used by Add to chat, Copy and Add to canvas.
  • e2e: a selection excludes "Tool:" and "completed"; the header authorization cue is transient while the transcript keeps the pending authorization (the stream closes after the authorization frames). No production change for the cue.

Behavior changes

Expansion never scrolls; collapsing and plain growth still follow. Copied selections omit hidden status text. Note: [aria-expanded="false"] also matches a collapsed Radix menu trigger inside the transcript, so opening a message action menu at the bottom also stops following until the reader scrolls back within 80px.

Screenshots

Before is the parent branch (#2318), after is this PR, with the same scenario code and offline mock data.

Scenario Before After
Expand the Activity line (30 tools) in the last message, reader at the bottom
The view jumped to the end of the list (scrollTop 1337 to 2057); the Activity toggle is above the viewport.

The toggle stays in place with the rows below it (scrollTop stays 1337). The scroll-to-bottom arrow shows because the reader is no longer following the bottom.
Select a tool row, then "Add to chat"
Draft: completed, Tool: Bash, Bash, command: ls, report.txt (hidden status text included).

Draft: Bash, command: ls, report.txt (visible text only).

The expansion has to be taller than the viewport for the toggle to leave the screen, so the data uses 30 tools (with 10 tools the toggle moved but stayed visible). The selection highlight is not visible in either image because clicking "Add to chat" clears it.

Verification

Biome, typecheck, the full Studio test suite and pnpm generated:check pass on this branch, run on the assembled stack. The full Playwright suite passes on the stack tip (122 passed, 30 skipped, 0 failed, including chat-approval against a real mecated build). The expand test fails with the click-capture condition neutralised, and the selection e2e fails with the old selection.toString(). The helper's unit tests are new with the helper. The disclosure unit test uses an injected button rather than a real Activity row.

Not included

Native Ctrl+C is unchanged.

Fully or partially written by an AI agent.

🤖 Generated with Claude Code

@kantord
kantord added this pull request to stack #2325 October 14, 2026 13:27
@kantord
kantord marked this pull request as ready for review October 15, 2026 08:56

@peppescg peppescg 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.

Thanks Daniel. The selection clean-up is neat, and keeping the toggle in place matches the prototype.

Before merge

  1. Follow stays off after a small expansion (apps/web/src/features/chat/chat-workspace.tsx:3308). The click capture on a collapsed element sets atTranscriptBottom to false, and only a scroll event sets it back. When the expansion is small (a reasoning toggle, or Activity with a few rows during a stream) or the click grows nothing (a menu trigger), the reader is still within 80px of the bottom, but nothing re-checks, so the streamed reply runs off-screen until they scroll by hand. Suspend follow for that one resize only, then re-check isNearTranscriptBottom once (in the observer callback or a rAF after it), so a small expansion keeps following and the 80px rule stays the single source of truth. Please add a test: expand at the bottom, grow by less than 80px, then grow again as if streaming; it should follow.

Nits:

  • apps/web/src/features/chat/selected-transcript-text.ts:13,17: the finally resets hidden to false rather than to its earlier value. Save and restore it (const was = el.hidden), so a .sr-only node hidden for its own reason isn't revealed.
  • apps/web/src/features/chat/chat-workspace.tsx:2722: captureTranscriptSelection runs on every pointerup in the transcript and toggles .sr-only nodes, forcing two layouts even for a plain click. Return early on selection.isCollapsed || selection.rangeCount === 0.
  • The disclosure unit test injects a bare button. One case with a real Activity or reasoning toggle would catch a later change that drops aria-expanded from them.

@kantord
kantord force-pushed the studio-followups/09-transcript-scroll-copy branch 4 times, most recently from ca20997 to 395c619 Compare October 15, 2026 09:29
@kantord

kantord commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

Thanks Giuseppe. Addressed in 8236dcd.

  1. Follow: the click on a collapsed element still stops following, so the observer holds still during the expansion's own resize (a tall expansion keeps the toggle in place, unchanged). It now also schedules a re-check two animation frames later (the frame after the expansion's layout and ResizeObserver callback) that sets atTranscriptBottom from isNearTranscriptBottom, so the 80px rule stays the single source of truth. A small expansion stays within 80px and follows again; a tall one leaves the reader where they are. I tied the re-check to frames rather than to "the next resize", so a click that grows nothing (a menu trigger) cannot leave follow stuck off and a later unrelated growth is never swallowed; a new click cancels a pending re-check. New tests: the one you described (expand at the bottom, grow by less than 80px, then grow as if streaming: it follows), a click that grows nothing, and a tall expansion that leaves follow off (a regression guard).
  2. selectedTranscriptText saves and restores each node's previous hidden value.
  3. captureTranscriptSelection returns early on isCollapsed || rangeCount === 0.
  4. Added a case that renders the real tool Activity list and reasoning disclosure and asserts the toggle matches [aria-expanded="false"].

Checked on top of the current head: Biome, typecheck, the full Studio test suite and generated:check pass, and the chat Playwright specs (header-stop, selection-copy, minimap, delivery-follow) pass, 13 of 13. A real-browser check of the tall expansion (30 tools) keeps scrollTop and the toggle in view. Not covered in a real browser: the small-expansion follow (happy-dom with fake rAF only), and an expansion whose layout lands more than one frame after the click.

Fully or partially written by an AI agent.

@kantord
kantord requested a review from peppescg October 15, 2026 09:39
@kantord

kantord commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

Added 1d3ede2 (test only): the chat-selection-copy e2e read the composer once with inputValue() right after clicking "Add to chat", which does not retry, so a slow CI run on the mobile project saw an empty draft (it failed once on #2320's CI). It now waits with toHaveValue. Under load (4 workers, 10 repeats, desktop and mobile) the old spec failed once in 20 runs and the new one passed 20 of 20. No product code changed.

Fully or partially written by an AI agent.

@kantord
kantord force-pushed the studio-followups/09-transcript-scroll-copy branch from 1d3ede2 to ce7a616 Compare October 15, 2026 12:25
Base automatically changed from studio-followups/08-escape-tests to main October 15, 2026 13:00
@kantord
kantord force-pushed the studio-followups/09-transcript-scroll-copy branch from ce7a616 to bada93e Compare October 15, 2026 13:00
kantord and others added 3 commits October 15, 2026 15:22
…clean selections

Expanding a disclosure no longer lets the content observer pull the reader
to the bottom. A transcript selection excludes screen-reader-only spans.
Adds the e2e for the transient authorization header cue.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…k the bottom

- Expanding a disclosure suspends follow only for the resize it causes;
  two frames later the 80 px rule decides again, so a small expansion (or
  a click that grows nothing) keeps following and a tall one stays put.
- selectedTranscriptText restores each .sr-only node's earlier hidden
  value instead of forcing false.
- captureTranscriptSelection returns early on a collapsed selection, so a
  plain click no longer forces two layouts.
- Tests cover the real Activity and reasoning toggles, not just a bare button.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… reading it once

The spec read the composer with `inputValue()` right after clicking "Add to
chat", which does not retry. The draft lands after a state update, so on a slow
CI run (seen on the mobile project) the read returned an empty string. Wait for
the value with `toHaveValue` first.

Under load (4 workers, 10 repeats on desktop and mobile) the old spec failed
once in 20 runs and the fixed one passed 20 of 20.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@kantord
kantord force-pushed the studio-followups/09-transcript-scroll-copy branch from bada93e to d20ca6f Compare October 15, 2026 13:26
@kantord
kantord merged commit 97f0d86 into main Oct 15, 2026
31 checks passed
@kantord
kantord deleted the studio-followups/09-transcript-scroll-copy branch October 15, 2026 14:01
@toolhive-release-app toolhive-release-app Bot mentioned this pull request Oct 20, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants