From 9aaf7f70d82669ddc0c7545e53922cf2ce2b3069 Mon Sep 17 00:00:00 2001 From: Wong Date: Tue, 21 Mar 2017 16:02:41 -0700 Subject: [PATCH 1/2] Allow syncing of scrollLeft and scrollTop from props MultiGrid manages scrollLeft and scrollTop through state and ignored them through props. Now syncs state if given props --- source/MultiGrid/MultiGrid.jest.js | 46 ++++++++++++++++++++++++++++++ source/MultiGrid/MultiGrid.js | 34 +++++++++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/source/MultiGrid/MultiGrid.jest.js b/source/MultiGrid/MultiGrid.jest.js index 5e426b598..623bc1e96 100644 --- a/source/MultiGrid/MultiGrid.jest.js +++ b/source/MultiGrid/MultiGrid.jest.js @@ -300,4 +300,50 @@ describe('MultiGrid', () => { expect(bottomRightGrid.style.backgroundColor).toEqual('red') }) }) + describe('scrollTop and scrollLeft', () => { + it('should adjust :scrollLeft for top-right and main grids when scrollLeft is used', () => { + const rendered = findDOMNode(render(getMarkup({ + columnWidth: 50, + fixedColumnCount: 2, + scrollLeft: 850 + }))) + const grids = rendered.querySelectorAll('.ReactVirtualized__Grid') + const topRightGrid = grids[1] + const bottomRightGrid = grids[3] + expect(topRightGrid.scrollLeft).toEqual(850) + expect(bottomRightGrid.scrollLeft).toEqual(850) + }) + + it('should adjust :scrollTop for bottom-left and main grids when scrollTop is used', () => { + const rendered = findDOMNode(render(getMarkup({ + columnWidth: 50, + fixedColumnCount: 2, + scrollTop: 500 + }))) + const grids = rendered.querySelectorAll('.ReactVirtualized__Grid') + const bottomLeftGrid = grids[2] + const bottomRightGrid = grids[3] + expect(bottomLeftGrid.scrollTop).toEqual(500) + expect(bottomRightGrid.scrollTop).toEqual(500) + }) + + it('should adjust :scrollTop and :scrollLeft when scrollTop and scrollLeft change', () => { + render(getMarkup({ + columnWidth: 50, + fixedColumnCount: 2 + })) + const rendered = findDOMNode(render(getMarkup({ + scrollTop: 750, + scrollLeft: 900 + }))) + const grids = rendered.querySelectorAll('.ReactVirtualized__Grid') + const topRightGrid = grids[1] + const bottomLeftGrid = grids[2] + const bottomRightGrid = grids[3] + expect(topRightGrid.scrollLeft).toEqual(900) + expect(bottomRightGrid.scrollLeft).toEqual(900) + expect(bottomLeftGrid.scrollTop).toEqual(750) + expect(bottomRightGrid.scrollTop).toEqual(750) + }) + }) }) diff --git a/source/MultiGrid/MultiGrid.js b/source/MultiGrid/MultiGrid.js index bcfeb554b..f1959589a 100644 --- a/source/MultiGrid/MultiGrid.js +++ b/source/MultiGrid/MultiGrid.js @@ -116,6 +116,21 @@ export default class MultiGrid extends PureComponent { } componentDidMount () { + const { scrollLeft, scrollTop } = this.props + + if (scrollLeft >= 0 || scrollTop >= 0) { + const newState = {} + + if (scrollLeft >= 0) { + newState.scrollLeft = scrollLeft + } + + if (scrollTop >= 0) { + newState.scrollTop = scrollTop + } + + this.setState(newState) + } this._handleInvalidatedGridSize() } @@ -144,6 +159,23 @@ export default class MultiGrid extends PureComponent { this._topGridHeight = null } + if ( + nextProps.scrollLeft !== this.state.scrollLeft || + nextProps.scrollTop !== this.state.scrollTop + ) { + const newState = {} + + if (nextProps.scrollLeft != null) { + newState.scrollLeft = nextProps.scrollLeft + } + + if (nextProps.scrollTop != null) { + newState.scrollTop = nextProps.scrollTop + } + + this.setState(newState) + } + this._maybeCalculateCachedStyles(this.props, nextProps, this.state, nextState) } @@ -165,7 +197,7 @@ export default class MultiGrid extends PureComponent { return null } - // scrollTop and scrollToRow props are explicitly filtered out and ignored + // scrollTop and scrollLeft props are explicitly filtered out and ignored const { scrollLeft, From 4dfd8650adacbca7f58ce27c1b77312b8d17e8a6 Mon Sep 17 00:00:00 2001 From: Wong Date: Wed, 22 Mar 2017 11:45:32 -0700 Subject: [PATCH 2/2] Change to compare nextProps with this.props instead of this.state Fix to compare nextProps with this.props to not interfere with user scrolling. Modify comparisons to account for negative --- source/MultiGrid/MultiGrid.jest.js | 5 +---- source/MultiGrid/MultiGrid.js | 20 +++++++++++++------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/source/MultiGrid/MultiGrid.jest.js b/source/MultiGrid/MultiGrid.jest.js index 623bc1e96..37023dbd3 100644 --- a/source/MultiGrid/MultiGrid.jest.js +++ b/source/MultiGrid/MultiGrid.jest.js @@ -328,10 +328,7 @@ describe('MultiGrid', () => { }) it('should adjust :scrollTop and :scrollLeft when scrollTop and scrollLeft change', () => { - render(getMarkup({ - columnWidth: 50, - fixedColumnCount: 2 - })) + render(getMarkup()) const rendered = findDOMNode(render(getMarkup({ scrollTop: 750, scrollLeft: 900 diff --git a/source/MultiGrid/MultiGrid.js b/source/MultiGrid/MultiGrid.js index f1959589a..9d4c469f2 100644 --- a/source/MultiGrid/MultiGrid.js +++ b/source/MultiGrid/MultiGrid.js @@ -118,14 +118,14 @@ export default class MultiGrid extends PureComponent { componentDidMount () { const { scrollLeft, scrollTop } = this.props - if (scrollLeft >= 0 || scrollTop >= 0) { + if (scrollLeft > 0 || scrollTop > 0) { const newState = {} - if (scrollLeft >= 0) { + if (scrollLeft > 0) { newState.scrollLeft = scrollLeft } - if (scrollTop >= 0) { + if (scrollTop > 0) { newState.scrollTop = scrollTop } @@ -160,16 +160,22 @@ export default class MultiGrid extends PureComponent { } if ( - nextProps.scrollLeft !== this.state.scrollLeft || - nextProps.scrollTop !== this.state.scrollTop + nextProps.scrollLeft !== this.props.scrollLeft || + nextProps.scrollTop !== this.props.scrollTop ) { const newState = {} - if (nextProps.scrollLeft != null) { + if ( + nextProps.scrollLeft != null && + nextProps.scrollLeft >= 0 + ) { newState.scrollLeft = nextProps.scrollLeft } - if (nextProps.scrollTop != null) { + if ( + nextProps.scrollTop != null && + nextProps.scrollTop >= 0 + ) { newState.scrollTop = nextProps.scrollTop }