Permalink
Browse files

VirtualizedList: fix bug where onViewableItemsChanged wouldn't trigger

Summary:
In the current implementation of the `VirtualizedList` the `onViewableItemsChanged` callback wouldn't trigger if the underlying list data changes. (see example snack https://snack.expo.io/Hk5703eBb)

I added a method in the `ViewabilityHelper` to invalidate the cached viewableIndices, which gets triggered when the list-data changes.
Closes #14922

Differential Revision: D5864537

Pulled By: sahrens

fbshipit-source-id: 37f617763596244208548817d5b138dadc12c75d
  • Loading branch information...
formatlos authored and facebook-github-bot committed Oct 16, 2017
1 parent d980632 commit 6747a36f5dec0c49fb32c68c8c980508e6ace7db
@@ -223,6 +223,13 @@ class ViewabilityHelper {
}
}
/**
* clean-up cached _viewableIndices to evaluate changed items on next update
*/
resetViewableIndices() {
this._viewableIndices = [];
}
/**
* Records that an interaction has happened even if there has been no scroll.
*/
@@ -516,6 +516,12 @@ class VirtualizedList extends React.PureComponent<Props, State> {
});
if (data !== this.props.data || extraData !== this.props.extraData) {
this._hasDataChangedSinceEndReached = true;
// clear the viewableIndices cache to also trigger
// the onViewableItemsChanged callback with the new data
this._viewabilityTuples.forEach(tuple => {
tuple.viewabilityHelper.resetViewableIndices();
});
}
}
@@ -385,4 +385,54 @@ describe('onUpdate', function() {
viewableItems: [{isViewable: true, key: 'a'}],
});
});
it('returns the right visible row after the underlying data changed', function() {
const helper = new ViewabilityHelper();
rowFrames = {
a: {y: 0, height: 200},
b: {y: 200, height: 200},
};
data = [{key: 'a'}, {key: 'b'}];
const onViewableItemsChanged = jest.fn();
helper.onUpdate(
data.length,
0,
200,
getFrameMetrics,
createViewToken,
onViewableItemsChanged,
);
expect(onViewableItemsChanged.mock.calls.length).toBe(1);
expect(onViewableItemsChanged.mock.calls[0][0]).toEqual({
changed: [{isViewable: true, key: 'a'}],
viewabilityConfig: {viewAreaCoveragePercentThreshold: 0},
viewableItems: [{isViewable: true, key: 'a'}],
});
// update data
rowFrames = {
c: {y: 0, height: 200},
a: {y: 200, height: 200},
b: {y: 400, height: 200},
};
data = [{key: 'c'}, {key: 'a'}, {key: 'b'}];
helper.resetViewableIndices();
helper.onUpdate(
data.length,
0,
200,
getFrameMetrics,
createViewToken,
onViewableItemsChanged,
);
expect(onViewableItemsChanged.mock.calls.length).toBe(2);
expect(onViewableItemsChanged.mock.calls[1][0]).toEqual({
changed: [{isViewable: true, key: 'c'}, {isViewable: false, key: 'a'}],
viewabilityConfig: {viewAreaCoveragePercentThreshold: 0},
viewableItems: [{isViewable: true, key: 'c'}],
});
});
});
@@ -160,4 +160,63 @@ describe('VirtualizedList', () => {
);
expect(component).toMatchSnapshot();
});
it('returns the viewableItems correctly in the onViewableItemsChanged callback after changing the data', () => {
const ITEM_HEIGHT = 800;
let data = [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}];
const nativeEvent = {
contentOffset: {y: 0, x: 0},
layoutMeasurement: {width: 300, height: 600},
contentSize: {width: 300, height: data.length * ITEM_HEIGHT},
zoomScale: 1,
contentInset: {right: 0, top: 0, left: 0, bottom: 0},
};
const onViewableItemsChanged = jest.fn();
const props = {
data,
renderItem: ({item}) => <item value={item.key} />,
getItem: (data, index) => data[index],
getItemCount: data => data.length,
getItemLayout: (data, index) => ({
length: ITEM_HEIGHT,
offset: ITEM_HEIGHT * index,
index,
}),
onViewableItemsChanged,
};
const component = ReactTestRenderer.create(<VirtualizedList {...props} />);
const instance = component.getInstance();
instance._onScrollBeginDrag({nativeEvent});
instance._onScroll({
timeStamp: 1000,
nativeEvent,
});
expect(onViewableItemsChanged).toHaveBeenCalledTimes(1);
expect(onViewableItemsChanged).toHaveBeenLastCalledWith(
expect.objectContaining({
viewableItems: [expect.objectContaining({isViewable: true, key: 'i1'})],
}),
);
data = [{key: 'i4'}, ...data];
component.update(<VirtualizedList {...props} data={data} />);
instance._onScroll({
timeStamp: 2000,
nativeEvent: {
...nativeEvent,
contentOffset: {y: 100, x: 0},
},
});
expect(onViewableItemsChanged).toHaveBeenCalledTimes(2);
expect(onViewableItemsChanged).toHaveBeenLastCalledWith(
expect.objectContaining({
viewableItems: [expect.objectContaining({isViewable: true, key: 'i4'})],
}),
);
});
});

0 comments on commit 6747a36

Please sign in to comment.