FixedSizeList fixes for onItemsRendered & partially-visible items

src/FixedSizeList.js

@@ -23,12 +23,13 @@ const FixedSizeList = createListComponent({
     // TODO Deprecate direction "horizontal"
     const isHorizontal = direction === 'horizontal' || layout === 'horizontal';
     const size = (((isHorizontal ? width : height): any): number);
-    const maxOffset = Math.max(
+    const lastViewportOffset = Math.max(
       0,
-      Math.min(
-        itemCount * ((itemSize: any): number) - size,
-        index * ((itemSize: any): number)
-      )
+      itemCount * ((itemSize: any): number) - size
+    );
+    const maxOffset = Math.min(
+      lastViewportOffset,
+      index * ((itemSize: any): number)
     );
     const minOffset = Math.max(
       0,
@@ -51,14 +52,35 @@ const FixedSizeList = createListComponent({
         return maxOffset;
       case 'end':
         return minOffset;
-      case 'center':
-        return Math.round(minOffset + (maxOffset - minOffset) / 2);
+      case 'center': {
+        // "Centered" offset is usually the average of the min and max
+        // offsets. But near the beginning or end of the list, this math
+        // doesn't produce the actual closest-to-center offset, so we
+        // override.
+        const centered = Math.round(minOffset + (maxOffset - minOffset) / 2);
+        if (centered < Math.ceil(size / 2)) {
+          return 0; // near the beginning
+        } else if (centered > lastViewportOffset + Math.floor(size / 2)) {
+          return lastViewportOffset; // near the end
+        } else {
+          return centered;
+        }
+      }
       case 'auto':
       default:
         if (scrollOffset >= minOffset && scrollOffset <= maxOffset) {
           return scrollOffset;
-        } else if (scrollOffset - minOffset < maxOffset - scrollOffset) {
-          return minOffset;
+        } else if (scrollOffset < minOffset) {
+          // For auto alignment, if size isn't an even multiple of itemSize,
+          // always put the partial item at the end of the viewport because it
+          // looks better than at the beginning. Exception: don't do this if the
+          // viewport is already scrolled to the end.
+          const remainder = size % ((itemSize: any): number);
+          if (minOffset + remainder < lastViewportOffset) {
+            return minOffset + remainder;
+          } else {
+            return minOffset;
+          }
         } else {
           return maxOffset;
         }
@@ -83,14 +105,17 @@ const FixedSizeList = createListComponent({
     const isHorizontal = direction === 'horizontal' || layout === 'horizontal';
     const offset = startIndex * ((itemSize: any): number);
     const size = (((isHorizontal ? width : height): any): number);
+    // How far before the scrollOffset does the first visible item start?
+    // Will be zero if scrollOffset is an item boundary.
+    const startingPartialSize = scrollOffset - offset;
+    const visibleItems = Math.ceil(
+      (size + startingPartialSize) / ((itemSize: any): number)
+    );
     return Math.max(
       0,
       Math.min(
         itemCount - 1,
-        startIndex +
-          Math.floor(
-            (size + (scrollOffset - offset)) / ((itemSize: any): number)
-          )
+        startIndex + visibleItems - 1 // -1 is because stop index is inclusive
       )
     );
   },

src/__tests__/FixedSizeList.js

@@ -77,15 +77,15 @@ describe('FixedSizeList', () => {
 
   it('should render a list of rows', () => {
     ReactTestRenderer.create(<FixedSizeList {...defaultProps} />);
-    expect(itemRenderer).toHaveBeenCalledTimes(7);
+    expect(itemRenderer).toHaveBeenCalledTimes(6);
     expect(onItemsRendered.mock.calls).toMatchSnapshot();
   });
 
   it('should render a list of columns', () => {
     ReactTestRenderer.create(
       <FixedSizeList {...defaultProps} layout="horizontal" />
     );
-    expect(itemRenderer).toHaveBeenCalledTimes(5);
+    expect(itemRenderer).toHaveBeenCalledTimes(4);
     expect(onItemsRendered.mock.calls).toMatchSnapshot();
   });
 
@@ -421,13 +421,59 @@ describe('FixedSizeList', () => {
       // Scroll down enough to show item 10 at the bottom.
       rendered.getInstance().scrollToItem(10, 'auto');
       // No need to scroll again; item 9 is already visible.
-      // Overscan indices will change though, since direction changes.
+      // Because there's no scrolling, it won't call onItemsRendered.
       rendered.getInstance().scrollToItem(9, 'auto');
       // Scroll up enough to show item 2 at the top.
       rendered.getInstance().scrollToItem(2, 'auto');
       expect(onItemsRendered.mock.calls).toMatchSnapshot();
     });
 
+    it('scroll with align = "auto" should work with partially-visible items', () => {
+      const rendered = ReactTestRenderer.create(
+        // Create list where items don't fit exactly into container.
+        // The container has space for 3 1/3 items.
+        <FixedSizeList {...defaultProps} itemSize={30} />
+      );
+      // Scroll down enough to show item 10 at the bottom.
+      // Should show 4 items: 3 full and one partial at the end
+      rendered.getInstance().scrollToItem(10, 'auto');
+      // No need to scroll again; item 9 is already visible.
+      // Because there's no scrolling, it won't call onItemsRendered.
+      rendered.getInstance().scrollToItem(9, 'auto');
+      // Scroll to near the end. #96 will be shown as partial.
+      rendered.getInstance().scrollToItem(99, 'auto');
+      // Scroll back to show #96 fully. This will cause #99 to be shown as a
+      // partial. Because #96 was already shown previously as a partial, all
+      // props of the onItemsRendered will be the same. This means that even
+      // though a scroll happened in the DOM, the onItemsRendered event is
+      // not triggered. IMHO this is probably a bug, but it's one we can fix
+      // later via a separate PR.
+      rendered.getInstance().scrollToItem(96, 'auto');
+      // Scroll forward again. Because item #99 was already shown partially,
+      // all props of the onItemsRendered will be the same.
+      rendered.getInstance().scrollToItem(99, 'auto');
+      // Scroll to the second item. A partial fifth item should
+      // be shown after it.
+      rendered.getInstance().scrollToItem(1, 'auto');
+      // Scroll to the first item. Now the fourth item should be a partial.
+      rendered.getInstance().scrollToItem(0, 'auto');
+      expect(onItemsRendered.mock.calls).toMatchSnapshot();
+    });
+
+    it('scroll with align = "auto" should work with very small lists and partial items', () => {
+      const rendered = ReactTestRenderer.create(
+        // Create list with only two items, one of which will be shown as a partial.
+        <FixedSizeList {...defaultProps} itemSize={60} itemCount={2} />
+      );
+      // Show the second item fully. The first item should be a partial.
+      rendered.getInstance().scrollToItem(1, 'auto');
+      // Go back to the first item. The second should be a partial again.
+      rendered.getInstance().scrollToItem(0, 'auto');
+      // None of the scrollToItem calls above should actually cause a scroll,
+      // so there will only be one snapshot.
+      expect(onItemsRendered.mock.calls).toMatchSnapshot();
+    });
+
     it('should scroll to the correct item for align = "start"', () => {
       const rendered = ReactTestRenderer.create(
         <FixedSizeList {...defaultProps} />
@@ -577,6 +623,29 @@ describe('FixedSizeList', () => {
       simulateScroll(instance, 200);
       expect(onScroll.mock.calls[0][0].scrollUpdateWasRequested).toBe(false);
     });
+
+    it('scrolling should report partial items correctly in onItemsRendered', () => {
+      // Use ReactDOM renderer so the container ref works correctly.
+      const instance = ReactDOM.render(
+        <FixedSizeList {...defaultProps} initialScrollOffset={20} />,
+        document.createElement('div')
+      );
+      // Scroll 2 items fwd, but thanks to the initialScrollOffset, we should
+      // still be showing partials on both ends.
+      simulateScroll(instance, 70);
+      // Scroll a little fwd to cause partials to be hidden
+      simulateScroll(instance, 75);
+      // Scroll backwards to show partials again
+      simulateScroll(instance, 70);
+      // Scroll near the end so that the last item is shown
+      // as a partial.
+      simulateScroll(instance, 96 * 25 - 5);
+      // Scroll to the end. No partials.
+      simulateScroll(instance, 96 * 25);
+      // Verify that backwards scrolling near the end works OK.
+      simulateScroll(instance, 96 * 25 - 5);
+      expect(onItemsRendered.mock.calls).toMatchSnapshot();
+    });
   });
 
   describe('itemKey', () => {