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

Remove setScrollLeft on Row and Cell #1793

Merged
merged 2 commits into from
Oct 16, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
"@types/react": "^16.9.5",
"@types/react-dom": "^16.9.1",
"@types/react-is": "^16.7.1",
"@types/shallowequal": "^1.1.1",
"@typescript-eslint/eslint-plugin": "^2.3.3",
"@typescript-eslint/parser": "^2.3.3",
"babel-loader": "^8.0.6",
Expand Down
2 changes: 0 additions & 2 deletions packages/react-data-grid/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,13 @@
"@material-ui/icons": "^4.5.1",
"classnames": "^2.2.6",
"react-is": "^16.10.2",
"shallowequal": "^1.1.0",
"tslib": "^1.10.0"
},
"peerDependencies": {
"@types/classnames": "^2.2",
"@types/react": "^16.8",
"@types/react-dom": "^16.8",
"@types/react-is": "^16.7",
"@types/shallowequal": "^1.1",
"react": "^16.8",
"react-dom": "^16.8"
}
Expand Down
28 changes: 5 additions & 23 deletions packages/react-data-grid/src/Canvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ export default function Canvas<R, K extends keyof R>({
const [scrollTop, setScrollTop] = useState(0);
const [scrollLeft, setScrollLeft] = useState(0);
const canvas = useRef<HTMLDivElement>(null);
const interactionMasks = useRef<InteractionMasks<R, K>>(null);
const prevScrollToRowIndex = useRef<number | undefined>();
const [rowRefs] = useState(() => new Map<number, Row<R>>());
const clientHeight = getClientHeight();
Expand Down Expand Up @@ -107,9 +106,6 @@ export default function Canvas<R, K extends keyof R>({

function handleScroll(e: React.UIEvent<HTMLDivElement>) {
const { scrollLeft: newScrollLeft, scrollTop: newScrollTop } = e.currentTarget;
// Freeze columns on legacy browsers
setComponentsScrollLeft(newScrollLeft);

setScrollLeft(newScrollLeft);
setScrollTop(newScrollTop);
onScroll({ scrollLeft: newScrollLeft, scrollTop: newScrollTop });
Expand Down Expand Up @@ -152,14 +148,15 @@ export default function Canvas<R, K extends keyof R>({
}

const setRowRef = useCallback((row: Row<R> | null, idx: number) => {
if (row) {
rowRefs.set(idx, row);
} else {
if (row === null) {
rowRefs.delete(idx);
} else {
rowRefs.set(idx, row);
}
}, [rowRefs]);

function getRows() {
const left = isPositionStickySupported() ? undefined : scrollLeft;
const rowElements = [];

for (let idx = rowOverscanStartIdx; idx <= rowOverscanEndIdx; idx++) {
Expand All @@ -181,7 +178,7 @@ export default function Canvas<R, K extends keyof R>({
rowHeight={rowHeight}
rowKey={rowKey}
rowRenderer={rowRenderer}
scrollLeft={scrollLeft}
scrollLeft={left}
selectedRows={selectedRows}
/>
);
Expand All @@ -190,20 +187,6 @@ export default function Canvas<R, K extends keyof R>({
return rowElements;
}

function setComponentsScrollLeft(scrollLeft: number) {
if (isPositionStickySupported()) return;
const { current } = interactionMasks;
if (current) {
current.setScrollLeft(scrollLeft);
}

rowRefs.forEach(row => {
if (row && row.setScrollLeft) {
row.setScrollLeft(scrollLeft);
}
});
}

function getRowColumns(rowIdx: number) {
const row = rowRefs.get(rowIdx);
return row && row.props ? row.props.columns : columnMetrics.columns;
Expand Down Expand Up @@ -236,7 +219,6 @@ export default function Canvas<R, K extends keyof R>({
onKeyUp={onCanvasKeyup}
>
<InteractionMasks<R, K>
ref={interactionMasks}
rowGetter={rowGetter}
rowsCount={rowsCount}
rowHeight={rowHeight}
Expand Down
32 changes: 2 additions & 30 deletions packages/react-data-grid/src/Cell.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import React from 'react';
import classNames from 'classnames';
import shallowEqual from 'shallowequal';

import { SubRowOptions, ColumnEventInfo, CellRenderer, CellRendererProps } from './common/types';
import { SubRowOptions, ColumnEventInfo, CellRendererProps } from './common/types';
import CellActions from './Cell/CellActions';
import CellExpand from './Cell/CellExpander';
import CellContent from './Cell/CellContent';
Expand All @@ -20,23 +19,11 @@ export interface CellProps<R> extends CellRendererProps<R> {
cellControls?: unknown;
}

export default class Cell<R> extends React.Component<CellProps<R>> implements CellRenderer {
export default class Cell<R> extends React.PureComponent<CellProps<R>> {
static defaultProps = {
value: ''
};

private readonly cell = React.createRef<HTMLDivElement>();

shouldComponentUpdate(nextProps: CellProps<R>) {
// TODO: optimize cellMetatData as it is recreated whenever `ReactDataGrid` renders
// On the modern browsers we are using position sticky so scrollLeft/setScrollLeft is not required
// On the legacy browsers scrollLeft sets the initial position so it can be safely ignored in the subsequent renders. Scrolling is handled by the setScrollLeft method
const { scrollLeft, ...rest } = this.props;
const { scrollLeft: nextScrollLeft, ...nextRest } = nextProps;

return !shallowEqual(rest, nextRest);
}

handleCellClick = () => {
const { idx, rowIdx, cellMetaData } = this.props;
cellMetaData.onCellClick({ idx, rowIdx });
Expand Down Expand Up @@ -106,20 +93,6 @@ export default class Cell<R> extends React.Component<CellProps<R>> implements Ce
);
}

setScrollLeft(scrollLeft: number) {
const node = this.cell.current;
if (node) {
node.style.transform = `translateX(${scrollLeft}px)`;
}
}

removeScroll() {
const node = this.cell.current;
if (node) {
node.style.transform = 'none';
}
}

getEvents() {
const { column, cellMetaData, idx, rowIdx, rowData } = this.props;
const columnEvents = column.events;
Expand Down Expand Up @@ -196,7 +169,6 @@ export default class Cell<R> extends React.Component<CellProps<R>> implements Ce

return (
<div
ref={this.cell}
className={className}
style={style}
{...events}
Expand Down
74 changes: 37 additions & 37 deletions packages/react-data-grid/src/Row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import classNames from 'classnames';

import Cell from './Cell';
import { isFrozen } from './utils/columnUtils';
import { IRowRenderer, IRowRendererProps, CellRenderer, CellRendererProps, CalculatedColumn } from './common/types';
import { isPositionStickySupported } from './utils';
import { IRowRendererProps } from './common/types';

export default class Row<R> extends React.Component<IRowRendererProps<R>> implements IRowRenderer {
export default class Row<R> extends React.Component<IRowRendererProps<R>> {
static displayName = 'Row';

static defaultProps = {
Expand All @@ -14,8 +15,6 @@ export default class Row<R> extends React.Component<IRowRendererProps<R>> implem
height: 35
};

private readonly cells = new Map<keyof R, CellRenderer>();

handleDragEnter = (e: React.DragEvent<HTMLDivElement>) => {
// Prevent default to allow drop
e.preventDefault();
Expand All @@ -34,34 +33,44 @@ export default class Row<R> extends React.Component<IRowRendererProps<R>> implem
e.preventDefault();
};

getCell(column: CalculatedColumn<R>) {
const Renderer = this.props.cellRenderer!;
const { idx, cellMetaData, row, scrollLeft, isRowSelected, onRowSelectionChange } = this.props;
const { key } = column;

const cellProps: CellRendererProps<R> & { ref: (cell: CellRenderer | null) => void } = {
ref: (cell) => cell ? this.cells.set(key, cell) : this.cells.delete(key),
idx: column.idx,
rowIdx: idx,
height: this.props.height,
column,
getCells() {
const {
cellMetaData,
value: this.getCellValue(key || String(column.idx) as keyof R) as R[keyof R], // FIXME: fix types
rowData: row,
expandableOptions: this.getExpandableOptions(key),
scrollLeft,
columns,
height,
idx,
isRowSelected,
onRowSelectionChange
};
onRowSelectionChange,
row,
scrollLeft
} = this.props;
const Renderer = this.props.cellRenderer!;
const canSticky = isPositionStickySupported();

return <Renderer key={`${key as keyof R}-${idx}`} {...cellProps} />; // FIXME: fix key type
}
// FIXME: do we need this, considering columnsMetrics should have these columns sorted already?
const frozenColumns = columns.slice(0, this.props.lastFrozenColumnIndex + 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember I tried in my local that to put frozenColumns first and then nonFrozenColumns later. The row cells displaying was screwed up. Not sure why that happened as I didn't take time to investigate.

const nonFrozenColumn = columns.slice(this.props.colOverscanStartIdx, this.props.colOverscanEndIdx + 1).filter(c => !isFrozen(c));

getCells() {
const { colOverscanStartIdx, colOverscanEndIdx, columns, lastFrozenColumnIndex } = this.props;
const frozenColumns = columns.slice(0, lastFrozenColumnIndex + 1);
const nonFrozenColumn = columns.slice(colOverscanStartIdx, colOverscanEndIdx + 1).filter(c => !isFrozen(c));
return nonFrozenColumn.concat(frozenColumns).map(c => this.getCell(c));
return nonFrozenColumn.concat(frozenColumns).map(column => {
const { key } = column;

return (
<Renderer
key={`${key as keyof R}-${idx}`} // FIXME: fix key type
idx={column.idx}
rowIdx={idx}
height={height}
column={column}
cellMetaData={cellMetaData}
value={this.getCellValue(key || String(column.idx) as keyof R) as R[keyof R]} // FIXME: fix types
rowData={row}
expandableOptions={this.getExpandableOptions(key)}
scrollLeft={!canSticky && isFrozen(column) ? scrollLeft : undefined}
isRowSelected={isRowSelected}
onRowSelectionChange={onRowSelectionChange}
/>
);
});
}

getCellValue(key: keyof R) {
Expand All @@ -88,15 +97,6 @@ export default class Row<R> extends React.Component<IRowRendererProps<R>> implem
};
}

setScrollLeft(scrollLeft: number) {
for (const column of this.props.columns) {
const { key } = column;
if (isFrozen(column) && this.cells.has(key)) {
this.cells.get(key)!.setScrollLeft(scrollLeft);
}
}
}

render() {
const className = classNames(
'rdg-row',
Expand Down
12 changes: 2 additions & 10 deletions packages/react-data-grid/src/RowRenderer.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, { memo } from 'react';
import { isElement } from 'react-is';
import shallowEqual from 'shallowequal';

import Row from './Row';
import RowGroup from './RowGroup';
Expand All @@ -25,7 +24,7 @@ interface RowRendererProps<R, K extends keyof R> extends SharedCanvasProps<R, K>
rowData: R;
colOverscanStartIdx: number;
colOverscanEndIdx: number;
scrollLeft: number;
scrollLeft: number | undefined;
setRowRef(row: Row<R> | null, idx: number): void;
}

Expand Down Expand Up @@ -133,11 +132,4 @@ function RowRenderer<R, K extends keyof R>({
return <Row<R> {...rendererProps} />;
}

function propsAreEqual<R, K extends keyof R>(prevProps: RowRendererProps<R, K>, nextProps: RowRendererProps<R, K>) {
const { scrollLeft, ...rest } = prevProps;
const { scrollLeft: nextScrollLeft, ...nextRest } = nextProps;

return shallowEqual(rest, nextRest);
}

export default memo(RowRenderer, propsAreEqual as () => boolean) as <R, K extends keyof R>(props: RowRendererProps<R, K>) => JSX.Element;
export default memo(RowRenderer) as <R, K extends keyof R>(props: RowRendererProps<R, K>) => JSX.Element;
12 changes: 2 additions & 10 deletions packages/react-data-grid/src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export interface CellRendererProps<TRow, TValue = unknown> {
column: CalculatedColumn<TRow>;
rowData: TRow;
cellMetaData: CellMetaData<TRow>;
scrollLeft: number;
scrollLeft: number | undefined;
expandableOptions?: ExpandableOptions;
isRowSelected: boolean;
onRowSelectionChange(rowIdx: number, row: TRow, checked: boolean, isShiftClick: boolean): void;
Expand All @@ -177,7 +177,7 @@ export interface IRowRendererProps<TRow> {
subRowDetails?: SubRowDetails;
colOverscanStartIdx: number;
colOverscanEndIdx: number;
scrollLeft: number;
scrollLeft: number | undefined;
lastFrozenColumnIndex: number;
isRowSelected: boolean;
onRowSelectionChange(rowIdx: number, row: TRow, checked: boolean, isShiftClick: boolean): void;
Expand Down Expand Up @@ -233,14 +233,6 @@ export interface ColumnEventInfo<TRow> extends Position {
column: CalculatedColumn<TRow>;
}

export interface CellRenderer {
setScrollLeft(scrollLeft: number): void;
}

export interface IRowRenderer {
setScrollLeft(scrollLeft: number): void;
}

export interface ScrollPosition {
scrollLeft: number;
scrollTop: number;
Expand Down
35 changes: 1 addition & 34 deletions packages/react-data-grid/src/masks/InteractionMasks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
selectedRangeIsSingleCell,
NextSelectedCellPosition
} from '../utils/selectedCellUtils';
import { isFrozen } from '../utils/columnUtils';

// Types
import { UpdateActions, CellNavigationMode, EventTypes } from '../common/enums';
Expand Down Expand Up @@ -108,7 +107,6 @@ export default class InteractionMasks<R, K extends keyof R> extends React.Compon
};

private readonly selectionMask = React.createRef<HTMLDivElement>();
private readonly copyMask = React.createRef<HTMLDivElement>();

private unsubscribeEventHandlers: Array<() => void> = [];

Expand Down Expand Up @@ -176,32 +174,6 @@ export default class InteractionMasks<R, K extends keyof R> extends React.Compon
};
}

setMaskScollLeft(mask: HTMLDivElement | null, position: Position | null, scrollLeft: number): void {
if (!mask || !position) return;

const { idx, rowIdx } = position;
if (!(idx >= 0 && rowIdx >= 0)) return;

const column = this.props.columns[idx];
if (!isFrozen(column)) return;

const top = this.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
*/
setScrollLeft(scrollLeft: number): void {
this.setMaskScollLeft(this.selectionMask.current, this.state.selectedPosition, scrollLeft);
this.setMaskScollLeft(this.copyMask.current, this.state.copiedPosition, scrollLeft);
}

onKeyDown = (e: React.KeyboardEvent<HTMLDivElement>): void => {
if (isCtrlKeyHeldDown(e)) {
this.onPressKeyWithCtrl(e);
Expand Down Expand Up @@ -692,12 +664,7 @@ export default class InteractionMasks<R, K extends keyof R> extends React.Compon
onKeyDown={this.onKeyDown}
onFocus={this.onFocus}
>
{copiedPosition && (
<CopyMask
{...this.getSelectedDimensions(copiedPosition)}
ref={this.copyMask}
/>
)}
{copiedPosition && <CopyMask {...this.getSelectedDimensions(copiedPosition)} />}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the CopyMask? Like ctrl c / ctrl v background?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
A mask that appears when you copy a cell.

{draggedPosition && (
<DragMask
draggedPosition={draggedPosition}
Expand Down