From 503821e3a6c08e03904f1d6dbe4cad342407fae4 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 16 Mar 2016 21:58:25 -0700 Subject: [PATCH 1/8] Removed arrow-key snapping behavior from Grid and moved it into a new ArrowKeyStepper HOC --- .../ArrowKeyStepper.example.css | 15 +++ .../ArrowKeyStepper.example.js | 97 +++++++++++++++++++ source/ArrowKeyStepper/ArrowKeyStepper.js | 80 +++++++++++++++ source/ArrowKeyStepper/index.js | 2 + source/Grid/Grid.js | 52 ---------- source/demo/Application.js | 9 +- source/index.js | 1 + 7 files changed, 203 insertions(+), 53 deletions(-) create mode 100644 source/ArrowKeyStepper/ArrowKeyStepper.example.css create mode 100644 source/ArrowKeyStepper/ArrowKeyStepper.example.js create mode 100644 source/ArrowKeyStepper/ArrowKeyStepper.js create mode 100644 source/ArrowKeyStepper/index.js diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.example.css b/source/ArrowKeyStepper/ArrowKeyStepper.example.css new file mode 100644 index 000000000..9ed4edecd --- /dev/null +++ b/source/ArrowKeyStepper/ArrowKeyStepper.example.css @@ -0,0 +1,15 @@ +.Grid { + border: 1px solid #e0e0e0; +} + +.EvenCell, +.OddCell { + height: 100%; + display: flex; + align-items: center; + justify-content: center; +} + +.OddCell { + background-color: rgba(0, 0, 0, .1); +} diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.example.js b/source/ArrowKeyStepper/ArrowKeyStepper.example.js new file mode 100644 index 000000000..fbe0dfcab --- /dev/null +++ b/source/ArrowKeyStepper/ArrowKeyStepper.example.js @@ -0,0 +1,97 @@ +/** @flow */ +import Immutable from 'immutable' +import React, { Component, PropTypes } from 'react' +import { ContentBox, ContentBoxHeader, ContentBoxParagraph } from '../demo/ContentBox' +import ArrowKeyStepper from './ArrowKeyStepper' +import AutoSizer from '../AutoSizer' +import Grid from '../Grid' +import shouldPureComponentUpdate from 'react-pure-render/function' +import styles from './ArrowKeyStepper.example.css' + +export default class ArrowKeyStepperExample extends Component { + shouldComponentUpdate = shouldPureComponentUpdate + + static propTypes = { + list: PropTypes.instanceOf(Immutable.List).isRequired + } + + constructor (props) { + super(props) + + this._getColumnWidth = this._getColumnWidth.bind(this) + this._getRowHeight = this._getRowHeight.bind(this) + this._renderCell = this._renderCell.bind(this) + } + + render () { + const { list, ...props } = this.props + + return ( + + + + + This high-order component decorates a VirtualScroll, FlexTable, or Grid and responds to arrow-key events by scrolling one row or column at a time. + Focus in the `Grid` below and type the left, right, up, or down arrow key for an example. + + + + {({ onKeyDown, onSectionRendered, scrollToColumn, scrollToRow }) => ( +
+ + {`Most-recently-stepped column: ${scrollToColumn}, row: ${scrollToRow}`} + + +
+ + {({ width }) => ( + + )} + +
+
+ )} + + + ) + } + + _getColumnWidth (index) { + return (1 + (index % 3)) * 60 + } + + _getRowHeight (index) { + return (1 + (index % 3)) * 30 + } + + _renderCell ({ columnIndex, rowIndex }) { + const className = rowIndex % 2 === 0 + ? columnIndex % 2 === 0 ? styles.EvenCell : styles.OddCell + : columnIndex % 2 !== 0 ? styles.EvenCell : styles.OddCell + + return ( +
+ {`r:${rowIndex}, c:${columnIndex}`} +
+ ) + } +} diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.js b/source/ArrowKeyStepper/ArrowKeyStepper.js new file mode 100644 index 000000000..4e63c4b31 --- /dev/null +++ b/source/ArrowKeyStepper/ArrowKeyStepper.js @@ -0,0 +1,80 @@ +/** @flow */ +import { Component, PropTypes } from 'react' +import shouldPureComponentUpdate from 'react-pure-render/function' + +/** + * This HOC decorates a virtualized component and responds to arrow-key events by scrolling one row or column at a time. + */ +export default class ArrowKeyStepper extends Component { + shouldComponentUpdate = shouldPureComponentUpdate + + static propTypes = { + children: PropTypes.func.isRequired, + columnsCount: PropTypes.number.isRequired, + rowsCount: PropTypes.number.isRequired + } + + constructor (props, context) { + super(props, context) + + this.state = { + scrollToColumn: 0, + scrollToRow: 0 + } + + this._onKeyDown = this._onKeyDown.bind(this) + this._onSectionRendered = this._onSectionRendered.bind(this) + } + + render () { + const { children } = this.props + const { scrollToColumn, scrollToRow } = this.state + + return children({ + onKeyDown: this._onKeyDown, + onSectionRendered: this._onSectionRendered, + scrollToColumn, + scrollToRow + }) + } + + _onKeyDown (event) { + const { columnsCount, rowsCount } = this.props + + // The above cases all prevent default event event behavior. + // This is to keep the grid from scrolling after the snap-to update. + switch (event.key) { + case 'ArrowDown': + event.preventDefault() + this.setState({ + scrollToRow: Math.min(this._rowStopIndex + 1, rowsCount - 1) + }) + break + case 'ArrowLeft': + event.preventDefault() + this.setState({ + scrollToColumn: Math.max(this._columnStartIndex - 1, 0) + }) + break + case 'ArrowRight': + event.preventDefault() + this.setState({ + scrollToColumn: Math.min(this._columnStopIndex + 1, columnsCount - 1) + }) + break + case 'ArrowUp': + event.preventDefault() + this.setState({ + scrollToRow: Math.max(this._rowStartIndex - 1, 0) + }) + break + } + } + + _onSectionRendered ({ columnStartIndex, columnStopIndex, rowStartIndex, rowStopIndex }) { + this._columnStartIndex = columnStartIndex + this._columnStopIndex = columnStopIndex + this._rowStartIndex = rowStartIndex + this._rowStopIndex = rowStopIndex + } +} diff --git a/source/ArrowKeyStepper/index.js b/source/ArrowKeyStepper/index.js new file mode 100644 index 000000000..bb6a72987 --- /dev/null +++ b/source/ArrowKeyStepper/index.js @@ -0,0 +1,2 @@ +export default from './ArrowKeyStepper' +export ArrowKeyStepper from './ArrowKeyStepper' diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js index 50e8e610a..cdd9d600a 100644 --- a/source/Grid/Grid.js +++ b/source/Grid/Grid.js @@ -151,7 +151,6 @@ export default class Grid extends Component { // Bind functions to instance so they don't lose context when passed around this._computeGridMetadata = this._computeGridMetadata.bind(this) this._invokeOnGridRenderedHelper = this._invokeOnGridRenderedHelper.bind(this) - this._onKeyPress = this._onKeyPress.bind(this) this._onScroll = this._onScroll.bind(this) this._updateScrollLeftForScrollToColumn = this._updateScrollLeftForScrollToColumn.bind(this) this._updateScrollTopForScrollToRow = this._updateScrollTopForScrollToRow.bind(this) @@ -470,7 +469,6 @@ export default class Grid extends Component {
+ {activeComponent === 'ArrowKeyStepper' && + + } {activeComponent === 'AutoSizer' && Date: Fri, 18 Mar 2016 08:53:09 -0700 Subject: [PATCH 2/8] Removed scroll-to-* and set-scroll-* methods from FlexTable, Grid, and VirtualScroll; properties should be used instead of these methods. --- source/FlexTable/FlexTable.js | 34 +--------- source/FlexTable/FlexTable.test.js | 3 - source/Grid/Grid.js | 72 +++++++++------------- source/Grid/Grid.test.js | 3 - source/VirtualScroll/VirtualScroll.js | 36 +---------- source/VirtualScroll/VirtualScroll.test.js | 3 - 6 files changed, 32 insertions(+), 119 deletions(-) diff --git a/source/FlexTable/FlexTable.js b/source/FlexTable/FlexTable.js index 0ac4afcbd..8d5047cea 100644 --- a/source/FlexTable/FlexTable.js +++ b/source/FlexTable/FlexTable.js @@ -146,33 +146,7 @@ export default class FlexTable extends Component { this.refs.Grid.recomputeGridSize() } - /** - * See Grid#scrollToIndex - */ - scrollToRow (scrollToIndex) { - this.refs.Grid.scrollToCell({ - scrollToColumn: 0, - scrollToRow: scrollToIndex - }) - } - - /** - * See Grid#setScrollPosition - */ - setScrollTop (scrollTop) { - this.refs.Grid.setScrollPosition({ - scrollLeft: 0, - scrollTop - }) - } - componentDidMount () { - const { scrollTop } = this.props - - if (scrollTop >= 0) { - this.setScrollTop(scrollTop) - } - this._setScrollbarWidth() } @@ -180,12 +154,6 @@ export default class FlexTable extends Component { this._setScrollbarWidth() } - componentWillUpdate (nextProps, nextState) { - if (nextProps.scrollTop !== this.props.scrollTop) { - this.setScrollTop(nextProps.scrollTop) - } - } - render () { const { className, @@ -200,6 +168,7 @@ export default class FlexTable extends Component { rowHeight, rowsCount, scrollToIndex, + scrollTop, width } = this.props const { scrollbarWidth } = this.state @@ -250,6 +219,7 @@ export default class FlexTable extends Component { rowHeight={rowHeight} rowsCount={rowsCount} scrollToRow={scrollToIndex} + scrollTop={scrollTop} width={width} />
diff --git a/source/FlexTable/FlexTable.test.js b/source/FlexTable/FlexTable.test.js index 829ffef6d..3eb430528 100644 --- a/source/FlexTable/FlexTable.test.js +++ b/source/FlexTable/FlexTable.test.js @@ -632,7 +632,4 @@ describe('FlexTable', () => { }) }) }) - - // TODO Add tests for :scrollToRow and :setScrollTop. - // This probably requires the creation of an inner test-only class with refs. }) diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js index 043b9ecbe..12a10488f 100644 --- a/source/Grid/Grid.js +++ b/source/Grid/Grid.js @@ -168,50 +168,13 @@ export default class Grid extends Component { }) } - /** - * Updates the Grid to ensure the cell at the specified row and column indices is visible. - * This method exists so that a user can forcefully scroll to the same cell twice. - * (The :scrollToColumn and :scrollToRow properties would not change in that case so it would not be picked up by the component.) - */ - scrollToCell ({ scrollToColumn, scrollToRow }) { - this._updateScrollLeftForScrollToColumn(scrollToColumn) - this._updateScrollTopForScrollToRow(scrollToRow) - } - - /** - * Set the :scrollLeft and :scrollTop position within the inner scroll container. - * Normally it is best to let Grid manage these properties or to use a method like :scrollToCell. - * This method enables Grid to be scroll-synced to another react-virtualized component though. - * It is appropriate to use in that case. - */ - setScrollPosition ({ scrollLeft, scrollTop }) { - const newState = { - scrollPositionChangeReason: SCROLL_POSITION_CHANGE_REASONS.REQUESTED - } - - if (scrollLeft >= 0) { - newState.scrollLeft = scrollLeft - } - - if (scrollTop >= 0) { - newState.scrollTop = scrollTop - } - - if ( - scrollLeft >= 0 && scrollLeft !== this.state.scrollLeft || - scrollTop >= 0 && scrollTop !== this.state.scrollTop - ) { - this.setState(newState) - } - } - componentDidMount () { const { scrollLeft, scrollToColumn, scrollTop, scrollToRow } = this.props this._scrollbarSize = getScrollbarSize() if (scrollLeft >= 0 || scrollTop >= 0) { - this.setScrollPosition({ scrollLeft, scrollTop }) + this._setScrollPosition({ scrollLeft, scrollTop }) } if (scrollToColumn >= 0 || scrollToRow >= 0) { @@ -310,22 +273,22 @@ export default class Grid extends Component { nextProps.columnsCount === 0 && nextState.scrollLeft !== 0 ) { - this.setScrollPosition({ scrollLeft: 0 }) + this._setScrollPosition({ scrollLeft: 0 }) } if ( nextProps.rowsCount === 0 && nextState.scrollTop !== 0 ) { - this.setScrollPosition({ scrollTop: 0 }) + this._setScrollPosition({ scrollTop: 0 }) } if (nextProps.scrollLeft !== this.props.scrollLeft) { - this.setScrollPosition({ scrollLeft: nextProps.scrollLeft }) + this._setScrollPosition({ scrollLeft: nextProps.scrollLeft }) } if (nextProps.scrollTop !== this.props.scrollTop) { - this.setScrollPosition({ scrollTop: nextProps.scrollTop }) + this._setScrollPosition({ scrollTop: nextProps.scrollTop }) } computeCellMetadataAndUpdateScrollOffsetHelper({ @@ -626,6 +589,27 @@ export default class Grid extends Component { }) } + _setScrollPosition ({ scrollLeft, scrollTop }) { + const newState = { + scrollPositionChangeReason: SCROLL_POSITION_CHANGE_REASONS.REQUESTED + } + + if (scrollLeft >= 0) { + newState.scrollLeft = scrollLeft + } + + if (scrollTop >= 0) { + newState.scrollTop = scrollTop + } + + if ( + scrollLeft >= 0 && scrollLeft !== this.state.scrollLeft || + scrollTop >= 0 && scrollTop !== this.state.scrollTop + ) { + this.setState(newState) + } + } + _updateScrollLeftForScrollToColumn (scrollToColumnOverride) { const scrollToColumn = scrollToColumnOverride != null ? scrollToColumnOverride @@ -643,7 +627,7 @@ export default class Grid extends Component { }) if (scrollLeft !== calculatedScrollLeft) { - this.setScrollPosition({ + this._setScrollPosition({ scrollLeft: calculatedScrollLeft }) } @@ -667,7 +651,7 @@ export default class Grid extends Component { }) if (scrollTop !== calculatedScrollTop) { - this.setScrollPosition({ + this._setScrollPosition({ scrollTop: calculatedScrollTop }) } diff --git a/source/Grid/Grid.test.js b/source/Grid/Grid.test.js index 6b7b84416..1900c5f40 100644 --- a/source/Grid/Grid.test.js +++ b/source/Grid/Grid.test.js @@ -508,7 +508,4 @@ describe('Grid', () => { expect(helper.rowStopIndex()).toEqual(4) }) }) - - // TODO Add tests for :scrollToCell and :setScrollPosition. - // This probably requires the creation of an inner test-only class with refs. }) diff --git a/source/VirtualScroll/VirtualScroll.js b/source/VirtualScroll/VirtualScroll.js index 7c802cf59..0bbb3c786 100644 --- a/source/VirtualScroll/VirtualScroll.js +++ b/source/VirtualScroll/VirtualScroll.js @@ -73,20 +73,6 @@ export default class VirtualScroll extends Component { overscanRowsCount: 10 } - componentDidMount () { - const { scrollTop } = this.props - - if (scrollTop >= 0) { - this.setScrollTop(scrollTop) - } - } - - componentWillUpdate (nextProps, nextState) { - if (nextProps.scrollTop !== this.props.scrollTop) { - this.setScrollTop(nextProps.scrollTop) - } - } - /** * See Grid#recomputeGridSize */ @@ -94,26 +80,6 @@ export default class VirtualScroll extends Component { this.refs.Grid.recomputeGridSize() } - /** - * See Grid#scrollToCell - */ - scrollToRow (scrollToIndex) { - this.refs.Grid.scrollToCell({ - scrollToColumn: 0, - scrollToRow: scrollToIndex - }) - } - - /** - * See Grid#setScrollPosition - */ - setScrollTop (scrollTop) { - this.refs.Grid.setScrollPosition({ - scrollLeft: 0, - scrollTop - }) - } - render () { const { className, @@ -126,6 +92,7 @@ export default class VirtualScroll extends Component { overscanRowsCount, rowsCount, scrollToIndex, + scrollTop, width } = this.props @@ -151,6 +118,7 @@ export default class VirtualScroll extends Component { rowHeight={rowHeight} rowsCount={rowsCount} scrollToRow={scrollToIndex} + scrollTop={scrollTop} width={width} /> ) diff --git a/source/VirtualScroll/VirtualScroll.test.js b/source/VirtualScroll/VirtualScroll.test.js index 8edd21330..0bd328f37 100644 --- a/source/VirtualScroll/VirtualScroll.test.js +++ b/source/VirtualScroll/VirtualScroll.test.js @@ -322,7 +322,4 @@ describe('VirtualScroll', () => { }) }) }) - - // TODO Add tests for :scrollToRow and :setScrollTop. - // This probably requires the creation of an inner test-only class with refs. }) From f14bd505452cc3ef8ffeeb8646bd4d328c9ce0aa Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 18 Mar 2016 18:43:12 -0700 Subject: [PATCH 3/8] Added docs for ArrowKeyStepper. Added tests. --- README.md | 1 + docs/ArrowKeyStepper.md | 57 ++++++++ docs/AutoSizer.md | 2 +- docs/ColumnSizer.md | 2 +- docs/README.md | 1 + docs/ScrollSync.md | 2 +- .../ArrowKeyStepper.example.js | 44 +++--- source/ArrowKeyStepper/ArrowKeyStepper.js | 22 ++- .../ArrowKeyStepper/ArrowKeyStepper.test.js | 126 ++++++++++++++++++ source/Grid/Grid.test.js | 3 + 10 files changed, 229 insertions(+), 31 deletions(-) create mode 100644 docs/ArrowKeyStepper.md create mode 100644 source/ArrowKeyStepper/ArrowKeyStepper.test.js diff --git a/README.md b/README.md index daffaa576..489e78a67 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ Examples for each component can be seen in [the documentation](docs/README.md). Here are some online demos of each component: +* [ArrowKeyStepper](https://bvaughn.github.io/react-virtualized/?component=ArrowKeyStepper) * [AutoSizer](https://bvaughn.github.io/react-virtualized/?component=AutoSizer) * [ColumnSizer](https://bvaughn.github.io/react-virtualized/?component=ColumnSizer) * [FlexTable](https://bvaughn.github.io/react-virtualized/?component=FlexTable) diff --git a/docs/ArrowKeyStepper.md b/docs/ArrowKeyStepper.md new file mode 100644 index 000000000..3a0f453cf --- /dev/null +++ b/docs/ArrowKeyStepper.md @@ -0,0 +1,57 @@ +ArrowKeyStepper +--------------- + +High-order component that decorates another virtualized component and responds to arrow-key events by scrolling one row or column at a time. +This provides a snap-to behavior rather than the default browser scrolling behavior. + +Note that unlike the other HOCs in react-virtualized, the `ArrowKeyStepper` adds a `
` element around its children in order to attach a key-down event handler. + +### Prop Types +| Property | Type | Required? | Description | +|:---|:---|:---:|:---| +| children | Function | ✓ | Function respondible for rendering children. This function should implement the following signature: `({ onKeyDown, onSectionRendered, scrollToColumn, scrollToRow }) => PropTypes.element` | +| columnsCount | Number | ✓ | Number of columns in grid; for `FlexTable` and `VirtualScroll` this property should always be `1`. | +| rowsCount | Number | ✓ | Number of rows in grid. | + +### Children function + +The child function is passed the following named parameters: + +| Parameter | Type | Description | +|:---|:---|:---:| +| onKeyDown | Function | Key-down event handler to be attached to the DOM hierarchy. | +| onSectionRendered | Function | Pass-through callback to be attached to child component; informs the key-stepper which range of cells are currently visible. | +| scrollToColumn | Number | Specifies which column in the child component should be visible | +| scrollToRow | Number | Specifies which row in the child component should be visible | + +### Examples + +You can decorate any virtualized component (eg. `FlexTable`, `Grid`, or `VirtualScroll`) with arrow-key snapping like so: + +```javascript +import React from 'react'; +import ReactDOM from 'react-dom'; +import { ArrowKeyStepper, Grid } from 'react-virtualized'; +import 'react-virtualized/styles.css'; // only needs to be imported once + +ReactDOM.render( + + {({ onKeyDown, onSectionRendered, scrollToColumn, scrollToRow }) => ( +
+ +
+ )} + , + document.getElementById('example') +); +``` diff --git a/docs/AutoSizer.md b/docs/AutoSizer.md index 5ddd7529c..e86c7ccc4 100644 --- a/docs/AutoSizer.md +++ b/docs/AutoSizer.md @@ -6,7 +6,7 @@ High-order component that automatically adjusts the width and height of a single ### Prop Types | Property | Type | Required? | Description | |:---|:---|:---:|:---| -| children | PropTypes.Element | ✓ | Function respondible for rendering children. This function should implement the following signature: `({ height, width }) => PropTypes.element` | +| children | Function | ✓ | Function respondible for rendering children. This function should implement the following signature: `({ height, width }) => PropTypes.element` | | disableHeight | Boolean | | If true the child's `height` property will not be managed | | disableWidth | Boolean | | If true the child's `width` property will not be managed | | onResize | Function | Callback to be invoked on-resize; it is passed the following named parameters: `({ height, width })` | diff --git a/docs/ColumnSizer.md b/docs/ColumnSizer.md index 0b30bc854..f604bda90 100644 --- a/docs/ColumnSizer.md +++ b/docs/ColumnSizer.md @@ -6,7 +6,7 @@ High-order component that auto-calculates column-widths for `Grid` cells. ### Prop Types | Property | Type | Required? | Description | |:---|:---|:---:|:---| -| children | PropTypes.Element | ✓ | Function respondible for rendering a virtualized Grid. This function should implement the following signature: `({ adjustedWidth, getColumnWidth, registerChild }) => PropTypes.element` | +| children | Function | ✓ | Function respondible for rendering a virtualized Grid. This function should implement the following signature: `({ adjustedWidth, getColumnWidth, registerChild }) => PropTypes.element` | | columnMaxWidth | Number | | Optional maximum allowed column width | | columnMinWidth | Number | | Optional minimum allowed column width | | width | Number | ✓ | Width of Grid or `FlexTable` child | diff --git a/docs/README.md b/docs/README.md index 68c835080..41ca5d8b9 100644 --- a/docs/README.md +++ b/docs/README.md @@ -10,6 +10,7 @@ Documentation * [VirtualScroll](VirtualScroll.md) ### High-Order Components +* [ArrowKeyStepper](ArrowKeyStepper.md) * [AutoSizer](AutoSizer.md) * [ColumnSizer](ColumnSizer.md) * [InfiniteLoader](InfiniteLoader.md) diff --git a/docs/ScrollSync.md b/docs/ScrollSync.md index f5ec4fa2b..c213f6be5 100644 --- a/docs/ScrollSync.md +++ b/docs/ScrollSync.md @@ -6,7 +6,7 @@ High order component that simplifies the process of synchronizing scrolling betw ### Prop Types | Property | Type | Required? | Description | |:---|:---|:---:|:---| -| children | PropTypes.Element | ✓ | Function respondible for rendering 2 or more virtualized components. This function should implement the following signature: `({ onScroll, scrollLeft, scrollTop }) => PropTypes.element` | +| children | Function | ✓ | Function respondible for rendering 2 or more virtualized components. This function should implement the following signature: `({ onScroll, scrollLeft, scrollTop }) => PropTypes.element` | ### Children function diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.example.js b/source/ArrowKeyStepper/ArrowKeyStepper.example.js index fbe0dfcab..c94243d2e 100644 --- a/source/ArrowKeyStepper/ArrowKeyStepper.example.js +++ b/source/ArrowKeyStepper/ArrowKeyStepper.example.js @@ -36,38 +36,40 @@ export default class ArrowKeyStepperExample extends Component { This high-order component decorates a VirtualScroll, FlexTable, or Grid and responds to arrow-key events by scrolling one row or column at a time. - Focus in the `Grid` below and type the left, right, up, or down arrow key for an example. + Focus in the `Grid` below and use the left, right, up, or down arrow keys to move around within the grid. + + + + Note that unlike the other HOCs in react-virtualized, the ArrowKeyStepper adds a <div> element around its children in order to attach a key-down event handler. - {({ onKeyDown, onSectionRendered, scrollToColumn, scrollToRow }) => ( + {({ onSectionRendered, scrollToColumn, scrollToRow }) => (
{`Most-recently-stepped column: ${scrollToColumn}, row: ${scrollToRow}`} -
- - {({ width }) => ( - - )} - -
+ + {({ width }) => ( + + )} +
)} diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.js b/source/ArrowKeyStepper/ArrowKeyStepper.js index 4e63c4b31..f4cd27546 100644 --- a/source/ArrowKeyStepper/ArrowKeyStepper.js +++ b/source/ArrowKeyStepper/ArrowKeyStepper.js @@ -1,5 +1,5 @@ /** @flow */ -import { Component, PropTypes } from 'react' +import React, { Component, PropTypes } from 'react' import shouldPureComponentUpdate from 'react-pure-render/function' /** @@ -22,6 +22,11 @@ export default class ArrowKeyStepper extends Component { scrollToRow: 0 } + this._columnStartIndex = 0 + this._columnStopIndex = 0 + this._rowStartIndex = 0 + this._rowStopIndex = 0 + this._onKeyDown = this._onKeyDown.bind(this) this._onSectionRendered = this._onSectionRendered.bind(this) } @@ -30,12 +35,15 @@ export default class ArrowKeyStepper extends Component { const { children } = this.props const { scrollToColumn, scrollToRow } = this.state - return children({ - onKeyDown: this._onKeyDown, - onSectionRendered: this._onSectionRendered, - scrollToColumn, - scrollToRow - }) + return ( +
+ {children({ + onSectionRendered: this._onSectionRendered, + scrollToColumn, + scrollToRow + })} +
+ ) } _onKeyDown (event) { diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.test.js b/source/ArrowKeyStepper/ArrowKeyStepper.test.js new file mode 100644 index 000000000..c12bf6b4d --- /dev/null +++ b/source/ArrowKeyStepper/ArrowKeyStepper.test.js @@ -0,0 +1,126 @@ +import React from 'react' +import { findDOMNode } from 'react-dom' +import { render } from '../TestUtils' +import ArrowKeyStepper from './ArrowKeyStepper' +import { Simulate } from 'react-addons-test-utils' + +function renderTextContent (scrollToColumn, scrollToRow) { + return `scrollToColumn:${scrollToColumn}, scrollToRow:${scrollToRow}` +} + +function ChildComponent ({ scrollToColumn, scrollToRow }) { + return ( +
{renderTextContent(scrollToColumn, scrollToRow)}
+ ) +} + +describe('ArrowKeyStepper', () => { + function renderHelper ({ + columnsCount = 10, + rowsCount = 10 + } = {}) { + let onSectionRenderedCallback + + const node = findDOMNode(render( + + {({ onSectionRendered, scrollToColumn, scrollToRow }) => { + onSectionRenderedCallback = onSectionRendered + + return ( + + ) + }} + + )) + + return { + node, + onSectionRendered: onSectionRenderedCallback + } + } + + function assertCurrentScrollTo (node, scrollToColumn, scrollToRow) { + expect(node.textContent).toEqual(renderTextContent(scrollToColumn, scrollToRow)) + } + + it('should update :scrollToColumn and :scrollToRow in response to arrow keys', () => { + const { node } = renderHelper() + assertCurrentScrollTo(node, 0, 0) + Simulate.keyDown(node, {key: 'ArrowDown'}) + assertCurrentScrollTo(node, 0, 1) + Simulate.keyDown(node, {key: 'ArrowRight'}) + assertCurrentScrollTo(node, 1, 1) + Simulate.keyDown(node, {key: 'ArrowUp'}) + assertCurrentScrollTo(node, 1, 0) + Simulate.keyDown(node, {key: 'ArrowLeft'}) + assertCurrentScrollTo(node, 0, 0) + }) + + it('should not scroll past the row and column boundaries provided', () => { + const { node } = renderHelper({ + columnsCount: 2, + rowsCount: 2 + }) + Simulate.keyDown(node, {key: 'ArrowDown'}) + Simulate.keyDown(node, {key: 'ArrowDown'}) + Simulate.keyDown(node, {key: 'ArrowDown'}) + assertCurrentScrollTo(node, 0, 1) + Simulate.keyDown(node, {key: 'ArrowUp'}) + Simulate.keyDown(node, {key: 'ArrowUp'}) + Simulate.keyDown(node, {key: 'ArrowUp'}) + assertCurrentScrollTo(node, 0, 0) + Simulate.keyDown(node, {key: 'ArrowRight'}) + Simulate.keyDown(node, {key: 'ArrowRight'}) + Simulate.keyDown(node, {key: 'ArrowRight'}) + assertCurrentScrollTo(node, 1, 0) + Simulate.keyDown(node, {key: 'ArrowLeft'}) + Simulate.keyDown(node, {key: 'ArrowLeft'}) + Simulate.keyDown(node, {key: 'ArrowLeft'}) + assertCurrentScrollTo(node, 0, 0) + }) + + it('should update :scrollToColumn and :scrollToRow relative to the most recent :onSectionRendered event', () => { + const { node, onSectionRendered } = renderHelper() + onSectionRendered({ // Simulate a scroll + columnStartIndex: 0, + columnStopIndex: 4, + rowStartIndex: 4, + rowStopIndex: 6 + }) + Simulate.keyDown(node, {key: 'ArrowDown'}) + assertCurrentScrollTo(node, 0, 7) + + onSectionRendered({ // Simulate a scroll + columnStartIndex: 5, + columnStopIndex: 10, + rowStartIndex: 2, + rowStopIndex: 4 + }) + Simulate.keyDown(node, {key: 'ArrowUp'}) + assertCurrentScrollTo(node, 0, 1) + + onSectionRendered({ // Simulate a scroll + columnStartIndex: 4, + columnStopIndex: 8, + rowStartIndex: 5, + rowStopIndex: 10 + }) + Simulate.keyDown(node, {key: 'ArrowRight'}) + assertCurrentScrollTo(node, 9, 1) + + onSectionRendered({ // Simulate a scroll + columnStartIndex: 2, + columnStopIndex: 4, + rowStartIndex: 2, + rowStopIndex: 4 + }) + Simulate.keyDown(node, {key: 'ArrowLeft'}) + assertCurrentScrollTo(node, 1, 1) + }) +}) diff --git a/source/Grid/Grid.test.js b/source/Grid/Grid.test.js index 1900c5f40..6657ddfbe 100644 --- a/source/Grid/Grid.test.js +++ b/source/Grid/Grid.test.js @@ -137,6 +137,9 @@ describe('Grid', () => { // Target offset for the last item then is 2,000 - 100 expect(grid.state.scrollTop).toEqual(1900) }) + + // @TODO Test updating with new row + scrollTop, removing row, scrolling, then re-adding row (with same scrollTop) + // @TODO Test updating with new row + scrollToIndex, removing row, scrolling, then re-adding row (with same scrollToIndex) }) describe('property updates', () => { From 001142eb6c7789d815b97eaa07708f84b0499cbd Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 18 Mar 2016 19:57:05 -0700 Subject: [PATCH 4/8] Added :className property to ArrowKeyStepper --- docs/ArrowKeyStepper.md | 2 ++ source/ArrowKeyStepper/ArrowKeyStepper.js | 8 ++++++-- source/ArrowKeyStepper/ArrowKeyStepper.test.js | 7 +++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/ArrowKeyStepper.md b/docs/ArrowKeyStepper.md index 3a0f453cf..e88f15cc9 100644 --- a/docs/ArrowKeyStepper.md +++ b/docs/ArrowKeyStepper.md @@ -5,11 +5,13 @@ High-order component that decorates another virtualized component and responds t This provides a snap-to behavior rather than the default browser scrolling behavior. Note that unlike the other HOCs in react-virtualized, the `ArrowKeyStepper` adds a `
` element around its children in order to attach a key-down event handler. +The appearance of this wrapper element can be customized using the `className` property. ### Prop Types | Property | Type | Required? | Description | |:---|:---|:---:|:---| | children | Function | ✓ | Function respondible for rendering children. This function should implement the following signature: `({ onKeyDown, onSectionRendered, scrollToColumn, scrollToRow }) => PropTypes.element` | +| className | String | | CSS class name to attach to the wrapper `
`. | | columnsCount | Number | ✓ | Number of columns in grid; for `FlexTable` and `VirtualScroll` this property should always be `1`. | | rowsCount | Number | ✓ | Number of rows in grid. | diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.js b/source/ArrowKeyStepper/ArrowKeyStepper.js index f4cd27546..b90d1f379 100644 --- a/source/ArrowKeyStepper/ArrowKeyStepper.js +++ b/source/ArrowKeyStepper/ArrowKeyStepper.js @@ -10,6 +10,7 @@ export default class ArrowKeyStepper extends Component { static propTypes = { children: PropTypes.func.isRequired, + className: PropTypes.string, columnsCount: PropTypes.number.isRequired, rowsCount: PropTypes.number.isRequired } @@ -32,11 +33,14 @@ export default class ArrowKeyStepper extends Component { } render () { - const { children } = this.props + const { className, children } = this.props const { scrollToColumn, scrollToRow } = this.state return ( -
+
{children({ onSectionRendered: this._onSectionRendered, scrollToColumn, diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.test.js b/source/ArrowKeyStepper/ArrowKeyStepper.test.js index c12bf6b4d..6225f063c 100644 --- a/source/ArrowKeyStepper/ArrowKeyStepper.test.js +++ b/source/ArrowKeyStepper/ArrowKeyStepper.test.js @@ -16,6 +16,7 @@ function ChildComponent ({ scrollToColumn, scrollToRow }) { describe('ArrowKeyStepper', () => { function renderHelper ({ + className, columnsCount = 10, rowsCount = 10 } = {}) { @@ -23,6 +24,7 @@ describe('ArrowKeyStepper', () => { const node = findDOMNode(render( @@ -49,6 +51,11 @@ describe('ArrowKeyStepper', () => { expect(node.textContent).toEqual(renderTextContent(scrollToColumn, scrollToRow)) } + it('should use a custom :className if one is specified', () => { + const { node } = renderHelper({ className: 'foo' }) + expect(node.className).toEqual('foo') + }) + it('should update :scrollToColumn and :scrollToRow in response to arrow keys', () => { const { node } = renderHelper() assertCurrentScrollTo(node, 0, 0) From 4e1383ca1fd5b661b92fdc4c2b13f09b593a4f5e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 18 Mar 2016 21:21:37 -0700 Subject: [PATCH 5/8] Added some edge-case tests to Grid. Batched up some scrollLeft/scrollTop state-updates. --- source/Grid/Grid.js | 28 +++++++++++++--------------- source/Grid/Grid.test.js | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js index 12a10488f..8bdebca1d 100644 --- a/source/Grid/Grid.js +++ b/source/Grid/Grid.js @@ -271,24 +271,22 @@ export default class Grid extends Component { componentWillUpdate (nextProps, nextState) { if ( nextProps.columnsCount === 0 && - nextState.scrollLeft !== 0 - ) { - this._setScrollPosition({ scrollLeft: 0 }) - } - - if ( + nextState.scrollLeft !== 0 || nextProps.rowsCount === 0 && nextState.scrollTop !== 0 ) { - this._setScrollPosition({ scrollTop: 0 }) - } - - if (nextProps.scrollLeft !== this.props.scrollLeft) { - this._setScrollPosition({ scrollLeft: nextProps.scrollLeft }) - } - - if (nextProps.scrollTop !== this.props.scrollTop) { - this._setScrollPosition({ scrollTop: nextProps.scrollTop }) + this._setScrollPosition({ + scrollLeft: 0, + scrollTop: 0 + }) + } else if ( + nextProps.scrollLeft !== this.props.scrollLeft || + nextProps.scrollTop !== this.props.scrollTop + ) { + this._setScrollPosition({ + scrollLeft: nextProps.scrollLeft, + scrollTop: nextProps.scrollTop + }) } computeCellMetadataAndUpdateScrollOffsetHelper({ diff --git a/source/Grid/Grid.test.js b/source/Grid/Grid.test.js index 6657ddfbe..ac6bbf18a 100644 --- a/source/Grid/Grid.test.js +++ b/source/Grid/Grid.test.js @@ -138,8 +138,36 @@ describe('Grid', () => { expect(grid.state.scrollTop).toEqual(1900) }) - // @TODO Test updating with new row + scrollTop, removing row, scrolling, then re-adding row (with same scrollTop) - // @TODO Test updating with new row + scrollToIndex, removing row, scrolling, then re-adding row (with same scrollToIndex) + it('should scroll to a row and column just added', () => { + let grid = render(getMarkup()) + expect(grid.state.scrollLeft).toEqual(0) + expect(grid.state.scrollTop).toEqual(0) + grid = render(getMarkup({ + columnsCount: NUM_COLUMNS + 1, + rowsCount: NUM_ROWS + 1, + scrollToColumn: NUM_COLUMNS, + scrollToRow: NUM_ROWS + })) + expect(grid.state.scrollLeft).toEqual(2350) + expect(grid.state.scrollTop).toEqual(1920) + }) + + it('should scroll back to a newly-added cell without a change in prop', () => { + let grid = render(getMarkup({ + columnsCount: NUM_COLUMNS, + rowsCount: NUM_ROWS, + scrollToColumn: NUM_COLUMNS, + scrollToRow: NUM_ROWS + })) + grid = render(getMarkup({ + columnsCount: NUM_COLUMNS + 1, + rowsCount: NUM_ROWS + 1, + scrollToColumn: NUM_COLUMNS, + scrollToRow: NUM_ROWS + })) + expect(grid.state.scrollLeft).toEqual(2350) + expect(grid.state.scrollTop).toEqual(1920) + }) }) describe('property updates', () => { From 02333a35dbed05d29f1c282040dced15bd9a126c Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 19 Mar 2016 07:56:45 -0700 Subject: [PATCH 6/8] Updated ArrowKeyStepper.example to demonstrate how the concept of "focus" could be added to a Grid --- source/ArrowKeyStepper/ArrowKeyStepper.example.css | 10 ++++++---- source/ArrowKeyStepper/ArrowKeyStepper.example.js | 11 ++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.example.css b/source/ArrowKeyStepper/ArrowKeyStepper.example.css index 9ed4edecd..ee3d156bf 100644 --- a/source/ArrowKeyStepper/ArrowKeyStepper.example.css +++ b/source/ArrowKeyStepper/ArrowKeyStepper.example.css @@ -2,14 +2,16 @@ border: 1px solid #e0e0e0; } -.EvenCell, -.OddCell { +.Cell { height: 100%; display: flex; align-items: center; justify-content: center; + border-right: 1px solid #e0e0e0; + border-bottom: 1px solid #e0e0e0; } -.OddCell { - background-color: rgba(0, 0, 0, .1); +.FocusedCell { + background-color: #e0e0e0; + font-weight: bold; } diff --git a/source/ArrowKeyStepper/ArrowKeyStepper.example.js b/source/ArrowKeyStepper/ArrowKeyStepper.example.js index c94243d2e..f27050af6 100644 --- a/source/ArrowKeyStepper/ArrowKeyStepper.example.js +++ b/source/ArrowKeyStepper/ArrowKeyStepper.example.js @@ -6,6 +6,7 @@ import ArrowKeyStepper from './ArrowKeyStepper' import AutoSizer from '../AutoSizer' import Grid from '../Grid' import shouldPureComponentUpdate from 'react-pure-render/function' +import cn from 'classnames' import styles from './ArrowKeyStepper.example.css' export default class ArrowKeyStepperExample extends Component { @@ -61,7 +62,7 @@ export default class ArrowKeyStepperExample extends Component { columnsCount={100} height={200} onSectionRendered={onSectionRendered} - renderCell={this._renderCell} + renderCell={({ columnIndex, rowIndex }) => this._renderCell({ columnIndex, rowIndex, scrollToColumn, scrollToRow }) } rowHeight={this._getRowHeight} rowsCount={100} scrollToColumn={scrollToColumn} @@ -85,10 +86,10 @@ export default class ArrowKeyStepperExample extends Component { return (1 + (index % 3)) * 30 } - _renderCell ({ columnIndex, rowIndex }) { - const className = rowIndex % 2 === 0 - ? columnIndex % 2 === 0 ? styles.EvenCell : styles.OddCell - : columnIndex % 2 !== 0 ? styles.EvenCell : styles.OddCell + _renderCell ({ columnIndex, rowIndex, scrollToColumn, scrollToRow }) { + const className = cn(styles.Cell, { + [styles.FocusedCell]: columnIndex === scrollToColumn && rowIndex === scrollToRow + }) return (
From 4cd4d363d761a1c6067c8ca4a4b19353aa4adf5f Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 19 Mar 2016 08:02:55 -0700 Subject: [PATCH 7/8] Moved source/utils to source/Grid/GridUtils for better organization --- source/Grid/Grid.js | 2 +- source/{utils.js => Grid/GridUtils.js} | 0 source/{utils.test.js => Grid/GridUtils.test.js} | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename source/{utils.js => Grid/GridUtils.js} (100%) rename source/{utils.test.js => Grid/GridUtils.test.js} (99%) diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js index 8bdebca1d..1afc64d7a 100644 --- a/source/Grid/Grid.js +++ b/source/Grid/Grid.js @@ -7,7 +7,7 @@ import { getVisibleCellIndices, initCellMetadata, updateScrollIndexHelper -} from '../utils' +} from './GridUtils' import cn from 'classnames' import raf from 'raf' import getScrollbarSize from 'dom-helpers/util/scrollbarSize' diff --git a/source/utils.js b/source/Grid/GridUtils.js similarity index 100% rename from source/utils.js rename to source/Grid/GridUtils.js diff --git a/source/utils.test.js b/source/Grid/GridUtils.test.js similarity index 99% rename from source/utils.test.js rename to source/Grid/GridUtils.test.js index e7c38c343..7359089e5 100644 --- a/source/utils.test.js +++ b/source/Grid/GridUtils.test.js @@ -6,7 +6,7 @@ import { getVisibleCellIndices, initCellMetadata, updateScrollIndexHelper -} from './utils' +} from './GridUtils' // Default cell sizes and offsets for use in below tests function getCellMetadata () { From b569da7869a021395809e221b6fb27fd7317423d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 19 Mar 2016 08:26:37 -0700 Subject: [PATCH 8/8] Added some docs in Grid to clarify when/why scroll offsets are updated --- source/Grid/Grid.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/source/Grid/Grid.js b/source/Grid/Grid.js index 1afc64d7a..04a864e0a 100644 --- a/source/Grid/Grid.js +++ b/source/Grid/Grid.js @@ -194,6 +194,11 @@ export default class Grid extends Component { }) } + /** + * @private + * This method updates scrollLeft/scrollTop in state for the following conditions: + * 1) New scroll-to-cell props have been set + */ componentDidUpdate (prevProps, prevState) { const { columnsCount, columnWidth, height, rowHeight, rowsCount, scrollToColumn, scrollToRow, width } = this.props const { scrollLeft, scrollPositionChangeReason, scrollTop } = this.state @@ -220,7 +225,7 @@ export default class Grid extends Component { } } - // Update scrollLeft if appropriate + // Update scroll offsets if the current :scrollToColumn or :scrollToRow values requires it updateScrollIndexHelper({ cellsCount: columnsCount, cellMetadata: this._columnMetadata, @@ -234,8 +239,6 @@ export default class Grid extends Component { size: width, updateScrollIndexCallback: this._updateScrollLeftForScrollToColumn }) - - // Update scrollTop if appropriate updateScrollIndexHelper({ cellsCount: rowsCount, cellMetadata: this._rowMetadata, @@ -268,6 +271,13 @@ export default class Grid extends Component { } } + /** + * @private + * This method updates scrollLeft/scrollTop in state for the following conditions: + * 1) Empty content (0 rows or columns) + * 2) New scroll props overriding the current state + * 3) Cells-count or cells-size has changed, making previous scroll offsets invalid + */ componentWillUpdate (nextProps, nextState) { if ( nextProps.columnsCount === 0 && @@ -289,6 +299,7 @@ export default class Grid extends Component { }) } + // Update scroll offsets if the size or number of cells have changed, invalidating the previous value computeCellMetadataAndUpdateScrollOffsetHelper({ cellsCount: this.props.columnsCount, cellSize: this.props.columnWidth, @@ -301,7 +312,6 @@ export default class Grid extends Component { scrollToIndex: this.props.scrollToColumn, updateScrollOffsetForScrollToIndex: this._updateScrollLeftForScrollToColumn }) - computeCellMetadataAndUpdateScrollOffsetHelper({ cellsCount: this.props.rowsCount, cellSize: this.props.rowHeight,