fix scrollToLocation and prevent regressions

facebook · Aug 9, 2019 · a2cc57e · a2cc57e
1 parent 2e32b8c
commit a2cc57e
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 16 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,7 +119,13 @@ 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>,
 };
@@ -138,22 +144,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);
+      },
+    );
+  });
 });