From 2fef940960766a79ae15ae84096cb58d71be2913 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 30 Oct 2018 13:26:53 -0500 Subject: [PATCH 1/2] Remove the hack to preserve scroll position Focus the selection mask instead of parent node --- packages/react-data-grid/src/Canvas.js | 12 ---- .../react-data-grid/src/masks/CellMask.js | 9 ++- .../src/masks/InteractionMasks.js | 58 +++++++++---------- .../src/masks/SelectionMask.js | 16 ++--- 4 files changed, 44 insertions(+), 51 deletions(-) diff --git a/packages/react-data-grid/src/Canvas.js b/packages/react-data-grid/src/Canvas.js index cfc53d7b1c..b9bd0fa41b 100644 --- a/packages/react-data-grid/src/Canvas.js +++ b/packages/react-data-grid/src/Canvas.js @@ -129,17 +129,6 @@ class Canvas extends React.PureComponent { ); }; - onFocusInteractionMask = (focus) => { - const { scrollTop, scrollLeft } = this._scroll; - const { pageXOffset, pageYOffset } = window; - focus(); - if (this.canvas) { - this.canvas.scrollTop = scrollTop; - this.canvas.scrollLeft = scrollLeft; - } - window.scroll(pageXOffset, pageYOffset); - }; - onScroll = (e) => { if (this.canvas !== e.target) { return; @@ -418,7 +407,6 @@ class Canvas extends React.PureComponent { onCellCopyPaste={this.props.onCellCopyPaste} onGridRowsUpdated={this.props.onGridRowsUpdated} onDragHandleDoubleClick={this.props.onDragHandleDoubleClick} - onBeforeFocus={this.onFocusInteractionMask} onCellSelected={this.props.onCellSelected} onCellDeSelected={this.props.onCellDeSelected} onCellRangeSelectionStarted={this.props.onCellRangeSelectionStarted} diff --git a/packages/react-data-grid/src/masks/CellMask.js b/packages/react-data-grid/src/masks/CellMask.js index ae50e1616c..510333815f 100644 --- a/packages/react-data-grid/src/masks/CellMask.js +++ b/packages/react-data-grid/src/masks/CellMask.js @@ -8,14 +8,16 @@ const setMaskStyle = ({ left, top, width, height, zIndex, position }) => { zIndex, position: position || 'absolute', pointerEvents: 'none', - transform: `translate(${left}px, ${top}px)` + transform: `translate(${left}px, ${top}px)`, + outline: 0 }; }; -const CellMask = ({ width, height, top, left, zIndex, children, position, ...rest }) => ( +const CellMask = ({ width, height, top, left, zIndex, children, position, innerRef, ...rest }) => (
{children} @@ -28,7 +30,8 @@ CellMask.propTypes = { top: PropTypes.number.isRequired, left: PropTypes.number.isRequired, zIndex: PropTypes.number.isRequired, - children: PropTypes.node + children: PropTypes.node, + innerRef: PropTypes.func }; export default CellMask; diff --git a/packages/react-data-grid/src/masks/InteractionMasks.js b/packages/react-data-grid/src/masks/InteractionMasks.js index 3e9c86ef5b..f074015779 100644 --- a/packages/react-data-grid/src/masks/InteractionMasks.js +++ b/packages/react-data-grid/src/masks/InteractionMasks.js @@ -19,7 +19,7 @@ import { isSelectedCellEditable, selectedRangeIsSingleCell } from '../utils/SelectedCellUtils'; -import {isFunction} from 'common/utils'; +import { isFunction } from 'common/utils'; import * as columnUtils from '../ColumnUtils'; import * as keyCodes from '../KeyCodes'; import { CellNavigationMode, EventTypes } from 'common/constants'; @@ -65,7 +65,6 @@ class InteractionMasks extends React.Component { onCellRangeSelectionCompleted: PropTypes.func, onCellsDragged: PropTypes.func, onDragHandleDoubleClick: PropTypes.func.isRequired, - onBeforeFocus: PropTypes.func.isRequired, scrollLeft: PropTypes.number.isRequired, prevScrollLeft: PropTypes.number.isRequired, scrollTop: PropTypes.number.isRequired, @@ -397,7 +396,7 @@ class InteractionMasks extends React.Component { }; isFocused = () => { - return document.activeElement === this.node; + return document.activeElement === this.selectionMask; }; isFocusedOnBody = () => { @@ -405,8 +404,8 @@ class InteractionMasks extends React.Component { }; focus = () => { - if (this.node && !this.isFocused()) { - this.props.onBeforeFocus(() => this.node.focus()); + if (this.selectionMask && !this.isFocused()) { + this.selectionMask.focus(); } }; @@ -579,22 +578,35 @@ class InteractionMasks extends React.Component { this.closeEditor(); }; + setSelectionMaskRef = (node) => { + this.selectionMask = node; + }; + + getSelectionMaskProps = () => { + const { columns, getSelectedRowHeight, getSelectedRowTop, scrollLeft, scrollTop, prevScrollLeft, prevScrollTop } = this.props; + const { prevSelectedPosition } = this.state; + + return { + columns, + scrollTop, + scrollLeft, + getSelectedRowHeight, + getSelectedRowTop, + prevScrollLeft, + prevScrollTop, + prevSelectedPosition, + isGroupedRow: this.isGroupedRowSelected(), + innerRef: this.setSelectionMaskRef + }; + }; + getSingleCellSelectView = () => { - const { columns, getSelectedRowHeight, getSelectedRowTop, scrollLeft, scrollTop, prevScrollLeft, prevScrollTop} = this.props; const { selectedPosition } = this.state; return ( !this.state.isEditorEnabled && this.isGridSelected() && ( {this.dragEnabled() && ( { - const { columns, rowHeight, getSelectedRowHeight, getSelectedRowTop, scrollLeft, scrollTop, prevScrollLeft, prevScrollTop } = this.props; + const { columns, rowHeight } = this.props; return [ ]; }; @@ -640,10 +644,6 @@ class InteractionMasks extends React.Component { const columns = getSelectedRowColumns(selectedPosition.rowIdx); return (
{ - this.node = node; - }} - tabIndex="0" onKeyDown={this.onKeyDown} onFocus={this.onFocus} > diff --git a/packages/react-data-grid/src/masks/SelectionMask.js b/packages/react-data-grid/src/masks/SelectionMask.js index c61548633b..0d5271e3e8 100644 --- a/packages/react-data-grid/src/masks/SelectionMask.js +++ b/packages/react-data-grid/src/masks/SelectionMask.js @@ -4,9 +4,9 @@ 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 isFrozenColumn = (columns, { idx }) => columnUtils.isFrozen(columnUtils.getColumn(columns, idx)); -const isScrollingHorizontallyWithoutCellChange = ({scrollTop, prevScrollTop, scrollLeft, prevScrollLeft, selectedPosition, prevSelectedPosition}) => { +const isScrollingHorizontallyWithoutCellChange = ({ scrollTop, prevScrollTop, scrollLeft, prevScrollLeft, selectedPosition, prevSelectedPosition }) => { return scrollLeft !== prevScrollLeft && (scrollTop === prevScrollTop) && selectedPosition.idx === prevSelectedPosition.idx; }; @@ -19,17 +19,17 @@ const getLeftPosition = (isFrozen, cellLeft, props) => { export const getCellMaskDimensions = (props) => { - const { selectedPosition, columns, getSelectedRowHeight, getSelectedRowTop} = 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 zIndex = frozen ? zIndexes.FROZEN_CELL_MASK : zIndexes.CELL_MASK; const left = getLeftPosition(frozen, column.left, props); - return {height, top, width: column.width, left, zIndex}; + return { height, top, width: column.width, left, zIndex }; }; -function SelectionMask({children, ...rest}) { +function SelectionMask({ children, innerRef, ...rest }) { const dimensions = getCellMaskDimensions(rest); const frozen = isFrozenColumn(rest.columns, rest.selectedPosition); const position = frozen && isScrollingHorizontallyWithoutCellChange(rest) ? 'fixed' : 'absolute'; @@ -38,6 +38,8 @@ function SelectionMask({children, ...rest}) { {...dimensions} className="rdg-selected" position={position} + innerRef={innerRef} + tabIndex="0" > {children} @@ -47,7 +49,7 @@ function SelectionMask({children, ...rest}) { SelectionMask.propTypes = { selectedPosition: PropTypes.object.isRequired, columns: PropTypes.array.isRequired, - rowHeight: PropTypes.number.isRequired + innerRef: PropTypes.func }; export default SelectionMask; From ee22e1b67440ec3916941b7d5e27c57ed6a46866 Mon Sep 17 00:00:00 2001 From: Aman Mahajan Date: Tue, 30 Oct 2018 14:22:51 -0500 Subject: [PATCH 2/2] Fix unit tests --- .../src/masks/__tests__/InteractionMasks.spec.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 657dbe2af2..90a088ee90 100644 --- a/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js +++ b/packages/react-data-grid/src/masks/__tests__/InteractionMasks.spec.js @@ -47,7 +47,6 @@ describe('', () => { enableCellSelect: true, cellNavigationMode: CellNavigationMode.NONE, eventBus, - onBeforeFocus: jasmine.createSpy().and.returnValue(() => null), getSelectedRowHeight: () => 50, getSelectedRowTop: () => 0, getSelectedRowColumns: jasmine.createSpy().and.callFake(() => columns), @@ -191,9 +190,12 @@ describe('', () => { it('should give focus to InteractionMasks once a selection has ended', () => { // We have to use mount, rather than shallow, so that InteractionMasks has a ref to it's node, used for focusing - const { props } = setup(undefined, undefined, mount); + const { props, wrapper } = setup(undefined, undefined, mount); + props.eventBus.dispatch(EventTypes.SELECT_START, { idx: 2, rowIdx: 2 }); + const { selectionMask } = wrapper.instance(); + spyOn(selectionMask, 'focus'); props.eventBus.dispatch(EventTypes.SELECT_END); - expect(props.onBeforeFocus).toHaveBeenCalled(); + expect(selectionMask.focus).toHaveBeenCalled(); }); }); });