mirror of
https://github.com/react/react-native.git
synced 2026-09-28 13:23:09 +08:00
Fix viewability for zero-sized lists (#58000)
Summary: Pull Request resolved: https://github.com/react/react-native/pull/58000 VirtualizedList viewability only considered the scroll-axis viewport length. A horizontal list with zero height could therefore report items as viewable. Record the cross-axis length in the same scroll metrics snapshot and require both viewport dimensions to be non-zero for viewability. Changelog: [General][Fixed] - Prevent zero-sized lists from reporting viewable items Reviewed By: zeyap, christophpurrer Differential Revision: D116356139 fbshipit-source-id: 77c1b61bddfde1dcac9e670ddab80915ef42149a
This commit is contained in:
committed by
meta-codesync[bot]
parent
14938717e6
commit
57c8285d40
@@ -1250,6 +1250,7 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
_pendingViewabilityUpdate: boolean = false;
|
||||
_prevParentOffset: number = 0;
|
||||
_scrollMetrics: {
|
||||
crossAxisLength: number,
|
||||
dOffset: number,
|
||||
dt: number,
|
||||
offset: number,
|
||||
@@ -1258,6 +1259,7 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
visibleLength: number,
|
||||
zoomScale: number,
|
||||
} = {
|
||||
crossAxisLength: 0,
|
||||
dOffset: 0,
|
||||
dt: 10,
|
||||
offset: 0,
|
||||
@@ -1381,6 +1383,7 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
this.context.getOutermostParentListRef().getScrollRef(),
|
||||
(x, y, width, height) => {
|
||||
this._offsetFromParentVirtualizedList = this._selectOffset({x, y});
|
||||
const crossAxisLength = this._selectCrossAxisLength({width, height});
|
||||
this._listMetrics.notifyListContentLayout({
|
||||
layout: {width, height},
|
||||
orientation: this._orientation(),
|
||||
@@ -1390,10 +1393,12 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
);
|
||||
|
||||
const metricsChanged =
|
||||
this._scrollMetrics.crossAxisLength !== crossAxisLength ||
|
||||
this._scrollMetrics.visibleLength !== scrollMetrics.visibleLength ||
|
||||
this._scrollMetrics.offset !== scrollMetrics.offset;
|
||||
|
||||
if (metricsChanged) {
|
||||
this._scrollMetrics.crossAxisLength = crossAxisLength;
|
||||
this._scrollMetrics.visibleLength = scrollMetrics.visibleLength;
|
||||
this._scrollMetrics.offset = scrollMetrics.offset;
|
||||
|
||||
@@ -1420,6 +1425,9 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
}
|
||||
|
||||
_onLayout = (e: LayoutChangeEvent) => {
|
||||
this._scrollMetrics.crossAxisLength = this._selectCrossAxisLength(
|
||||
e.nativeEvent.layout,
|
||||
);
|
||||
if (this._isNestedWithSameOrientation()) {
|
||||
// Need to adjust our scroll metrics to be relative to our containing
|
||||
// VirtualizedList before we can make claims about list item viewability
|
||||
@@ -1527,6 +1535,18 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
: metrics.width;
|
||||
}
|
||||
|
||||
_selectCrossAxisLength(
|
||||
metrics: Readonly<{
|
||||
height: number,
|
||||
width: number,
|
||||
...
|
||||
}>,
|
||||
): number {
|
||||
return !horizontalOrDefault(this.props.horizontal)
|
||||
? metrics.width
|
||||
: metrics.height;
|
||||
}
|
||||
|
||||
_selectOffset({x, y}: Readonly<{x: number, y: number, ...}>): number {
|
||||
return this._orientation().horizontal ? x : y;
|
||||
}
|
||||
@@ -1710,6 +1730,9 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
this.props.onScroll(e);
|
||||
}
|
||||
const timestamp = e.timeStamp;
|
||||
let crossAxisLength = this._selectCrossAxisLength(
|
||||
e.nativeEvent.layoutMeasurement,
|
||||
);
|
||||
let visibleLength = this._selectLength(e.nativeEvent.layoutMeasurement);
|
||||
let contentLength = this._selectLength(e.nativeEvent.contentSize);
|
||||
let offset = this._offsetFromScrollEvent(e);
|
||||
@@ -1726,6 +1749,7 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
visibleLength,
|
||||
offset,
|
||||
}));
|
||||
crossAxisLength = this._scrollMetrics.crossAxisLength;
|
||||
}
|
||||
|
||||
const dt = this._scrollMetrics.timestamp
|
||||
@@ -1751,6 +1775,7 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
// For invalid negative values (w/ RTL), set this to 1.
|
||||
const zoomScale = e.nativeEvent.zoomScale < 0 ? 1 : e.nativeEvent.zoomScale;
|
||||
this._scrollMetrics = {
|
||||
crossAxisLength,
|
||||
dt,
|
||||
dOffset,
|
||||
offset,
|
||||
@@ -2032,11 +2057,15 @@ class VirtualizedList extends StateSafePureComponent<
|
||||
if (this.state.pendingScrollUpdateCount > 0) {
|
||||
return;
|
||||
}
|
||||
const visibleLength =
|
||||
this._scrollMetrics.crossAxisLength > 0
|
||||
? this._scrollMetrics.visibleLength
|
||||
: 0;
|
||||
this._viewabilityTuples.forEach(tuple => {
|
||||
tuple.viewabilityHelper.onUpdate(
|
||||
props,
|
||||
this._scrollMetrics.offset,
|
||||
this._scrollMetrics.visibleLength,
|
||||
visibleLength,
|
||||
this._listMetrics,
|
||||
this._createViewToken,
|
||||
tuple.onViewableItemsChanged,
|
||||
|
||||
@@ -468,6 +468,7 @@ describe('VirtualizedList', () => {
|
||||
|
||||
const instance = component.getInstance();
|
||||
|
||||
simulateViewportLayout(component, {width: 300, height: 600});
|
||||
instance._onScrollBeginDrag({nativeEvent});
|
||||
instance._onScroll({
|
||||
timeStamp: 1000,
|
||||
@@ -504,6 +505,56 @@ describe('VirtualizedList', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('does not report viewable items when scroll metrics have an empty cross-axis viewport', async () => {
|
||||
const data = [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}];
|
||||
const onViewableItemsChanged = jest.fn();
|
||||
let component;
|
||||
|
||||
await act(() => {
|
||||
component = create(
|
||||
<VirtualizedList
|
||||
data={data}
|
||||
getItem={(items, index) => items[index]}
|
||||
getItemCount={items => items.length}
|
||||
getItemLayout={(items, index) => ({
|
||||
index,
|
||||
length: 100,
|
||||
offset: index * 100,
|
||||
})}
|
||||
horizontal={true}
|
||||
onViewableItemsChanged={onViewableItemsChanged}
|
||||
renderItem={({item}) => <item value={item.key} />}
|
||||
/>,
|
||||
);
|
||||
});
|
||||
|
||||
component.getInstance()._onScroll({
|
||||
timeStamp: 1000,
|
||||
nativeEvent: {
|
||||
contentInset: {bottom: 0, left: 0, right: 0, top: 0},
|
||||
contentOffset: {x: 0, y: 0},
|
||||
contentSize: {width: 300, height: 0},
|
||||
layoutMeasurement: {width: 300, height: 0},
|
||||
zoomScale: 1,
|
||||
},
|
||||
});
|
||||
|
||||
expect(onViewableItemsChanged).not.toHaveBeenCalled();
|
||||
|
||||
component.getInstance()._onScroll({
|
||||
timeStamp: 2000,
|
||||
nativeEvent: {
|
||||
contentInset: {bottom: 0, left: 0, right: 0, top: 0},
|
||||
contentOffset: {x: 0, y: 0},
|
||||
contentSize: {width: 300, height: 100},
|
||||
layoutMeasurement: {width: 300, height: 100},
|
||||
zoomScale: 1,
|
||||
},
|
||||
});
|
||||
|
||||
expect(onViewableItemsChanged).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('getScrollRef for case where it returns a ScrollView', async () => {
|
||||
const listRef = createRef(null);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user