Skip to content

fix(web): a selection in the chat survives the list moving its row - #733

Merged
badcuban merged 3 commits into
mainfrom
fix/selection-survives-row-move
Oct 31, 2026
Merged

badcuban merged 3 commits into
mainfrom
fix/selection-survives-row-move

Conversation

@badcuban

Copy link
Copy Markdown
Collaborator

"keeps the actions beside the selected text as the list scrolls" kept failing on CI after #731 gave it longer waits. #731 treated it as slowness; it wasn't. This is a real bug readers can hit.

What happens. The chat list (@legendapp/list) keeps its rows' elements in DOM order by moving them, and a scroll can set that off. Moving an element collapses every live range inside it, including the reader's selection and the selection popover's own Range, and no event fires. The selection vanished, and the Copy / Add note actions stayed mounted but hidden for good, even with the text back in view. Row heights decide when the list moves rows, so it showed on Linux CI and not on the Mac.

Evidence. Diagnostic runs on CI (a throwaway branch run with workflow_dispatch, no PR) logged the selection and DOM changes per step. The failing runs show the row element of message 51 removed and re-added under its parent during a scroll. The page selection then sits collapsed at that parent (rangeCount: 1, isCollapsed: true), and the popover is hidden while the word is in view.

Fix. TranscriptSelectionPopover watches the timeline for its message body's row leaving its parent. It then finds the text again from its offsets and, in actions mode, gives back the selection the move collapsed. It does that only while the caret still sits exactly where the move left it, so a click elsewhere stays the reader's. A note keeps its own highlight and focus. The watched body is kept apart from the cached range, so an edit inside the message just before a move can't hide the move. These two guards came from a review.

Tests. A new real-list test moves the row with moveBefore:

  • a plain move: the old code fails this every time;
  • an edit plus a move: the first version of the fix failed this;
  • a click right after a move: the actions go and nothing is restored. In this ordering the chat's own dismissal wins, so this step doesn't isolate the caret guard; that guard covers a narrower timing that a test can't force.

On CI, the flaky test passed 15/15 repeats with the fix; two of four diagnostic runs failed without it. Locally: vp fmt, vp lint, typecheck, and the full browser suite pass.

The chat list keeps its rows' elements in order by moving them, and a
scroll can set that off. Moving an element collapses every live range
inside it: the reader's selection disappeared, and the Copy / Add note
actions lost their place in the text and stayed hidden, with no event to
tell anyone. Row heights decide when the list moves rows, so this showed
up on Linux (and as the flaky "keeps the actions beside the selected text
as the list scrolls" test on CI).

The actions now watch for their row's element leaving its parent, find
the text again from its offsets, and give back a selection the move
collapsed (not one the reader changed; a note in progress keeps its own
highlight and focus). A new test moves the row by hand and fails without
the fix every time.
…a move after an edit

Review follow-ups. The selection is given back only while the page's
caret still sits where the move collapsed it, so a click elsewhere in
the same moment stays the reader's. The observed message body is kept
apart from the cached range, so an edit inside the message just before
a move no longer hides the move. The test covers an edit with a move,
and a click right after one.
@vercel

vercel Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
threadlines Skipped Skipped Oct 31, 2026 8:02am UTC

Request Review

… parent

The list moving other rows shifts the caret's offset there, so matching
the offset too refused to give the selection back (the new test failed
once on CI). A click into any text puts the caret elsewhere, so it still
stays the reader's.
@badcuban
badcuban added this pull request to the merge queue Oct 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 31, 2026
@badcuban
badcuban added this pull request to the merge queue Oct 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 31, 2026
@badcuban
badcuban added this pull request to the merge queue Oct 31, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 31, 2026
@badcuban
badcuban added this pull request to the merge queue Oct 31, 2026
Merged via the queue into main with commit dc6121f Oct 31, 2026
22 checks passed
@badcuban
badcuban deleted the fix/selection-survives-row-move branch October 31, 2026 08:51

This branch was previously deployed

1 inactive deployment
Preview — 7744a2a8 Deployed Oct 31, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant