Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom element option to WindowScroller #481

Closed
wants to merge 43 commits into from
Closed
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
b7ac151
Add util for abstracting API differences between window and other con…
andrewbranch Nov 22, 2016
754cb2a
Modify onScroll utils to accept arbitrary scroll element
andrewbranch Nov 22, 2016
1981162
Allow WindowScroller to receive custom scrollElement prop
andrewbranch Nov 22, 2016
b9399dd
Fix copy/paste error
andrewbranch Nov 22, 2016
a0df5f8
Look at what element scroll listener was attached to, not initiator
andrewbranch Nov 22, 2016
57efa5f
Update demo to include custom scroll element option
andrewbranch Nov 22, 2016
27f2b69
Fix case where prop changes from custom element to nothing
andrewbranch Nov 22, 2016
e06f19e
Make sure my custom element is scrolling, not ContentBox
andrewbranch Nov 22, 2016
ca60f00
Auto fix linter errors
andrewbranch Nov 22, 2016
8da79fb
Fix linter errors / things that will break server-side
andrewbranch Nov 22, 2016
6fd41a0
Update pointerEvents assertions
andrewbranch Nov 22, 2016
18612a6
Fix failing "restore pointer events on unmount" test
andrewbranch Nov 22, 2016
8ff9cfc
Add missing _updateDimensions call
andrewbranch Nov 22, 2016
dc7a7ac
Replace PropTypes.object with any for HTMLElement props
andrewbranch Nov 22, 2016
2a90fbf
Add autoprefixer-loader (for now?)
andrewbranch Nov 22, 2016
36e14b6
Prevent internal scroll position overwriting scroll position from props
Nov 23, 2016
a8f039f
Updated cellRenderer callback parameters to also match List rowRenderer
romulof Nov 29, 2016
42493ce
Update CellMeasurer documentation
romulof Nov 29, 2016
2ddaed2
Fixed code-style issue
romulof Nov 29, 2016
919e342
Updated tests for new CellMeasurer render :index param
Nov 30, 2016
197a5c3
Removed outdated warning about using CellMeasurer with List now that …
Nov 30, 2016
c7bb0b7
Updated CHANGELOG in advance of upcoming release
Nov 30, 2016
cc83019
Merge pull request #482 from lic-nz/fix-scroll-position-from-props
mbrevda Dec 1, 2016
643d8a2
Minor formatting tweaks
Dec 1, 2016
05570f8
Updated CHANGELOG for upcoming release
Dec 1, 2016
2a721a7
Prepping 8.6.0 release
Dec 1, 2016
fe11056
Revert "Add autoprefixer-loader (for now?)"
andrewbranch Dec 1, 2016
78781c7
Revert "Modify onScroll utils to accept arbitrary scroll element"
andrewbranch Dec 1, 2016
335d6df
onScroll take 2: attach listener to arbitrary scroll element, but onl…
andrewbranch Dec 1, 2016
15c9fde
Revert "Update pointerEvents assertions"
andrewbranch Dec 1, 2016
bb804f2
Fix clearTimeout never being called
andrewbranch Dec 1, 2016
29cec61
Update CellSizeCache interface for better perfomance (#486)
arusakov Dec 1, 2016
5c2b141
Merge branch 'arusakov-486_cell_size_cache_optimisation'
Dec 2, 2016
d44dde1
Prepping for 8.6.1 release
Dec 2, 2016
188d691
Fix checkbox bug/typo
andrewbranch Dec 2, 2016
43691da
Fix demo bug: make sure container does not shrink when Grid is not ye…
andrewbranch Dec 2, 2016
c86015e
Fix bug where new height was calculated with old element (never changed)
andrewbranch Dec 2, 2016
2e91ec7
Added to to handle case when header items change or resize. also b…
Dec 3, 2016
32454e0
Reverted part of the change introduced in version 8.6.0 that changed …
Dec 3, 2016
36a54f9
Resolved conflicts
Dec 3, 2016
87fe0f1
Fix positionFromTop calculation
andrewbranch Dec 15, 2016
f2199ac
Add scroll handlers to distinct scrollElements
andrewbranch Dec 22, 2016
7c78188
Avoid some unnecessary updates to some WindowScrollers
andrewbranch Jan 11, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
Changelog
------------

##### 8.7.1
Reverted part of the change introduced in version 8.6.0 that changed the behavior regarding controlled/uncontrolled `scrollTop` and `scrollLeft` props and `Grid` in a way that was not backwards compatible. (See issue #490 for more.)

##### 8.7.0
Added `updatePosition` to `WindowScroller` to handle case when header items change or resize. `WindowScroller` also better handles window resize events.

##### 8.6.1
Updated `CellSizeCache` interface for the better perfomance by removing `has` methods, reducing a double hashtable lookup to a single lookup. Special thanks to @arusakov for this contribution!

##### 8.6.0
`CellMeasurer` passes `index` param (duplicate of `rowIndex`) in order to more easily integrate with `List` by default.

`Grid` now better handles a mix of controlled and uncontrolled scroll offsets. Previously changes to one offset would wipe out the other causing cells to disappear (see PR #482). This is an edge-case bug and only impacted an uncommon usecase for `Grid`. As such this change is expected to only impact only a small percetange of users.

##### 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.
Expand Down
40 changes: 3 additions & 37 deletions docs/CellMeasurer.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ This is an advanced component and has some limitations and performance considera
### Prop Types
| Property | Type | Required? | Description |
|:---|:---|:---:|:---|
| cellRenderer | Function | ✓ | Renders a cell given its indices. `({ columnIndex: number, rowIndex: number }): PropTypes.node` |
| cellRenderer | Function | ✓ | Renders a cell given its indices. `({ columnIndex: number, rowIndex: number, index: number }): PropTypes.node`.<br/>**NOTE**: `index` is just an alias to `rowIndex` |
| cellSizeCache | Object | | Optional, custom caching strategy for cell sizes. Learn more [here](#cellsizecache). |
| children | Function | ✓ | Function responsible for rendering a virtualized component; `({ getColumnWidth: Function, getRowHeight: Function, resetMeasurements: Function }) => PropTypes.element` |
| columnCount | number | ✓ | Number of columns in the `Grid`; in order to measure a row's height, all of that row's columns must be rendered. |
Expand Down Expand Up @@ -43,10 +43,8 @@ class CellSizeCache {
clearAllRowHeights (): void;
clearColumnWidth (index: number): void;
clearRowHeight (index: number): void;
getColumnWidth (index: number): number;
getRowHeight (index: number): number;
hasColumnWidth (index: number): boolean;
hasRowHeight (index: number): boolean;
getColumnWidth (index: number): number | undefined | null;
getRowHeight (index: number): number | undefined | null;
setColumnWidth (index: number, width: number): void;
setRowHeight (index: number, height: number): void;
}
Expand Down Expand Up @@ -162,35 +160,3 @@ For this reason it may not be a good idea to use this HOC for `Grid`s containing

Since this component measures one cell at a time to determine it's width/height, it will likely be slow if a user skips many rows (or columns) at once by scrolling with a scrollbar or via a scroll-to-cell prop.
There is (unfortunately) no workaround for this performance limitation at the moment.

### Using `CellMeasurer` with `List`

This HOC is intended for use with a `Grid`.
That means it passes `cellRenderer` the same named index parameter used by `Grid` (`rowIndex`).
However the `rowRenderer` used by `List` expects a slightly different named index parameter (`index`).
To use `CellMeasurerer` with `List` then you need to provide an adapter method that converts the param names.
For example:

```jsx
import { CellMeasurer, List } from 'react-virtualized';

function renderList(listProps, cellMeasurerProps = Object.create(null)) {
return (
<CellMeasurer
{...cellMeasurerProps}
cellRenderer={
({ rowIndex, ...rest }) => listProps.rowRenderer({ index: rowIndex, ...rest })
}
columnCount={1}
rowCount={listProps.rowCount}
>
{({ getRowHeight }) => (
<List
{...listProps}
rowHeight={getRowHeight}
/>
)}
</CellMeasurer>
)
}
```
8 changes: 8 additions & 0 deletions docs/WindowScroller.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ This may change with a future release but for the time being this HOC is should
| onResize | Function | | Callback to be invoked on-resize; it is passed the following named parameters: `({ height: number })`. |
| onScroll | Function | | Callback to be invoked on-scroll; it is passed the following named parameters: `({ scrollTop: number })`. |

### Public Methods

##### updatePosition

Recalculates scroll position from the top of page.

This methoed is automatically triggered when the component mounts as well as when the browser resizes. It should be manually called if the page header (eg any items in the DOM "above" the `WindowScroller`) resizes or changes.

### Examples

```javascript
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"description": "React components for efficiently rendering large, scrollable lists and tabular data",
"author": "Brian Vaughn <[email protected]>",
"user": "bvaughn",
"version": "8.5.3",
"version": "8.7.1",
"homepage": "https://github.com/bvaughn/react-virtualized",
"main": "dist/commonjs/index.js",
"module": "dist/es/index.js",
Expand Down
11 changes: 7 additions & 4 deletions source/CellMeasurer/CellMeasurer.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ export default class CellMeasurer extends Component {
}

getColumnWidth ({ index }) {
if (this._cellSizeCache.hasColumnWidth(index)) {
return this._cellSizeCache.getColumnWidth(index)
const columnWidth = this._cellSizeCache.getColumnWidth(index)
if (columnWidth != null) {
return columnWidth
}

const { rowCount } = this.props
Expand All @@ -95,8 +96,9 @@ export default class CellMeasurer extends Component {
}

getRowHeight ({ index }) {
if (this._cellSizeCache.hasRowHeight(index)) {
return this._cellSizeCache.getRowHeight(index)
const rowHeight = this._cellSizeCache.getRowHeight(index)
if (rowHeight != null) {
return rowHeight
}

const { columnCount } = this.props
Expand Down Expand Up @@ -189,6 +191,7 @@ export default class CellMeasurer extends Component {

const rendered = cellRenderer({
columnIndex,
index: rowIndex, // Simplify List :rowRenderer use case
rowIndex
})

Expand Down
83 changes: 49 additions & 34 deletions source/CellMeasurer/CellMeasurer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('CellMeasurer', () => {
})
expect(cellRendererParams).toEqual([])
expect(getRowHeight({ index: 0 })).toEqual(75)
expect(cellRendererParams).toEqual([{ columnIndex: 0, rowIndex: 0 }])
expect(cellRendererParams).toEqual([{ columnIndex: 0, index: 0, rowIndex: 0 }])
expect(getColumnWidth({ index: 0 })).toEqual(100)

// For some reason this explicit unmount is necessary.
Expand All @@ -94,7 +94,7 @@ describe('CellMeasurer', () => {
})
expect(cellRendererParams).toEqual([])
expect(getColumnWidth({ index: 0 })).toEqual(125)
expect(cellRendererParams).toEqual([{ columnIndex: 0, rowIndex: 0 }])
expect(cellRendererParams).toEqual([{ columnIndex: 0, index: 0, rowIndex: 0 }])
expect(getRowHeight({ index: 0 })).toEqual(50)
})

Expand Down Expand Up @@ -136,6 +136,23 @@ describe('CellMeasurer', () => {
expect(getRowHeight({ index: 0 })).toEqual(50)
})

it('should support :rowRenderer via :index param for easier List integration', () => {
const {
cellRenderer,
cellRendererParams
} = createCellRenderer()
const { getColumnWidth } = renderHelper({
cellRenderer,
rowCount: 5,
rowHeight: 50
})
getColumnWidth({ index: 0 })
expect(cellRendererParams.length).toEqual(5)
for (let i = 0; i < 5; i++) {
expect(cellRendererParams[i].index).toEqual(i)
}
})

it('should cache cell measurements once a cell has been rendered', () => {
const {
cellRenderer,
Expand All @@ -149,15 +166,15 @@ describe('CellMeasurer', () => {
getRowHeight({ index: 0 })
getRowHeight({ index: 1 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 0, rowIndex: 1 }
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 0, index: 1, rowIndex: 1 }
])

getRowHeight({ index: 0 })
getRowHeight({ index: 1 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 0, rowIndex: 1 }
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 0, index: 1, rowIndex: 1 }
])
})

Expand All @@ -175,19 +192,19 @@ describe('CellMeasurer', () => {
getRowHeight({ index: 0 })
getRowHeight({ index: 1 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 0, rowIndex: 1 }
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 0, index: 1, rowIndex: 1 }
])

resetMeasurements()

getRowHeight({ index: 0 })
getRowHeight({ index: 1 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 0, rowIndex: 1 },
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 0, rowIndex: 1 }
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 0, index: 1, rowIndex: 1 },
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 0, index: 1, rowIndex: 1 }
])
})

Expand All @@ -205,18 +222,18 @@ describe('CellMeasurer', () => {
getColumnWidth({ index: 0 })
getColumnWidth({ index: 1 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 1, rowIndex: 0 }
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 1, index: 0, rowIndex: 0 }
])

resetMeasurementForColumn(0)

getColumnWidth({ index: 0 })
getColumnWidth({ index: 1 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 1, rowIndex: 0 },
{ columnIndex: 0, rowIndex: 0 }
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 1, index: 0, rowIndex: 0 },
{ columnIndex: 0, index: 0, rowIndex: 0 }
])
})

Expand All @@ -234,18 +251,18 @@ describe('CellMeasurer', () => {
getRowHeight({ index: 0 })
getRowHeight({ index: 1 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 0, rowIndex: 1 }
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 0, index: 1, rowIndex: 1 }
])

resetMeasurementForRow(0)

getRowHeight({ index: 0 })
getRowHeight({ index: 1 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 },
{ columnIndex: 0, rowIndex: 1 },
{ columnIndex: 0, rowIndex: 0 }
{ columnIndex: 0, index: 0, rowIndex: 0 },
{ columnIndex: 0, index: 1, rowIndex: 1 },
{ columnIndex: 0, index: 0, rowIndex: 0 }
])
})

Expand All @@ -267,19 +284,17 @@ describe('CellMeasurer', () => {
rowHeight: 50
})

expect(customCellSizeCache.hasColumnWidth(0)).toEqual(false)
expect(customCellSizeCache.getColumnWidth(0)).toEqual(undefined)
expect(cellRendererParams.length).toEqual(0)
expect(getColumnWidth({ index: 0 })).toEqual(200)
expect(customCellSizeCache.hasColumnWidth(0)).toEqual(true)
expect(customCellSizeCache.getColumnWidth(0)).toEqual(200)
expect(cellRendererParams.length).toEqual(2)
expect(getColumnWidth({ index: 0 })).toEqual(200)
expect(cellRendererParams.length).toEqual(2)

expect(customCellSizeCache.hasRowHeight(0)).toEqual(false)
expect(customCellSizeCache.getRowHeight(0)).toEqual(undefined)
expect(cellRendererParams.length).toEqual(2)
expect(getRowHeight({ index: 0 })).toEqual(50)
expect(customCellSizeCache.hasRowHeight(0)).toEqual(true)
expect(customCellSizeCache.getRowHeight(0)).toEqual(50)
expect(cellRendererParams.length).toEqual(7)
expect(getRowHeight({ index: 0 })).toEqual(50)
Expand All @@ -296,29 +311,29 @@ describe('CellMeasurer', () => {
columnCount: 5,
columnWidth: 200
})
expect(customCellSizeCacheA.hasColumnWidth(0)).toEqual(false)
expect(customCellSizeCacheA.getColumnWidth(0)).toEqual(undefined)
expect(getColumnWidthA({ index: 0 })).toEqual(200)
expect(customCellSizeCacheA.hasColumnWidth(0)).toEqual(true)
expect(customCellSizeCacheA.getColumnWidth(0)).toEqual(200)

const { getColumnWidth: getColumnWidthB } = renderHelper({
cellRenderer,
cellSizeCache: customCellSizeCacheA,
columnCount: 5,
columnWidth: 100
})
expect(customCellSizeCacheA.hasColumnWidth(0)).toEqual(true)
expect(customCellSizeCacheA.getColumnWidth(0)).toEqual(200)
expect(getColumnWidthB({ index: 0 })).toEqual(200)
expect(customCellSizeCacheA.hasColumnWidth(0)).toEqual(true)
expect(customCellSizeCacheA.getColumnWidth(0)).toEqual(200)

const { getColumnWidth: getColumnWidthC } = renderHelper({
cellRenderer,
cellSizeCache: customCellSizeCacheB,
columnCount: 5,
columnWidth: 50
})
expect(customCellSizeCacheB.hasColumnWidth(0)).toEqual(false)
expect(customCellSizeCacheB.getColumnWidth(0)).toEqual(undefined)
expect(getColumnWidthC({ index: 0 })).toEqual(50)
expect(customCellSizeCacheB.hasColumnWidth(0)).toEqual(true)
expect(customCellSizeCacheB.getColumnWidth(0)).toEqual(50)
})

it('should calculate row height just once when using the alternative uniform-size cell size cache', () => {
Expand All @@ -342,7 +357,7 @@ describe('CellMeasurer', () => {
const height2 = getRowHeight({ index: 1 })
const height3 = getRowHeight({ index: 0 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 }
{ columnIndex: 0, index: 0, rowIndex: 0 }
])

const expectedHeight = HEIGHTS[0]
Expand Down Expand Up @@ -373,7 +388,7 @@ describe('CellMeasurer', () => {
const width2 = getColumnWidth({ index: 1 })
const width3 = getColumnWidth({ index: 0 })
expect(cellRendererParams).toEqual([
{ columnIndex: 0, rowIndex: 0 }
{ columnIndex: 0, index: 0, rowIndex: 0 }
])

const expectedWidth = WIDTHS[0]
Expand Down
19 changes: 5 additions & 14 deletions source/CellMeasurer/defaultCellSizeCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export default class CellSizeCache {
this._uniformRowHeight = uniformRowHeight
this._uniformColumnWidth = uniformColumnWidth

this._cachedColumnWidth = undefined
this._cachedRowHeight = undefined

this._cachedColumnWidths = {}
this._cachedRowHeights = {}
}
Expand All @@ -37,30 +40,18 @@ export default class CellSizeCache {
delete this._cachedRowHeights[index]
}

getColumnWidth (index: number): number {
getColumnWidth (index: number): ?number {
return this._uniformColumnWidth
? this._cachedColumnWidth
: this._cachedColumnWidths[index]
}

getRowHeight (index: number): number {
getRowHeight (index: number): ?number {
return this._uniformRowHeight
? this._cachedRowHeight
: this._cachedRowHeights[index]
}

hasColumnWidth (index: number): boolean {
return this._uniformColumnWidth
? !!this._cachedColumnWidth
: !!this._cachedColumnWidths[index]
}

hasRowHeight (index: number): boolean {
return this._uniformRowHeight
? !!this._cachedRowHeight
: !!this._cachedRowHeights[index]
}

setColumnWidth (index: number, width: number) {
this._cachedColumnWidth = width
this._cachedColumnWidths[index] = width
Expand Down
Loading