WIP: try legends on native again - #29361
Open
chrisnojima wants to merge 6 commits into
Open
Conversation
chrisnojima
force-pushed
the
nojima/HOTPOT-legends-310
branch
2 times, most recently
from
July 9, 2026 19:16
c11edb7 to
b7612ce
Compare
chrisnojima
force-pushed
the
nojima/HOTPOT-message-fixes
branch
from
July 9, 2026 19:23
a621b5c to
32a520c
Compare
chrisnojima
force-pushed
the
nojima/HOTPOT-legends-310
branch
from
July 9, 2026 19:23
b7612ce to
5e96b94
Compare
chrisnojima
force-pushed
the
nojima/HOTPOT-legends-310
branch
2 times, most recently
from
July 17, 2026 20:39
e016702 to
9d9678d
Compare
…ights These are list-agnostic row fixes, split out of the native LegendList work. Desktop already renders the thread with a recycling LegendList, so each of these is a live bug there today; on the native FlatList they are inert guards. - per-row state (sent animation, reaction picker, exploding retained height, ash tower) is now keyed to messageKey. A recycled container reuses the component instance for a different message, so mount-captured state leaked across rows: the picker stayed open on the wrong row, the sent-animation wrapper stuck on, and a measured retain height was applied to the wrong message (and could not self-correct, since retainHeight forces the style height so onLayout only ever reports the forced value back) - recycle-pool suffixes are limited to ones that are stable for the message's lifetime (:failed, :reply). The pool label is recorded when a container is allocated and never updated in place, so :pending (flips on every send confirmation) and :reactions (toggles) left stale labels behind and recycled containers painted at the wrong pooled height - getItemType splits headered rows into their own pool (:hdr). A message that leads its author group is ~40px taller than a grouped follow-on of the same render type, so a shared pool paints recycled views at the wrong height for a frame. It reads the same sticky username cache the rows render with, else a row that keeps its header after a scroll-back load is typed headerless and poisons the headerless pool's height average - useSyncRowLayout: when a row's content settles to a new height after first paint (flip result streams in, reactions appear, an unfurl loads), flush the row measure synchronously so the list's bottom re-pin uses the final height on the same frame instead of a frame late, which otherwise parks the thread above the newest message - SwipeableRow takes an `enabled` prop that conditionally spreads its pan handlers, so a list can shed per-row touch evaluation during a fast fling without unmounting the row. Toggling the Swipeable's subtree instead would remount its children and flash images. No caller passes it yet - desktop LegendList: dataKey replaces the key remount on conversation switch, and maintainScrollAtEnd opts back into footerLayout, which 3.x stopped implying once the trigger set is given explicitly
Replaces the inverted FlatList + KeyboardChatScrollView native thread with a non-inverted KeyboardAwareLegendList, on top of the list-agnostic row fixes in the parent commit. - non-inverted list: rows render in natural order, so the sent-message animation, separators, and header/footer components swap ends. Bottom clearance for the input bar is reserved statically via contentContainerStyle, with the keyboard composer inset seeded to 0 so the two don't stack. - pagination: onStartReached/onEndReached replace the viewability-window heuristic (useNativeSafeOnViewableItemsChanged is deleted). - prepend jump: maintainScrollAtEnd's dataChange trigger re-pins on ANY data change within maintainScrollAtEndThreshold of the end, so on short threads a load-older prepend yanked the view to the bottom. The threshold stays wide (0.5) because initialScrollAtEnd positions from estimatedItemSize and lands short on our tall rows; instead the re-pin is suspended while a prepend is in flight (prependActive). - initial position: the list is not mounted until the thread is loaded, so its first render always has data and initialScrollAtEnd lands at the newest message. Previously, arriving from the inbox mounted the list empty and the initial scroll ran against no data and never re-fired. - centering: scrollToItem(viewPosition: 0.5) lands accurately here, so the closed-loop offset corrector (viewable-range feedback, damped item-delta scrolls) is deleted in favor of re-asserting across the pagination settle. onScrollToIndexFailed goes with it — LegendList has no such prop. - maintainVisibleContentPosition is on from mount so prepends hold position. - fling cost: experimental_adaptiveRender lets rows shed their swipe pan handlers during fast scroll, via useAdaptiveRender in long-pressable driving the SwipeableRow `enabled` prop added in the parent commit. useAdaptiveRender reads LegendList's state context and throws outside a LegendList, so it can only be wired up here. The Swipeable stays mounted — toggling its tree would remount children and flash images. - stable renderItem: NativeRow reads the centered highlight itself, so renderItem identity never changes and a highlight change doesn't re-render every visible row. Includes a temporary [LISTDBG] dump for the initial-load settle; remove before merging.
…ssal The keyboard-hide contentOffset unwind shifts LegendList's draw window, so estimate-only rows measure in at their real sizes mid-dismiss; LegendList's JS MVCP compensates against that moving target and settles short of the end, leaving the newest message below the fold. Re-pin frame-by-frame from keyboardWillHide until shortly after keyboardDidHide when we were at the end when the hide started.
chrisnojima
force-pushed
the
nojima/HOTPOT-legends-310
branch
from
August 10, 2026 22:29
f20e0f9 to
8a016ed
Compare
Two defects kept a search hit from landing where it was asked to, one per platform, both fixed in the library rather than worked around here. On desktop the scroll extent legend-list clamps against is measured from the DOM, which lags a React commit. The jump to a hit runs on the tick the thread data changes, so the requested offset was clamped to a stale extent - usually 0 - and the thread sat at the top. On native the scroll landed against estimated row heights and nothing re-aimed once the rows above measured taller, so hits drifted off screen. With the library fixed, the elaborate app-side compensation is unnecessary: the desktop centering loop, the native re-assert timers, the prepend guard and the keyboard re-pin loop all come out, leaving a single scrollToIndex per target. maintainVisibleContentPosition on native now stays mounted with one config instead of being switched off while centered. It is what holds the hit in place while the centered load streams older messages in, and toggling the prop makes the list jump. Patch regenerated from the fork build; both fixes are proposed upstream.
Review of the patch found three user-visible problems with the first cut, all fixed upstream in the fork and regenerated here. - The web fix gated every imperative scroll on the document extent matching the list's content size. Those disagree permanently whenever contentInset is in play, since web never applies the base inset to layout, so every scroll would have waited out the full 800ms readiness timeout. It is now a bounded re-issue inside the web scroll view, which also covers the initial scroll, scroll adjustments and completion checks rather than only the imperative path. - Settling a scroll target cancelled on onScrollBeginDrag, which is not wired on web. Scrolling within a second of jumping to a search hit yanked the list back. Cancellation is now driven by a scroll event landing somewhere other than where it was asked to, which covers wheel, trackpad, scrollbar and keyboard. - Settling armed for animated scrolls too, turning them into an instant jump, and corrected from inside the layout pass rather than after it. Also bumps @legendapp/list to 3.3.5 and drops the stale 3.3.4 patch, matching what #29526 did on master. The patch here targets 3.3.5, so without the bump patch-package would silently skip it on a clean install.
Both fixes had blockers a review caught, so the patch is regenerated from the reworked fork branches. Web scroll re-issue: - it kept scrolling after the list unmounted, which in window scroll mode meant a dead component scrolling the page for up to ten frames - scrollToEnd never re-issued at all, because it asks for the extent it just measured and so was never out of reach - the one path that matters most when content has just grown - retries now stop as soon as the extent stops growing rather than always burning the full frame budget, and interference is judged against where the scroll actually landed so a platform that clamps differently is not mistaken for the user Scroll target settling: - the user-scroll release was dead code: onScroll assigns state.scrollPending to the incoming offset immediately before calling updateScroll, so the comparison was always zero. Corrections no longer claim the scroll session, which lets the existing user-scroll detection do the work - the correction budget was spent per layout pass rather than per correction, and layout runs many passes per frame while items measure, so on the very lists this targets it could spend the whole budget and issue nothing - maintainScrollAtEnd now releases the target when it requests the end anchor rather than a frame later, so a queued correction cannot slip in first
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.
No description provided.