Skip to content

Commit

Permalink
Grid: Allow users to opt out of rerendering on scroll stop (#1131)
Browse files Browse the repository at this point in the history
* don't clear visible cells from cellCache if isScrollingOptOut is specified

* adding test

* adding docs

* typo
  • Loading branch information
wuweiweiwu authored Jun 13, 2018
1 parent c1d7377 commit 438b567
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 1 deletion.
1 change: 1 addition & 0 deletions docs/Grid.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ A windowed grid of elements. `Grid` only renders cells necessary to fill itself
| height | Number || Height of Grid; this property determines the number of visible (vs virtualized) rows. |
| id | String | | Optional custom id to attach to root `Grid` element. |
| isScrolling | Boolean | | Override internal is-scrolling state tracking. This property is primarily intended for use with the WindowScroller component. |
| isScrollingOptOut | Boolean | | Prevents re-rendering of visible cells on scroll end. |
| noContentRenderer | Function | | Optional renderer to be rendered inside the grid when either `rowCount` or `columnCount` is empty: `(): PropTypes.node` |
| onSectionRendered | Function | | Callback invoked with information about the section of the Grid that was just rendered. This callback is only invoked when visible rows have changed: `({ columnOverscanStartIndex: number, columnOverscanStopIndex: number, columnStartIndex: number, columnStopIndex: number, rowOverscanStartIndex: number, rowOverscanStopIndex: number, rowStartIndex: number, rowStopIndex: number }): void` |
| onScroll | Function | | Callback invoked whenever the scroll offset changes within the inner scrollable region: `({ clientHeight: number, clientWidth: number, scrollHeight: number, scrollLeft: number, scrollTop: number, scrollWidth: number }): void` |
Expand Down
46 changes: 46 additions & 0 deletions source/Grid/Grid.jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -1905,6 +1905,52 @@ describe('Grid', () => {
expect(cellRendererCalls.length).not.toEqual(0);
});

it('should not clear cache if :isScrollingOptOut is true', () => {
const cellRendererCalls = [];
function cellRenderer({columnIndex, key, rowIndex, style}) {
cellRendererCalls.push({columnIndex, rowIndex});
return defaultCellRenderer({columnIndex, key, rowIndex, style});
}
const props = {
cellRenderer,
columnWidth: 100,
height: 40,
rowHeight: 20,
scrollTop: 0,
width: 100,
isScrollingOptOut: true,
};

render(getMarkup(props));
render(getMarkup(props));
expect(cellRendererCalls).toEqual([
{columnIndex: 0, rowIndex: 0},
{columnIndex: 0, rowIndex: 1},
]);

cellRendererCalls.splice(0);

render(
getMarkup({
...props,
isScrolling: false,
}),
);

// Visible cells are cached
expect(cellRendererCalls.length).toEqual(0);

render(
getMarkup({
...props,
isScrolling: true,
}),
);

// Only cleared non-visible cells
expect(cellRendererCalls.length).toEqual(0);
});

it('should support a custom :scrollingResetTimeInterval prop', async done => {
const cellRendererCalls = [];
const scrollingResetTimeInterval =
Expand Down
17 changes: 17 additions & 0 deletions source/Grid/Grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ type Props = {
*/
isScrolling?: boolean,

/**
* Opt-out of isScrolling param passed to cellRangeRenderer.
* To avoid the extra render when scroll stops.
*/
isScrollingOptOut: boolean,

/** Optional renderer to be used in place of rows when either :rowCount or :columnCount is 0. */
noContentRenderer: NoContentRenderer,

Expand Down Expand Up @@ -276,6 +282,7 @@ class Grid extends React.PureComponent<Props, State> {
scrollToRow: -1,
style: {},
tabIndex: 0,
isScrollingOptOut: false,
};

// Invokes onSectionRendered callback only when start/stop row or column indices change
Expand Down Expand Up @@ -1078,6 +1085,7 @@ class Grid extends React.PureComponent<Props, State> {
overscanRowCount,
rowCount,
width,
isScrollingOptOut,
} = props;

const {
Expand Down Expand Up @@ -1214,6 +1222,7 @@ class Grid extends React.PureComponent<Props, State> {
deferredMeasurementCache,
horizontalOffsetAdjustment,
isScrolling,
isScrollingOptOut,
parent: this,
rowSizeAndPositionManager: instanceProps.rowSizeAndPositionManager,
rowStartIndex,
Expand Down Expand Up @@ -1546,11 +1555,15 @@ class Grid extends React.PureComponent<Props, State> {

_resetStyleCache() {
const styleCache = this._styleCache;
const cellCache = this._cellCache;
const {isScrollingOptOut} = this.props;

// Reset cell and style caches once scrolling stops.
// This makes Grid simpler to use (since cells commonly change).
// And it keeps the caches from growing too large.
// Performance is most sensitive when a user is scrolling.
// Don't clear visible cells from cellCache if isScrollingOptOut is specified.
// This keeps the cellCache to a resonable size.
this._cellCache = {};
this._styleCache = {};

Expand All @@ -1567,6 +1580,10 @@ class Grid extends React.PureComponent<Props, State> {
) {
let key = `${rowIndex}-${columnIndex}`;
this._styleCache[key] = styleCache[key];

if (isScrollingOptOut) {
this._cellCache[key] = cellCache[key];
}
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion source/Grid/defaultCellRangeRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default function defaultCellRangeRenderer({
deferredMeasurementCache,
horizontalOffsetAdjustment,
isScrolling,
isScrollingOptOut,
parent, // Grid (or List or Table)
rowSizeAndPositionManager,
rowStartIndex,
Expand Down Expand Up @@ -109,8 +110,11 @@ export default function defaultCellRangeRenderer({
// However if we are scaling scroll positions and sizes, we should also avoid caching.
// This is because the offset changes slightly as scroll position changes and caching leads to stale values.
// For more info refer to issue #395
//
// If isScrollingOptOut is specified, we always cache cells.
// For more info refer to issue #1028
if (
isScrolling &&
(isScrollingOptOut || isScrolling) &&
!horizontalOffsetAdjustment &&
!verticalOffsetAdjustment
) {
Expand Down
1 change: 1 addition & 0 deletions source/Grid/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export type CellRangeRendererParams = {
deferredMeasurementCache?: Object,
horizontalOffsetAdjustment: number,
isScrolling: boolean,
isScrollingOptOut: boolean,
parent: Object,
rowSizeAndPositionManager: ScalingCellSizeAndPositionManager,
rowStartIndex: number,
Expand Down

0 comments on commit 438b567

Please sign in to comment.