Resolved edge-case bug that caused the bottom/right cells in a to be …

…partially overlapped by a scrollbar.

CHANGELOG.md

@@ -1,6 +1,10 @@
 Changelog
 ------------
 
+##### 7.1.1
+Resolved edge-case bug that caused the bottom/right cells in a `Grid` to be partially overlapped by a scrollbar.
+Thanks to @anjianshi for reporting this and collaborating on the fix!
+
 ##### 7.1.0
 Added `scrollToAlignment` property to `Collection`, `Grid`, `FlexTable`, and `VirtualScroll` to offer finer-grained control of how scrolled-to cells are aligned.
 Default behavior ("_auto_") remains unchanged- the least amount of scrolling will occur to ensure that the specified cell is visible.

package.json

@@ -3,7 +3,7 @@
   "description": "React components for efficiently rendering large, scrollable lists and tabular data",
   "author": "Brian Vaughn <brian.david.vaughn@gmail.com>",
   "user": "bvaughn",
-  "version": "7.1.0",
+  "version": "7.1.1",
   "homepage": "https://github.com/bvaughn/react-virtualized",
   "main": "dist/commonjs/index.js",
   "jsnext:main": "dist/es/index.js",

source/Grid/Grid.js

@@ -229,7 +229,13 @@ export default class Grid extends Component {
   componentDidMount () {
     const { scrollLeft, scrollToColumn, scrollTop, scrollToRow } = this.props
 
-    this._scrollbarSize = getScrollbarSize()
+    // If this component was first rendered server-side, scrollbar size will be undefined.
+    // In that event we need to remeasure.
+    if (!this._scrollbarSizeMeasured) {
+      this._scrollbarSize = getScrollbarSize()
+      this._scrollbarSizeMeasured = true
+      this.setState({})
+    }
 
     if (scrollLeft >= 0 || scrollTop >= 0) {
       this._setScrollPosition({ scrollLeft, scrollTop })
@@ -316,6 +322,18 @@ export default class Grid extends Component {
     this._invokeOnGridRenderedHelper()
   }
 
+  componentWillMount () {
+    // If this component is being rendered server-side, getScrollbarSize() will return undefined.
+    // We handle this case in componentDidMount()
+    this._scrollbarSize = getScrollbarSize()
+    if (this._scrollbarSize === undefined) {
+      this._scrollbarSizeMeasured = false
+      this._scrollbarSize = 0
+    } else {
+      this._scrollbarSizeMeasured = true
+    }
+  }
+
   componentWillUnmount () {
     if (this._disablePointerEventsTimeoutId) {
       clearTimeout(this._disablePointerEventsTimeoutId)
@@ -479,11 +497,14 @@ export default class Grid extends Component {
     // Force browser to hide scrollbars when we know they aren't necessary.
     // Otherwise once scrollbars appear they may not disappear again.
     // For more info see issue #116
-    if (totalColumnsWidth <= width) {
+    const verticalScrollBarSize = totalRowsHeight > height ? this._scrollbarSize : 0
+    const horizontalScrollBarSize = totalColumnsWidth > width ? this._scrollbarSize : 0
+
+    if (totalColumnsWidth + verticalScrollBarSize <= width) {
       gridStyle.overflowX = 'hidden'
     }
 
-    if (totalRowsHeight <= height) {
+    if (totalRowsHeight + horizontalScrollBarSize <= height) {
       gridStyle.overflowY = 'hidden'
     }
 

source/Grid/Grid.test.js

@@ -1,7 +1,8 @@
+import getScrollbarSize from 'dom-helpers/util/scrollbarSize'
 import React from 'react'
 import { findDOMNode } from 'react-dom'
-import { render } from '../TestUtils'
 import { Simulate } from 'react-addons-test-utils'
+import { render } from '../TestUtils'
 import Grid from './Grid'
 
 const NUM_ROWS = 100
@@ -119,23 +120,75 @@ describe('Grid', () => {
   })
 
   describe('shows and hides scrollbars based on rendered content', () => {
-    it('should set overflowX:hidden on scroll-container if columns fit within the available width', () => {
-      const rendered = findDOMNode(render(getMarkup({ columnCount: 4 })))
+    let scrollbarSize
+
+    beforeAll(() => {
+      scrollbarSize = getScrollbarSize()
+    })
+
+    it('should set overflowX:hidden if columns fit within the available width and y-axis has no scrollbar', () => {
+      const rendered = findDOMNode(render(getMarkup({
+        columnCount: 4,
+        rowCount: 5
+      })))
+      expect(rendered.style.overflowX).toEqual('hidden')
+    })
+
+    it('should set overflowX:hidden if columns and y-axis scrollbar fit within the available width', () => {
+      const rendered = findDOMNode(render(getMarkup({
+        columnCount: 4,
+        width: 200 + scrollbarSize
+      })))
       expect(rendered.style.overflowX).toEqual('hidden')
     })
 
-    it('should leave overflowX:auto on scroll-container if columns require more than the available width', () => {
-      const rendered = findDOMNode(render(getMarkup({ columnCount: 25 })))
+    it('should leave overflowX:auto if columns require more than the available width', () => {
+      const rendered = findDOMNode(render(getMarkup({
+        columnCount: 4,
+        width: 200 - 1,
+        rowCount: 5
+      })))
       expect(rendered.style.overflowX).not.toEqual('hidden')
     })
 
-    it('should set overflowY:hidden on scroll-container if rows fit within the available height', () => {
-      const rendered = findDOMNode(render(getMarkup({ rowCount: 5 })))
+    it('should leave overflowX:auto if columns and y-axis scrollbar require more than the available width', () => {
+      const rendered = findDOMNode(render(getMarkup({
+        columnCount: 4,
+        width: 200 + scrollbarSize - 1
+      })))
+      expect(rendered.style.overflowX).not.toEqual('hidden')
+    })
+
+    it('should set overflowY:hidden if rows fit within the available width and xaxis has no scrollbar', () => {
+      const rendered = findDOMNode(render(getMarkup({
+        rowCount: 5,
+        columnCount: 4
+      })))
       expect(rendered.style.overflowY).toEqual('hidden')
     })
 
-    it('should leave overflowY:auto on scroll-container if rows require more than the available height', () => {
-      const rendered = findDOMNode(render(getMarkup({ rowCount: 25 })))
+    it('should set overflowY:hidden if rows and x-axis scrollbar fit within the available width', () => {
+      const rendered = findDOMNode(render(getMarkup({
+        rowCount: 5,
+        height: 100 + scrollbarSize
+      })))
+      expect(rendered.style.overflowY).toEqual('hidden')
+    })
+
+    it('should leave overflowY:auto if rows require more than the available width', () => {
+      const rendered = findDOMNode(render(getMarkup({
+        rowCount: 5,
+        height: 100 - 1,
+        columnCount: 4
+      })))
+      expect(rendered.style.overflowY).not.toEqual('hidden')
+    })
+
+    it('should leave overflowY:auto if rows and x-axis scrollbar require more than the available width', () => {
+      const rendered = findDOMNode(render(getMarkup({
+        rowCount: 5,
+        height: 100 + scrollbarSize - 1
+      })))
       expect(rendered.style.overflowY).not.toEqual('hidden')
     })
   })