Skip to content

Commit 3884903

Browse files
ramya-2196facebook-github-bot
authored andcommitted
Use the sticky wrapper's layout for sticky header cells (#59199)
Summary: ### What `VirtualizedList` now records the position of a sticky header cell from its sticky wrapper, instead of from the cell's own layout. This affects `FlatList` with `stickyHeaderIndices` and `SectionList` with sticky section headers. JS only; no new props or public API. - `VirtualizedList.js`: sticky cells get an `onLayout` callback, and that layout takes priority over the cell's own `onCellLayout` measurement. - `VirtualizedListCellRenderer.js`: adds the optional `onLayout` prop type. - `VirtualizedList.macos.js`, `VirtualizedList.windows.js`, `VirtualizedListCellRenderer.macos.js`: the same change. ### Why Fixes part of #37439. With sticky headers, `onViewableItemsChanged` never reports the next sticky header or any item after it, even when they are fully on screen. `ScrollView` wraps every sticky child in `ScrollViewStickyHeader`. The cell's inner `View` therefore measures its layout relative to that wrapper, so a sticky cell's offset is always about 0 instead of its position in the list. Viewability scans cells in order and stops at the first cell that is not visible after a visible one. A sticky header stored at offset 0 looks scrolled off the top, so the scan stops there and drops it and everything after it. Example: groups of one 50px header and five 80px rows, scrolled to y=254 with header A pinned. Header B (at 450) and B1-B4 are on screen, but only `A4, A5` are reported. ### How `ScrollViewStickyHeader` already calls the child element's `onLayout` with the wrapper's layout, which is relative to the list content. Since cells moved their layout callback to `onCellLayout`, nothing received it. `VirtualizedList` now passes `onLayout` to sticky cells and records that layout. The cell's own measurement is still recorded until the wrapper reports, so a custom `StickyHeaderComponent` that does not forward `onLayout` behaves exactly as before. Once the wrapper has reported, later inner measurements for that cell are ignored. The flag is cleared when the cell unmounts or stops being sticky. Alternative considered: changing `ScrollViewStickyHeader` to forward `onCellLayout` as well. Rejected because it would teach `ScrollView` about a list-specific prop. ### For Reviewers - This does not fix the other half of #37439: items hidden behind the pinned header are still reported as viewable. That needs a separate change in `ViewabilityHelper`. - Sticky cell offsets also feed render windowing, `scrollToIndex`, and fill-rate tracking, so those now use the correct positions too. Changelog: [General][Fixed] - Fix `onViewableItemsChanged` skipping the next sticky header and the items after it in `FlatList` and `SectionList` Differential Revision: D126073361
1 parent 47f744d commit 3884903

5 files changed

Lines changed: 303 additions & 3 deletions

File tree

‎packages/react-native/Libraries/Lists/__tests__/FlatList-itest.js‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,65 @@ describe('<FlatList>', () => {
584584
expect(viewableKeys).toContain('3');
585585
});
586586

587+
it('reports the next sticky header and the items after it', () => {
588+
// Two groups, each one 50px sticky header followed by five 80px rows:
589+
// header A at 0, rows A1-A5, header B at 450, rows B1-B5.
590+
const data = ['A', 'B'].flatMap(group => [
591+
{key: `header ${group}`, height: 50},
592+
...Array.from({length: 5}, (_, i) => ({
593+
key: `${group}${i + 1}`,
594+
height: 80,
595+
})),
596+
]);
597+
const root = Fantom.createRoot({
598+
viewportWidth: 400,
599+
viewportHeight: 600,
600+
});
601+
const onViewableItemsChanged = jest.fn();
602+
const flatListRef = createRef<FlatList<{key: string, height: number}>>();
603+
const timers = Fantom.installTimerMock();
604+
try {
605+
Fantom.runTask(() => {
606+
root.render(
607+
<FlatList
608+
ref={flatListRef}
609+
data={data}
610+
renderItem={({item}) => (
611+
<View style={{height: item.height}} collapsable={false} />
612+
)}
613+
stickyHeaderIndices={[0, 6]}
614+
initialNumToRender={data.length}
615+
viewabilityConfig={{itemVisiblePercentThreshold: 100}}
616+
onViewableItemsChanged={onViewableItemsChanged}
617+
/>,
618+
);
619+
});
620+
621+
const scrollView = nullthrows(
622+
nullthrows(flatListRef.current).getNativeScrollRef(),
623+
);
624+
625+
// Header A is pinned. Header B (at 450) and B1-B4 are fully on screen.
626+
Fantom.scrollTo(scrollView, {x: 0, y: 254});
627+
// Flush the render batches scheduled by cell layout events.
628+
timers.runAllTimers();
629+
Fantom.runWorkLoop();
630+
631+
const lastCall =
632+
onViewableItemsChanged.mock.calls[
633+
onViewableItemsChanged.mock.calls.length - 1
634+
][0];
635+
const viewableKeys = lastCall.viewableItems.map(
636+
(item: {key: string, ...}) => item.key,
637+
);
638+
for (const key of ['header B', 'B1', 'B2', 'B3', 'B4']) {
639+
expect(viewableKeys).toContain(key);
640+
}
641+
} finally {
642+
timers.uninstall();
643+
}
644+
});
645+
587646
it('supports viewabilityConfigCallbackPairs', () => {
588647
const root = Fantom.createRoot({
589648
viewportWidth: 400,

‎packages/react-native/Libraries/Lists/__tests__/SectionList-itest.js‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import * as Fantom from '@react-native/fantom';
1616
import nullthrows from 'nullthrows';
1717
import * as React from 'react';
1818
import {createRef} from 'react';
19-
import {SectionList, Text} from 'react-native';
19+
import {SectionList, Text, View} from 'react-native';
2020

2121
type Item = {key: string};
2222

@@ -150,6 +150,75 @@ describe('<SectionList>', () => {
150150
</rn-scrollView>,
151151
);
152152
});
153+
154+
it('reports the next sticky section header and the items after it', () => {
155+
// Two sections, each one 50px sticky header followed by five 80px rows:
156+
// header A at 0, rows A1-A5, header B at 450, rows B1-B5.
157+
const sections: Array<SectionBase<Item>> = ['A', 'B'].map(name => ({
158+
key: name,
159+
data: Array.from({length: 5}, (_, i) => ({key: `${name}${i + 1}`})),
160+
}));
161+
const root = Fantom.createRoot({
162+
viewportWidth: 400,
163+
viewportHeight: 600,
164+
});
165+
const onViewableItemsChanged = jest.fn();
166+
const sectionListRef = createRef<SectionList<Item>>();
167+
const timers = Fantom.installTimerMock();
168+
try {
169+
Fantom.runTask(() => {
170+
root.render(
171+
<SectionList
172+
ref={sectionListRef}
173+
stickySectionHeadersEnabled={true}
174+
sections={sections}
175+
renderItem={() => (
176+
<View style={{height: 80}} collapsable={false} />
177+
)}
178+
renderSectionHeader={() => (
179+
<View style={{height: 50}} collapsable={false} />
180+
)}
181+
initialNumToRender={20}
182+
viewabilityConfig={{itemVisiblePercentThreshold: 100}}
183+
onViewableItemsChanged={onViewableItemsChanged}
184+
/>,
185+
);
186+
});
187+
188+
const scrollView = nullthrows(
189+
nullthrows(
190+
nullthrows(sectionListRef.current).getScrollResponder(),
191+
).getNativeScrollRef(),
192+
);
193+
194+
// Header A is pinned. Header B (at 450) and B1-B4 are fully on screen.
195+
Fantom.scrollTo(scrollView, {x: 0, y: 254});
196+
// Flush the render batches scheduled by cell layout events.
197+
timers.runAllTimers();
198+
Fantom.runWorkLoop();
199+
200+
const lastCall =
201+
onViewableItemsChanged.mock.calls[
202+
onViewableItemsChanged.mock.calls.length - 1
203+
][0];
204+
const viewableLabels = lastCall.viewableItems.map(
205+
(token: {
206+
index: ?number,
207+
item: Item,
208+
section: {key: string, ...},
209+
...
210+
}) =>
211+
token.index == null
212+
? `header ${token.section.key}`
213+
: token.item.key,
214+
);
215+
for (const label of ['header B', 'B1', 'B2', 'B3', 'B4']) {
216+
expect(viewableLabels).toContain(label);
217+
}
218+
} finally {
219+
timers.uninstall();
220+
}
221+
});
153222
});
154223

155224
describe('renders section footer', () => {

‎packages/virtualized-lists/Lists/VirtualizedList.js‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -959,8 +959,12 @@ class VirtualizedList extends StateSafePureComponent<
959959
const key = VirtualizedList._keyExtractor(item, ii, this.props);
960960

961961
this._indicesToKeys.set(ii, key);
962-
if (stickyIndicesFromProps?.has(ii + stickyOffset)) {
962+
const isStickyHeader =
963+
stickyIndicesFromProps?.has(ii + stickyOffset) === true;
964+
if (isStickyHeader) {
963965
stickyHeaderIndices.push(cells.length);
966+
} else {
967+
this._stickyHeaderKeysLaidOutByWrapper.delete(key);
964968
}
965969

966970
const shouldListenForLayout =
@@ -988,6 +992,14 @@ class VirtualizedList extends StateSafePureComponent<
988992
{...(shouldListenForLayout && {
989993
onCellLayout: this._onCellLayout,
990994
})}
995+
// `ScrollViewStickyHeader` calls this with the layout of its wrapper,
996+
// which is relative to the list content.
997+
onLayout={
998+
shouldListenForLayout && isStickyHeader
999+
? (event: LayoutChangeEvent) =>
1000+
this._onStickyHeaderCellLayout(event, key, ii)
1001+
: undefined
1002+
}
9911003
/>,
9921004
);
9931005
prevCellKey = key;
@@ -1481,6 +1493,8 @@ class VirtualizedList extends StateSafePureComponent<
14811493
_scrollRef: ?React.ElementRef<typeof ScrollView> = null;
14821494
_sentStartForContentLength = 0;
14831495
_sentEndForContentLength = 0;
1496+
// Sticky header cells whose position comes from their sticky wrapper.
1497+
_stickyHeaderKeysLaidOutByWrapper: Set<string> = new Set();
14841498
_updateCellsToRenderTimeoutID: ?ReturnType<typeof setTimeout> = null;
14851499
_viewabilityTuples: Array<ViewabilityHelperCallbackTuple> = [];
14861500

@@ -1543,6 +1557,28 @@ class VirtualizedList extends StateSafePureComponent<
15431557
cellKey: string,
15441558
cellIndex: number,
15451559
): void => {
1560+
// A sticky header cell measures itself relative to its sticky wrapper, so
1561+
// prefer the wrapper's layout once it has been reported.
1562+
if (this._stickyHeaderKeysLaidOutByWrapper.has(cellKey)) {
1563+
return;
1564+
}
1565+
this._handleCellLayout(e, cellKey, cellIndex);
1566+
};
1567+
1568+
_onStickyHeaderCellLayout = (
1569+
e: LayoutChangeEvent,
1570+
cellKey: string,
1571+
cellIndex: number,
1572+
): void => {
1573+
this._stickyHeaderKeysLaidOutByWrapper.add(cellKey);
1574+
this._handleCellLayout(e, cellKey, cellIndex);
1575+
};
1576+
1577+
_handleCellLayout(
1578+
e: LayoutChangeEvent,
1579+
cellKey: string,
1580+
cellIndex: number,
1581+
): void {
15461582
const layoutHasChanged = this._listMetrics.notifyCellLayout({
15471583
cellIndex,
15481584
cellKey,
@@ -1558,7 +1594,7 @@ class VirtualizedList extends StateSafePureComponent<
15581594
this._computeBlankness();
15591595
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
15601596
this._notifyCrossOrientationChildren();
1561-
};
1597+
}
15621598

15631599
_onCellFocusCapture = (cellKey: string) => {
15641600
this._lastFocusedCellKey = cellKey;
@@ -1572,6 +1608,7 @@ class VirtualizedList extends StateSafePureComponent<
15721608

15731609
_onCellUnmount = (cellKey: string) => {
15741610
delete this._cellRefs[cellKey];
1611+
this._stickyHeaderKeysLaidOutByWrapper.delete(cellKey);
15751612
this._listMetrics.notifyCellUnmounted(cellKey);
15761613
};
15771614

‎packages/virtualized-lists/Lists/VirtualizedListCellRenderer.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ export type Props<ItemT> = {
4040
index: number,
4141
) => void,
4242
onCellFocusCapture?: (cellKey: string) => void,
43+
// Not used by CellRenderer. A sticky header wrapper reads it from the cell
44+
// element and calls it with the wrapper's layout.
45+
onLayout?: (event: LayoutChangeEvent) => void,
4346
onUnmount: (cellKey: string) => void,
4447
onUpdateSeparators: (
4548
cellKeys: Array<?string>,

‎packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010

1111
import VirtualizedList from '../VirtualizedList';
12+
import CellRenderer from '../VirtualizedListCellRenderer';
1213
import {format} from 'node:util';
1314
import * as React from 'react';
1415
import {createElement, createRef} from 'react';
@@ -1140,6 +1141,107 @@ describe('VirtualizedList', () => {
11401141
{first: 13, last: 19, isSpacer: true},
11411142
]);
11421143
});
1144+
1145+
it('uses the sticky wrapper layout for sticky header cells', async () => {
1146+
// Sticky headers at 0, 5 and 10. Every cell is 50dip tall.
1147+
const items = generateItemsStickyEveryN(15, 5);
1148+
1149+
let component;
1150+
await act(() => {
1151+
component = create(
1152+
<VirtualizedList initialNumToRender={15} {...baseItemProps(items)} />,
1153+
);
1154+
});
1155+
1156+
await act(() => {
1157+
simulateLayout(component, {
1158+
viewport: {width: 10, height: 100},
1159+
content: {width: 10, height: 750},
1160+
});
1161+
for (let i = 0; i < items.length; i++) {
1162+
// Sticky header cells measure themselves relative to their wrapper.
1163+
const y = items[i].sticky === true ? 0 : i * 50;
1164+
simulateCellLayout(component, items, i, {
1165+
width: 10,
1166+
height: 50,
1167+
x: 0,
1168+
y,
1169+
});
1170+
}
1171+
});
1172+
1173+
// The wrapper reports after the cell's own layout.
1174+
await act(() => {
1175+
simulateStickyHeaderWrapperLayout(component, items, 5, {
1176+
width: 10,
1177+
height: 50,
1178+
x: 0,
1179+
y: 250,
1180+
});
1181+
});
1182+
expect(getCellOffset(component, 5)).toBe(250);
1183+
1184+
// The wrapper reports before the cell's own layout.
1185+
await act(() => {
1186+
simulateStickyHeaderWrapperLayout(component, items, 10, {
1187+
width: 10,
1188+
height: 50,
1189+
x: 0,
1190+
y: 500,
1191+
});
1192+
simulateCellLayout(component, items, 10, {
1193+
width: 10,
1194+
height: 50,
1195+
x: 0,
1196+
y: 0,
1197+
});
1198+
});
1199+
expect(getCellOffset(component, 10)).toBe(500);
1200+
1201+
// Cells that are not sticky keep using their own layout.
1202+
expect(getCellOffset(component, 7)).toBe(350);
1203+
});
1204+
1205+
it('uses the cell layout again once a cell is no longer sticky', async () => {
1206+
const items = generateItemsStickyEveryN(10, 5);
1207+
const props = {initialNumToRender: 10, ...baseItemProps(items)};
1208+
1209+
let component;
1210+
await act(() => {
1211+
component = create(<VirtualizedList {...props} />);
1212+
});
1213+
1214+
await act(() => {
1215+
simulateLayout(component, {
1216+
viewport: {width: 10, height: 100},
1217+
content: {width: 10, height: 500},
1218+
});
1219+
simulateStickyHeaderWrapperLayout(component, items, 5, {
1220+
width: 10,
1221+
height: 50,
1222+
x: 0,
1223+
y: 250,
1224+
});
1225+
});
1226+
expect(getCellOffset(component, 5)).toBe(250);
1227+
1228+
await act(() => {
1229+
component.update(<VirtualizedList {...props} stickyHeaderIndices={[]} />);
1230+
});
1231+
expect(findCellRenderer(component, items, 5).props.onLayout).toBe(
1232+
undefined,
1233+
);
1234+
1235+
await act(() => {
1236+
simulateCellLayout(component, items, 5, {
1237+
width: 10,
1238+
height: 50,
1239+
x: 0,
1240+
y: 260,
1241+
});
1242+
});
1243+
expect(getCellOffset(component, 5)).toBe(260);
1244+
});
11431245
});
11441246

11451247
it('unmounts sticky headers moved below viewport', async () => {
@@ -3141,6 +3243,36 @@ function simulateCellLayout(component, items, itemIndex, dimensions) {
31413243
);
31423244
}
31433245

3246+
function findCellRenderer(component, items, itemIndex) {
3247+
const cellKey = VirtualizedList._keyExtractor(
3248+
items[itemIndex],
3249+
itemIndex,
3250+
component.getInstance().props,
3251+
);
3252+
return component.root.find(
3253+
node => node.type === CellRenderer && node.props.cellKey === cellKey,
3254+
);
3255+
}
3256+
3257+
// Mimics `ScrollViewStickyHeader`, which calls the cell element's `onLayout`
3258+
// with the layout of its wrapper.
3259+
function simulateStickyHeaderWrapperLayout(
3260+
component,
3261+
items,
3262+
itemIndex,
3263+
dimensions,
3264+
) {
3265+
findCellRenderer(component, items, itemIndex).props.onLayout({
3266+
nativeEvent: {layout: dimensions, zoomScale: 1},
3267+
});
3268+
}
3269+
3270+
function getCellOffset(component, itemIndex) {
3271+
const instance = component.getInstance();
3272+
return instance.__getListMetrics().getCellMetrics(itemIndex, instance.props)
3273+
.offset;
3274+
}
3275+
31443276
function simulateScroll(component, position) {
31453277
component.getInstance()._onScroll({
31463278
nativeEvent: {

0 commit comments

Comments
 (0)