Repository navigation
test(threads): render before checking the reader across activity resizes - #2389
Merged
Merged
Conversation
"thread preserves bottom and reading position through activity and composer resizing" fails on Linux WebKit with the reader 16 px off after activity above it ends (Expected < 4, Received 16). It started with #1729 and hit four unrelated branches within two hours. ThreadPanel corrects each resize in its ResizeObserver, which runs only in a rendering update. On a loaded Linux WebKit runner those are far apart, so settle() can see geometry rest while a correction is still pending. Leaving activity also gets aria-hidden at once, so the region check passed before the section left. The leave animation stops at the section's 16 px padding floor, and the section stays mounted until the animation ends. Its removal is the last resize, and the check ran before the correction for it. The test now waits for that lifecycle and then renders: - Every settle first waits two rendering updates (frames). - Arriving activity waits for the section's own animations to finish. - Leaving activity waits for the section to leave the DOM, not just the accessibility tree. The product corrects in the same rendering update, before paint; only the test's timing changes. Fail-then-pass on Linux WebKit (Playwright v1.63.0-noble container, all 16 CPUs busy, 4 workers): the original failed 9/12 (8 at the reading check after activity ends, as in CI). Fixing only the reading checks left 5/24 failing at the bottom check after the composer grows (Received 168). With every settle rendering first, 24/24 passed. Unloaded, the full file passed 36/36 (3 repeats, both engines); on macOS the test passed 8/8 (both engines). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
kalvinnchau
approved these changes
Nov 4, 2026
gilgtm
added a commit
that referenced
this pull request
Nov 4, 2026
* origin/main: (52 commits) feat(agents): give plugin agent types the Buzz side of a turn (#2364) feat(plugins): pin buzz://plugin-install links to a path and commit (#2315) feat(plugins): tint currentColor SVG page icons like host icons (#2314) feat(notifications): add a soft notification sound (#1536) feat(agents): let agents propose a profile picture (#2267) fix(plugins): default reminders and workflows off for fresh installs (#2291) Show all reactors in compact, separate emoji capsules (#2179) fix(panels): remember page split widths per workspace (#2264) feat(search): browse open channels from a Channels tab in search (#2164) perf(relay): page combined replay filters after a reconnect (#2308) fix(composer): make native Mac text shortcuts work past link chips (#2390) perf(relay): read creator proofs for many channels per filter (#2312) fix(channels): let published leave and delete confirm through access loss (#2357) fix(media): save native context menu media downloads and hide loopback links on macOS (#2131) fix(links): say why a message preview is unavailable, and preview unjoined public channels (#2136) test(browser): serve emoji artwork in the reaction hit-area journey (#2138) test(browser): click a reply link only after its row settles (#2358) fix(agents): restore delegated workflow mentions (#2399) feat(identity): always back up keys in managed builds (#2398) test(threads): render before checking the reader across activity resizes (#2389) ... Signed-off-by: Gil Birman <gil@squareup.com> Co-authored-by: Claude <noreply@anthropic.com> # Conflicts: # src/plugins/author.test.mjs
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.
Problem
thread-resize.spec.mjs:99"thread preserves bottom and reading position through activity and composer resizing" fails on Linux WebKit at line 292: the reading check after activity above the reader ends (Expected: < 4,Received: 16). It started after #1729 merged (6565eea) and failed on four unrelated branches within two hours:tho/solo-share-view-only,fix/solo-access-journey-budget,kalvin/managed-nsec-backup-consentanduser-statuss-dont-show-emoji-name-on-hover(#2384). Main's own runs at the same commits passed.Cause
ThreadPanelkeeps the reader in place by correcting each resize in itsResizeObserver. That observer runs only in a rendering update, and on a loaded Linux WebKit runner those are far apart.settle()can then see geometry rest while a correction is still pending.The CI trace shows the sequence. When activity starts,
scrollTopmoves 956 → 1032 (+76). When it stops, it moves only 1032 → 972 (−60). The leave animation stops at the section's 16 px padding floor (0frcan't collapse the child's padding), and the section stays mounted until the animation ends. Leaving also setsaria-hiddenat once, so the test'sgetByRole("region")…toHaveCount(0)passed about 60 ms afterturn_completed. The section's removal is the last resize, and the check ran before the correction for it. Arrival can fail the same way (line 281), and so can the bottom check after the composer grows (line 193).The product corrects in the same rendering update as the layout change, before paint, so users don't see a jump. Only the test's timing changes.
Change
Only
tests/browser/thread-resize.spec.mjschanges:settled(): wait two rendering updates with the existingframes()helper, thensettle(). Every settle in this test uses it.arrived(row): wait for the activity section's own animations to finish, asActivityAccessorydoes before it unmounts the section.left(row): wait for[data-message-activity]to leave the DOM, not just the accessibility tree.The shared
settle()helper is unchanged. Browser cases added or removed: none.Validation
All runs use the Playwright
v1.63.0-noblecontainer (Linux WebKit, as in CI) withplaywright.ci.config.mjs --no-depsand 4 workers. "Loaded" means all 16 container CPUs were busy.Received: 168)Instrumenting
ThreadPanelshowed the correction for the removal landing after the test's check. Logging that read geometry inside the observer callback hid the race (0/12 failures), which points to rendering-update timing, not to the correction logic.🤖 Generated with Claude Code