Merged master, resolved conflicts, tweaked styling, etc

bvaughn · Jan 15, 2017 · 5cc2ccb · 5cc2ccb
1 parent a900a6e
commit 5cc2ccb
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 110 deletions.
diff --git 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 {
 
         <div className={styles.WindowScrollerWrapper}>
           <WindowScroller
-            ref={this._setRef}
+            ref={(ref) => {
+              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
@@ -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
@@ -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
@@ -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
@@ -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) {