Skip to content

Commit

Permalink
AutoSizer no longer re-renders nor calls onResize callback unless wid…
Browse files Browse the repository at this point in the history
…th and/or height have changed (depending on which properties are being watched).
  • Loading branch information
Brian Vaughn committed Mar 26, 2017
1 parent 978ec14 commit 049c039
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Changelog
* ✨ Replaced inline `require` statement with header `import` in `Grid` for better integration with the Rollup module bundler. ([@odogono](https://github.com/odogono) - [#617](https://github.com/bvaughn/react-virtualized/pull/617))
* 🐛 Improved guard for edge-case scrolling issue with rubberband scrolling in iOS. ([@dtoddtarsi](https://github.com/offsky) - [#616](https://github.com/bvaughn/react-virtualized/pull/616))
* ✨ Replaced `getBoundingClientRect()` with slightly faster `offsetWidth` and `offsetHeight` inside of `AutoSizer`.
*`AutoSizer` no longer re-renders nor calls `onResize` callback unless `width` and/or `height` have changed (depending on which properties are being watched).

##### 9.3.0
* 🎉 Added `resetLoadMoreRowsCache` method to `InfiniteLoader` to reset any cached data about loaded rows. This method should be called if any/all loaded data needs to be refetched (eg a filtered list where the search criteria changes). ([#612](https://github.com/bvaughn/react-virtualized/issues/612))
Expand Down
71 changes: 69 additions & 2 deletions source/AutoSizer/AutoSizer.jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { findDOMNode } from 'react-dom'
import { render } from '../TestUtils'
import AutoSizer from './AutoSizer'

function ChildComponent ({ height, width, foo, bar }) {
function DefaultChildComponent ({ height, width, foo, bar }) {
return (
<div>{`width:${width}, height:${height}, foo:${foo}, bar:${bar}`}</div>
)
Expand All @@ -14,10 +14,12 @@ function ChildComponent ({ height, width, foo, bar }) {
describe('AutoSizer', () => {
function getMarkup ({
bar = 123,
ChildComponent = DefaultChildComponent,
disableHeight = false,
disableWidth = false,
foo = 456,
height = 100,
onResize,
paddingBottom = 0,
paddingLeft = 0,
paddingRight = 0,
Expand All @@ -38,7 +40,11 @@ describe('AutoSizer', () => {

return (
<div style={style}>
<AutoSizer>
<AutoSizer
disableHeight={disableHeight}
disableWidth={disableWidth}
onResize={onResize}
>
{({ height, width }) => (
<ChildComponent
width={disableWidth ? undefined : width}
Expand Down Expand Up @@ -124,4 +130,65 @@ describe('AutoSizer', () => {
expect(rendered.textContent).toContain('width:300')
done()
})

describe('onResize and (re)render', () => {
it('should trigger when size changes', async (done) => {
const onResize = jest.fn()
const ChildComponent = jest.fn().mockImplementation(DefaultChildComponent)
const rendered = findDOMNode(render(getMarkup({
ChildComponent,
height: 100,
onResize,
width: 200
})))
ChildComponent.mockClear() // TODO Improve initial check in version 10; see AutoSizer render()
expect(onResize).toHaveBeenCalledTimes(1)
await simulateResize({ element: rendered, height: 400, width: 300 })
expect(ChildComponent).toHaveBeenCalledTimes(1)
expect(onResize).toHaveBeenCalledTimes(2)
done()
})

it('should only trigger when height changes for disableWidth == true', async (done) => {
const onResize = jest.fn()
const ChildComponent = jest.fn().mockImplementation(DefaultChildComponent)
const rendered = findDOMNode(render(getMarkup({
ChildComponent,
disableWidth: true,
height: 100,
onResize,
width: 200
})))
ChildComponent.mockClear() // TODO Improve initial check in version 10; see AutoSizer render()
expect(onResize).toHaveBeenCalledTimes(1)
await simulateResize({ element: rendered, height: 100, width: 300 })
expect(ChildComponent).toHaveBeenCalledTimes(0)
expect(onResize).toHaveBeenCalledTimes(1)
await simulateResize({ element: rendered, height: 200, width: 300 })
expect(ChildComponent).toHaveBeenCalledTimes(1)
expect(onResize).toHaveBeenCalledTimes(2)
done()
})

it('should only trigger when width changes for disableHeight == true', async (done) => {
const onResize = jest.fn()
const ChildComponent = jest.fn().mockImplementation(DefaultChildComponent)
const rendered = findDOMNode(render(getMarkup({
ChildComponent,
disableHeight: true,
height: 100,
onResize,
width: 200
})))
ChildComponent.mockClear() // TODO Improve initial check in version 10; see AutoSizer render()
expect(onResize).toHaveBeenCalledTimes(1)
await simulateResize({ element: rendered, height: 200, width: 200 })
expect(ChildComponent).toHaveBeenCalledTimes(0)
expect(onResize).toHaveBeenCalledTimes(1)
await simulateResize({ element: rendered, height: 200, width: 300 })
expect(ChildComponent).toHaveBeenCalledTimes(1)
expect(onResize).toHaveBeenCalledTimes(2)
done()
})
})
})
40 changes: 33 additions & 7 deletions source/AutoSizer/AutoSizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,18 @@ export default class AutoSizer extends PureComponent {
outerStyle.width = 0
}

/**
* TODO: Avoid rendering children before the initial measurements have been collected.
* At best this would just be wasting cycles.
* Add this check into version 10 though as it could break too many ref callbacks in version 9.
if (
height !== 0 &&
width !== 0
) {
child = children({ height, width })
}
*/

return (
<div
ref={this._setRef}
Expand All @@ -90,7 +102,11 @@ export default class AutoSizer extends PureComponent {
}

_onResize () {
const { onResize } = this.props
const {
disableHeight,
disableWidth,
onResize
} = this.props

// Guard against AutoSizer component being removed from the DOM immediately after being added.
// This can result in invalid style values which can result in NaN values if we don't handle them.
Expand All @@ -105,12 +121,22 @@ export default class AutoSizer extends PureComponent {
const paddingTop = parseInt(style.paddingTop, 10) || 0
const paddingBottom = parseInt(style.paddingBottom, 10) || 0

this.setState({
height: height - paddingTop - paddingBottom,
width: width - paddingLeft - paddingRight
})

onResize({ height, width })
const newHeight = height - paddingTop - paddingBottom
const newWidth = width - paddingLeft - paddingRight

if (
!disableHeight &&
this.state.height !== newHeight ||
!disableWidth &&
this.state.width !== newWidth
) {
this.setState({
height: height - paddingTop - paddingBottom,
width: width - paddingLeft - paddingRight
})

onResize({ height, width })
}
}

_setRef (autoSizer) {
Expand Down

0 comments on commit 049c039

Please sign in to comment.