Gracefully handle negative initialScrollIndex in VirtualizedList_EXPE…

…RIMENTAL Summary: Existing code may pass negative values for `initialScrollIndex`, which the previous implemnentation handled gracefully. Handle the case gracefully in VirtualizedList_EXPERIMENTAL as well. Changelog: [Internal][Fixed] - Gracefully handle negative initialScrollIndex in VirtualizedList_EXPERIMENTAL Reviewed By: rshest Differential Revision: D38183625 fbshipit-source-id: 9aea91c4cf3ce2190d769f34ce728a109efc88d4
Expensify · Aug 17, 2022 · e41fffc · e41fffc
1 parent 5a56e99
commit e41fffc
Show file tree

Hide file tree

Showing 4 changed files with 201 additions and 2 deletions.
diff --git a/Libraries/Lists/VirtualizedList_EXPERIMENTAL.js b/Libraries/Lists/VirtualizedList_EXPERIMENTAL.js
@@ -786,7 +786,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
 
       // The initially rendered cells are retained as part of the
       // "scroll-to-top" optimization
-      if (props.initialScrollIndex == null || props.initialScrollIndex === 0) {
+      if (props.initialScrollIndex == null || props.initialScrollIndex <= 0) {
         const initialRegion = VirtualizedList._initialRenderRegion(props);
         renderMask.addCells(initialRegion);
       }
@@ -808,7 +808,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
 
   static _initialRenderRegion(props: Props): {first: number, last: number} {
     const itemCount = props.getItemCount(props.data);
-    const scrollIndex = props.initialScrollIndex || 0;
+    const scrollIndex = Math.max(0, props.initialScrollIndex ?? 0);
 
     return {
       first: scrollIndex,

diff --git a/Libraries/Lists/__tests__/VirtualizedList-test.js b/Libraries/Lists/__tests__/VirtualizedList-test.js
@@ -701,6 +701,24 @@ it('unmounts sticky headers moved below viewport', () => {
   expect(component).toMatchSnapshot();
 });
 
+it('gracefully handles negaitve initialScrollIndex', () => {
+  const items = generateItems(10);
+  const ITEM_HEIGHT = 10;
+
+  const component = ReactTestRenderer.create(
+    <VirtualizedList
+      initialScrollIndex={-1}
+      initialNumToRender={4}
+      {...baseItemProps(items)}
+      {...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
+    />,
+  );
+
+  // Existing code assumes we handle this in some way. Do something reasonable
+  // here.
+  expect(component).toMatchSnapshot();
+});
+
 it('renders offset cells in initial render when initialScrollIndex set', () => {
   const items = generateItems(10);
   const ITEM_HEIGHT = 10;

diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList-test.js.snap
@@ -2714,6 +2714,91 @@ exports[`expands render area by maxToRenderPerBatch on tick 1`] = `
 </RCTScrollView>
 `;
 
+exports[`gracefully handles negaitve initialScrollIndex 1`] = `
+<RCTScrollView
+  data={
+    Array [
+      Object {
+        "key": 0,
+      },
+      Object {
+        "key": 1,
+      },
+      Object {
+        "key": 2,
+      },
+      Object {
+        "key": 3,
+      },
+      Object {
+        "key": 4,
+      },
+      Object {
+        "key": 5,
+      },
+      Object {
+        "key": 6,
+      },
+      Object {
+        "key": 7,
+      },
+      Object {
+        "key": 8,
+      },
+      Object {
+        "key": 9,
+      },
+    ]
+  }
+  getItem={[Function]}
+  getItemCount={[Function]}
+  getItemLayout={[Function]}
+  initialNumToRender={4}
+  initialScrollIndex={-1}
+  onContentSizeChange={[Function]}
+  onLayout={[Function]}
+  onMomentumScrollBegin={[Function]}
+  onMomentumScrollEnd={[Function]}
+  onScroll={[Function]}
+  onScrollBeginDrag={[Function]}
+  onScrollEndDrag={[Function]}
+  renderItem={[Function]}
+  scrollEventThrottle={50}
+  stickyHeaderIndices={Array []}
+>
+  <View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={0}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={1}
+      />
+    </View>
+    <View
+      style={null}
+    >
+      <MockCellItem
+        value={2}
+      />
+    </View>
+    <View
+      style={
+        Object {
+          "height": 70,
+        }
+      }
+    />
+  </View>
+</RCTScrollView>
+`;
+
 exports[`renders a zero-height tail spacer on initial render if getItemLayout not defined 1`] = `
 <RCTScrollView
   data={

diff --git a/Libraries/Lists/__tests__/__snapshots__/VirtualizedList_EXPERIMENTAL-test.js.snap b/Libraries/Lists/__tests__/__snapshots__/VirtualizedList_EXPERIMENTAL-test.js.snap
@@ -3491,6 +3491,102 @@ exports[`expands render area by maxToRenderPerBatch on tick 1`] = `
 </RCTScrollView>
 `;
 
+exports[`gracefully handles negaitve initialScrollIndex 1`] = `
+<RCTScrollView
+  data={
+    Array [
+      Object {
+        "key": 0,
+      },
+      Object {
+        "key": 1,
+      },
+      Object {
+        "key": 2,
+      },
+      Object {
+        "key": 3,
+      },
+      Object {
+        "key": 4,
+      },
+      Object {
+        "key": 5,
+      },
+      Object {
+        "key": 6,
+      },
+      Object {
+        "key": 7,
+      },
+      Object {
+        "key": 8,
+      },
+      Object {
+        "key": 9,
+      },
+    ]
+  }
+  getItem={[Function]}
+  getItemCount={[Function]}
+  getItemLayout={[Function]}
+  initialNumToRender={4}
+  initialScrollIndex={-1}
+  onContentSizeChange={[Function]}
+  onLayout={[Function]}
+  onMomentumScrollBegin={[Function]}
+  onMomentumScrollEnd={[Function]}
+  onScroll={[Function]}
+  onScrollBeginDrag={[Function]}
+  onScrollEndDrag={[Function]}
+  renderItem={[Function]}
+  scrollEventThrottle={50}
+  stickyHeaderIndices={Array []}
+>
+  <View>
+    <View
+      onFocusCapture={[Function]}
+      style={null}
+    >
+      <MockCellItem
+        value={0}
+      />
+    </View>
+    <View
+      onFocusCapture={[Function]}
+      style={null}
+    >
+      <MockCellItem
+        value={1}
+      />
+    </View>
+    <View
+      onFocusCapture={[Function]}
+      style={null}
+    >
+      <MockCellItem
+        value={2}
+      />
+    </View>
+    <View
+      onFocusCapture={[Function]}
+      style={null}
+    >
+      <MockCellItem
+        value={3}
+      />
+    </View>
+    <View
+      style={
+        Object {
+          "height": 60,
+        }
+      }
+    />
+  </View>
+</RCTScrollView>
+`;
+
 exports[`renders a zero-height tail spacer on initial render if getItemLayout not defined 1`] = `
 <RCTScrollView
   data={