Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

49 changes: 2 additions & 47 deletions patches/@shopify/flash-list/details.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
1. **First `useLayoutEffect`** (measures parent container): After calling `measureParentSize()`, if both width and height are 0, return early before calling `updateLayoutParams()` or updating `containerViewSizeRef`. This preserves the last known valid window size and prevents the layout manager from receiving zero dimensions.
2. **Second `useLayoutEffect`** (measures individual items): If `containerViewSizeRef.current` is 0x0 (because the first effect bailed out), return early before calling `modifyChildrenLayout()`. This prevents item measurements taken under `display: none` (also 0) from corrupting stored layouts.
When the container becomes visible again, `onLayout` fires (React Native Web uses ResizeObserver), triggering a re-render with correct dimensions so FlashList resumes normally without re-initialization.
- Upstream PR/issue: https://github.com/Shopify/flash-list/issues/2231
- Files changed: Both `src/recyclerview/RecyclerView.tsx` and `dist/recyclerview/RecyclerView.js`. The `src/` file contains the full explanatory comments describing the intent of each guard. The `dist/` file contains only the bare code without comments, since it is compiled output. If the `dist/` file changes in a future version, refer to the `src/` diff to understand the intent and re-apply the equivalent guards.
- Upstream PR/issue: TBD
- E/App issue: https://github.com/Expensify/App/issues/83976
- PR introducing patch: https://github.com/Expensify/App/pull/84887

Expand Down Expand Up @@ -63,49 +64,3 @@
- Upstream PR/issue: TBD
- E/App issue: https://github.com/Expensify/App/issues/89768
- PR introducing patch: https://github.com/Expensify/App/pull/90218

### [@shopify+flash-list+2.3.0+009+sort-for-natural-DOM-order.patch](@shopify+flash-list+2.3.0+009+sort-for-natural-DOM-order.patch)

- Reason: Fixes scrambled DOM order in virtualized list items on web. FlashList uses `position: absolute` to position items, so visual order is determined by CSS `top`/`left` values rather than DOM order. Due to recycling (reusing ViewHolder components for different data items), the DOM order reflects Map insertion order rather than data index order. This causes three web-specific issues:

1. **Screen reader reading order**: Assistive technologies follow DOM order, so items are read in a scrambled sequence that doesn't match the visual layout.
2. **Keyboard Tab navigation**: Tab key follows DOM order, so focus jumps unpredictably between items instead of following the visual top-to-bottom sequence.
3. **Cross-item text selection**: Selecting text across multiple list items selects them in DOM order rather than visual order, producing garbled selections.

**How it works:**

1. **Stable render order during scroll**: Render entries are maintained in a ref (`renderEntriesRef`) that preserves its order across renders. On each render, a reconcile step removes keys that left the render stack and appends new keys. Because FlashList's recycling mutates index values in place on shared object references (`keyInfo.index = newIndex`), the entries in the ref always have current index values without needing updates — only the array order can be stale. This means during normal scrolling, React sees children in the same order and produces zero `insertBefore` calls, avoiding any DOM reordering.

2. **Deferred sort after scroll** (default `SORT_DELAY_MS` = 1000ms): After scrolling pauses, a single-slot `setTimeout` (armed by `schedulePendingSort`, handle in `pendingSortTimeoutRef`) sorts the ref by data index and triggers a re-render. This is the only moment React reorders DOM nodes via `insertBefore`. The delay gives the browser time to process queued pointer events (hover state cleanup) from CSS position changes before the structural DOM reorder occurs. When the timer fires, it re-checks scroll state via `isScrolling()` — if any scroll is still in progress (a freshly started mousewheel, a continued momentum scroll, etc.), the timer reschedules itself rather than committing, so a long-running scroll never lets a stale timer fire in the middle of motion. The sort uses a separate, sort-only re-render trigger (`bumpSortVersion` from a `useReducer` counter) instead of reusing FlashList's `renderId`, so the sort does not fire lifecycle callbacks (`onCommitLayoutEffect`, `onCommitEffect`) that would cause duplicate `onViewableItemsChanged` or `onEndReached` calls.

3. **Focus-aware sort triggering**: Tab navigation walks DOM order on web, so an out-of-date order makes the next Tab press land on the wrong row. A `focusin` event listener on the container resolves which logical row received focus by reading a `data-flashlist-index` DOM marker that each `ViewHolder` renders alongside its children, and routes real focus changes to `maybeDoSortOnFocus`. Spurious refocus events caused by recycling and React's mutation-phase selection-preservation are filtered out so they don't trigger a sort cascade — see [viewholder-marker-and-focus-filter.md](viewholder-marker-and-focus-filter.md) for the full filter design. Tab itself doesn't scroll, but tabbing to a row that's outside the viewport makes the browser auto-scroll to bring it into view; that scroll re-renders the list and runs a separate `maybeDoSortOnScroll` callback. The actual synchronous sort during Tab navigation happens in the scroll callback (see #4); the focus callback typically just schedules a deferred sort.

4. **Two `maybeDoSort` callbacks + programmatic-scroll gating**: The focus path and the scroll path have different decisions to make, so the original single `maybeDoSort` is split into two callbacks that cooperate via a one-shot flag (`shouldSortOnNextFocusRef`):

- **`maybeDoSortOnScroll`** runs from the effect that fires on `renderStack` / `renderId` changes — i.e. whenever recycling produced a new layout. It arms `shouldSortOnNextFocusRef`, evicts any pending-sort timer (a stale timer from a previous scroll's drain cannot fire mid-motion during rapid arrow-key repeats), then picks one of three branches:
- *Programmatic scroll queued or in flight* (`isScrollingProgrammatically()` is true): hand off via `runAfterProgrammaticScroll` → `schedulePendingSort`. Once the scroll settles we still wait an additional `SORT_DELAY_MS` for queued pointer/focus events to land before committing. The flag stays armed.
- *In-motion scroll caused by a recent focus* (`isScrolling()` is true and the last `scroll` event landed within `FOCUS_INDUCED_SCROLL_WINDOW_MS` = 30 ms after the last `focusin`): call `doSort` synchronously and reset the flag. This is the browser's auto-scroll-into-view from a Tab/focus on an off-viewport row — keeping DOM order synced is critical for the next Tab to land on the right row, even at the cost of perturbing the auto-scroll. **This is the path that does the sync sort during Tab navigation.**
- *Anything else* (user mousewheel/scrollbar/touch, or a quiet list): schedule the deferred sort. The flag stays armed for the next focusin to consume.

- **`maybeDoSortOnFocus`** runs from the `focusin` listener. It evicts any pending-sort timer; if `shouldSortOnNextFocusRef` is armed it consumes the flag and commits `doSort` synchronously; either way it then schedules a fresh deferred sort. In the common Tab → auto-scroll flow, `maybeDoSortOnScroll`'s focus-induced branch has already done the sync sort and reset the flag *before* the next focusin gets here, so the sync-sort path inside this callback is mainly a safety net for scroll-less re-renders and for the programmatic-scroll branch (where the flag was armed but no sync sort fired).

The deferred-sort timer is provided by `useDeferredCallback`, a small inline hook that wraps a single-slot `setTimeout` with a fire-time `shouldDefer` predicate. When the timer expires it re-checks `isScrolling()` and reschedules itself if a scroll is still in progress, so a long-running scroll never lets a stale timer fire in the middle of motion. The "scroll has truly ended" signal driving the programmatic-defer drain is FlashList's existing `isMomentumEnd`, fired by `VelocityTracker` ~100 ms after the last `scroll` event — distance-independent and naturally overlap-safe (the browser merges overlapping smooth scrolls into one).

5. **Pre-scroll announcement (`queueProgrammaticScroll`)**: A new public method on `FlashListRef` lets the consumer announce an imminent programmatic scroll *before* `scrollToIndex` is actually called. It flips an "is queued" ref that `isScrollingProgrammatically()` already ORs in, so any sort triggered by an intervening event (notably the `focusin` that fires when the consumer focuses the target row first and only then calls `scrollToIndex`) is correctly held off rather than committing immediately and cancelling the upcoming smooth scroll. The queued flag is handed off to the in-flight ref at `scrollToIndex` entry and finally cleared when the scroll settles, so it cannot get stuck on.

**Why the deferred approach is necessary:**

Two distinct web-only hazards make immediate, mid-scroll DOM reordering wrong:

1. **Hover/pointer state loss**: When recycling moves items to new CSS positions, the browser queues `mouseleave`/`pointerleave` events for elements that are no longer under the pointer. However, if `insertBefore` executes before the browser has processed those queued pointer events, the structural DOM move interferes with the browser's hover tracking — the pending `mouseleave` is effectively lost, and recycled items retain stale hover/tooltip states. Keeping the array order stable during scrolling and only committing after the list goes idle gives the browser time to drain those events before any reorder.

2. **Smooth-scroll cancellation**: When a list row is focused and a sort commit lands during an in-flight smooth `scrollToIndex`, React's commit-time selection-preservation logic saves and writes back `scrollTop` on every scrollable ancestor of the focused element (including the FlashList scroll container). Per CSSOM, writing `scrollTop` performs an instant scroll, which aborts any in-flight `behavior: 'smooth'` animation on that element — the visible "scroll starts then freezes" symptom on long arrow-key navigations. The programmatic-scroll gating in both `maybeDoSort*` callbacks keeps commits out of the smooth-scroll window, so a `scrollToIndex` animation lands only after it has truly ended (`isMomentumEnd`). Browser auto-scroll-into-view triggered by Tab focusing an off-viewport row is intentionally *not* gated this way (see #4 above) — Tab-navigation correctness takes priority over preserving that auto-scroll's centring.

**Platform gating:**

On web: render entries are held in the order-preserving ref, the deferred sort fires after scrolling pauses, the `focusin` listener (filtered via the `data-flashlist-index` marker) routes real focus changes through `maybeDoSortOnFocus`, and `maybeDoSortOnScroll` decides per-render whether to sort synchronously, defer until momentum-end, or defer the standard `SORT_DELAY_MS`. The deferred path itself reschedules until any scroll has settled, via `useDeferredCallback`'s timer-fire `isScrolling()` re-check.
On non-web: the ref is set to a fresh `Array.from(renderStack.entries())` on every render, preserving original behavior identically. The marker JSX, the focusin listener, and both `maybeDoSort*` callbacks are gated to web only.

- Upstream PR/issue: https://github.com/Shopify/flash-list/issues/1955
- E/App issue: https://github.com/Expensify/App/issues/86126
- PR introducing patch: https://github.com/Expensify/App/pull/85825
3 changes: 0 additions & 3 deletions src/components/Search/SearchList/BaseSearchList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ function BaseSearchList({
onFocusedIndexChange: (index: number) => {
scrollToIndex?.(index);
},
onArrowUpDownCallback: () => {
ref?.current?.announceProgrammaticScroll();
},
setHasKeyBeenPressed,
isFocused,
captureOnInputs: false,
Expand Down
4 changes: 2 additions & 2 deletions src/components/Search/SearchList/BaseSearchList/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {FlashListProps, FlashListRef} from '@shopify/flash-list';
import type {RefObject} from 'react';
import type {ForwardedRef} from 'react';
import type {NativeSyntheticEvent} from 'react-native';
import type {SearchListItem} from '@components/Search/SearchList/ListItem/types';
import type {SearchColumnType, SelectedTransactions} from '@components/Search/types';
Expand Down Expand Up @@ -37,7 +37,7 @@ type BaseSearchListProps = Pick<
onSelectRow: (item: SearchListItem) => void;

/** The ref to the list */
ref: RefObject<FlashListRef<SearchListItem> | null>;
ref: ForwardedRef<FlashListRef<SearchListItem>>;

/** The function to scroll to an index */
scrollToIndex?: (index: number, animated?: boolean) => void;
Expand Down
9 changes: 5 additions & 4 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ function BaseSelectionList<TItem extends ListItem>({

const debouncedScrollToIndex = useDebounce(scrollToIndex, CONST.TIMING.LIST_SCROLLING_DEBOUNCE_TIME, {leading: true, trailing: true});

const onArrowUpDownCallback = useCallback(() => {
setShouldDisableHoverStyle(true);
}, [setShouldDisableHoverStyle]);

const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({
initialFocusedIndex,
maxIndex: data.length - 1,
Expand All @@ -222,10 +226,7 @@ function BaseSelectionList<TItem extends ListItem>({
},
setHasKeyBeenPressed,
isFocused,
onArrowUpDownCallback: () => {
setShouldDisableHoverStyle(true);
listRef.current?.announceProgrammaticScroll();
},
onArrowUpDownCallback,
});

// Keep the cursor on the restored row so keyboard nav continues from there, but don't scroll to it on the way back.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,7 @@ function BaseSelectionListWithSections<TItem extends ListItem>({
},
setHasKeyBeenPressed,
isFocused: isScreenFocused,
onArrowUpDownCallback: () => {
setShouldDisableHoverStyle(true);
listRef.current?.announceProgrammaticScroll();
},
onArrowUpDownCallback: () => setShouldDisableHoverStyle(true),
});

// Move the cursor, and skip the scroll the move would otherwise trigger when the index actually changes.
Expand Down
Loading