diff --git a/source/Grid/Grid.jest.js b/source/Grid/Grid.jest.js index 501c4401e..a636d8d18 100644 --- a/source/Grid/Grid.jest.js +++ b/source/Grid/Grid.jest.js @@ -1,4 +1,3 @@ -import getScrollbarSize from 'dom-helpers/util/scrollbarSize' import React from 'react' import { findDOMNode } from 'react-dom' import { Simulate } from 'react-addons-test-utils' @@ -15,6 +14,14 @@ const DEFAULT_WIDTH = 200 const NUM_ROWS = 100 const NUM_COLUMNS = 50 +function getScrollbarSize0 () { + return 0 +} + +function getScrollbarSize20 () { + return 20 +} + describe('Grid', () => { function defaultCellRenderer ({ columnIndex, key, rowIndex, style }) { return ( @@ -44,6 +51,7 @@ describe('Grid', () => { cellRenderer={defaultCellRenderer} columnCount={NUM_COLUMNS} columnWidth={DEFAULT_COLUMN_WIDTH} + getScrollbarSize={getScrollbarSize0} height={DEFAULT_HEIGHT} overscanColumnCount={0} overscanRowCount={0} @@ -105,15 +113,10 @@ describe('Grid', () => { }) describe('shows and hides scrollbars based on rendered content', () => { - 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, + getScrollbarSize: getScrollbarSize20, rowCount: 5 }))) expect(rendered.style.overflowX).toEqual('hidden') @@ -122,7 +125,8 @@ describe('Grid', () => { 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 + getScrollbarSize: getScrollbarSize20, + width: 200 + getScrollbarSize20() }))) expect(rendered.style.overflowX).toEqual('hidden') }) @@ -130,6 +134,7 @@ describe('Grid', () => { it('should leave overflowX:auto if columns require more than the available width', () => { const rendered = findDOMNode(render(getMarkup({ columnCount: 4, + getScrollbarSize: getScrollbarSize20, width: 200 - 1, rowCount: 5 }))) @@ -139,13 +144,15 @@ describe('Grid', () => { 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 + getScrollbarSize: getScrollbarSize20, + width: 200 + getScrollbarSize20() - 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({ + getScrollbarSize: getScrollbarSize20, rowCount: 5, columnCount: 4 }))) @@ -154,14 +161,16 @@ describe('Grid', () => { it('should set overflowY:hidden if rows and x-axis scrollbar fit within the available width', () => { const rendered = findDOMNode(render(getMarkup({ + getScrollbarSize: getScrollbarSize20, rowCount: 5, - height: 100 + scrollbarSize + height: 100 + getScrollbarSize20() }))) expect(rendered.style.overflowY).toEqual('hidden') }) it('should leave overflowY:auto if rows require more than the available width', () => { const rendered = findDOMNode(render(getMarkup({ + getScrollbarSize: getScrollbarSize20, rowCount: 5, height: 100 - 1, columnCount: 4 @@ -171,8 +180,9 @@ describe('Grid', () => { it('should leave overflowY:auto if rows and x-axis scrollbar require more than the available width', () => { const rendered = findDOMNode(render(getMarkup({ + getScrollbarSize: getScrollbarSize20, rowCount: 5, - height: 100 + scrollbarSize - 1 + height: 100 + getScrollbarSize20() - 1 }))) expect(rendered.style.overflowY).not.toEqual('hidden') }) @@ -180,6 +190,7 @@ describe('Grid', () => { it('should accept styles that overwrite calculated ones', () => { const rendered = findDOMNode(render(getMarkup({ columnCount: 1, + getScrollbarSize: getScrollbarSize20, height: 1, rowCount: 1, style: { @@ -412,6 +423,23 @@ describe('Grid', () => { expect(grid.state.scrollLeft).toEqual(2450) expect(grid.state.scrollTop).toEqual(920) }) + + it('should take scrollbar size into account when aligning cells', () => { + const grid = render(getMarkup({ + columnWidth: 50, + columnCount: 100, + getScrollbarSize: getScrollbarSize20, + height: 100, + rowCount: 100, + rowHeight: 20, + scrollToColumn: 50, + scrollToRow: 50, + width: 100 + })) + + expect(grid.state.scrollLeft).toEqual(2450 + getScrollbarSize20()) + expect(grid.state.scrollTop).toEqual(920 + getScrollbarSize20()) + }) }) describe('property updates', () => { diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js index a3706cc38..3365770d6 100644 --- a/source/Grid/Grid.js +++ b/source/Grid/Grid.js @@ -5,7 +5,6 @@ import calculateSizeAndPositionDataAndUpdateScrollOffset from './utils/calculate import ScalingCellSizeAndPositionManager from './utils/ScalingCellSizeAndPositionManager' import createCallbackMemoizer from '../utils/createCallbackMemoizer' import defaultOverscanIndicesGetter, { SCROLL_DIRECTION_BACKWARD, SCROLL_DIRECTION_FORWARD } from './utils/defaultOverscanIndicesGetter' -import getScrollbarSize from 'dom-helpers/util/scrollbarSize' import updateScrollIndexHelper from './utils/updateScrollIndexHelper' import defaultCellRangeRenderer from './defaultCellRangeRenderer' @@ -105,6 +104,11 @@ export default class Grid extends PureComponent { */ estimatedRowSize: PropTypes.number.isRequired, + /** + * Exposed for testing purposes only. + */ + getScrollbarSize: PropTypes.func.isRequired, + /** * Height of Grid; this property determines the number of visible (vs virtualized) rows. */ @@ -212,6 +216,7 @@ export default class Grid extends PureComponent { cellRangeRenderer: defaultCellRangeRenderer, estimatedColumnSize: 100, estimatedRowSize: 30, + getScrollbarSize: require('dom-helpers/util/scrollbarSize'), noContentRenderer: () => null, onScroll: () => null, onSectionRendered: () => null, @@ -354,7 +359,7 @@ export default class Grid extends PureComponent { } componentDidMount () { - const { scrollLeft, scrollToColumn, scrollTop, scrollToRow } = this.props + const { getScrollbarSize, scrollLeft, scrollToColumn, scrollTop, scrollToRow } = this.props // If cell sizes have been invalidated (eg we are using CellMeasurer) then reset cached positions. // We must do this at the start of the method as we may calculate and update scroll position below. @@ -500,6 +505,8 @@ export default class Grid extends PureComponent { } componentWillMount () { + const { getScrollbarSize } = this.props + // If this component is being rendered server-side, getScrollbarSize() will return undefined. // We handle this case in componentDidMount() this._scrollbarSize = getScrollbarSize() @@ -957,15 +964,17 @@ export default class Grid extends PureComponent { } _updateScrollLeftForScrollToColumn (props = this.props, state = this.state) { - const { columnCount, scrollToAlignment, scrollToColumn, width } = props + const { columnCount, height, scrollToAlignment, scrollToColumn, width } = props const { scrollLeft } = state if (scrollToColumn >= 0 && columnCount > 0) { const targetIndex = Math.max(0, Math.min(columnCount - 1, scrollToColumn)) + const totalRowsHeight = this._rowSizeAndPositionManager.getTotalSize() + const scrollBarSize = totalRowsHeight > height ? this._scrollbarSize : 0 const calculatedScrollLeft = this._columnSizeAndPositionManager.getUpdatedOffsetForIndex({ align: scrollToAlignment, - containerSize: width, + containerSize: width - scrollBarSize, currentOffset: scrollLeft, targetIndex }) @@ -979,15 +988,17 @@ export default class Grid extends PureComponent { } _updateScrollTopForScrollToRow (props = this.props, state = this.state) { - const { height, rowCount, scrollToAlignment, scrollToRow } = props + const { height, rowCount, scrollToAlignment, scrollToRow, width } = props const { scrollTop } = state if (scrollToRow >= 0 && rowCount > 0) { const targetIndex = Math.max(0, Math.min(rowCount - 1, scrollToRow)) + const totalColumnsWidth = this._columnSizeAndPositionManager.getTotalSize() + const scrollBarSize = totalColumnsWidth > width ? this._scrollbarSize : 0 const calculatedScrollTop = this._rowSizeAndPositionManager.getUpdatedOffsetForIndex({ align: scrollToAlignment, - containerSize: height, + containerSize: height - scrollBarSize, currentOffset: scrollTop, targetIndex })