Fix VirtualizedSectionList.scrollToLocation off-by-one and sticky header offset (#58329)

Summary:

`VirtualizedSectionList.scrollToLocation`, and `SectionList` through its wrapper, mapped `itemIndex` to the underlying flat list without skipping the section header. As a result, `itemIndex: 0` targeted the header and every other item was one row early. Sticky-header compensation was also skipped for the first item, allowing the header to obscure it.

This change adds the header row to the flattened target index and always applies the sticky-header offset using the current section header metrics. The public API remains zero-based, but callers that compensated for the old behavior must remove that compensation.

Migration:
- Replace `itemIndex: n + 1` workarounds with `itemIndex: n`.
- Replace `itemIndex: data.length` last-item workarounds with `itemIndex: data.length - 1`.

Thanks to Marc Rousavy (mrousavy) for the diagnosis: #50143

Changelog:
[General][Breaking] - Fix `SectionList` and `VirtualizedSectionList` `scrollToLocation` to account for section headers and sticky headers correctly.

Reviewed By: javache, ryanfrawley

Differential Revision: D118547738

Author: amroaltah

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: {

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<SectionBase<{key: string}>>,
+      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<SectionBase<{key: string}>>;
+
+      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(
           <VirtualizedSectionList
-            sections={
-              [
-                // $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<SectionBase<{key: string}>>
-            }
+            sections={sections}
             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}
+            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<SectionBase<{key: string}>>,
+        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,
+      });
+    });
   });
 });