Skip to content

Commit 6747a36

Browse files
formatlosfacebook-github-bot
authored andcommitted
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 facebook#14922 Differential Revision: D5864537 Pulled By: sahrens fbshipit-source-id: 37f617763596244208548817d5b138dadc12c75d
1 parent d980632 commit 6747a36

File tree

4 files changed

+122
-0
lines changed

4 files changed

+122
-0
lines changed

Libraries/Lists/ViewabilityHelper.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,13 @@ class ViewabilityHelper {
223223
}
224224
}
225225

226+
/**
227+
* clean-up cached _viewableIndices to evaluate changed items on next update
228+
*/
229+
resetViewableIndices() {
230+
this._viewableIndices = [];
231+
}
232+
226233
/**
227234
* Records that an interaction has happened even if there has been no scroll.
228235
*/

Libraries/Lists/VirtualizedList.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,12 @@ class VirtualizedList extends React.PureComponent<Props, State> {
516516
});
517517
if (data !== this.props.data || extraData !== this.props.extraData) {
518518
this._hasDataChangedSinceEndReached = true;
519+
520+
// clear the viewableIndices cache to also trigger
521+
// the onViewableItemsChanged callback with the new data
522+
this._viewabilityTuples.forEach(tuple => {
523+
tuple.viewabilityHelper.resetViewableIndices();
524+
});
519525
}
520526
}
521527

Libraries/Lists/__tests__/ViewabilityHelper-test.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,4 +385,54 @@ describe('onUpdate', function() {
385385
viewableItems: [{isViewable: true, key: 'a'}],
386386
});
387387
});
388+
389+
it('returns the right visible row after the underlying data changed', function() {
390+
const helper = new ViewabilityHelper();
391+
rowFrames = {
392+
a: {y: 0, height: 200},
393+
b: {y: 200, height: 200},
394+
};
395+
data = [{key: 'a'}, {key: 'b'}];
396+
const onViewableItemsChanged = jest.fn();
397+
helper.onUpdate(
398+
data.length,
399+
0,
400+
200,
401+
getFrameMetrics,
402+
createViewToken,
403+
onViewableItemsChanged,
404+
);
405+
expect(onViewableItemsChanged.mock.calls.length).toBe(1);
406+
expect(onViewableItemsChanged.mock.calls[0][0]).toEqual({
407+
changed: [{isViewable: true, key: 'a'}],
408+
viewabilityConfig: {viewAreaCoveragePercentThreshold: 0},
409+
viewableItems: [{isViewable: true, key: 'a'}],
410+
});
411+
412+
// update data
413+
rowFrames = {
414+
c: {y: 0, height: 200},
415+
a: {y: 200, height: 200},
416+
b: {y: 400, height: 200},
417+
};
418+
data = [{key: 'c'}, {key: 'a'}, {key: 'b'}];
419+
420+
helper.resetViewableIndices();
421+
422+
helper.onUpdate(
423+
data.length,
424+
0,
425+
200,
426+
getFrameMetrics,
427+
createViewToken,
428+
onViewableItemsChanged,
429+
);
430+
431+
expect(onViewableItemsChanged.mock.calls.length).toBe(2);
432+
expect(onViewableItemsChanged.mock.calls[1][0]).toEqual({
433+
changed: [{isViewable: true, key: 'c'}, {isViewable: false, key: 'a'}],
434+
viewabilityConfig: {viewAreaCoveragePercentThreshold: 0},
435+
viewableItems: [{isViewable: true, key: 'c'}],
436+
});
437+
});
388438
});

Libraries/Lists/__tests__/VirtualizedList-test.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,63 @@ describe('VirtualizedList', () => {
160160
);
161161
expect(component).toMatchSnapshot();
162162
});
163+
164+
it('returns the viewableItems correctly in the onViewableItemsChanged callback after changing the data', () => {
165+
const ITEM_HEIGHT = 800;
166+
let data = [{key: 'i1'}, {key: 'i2'}, {key: 'i3'}];
167+
const nativeEvent = {
168+
contentOffset: {y: 0, x: 0},
169+
layoutMeasurement: {width: 300, height: 600},
170+
contentSize: {width: 300, height: data.length * ITEM_HEIGHT},
171+
zoomScale: 1,
172+
contentInset: {right: 0, top: 0, left: 0, bottom: 0},
173+
};
174+
const onViewableItemsChanged = jest.fn();
175+
const props = {
176+
data,
177+
renderItem: ({item}) => <item value={item.key} />,
178+
getItem: (data, index) => data[index],
179+
getItemCount: data => data.length,
180+
getItemLayout: (data, index) => ({
181+
length: ITEM_HEIGHT,
182+
offset: ITEM_HEIGHT * index,
183+
index,
184+
}),
185+
onViewableItemsChanged,
186+
};
187+
188+
const component = ReactTestRenderer.create(<VirtualizedList {...props} />);
189+
190+
const instance = component.getInstance();
191+
192+
instance._onScrollBeginDrag({nativeEvent});
193+
instance._onScroll({
194+
timeStamp: 1000,
195+
nativeEvent,
196+
});
197+
198+
expect(onViewableItemsChanged).toHaveBeenCalledTimes(1);
199+
expect(onViewableItemsChanged).toHaveBeenLastCalledWith(
200+
expect.objectContaining({
201+
viewableItems: [expect.objectContaining({isViewable: true, key: 'i1'})],
202+
}),
203+
);
204+
data = [{key: 'i4'}, ...data];
205+
component.update(<VirtualizedList {...props} data={data} />);
206+
207+
instance._onScroll({
208+
timeStamp: 2000,
209+
nativeEvent: {
210+
...nativeEvent,
211+
contentOffset: {y: 100, x: 0},
212+
},
213+
});
214+
215+
expect(onViewableItemsChanged).toHaveBeenCalledTimes(2);
216+
expect(onViewableItemsChanged).toHaveBeenLastCalledWith(
217+
expect.objectContaining({
218+
viewableItems: [expect.objectContaining({isViewable: true, key: 'i4'})],
219+
}),
220+
);
221+
});
163222
});

0 commit comments

Comments
 (0)