From 5cc2ccb328bc37709264ec61f2a09502b6e9eaa5 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 15 Jan 2017 11:02:33 -0800 Subject: [PATCH] Merged master, resolved conflicts, tweaked styling, etc --- .../WindowScroller/WindowScroller.example.js | 15 +- source/WindowScroller/WindowScroller.js | 133 ++++++++---------- source/WindowScroller/WindowScroller.test.js | 16 +-- source/WindowScroller/utils/dimensions.js | 34 +++-- source/WindowScroller/utils/onScroll.js | 24 ++-- 5 files changed, 112 insertions(+), 110 deletions(-) diff --git a/source/WindowScroller/WindowScroller.example.js b/source/WindowScroller/WindowScroller.example.js index 60f8082fb..9f3af39a7 100644 --- a/source/WindowScroller/WindowScroller.example.js +++ b/source/WindowScroller/WindowScroller.example.js @@ -25,9 +25,8 @@ export default class WindowScrollerExample extends Component { } this._hideHeader = this._hideHeader.bind(this) - this._onCheckboxChange = this._onCheckboxChange.bind(this) this._rowRenderer = this._rowRenderer.bind(this) - this._setRef = this._setRef.bind(this) + this._onCheckboxChange = this._onCheckboxChange.bind(this) } render () { @@ -72,7 +71,9 @@ export default class WindowScrollerExample extends Component {
{ + this._windowScroller = ref + }} scrollElement={isScrollingCustomElement ? customElement : null} > {({ height, isScrolling, scrollTop }) => ( @@ -112,10 +113,6 @@ export default class WindowScrollerExample extends Component { }) } - _onCheckboxChange (event) { - this.context.setScrollingCustomElement(event.target.checked) - } - _rowRenderer ({ index, isScrolling, isVisible, key, style }) { const { list } = this.context const row = list.get(index) @@ -135,7 +132,7 @@ export default class WindowScrollerExample extends Component { ) } - _setRef (ref) { - this._windowScroller = ref + _onCheckboxChange (event) { + this.context.setScrollingCustomElement(event.target.checked) } } diff --git a/source/WindowScroller/WindowScroller.js b/source/WindowScroller/WindowScroller.js index 91aab4462..6219b7b3d 100644 --- a/source/WindowScroller/WindowScroller.js +++ b/source/WindowScroller/WindowScroller.js @@ -3,25 +3,25 @@ import { Component, PropTypes } from 'react' import ReactDOM from 'react-dom' import shallowCompare from 'react-addons-shallow-compare' import { registerScrollListener, unregisterScrollListener } from './utils/onScroll' -import { getVerticalScroll, getPositionFromTop, getHeight } from './utils/dimensions' +import { getHeight, getPositionFromTop, getScrollTop } from './utils/dimensions' export default class WindowScroller extends Component { static propTypes = { /** * Function responsible for rendering children. * This function should implement the following signature: - * ({ height, scrollTop }) => PropTypes.element + * ({ height, isScrolling, scrollTop }) => PropTypes.element */ children: PropTypes.func.isRequired, - /** Element to attach scroll event listeners. Defaults to window. */ - scrollElement: PropTypes.any, - /** Callback to be invoked on-resize: ({ height }) */ onResize: PropTypes.func.isRequired, /** Callback to be invoked on-scroll: ({ scrollTop }) */ - onScroll: PropTypes.func.isRequired + onScroll: PropTypes.func.isRequired, + + /** Element to attach scroll event listeners. Defaults to window. */ + scrollElement: PropTypes.any }; static defaultProps = { @@ -32,68 +32,74 @@ export default class WindowScroller extends Component { constructor (props) { super(props) - const height = typeof this.scrollElement !== 'undefined' - ? getHeight(this.scrollElement) + // Handle server-side rendering case + const height = typeof window !== 'undefined' + ? getHeight(props.scrollElement || window) : 0 this.state = { - isScrolling: false, height, + isScrolling: false, scrollTop: 0 } - this._onScrollWindow = this._onScrollWindow.bind(this) - this._onResizeWindow = this._onResizeWindow.bind(this) - this._enablePointerEventsAfterDelayCallback = this._enablePointerEventsAfterDelayCallback.bind(this) + this._onResize = this._onResize.bind(this) + this.__handleWindowScrollEvent = this.__handleWindowScrollEvent.bind(this) + this.__resetIsScrolling = this.__resetIsScrolling.bind(this) } - // Can’t really use defaultProps for `window` without breaking server-side rendering + // Can’t use defaultProps for scrollElement without breaking server-side rendering get scrollElement () { return this.props.scrollElement || window } - updatePosition () { - // Subtract documentElement top to handle edge-case where a user is navigating back (history) from an already-scrolled bage. - // In this case the body's top position will be a negative number and this element's top will be increased (by that amount). - this._positionFromTop = - ReactDOM.findDOMNode(this).getBoundingClientRect().top - - document.documentElement.getBoundingClientRect().top - } - - componentDidMount () { + updatePosition (scrollElement) { + const { onResize } = this.props const { height } = this.state - this.updatePosition() + scrollElement = scrollElement || this.props.scrollElement || window + this._positionFromTop = getPositionFromTop( + ReactDOM.findDOMNode(this), + scrollElement + ) - const scrollElementHeight = getHeight(this.scrollElement) - - if (height !== scrollElementHeight) { + const newHeight = getHeight(scrollElement) + if (height !== newHeight) { this.setState({ - height: scrollElementHeight + height: newHeight + }) + onResize({ + height: newHeight }) } + } + + componentDidMount () { + const scrollElement = this.props.scrollElement || window + + this.updatePosition(scrollElement) - registerScrollListener(this) + registerScrollListener(this, scrollElement) - window.addEventListener('resize', this._onResizeWindow, false) + window.addEventListener('resize', this._onResize, false) } componentWillReceiveProps (nextProps) { - if (nextProps.scrollElement && nextProps.scrollElement !== this.scrollElement) { - this._updateDimensions(nextProps.scrollElement) - unregisterScrollListener(this, this.scrollElement) - registerScrollListener(this, nextProps.scrollElement) - } else if (!nextProps.scrollElement && this.scrollElement !== window) { - this._updateDimensions(window) - unregisterScrollListener(this, this.scrollElement) - registerScrollListener(this, window) + const scrollElement = this.props.scrollElement || window + const nextScrollElement = nextProps.scrollElement || window + + if (scrollElement !== nextScrollElement) { + this.updatePosition(nextScrollElement) + + unregisterScrollListener(this, scrollElement) + registerScrollListener(this, nextScrollElement) } } componentWillUnmount () { - unregisterScrollListener(this, this.scrollElement) + unregisterScrollListener(this, this.props.scrollElement || window) - window.removeEventListener('resize', this._onResizeWindow, false) + window.removeEventListener('resize', this._onResize, false) } render () { @@ -111,50 +117,31 @@ export default class WindowScroller extends Component { return shallowCompare(this, nextProps, nextState) } - _updateDimensions (scrollElement = this.scrollElement) { - const { height } = this.state - - this._positionFromTop = getPositionFromTop(ReactDOM.findDOMNode(this), scrollElement) - - const newHeight = getHeight(scrollElement) - - if (height !== newHeight) { - this.setState({ - height: newHeight - }) - } - } - - _enablePointerEventsAfterDelayCallback () { - this.setState({ - isScrolling: false - }) - } - - _onResizeWindow (event) { - const { onResize } = this.props - + _onResize (event) { this.updatePosition() - - const height = getHeight(this.scrollElement) - - this.setState({ height }) - - onResize({ height }) } - _onScrollWindow (event) { + // Referenced by utils/onScroll + __handleWindowScrollEvent (event) { const { onScroll } = this.props - const scrollY = getVerticalScroll(this.scrollElement) - - const scrollTop = Math.max(0, scrollY - this._positionFromTop) + const scrollElement = this.props.scrollElement || window + const scrollTop = Math.max(0, getScrollTop(scrollElement) - this._positionFromTop) this.setState({ isScrolling: true, scrollTop }) - onScroll({ scrollTop }) + onScroll({ + scrollTop + }) + } + + // Referenced by utils/onScroll + __resetIsScrolling () { + this.setState({ + isScrolling: false + }) } } diff --git a/source/WindowScroller/WindowScroller.test.js b/source/WindowScroller/WindowScroller.test.js index 89941b22d..cffc74059 100644 --- a/source/WindowScroller/WindowScroller.test.js +++ b/source/WindowScroller/WindowScroller.test.js @@ -55,6 +55,12 @@ describe('WindowScroller', () => { } } + // Set default window height and scroll position between tests + beforeEach(() => { + window.scrollY = 0 + window.innerHeight = 500 + }) + // Starts updating scrollTop only when the top position is reached it('should have correct top property to be defined on :_positionFromTop', () => { const component = render(getMarkup()) @@ -65,6 +71,8 @@ describe('WindowScroller', () => { // Test edge-case reported in bvaughn/react-virtualized/pull/346 it('should have correct top property to be defined on :_positionFromTop if documentElement is scrolled', () => { + render.unmount() + // Simulate scrolled documentElement document.documentElement.getBoundingClientRect = () => ({ top: -100 @@ -78,8 +86,6 @@ describe('WindowScroller', () => { }) it('inherits the window height and passes it to child component', () => { - // Set default window height - window.innerHeight = 500 const component = render(getMarkup()) const rendered = findDOMNode(component) @@ -175,9 +181,6 @@ describe('WindowScroller', () => { expect(onResizeCalls[0]).toEqual({ height: 1000 }) - - // Set default window height - window.innerHeight = 500 }) it('should update height when window resizes', () => { @@ -194,9 +197,6 @@ describe('WindowScroller', () => { expect(component.state.height).toEqual(window.innerHeight) expect(component.state.height).toEqual(1000) expect(rendered.textContent).toContain('height:1000') - - // Set default window height - window.innerHeight = 500 }) }) diff --git a/source/WindowScroller/utils/dimensions.js b/source/WindowScroller/utils/dimensions.js index 9c7b90950..539caed94 100644 --- a/source/WindowScroller/utils/dimensions.js +++ b/source/WindowScroller/utils/dimensions.js @@ -1,13 +1,3 @@ -/** - * Gets the vertical scroll amount of the element, accounting for IE compatibility - * and API differences between `window` and other DOM elements. - */ -export function getVerticalScroll (element) { - return element === window - ? (('scrollY' in window) ? window.scrollY : document.documentElement.scrollTop) - : element.scrollTop -} - /** * Gets the height of the element, accounting for API differences between * `window` and other DOM elements. @@ -25,6 +15,26 @@ export function getHeight (element) { * In this case the body’s top position will be a negative number and this element’s top will be increased (by that amount). */ export function getPositionFromTop (element, container) { - const containerElement = container === window ? document.documentElement : container - return element.getBoundingClientRect().top + getVerticalScroll(container) - containerElement.getBoundingClientRect().top + const containerElement = container === window + ? document.documentElement + : container + return ( + element.getBoundingClientRect().top + + getScrollTop(container) - + containerElement.getBoundingClientRect().top + ) +} + +/** + * Gets the vertical scroll amount of the element, accounting for IE compatibility + * and API differences between `window` and other DOM elements. + */ +export function getScrollTop (element) { + if (element === window) { + return ('scrollY' in window) + ? window.scrollY + : document.documentElement.scrollTop + } else { + return element.scrollTop + } } diff --git a/source/WindowScroller/utils/onScroll.js b/source/WindowScroller/utils/onScroll.js index 985f7968b..73071d2d9 100644 --- a/source/WindowScroller/utils/onScroll.js +++ b/source/WindowScroller/utils/onScroll.js @@ -20,7 +20,9 @@ function enablePointerEventsIfDisabled () { function enablePointerEventsAfterDelayCallback () { enablePointerEventsIfDisabled() - mountedInstances.forEach(component => component._enablePointerEventsAfterDelayCallback()) + mountedInstances.forEach( + instance => instance.__resetIsScrolling() + ) } function enablePointerEventsAfterDelay () { @@ -41,22 +43,28 @@ function onScrollWindow (event) { document.body.style.pointerEvents = 'none' } enablePointerEventsAfterDelay() - mountedInstances.forEach(component => { - if (component.scrollElement === event.currentTarget) { - component._onScrollWindow(event) + mountedInstances.forEach(instance => { + if (instance.scrollElement === event.currentTarget) { + instance.__handleWindowScrollEvent(event) } }) } -export function registerScrollListener (component, element = window) { - if (!mountedInstances.some(c => c.scrollElement === element)) { +export function registerScrollListener (component, element) { + if ( + !mountedInstances.some( + instance => instance.scrollElement === element + ) + ) { element.addEventListener('scroll', onScrollWindow) } mountedInstances.push(component) } -export function unregisterScrollListener (component, element = window) { - mountedInstances = mountedInstances.filter(c => (c !== component)) +export function unregisterScrollListener (component, element) { + mountedInstances = mountedInstances.filter( + instance => instance !== component + ) if (!mountedInstances.length) { element.removeEventListener('scroll', onScrollWindow) if (disablePointerEventsTimeoutId) {