Changed default overscan behavior for List and Table to ensure there'…

…s always at least 1 row overscanned in a given direction. This helps with a keyboard accessibility (TAB / TAB+SHIFT) problem reported in issue #625. Grid overscan behavior is not adjusted by default because I didn't want to impact performance too much.

source/Grid/Grid.jest.js

@@ -4,7 +4,7 @@ import { Simulate } from 'react-addons-test-utils'
 import { render } from '../TestUtils'
 import Grid, { DEFAULT_SCROLLING_RESET_TIME_INTERVAL } from './Grid'
 import { CellMeasurer, CellMeasurerCache } from '../CellMeasurer'
-import { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './utils/defaultOverscanIndicesGetter'
+import { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './defaultOverscanIndicesGetter'
 import { DEFAULT_MAX_SCROLL_SIZE } from './utils/ScalingCellSizeAndPositionManager'
 
 const DEFAULT_COLUMN_WIDTH = 50
@@ -424,7 +424,7 @@ describe('Grid', () => {
       expect(grid.state.scrollTop).toEqual(920)
     })
 
-    it('should take scrollbar size into account when aligning cells', () => {
+    it('should take scrollbar size into account when aligning cells', () => {
       const grid = render(getMarkup({
         columnWidth: 50,
         columnCount: 100,

source/Grid/Grid.js

@@ -4,7 +4,7 @@ import cn from 'classnames'
 import calculateSizeAndPositionDataAndUpdateScrollOffset from './utils/calculateSizeAndPositionDataAndUpdateScrollOffset'
 import ScalingCellSizeAndPositionManager from './utils/ScalingCellSizeAndPositionManager'
 import createCallbackMemoizer from '../utils/createCallbackMemoizer'
-import defaultOverscanIndicesGetter, { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './utils/defaultOverscanIndicesGetter'
+import defaultOverscanIndicesGetter, { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './defaultOverscanIndicesGetter'
 import updateScrollIndexHelper from './utils/updateScrollIndexHelper'
 import defaultCellRangeRenderer from './defaultCellRangeRenderer'
 import scrollbarSize from 'dom-helpers/util/scrollbarSize'

source/Grid/accessibilityOverscanIndicesGetter.jest.js

@@ -0,0 +1,110 @@
+import overscanIndicesGetter, {
+  SCROLL_DIRECTION_BACKWARD,
+  SCROLL_DIRECTION_FORWARD
+} from './accessibilityOverscanIndicesGetter'
+
+describe('overscanIndicesGetter', () => {
+  function testHelper ({
+    cellCount,
+    startIndex,
+    stopIndex,
+    overscanCellsCount,
+    scrollDirection
+  }) {
+    return overscanIndicesGetter({
+      cellCount,
+      overscanCellsCount,
+      scrollDirection,
+      startIndex,
+      stopIndex
+    })
+  }
+
+  it('should still overscan by 1 (for keyboard accessibility) if :overscanCellsCount is 0', () => {
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 10,
+        stopIndex: 20,
+        overscanCellsCount: 0,
+        scrollDirection: SCROLL_DIRECTION_BACKWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 9,
+      overscanStopIndex: 21
+    })
+
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 10,
+        stopIndex: 20,
+        overscanCellsCount: 0,
+        scrollDirection: SCROLL_DIRECTION_FORWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 9,
+      overscanStopIndex: 21
+    })
+  })
+
+  it('should overscan forward', () => {
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 20,
+        stopIndex: 30,
+        overscanCellsCount: 10,
+        scrollDirection: SCROLL_DIRECTION_FORWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 19,
+      overscanStopIndex: 40
+    })
+  })
+
+  it('should overscan backward', () => {
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 20,
+        stopIndex: 30,
+        overscanCellsCount: 10,
+        scrollDirection: SCROLL_DIRECTION_BACKWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 10,
+      overscanStopIndex: 31
+    })
+  })
+
+  it('should not overscan beyond the start of the list', () => {
+    expect(
+      testHelper({
+        cellCount: 100,
+        startIndex: 5,
+        stopIndex: 15,
+        overscanCellsCount: 10,
+        scrollDirection: SCROLL_DIRECTION_BACKWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 0,
+      overscanStopIndex: 16
+    })
+  })
+
+  it('should not overscan beyond the end of the list', () => {
+    expect(
+      testHelper({
+        cellCount: 25,
+        startIndex: 10,
+        stopIndex: 20,
+        overscanCellsCount: 10,
+        scrollDirection: SCROLL_DIRECTION_FORWARD
+      })
+    ).toEqual({
+      overscanStartIndex: 9,
+      overscanStopIndex: 24
+    })
+  })
+})

source/Grid/accessibilityOverscanIndicesGetter.js

@@ -0,0 +1,41 @@
+export const SCROLL_DIRECTION_BACKWARD = -1
+export const SCROLL_DIRECTION_FORWARD = 1
+
+export const SCROLL_DIRECTION_HORIZONTAL = 'horizontal'
+export const SCROLL_DIRECTION_VERTICAL = 'vertical'
+/**
+ * Calculates the number of cells to overscan before and after a specified range.
+ * This function ensures that overscanning doesn't exceed the available cells.
+ *
+ * @param direction One of SCROLL_DIRECTION_HORIZONTAL or SCROLL_DIRECTION_VERTICAL
+ * @param cellCount Number of rows or columns in the current axis
+ * @param scrollDirection One of SCROLL_DIRECTION_BACKWARD or SCROLL_DIRECTION_FORWARD
+ * @param overscanCellsCount Maximum number of cells to over-render in either direction
+ * @param startIndex Begin of range of visible cells
+ * @param stopIndex End of range of visible cells
+ */
+export default function defaultOverscanIndicesGetter ({ direction, cellCount, overscanCellsCount, scrollDirection, startIndex, stopIndex }) {
+  let overscanStartIndex
+  let overscanStopIndex
+
+  // Make sure we render at least 1 cell extra before and after (except near boundaries)
+  // This is necessary in order to support keyboard navigation (TAB/SHIFT+TAB) in some cases
+  // For more info see issues #625
+  overscanCellsCount = Math.max(1, overscanCellsCount)
+
+  switch (scrollDirection) {
+    case SCROLL_DIRECTION_FORWARD:
+      overscanStartIndex = startIndex - 1
+      overscanStopIndex = stopIndex + overscanCellsCount
+      break
+    case SCROLL_DIRECTION_BACKWARD:
+      overscanStartIndex = startIndex - overscanCellsCount
+      overscanStopIndex = stopIndex + 1
+      break
+  }
+
+  return {
+    overscanStartIndex: Math.max(0, overscanStartIndex),
+    overscanStopIndex: Math.min(cellCount - 1, overscanStopIndex)
+  }
+}

source/Grid/index.js

@@ -1,4 +1,6 @@
 /** @flow */
 export default from './Grid'
 export Grid from './Grid'
+export accessibilityOverscanIndicesGetter from './accessibilityOverscanIndicesGetter'
 export defaultCellRangeRenderer from './defaultCellRangeRenderer'
+export defaultOverscanIndicesGetter from './defaultOverscanIndicesGetter'

source/List/List.jest.js

@@ -4,6 +4,7 @@ import { render } from '../TestUtils'
 import { Simulate } from 'react-addons-test-utils'
 import Immutable from 'immutable'
 import List from './List'
+import { defaultOverscanIndicesGetter } from '../Grid'
 
 describe('List', () => {
   const array = []
@@ -12,6 +13,15 @@ describe('List', () => {
   }
   const names = Immutable.fromJS(array)
 
+  // Override default behavior of overscanning by at least 1 (for accessibility)
+  // Because it makes for simple tests below
+  function overscanIndicesGetter ({ direction, cellCount, overscanCellsCount, scrollDirection, startIndex, stopIndex }) {
+    return {
+      overscanStartIndex: startIndex,
+      overscanStopIndex: stopIndex
+    }
+  }
+
   function getMarkup (props = {}) {
     function rowRenderer ({ index, key, style }) {
       return (
@@ -28,6 +38,7 @@ describe('List', () => {
     return (
       <List
         height={100}
+        overscanIndicesGetter={overscanIndicesGetter}
         overscanRowCount={0}
         rowHeight={10}
         rowCount={names.size}
@@ -281,50 +292,26 @@ describe('List', () => {
 
   describe('overscanRowCount', () => {
     it('should not overscan by default', () => {
-      let overscanStartIndex, overscanStopIndex, startIndex, stopIndex
+      const mock = jest.fn()
+      mock.mockImplementation(overscanIndicesGetter)
+
       render(getMarkup({
-        onRowsRendered: params => ({ overscanStartIndex, overscanStopIndex, startIndex, stopIndex } = params)
+        overscanIndicesGetter: mock
       }))
-      expect(overscanStartIndex).toEqual(startIndex)
-      expect(overscanStopIndex).toEqual(stopIndex)
+      expect(mock.mock.calls[0][0].overscanCellsCount).toEqual(0)
+      expect(mock.mock.calls[1][0].overscanCellsCount).toEqual(0)
     })
 
     it('should overscan the specified amount', () => {
-      let overscanStartIndex, overscanStopIndex, startIndex, stopIndex
-      render(getMarkup({
-        onRowsRendered: params => ({ overscanStartIndex, overscanStopIndex, startIndex, stopIndex } = params),
-        overscanRowCount: 10,
-        scrollToIndex: 30
-      }))
-      expect(overscanStartIndex).toEqual(21)
-      expect(startIndex).toEqual(21)
-      expect(stopIndex).toEqual(30)
-      expect(overscanStopIndex).toEqual(40)
-    })
+      const mock = jest.fn()
+      mock.mockImplementation(overscanIndicesGetter)
 
-    it('should not overscan beyond the start of the list', () => {
-      let overscanStartIndex, overscanStopIndex, startIndex, stopIndex
       render(getMarkup({
-        onRowsRendered: params => ({ overscanStartIndex, overscanStopIndex, startIndex, stopIndex } = params),
+        overscanIndicesGetter: mock,
         overscanRowCount: 10
       }))
-      expect(overscanStartIndex).toEqual(0)
-      expect(startIndex).toEqual(0)
-      expect(stopIndex).toEqual(9)
-      expect(overscanStopIndex).toEqual(19)
-    })
-
-    it('should not overscan beyond the end of the list', () => {
-      let overscanStartIndex, overscanStopIndex, startIndex, stopIndex
-      render(getMarkup({
-        onRowsRendered: params => ({ overscanStartIndex, overscanStopIndex, startIndex, stopIndex } = params),
-        overscanRowCount: 10,
-        rowCount: 15
-      }))
-      expect(overscanStartIndex).toEqual(0)
-      expect(startIndex).toEqual(0)
-      expect(stopIndex).toEqual(9)
-      expect(overscanStopIndex).toEqual(14)
+      expect(mock.mock.calls[0][0].overscanCellsCount).toEqual(0)
+      expect(mock.mock.calls[1][0].overscanCellsCount).toEqual(10)
     })
   })
 
@@ -446,6 +433,7 @@ describe('List', () => {
     }
     findDOMNode(render(getMarkup({
       height: 50,
+      overscanIndicesGetter: defaultOverscanIndicesGetter,
       overscanRowCount: 1,
       rowHeight: 50,
       rowRenderer

source/List/List.js

@@ -1,5 +1,5 @@
 /** @flow */
-import Grid from '../Grid'
+import Grid, { accessibilityOverscanIndicesGetter } from '../Grid'
 import React, { PropTypes, PureComponent } from 'react'
 import cn from 'classnames'
 
@@ -42,19 +42,22 @@ export default class List extends PureComponent {
      */
     onRowsRendered: PropTypes.func.isRequired,
 
-    /**
-     * Number of rows to render above/below the visible bounds of the list.
-     * These rows can help for smoother scrolling on touch devices.
-     */
-    overscanRowCount: PropTypes.number.isRequired,
-
     /**
      * Callback invoked whenever the scroll offset changes within the inner scrollable region.
      * This callback can be used to sync scrolling between lists, tables, or grids.
      * ({ clientHeight, scrollHeight, scrollTop }): void
      */
     onScroll: PropTypes.func.isRequired,
 
+    /** See Grid#overscanIndicesGetter */
+    overscanIndicesGetter: PropTypes.func.isRequired,
+
+    /**
+     * Number of rows to render above/below the visible bounds of the list.
+     * These rows can help for smoother scrolling on touch devices.
+     */
+    overscanRowCount: PropTypes.number.isRequired,
+
     /**
      * Either a fixed row height (number) or a function that returns the height of a row given its index.
      * ({ index: number }): number
@@ -91,6 +94,7 @@ export default class List extends PureComponent {
     noRowsRenderer: () => null,
     onRowsRendered: () => null,
     onScroll: () => null,
+    overscanIndicesGetter: accessibilityOverscanIndicesGetter,
     overscanRowCount: 10,
     scrollToAlignment: 'auto',
     scrollToIndex: -1,