From 8a82503b54e3c63230a07de99ec082b2dcb54bc7 Mon Sep 17 00:00:00 2001 From: Vojtech Novak Date: Tue, 13 Aug 2019 07:49:04 -0700 Subject: [PATCH] fix SectionList scrollToLocation and prevent regressions (#25997) Summary: Recently there were quite a few changes to this functionality, and they caused breakages https://github.com/facebook/react-native/issues/21577 https://github.com/facebook/react-native/issues/24034 https://github.com/facebook/react-native/issues/24734 https://github.com/facebook/react-native/issues/24735 Currently, whichever `viewOffset` I pass, it will be overridden (either by 0 or something computed in the if body). This fixes the issue and also adds tests to make sure there is no regression. ## Changelog [Javascript] [Fixed] - VirtualizedSectionList scrollToLocation viewOffset param ignored Pull Request resolved: https://github.com/facebook/react-native/pull/25997 Test Plan: tests pass Differential Revision: D16784036 Pulled By: cpojer fbshipit-source-id: 46421250993785176634b30a2629a6e12f0c2278 --- Libraries/Lists/SectionList.js | 9 +- Libraries/Lists/VirtualizedSectionList.js | 19 ++-- .../__tests__/VirtualizedSectionList-test.js | 94 +++++++++++++++++++ 3 files changed, 107 insertions(+), 15 deletions(-) diff --git a/Libraries/Lists/SectionList.js b/Libraries/Lists/SectionList.js index 90c180fae2a600..99b621a2eb350f 100644 --- a/Libraries/Lists/SectionList.js +++ b/Libraries/Lists/SectionList.js @@ -18,6 +18,7 @@ import type {ViewToken} from './ViewabilityHelper'; import type { SectionBase as _SectionBase, Props as VirtualizedSectionListProps, + ScrollToLocationParamsType, } from './VirtualizedSectionList'; type Item = any; @@ -245,13 +246,7 @@ class SectionList> extends React.PureComponent< * Note: cannot scroll to locations outside the render window without specifying the * `getItemLayout` prop. */ - scrollToLocation(params: { - animated?: ?boolean, - itemIndex: number, - sectionIndex: number, - viewOffset?: number, - viewPosition?: number, - }) { + scrollToLocation(params: ScrollToLocationParamsType) { if (this._wrapperListRef != null) { this._wrapperListRef.scrollToLocation(params); } diff --git a/Libraries/Lists/VirtualizedSectionList.js b/Libraries/Lists/VirtualizedSectionList.js index f02ba6771cfbdb..1b8b739957d0cb 100644 --- a/Libraries/Lists/VirtualizedSectionList.js +++ b/Libraries/Lists/VirtualizedSectionList.js @@ -119,11 +119,19 @@ type OptionalProps> = { export type Props = RequiredProps & OptionalProps & VirtualizedListProps; +export type ScrollToLocationParamsType = {| + animated?: ?boolean, + itemIndex: number, + sectionIndex: number, + viewOffset?: number, + viewPosition?: number, +|}; type DefaultProps = {| ...typeof VirtualizedList.defaultProps, data: $ReadOnlyArray, |}; + type State = {childProps: VirtualizedListProps}; /** @@ -139,22 +147,17 @@ class VirtualizedSectionList< data: [], }; - scrollToLocation(params: { - animated?: ?boolean, - itemIndex: number, - sectionIndex: number, - viewPosition?: number, - }) { + scrollToLocation(params: ScrollToLocationParamsType) { let index = params.itemIndex; for (let i = 0; i < params.sectionIndex; i++) { index += this.props.getItemCount(this.props.sections[i].data) + 2; } - let viewOffset = 0; + let viewOffset = params.viewOffset || 0; if (params.itemIndex > 0 && this.props.stickySectionHeadersEnabled) { const frame = this._listRef._getFrameMetricsApprox( index - params.itemIndex, ); - viewOffset = frame.length; + viewOffset += frame.length; } const toIndexParams = { ...params, diff --git a/Libraries/Lists/__tests__/VirtualizedSectionList-test.js b/Libraries/Lists/__tests__/VirtualizedSectionList-test.js index d52e9b06bff835..2ecbe701ca5644 100644 --- a/Libraries/Lists/__tests__/VirtualizedSectionList-test.js +++ b/Libraries/Lists/__tests__/VirtualizedSectionList-test.js @@ -161,4 +161,98 @@ describe('VirtualizedSectionList', () => { ); expect(component).toMatchSnapshot(); }); + + describe('scrollToLocation', () => { + const ITEM_HEIGHT = 100; + + const createVirtualizedSectionList = props => { + const component = ReactTestRenderer.create( + } + getItem={(data, key) => data[key]} + getItemCount={data => data.length} + getItemLayout={(data, index) => ({ + length: ITEM_HEIGHT, + offset: ITEM_HEIGHT * index, + index, + })} + {...props} + />, + ); + const instance = component.getInstance(); + const spy = jest.fn(); + instance._listRef.scrollToIndex = spy; + return { + instance, + spy, + }; + }; + + it('when sticky stickySectionHeadersEnabled={true}, header height is added to the developer-provided viewOffset', () => { + const {instance, spy} = createVirtualizedSectionList({ + stickySectionHeadersEnabled: true, + }); + + const viewOffset = 25; + + instance.scrollToLocation({ + sectionIndex: 0, + itemIndex: 1, + viewOffset, + }); + expect(spy).toHaveBeenCalledWith({ + index: 1, + itemIndex: 1, + sectionIndex: 0, + viewOffset: viewOffset + ITEM_HEIGHT, + }); + }); + + it.each([ + [ + // prevents #18098 + {sectionIndex: 0, itemIndex: 0}, + { + index: 0, + itemIndex: 0, + sectionIndex: 0, + viewOffset: 0, + }, + ], + [ + {sectionIndex: 2, itemIndex: 1}, + { + index: 11, + itemIndex: 1, + sectionIndex: 2, + viewOffset: 0, + }, + ], + [ + { + sectionIndex: 0, + itemIndex: 1, + viewOffset: 25, + }, + { + index: 1, + itemIndex: 1, + sectionIndex: 0, + viewOffset: 25, + }, + ], + ])( + 'given sectionIndex, itemIndex and viewOffset, scrollToIndex is called with correct params', + (scrollToLocationParams, expected) => { + const {instance, spy} = createVirtualizedSectionList(); + + instance.scrollToLocation(scrollToLocationParams); + expect(spy).toHaveBeenCalledWith(expected); + }, + ); + }); });