feat(thread): anchor the thread on the tapped note, never auto-pin#225
Open
Kelbie wants to merge 3 commits into
Open
feat(thread): anchor the thread on the tapped note, never auto-pin#225Kelbie wants to merge 3 commits into
Kelbie wants to merge 3 commits into
Conversation
Opening a thread on a reply painted in stages — a seed/skeleton rendered the tapped note first, then the full result prepended the parent chain above it and rows snapped from estimated to measured height — both moving the focused note under the reader's thumb. The old defense was an opacity-0-until-settled reveal that masked only the sort-tabs row. Anchor the LegendList on the target via maintainVisibleContentPosition so the parent chain fills in off-screen above and a height snap never shifts it, and keep initialScrollIndex landing on the note. Deliberately omit maintainScrollAtEnd: replies load below without yanking the view down (a thread is not a chat). Remove the now-redundant opacity-reveal hack. Docs: ADR 0003 (thread list anchoring) + CONTEXT.md glossary.
Structural health |
maintainVisibleContentPosition holds the anchor against row resize and reply append, but not against the parent chain arriving after the target has painted: a prepended parent first lays out at the flat estimatedItemSize, the data-anchor corrects against that estimate, then the parent measures to its real (variable, taller) height and the delta shoves the focused note down — a known upstream gap for variable-height prepends. Drive an explicit one-shot scrollToIndex(targetIndex) when parents first appear. An active scroll target is re-resolved on every layout pass, so it pins the note through the measurement settle. Fires once per thread and bails if the reader has already dragged the list (onScrollBeginDrag). The DM screen doesn't need this — uniform short bubbles make the estimate error negligible.
The one-shot scrollToIndex completes against the parent's estimated height, so when the parent then measures to its real (taller) height the focused note still drops — the library's size-anchor corrects against the estimate, not the measurement (a known upstream gap). Re-assert the pin from onItemSizeChanged whenever a row ABOVE the target changes size, i.e. as the parent chain measures. The height delta then fills in off-screen above instead of shoving the note down. Self-correcting (scrollToIndex is absolute, no drift), stops once parents settle, and bails once the reader has dragged the list.
Kelbie
added a commit
that referenced
this pull request
Jun 22, 2026
Live on-device trace (now that the build actually runs this code) showed the residual: mVCP scrolls to hold the focused note when the parent prepends, but it anchors against the parent's estimatedItemSize (200) and never reconciles the gap when the parent measures to its real height (145) — leaving the note off by exactly estimate-measured (here +55px up; an earlier media parent measured 523 -> -323px down). A single global estimate can't cover parents ranging ~145-523px, and v3 has no per-item estimate. Own that one reconciliation: when a row above the note changes size before the reader has scrolled, counter-scroll by the delta (scrollToOffset(scroll + size - previous)). One-time per parent measure (not a sustained re-pin, so it doesn't race mVCP like PR #225 did); onScrollBeginDrag hands control back to the user. ADR 0004 updated.
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.
What & why
Opening a thread on a reply reshuffled content under the thumb. It paints in stages: a seed (own-note /
threadSeedCache) or skeletons render the tapped note first, then the fullgetThreadresult replaces it with[...parents, target, ...replies]— prepending the parent chain above the already-visible note — and rows snap fromestimatedItemSizeto measured height a frame later. Both moved the focused note.initialScrollIndexonly fires at mount, so it couldn't hold the note once parents arrived; the prior defense was an opacity-0-until-handleTargetSettledreveal that masked only the sort-tabs row.Change
Anchor the thread
LegendListon the tapped note (same mechanism the DM screen already uses for prepended older messages):maintainVisibleContentPosition— holds the first visible row (the target), so parents fill in off-screen above and a height snap above the anchor never shifts it.initialScrollIndexlanding on the note ("start at the bottom" = scrolled past the parent chain).maintainScrollAtEnd— a thread is not a chat: replies appended below (initial fill orloadMoreReplies) don't yank the view down.revealOpacity/handleTargetSettled+ sort-tabs wrapper) —maintainVisibleContentPositionsubsumes it (no-shim policy).getFixedItemSize/estimatedItemSizestay (they feed offset math + virtualization, not just shift-masking).Net: +34 / −43, reduction-heavy.
Not doing (deliberately)
inverted(unsupported by legend-list; breaks animations) and noalignItemsAtEnd/initialScrollAtEnd(those anchor the newest reply, not the tapped note).maintainVisibleContentPosition. Forced abstraction avoided.@legendapp/listbump (stays 3.0.0; flagged v3.0.4/3.0.6 fixes as a follow-up).Verification
tsc --noEmitclean · eslint 0 errors (15 pre-existingreact-perfwarnings) · knip orphaned nothing · jest green except one pre-existing, unrelated failure innaggFeedClient.test.ts(confirmed failing onmainwith this change stashed).thread.shift.*/visualListtaxonomy — not unit-testable. Manual checks: open a thread on a reply (seed + skeleton paths), watch parents stream in above and replies load below → no jump, no auto-scroll; change reply sort → anchor holds.Docs
docs/adr/0003-thread-list-anchoring.md— the anchoring decision + alternatives.CONTEXT.md— "Thread reading" glossary (thread anchor,maintainVisibleContentPosition).🤖 Generated with Claude Code