diff --git a/packages/virtualized-lists/Lists/VirtualizedSectionList.js b/packages/virtualized-lists/Lists/VirtualizedSectionList.js index b9551b65542..f2d257d377d 100644 --- a/packages/virtualized-lists/Lists/VirtualizedSectionList.js +++ b/packages/virtualized-lists/Lists/VirtualizedSectionList.js @@ -138,7 +138,7 @@ class VirtualizedSectionList< State, > { scrollToLocation(params: ScrollToLocationParamsType) { - let index = params.itemIndex; + let index = params.itemIndex + 1; for (let i = 0; i < params.sectionIndex; i++) { index += this.props.getItemCount(this.props.sections[i].data) + 2; } @@ -147,10 +147,10 @@ class VirtualizedSectionList< return; } const listRef = this._listRef; - if (params.itemIndex > 0 && this.props.stickySectionHeadersEnabled) { + if (this.props.stickySectionHeadersEnabled) { const frame = listRef .__getListMetrics() - .getCellMetricsApprox(index - params.itemIndex, listRef.props); + .getCellMetricsApprox(index - params.itemIndex - 1, listRef.props); viewOffset += frame.length; } const toIndexParams: { diff --git a/packages/virtualized-lists/Lists/__tests__/VirtualizedSectionList-test.js b/packages/virtualized-lists/Lists/__tests__/VirtualizedSectionList-test.js index 24bb057cc04..71a3f9cf7e1 100644 --- a/packages/virtualized-lists/Lists/__tests__/VirtualizedSectionList-test.js +++ b/packages/virtualized-lists/Lists/__tests__/VirtualizedSectionList-test.js @@ -219,35 +219,61 @@ describe('VirtualizedSectionList', () => { const ITEM_HEIGHT = 100; const createVirtualizedSectionList = async (props?: { - stickySectionHeadersEnabled: boolean, + stickySectionHeadersEnabled?: boolean, + sections?: Array>, + getItemLayout?: ( + data: unknown, + index: number, + ) => { + length: number, + offset: number, + index: number, + }, }) => { + const defaultSections = [ + // $FlowFixMe[incompatible-type] + { + title: 's1', + data: [{key: 'i1.1'}, {key: 'i1.2'}, {key: 'i1.3'}], + }, + // $FlowFixMe[incompatible-type] + { + title: 's2', + data: [{key: 'i2.1'}, {key: 'i2.2'}, {key: 'i2.3'}], + }, + ] as Array>; + + const sections = props?.sections ?? defaultSections; + let getItemLayout; + // Use `in` check to allow explicitly passing `getItemLayout: undefined` + // to disable the default layout (distinct from not passing the prop at all). + if (props != null && 'getItemLayout' in props) { + getItemLayout = props.getItemLayout; + } else { + getItemLayout = (data: unknown, index: number) => ({ + length: ITEM_HEIGHT, + offset: ITEM_HEIGHT * index, + index, + }); + } + const { + sections: _sections, + getItemLayout: _getItemLayout, + ...restProps + } = props ?? {}; + void _sections; + void _getItemLayout; + let component; await ReactTestRenderer.act(() => { component = ReactTestRenderer.create( > - } + sections={sections} renderItem={({item}) => } getItem={(data, key) => data[key]} getItemCount={data => data.length} - getItemLayout={(data, index) => ({ - length: ITEM_HEIGHT, - offset: ITEM_HEIGHT * index, - index, - })} - {...props} + getItemLayout={getItemLayout} + {...restProps} />, ); }); @@ -265,7 +291,7 @@ describe('VirtualizedSectionList', () => { }; }; - it('when sticky stickySectionHeadersEnabled={true}, header height is added to the developer-provided viewOffset', async () => { + it('when sticky headers enabled and itemIndex is 1, header height is added to viewOffset', async () => { const {instance, spy} = await createVirtualizedSectionList({ stickySectionHeadersEnabled: true, }); @@ -279,7 +305,7 @@ describe('VirtualizedSectionList', () => { viewOffset, }); expect(spy).toHaveBeenCalledWith({ - index: 1, + index: 2, itemIndex: 1, sectionIndex: 0, viewOffset: viewOffset + ITEM_HEIGHT, @@ -291,7 +317,7 @@ describe('VirtualizedSectionList', () => { // prevents #18098 {sectionIndex: 0, itemIndex: 0}, { - index: 0, + index: 1, itemIndex: 0, sectionIndex: 0, viewOffset: 0, @@ -300,7 +326,7 @@ describe('VirtualizedSectionList', () => { [ {sectionIndex: 2, itemIndex: 1}, { - index: 11, + index: 12, itemIndex: 1, sectionIndex: 2, viewOffset: 0, @@ -313,7 +339,7 @@ describe('VirtualizedSectionList', () => { viewOffset: 25, }, { - index: 1, + index: 2, itemIndex: 1, sectionIndex: 0, viewOffset: 25, @@ -328,5 +354,125 @@ describe('VirtualizedSectionList', () => { expect(spy).toHaveBeenCalledWith(expected); }, ); + + it('scrolls to first item of first section', async () => { + const {instance, spy} = await createVirtualizedSectionList(); + // $FlowFixMe[prop-missing] scrollToLocation not on instance + instance?.scrollToLocation({sectionIndex: 0, itemIndex: 0}); + expect(spy).toHaveBeenCalledWith({ + index: 1, + itemIndex: 0, + sectionIndex: 0, + viewOffset: 0, + }); + }); + + it('scrolls to first item of a later section', async () => { + const {instance, spy} = await createVirtualizedSectionList(); + // $FlowFixMe[prop-missing] scrollToLocation not on instance + instance?.scrollToLocation({sectionIndex: 1, itemIndex: 0}); + expect(spy).toHaveBeenCalledWith({ + index: 6, + itemIndex: 0, + sectionIndex: 1, + viewOffset: 0, + }); + }); + + it('when sticky headers enabled and itemIndex is 0, header height is added to viewOffset (was previously skipped)', async () => { + // Use distinct heights per index so only the correct header's height can satisfy the assertion. + // Header at flat index 0 has height 37, item at index 1 has height 41 — an off-by-one + // in the header lookup would produce 41 and fail. + const HEADER_HEIGHT = 37; + const ITEM_HEIGHT_DISTINCT = 41; + const getItemLayout = (data: unknown, index: number) => ({ + length: index === 0 ? HEADER_HEIGHT : ITEM_HEIGHT_DISTINCT + index, + offset: 0, + index, + }); + const {instance, spy} = await createVirtualizedSectionList({ + stickySectionHeadersEnabled: true, + getItemLayout, + }); + // $FlowFixMe[prop-missing] scrollToLocation not on instance + instance?.scrollToLocation({sectionIndex: 0, itemIndex: 0}); + expect(spy).toHaveBeenCalledWith({ + index: 1, + itemIndex: 0, + sectionIndex: 0, + viewOffset: HEADER_HEIGHT, + }); + }); + + it('preserves caller-supplied viewOffset and adds header height when sticky', async () => { + const {instance, spy} = await createVirtualizedSectionList({ + stickySectionHeadersEnabled: true, + }); + // $FlowFixMe[prop-missing] scrollToLocation not on instance + instance?.scrollToLocation({ + sectionIndex: 1, + itemIndex: 0, + viewOffset: 10, + }); + expect(spy).toHaveBeenCalledWith({ + index: 6, + itemIndex: 0, + sectionIndex: 1, + viewOffset: 10 + ITEM_HEIGHT, + }); + }); + + it('preserves caller-supplied viewOffset without sticky headers', async () => { + const {instance, spy} = await createVirtualizedSectionList(); + // $FlowFixMe[prop-missing] scrollToLocation not on instance + instance?.scrollToLocation({ + sectionIndex: 1, + itemIndex: 2, + viewOffset: 15, + }); + expect(spy).toHaveBeenCalledWith({ + index: 8, + itemIndex: 2, + sectionIndex: 1, + viewOffset: 15, + }); + }); + + it('handles out-of-range itemIndex', async () => { + const {instance, spy} = await createVirtualizedSectionList(); + // $FlowFixMe[prop-missing] scrollToLocation not on instance + instance?.scrollToLocation({sectionIndex: 1, itemIndex: 10}); + // 10 + 1 + (3 + 2) = 16, out of range for 10-item list but still forwarded + expect(spy).toHaveBeenCalledWith({ + index: 16, + itemIndex: 10, + sectionIndex: 1, + viewOffset: 0, + }); + }); + + it('works with varying item heights and no getItemLayout', async () => { + const {instance, spy} = await createVirtualizedSectionList({ + sections: [ + // $FlowFixMe[incompatible-type] + {title: 's1', data: [{key: 'a1'}, {key: 'a2'}]}, + // $FlowFixMe[incompatible-type] + { + title: 's2', + data: [{key: 'b1'}, {key: 'b2'}, {key: 'b3'}, {key: 'b4'}], + }, + ] as Array>, + getItemLayout: undefined, + }); + // $FlowFixMe[prop-missing] scrollToLocation not on instance + instance?.scrollToLocation({sectionIndex: 1, itemIndex: 0}); + // section 0: 2 items + header/footer = 4, so first item of section 1 is at 1 + 4 = 5 + expect(spy).toHaveBeenCalledWith({ + index: 5, + itemIndex: 0, + sectionIndex: 1, + viewOffset: 0, + }); + }); }); });