feat(desktop): re-land virtualized timeline to fix macOS beachball#1250
Open
wpfleger96 wants to merge 6 commits into
Open
feat(desktop): re-land virtualized timeline to fix macOS beachball#1250wpfleger96 wants to merge 6 commits into
wpfleger96 wants to merge 6 commits into
Conversation
Port the virtualized timeline subsystem onto main's day-group render structure, re-threading the read-marker work through the virtualized rows. main builds every row synchronously on first mount, so cold channel-switch cost climbs with channel depth; virtualization renders only the visible window, making cost independent of depth. Ports timelineItems/scrollConvergence (+ lib tests), useLoadOlderOnScroll, useConvergentScrollToMessage, and the virtualizer-index restore in useAnchoredScroll. main's unread-counter fix is preserved, confined to the unread-count increment block. The two perf-hoist props the reference branch passed into MessageRow are dropped: virtualization already bounds rendered rows to the visible window, so the hoist optimizes a cost the mechanism eliminates, and MessageRow stays untouched. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The timeline-virtualization acceptance gate is a same-harness delta (header-arm longest-longtask <= B + 15ms), but the instrument that produced the baseline was ad-hoc and never committed, so it evaporated between sessions. Commit it so the gate's own instrument survives. Measures main-thread longtasks during the first (cold) switch into the 600-message deep-history channel under 4x CPU throttle, windowed to the 300-row ceiling. Reports median-of-5 longest-longtask, run-to-run spread, and total-longtask-time-in-window. Instrument, not a gate: the only assertion confirms the switch exercised the mount under throttle. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ized timeline A sticky header inside the scroll container drifts ~49px at scrollTop 0: once older history prepends and the scroll restores, the header pins to its clamped offset, but at the top it had been sitting at its larger natural flow offset. The fix portals the header into a non-scrolling overlay container outside the scroll element (mirroring the unread-pill overlay), so it pins to a fixed offset regardless of scroll position and cannot move as content prepends above the anchor. The per-scroll re-render that resolves the topmost visible day stays localized to VirtualizedList rather than forcing MessageTimeline to re-render on every scroll. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The timeline-virtualization port regressed scroll-anchoring: it dropped
the non-virtualized list's implicit "full content height exists at pin
time" invariant. Rows mount estimate-sized and measure to real height on
scroll, so the raw `scrollTo(scrollHeight)` mount pin landed short and
`scrollHeight` grew as off-screen rows measured.
Drive the mount bottom-pin through `virtualizer.scrollToIndex(lastIndex,
{align:"end"})` so TanStack lands the true last row through its own
measurement. Arm the existing settle-guard on smooth `scrollToBottom`
too — an animated jump is not atomic, so a mid-animation `onScroll`
latched a stale mid-history anchor that the ResizeObserver then restored.
Teach the prepend-restore loop to re-aim at the bottom when the user
abandons to bottom mid-restore, and the all-gone fallback to keep a
windowed-out anchor (vs. only pinning on genuine deletion).
The teleport spec's `scrollHeight <= baseline+100` setup proxy assumed
the non-virtualized contract (scrollHeight changes only on DOM adds); a
virtualizer grows `getTotalSize()` from lazy measurement alone. Replace
it with the direct in-flight signal the suite already keys on. Seed loops
that omitted `createdAt` collided on one whole-second stamp and sorted by
random UUID, so the asserted last/target row landed at a random index
often outside the virtualized window — make them monotonic to match the
file's own channel-intro seed precedent. No product-property assertion
text changed.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…lization The prior helper asserted symmetric balance (gapAbove == gapBelow within 1px), valid when intro, divider, and first row shared one flow layout. The virtualization re-land moved the divider and first row into the translateY track while the intro stays a flex sibling, so the two gaps are no longer comparable quantities. The fix had collapsed the oracle to bare non-overlap (>= 0 on both gaps), which gutted the layout-regression guard the test exists to provide. Source measurement showed the intro -> divider gap is the layout-controlled 8px and rock-stable, while the divider -> message gap is ~0 by construction (virtualizer rows are back-to-back) plus MessageRow avatar/font render jitter, genuinely variable run-to-run. So band the stable gap (8 +/- 2) as the real guard and keep the variable one as a non-overlap reading-order check. Renamed to match what it now verifies. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
force-pushed
the
duncan/timeline-virtualization-reland
branch
from
be2d9e5 to
cbfc4e6
Compare
The keyed scroll container remounts per channel, but the virtualizer and the anchored-scroll ResizeObserver are owned by the parent and persisted across switches, so on switch-back they kept pointing at the previous channel's detached nodes. The mount-pin then fired scrollToIndex against a stale virtualizer (scrollElement on a detached node), landing at the top instead of the bottom, and the late-measurement bottom-chase never ran — so the top-anchored channel intro painted when it should be windowed out. Match the JS objects' lifetimes to the scroll node's: key TimelineMessageList on channelId so the virtualizer remounts fresh, register it in a layout effect so the parent's same-commit mount-pin reads the fresh instance, and add channelId to the observer deps so it re-observes the live content node. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
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
1 participant
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.
Re-lands timeline virtualization on the desktop client, eliminating the macOS scroll beachball caused by rendering the entire message list into the DOM. Only the visible window is now mounted, and scroll anchoring is rebuilt to behave correctly under virtualization.
What changes
TimelineMessageListandVirtualizedListmount only the windowed rows instead of the full history.MessageTimelinelands at the true bottom on mount viascrollToIndex(lastIndex, { align: "end" })so the initial view is pinned correctly on the TanStack measurement path.useAnchoredScroll,useConvergentScrollToMessage,useLoadOlderOnScroll, andscrollConvergencepreserve the user's scroll position across history loads and deep-link/teleport navigation, with a settle guard so smooth-scroll animations can't latch a mid-animation anchor.ActiveDayHeaderprojects the current day divider as a drift-immune floating header over the virtualized list.cold-switch-longtask.perf.ts) measures channel-switch main-thread cost so the beachball budget is guarded going forward.scroll-history.spec.tsthat omittedcreatedAtcollided on a single whole-second stamp and sorted by random UUID — latent on the non-virtualized DOM, but decisive once only the windowed rows are mounted. Those loops now seed monotoniccreatedAt, matching the existing in-file precedent, with assertions unchanged. Thechannels.spec.tsintro/day-divider spacing oracle is re-expressed to band the layout-controlledgap-2(8px) spacing and assert non-overlap on the back-to-back row gap.Known follow-ups (non-blocking)
>=comparator in the anchor restore path is looser than its comment claims.Supersedes #1123 (abandoned hybrid attempt).