Skip to content

WIP: try legends on native again - #29361

Open
chrisnojima wants to merge 6 commits into
masterfrom
nojima/HOTPOT-legends-310
Open

WIP: try legends on native again#29361
chrisnojima wants to merge 6 commits into
masterfrom
nojima/HOTPOT-legends-310

Conversation

@chrisnojima

Copy link
Copy Markdown
Contributor

No description provided.

@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-legends-310 branch 2 times, most recently from c11edb7 to b7612ce Compare July 9, 2026 19:16
@chrisnojima
chrisnojima changed the base branch from master to nojima/HOTPOT-message-fixes July 9, 2026 19:17
@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-message-fixes branch from a621b5c to 32a520c Compare July 9, 2026 19:23
@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-legends-310 branch from b7612ce to 5e96b94 Compare July 9, 2026 19:23
Base automatically changed from nojima/HOTPOT-message-fixes to master July 9, 2026 20:34
@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-legends-310 branch 2 times, most recently from e016702 to 9d9678d Compare July 17, 2026 20:39
…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
chrisnojima force-pushed the nojima/HOTPOT-legends-310 branch from f20e0f9 to 8a016ed Compare August 10, 2026 22:29
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant