Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cell focusing logic #1352

Merged
merged 3 commits into from
Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions packages/react-data-grid/src/Canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}
Expand Down
9 changes: 6 additions & 3 deletions packages/react-data-grid/src/masks/CellMask.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => (
<div
style={setMaskStyle({ left, top, width, height, zIndex, position })}
data-test="cell-mask"
ref={innerRef}
{...rest}
>
{children}
Expand All @@ -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;
58 changes: 29 additions & 29 deletions packages/react-data-grid/src/masks/InteractionMasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -397,16 +396,16 @@ class InteractionMasks extends React.Component {
};

isFocused = () => {
return document.activeElement === this.node;
return document.activeElement === this.selectionMask;
};

isFocusedOnBody = () => {
return document.activeElement === document.body;
};

focus = () => {
if (this.node && !this.isFocused()) {
this.props.onBeforeFocus(() => this.node.focus());
if (this.selectionMask && !this.isFocused()) {
this.selectionMask.focus();
}
};

Expand Down Expand Up @@ -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() && (
<SelectionMask
selectedPosition={selectedPosition}
columns={columns}
isGroupedRow={this.isGroupedRowSelected()}
scrollTop={scrollTop}
scrollLeft={scrollLeft}
getSelectedRowHeight={getSelectedRowHeight}
getSelectedRowTop={getSelectedRowTop}
prevScrollLeft={prevScrollLeft}
prevScrollTop={prevScrollTop}
prevSelectedPosition={this.state.prevSelectedPosition}
{...this.getSelectionMaskProps()}
>
{this.dragEnabled() && (
<DragHandle
Expand All @@ -609,7 +621,7 @@ class InteractionMasks extends React.Component {
};

getCellRangeSelectView = () => {
const { columns, rowHeight, getSelectedRowHeight, getSelectedRowTop, scrollLeft, scrollTop, prevScrollLeft, prevScrollTop } = this.props;
const { columns, rowHeight } = this.props;
return [
<SelectionRangeMask
key="range-mask"
Expand All @@ -620,15 +632,7 @@ class InteractionMasks extends React.Component {
<SelectionMask
key="selection-mask"
selectedPosition={this.state.selectedRange.startCell}
columns={columns}
rowHeight={rowHeight}
scrollLeft={scrollLeft}
scrollTop={scrollTop}
getSelectedRowHeight={getSelectedRowHeight}
getSelectedRowTop={getSelectedRowTop}
prevScrollLeft={prevScrollLeft}
prevScrollTop={prevScrollTop}
prevSelectedPosition={this.state.prevSelectedPosition}
{...this.getSelectionMaskProps()}
/>
];
};
Expand All @@ -640,10 +644,6 @@ class InteractionMasks extends React.Component {
const columns = getSelectedRowColumns(selectedPosition.rowIdx);
return (
<div
ref={node => {
this.node = node;
}}
tabIndex="0"
onKeyDown={this.onKeyDown}
onFocus={this.onFocus}
>
Expand Down
16 changes: 9 additions & 7 deletions packages/react-data-grid/src/masks/SelectionMask.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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';
Expand All @@ -38,6 +38,8 @@ function SelectionMask({children, ...rest}) {
{...dimensions}
className="rdg-selected"
position={position}
innerRef={innerRef}
tabIndex="0"
>
{children}
</CellMask>
Expand All @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ describe('<InteractionMasks/>', () => {
enableCellSelect: true,
cellNavigationMode: CellNavigationMode.NONE,
eventBus,
onBeforeFocus: jasmine.createSpy().and.returnValue(() => null),
getSelectedRowHeight: () => 50,
getSelectedRowTop: () => 0,
getSelectedRowColumns: jasmine.createSpy().and.callFake(() => columns),
Expand Down Expand Up @@ -191,9 +190,12 @@ describe('<InteractionMasks/>', () => {

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();
});
});
});
Expand Down