From 264045ff3a1638688e1b902be0ea0c2e6bdfc3a8 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 20 Nov 2018 11:26:03 -0600 Subject: [PATCH] POC: use react portals for cell editors (#1369) * POC: use react portals for cell editors * Delete obsolete tests * Fix selection/copy/drag masks positions on frozen columns * Simplify selectionMask position logic Remove unused props * Fix unit tests * Remove getSelectedRowTop * Fix unit tests * Fix unit tests in FF * Remove position prop in favor of left/top props * MInor cleanup * Remove fdescribe and fix unit tests * Remove empty event handlers * Add some comments * - Fix row heights for copy and drag masks - Add back dynamic row logic --- packages/common/editors/EditorContainer.js | 72 +++++----- packages/common/editors/EditorPortal.js | 28 ++++ .../editors/__tests__/EditorContainer.spec.js | 37 +---- packages/react-data-grid-addons/package.json | 4 +- packages/react-data-grid/package.json | 4 +- packages/react-data-grid/src/Canvas.js | 86 ++++++----- 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/Row.js | 15 +- packages/react-data-grid/src/Viewport.js | 4 +- .../src/__tests__/Canvas.spec.js | 2 +- .../src/__tests__/Header.spec.js | 5 + .../src/__tests__/Viewport.spec.js | 4 - .../react-data-grid/src/getScrollbarSize.js | 4 +- .../react-data-grid/src/masks/CellMask.js | 8 +- .../react-data-grid/src/masks/CopyMask.js | 10 +- .../react-data-grid/src/masks/DragMask.js | 10 +- .../src/masks/InteractionMasks.js | 135 +++++++++++------- .../src/masks/SelectionMask.js | 39 +---- .../src/masks/__tests__/CopyMask.spec.js | 15 +- .../src/masks/__tests__/DragMask.spec.js | 32 +++-- .../masks/__tests__/InteractionMasks.spec.js | 20 +-- .../src/masks/__tests__/SelectionMask.spec.js | 20 ++- .../src/utils/SelectedCellUtils.js | 12 +- 25 files changed, 299 insertions(+), 273 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..720d5c0cb3 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; @@ -17,19 +19,19 @@ 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, - top: PropTypes.number, - left: PropTypes.number, scrollLeft: PropTypes.number, scrollTop: PropTypes.number }; - state = {isInvalid: false}; + state = { isInvalid: false }; changeCommitted = false; changeCanceled = false; @@ -42,7 +44,6 @@ class EditorContainer extends React.Component { inputNode.style.height = this.props.height - 1 + 'px'; } } - window.addEventListener('scroll', this.setContainerPosition); } componentDidUpdate(prevProps) { @@ -53,24 +54,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 +119,23 @@ class EditorContainer extends React.Component { return ; } - return {}} commit={() => {}}/>; + return ( + + ); }; onPressEnter = () => { - this.commit({key: 'Enter'}); + this.commit({ key: 'Enter' }); }; onPressTab = () => { - this.commit({key: 'Tab'}); + this.commit({ key: 'Tab' }); }; onPressEscape = (e) => { @@ -239,13 +234,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 +252,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 +301,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 +316,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 +344,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, left, top, column } = this.props; + const zIndex = isFrozen(column) ? zIndexes.FROZEN_EDITOR_CONTAINER : zIndexes.EDITOR_CONTAINER; + const style = { position: 'absolute', height, width, left, top, zIndex }; 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 b8797d92f7..d793ee743a 100644 --- a/packages/common/editors/__tests__/EditorContainer.spec.js +++ b/packages/common/editors/__tests__/EditorContainer.spec.js @@ -19,7 +19,9 @@ const defaultProps = { }, column: fakeColumn, value: 'Adwolf', - height: 50 + height: 50, + onCommit: jasmine.createSpy(), + onCommitCancel: jasmine.createSpy() }; const getComponent = (props) => { @@ -76,31 +78,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 - }; - - it('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', () => { - 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', () => { @@ -189,12 +166,11 @@ 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 }); + component = mount(, { attachTo: container }); }); afterEach(() => { @@ -214,6 +190,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); diff --git a/packages/react-data-grid-addons/package.json b/packages/react-data-grid-addons/package.json index eeca8523fc..dbca38e57b 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.4" }, "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 3014db4d13..16bcc2f342 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/Canvas.js b/packages/react-data-grid/src/Canvas.js index 811ad2abdd..ffef748ca1 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 DefaultRowsContainer from './RowsContainer'; import cellMetaDataShape from 'common/prop-shapes/CellActionShape'; @@ -219,9 +219,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); } @@ -231,44 +235,46 @@ 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]; }; - getSelectedRowTop = (rowIdx) => { + getRowTop = (rowIdx) => { const row = this.getRowByRef(rowIdx); - if (row) { - const node = ReactDOM.findDOMNode(row); - return node && node.offsetTop; + if (row && isFunction(row.getRowTop)) { + return row.getRowTop(); } return this.props.rowHeight * rowIdx; - } + }; - getSelectedRowHeight = (rowIdx) => { + getRowHeight = (rowIdx) => { const row = this.getRowByRef(rowIdx); - if (row) { - const node = ReactDOM.findDOMNode(row); - return node && node.clientHeight > 0 ? node.clientHeight : this.props.rowHeight; + if (row && isFunction(row.getRowHeight)) { + return row.getRowHeight(); } return this.props.rowHeight; - } + }; - getSelectedRowColumns = (rowIdx) => { + getRowColumns = (rowIdx) => { const row = this.getRowByRef(rowIdx); 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; @@ -287,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); } @@ -316,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) =>
+ ) + } +
); }; @@ -383,6 +392,7 @@ class Canvas extends React.PureComponent { onScroll={this.onScroll} className="react-grid-Canvas">
{rows}
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/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 (
{ 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/__tests__/Viewport.spec.js b/packages/react-data-grid/src/__tests__/Viewport.spec.js index a2868bf301..96ef300923 100644 --- a/packages/react-data-grid/src/__tests__/Viewport.spec.js +++ b/packages/react-data-grid/src/__tests__/Viewport.spec.js @@ -97,8 +97,6 @@ describe('', () => { height: viewportProps.minHeight, scrollLeft: scrollLeft, scrollTop: scrollTop, - prevScrollTop: 0, - prevScrollLeft: 0, rowVisibleEndIdx: 21, rowVisibleStartIdx: 6, isScrolling: true, @@ -154,8 +152,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/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; diff --git a/packages/react-data-grid/src/masks/CellMask.js b/packages/react-data-grid/src/masks/CellMask.js index 510333815f..c49eeaeafa 100644 --- a/packages/react-data-grid/src/masks/CellMask.js +++ b/packages/react-data-grid/src/masks/CellMask.js @@ -1,21 +1,21 @@ import React from 'react'; import PropTypes from 'prop-types'; -const setMaskStyle = ({ left, top, width, height, zIndex, position }) => { +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 }) => (
); } CopyMask.propTypes = { copiedPosition: PropTypes.object.isRequired, - columns: PropTypes.array.isRequired, - rowHeight: PropTypes.number.isRequired + getSelectedDimensions: PropTypes.func.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..f7e40d3715 100644 --- a/packages/react-data-grid/src/masks/DragMask.js +++ b/packages/react-data-grid/src/masks/DragMask.js @@ -1,10 +1,9 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { getSelectedDimensions } from '../utils/SelectedCellUtils'; import CellMask from './CellMask'; -function DragMask({ draggedPosition, columns, rowHeight }) { +function DragMask({ draggedPosition, getSelectedDimensions }) { const { overRowIdx, idx, rowIdx } = draggedPosition; if (overRowIdx != null && rowIdx !== overRowIdx) { const isDraggedOverDown = rowIdx < overRowIdx; @@ -12,9 +11,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({ idx, rowIdx: startRowIdx }); for (let currentRowIdx = startRowIdx + 1; currentRowIdx <= endRowIdx; currentRowIdx++) { - const { height } = getSelectedDimensions({ selectedPosition: { idx, rowIdx: currentRowIdx }, columns, rowHeight }); + const { height } = getSelectedDimensions({ idx, rowIdx: currentRowIdx }); dimensions.height += height; } @@ -30,8 +29,7 @@ function DragMask({ draggedPosition, columns, rowHeight }) { DragMask.propTypes = { draggedPosition: PropTypes.object.isRequired, - columns: PropTypes.array.isRequired, - rowHeight: PropTypes.number.isRequired + getSelectedDimensions: PropTypes.func.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 8bd9aff6c9..36e7a938f8 100644 --- a/packages/react-data-grid/src/masks/InteractionMasks.js +++ b/packages/react-data-grid/src/masks/InteractionMasks.js @@ -29,6 +29,8 @@ require('../../../../themes/interaction-masks.css'); const SCROLL_CELL_BUFFER = 2; class InteractionMasks extends React.Component { + static dispplayName = 'InteractionMasks'; + static propTypes = { colVisibleStartIdx: PropTypes.number.isRequired, colVisibleEndIdx: PropTypes.number.isRequired, @@ -66,13 +68,11 @@ 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 + getRowHeight: PropTypes.func.isRequired, + getRowTop: PropTypes.func.isRequired, + getRowColumns: PropTypes.func.isRequired }; state = { @@ -90,7 +90,6 @@ class InteractionMasks extends React.Component { }, copiedPosition: null, draggedPosition: null, - frozenPosition: null, isEditorEnabled: false, firstEditorKeyPress: null }; @@ -115,6 +114,7 @@ class InteractionMasks extends React.Component { if ((isSelectedPositionChanged && this.isCellWithinBounds(selectedPosition)) || isEditorClosed) { this.focus(); + this.saveEditorPosition(); } } @@ -139,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); @@ -351,7 +389,7 @@ class InteractionMasks extends React.Component { const keyNavAction = this.getKeyNavActionFromEvent(e); const next = this.getNextSelectedCellPositionForKeyNavAction(keyNavAction, currentPosition, cellNavigationMode); this.checkIsAtGridBoundary(keyNavAction, next); - this.selectCell({...next}); + this.selectCell({ ...next }); } changeSelectedRangeFromArrowKeyAction(e) { @@ -360,7 +398,7 @@ class InteractionMasks extends React.Component { const keyNavAction = this.getKeyNavActionFromEvent(e); const next = this.getNextSelectedCellPositionForKeyNavAction(keyNavAction, currentPosition, cellNavigationMode); this.checkIsAtGridBoundary(keyNavAction, next); - this.onSelectCellRangeUpdated({...next}, true, () => { this.onSelectCellRangeEnded(); }); + this.onSelectCellRangeUpdated({ ...next }, true, () => { this.onSelectCellRangeEnded(); }); } getNextSelectedCellPositionForKeyNavAction(keyNavAction, currentPosition, cellNavigationMode) { @@ -421,7 +459,6 @@ class InteractionMasks extends React.Component { if (this.isCellWithinBounds(next)) { return { selectedPosition: next, - prevSelectedPosition: cell, selectedRange: { topLeft: next, bottomRight: next, @@ -473,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 @@ -585,31 +622,26 @@ class InteractionMasks extends React.Component { this.selectionMask = node; }; - getSelectionMaskProps = () => { - const { columns, getSelectedRowHeight, getSelectedRowTop, scrollLeft, scrollTop, prevScrollLeft, prevScrollTop } = this.props; - const { prevSelectedPosition } = this.state; + setCopyMaskRef = (node) => { + this.copyMask = node; + }; - return { - columns, - scrollTop, - scrollLeft, - getSelectedRowHeight, - getSelectedRowTop, - prevScrollLeft, - prevScrollTop, - prevSelectedPosition, - 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 } = 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 && } + {isEditorEnabled && ( + + )} {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 0d5271e3e8..009d0a833f 100644 --- a/packages/react-data-grid/src/masks/SelectionMask.js +++ b/packages/react-data-grid/src/masks/SelectionMask.js @@ -1,43 +1,14 @@ 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)); -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); - 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 = getLeftPosition(frozen, column.left, props); - return { height, top, width: column.width, left, zIndex }; -}; +import CellMask from './CellMask'; -function SelectionMask({ children, innerRef, ...rest }) { - const dimensions = getCellMaskDimensions(rest); - const frozen = isFrozenColumn(rest.columns, rest.selectedPosition); - const position = frozen && isScrollingHorizontallyWithoutCellChange(rest) ? 'fixed' : 'absolute'; +function SelectionMask({ selectedPosition, innerRef, getSelectedDimensions, children }) { + const dimensions = getSelectedDimensions(selectedPosition); return ( @@ -48,8 +19,8 @@ function SelectionMask({ children, innerRef, ...rest }) { SelectionMask.propTypes = { selectedPosition: PropTypes.object.isRequired, - columns: PropTypes.array.isRequired, - innerRef: PropTypes.func + getSelectedDimensions: PropTypes.func.isRequired, + innerRef: PropTypes.func.isRequired }; export default SelectionMask; diff --git a/packages/react-data-grid/src/masks/__tests__/CopyMask.spec.js b/packages/react-data-grid/src/masks/__tests__/CopyMask.spec.js index 806489fbf2..3944a81b08 100644 --- a/packages/react-data-grid/src/masks/__tests__/CopyMask.spec.js +++ b/packages/react-data-grid/src/masks/__tests__/CopyMask.spec.js @@ -8,10 +8,12 @@ import zIndexes from 'common/constants/zIndexes'; describe('CopyMask', () => { 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 306f9d6390..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,9 +49,9 @@ describe('', () => { enableCellSelect: true, cellNavigationMode: CellNavigationMode.NONE, eventBus, - getSelectedRowHeight: () => 50, - getSelectedRowTop: () => 0, - getSelectedRowColumns: jasmine.createSpy().and.callFake(() => columns), + getRowColumns: () => columns, + getRowHeight: () => 50, + getRowTop: () => 0, ...overrideProps }; const wrapper = render(, { disableLifecycleMethods: false }); @@ -658,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 }; @@ -698,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 962fdf4d7d..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,21 +3,18 @@ import { shallow } from 'enzyme'; import CellMask from '../CellMask'; import SelectionMask from '../SelectionMask'; -import zIndexes from 'common/constants/zIndexes'; describe('SelectionMask', () => { - const TOP = 45; const ROW_HEIGHT = 50; const setup = (propsOverride = {}) => { const props = { selectedPosition: { idx: 0, rowIdx: 3 }, - columns: [ - { width: 50, left: 5 } - ], - getSelectedRowHeight: () => ROW_HEIGHT, - getSelectedRowTop: () => TOP, - isGroupedRow: false, - scrollLeft: 0, + getSelectedDimensions: () => ({ + height: 30, + width: 50, + left: 5, + top: 90 + }), ...propsOverride }; @@ -30,11 +27,10 @@ describe('SelectionMask', () => { expect(mask.props()).toEqual( jasmine.objectContaining({ - height: ROW_HEIGHT, + height: 30, width: 50, left: 5, - top: TOP, - zIndex: zIndexes.CELL_MASK, + top: 90, className: 'rdg-selected', children: undefined }) diff --git a/packages/react-data-grid/src/utils/SelectedCellUtils.js b/packages/react-data-grid/src/utils/SelectedCellUtils.js index 483a844bca..fd5c42895f 100644 --- a/packages/react-data-grid/src/utils/SelectedCellUtils.js +++ b/packages/react-data-grid/src/utils/SelectedCellUtils.js @@ -1,23 +1,25 @@ 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'; -const getRowTop = (rowIdx, rowHeight) => rowIdx * rowHeight; +export const getRowTop = (rowIdx, rowHeight) => rowIdx * rowHeight; export const getSelectedRow = ({ selectedPosition, rowGetter }) => { const { rowIdx } = selectedPosition; 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(columns) ? 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 };