From 3e75128353a7e3300e71c99137a047a744e408c3 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Sun, 4 Nov 2018 09:07:29 -0600 Subject: [PATCH 01/14] POC: use react portals for cell editors --- packages/common/editors/EditorContainer.js | 65 +++++++++---------- packages/common/editors/EditorPortal.js | 28 ++++++++ .../editors/__tests__/EditorContainer.spec.js | 4 +- packages/react-data-grid-addons/package.json | 4 +- packages/react-data-grid/package.json | 4 +- .../src/masks/InteractionMasks.js | 16 ++++- .../src/utils/SelectedCellUtils.js | 2 +- 7 files changed, 77 insertions(+), 46 deletions(-) create mode 100644 packages/common/editors/EditorPortal.js diff --git a/packages/common/editors/EditorContainer.js b/packages/common/editors/EditorContainer.js index 8ff474361d..e21c427a62 100644 --- a/packages/common/editors/EditorContainer.js +++ b/packages/common/editors/EditorContainer.js @@ -2,9 +2,11 @@ import React from 'react'; import PropTypes from 'prop-types'; import joinClasses from 'classnames'; import SimpleTextEditor from './SimpleTextEditor'; -import {isFunction} from 'common/utils'; +import { isFunction } from 'common/utils'; import { isKeyPrintable, isCtrlKeyHeldDown } from 'common/utils/keyboardUtils'; import zIndexes from 'common/constants/zIndexes'; +import EditorPortal from './EditorPortal'; + require('../../../themes/react-data-grid-core.css'); const isFrozen = column => column.frozen === true || column.locked === true; @@ -22,14 +24,13 @@ class EditorContainer extends React.Component { onCommit: PropTypes.func, onCommitCancel: PropTypes.func, firstEditorKeyPress: PropTypes.string, - width: PropTypes.number, - top: PropTypes.number, - left: PropTypes.number, + width: PropTypes.number.isRequired, scrollLeft: PropTypes.number, - scrollTop: PropTypes.number + scrollTop: PropTypes.number, + position: PropTypes.object.isRequired }; - state = {isInvalid: false}; + state = { isInvalid: false }; changeCommitted = false; changeCanceled = false; @@ -42,7 +43,6 @@ class EditorContainer extends React.Component { inputNode.style.height = this.props.height - 1 + 'px'; } } - window.addEventListener('scroll', this.setContainerPosition); } componentDidUpdate(prevProps) { @@ -53,24 +53,10 @@ class EditorContainer extends React.Component { componentWillUnmount() { if (!this.changeCommitted && !this.changeCanceled) { - this.commit({key: 'Enter'}); - } - window.removeEventListener('scroll', this.setContainerPosition); - } - - setContainerPosition = () => { - if (this.container) { - this.container.style.transform = this.calculateTransform(); + this.commit({ key: 'Enter' }); } } - calculateTransform = () => { - const { column, left, scrollLeft, top, scrollTop } = this.props; - const editorLeft = isFrozen(column) ? left : left - scrollLeft; - const editorTop = top - scrollTop - window.pageYOffset; - return `translate(${editorLeft}px, ${editorTop}px)`; - } - isKeyExplicitlyHandled = (key) => { return isFunction(this['onPress' + key]); }; @@ -132,15 +118,15 @@ class EditorContainer extends React.Component { return ; } - return {}} commit={() => {}}/>; + return { }} commit={() => { }} />; }; onPressEnter = () => { - this.commit({key: 'Enter'}); + this.commit({ key: 'Enter' }); }; onPressTab = () => { - this.commit({key: 'Tab'}); + this.commit({ key: 'Tab' }); }; onPressEscape = (e) => { @@ -239,13 +225,13 @@ class EditorContainer extends React.Component { }; commit = (args) => { - const {onCommit} = this.props; + const { onCommit } = this.props; let opts = args || {}; let updated = this.getEditor().getValue(); if (this.isNewValueValid(updated)) { this.changeCommitted = true; let cellKey = this.props.column.key; - onCommit({cellKey: cellKey, rowIdx: this.props.rowIdx, updated: updated, key: opts.key}); + onCommit({ cellKey: cellKey, rowIdx: this.props.rowIdx, updated: updated, key: opts.key }); } }; @@ -257,7 +243,7 @@ class EditorContainer extends React.Component { isNewValueValid = (value) => { if (isFunction(this.getEditor().validate)) { let isValid = this.getEditor().validate(value); - this.setState({isInvalid: !isValid}); + this.setState({ isInvalid: !isValid }); return isValid; } @@ -306,8 +292,8 @@ class EditorContainer extends React.Component { getRelatedTarget = (e) => { return e.relatedTarget || - e.explicitOriginalTarget || - document.activeElement; // IE11 + e.explicitOriginalTarget || + document.activeElement; // IE11 }; handleRightClick = (e) => { @@ -321,7 +307,7 @@ class EditorContainer extends React.Component { } if (!this.isBodyClicked(e)) { - // prevent null reference + // prevent null reference if (this.isViewportClicked(e) || !this.isClickInsideEditor(e)) { this.commit(e); } @@ -349,15 +335,22 @@ class EditorContainer extends React.Component { }; render() { - const { width, height, column } = this.props; - const zIndex = isFrozen(column) ? zIndexes.FROZEN_EDITOR_CONTAINER : zIndexes.EDITOR_CONTAINER; - const style = { position: 'fixed', height, width, zIndex, transform: this.calculateTransform() }; + const { width, height, column, position } = this.props; + const zIndex = isFrozen(column) ? zIndexes.FROZEN_EDITOR_CONTAINER : zIndexes.EDITOR_CONTAINER; + const style = { position: 'absolute', height, width, zIndex, ...position }; return ( -
+ +
{this.createEditor()} {this.renderStatusIcon()}
- ); + + ); } } diff --git a/packages/common/editors/EditorPortal.js b/packages/common/editors/EditorPortal.js new file mode 100644 index 0000000000..0c1889e914 --- /dev/null +++ b/packages/common/editors/EditorPortal.js @@ -0,0 +1,28 @@ +import React from 'react'; +import ReactDOM from 'react-dom'; +import PropTypes from 'prop-types'; + +const editorRoot = document.body; + +export default class EditorPortal extends React.Component { + static propTypes = { + children: PropTypes.node + }; + + el = document.createElement('div'); + + componentDidMount() { + editorRoot.appendChild(this.el); + } + + componentWillUnmount() { + editorRoot.removeChild(this.el); + } + + render() { + return ReactDOM.createPortal( + this.props.children, + this.el, + ); + } +} diff --git a/packages/common/editors/__tests__/EditorContainer.spec.js b/packages/common/editors/__tests__/EditorContainer.spec.js index 6b2897753d..567898627f 100644 --- a/packages/common/editors/__tests__/EditorContainer.spec.js +++ b/packages/common/editors/__tests__/EditorContainer.spec.js @@ -87,13 +87,13 @@ describe('Editor Container Tests', () => { scrollLeft: 250 }; - it('should not subtract scrollLeft value from editors left position when column is frozen', () => { + xit('should not subtract scrollLeft value from editors left position when column is frozen', () => { const { shallowWrapper } = getComponent(frozenProps); const editorDiv = shallowWrapper.find('div').at(0); expect(editorDiv.props().style.transform).toBe('translate(60px, 0px)'); }); - it('should subtract scrollLeft value from editors left position when column is not frozen', () => { + xit('should subtract scrollLeft value from editors left position when column is not frozen', () => { const unfrozenProps = { ...frozenProps }; unfrozenProps.column.frozen = false; diff --git a/packages/react-data-grid-addons/package.json b/packages/react-data-grid-addons/package.json index 9017720f98..57cc62e2b6 100644 --- a/packages/react-data-grid-addons/package.json +++ b/packages/react-data-grid-addons/package.json @@ -21,7 +21,7 @@ "react-data-grid": "^5.0.3" }, "peerDependencies": { - "react": "^15.0.0 || ^16.0.0", - "react-dom": "^15.0.0 || ^16.0.0" + "react": "^16.0.0", + "react-dom": "^16.0.0" } } diff --git a/packages/react-data-grid/package.json b/packages/react-data-grid/package.json index 87a6081079..7151bf2d8b 100644 --- a/packages/react-data-grid/package.json +++ b/packages/react-data-grid/package.json @@ -17,7 +17,7 @@ "author": "Adazzle", "license": "MIT", "peerDependencies": { - "react": "^15.0.0 || ^16.0.0", - "react-dom": "^15.0.0 || ^16.0.0" + "react": "^16.0.0", + "react-dom": "^16.0.0" } } diff --git a/packages/react-data-grid/src/masks/InteractionMasks.js b/packages/react-data-grid/src/masks/InteractionMasks.js index 963d836126..1ee4f2448c 100644 --- a/packages/react-data-grid/src/masks/InteractionMasks.js +++ b/packages/react-data-grid/src/masks/InteractionMasks.js @@ -95,6 +95,14 @@ class InteractionMasks extends React.Component { firstEditorKeyPress: null }; + saveEditorPosition = () => { + if (this.selectionMask) { + const { left, top } = this.selectionMask.getBoundingClientRect(); + const { scrollLeft, scrollTop } = document.documentElement; + this.editorPosition = { left: left + scrollLeft, top: top + scrollTop }; + } + }; + componentDidUpdate(prevProps, prevState) { const { selectedPosition, isEditorEnabled } = this.state; const { selectedPosition: prevSelectedPosition, isEditorEnabled: prevIsEditorEnabled } = prevState; @@ -115,6 +123,7 @@ class InteractionMasks extends React.Component { if ((isSelectedPositionChanged && this.isCellWithinBounds(selectedPosition)) || isEditorClosed) { this.focus(); + this.saveEditorPosition(); } } @@ -645,7 +654,7 @@ class InteractionMasks extends React.Component { }; render() { - const { rowGetter, contextMenu, rowHeight, getSelectedRowColumns } = this.props; + const { rowGetter, contextMenu, rowHeight, getSelectedRowColumns, scrollLeft, scrollTop } = this.props; const { isEditorEnabled, firstEditorKeyPress, selectedPosition, draggedPosition, copiedPosition } = this.state; const rowData = getSelectedRow({ selectedPosition, rowGetter }); const columns = getSelectedRowColumns(selectedPosition.rowIdx); @@ -680,8 +689,9 @@ class InteractionMasks extends React.Component { value={getSelectedCellValue({ selectedPosition, columns, rowGetter })} rowData={rowData} column={getSelectedColumn({ selectedPosition, columns })} - scrollLeft={this.props.scrollLeft} - scrollTop={this.props.scrollTop} + scrollLeft={scrollLeft} + scrollTop={scrollTop} + position={this.editorPosition} {...getSelectedDimensions({ selectedPosition, rowHeight, columns })} />} {isValidElement(contextMenu) && cloneElement(contextMenu, { ...selectedPosition })} diff --git a/packages/react-data-grid/src/utils/SelectedCellUtils.js b/packages/react-data-grid/src/utils/SelectedCellUtils.js index 483a844bca..b2d9856666 100644 --- a/packages/react-data-grid/src/utils/SelectedCellUtils.js +++ b/packages/react-data-grid/src/utils/SelectedCellUtils.js @@ -17,7 +17,7 @@ export const getSelectedDimensions = ({ selectedPosition, columns, rowHeight }) const column = columnUtils.getColumn(columns, idx); const { width, left } = column; const top = getRowTop(rowIdx, rowHeight); - const zIndex = columnUtils.isFrozen(columns) ? zIndexes.FROZEN_CELL_MASK : zIndexes.CELL_MASK; + const zIndex = columnUtils.isFrozen(column) ? zIndexes.FROZEN_CELL_MASK : zIndexes.CELL_MASK; return { width, left, top, height: rowHeight, zIndex }; } return { width: 0, left: 0, top: 0, height: rowHeight, zIndex: 1 }; From e9c97bab673eb020a1611bb23b6a2fb2f14507b4 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 7 Nov 2018 08:10:03 -0600 Subject: [PATCH 02/14] Delete obsolete tests --- .../editors/__tests__/EditorContainer.spec.js | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/packages/common/editors/__tests__/EditorContainer.spec.js b/packages/common/editors/__tests__/EditorContainer.spec.js index 567898627f..2d43e1898b 100644 --- a/packages/common/editors/__tests__/EditorContainer.spec.js +++ b/packages/common/editors/__tests__/EditorContainer.spec.js @@ -77,31 +77,6 @@ describe('Editor Container Tests', () => { expect(editorDiv.props().onKeyDown).toBeDefined(); expect(editorDiv.props().children).toBeDefined(); }); - - describe('Frozen columns', () => { - const frozenProps = { - column: { ...fakeColumn, frozen: true }, - left: 60, - top: 0, - scrollTop: 0, - scrollLeft: 250 - }; - - xit('should not subtract scrollLeft value from editors left position when column is frozen', () => { - const { shallowWrapper } = getComponent(frozenProps); - const editorDiv = shallowWrapper.find('div').at(0); - expect(editorDiv.props().style.transform).toBe('translate(60px, 0px)'); - }); - - xit('should subtract scrollLeft value from editors left position when column is not frozen', () => { - const unfrozenProps = { ...frozenProps }; - unfrozenProps.column.frozen = false; - - const { shallowWrapper } = getComponent(unfrozenProps); - const editorDiv = shallowWrapper.find('div').at(0); - expect(editorDiv.props().style.transform).toBe('translate(-190px, 0px)'); - }); - }); }); describe('Custom Editors', () => { From 91e2cc25466b98a42f09c3c67ea5f4d5a0d557f4 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 7 Nov 2018 13:36:03 -0600 Subject: [PATCH 03/14] Fix selection/copy/drag masks positions on frozen columns --- packages/react-data-grid/src/Canvas.js | 15 +++++++-- .../react-data-grid/src/masks/CopyMask.js | 9 ++++-- .../react-data-grid/src/masks/DragMask.js | 9 +++--- .../src/masks/InteractionMasks.js | 32 +++++++++++++++++++ .../src/masks/SelectionMask.js | 19 ++--------- .../src/utils/SelectedCellUtils.js | 10 +++--- 6 files changed, 63 insertions(+), 31 deletions(-) diff --git a/packages/react-data-grid/src/Canvas.js b/packages/react-data-grid/src/Canvas.js index b9bd0fa41b..c4bce71101 100644 --- a/packages/react-data-grid/src/Canvas.js +++ b/packages/react-data-grid/src/Canvas.js @@ -218,9 +218,13 @@ class Canvas extends React.PureComponent { }; setScrollLeft = (scrollLeft) => { + if (this.interactionMasks && this.interactionMasks.setScrollLeft) { + this.interactionMasks.setScrollLeft(scrollLeft); + } + this.rows.forEach((r, idx) => { if (r) { - let row = this.getRowByRef(idx); + const row = this.getRowByRef(idx); if (row && row.setScrollLeft) { row.setScrollLeft(scrollLeft); } @@ -260,14 +264,18 @@ class Canvas extends React.PureComponent { return row && row.props ? row.props.columns : this.props.columns; } - setCanvasRef = (canvas) => { - this.canvas = canvas; + setCanvasRef = el => { + this.canvas = el; }; setRowRef = idx => row => { this.rows[idx] = row; }; + setInteractionMasksRef = el => { + this.interactionMasks = el; + }; + renderCustomRowRenderer(props) { const { ref, ...otherProps } = props; const CustomRowRenderer = this.props.rowRenderer; @@ -382,6 +390,7 @@ class Canvas extends React.PureComponent { onScroll={this.onScroll} className="react-grid-Canvas"> ); } @@ -17,7 +18,9 @@ function CopyMask({ copiedPosition, columns, rowHeight }) { CopyMask.propTypes = { copiedPosition: PropTypes.object.isRequired, columns: PropTypes.array.isRequired, - rowHeight: PropTypes.number.isRequired + rowHeight: PropTypes.number.isRequired, + scrollLeft: PropTypes.number.isRequired, + innerRef: PropTypes.func.isRequired }; export default CopyMask; diff --git a/packages/react-data-grid/src/masks/DragMask.js b/packages/react-data-grid/src/masks/DragMask.js index 8ce16bce95..38a5063d8c 100644 --- a/packages/react-data-grid/src/masks/DragMask.js +++ b/packages/react-data-grid/src/masks/DragMask.js @@ -4,7 +4,7 @@ import PropTypes from 'prop-types'; import { getSelectedDimensions } from '../utils/SelectedCellUtils'; import CellMask from './CellMask'; -function DragMask({ draggedPosition, columns, rowHeight }) { +function DragMask({ draggedPosition, ...rest }) { const { overRowIdx, idx, rowIdx } = draggedPosition; if (overRowIdx != null && rowIdx !== overRowIdx) { const isDraggedOverDown = rowIdx < overRowIdx; @@ -12,9 +12,9 @@ function DragMask({ draggedPosition, columns, rowHeight }) { const endRowIdx = isDraggedOverDown ? overRowIdx : rowIdx - 1; const className = isDraggedOverDown ? 'react-grid-cell-dragged-over-down' : 'react-grid-cell-dragged-over-up'; - const dimensions = getSelectedDimensions({ selectedPosition: { idx, rowIdx: startRowIdx }, columns, rowHeight }); + const dimensions = getSelectedDimensions({ selectedPosition: { idx, rowIdx: startRowIdx }, ...rest }); for (let currentRowIdx = startRowIdx + 1; currentRowIdx <= endRowIdx; currentRowIdx++) { - const { height } = getSelectedDimensions({ selectedPosition: { idx, rowIdx: currentRowIdx }, columns, rowHeight }); + const { height } = getSelectedDimensions({ selectedPosition: { idx, rowIdx: currentRowIdx }, ...rest }); dimensions.height += height; } @@ -31,7 +31,8 @@ function DragMask({ draggedPosition, columns, rowHeight }) { DragMask.propTypes = { draggedPosition: PropTypes.object.isRequired, columns: PropTypes.array.isRequired, - rowHeight: PropTypes.number.isRequired + rowHeight: PropTypes.number.isRequired, + scrollLeft: PropTypes.number.isRequired }; export default DragMask; diff --git a/packages/react-data-grid/src/masks/InteractionMasks.js b/packages/react-data-grid/src/masks/InteractionMasks.js index 1ee4f2448c..35732ffb6a 100644 --- a/packages/react-data-grid/src/masks/InteractionMasks.js +++ b/packages/react-data-grid/src/masks/InteractionMasks.js @@ -103,6 +103,31 @@ class InteractionMasks extends React.Component { } }; + setMaskScollLeft = (mask, position, scrollLeft) => { + if (mask) { + const { idx, rowIdx } = position; + if (idx >= 0 && rowIdx >= 0) { + const { columns, getSelectedRowTop } = this.props; + const column = columnUtils.getColumn(columns, idx); + const frozen = columnUtils.isFrozen(column); + if (frozen) { + const top = getSelectedRowTop(rowIdx); + const left = scrollLeft + column.left; + const transform = `translate(${left}px, ${top}px)`; + if (mask.style.transform !== transform) { + mask.style.transform = transform; + } + } + } + } + }; + + setScrollLeft = (scrollLeft) => { + const { selectionMask, copyMask, state: { selectedPosition, copiedPosition } } = this; + this.setMaskScollLeft(selectionMask, selectedPosition, scrollLeft); + this.setMaskScollLeft(copyMask, copiedPosition, scrollLeft); + }; + componentDidUpdate(prevProps, prevState) { const { selectedPosition, isEditorEnabled } = this.state; const { selectedPosition: prevSelectedPosition, isEditorEnabled: prevIsEditorEnabled } = prevState; @@ -598,6 +623,10 @@ class InteractionMasks extends React.Component { this.selectionMask = node; }; + setCopyMaskRef = (node) => { + this.copyMask = node; + }; + getSelectionMaskProps = () => { const { columns, getSelectedRowHeight, getSelectedRowTop, scrollLeft, scrollTop, prevScrollLeft, prevScrollTop } = this.props; const { prevSelectedPosition } = this.state; @@ -667,6 +696,8 @@ class InteractionMasks extends React.Component { )} @@ -674,6 +705,7 @@ class InteractionMasks extends React.Component { )} diff --git a/packages/react-data-grid/src/masks/SelectionMask.js b/packages/react-data-grid/src/masks/SelectionMask.js index 0d5271e3e8..a9ed28949a 100644 --- a/packages/react-data-grid/src/masks/SelectionMask.js +++ b/packages/react-data-grid/src/masks/SelectionMask.js @@ -6,18 +6,6 @@ import zIndexes from 'common/constants/zIndexes'; const isFrozenColumn = (columns, { idx }) => columnUtils.isFrozen(columnUtils.getColumn(columns, idx)); -const isScrollingHorizontallyWithoutCellChange = ({ scrollTop, prevScrollTop, scrollLeft, prevScrollLeft, selectedPosition, prevSelectedPosition }) => { - return scrollLeft !== prevScrollLeft && (scrollTop === prevScrollTop) && selectedPosition.idx === prevSelectedPosition.idx; -}; - -const getLeftPosition = (isFrozen, cellLeft, props) => { - if (isFrozen && !isScrollingHorizontallyWithoutCellChange(props)) { - return props.scrollLeft + cellLeft; - } - return cellLeft; -}; - - export const getCellMaskDimensions = (props) => { const { selectedPosition, columns, getSelectedRowHeight, getSelectedRowTop } = props; const column = columnUtils.getColumn(columns, selectedPosition.idx); @@ -25,19 +13,16 @@ export const getCellMaskDimensions = (props) => { const top = getSelectedRowTop(selectedPosition.rowIdx); const frozen = isFrozenColumn(columns, selectedPosition); const zIndex = frozen ? zIndexes.FROZEN_CELL_MASK : zIndexes.CELL_MASK; - const left = getLeftPosition(frozen, column.left, props); + const left = frozen ? column.left + props.scrollLeft : column.left; return { height, top, width: column.width, left, zIndex }; }; function SelectionMask({ children, innerRef, ...rest }) { const dimensions = getCellMaskDimensions(rest); - const frozen = isFrozenColumn(rest.columns, rest.selectedPosition); - const position = frozen && isScrollingHorizontallyWithoutCellChange(rest) ? 'fixed' : 'absolute'; return ( @@ -49,7 +34,7 @@ function SelectionMask({ children, innerRef, ...rest }) { SelectionMask.propTypes = { selectedPosition: PropTypes.object.isRequired, columns: PropTypes.array.isRequired, - innerRef: PropTypes.func + innerRef: PropTypes.func.isRequired }; export default SelectionMask; diff --git a/packages/react-data-grid/src/utils/SelectedCellUtils.js b/packages/react-data-grid/src/utils/SelectedCellUtils.js index b2d9856666..814b228874 100644 --- a/packages/react-data-grid/src/utils/SelectedCellUtils.js +++ b/packages/react-data-grid/src/utils/SelectedCellUtils.js @@ -1,5 +1,5 @@ import { CellNavigationMode } from 'common/constants'; -import {isFunction} from 'common/utils'; +import { isFunction } from 'common/utils'; import * as rowUtils from '../RowUtils'; import * as columnUtils from '../ColumnUtils'; import zIndexes from 'common/constants/zIndexes'; @@ -11,13 +11,15 @@ export const getSelectedRow = ({ selectedPosition, rowGetter }) => { return rowGetter(rowIdx); }; -export const getSelectedDimensions = ({ selectedPosition, columns, rowHeight }) => { +export const getSelectedDimensions = ({ selectedPosition, columns, rowHeight, scrollLeft }) => { const { idx, rowIdx } = selectedPosition; if (idx >= 0) { const column = columnUtils.getColumn(columns, idx); - const { width, left } = column; + const isFrozen = columnUtils.isFrozen(column); + const { width } = column; + const left = isFrozen ? column.left + scrollLeft : column.left; const top = getRowTop(rowIdx, rowHeight); - const zIndex = columnUtils.isFrozen(column) ? zIndexes.FROZEN_CELL_MASK : zIndexes.CELL_MASK; + const zIndex = isFrozen ? zIndexes.FROZEN_CELL_MASK : zIndexes.CELL_MASK; return { width, left, top, height: rowHeight, zIndex }; } return { width: 0, left: 0, top: 0, height: rowHeight, zIndex: 1 }; From 6488ef14eb31e1b6b2767a53091c6c9fd39553af Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 7 Nov 2018 14:47:17 -0600 Subject: [PATCH 04/14] Simplify selectionMask position logic Remove unused props --- packages/react-data-grid/src/Canvas.js | 12 ------------ packages/react-data-grid/src/Viewport.js | 4 +--- .../src/masks/InteractionMasks.js | 16 +++------------- .../src/masks/SelectionMask.js | 19 +++---------------- 4 files changed, 7 insertions(+), 44 deletions(-) diff --git a/packages/react-data-grid/src/Canvas.js b/packages/react-data-grid/src/Canvas.js index c4bce71101..50119c5412 100644 --- a/packages/react-data-grid/src/Canvas.js +++ b/packages/react-data-grid/src/Canvas.js @@ -250,15 +250,6 @@ class Canvas extends React.PureComponent { return this.props.rowHeight * rowIdx; } - getSelectedRowHeight = (rowIdx) => { - const row = this.getRowByRef(rowIdx); - if (row) { - const node = ReactDOM.findDOMNode(row); - return node && node.clientHeight > 0 ? node.clientHeight : this.props.rowHeight; - } - return this.props.rowHeight; - } - getSelectedRowColumns = (rowIdx) => { const row = this.getRowByRef(rowIdx); return row && row.props ? row.props.columns : this.props.columns; @@ -423,9 +414,6 @@ class Canvas extends React.PureComponent { onCellRangeSelectionCompleted={this.props.onCellRangeSelectionCompleted} scrollLeft={this._scroll.scrollLeft} scrollTop={this._scroll.scrollTop} - prevScrollLeft={this.props.prevScrollLeft} - prevScrollTop={this.props.prevScrollTop} - getSelectedRowHeight={this.getSelectedRowHeight} getSelectedRowTop={this.getSelectedRowTop} getSelectedRowColumns={this.getSelectedRowColumns} /> diff --git a/packages/react-data-grid/src/Viewport.js b/packages/react-data-grid/src/Viewport.js index 57c6e2f6de..d5333c8233 100644 --- a/packages/react-data-grid/src/Viewport.js +++ b/packages/react-data-grid/src/Viewport.js @@ -137,9 +137,7 @@ class Viewport extends React.Component { colOverscanEndIdx, scrollDirection, lastFrozenColumnIndex, - isScrolling, - prevScrollTop: this.state.scrollTop, - prevScrollLeft: this.state.scrollTop + isScrolling }; } diff --git a/packages/react-data-grid/src/masks/InteractionMasks.js b/packages/react-data-grid/src/masks/InteractionMasks.js index 35732ffb6a..060c96e129 100644 --- a/packages/react-data-grid/src/masks/InteractionMasks.js +++ b/packages/react-data-grid/src/masks/InteractionMasks.js @@ -66,11 +66,8 @@ class InteractionMasks extends React.Component { onCellsDragged: PropTypes.func, onDragHandleDoubleClick: PropTypes.func.isRequired, scrollLeft: PropTypes.number.isRequired, - prevScrollLeft: PropTypes.number.isRequired, scrollTop: PropTypes.number.isRequired, - prevScrollTop: PropTypes.number.isRequired, rows: PropTypes.array.isRequired, - getSelectedRowHeight: PropTypes.func.isRequired, getSelectedRowTop: PropTypes.func.isRequired, getSelectedRowColumns: PropTypes.func.isRequired }; @@ -459,7 +456,6 @@ class InteractionMasks extends React.Component { if (this.isCellWithinBounds(next)) { return { selectedPosition: next, - prevSelectedPosition: cell, selectedRange: { topLeft: next, bottomRight: next, @@ -628,18 +624,12 @@ class InteractionMasks extends React.Component { }; getSelectionMaskProps = () => { - const { columns, getSelectedRowHeight, getSelectedRowTop, scrollLeft, scrollTop, prevScrollLeft, prevScrollTop } = this.props; - const { prevSelectedPosition } = this.state; + const { columns, scrollLeft, rowHeight } = this.props; return { columns, - scrollTop, scrollLeft, - getSelectedRowHeight, - getSelectedRowTop, - prevScrollLeft, - prevScrollTop, - prevSelectedPosition, + rowHeight, isGroupedRow: this.isGroupedRowSelected(), innerRef: this.setSelectionMaskRef }; @@ -724,7 +714,7 @@ class InteractionMasks extends React.Component { scrollLeft={scrollLeft} scrollTop={scrollTop} position={this.editorPosition} - {...getSelectedDimensions({ selectedPosition, rowHeight, columns })} + {...getSelectedDimensions({ selectedPosition, rowHeight, columns, scrollLeft })} />} {isValidElement(contextMenu) && cloneElement(contextMenu, { ...selectedPosition })}
diff --git a/packages/react-data-grid/src/masks/SelectionMask.js b/packages/react-data-grid/src/masks/SelectionMask.js index a9ed28949a..8518082907 100644 --- a/packages/react-data-grid/src/masks/SelectionMask.js +++ b/packages/react-data-grid/src/masks/SelectionMask.js @@ -1,24 +1,11 @@ import React from 'react'; import PropTypes from 'prop-types'; -import CellMask from './CellMask'; -import * as columnUtils from '../ColumnUtils'; -import zIndexes from 'common/constants/zIndexes'; - -const isFrozenColumn = (columns, { idx }) => columnUtils.isFrozen(columnUtils.getColumn(columns, idx)); -export const getCellMaskDimensions = (props) => { - const { selectedPosition, columns, getSelectedRowHeight, getSelectedRowTop } = props; - const column = columnUtils.getColumn(columns, selectedPosition.idx); - const height = getSelectedRowHeight(selectedPosition.rowIdx); - const top = getSelectedRowTop(selectedPosition.rowIdx); - const frozen = isFrozenColumn(columns, selectedPosition); - const zIndex = frozen ? zIndexes.FROZEN_CELL_MASK : zIndexes.CELL_MASK; - const left = frozen ? column.left + props.scrollLeft : column.left; - return { height, top, width: column.width, left, zIndex }; -}; +import CellMask from './CellMask'; +import { getSelectedDimensions } from '../utils/SelectedCellUtils'; function SelectionMask({ children, innerRef, ...rest }) { - const dimensions = getCellMaskDimensions(rest); + const dimensions = getSelectedDimensions(rest); return ( Date: Wed, 7 Nov 2018 15:08:27 -0600 Subject: [PATCH 05/14] Fix unit tests --- packages/common/editors/__tests__/EditorContainer.spec.js | 4 +++- packages/react-data-grid/src/__tests__/Viewport.spec.js | 4 ---- .../src/masks/__tests__/SelectionMask.spec.js | 6 ++---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/common/editors/__tests__/EditorContainer.spec.js b/packages/common/editors/__tests__/EditorContainer.spec.js index 2d43e1898b..b685746f36 100644 --- a/packages/common/editors/__tests__/EditorContainer.spec.js +++ b/packages/common/editors/__tests__/EditorContainer.spec.js @@ -20,7 +20,9 @@ const defaultProps = { }, column: fakeColumn, value: 'Adwolf', - height: 50 + height: 50, + onCommit: jasmine.createSpy(), + onCommitCancel: jasmine.createSpy() }; const getComponent = (props) => { diff --git a/packages/react-data-grid/src/__tests__/Viewport.spec.js b/packages/react-data-grid/src/__tests__/Viewport.spec.js index d2599f9ea1..72a6790a99 100644 --- a/packages/react-data-grid/src/__tests__/Viewport.spec.js +++ b/packages/react-data-grid/src/__tests__/Viewport.spec.js @@ -96,8 +96,6 @@ describe('', () => { height: viewportProps.minHeight, scrollLeft: scrollLeft, scrollTop: scrollTop, - prevScrollTop: 0, - prevScrollLeft: 0, rowVisibleEndIdx: 21, rowVisibleStartIdx: 6, isScrolling: true, @@ -153,8 +151,6 @@ describe('', () => { height: newHeight, scrollLeft: 0, scrollTop: 0, - prevScrollTop: 0, - prevScrollLeft: 0, rowVisibleEndIdx: 29, rowVisibleStartIdx: 0, isScrolling: true, diff --git a/packages/react-data-grid/src/masks/__tests__/SelectionMask.spec.js b/packages/react-data-grid/src/masks/__tests__/SelectionMask.spec.js index 962fdf4d7d..632aa0225c 100644 --- a/packages/react-data-grid/src/masks/__tests__/SelectionMask.spec.js +++ b/packages/react-data-grid/src/masks/__tests__/SelectionMask.spec.js @@ -6,7 +6,6 @@ import SelectionMask from '../SelectionMask'; import zIndexes from 'common/constants/zIndexes'; describe('SelectionMask', () => { - const TOP = 45; const ROW_HEIGHT = 50; const setup = (propsOverride = {}) => { const props = { @@ -14,8 +13,7 @@ describe('SelectionMask', () => { columns: [ { width: 50, left: 5 } ], - getSelectedRowHeight: () => ROW_HEIGHT, - getSelectedRowTop: () => TOP, + rowHeight: ROW_HEIGHT, isGroupedRow: false, scrollLeft: 0, ...propsOverride @@ -33,7 +31,7 @@ describe('SelectionMask', () => { height: ROW_HEIGHT, width: 50, left: 5, - top: TOP, + top: 150, zIndex: zIndexes.CELL_MASK, className: 'rdg-selected', children: undefined From 55df83b84f70eda91a597ac3651eeb988261da13 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 7 Nov 2018 16:47:58 -0600 Subject: [PATCH 06/14] Remove getSelectedRowTop --- packages/react-data-grid/src/Canvas.js | 12 +----------- .../react-data-grid/src/masks/InteractionMasks.js | 6 +++--- .../src/masks/__tests__/InteractionMasks.spec.js | 2 -- .../react-data-grid/src/utils/SelectedCellUtils.js | 2 +- 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/packages/react-data-grid/src/Canvas.js b/packages/react-data-grid/src/Canvas.js index 50119c5412..487457855d 100644 --- a/packages/react-data-grid/src/Canvas.js +++ b/packages/react-data-grid/src/Canvas.js @@ -1,6 +1,6 @@ import React from 'react'; -import ReactDOM from 'react-dom'; import PropTypes from 'prop-types'; + import Row from './Row'; import cellMetaDataShape from 'common/prop-shapes/CellActionShape'; import * as rowUtils from './RowUtils'; @@ -241,15 +241,6 @@ class Canvas extends React.PureComponent { return this.rows[i]; }; - getSelectedRowTop = (rowIdx) => { - const row = this.getRowByRef(rowIdx); - if (row) { - const node = ReactDOM.findDOMNode(row); - return node && node.offsetTop; - } - return this.props.rowHeight * rowIdx; - } - getSelectedRowColumns = (rowIdx) => { const row = this.getRowByRef(rowIdx); return row && row.props ? row.props.columns : this.props.columns; @@ -414,7 +405,6 @@ class Canvas extends React.PureComponent { onCellRangeSelectionCompleted={this.props.onCellRangeSelectionCompleted} scrollLeft={this._scroll.scrollLeft} scrollTop={this._scroll.scrollTop} - getSelectedRowTop={this.getSelectedRowTop} getSelectedRowColumns={this.getSelectedRowColumns} /> diff --git a/packages/react-data-grid/src/masks/InteractionMasks.js b/packages/react-data-grid/src/masks/InteractionMasks.js index 060c96e129..0cdb7ed9f9 100644 --- a/packages/react-data-grid/src/masks/InteractionMasks.js +++ b/packages/react-data-grid/src/masks/InteractionMasks.js @@ -10,6 +10,7 @@ import EditorContainer from 'common/editors/EditorContainer'; import { UpdateActions } from 'common/constants'; import { isKeyPrintable, isCtrlKeyHeldDown } from 'common/utils/keyboardUtils'; import { + getRowTop, getSelectedDimensions, getSelectedCellValue, getSelectedRow, @@ -68,7 +69,6 @@ class InteractionMasks extends React.Component { scrollLeft: PropTypes.number.isRequired, scrollTop: PropTypes.number.isRequired, rows: PropTypes.array.isRequired, - getSelectedRowTop: PropTypes.func.isRequired, getSelectedRowColumns: PropTypes.func.isRequired }; @@ -104,11 +104,11 @@ class InteractionMasks extends React.Component { if (mask) { const { idx, rowIdx } = position; if (idx >= 0 && rowIdx >= 0) { - const { columns, getSelectedRowTop } = this.props; + const { columns, rowHeight } = this.props; const column = columnUtils.getColumn(columns, idx); const frozen = columnUtils.isFrozen(column); if (frozen) { - const top = getSelectedRowTop(rowIdx); + const top = getRowTop(rowIdx, rowHeight); const left = scrollLeft + column.left; const transform = `translate(${left}px, ${top}px)`; if (mask.style.transform !== transform) { diff --git a/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js b/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js index 90a088ee90..5596cf04ba 100644 --- a/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js +++ b/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js @@ -47,8 +47,6 @@ describe('', () => { enableCellSelect: true, cellNavigationMode: CellNavigationMode.NONE, eventBus, - getSelectedRowHeight: () => 50, - getSelectedRowTop: () => 0, getSelectedRowColumns: jasmine.createSpy().and.callFake(() => columns), ...overrideProps }; diff --git a/packages/react-data-grid/src/utils/SelectedCellUtils.js b/packages/react-data-grid/src/utils/SelectedCellUtils.js index 814b228874..fd5c42895f 100644 --- a/packages/react-data-grid/src/utils/SelectedCellUtils.js +++ b/packages/react-data-grid/src/utils/SelectedCellUtils.js @@ -4,7 +4,7 @@ import * as rowUtils from '../RowUtils'; import * as columnUtils from '../ColumnUtils'; import zIndexes from 'common/constants/zIndexes'; -const getRowTop = (rowIdx, rowHeight) => rowIdx * rowHeight; +export const getRowTop = (rowIdx, rowHeight) => rowIdx * rowHeight; export const getSelectedRow = ({ selectedPosition, rowGetter }) => { const { rowIdx } = selectedPosition; From 157aff037473cbe90eaca43cad5a0dbf62a34589 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 7 Nov 2018 22:16:22 -0600 Subject: [PATCH 07/14] Fix unit tests --- packages/common/editors/__tests__/EditorContainer.spec.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/common/editors/__tests__/EditorContainer.spec.js b/packages/common/editors/__tests__/EditorContainer.spec.js index b685746f36..072dfc1d05 100644 --- a/packages/common/editors/__tests__/EditorContainer.spec.js +++ b/packages/common/editors/__tests__/EditorContainer.spec.js @@ -169,10 +169,7 @@ describe('Editor Container Tests', () => { beforeEach(() => { container = document.createElement('div'); document.body.appendChild(container); - component = mount(, { attachTo: container }); + component = mount(, { attachTo: container }); }); afterEach(() => { @@ -192,6 +189,7 @@ describe('Editor Container Tests', () => { const editor = component.find(SimpleTextEditor); editor.simulate('keydown', { key: 'Escape' }); + component.detach(); expect(component.props().onCommitCancel).toHaveBeenCalled(); expect(component.props().onCommitCancel.calls.count()).toEqual(1); From 111f2c2fe907438ae5cf6f7be747b26c88e0321e Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Wed, 7 Nov 2018 22:55:26 -0600 Subject: [PATCH 08/14] Fix unit tests in FF --- packages/common/editors/__tests__/EditorContainer.spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/common/editors/__tests__/EditorContainer.spec.js b/packages/common/editors/__tests__/EditorContainer.spec.js index 072dfc1d05..04ca7353c7 100644 --- a/packages/common/editors/__tests__/EditorContainer.spec.js +++ b/packages/common/editors/__tests__/EditorContainer.spec.js @@ -167,6 +167,8 @@ describe('Editor Container Tests', () => { let container; beforeEach(() => { + defaultProps.onCommit.calls.reset(); + defaultProps.onCommitCancel.calls.reset(); container = document.createElement('div'); document.body.appendChild(container); component = mount(, { attachTo: container }); From 89b11437827c2e1865dff32ecaa32ade4bbd1f18 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 15 Nov 2018 12:13:37 -0600 Subject: [PATCH 09/14] Remove position prop in favor of left/top props --- packages/common/editors/EditorContainer.js | 11 ++++++----- .../react-data-grid/src/masks/InteractionMasks.js | 10 ++++++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/common/editors/EditorContainer.js b/packages/common/editors/EditorContainer.js index e21c427a62..34cc269024 100644 --- a/packages/common/editors/EditorContainer.js +++ b/packages/common/editors/EditorContainer.js @@ -19,15 +19,16 @@ class EditorContainer extends React.Component { rowData: PropTypes.object.isRequired, value: PropTypes.oneOfType([PropTypes.string, PropTypes.number, PropTypes.object, PropTypes.bool]).isRequired, column: PropTypes.object.isRequired, + width: PropTypes.number.isRequired, height: PropTypes.number.isRequired, + left: PropTypes.number.isRequired, + top: PropTypes.number.isRequired, onGridKeyDown: PropTypes.func, onCommit: PropTypes.func, onCommitCancel: PropTypes.func, firstEditorKeyPress: PropTypes.string, - width: PropTypes.number.isRequired, scrollLeft: PropTypes.number, - scrollTop: PropTypes.number, - position: PropTypes.object.isRequired + scrollTop: PropTypes.number }; state = { isInvalid: false }; @@ -335,9 +336,9 @@ class EditorContainer extends React.Component { }; render() { - const { width, height, column, position } = this.props; + const { width, height, left, top, column } = this.props; const zIndex = isFrozen(column) ? zIndexes.FROZEN_EDITOR_CONTAINER : zIndexes.EDITOR_CONTAINER; - const style = { position: 'absolute', height, width, zIndex, ...position }; + const style = { position: 'absolute', height, width, left, top, zIndex }; return (
{ this.onSelectCellRangeEnded(); }); + this.onSelectCellRangeUpdated({ ...next }, true, () => { this.onSelectCellRangeEnded(); }); } getNextSelectedCellPositionForKeyNavAction(keyNavAction, currentPosition, cellNavigationMode) { @@ -709,8 +709,10 @@ class InteractionMasks extends React.Component { column={getSelectedColumn({ selectedPosition, columns })} scrollLeft={scrollLeft} scrollTop={scrollTop} - position={this.editorPosition} - {...getSelectedDimensions({ selectedPosition, rowHeight, columns, scrollLeft })} + {...{ + ...getSelectedDimensions({ selectedPosition, rowHeight, columns, scrollLeft }), + ...this.editorPosition + }} />} {isValidElement(contextMenu) && cloneElement(contextMenu, { ...selectedPosition })}
From 86b742374dcd49083255c2a0ff9dbbabfb75c3e3 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 15 Nov 2018 12:18:21 -0600 Subject: [PATCH 10/14] MInor cleanup --- .../src/masks/InteractionMasks.js | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/react-data-grid/src/masks/InteractionMasks.js b/packages/react-data-grid/src/masks/InteractionMasks.js index d00d2a2014..cf69aa67ee 100644 --- a/packages/react-data-grid/src/masks/InteractionMasks.js +++ b/packages/react-data-grid/src/masks/InteractionMasks.js @@ -699,21 +699,23 @@ class InteractionMasks extends React.Component { this.getSingleCellSelectView() : this.getCellRangeSelectView() } - {isEditorEnabled && } + {isEditorEnabled && ( + + )} {isValidElement(contextMenu) && cloneElement(contextMenu, { ...selectedPosition })} ); From d62f89a56f08d52fe40cb1c65b6b3590c3d78941 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 15 Nov 2018 13:01:28 -0600 Subject: [PATCH 11/14] Remove fdescribe and fix unit tests --- packages/react-data-grid/src/ColumnMetrics.js | 2 +- packages/react-data-grid/src/Header.js | 2 +- packages/react-data-grid/src/HeaderRow.js | 2 +- packages/react-data-grid/src/__tests__/Canvas.spec.js | 2 +- packages/react-data-grid/src/__tests__/Header.spec.js | 5 +++++ packages/react-data-grid/src/getScrollbarSize.js | 4 +--- 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/react-data-grid/src/ColumnMetrics.js b/packages/react-data-grid/src/ColumnMetrics.js index e56b7f45e3..8767e1a819 100644 --- a/packages/react-data-grid/src/ColumnMetrics.js +++ b/packages/react-data-grid/src/ColumnMetrics.js @@ -1,7 +1,7 @@ const shallowCloneObject = require('./shallowCloneObject'); const sameColumn = require('./ColumnComparer'); const ColumnUtils = require('./ColumnUtils'); -const getScrollbarSize = require('./getScrollbarSize'); +import getScrollbarSize from './getScrollbarSize'; import {isColumnsImmutable} from 'common/utils'; function setColumnWidths(columns, totalWidth) { diff --git a/packages/react-data-grid/src/Header.js b/packages/react-data-grid/src/Header.js index 7ccf93bfa0..ede966ce4b 100644 --- a/packages/react-data-grid/src/Header.js +++ b/packages/react-data-grid/src/Header.js @@ -5,7 +5,7 @@ const shallowCloneObject = require('./shallowCloneObject'); const ColumnMetrics = require('./ColumnMetrics'); const ColumnUtils = require('./ColumnUtils'); const HeaderRow = require('./HeaderRow'); -const getScrollbarSize = require('./getScrollbarSize'); +import getScrollbarSize from './getScrollbarSize'; import PropTypes from 'prop-types'; const createObjectWithProperties = require('./createObjectWithProperties'); const cellMetaDataShape = require('common/prop-shapes/CellMetaDataShape'); diff --git a/packages/react-data-grid/src/HeaderRow.js b/packages/react-data-grid/src/HeaderRow.js index e31a38ac0c..e9b359cc77 100644 --- a/packages/react-data-grid/src/HeaderRow.js +++ b/packages/react-data-grid/src/HeaderRow.js @@ -1,7 +1,7 @@ const React = require('react'); const shallowEqual = require('shallowequal'); const BaseHeaderCell = require('./HeaderCell'); -const getScrollbarSize = require('./getScrollbarSize'); +import getScrollbarSize from './getScrollbarSize'; const columnUtils = require('./ColumnUtils'); const SortableHeaderCell = require('common/cells/headerCells/SortableHeaderCell'); const FilterableHeaderCell = require('common/cells/headerCells/FilterableHeaderCell'); diff --git a/packages/react-data-grid/src/__tests__/Canvas.spec.js b/packages/react-data-grid/src/__tests__/Canvas.spec.js index a0db49810e..143fc63efc 100644 --- a/packages/react-data-grid/src/__tests__/Canvas.spec.js +++ b/packages/react-data-grid/src/__tests__/Canvas.spec.js @@ -53,7 +53,7 @@ const renderComponent = (extraProps) => { return shallow(); }; -fdescribe('Canvas Tests', () => { +describe('Canvas Tests', () => { let wrapper; let testElementNode; diff --git a/packages/react-data-grid/src/__tests__/Header.spec.js b/packages/react-data-grid/src/__tests__/Header.spec.js index 8e3666ab43..6de479cae9 100644 --- a/packages/react-data-grid/src/__tests__/Header.spec.js +++ b/packages/react-data-grid/src/__tests__/Header.spec.js @@ -2,10 +2,15 @@ import React from 'react'; import Header from '../Header'; import HeaderRow from '../HeaderRow'; import helpers, {fakeCellMetaData} from '../helpers/test/GridPropHelpers'; +import * as GetScrollbarSize from '../getScrollbarSize'; import { shallow } from 'enzyme'; const SCROLL_BAR_SIZE = 17; describe('Header Unit Tests', () => { + beforeEach(() => { + spyOn(GetScrollbarSize, 'default').and.returnValue(SCROLL_BAR_SIZE); + }); + const testProps = { columnMetrics: { columns: helpers.columns, diff --git a/packages/react-data-grid/src/getScrollbarSize.js b/packages/react-data-grid/src/getScrollbarSize.js index 85f5a37401..7ac37f1add 100644 --- a/packages/react-data-grid/src/getScrollbarSize.js +++ b/packages/react-data-grid/src/getScrollbarSize.js @@ -1,6 +1,6 @@ let size; -function getScrollbarSize() { +export default function getScrollbarSize() { if (size === undefined) { let outer = document.createElement('div'); outer.style.width = '50px'; @@ -27,5 +27,3 @@ function getScrollbarSize() { return size; } - -module.exports = getScrollbarSize; From 4b93765b72bca8740ca3282a8d2a6d2f6324840b Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 15 Nov 2018 16:01:06 -0600 Subject: [PATCH 12/14] Remove empty event handlers --- packages/common/editors/EditorContainer.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/common/editors/EditorContainer.js b/packages/common/editors/EditorContainer.js index 34cc269024..720d5c0cb3 100644 --- a/packages/common/editors/EditorContainer.js +++ b/packages/common/editors/EditorContainer.js @@ -119,7 +119,15 @@ class EditorContainer extends React.Component { return ; } - return { }} commit={() => { }} />; + return ( + + ); }; onPressEnter = () => { From e18bf66479c3edda18ac3df21ef763339b4013d4 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Thu, 15 Nov 2018 16:06:42 -0600 Subject: [PATCH 13/14] Add some comments --- packages/react-data-grid/src/masks/InteractionMasks.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/react-data-grid/src/masks/InteractionMasks.js b/packages/react-data-grid/src/masks/InteractionMasks.js index cf69aa67ee..50c7150f0b 100644 --- a/packages/react-data-grid/src/masks/InteractionMasks.js +++ b/packages/react-data-grid/src/masks/InteractionMasks.js @@ -119,6 +119,11 @@ class InteractionMasks extends React.Component { } }; + /** + * Sets the position of SelectionMask and CopyMask components when the canvas is scrolled + * This is only required on the frozen columns + * @param scrollLeft number + */ setScrollLeft = (scrollLeft) => { const { selectionMask, copyMask, state: { selectedPosition, copiedPosition } } = this; this.setMaskScollLeft(selectionMask, selectedPosition, scrollLeft); From 367b5a8d8960ed478c68a32ecad3e11705a5e256 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Sun, 18 Nov 2018 22:08:59 -0600 Subject: [PATCH 14/14] - Fix row heights for copy and drag masks - Add back dynamic row logic --- packages/react-data-grid/src/Canvas.js | 63 ++++++--- packages/react-data-grid/src/Row.js | 15 +- .../react-data-grid/src/masks/CellMask.js | 8 +- .../react-data-grid/src/masks/CopyMask.js | 9 +- .../react-data-grid/src/masks/DragMask.js | 11 +- .../src/masks/InteractionMasks.js | 130 +++++++++--------- .../src/masks/SelectionMask.js | 7 +- .../src/masks/__tests__/CopyMask.spec.js | 15 +- .../src/masks/__tests__/DragMask.spec.js | 32 +++-- .../masks/__tests__/InteractionMasks.spec.js | 18 +-- .../src/masks/__tests__/SelectionMask.spec.js | 18 ++- 11 files changed, 173 insertions(+), 153 deletions(-) diff --git a/packages/react-data-grid/src/Canvas.js b/packages/react-data-grid/src/Canvas.js index 992098a182..ffef748ca1 100644 --- a/packages/react-data-grid/src/Canvas.js +++ b/packages/react-data-grid/src/Canvas.js @@ -235,17 +235,33 @@ class Canvas extends React.PureComponent { getRowByRef = (i) => { // check if wrapped with React DND drop target - let wrappedRow = this.rows[i] && this.rows[i].getDecoratedComponentInstance ? this.rows[i].getDecoratedComponentInstance(i) : null; + const wrappedRow = this.rows[i] && this.rows[i].getDecoratedComponentInstance ? this.rows[i].getDecoratedComponentInstance(i) : null; if (wrappedRow) { return wrappedRow.row; } return this.rows[i]; }; - getSelectedRowColumns = (rowIdx) => { + getRowTop = (rowIdx) => { + const row = this.getRowByRef(rowIdx); + if (row && isFunction(row.getRowTop)) { + return row.getRowTop(); + } + return this.props.rowHeight * rowIdx; + }; + + getRowHeight = (rowIdx) => { + const row = this.getRowByRef(rowIdx); + if (row && isFunction(row.getRowHeight)) { + return row.getRowHeight(); + } + return this.props.rowHeight; + }; + + getRowColumns = (rowIdx) => { const row = this.getRowByRef(rowIdx); return row && row.props ? row.props.columns : this.props.columns; - } + }; setCanvasRef = el => { this.canvas = el; @@ -277,19 +293,21 @@ class Canvas extends React.PureComponent { renderGroupRow(props) { const { ref, ...rowGroupProps } = props; - return ( } - />); + return ( + } + /> + ); } renderRow = (props) => { - let row = props.row; + const { row } = props; if (row.__metaData && row.__metaData.getRowRenderer) { return row.__metaData.getRowRenderer(this.props, props.idx); } @@ -306,13 +324,14 @@ class Canvas extends React.PureComponent { renderPlaceholder = (key, height) => { // just renders empty cells // if we wanted to show gridlines, we'd need classes and position as with renderScrollingPlaceholder - return (
- { - this.props.columns.map( - (column, idx) =>
- ) - } -
+ return ( +
+ { + this.props.columns.map( + (column, idx) =>
+ ) + } +
); }; @@ -406,7 +425,9 @@ class Canvas extends React.PureComponent { onCellRangeSelectionCompleted={this.props.onCellRangeSelectionCompleted} scrollLeft={this._scroll.scrollLeft} scrollTop={this._scroll.scrollTop} - getSelectedRowColumns={this.getSelectedRowColumns} + getRowHeight={this.getRowHeight} + getRowTop={this.getRowTop} + getRowColumns={this.getRowColumns} />
{rows}
diff --git a/packages/react-data-grid/src/Row.js b/packages/react-data-grid/src/Row.js index 3d7126cc06..b9b5d89196 100644 --- a/packages/react-data-grid/src/Row.js +++ b/packages/react-data-grid/src/Row.js @@ -95,10 +95,16 @@ class Row extends React.Component { .map(column => this.getCell(column)); }; + getRowTop = () => { + if (this.row) { + return this.row.offsetTop; + } + }; + getRowHeight = () => { - let rows = this.props.expandedRows || null; + const rows = this.props.expandedRows || null; if (rows && this.props.idx) { - let row = rows[this.props.idx] || null; + const row = rows[this.props.idx] || null; if (row) { return row.height; } @@ -135,6 +141,10 @@ class Row extends React.Component { }); }; + setRowRef = el => { + this.row = el; + }; + getKnownDivProps = () => { return createObjectWithProperties(this.props, knownDivPropertyKeys); }; @@ -159,6 +169,7 @@ class Row extends React.Component { return (
{ +const setMaskStyle = ({ left, top, width, height, zIndex }) => { return { height, width, zIndex, - position: position || 'absolute', + position: 'absolute', pointerEvents: 'none', transform: `translate(${left}px, ${top}px)`, outline: 0 }; }; -const CellMask = ({ width, height, top, left, zIndex, children, position, innerRef, ...rest }) => ( +const CellMask = ({ width, height, top, left, zIndex, children, innerRef, ...rest }) => (
{ - if (this.selectionMask) { - const { left, top } = this.selectionMask.getBoundingClientRect(); - const { scrollLeft, scrollTop } = document.documentElement; - this.editorPosition = { left: left + scrollLeft, top: top + scrollTop }; - } - }; - - setMaskScollLeft = (mask, position, scrollLeft) => { - if (mask) { - const { idx, rowIdx } = position; - if (idx >= 0 && rowIdx >= 0) { - const { columns, rowHeight } = this.props; - const column = columnUtils.getColumn(columns, idx); - const frozen = columnUtils.isFrozen(column); - if (frozen) { - const top = getRowTop(rowIdx, rowHeight); - const left = scrollLeft + column.left; - const transform = `translate(${left}px, ${top}px)`; - if (mask.style.transform !== transform) { - mask.style.transform = transform; - } - } - } - } - }; - - /** - * Sets the position of SelectionMask and CopyMask components when the canvas is scrolled - * This is only required on the frozen columns - * @param scrollLeft number - */ - setScrollLeft = (scrollLeft) => { - const { selectionMask, copyMask, state: { selectedPosition, copiedPosition } } = this; - this.setMaskScollLeft(selectionMask, selectedPosition, scrollLeft); - this.setMaskScollLeft(copyMask, copiedPosition, scrollLeft); - }; - componentDidUpdate(prevProps, prevState) { const { selectedPosition, isEditorEnabled } = this.state; const { selectedPosition: prevSelectedPosition, isEditorEnabled: prevIsEditorEnabled } = prevState; @@ -175,6 +139,44 @@ class InteractionMasks extends React.Component { this.unsubscribeDragEnter(); } + saveEditorPosition = () => { + if (this.selectionMask) { + const { left, top } = this.selectionMask.getBoundingClientRect(); + const { scrollLeft, scrollTop } = document.documentElement; + this.editorPosition = { left: left + scrollLeft, top: top + scrollTop }; + } + }; + + setMaskScollLeft = (mask, position, scrollLeft) => { + if (mask) { + const { idx, rowIdx } = position; + if (idx >= 0 && rowIdx >= 0) { + const { columns, getRowTop } = this.props; + const column = columnUtils.getColumn(columns, idx); + const frozen = columnUtils.isFrozen(column); + if (frozen) { + const top = getRowTop(rowIdx); + const left = scrollLeft + column.left; + const transform = `translate(${left}px, ${top}px)`; + if (mask.style.transform !== transform) { + mask.style.transform = transform; + } + } + } + } + }; + + /** + * Sets the position of SelectionMask and CopyMask components when the canvas is scrolled + * This is only required on the frozen columns + * @param scrollLeft number + */ + setScrollLeft = (scrollLeft) => { + const { selectionMask, copyMask, state: { selectedPosition, copiedPosition } } = this; + this.setMaskScollLeft(selectionMask, selectedPosition, scrollLeft); + this.setMaskScollLeft(copyMask, copiedPosition, scrollLeft); + }; + onKeyDown = e => { if (isCtrlKeyHeldDown(e)) { this.onPressKeyWithCtrl(e); @@ -508,7 +510,7 @@ class InteractionMasks extends React.Component { const selectedRange = { // default the startCell to the selected cell, in case we've just started via keyboard - ...{ startCell: this.state.selectedPosition }, + startCell: this.state.selectedPosition, // assign the previous state (which will override the startCell if we already have one) ...this.state.selectedRange, // assign the new state - the bounds of the range, and the new cursor cell @@ -624,25 +626,22 @@ class InteractionMasks extends React.Component { this.copyMask = node; }; - getSelectionMaskProps = () => { - const { columns, scrollLeft, rowHeight } = this.props; - - return { - columns, - scrollLeft, - rowHeight, - isGroupedRow: this.isGroupedRowSelected(), - innerRef: this.setSelectionMaskRef - }; + getSelectedDimensions = (selectedPosition) => { + const { scrollLeft, getRowHeight, getRowTop, getRowColumns } = this.props; + const columns = getRowColumns(selectedPosition.idx); + const top = getRowTop(selectedPosition.rowIdx); + const rowHeight = getRowHeight(selectedPosition.rowIdx); + return { ...getSelectedDimensions({ selectedPosition, columns, scrollLeft, rowHeight }), top }; }; - getSingleCellSelectView = () => { + renderSingleCellSelectView = () => { const { selectedPosition } = this.state; return ( !this.state.isEditorEnabled && this.isGridSelected() && ( {this.isDragEnabled() && ( { + renderCellRangeSelectView = () => { const { columns, rowHeight } = this.props; return [ ]; }; render() { - const { rowGetter, contextMenu, rowHeight, getSelectedRowColumns, scrollLeft, scrollTop } = this.props; + const { rowGetter, contextMenu, getRowColumns, scrollLeft, scrollTop } = this.props; const { isEditorEnabled, firstEditorKeyPress, selectedPosition, draggedPosition, copiedPosition } = this.state; const rowData = getSelectedRow({ selectedPosition, rowGetter }); - const columns = getSelectedRowColumns(selectedPosition.rowIdx); + const columns = getRowColumns(selectedPosition.rowIdx); return (
)} {draggedPosition && ( )} {selectedRangeIsSingleCell(this.state.selectedRange) ? - this.getSingleCellSelectView() : - this.getCellRangeSelectView() + this.renderSingleCellSelectView() : + this.renderCellRangeSelectView() } {isEditorEnabled && ( diff --git a/packages/react-data-grid/src/masks/SelectionMask.js b/packages/react-data-grid/src/masks/SelectionMask.js index 8518082907..009d0a833f 100644 --- a/packages/react-data-grid/src/masks/SelectionMask.js +++ b/packages/react-data-grid/src/masks/SelectionMask.js @@ -2,10 +2,9 @@ import React from 'react'; import PropTypes from 'prop-types'; import CellMask from './CellMask'; -import { getSelectedDimensions } from '../utils/SelectedCellUtils'; -function SelectionMask({ children, innerRef, ...rest }) { - const dimensions = getSelectedDimensions(rest); +function SelectionMask({ selectedPosition, innerRef, getSelectedDimensions, children }) { + const dimensions = getSelectedDimensions(selectedPosition); return ( { const setup = (propsOverride = {}) => { const props = { - columns: [ - { width: 50, left: 5 } - ], - rowHeight: 30, + getSelectedDimensions: () => ({ + height: 30, + width: 50, + left: 5, + top: 90 + }), ...propsOverride }; @@ -24,11 +26,10 @@ describe('CopyMask', () => { expect(mask.props()).toEqual( jasmine.objectContaining({ - height: 30, // = rowHeight + height: 30, width: 50, left: 5, - top: 90, // = rowHeight * rowIdx - zIndex: zIndexes.CELL_MASK, + top: 90, className: 'react-grid-cell-copied' }) ); diff --git a/packages/react-data-grid/src/masks/__tests__/DragMask.spec.js b/packages/react-data-grid/src/masks/__tests__/DragMask.spec.js index 5c771114d3..ee9e65e2b2 100644 --- a/packages/react-data-grid/src/masks/__tests__/DragMask.spec.js +++ b/packages/react-data-grid/src/masks/__tests__/DragMask.spec.js @@ -8,10 +8,22 @@ import zIndexes from 'common/constants/zIndexes'; describe('DragMask', () => { const setup = (propsOverride = {}) => { const props = { - columns: [ - { width: 50, left: 5 } - ], - rowHeight: 30, + getSelectedDimensions: ({ rowIdx }) => { + const { [rowIdx]: height } = { + 2: 20, + 3: 30, + 4: 40, + 5: 50, + 6: 60 + }; + + return { + height, + width: 50, + left: 5, + top: 90 + }; + }, ...propsOverride }; @@ -26,15 +38,14 @@ describe('DragMask', () => { }); it('should render the CellMask component with correct position for the dragged down cell', () => { - const mask = setup({ draggedPosition: { idx: 0, rowIdx: 2, overRowIdx: 6 } }); + const mask = setup({ draggedPosition: { idx: 0, rowIdx: 2, overRowIdx: 4 } }); expect(mask.props()).toEqual( jasmine.objectContaining({ - height: 120, // rowHeight * (overRowIdx - rowIdx) + height: 70, width: 50, left: 5, - top: 90, // = rowHeight * rowIdx - zIndex: zIndexes.CELL_MASK + top: 90 }) ); }); @@ -44,11 +55,10 @@ describe('DragMask', () => { expect(mask.props()).toEqual( jasmine.objectContaining({ - height: 60, // rowHeight * (rowIdx - overRowIdx) + height: 90, width: 50, left: 5, - top: 120, // = rowHeight * overRowIdx - zIndex: zIndexes.CELL_MASK + top: 90 }) ); }); diff --git a/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js b/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js index 7be59c868e..a6b782ba9e 100644 --- a/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js +++ b/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js @@ -49,7 +49,9 @@ describe('', () => { enableCellSelect: true, cellNavigationMode: CellNavigationMode.NONE, eventBus, - getSelectedRowColumns: jasmine.createSpy().and.callFake(() => columns), + getRowColumns: () => columns, + getRowHeight: () => 50, + getRowTop: () => 0, ...overrideProps }; const wrapper = render(, { disableLifecycleMethods: false }); @@ -656,7 +658,7 @@ describe('', () => { it('goes to the last cell in the row when the user presses Shift+Tab and they are at the beginning of a row', () => { const selectedPosition = { rowIdx: 0, idx: 0 }; assertSelectedCellOnTab({ cellNavigationMode }, true, { selectedPosition }) - .toEqual({ rowIdx: 0, idx: NUMBER_OF_COLUMNS - 1, changeRowOrColumn: true}); + .toEqual({ rowIdx: 0, idx: NUMBER_OF_COLUMNS - 1, changeRowOrColumn: true }); }); it('goes to the next cell when the user presses Tab and they are not at the end of a row', () => { const selectedPosition = { rowIdx: 3, idx: 3 }; @@ -696,18 +698,6 @@ describe('', () => { expect(wrapper.find(CopyMask).props().copiedPosition).toEqual({ idx: 1, rowIdx: 2, value: '3' }); }); - it('should render a CopyMask component with correct columns', () => { - const { wrapper, props } = setupCopy(); - const { columns: selectedRowColumns, getSelectedRowColumns } = props; - const copyRowColumns = selectedRowColumns.slice(0, 5); - getSelectedRowColumns.and.callFake(rowIdx => rowIdx === 2 ? copyRowColumns : selectedRowColumns); - // Copy selected cell - pressKey(wrapper, 'c', { keyCode: keyCodes.c, ctrlKey: true }); - // Change selected row - pressKey(wrapper, 'ArrowUp'); - expect(wrapper.find(CopyMask).props().columns).toEqual(copyRowColumns); - }); - it('should remove the CopyMask component on escape', () => { const { wrapper } = setupCopy(); pressKey(wrapper, 'c', { keyCode: keyCodes.c, ctrlKey: true }); diff --git a/packages/react-data-grid/src/masks/__tests__/SelectionMask.spec.js b/packages/react-data-grid/src/masks/__tests__/SelectionMask.spec.js index 632aa0225c..d30c1864df 100644 --- a/packages/react-data-grid/src/masks/__tests__/SelectionMask.spec.js +++ b/packages/react-data-grid/src/masks/__tests__/SelectionMask.spec.js @@ -3,19 +3,18 @@ import { shallow } from 'enzyme'; import CellMask from '../CellMask'; import SelectionMask from '../SelectionMask'; -import zIndexes from 'common/constants/zIndexes'; describe('SelectionMask', () => { const ROW_HEIGHT = 50; const setup = (propsOverride = {}) => { const props = { selectedPosition: { idx: 0, rowIdx: 3 }, - columns: [ - { width: 50, left: 5 } - ], - rowHeight: ROW_HEIGHT, - isGroupedRow: false, - scrollLeft: 0, + getSelectedDimensions: () => ({ + height: 30, + width: 50, + left: 5, + top: 90 + }), ...propsOverride }; @@ -28,11 +27,10 @@ describe('SelectionMask', () => { expect(mask.props()).toEqual( jasmine.objectContaining({ - height: ROW_HEIGHT, + height: 30, width: 50, left: 5, - top: 150, - zIndex: zIndexes.CELL_MASK, + top: 90, className: 'rdg-selected', children: undefined })