diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d754405f..7442a1260 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ Changelog ------------ +##### 8.5.3 +Changed overscan rows/cols behavior as described [here](https://github.com/bvaughn/react-virtualized/pull/478). +This change targets performance improvements only and should have no other noticeable impact. + ##### 8.5.2 Added guard against potential `null` return value from `getComputedStyle` for hidden elements (see PR #465). diff --git a/package.json b/package.json index ae801d107..432d4d7ad 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "description": "React components for efficiently rendering large, scrollable lists and tabular data", "author": "Brian Vaughn ", "user": "bvaughn", - "version": "8.5.2", + "version": "8.5.3", "homepage": "https://github.com/bvaughn/react-virtualized", "main": "dist/commonjs/index.js", "module": "dist/es/index.js", diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js index 66e57f91e..15f6f4302 100644 --- a/source/Grid/Grid.js +++ b/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 getOverscanIndices, { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FIXED, SCROLL_DIRECTION_FORWARD } from './utils/getOverscanIndices' +import getOverscanIndices, { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './utils/getOverscanIndices' import getScrollbarSize from 'dom-helpers/util/scrollbarSize' import shallowCompare from 'react-addons-shallow-compare' import updateScrollIndexHelper from './utils/updateScrollIndexHelper' @@ -210,8 +210,8 @@ export default class Grid extends Component { this.state = { isScrolling: false, - scrollDirectionHorizontal: SCROLL_DIRECTION_FIXED, - scrollDirectionVertical: SCROLL_DIRECTION_FIXED, + scrollDirectionHorizontal: SCROLL_DIRECTION_FORWARD, + scrollDirectionVertical: SCROLL_DIRECTION_FORWARD, scrollLeft: 0, scrollTop: 0 } @@ -705,9 +705,7 @@ export default class Grid extends Component { this._cellCache = {} this.setState({ - isScrolling: false, - scrollDirectionHorizontal: SCROLL_DIRECTION_FIXED, - scrollDirectionVertical: SCROLL_DIRECTION_FIXED + isScrolling: false }) } diff --git a/source/Grid/Grid.test.js b/source/Grid/Grid.test.js index 21865192d..848a0f9a4 100644 --- a/source/Grid/Grid.test.js +++ b/source/Grid/Grid.test.js @@ -4,7 +4,7 @@ import { findDOMNode } from 'react-dom' import { Simulate } from 'react-addons-test-utils' import { render } from '../TestUtils' import Grid, { DEFAULT_SCROLLING_RESET_TIME_INTERVAL } from './Grid' -import { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FIXED, SCROLL_DIRECTION_FORWARD } from './utils/getOverscanIndices' +import { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './utils/getOverscanIndices' const DEFAULT_COLUMN_WIDTH = 50 const DEFAULT_HEIGHT = 100 @@ -800,11 +800,11 @@ describe('Grid', () => { scrollToColumn: 25, scrollToRow: 50 })) - expect(helper.columnOverscanStartIndex()).toEqual(20) + expect(helper.columnOverscanStartIndex()).toEqual(22) expect(helper.columnOverscanStopIndex()).toEqual(27) expect(helper.columnStartIndex()).toEqual(22) expect(helper.columnStopIndex()).toEqual(25) - expect(helper.rowOverscanStartIndex()).toEqual(41) + expect(helper.rowOverscanStartIndex()).toEqual(46) expect(helper.rowOverscanStopIndex()).toEqual(55) expect(helper.rowStartIndex()).toEqual(46) expect(helper.rowStopIndex()).toEqual(50) @@ -835,15 +835,6 @@ describe('Grid', () => { scrollTop: 50 })) - expect(grid.state.scrollDirectionHorizontal).toEqual(SCROLL_DIRECTION_FIXED) - expect(grid.state.scrollDirectionVertical).toEqual(SCROLL_DIRECTION_FIXED) - - simulateScroll({ - grid, - scrollLeft: 100, - scrollTop: 100 - }) - expect(grid.state.scrollDirectionHorizontal).toEqual(SCROLL_DIRECTION_FORWARD) expect(grid.state.scrollDirectionVertical).toEqual(SCROLL_DIRECTION_FORWARD) @@ -855,9 +846,18 @@ describe('Grid', () => { expect(grid.state.scrollDirectionHorizontal).toEqual(SCROLL_DIRECTION_BACKWARD) expect(grid.state.scrollDirectionVertical).toEqual(SCROLL_DIRECTION_BACKWARD) + + simulateScroll({ + grid, + scrollLeft: 100, + scrollTop: 100 + }) + + expect(grid.state.scrollDirectionHorizontal).toEqual(SCROLL_DIRECTION_FORWARD) + expect(grid.state.scrollDirectionVertical).toEqual(SCROLL_DIRECTION_FORWARD) }) - it('should overscan more in the direction being scrolled', async (done) => { + it('should overscan in the direction being scrolled', async (done) => { const helper = createHelper() let onSectionRenderedResolve @@ -891,26 +891,36 @@ describe('Grid', () => { // It should overscan in the direction being scrolled while scroll is in progress expect(helper.columnOverscanStartIndex()).toEqual(4) - expect(helper.columnOverscanStopIndex()).toEqual(11) + expect(helper.columnOverscanStopIndex()).toEqual(9) expect(helper.columnStartIndex()).toEqual(4) expect(helper.columnStopIndex()).toEqual(7) expect(helper.rowOverscanStartIndex()).toEqual(10) - expect(helper.rowOverscanStopIndex()).toEqual(24) + expect(helper.rowOverscanStopIndex()).toEqual(19) expect(helper.rowStartIndex()).toEqual(10) expect(helper.rowStopIndex()).toEqual(14) - // Allow scrolling timeout to complete so that cell cache is reset - await new Promise(resolve => setTimeout(resolve, 500)) + // Wait until the onSectionRendered handler / debouncer has processed + onSectionRenderedPromise = new Promise(resolve => { + onSectionRenderedResolve = resolve + }) + + simulateScroll({ + grid, + scrollLeft: 100, + scrollTop: 100 + }) + + await onSectionRenderedPromise // It reset overscan once scrolling has finished - expect(helper.columnOverscanStartIndex()).toEqual(2) - expect(helper.columnOverscanStopIndex()).toEqual(9) - expect(helper.columnStartIndex()).toEqual(4) - expect(helper.columnStopIndex()).toEqual(7) - expect(helper.rowOverscanStartIndex()).toEqual(5) - expect(helper.rowOverscanStopIndex()).toEqual(19) - expect(helper.rowStartIndex()).toEqual(10) - expect(helper.rowStopIndex()).toEqual(14) + expect(helper.columnOverscanStartIndex()).toEqual(0) + expect(helper.columnOverscanStopIndex()).toEqual(5) + expect(helper.columnStartIndex()).toEqual(2) + expect(helper.columnStopIndex()).toEqual(5) + expect(helper.rowOverscanStartIndex()).toEqual(0) + expect(helper.rowOverscanStopIndex()).toEqual(9) + expect(helper.rowStartIndex()).toEqual(5) + expect(helper.rowStopIndex()).toEqual(9) done() }) diff --git a/source/Grid/utils/getOverscanIndices.js b/source/Grid/utils/getOverscanIndices.js index 11841dfff..203513b93 100644 --- a/source/Grid/utils/getOverscanIndices.js +++ b/source/Grid/utils/getOverscanIndices.js @@ -1,5 +1,4 @@ export const SCROLL_DIRECTION_BACKWARD = -1 -export const SCROLL_DIRECTION_FIXED = 0 export const SCROLL_DIRECTION_FORWARD = 1 /** @@ -16,15 +15,15 @@ export default function getOverscanIndices ({ cellCount, overscanCellsCount, scr let overscanStartIndex let overscanStopIndex - if (scrollDirection === SCROLL_DIRECTION_FORWARD) { - overscanStartIndex = startIndex - overscanStopIndex = stopIndex + overscanCellsCount * 2 - } else if (scrollDirection === SCROLL_DIRECTION_BACKWARD) { - overscanStartIndex = startIndex - overscanCellsCount * 2 - overscanStopIndex = stopIndex - } else { - overscanStartIndex = startIndex - overscanCellsCount - overscanStopIndex = stopIndex + overscanCellsCount + switch (scrollDirection) { + case SCROLL_DIRECTION_FORWARD: + overscanStartIndex = startIndex + overscanStopIndex = stopIndex + overscanCellsCount + break + case SCROLL_DIRECTION_BACKWARD: + overscanStartIndex = startIndex - overscanCellsCount + overscanStopIndex = stopIndex + break } return { diff --git a/source/Grid/utils/getOverscanIndices.test.js b/source/Grid/utils/getOverscanIndices.test.js index 6e159247e..cb7124448 100644 --- a/source/Grid/utils/getOverscanIndices.test.js +++ b/source/Grid/utils/getOverscanIndices.test.js @@ -1,11 +1,16 @@ import getOverscanIndices, { SCROLL_DIRECTION_BACKWARD, - SCROLL_DIRECTION_FIXED, SCROLL_DIRECTION_FORWARD } from './getOverscanIndices' describe('getOverscanIndices', () => { - function testHelper (cellCount, startIndex, stopIndex, overscanCellsCount, scrollDirection = SCROLL_DIRECTION_FIXED) { + function testHelper ({ + cellCount, + startIndex, + stopIndex, + overscanCellsCount, + scrollDirection + }) { return getOverscanIndices({ cellCount, overscanCellsCount, @@ -16,41 +21,89 @@ describe('getOverscanIndices', () => { } it('should not overscan if :overscanCellsCount is 0', () => { - expect(testHelper(100, 10, 20, 0)).toEqual({ + expect( + testHelper({ + cellCount: 100, + startIndex: 10, + stopIndex: 20, + overscanCellsCount: 0, + scrollDirection: SCROLL_DIRECTION_BACKWARD + }) + ).toEqual({ overscanStartIndex: 10, overscanStopIndex: 20 }) - }) - it('should overscan by the specified :overscanCellsCount', () => { - expect(testHelper(100, 10, 20, 10)).toEqual({ - overscanStartIndex: 0, - overscanStopIndex: 30 + expect( + testHelper({ + cellCount: 100, + startIndex: 10, + stopIndex: 20, + overscanCellsCount: 0, + scrollDirection: SCROLL_DIRECTION_FORWARD + }) + ).toEqual({ + overscanStartIndex: 10, + overscanStopIndex: 20 }) }) - it('should double the overscan in the direction being scrolled', () => { - expect(testHelper(100, 20, 30, 10, SCROLL_DIRECTION_FORWARD)).toEqual({ + it('should overscan forward', () => { + expect( + testHelper({ + cellCount: 100, + startIndex: 20, + stopIndex: 30, + overscanCellsCount: 10, + scrollDirection: SCROLL_DIRECTION_FORWARD + }) + ).toEqual({ overscanStartIndex: 20, - overscanStopIndex: 50 + overscanStopIndex: 40 }) + }) - expect(testHelper(100, 20, 30, 10, SCROLL_DIRECTION_BACKWARD)).toEqual({ - overscanStartIndex: 0, + it('should overscan backward', () => { + expect( + testHelper({ + cellCount: 100, + startIndex: 20, + stopIndex: 30, + overscanCellsCount: 10, + scrollDirection: SCROLL_DIRECTION_BACKWARD + }) + ).toEqual({ + overscanStartIndex: 10, overscanStopIndex: 30 }) }) it('should not overscan beyond the start of the list', () => { - expect(testHelper(100, 5, 15, 10)).toEqual({ + expect( + testHelper({ + cellCount: 100, + startIndex: 5, + stopIndex: 15, + overscanCellsCount: 10, + scrollDirection: SCROLL_DIRECTION_BACKWARD + }) + ).toEqual({ overscanStartIndex: 0, - overscanStopIndex: 25 + overscanStopIndex: 15 }) }) it('should not overscan beyond the end of the list', () => { - expect(testHelper(25, 10, 20, 10)).toEqual({ - overscanStartIndex: 0, + expect( + testHelper({ + cellCount: 25, + startIndex: 10, + stopIndex: 20, + overscanCellsCount: 10, + scrollDirection: SCROLL_DIRECTION_FORWARD + }) + ).toEqual({ + overscanStartIndex: 10, overscanStopIndex: 24 }) }) diff --git a/source/List/List.test.js b/source/List/List.test.js index 27827a841..a89ef5545 100644 --- a/source/List/List.test.js +++ b/source/List/List.test.js @@ -296,7 +296,7 @@ describe('List', () => { overscanRowCount: 10, scrollToIndex: 30 })) - expect(overscanStartIndex).toEqual(11) + expect(overscanStartIndex).toEqual(21) expect(startIndex).toEqual(21) expect(stopIndex).toEqual(30) expect(overscanStopIndex).toEqual(40) diff --git a/source/Table/Table.test.js b/source/Table/Table.test.js index 47a207096..e081cac01 100644 --- a/source/Table/Table.test.js +++ b/source/Table/Table.test.js @@ -778,7 +778,7 @@ describe('Table', () => { overscanRowCount: 10, scrollToIndex: 30 })) - expect(overscanStartIndex).toEqual(13) + expect(overscanStartIndex).toEqual(23) expect(startIndex).toEqual(23) expect(stopIndex).toEqual(30) expect(overscanStopIndex).toEqual(40)