Repository navigation
Hold focus in thread seats and cards with holdFocus - #1320
Merged
Merged
Conversation
`focus.js` `holdFocus` owns handing the user's place across a DOM change. `ThreadSeat.present`, `ThreadSeat.retain`, and `ThreadView.present` each kept their own copy: they read the focused node and its caret before a render and put the user back by hand afterwards. All three now take a `holdFocus` hold. `holdFocus` accepts a function as a stand-in: the caller's own landing when the user's place is gone. A seat whose box gave way to the thread it started, and a page thread whose render removed the user's place, both land through `landInThread`, which now runs only when nothing newer placed the user. A refused thread batch now restores through a hold the batch takes for each seat as the seat joins it, instead of a record on the seat that outlived a superseded batch. Behaviour that moved: - A user who moves to another control while a thread reading waits keeps their new place when that reading is refused. Before, the refusal pulled them back to where they stood when it began. Covered by `test_a_refused_thread_reading_leaves_a_user_who_moved_on_where_they_went`, which fails on the base. - Any card render that moves the node the user stands on keeps them on it. Before, only a new summary covering their message did. `test_a_summary_gathering_the_message_the_user_is_on_keeps_them_on_it` holds the summary case. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
leaf-agent
reviewed
Sep 27, 2026
leaf-agent
left a comment
Collaborator
There was a problem hiding this comment.
The batch hold needs to be released when a reading succeeds or is superseded. present takes one holdFocus for the batch, but only retainThreadSeats invokes it; commitThreadSeats drops the map, and beginThreadSeats can replace an unfinished batch. Since holdFocus removes its focusin listener only when restored or when focus next moves, each successful reading while the user stays in a seat adds another live listener. Repeated updates can accumulate them without bound. Give the hold an explicit cancellation path and use it on commit and supersession, while keeping restoration for refusal.
A hold its caller never restores, such as a thread batch's that commits, left a focusin listener behind until the user's next focus move, so repeated readings while the user stayed in a seat accumulated them. holdFocus now compares a module-level placement count instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
leaf-agent
approved these changes
Sep 27, 2026
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.
focus.jsholdFocusowns handing the user's place across a DOM change. Three thread sites kept their own copy of it:ThreadSeat.present,ThreadSeat.retainandThreadView.presenteach read the focused node (and caret) before a render and put the user back by hand. That boundary was wrong because each copy knew less than the owner:retaincould not tell that the user had moved on while the batch waited, and the card restored focus only for the one case its author had seen, a new summary covering the focused message. All three now take aholdFocushold, and the hand-rolled pairing is deleted.The primitive. A seat whose box gives way to the thread its first message started, and a page thread whose render removed the user's place, both send the user on through
landInThread, which also reveals and scrolls. That destination is a different place, not a replacement for the held node, soholdFocusnow accepts a function as a stand-in. The function is the caller's own landing: it takes no caret and returns whether it landed. The landing runs only when the held node is gone and nothing newer has placed the user, so neither caller restates that check.A hold also no longer registers a
focusinlistener of its own. One module-level listener counts focus placements, and a hold compares against the count it began at. That lets a hold whose caller never restores it, such as a batch's that commits, leave nothing behind. Before, each such hold stayed listening until the user's next focus move.The batch. A refused thread batch now restores through a hold the batch takes for each seat when that seat joins it (
batch.seatsis aMapfrom seat to hold). This replacesThreadSeat.#focus, which a superseded batch never cleared, so a later refusal could pull the user back to where an earlier batch began.Deleted:
ThreadSeat.#focusand its clearing incommit/retain, the seat's tworeadCaret/focusDestinationpairings, and the card'sheldFocusandsummaryReplacedFocusedMessagebranches. Places that still carry focus by key rather than by node (carry.js, the margin'scaptureStanding) do so across a replaced document, where no node survives to hold.Behaviour that moved
test_a_refused_thread_reading_leaves_a_user_who_moved_on_where_they_went, which fails on the base.test_a_summary_gathering_the_message_the_user_is_on_keeps_them_on_itcovers the summary case. It fails with the card's hold removed.A live tree (
leaf.drop-thread-titles) has uncommitted edits tothread-card.js. Its edits are in the imports andthreadReading, not inThreadView.present.Checks:
tests/test_render_threads.py,tests/test_render_application_boundary.py,tests/test_render_drafts.py,tests/test_render_margin.py,tests/test_chrome_contracts.py, andnpm run test:runtime. Locally,test_a_single_space_is_message_content_in_every_composer[reply]fails, and it fails the same way on the base runtime.🤖 Generated with Claude Code