fix SectionList scrollToLocation and prevent regressions (#25997)

Summary: Recently there were quite a few changes to this functionality, and they caused breakages #21577 #24034 #24734 #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: #25997 Test Plan: tests pass Differential Revision: D16784036 Pulled By: cpojer fbshipit-source-id: 46421250993785176634b30a2629a6e12f0c2278
facebook · Aug 13, 2019 · 8a82503 · 8a82503
1 parent 427b54e
commit 8a82503
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 15 deletions.
diff --git 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<SectionT: SectionBase<any>> 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
@@ -119,11 +119,19 @@ type OptionalProps<SectionT: SectionBase<any>> = {
 export type Props<SectionT> = RequiredProps<SectionT> &
   OptionalProps<SectionT> &
   VirtualizedListProps;
+export type ScrollToLocationParamsType = {|
+  animated?: ?boolean,
+  itemIndex: number,
+  sectionIndex: number,
+  viewOffset?: number,
+  viewPosition?: number,
+|};
 
 type DefaultProps = {|
   ...typeof VirtualizedList.defaultProps,
   data: $ReadOnlyArray<Item>,
 |};
+
 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
@@ -161,4 +161,98 @@ describe('VirtualizedSectionList', () => {
     );
     expect(component).toMatchSnapshot();
   });
+
+  describe('scrollToLocation', () => {
+    const ITEM_HEIGHT = 100;
+
+    const createVirtualizedSectionList = props => {
+      const component = ReactTestRenderer.create(
+        <VirtualizedSectionList
+          sections={[
+            {title: 's1', data: [{key: 'i1.1'}, {key: 'i1.2'}, {key: 'i1.3'}]},
+            {title: 's2', data: [{key: 'i2.1'}, {key: 'i2.2'}, {key: 'i2.3'}]},
+          ]}
+          renderItem={({item}) => <item value={item.key} />}
+          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);
+      },
+    );
+  });
 });