Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix items disappear when zooming VirtualizedList #33765

Closed
wants to merge 3 commits into from
Closed

Fix items disappear when zooming VirtualizedList #33765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
989177d
Fixed a bug that items disappear when zooming VirtualizedList
islandryu May 1, 2022
1f6cf50
fix test
islandryu May 6, 2022
c721cf6
fix type
islandryu May 6, 2022
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
10 changes: 7 additions & 3 deletions Libraries/Lists/VirtualizeUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@ export function elementsThatOverlapOffsets(
offset: number,
...
},
zoomScale: number,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires calls in VirtualizeUtils-test.js to be updated. Otherwise, the Jest test fails.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed so that the test passes.

): Array<number> {
const out = [];
let outLength = 0;
for (let ii = 0; ii < itemCount; ii++) {
const frame = getFrameMetrics(ii);
const trailingOffset = frame.offset + frame.length;
const trailingOffset = (frame.offset + frame.length) * zoomScale;
for (let kk = 0; kk < offsets.length; kk++) {
if (out[kk] == null && trailingOffset >= offsets[kk]) {
out[kk] = ii;
Expand Down Expand Up @@ -104,6 +105,7 @@ export function computeWindowedRenderLimits(
offset: number,
velocity: number,
visibleLength: number,
zoomScale: number,
...
},
): {
Expand All @@ -115,7 +117,7 @@ export function computeWindowedRenderLimits(
if (itemCount === 0) {
return prev;
}
const {offset, velocity, visibleLength} = scrollMetrics;
const {offset, velocity, visibleLength, zoomScale} = scrollMetrics;

// Start with visible area, then compute maximum overscan region by expanding from there, biased
// in the direction of scroll. Total overscan area is capped, which should cap memory consumption
Expand All @@ -136,7 +138,8 @@ export function computeWindowedRenderLimits(
);
const overscanEnd = Math.max(0, visibleEnd + leadFactor * overscanLength);

const lastItemOffset = getFrameMetricsApprox(itemCount - 1).offset;
const lastItemOffset =
getFrameMetricsApprox(itemCount - 1).offset * zoomScale;
if (lastItemOffset < overscanBegin) {
// Entire list is before our overscan window
return {
Expand All @@ -150,6 +153,7 @@ export function computeWindowedRenderLimits(
[overscanBegin, visibleBegin, visibleEnd, overscanEnd],
itemCount,
getFrameMetricsApprox,
zoomScale,
);
overscanFirst = overscanFirst == null ? 0 : overscanFirst;
first = first == null ? Math.max(0, overscanFirst) : first;
Expand Down
5 changes: 5 additions & 0 deletions Libraries/Lists/VirtualizedList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
timestamp: 0,
velocity: 0,
visibleLength: 0,
zoomScale: 1,
};
_scrollRef: ?React.ElementRef<any> = null;
_sentEndForContentLength = 0;
Expand Down Expand Up @@ -1618,6 +1619,9 @@ class VirtualizedList extends React.PureComponent<Props, State> {
);
this._hasWarned.perf = true;
}

const zoomScale = e.nativeEvent.zoomScale;

this._scrollMetrics = {
contentLength,
dt,
Expand All @@ -1626,6 +1630,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
timestamp,
velocity,
visibleLength,
zoomScale,
};
this._updateViewableItems(this.props.data);
if (!this.props) {
Expand Down
1 change: 1 addition & 0 deletions Libraries/Lists/VirtualizedListContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type Context = $ReadOnly<{
timestamp: number,
velocity: number,
visibleLength: number,
zoomScale: number,
},
horizontal: ?boolean,
getOutermostParentListRef: () => VirtualizedList,
Expand Down
12 changes: 6 additions & 6 deletions Libraries/Lists/__tests__/VirtualizeUtils-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ describe('elementsThatOverlapOffsets', function () {
offset: 100 * index,
};
}
expect(elementsThatOverlapOffsets(offsets, 100, getFrameMetrics)).toEqual([
0, 2, 3, 4,
]);
expect(
elementsThatOverlapOffsets(offsets, 100, getFrameMetrics, 1),
).toEqual([0, 2, 3, 4]);
});
it('handles variable length', function () {
const offsets = [150, 250, 900];
Expand All @@ -62,7 +62,7 @@ describe('elementsThatOverlapOffsets', function () {
{offset: 950, length: 150},
];
expect(
elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii]),
elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii], 1),
).toEqual([1, 1, 3]);
});
it('handles out of bounds', function () {
Expand All @@ -73,7 +73,7 @@ describe('elementsThatOverlapOffsets', function () {
{offset: 250, length: 100},
];
expect(
elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii]),
elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii], 1),
).toEqual([1]);
});
it('errors on non-increasing offsets', function () {
Expand All @@ -84,7 +84,7 @@ describe('elementsThatOverlapOffsets', function () {
{offset: 250, length: 100},
];
expect(() => {
elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii]);
elementsThatOverlapOffsets(offsets, frames.length, ii => frames[ii], 1);
}).toThrowErrorMatchingSnapshot();
});
});
11 changes: 7 additions & 4 deletions Libraries/Lists/__tests__/VirtualizedList-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ describe('VirtualizedList', () => {

const instance = component.getInstance();

instance._onLayout({nativeEvent: {layout}});
instance._onLayout({nativeEvent: {layout, zoomScale: 1}});

const initialContentHeight = props.initialNumToRender * ITEM_HEIGHT;

Expand Down Expand Up @@ -1490,7 +1490,7 @@ it('calls _onCellLayout properly', () => {
);
const cell = virtualList._cellRefs.i4;
const event = {
nativeEvent: {layout: {x: 0, y: 0, width: 50, height: 50}},
nativeEvent: {layout: {x: 0, y: 0, width: 50, height: 50}, zoomScale: 1},
};
cell._onLayout(event);
expect(mock).toHaveBeenCalledWith(event, 'i4', 3);
Expand Down Expand Up @@ -1544,7 +1544,9 @@ function simulateLayout(component, args) {

function simulateViewportLayout(component, dimensions) {
lastViewportLayout = dimensions;
component.getInstance()._onLayout({nativeEvent: {layout: dimensions}});
component
.getInstance()
._onLayout({nativeEvent: {layout: dimensions}, zoomScale: 1});
}

function simulateContentLayout(component, dimensions) {
Expand All @@ -1558,7 +1560,7 @@ function simulateCellLayout(component, items, itemIndex, dimensions) {
const instance = component.getInstance();
const cellKey = instance._keyExtractor(items[itemIndex], itemIndex);
instance._onCellLayout(
{nativeEvent: {layout: dimensions}},
{nativeEvent: {layout: dimensions, zoomScale: 1}},
cellKey,
itemIndex,
);
Expand All @@ -1570,6 +1572,7 @@ function simulateScroll(component, position) {
contentOffset: position,
contentSize: lastContentLayout,
layoutMeasurement: lastViewportLayout,
zoomScale: 1,
},
});
}
Expand Down