Skip to content

Commit 87dd004

Browse files
cipolleschifacebook-github-bot
authored andcommitted
Handle zero-area layouts in VirtualizedList viewability
Summary: VirtualizedList treated horizontal lists with zero cross-axis size as visible because viewability only consumed the main-axis visible length. Track zero-area layouts, invalidate pending viewability updates, and report a zero visible length while collapsed. Keep RNTester screenshot checks deterministic by waiting for the configured `minimumViewTime` in the off-screen example and excluding variable callback content from the stable layout screenshot. Changelog: [General][Fixed] - Prevent zero-area lists from reporting items as viewable Differential Revision: D116026116
1 parent 8804333 commit 87dd004

4 files changed

Lines changed: 94 additions & 4 deletions

File tree

packages/rn-tester/js/examples/SectionList/SectionList-BaseOnViewableItemsChanged.js

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@ import type {
1515

1616
import SectionListBaseExample from './SectionListBaseExample';
1717
import * as React from 'react';
18-
import {useRef, useState} from 'react';
18+
import {useEffect, useRef, useState} from 'react';
1919
import {StyleSheet, View} from 'react-native';
2020

2121
const BASE_VIEWABILITY_CONFIG = {
2222
minimumViewTime: 1000,
2323
viewAreaCoveragePercentThreshold: 100,
2424
};
25+
const VIEWABILITY_OBSERVATION_TIME_MS =
26+
BASE_VIEWABILITY_CONFIG.minimumViewTime * 2;
2527

2628
export function SectionList_BaseOnViewableItemsChanged(props: {
2729
offScreen?: ?boolean,
@@ -30,7 +32,28 @@ export function SectionList_BaseOnViewableItemsChanged(props: {
3032
waitForInteraction?: ?boolean,
3133
}): React.Node {
3234
const {offScreen, horizontal, useScrollRefScroll, waitForInteraction} = props;
35+
const [observationComplete, setObservationComplete] = useState(false);
3336
const [output, setOutput] = useState('');
37+
const observationTimeoutRef = useRef<?TimeoutID>(null);
38+
useEffect(() => {
39+
return () => {
40+
if (observationTimeoutRef.current != null) {
41+
clearTimeout(observationTimeoutRef.current);
42+
}
43+
};
44+
}, []);
45+
const onListLayout =
46+
offScreen === true
47+
? () => {
48+
if (observationTimeoutRef.current != null) {
49+
clearTimeout(observationTimeoutRef.current);
50+
}
51+
setObservationComplete(false);
52+
observationTimeoutRef.current = setTimeout(() => {
53+
setObservationComplete(true);
54+
}, VIEWABILITY_OBSERVATION_TIME_MS);
55+
}
56+
: undefined;
3457
const viewabilityConfig: ViewabilityConfig = {
3558
...BASE_VIEWABILITY_CONFIG,
3659
waitForInteraction: waitForInteraction ?? false,
@@ -49,6 +72,7 @@ export function SectionList_BaseOnViewableItemsChanged(props: {
4972
),
5073
viewabilityConfig,
5174
horizontal,
75+
onLayout: onListLayout,
5276
};
5377
const ref = useRef<any>(null);
5478
const onTest =
@@ -63,6 +87,11 @@ export function SectionList_BaseOnViewableItemsChanged(props: {
6387
ref={ref}
6488
exampleProps={exampleProps}
6589
onTest={onTest}
90+
testContainerTestID={
91+
observationComplete
92+
? 'viewability_observation_complete'
93+
: 'test_container'
94+
}
6695
testOutput={output}>
6796
{offScreen === true ? <View style={styles.offScreen} /> : null}
6897
</SectionListBaseExample>

packages/rn-tester/js/examples/SectionList/SectionListBaseExample.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ type Props = Readonly<{
6868
// $FlowFixMe[unclear-type]
6969
exampleProps: Partial<React.ElementConfig<typeof SectionList<any>>>,
7070
onTest?: ?() => void,
71+
testContainerTestID?: ?string,
7172
testLabel?: ?string,
7273
testOutput?: ?string,
7374
children?: ?React.Node,
@@ -88,7 +89,9 @@ const SectionListBaseExample: component(
8889
return (
8990
<View style={styles.container}>
9091
{props.testOutput != null ? (
91-
<View testID="test_container" style={styles.testContainer}>
92+
<View
93+
testID={props.testContainerTestID ?? 'test_container'}
94+
style={styles.testContainer}>
9295
<Text style={styles.output} numberOfLines={1} testID="output">
9396
{props.testOutput}
9497
</Text>

packages/virtualized-lists/Lists/VirtualizedList.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,6 +1242,7 @@ class VirtualizedList extends StateSafePureComponent<
12421242
_hasWarned: {[string]: boolean} = {};
12431243
_headerLength = 0;
12441244
_hiPriInProgress: boolean = false; // flag to prevent infinite hiPri cell limit update
1245+
_hasZeroArea: boolean = false;
12451246
_indicesToKeys: Map<number, string> = new Map();
12461247
_lastFocusedCellKey: ?string = null;
12471248
_nestedChildLists: ChildListCollection<VirtualizedList> =
@@ -1420,6 +1421,9 @@ class VirtualizedList extends StateSafePureComponent<
14201421
}
14211422

14221423
_onLayout = (e: LayoutChangeEvent) => {
1424+
const hadZeroArea = this._hasZeroArea;
1425+
const {height, width} = e.nativeEvent.layout;
1426+
this._hasZeroArea = height === 0 || width === 0;
14231427
if (this._isNestedWithSameOrientation()) {
14241428
// Need to adjust our scroll metrics to be relative to our containing
14251429
// VirtualizedList before we can make claims about list item viewability
@@ -1431,6 +1435,9 @@ class VirtualizedList extends StateSafePureComponent<
14311435
}
14321436
this.props.onLayout && this.props.onLayout(e);
14331437
this._scheduleCellsToRenderUpdate();
1438+
if (hadZeroArea !== this._hasZeroArea) {
1439+
this._updateViewableItems(this.props, this.state.cellsAroundViewport);
1440+
}
14341441
this._maybeCallOnEdgeReached();
14351442
};
14361443

@@ -2031,14 +2038,14 @@ class VirtualizedList extends StateSafePureComponent<
20312038
) {
20322039
// If we have any pending scroll updates it means that the scroll metrics
20332040
// are out of date and we should not call any of the visibility callbacks.
2034-
if (this.state.pendingScrollUpdateCount > 0) {
2041+
if (this.state.pendingScrollUpdateCount > 0 && !this._hasZeroArea) {
20352042
return;
20362043
}
20372044
this._viewabilityTuples.forEach(tuple => {
20382045
tuple.viewabilityHelper.onUpdate(
20392046
props,
20402047
this._scrollMetrics.offset,
2041-
this._scrollMetrics.visibleLength,
2048+
this._hasZeroArea ? 0 : this._scrollMetrics.visibleLength,
20422049
this._listMetrics,
20432050
this._createViewToken,
20442051
tuple.onViewableItemsChanged,

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,57 @@ describe('VirtualizedList', () => {
504504
);
505505
});
506506

507+
it('does not report horizontal items after the list collapses vertically', async () => {
508+
const data = [{key: 'i1'}];
509+
const onViewableItemsChanged = jest.fn();
510+
const viewabilityConfig = {
511+
minimumViewTime: 1000,
512+
viewAreaCoveragePercentThreshold: 100,
513+
};
514+
let component;
515+
await act(() => {
516+
component = create(
517+
<VirtualizedList
518+
data={data}
519+
getItem={(items, index) => items[index]}
520+
getItemCount={items => items.length}
521+
getItemLayout={(items, index) => ({
522+
index,
523+
length: 100,
524+
offset: index * 100,
525+
})}
526+
horizontal={true}
527+
onViewableItemsChanged={onViewableItemsChanged}
528+
renderItem={({item}) => <item value={item.key} />}
529+
viewabilityConfig={viewabilityConfig}
530+
/>,
531+
);
532+
});
533+
534+
const instance = component.getInstance();
535+
await act(async () => {
536+
instance._onLayout({
537+
nativeEvent: {layout: {height: 100, width: 300}, zoomScale: 1},
538+
});
539+
instance._onScroll({
540+
timeStamp: 1000,
541+
nativeEvent: {
542+
contentInset: {bottom: 0, left: 0, right: 0, top: 0},
543+
contentOffset: {x: 0, y: 0},
544+
contentSize: {height: 100, width: 300},
545+
layoutMeasurement: {height: 100, width: 300},
546+
zoomScale: 1,
547+
},
548+
});
549+
instance._onLayout({
550+
nativeEvent: {layout: {height: 0, width: 300}, zoomScale: 1},
551+
});
552+
await jest.runAllTimersAsync();
553+
});
554+
555+
expect(onViewableItemsChanged).not.toHaveBeenCalled();
556+
});
557+
507558
it('getScrollRef for case where it returns a ScrollView', async () => {
508559
const listRef = createRef(null);
509560

0 commit comments

Comments
 (0)