Skip to content

Commit

Permalink
[table] Perf improvements with selections (#2005)
Browse files Browse the repository at this point in the history 
* table performance improvements - reducing rerendering where possible

* reinstate enum strings

* fix compile errors

* fix lint errors

* fix test

* cell: revert changes

* code review suggestions; add forceRerenderOnSelectionChange flag

* lint

* default to false
  • Loading branch information
@mcintyret @giladgray
mcintyret authored and giladgray committed Nov 2, 2018
1 parent 31a7d0c commit dcad422
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 22 deletions.
4 changes: 4 additions & 0 deletions packages/table-dev-app/src/mutableTable.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ export interface IMutableTableState {
scrollToRowIndex?: number;
selectedFocusStyle?: FocusStyle;
selectedRegionTransformPreset?: SelectedRegionTransformPreset;
selectedRegions?: IRegion[];
showCallbackLogs?: boolean;
showCellsLoading?: boolean;
showColumnHeadersLoading?: boolean;
Expand Down Expand Up @@ -283,6 +284,7 @@ const DEFAULT_STATE: IMutableTableState = {
scrollToRowIndex: 0,
selectedFocusStyle: FocusStyle.TAB,
selectedRegionTransformPreset: SelectedRegionTransformPreset.CELL,
selectedRegions: [],
showCallbackLogs: true,
showCellsLoading: false,
showColumnHeadersLoading: false,
Expand Down Expand Up @@ -403,6 +405,7 @@ export class MutableTable extends React.Component<{}, IMutableTableState> {
rowHeaderCellRenderer={this.renderRowHeader}
selectedRegionTransform={this.getSelectedRegionTransform()}
selectionModes={this.getEnabledSelectionModes()}
selectedRegions={this.state.selectedRegions}
styledRegionGroups={this.getStyledRegionGroups()}
>
{this.renderColumns()}
Expand Down Expand Up @@ -944,6 +947,7 @@ export class MutableTable extends React.Component<{}, IMutableTableState> {

private onSelection = (selectedRegions: IRegion[]) => {
this.maybeLogCallback(`[onSelection] selectedRegions =`, ...selectedRegions);
this.setState({ selectedRegions });
};

private onColumnsReordered = (oldIndex: number, newIndex: number, length: number) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/table/src/common/batcher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,14 @@ export class Batcher<T> {
const keysToAdd = this.setKeysDifference(this.batchArgs, this.currentObjects, addNewLimit);
keysToAdd.forEach(key => (this.currentObjects[key] = callback.apply(undefined, this.batchArgs[key])));

// set `done` to true of sets match exactly after add/remove and there
// set `done` to true if sets match exactly after add/remove and there
// are no "old objects" remaining
this.done =
this.setHasSameKeys(this.batchArgs, this.currentObjects) && Object.keys(this.oldObjects).length === 0;
}

/**
* Returns true of the "current" set matches the "batch" set.
* Returns true if the "current" set matches the "batch" set.
*/
public isDone() {
return this.done;
Expand Down
75 changes: 55 additions & 20 deletions packages/table/src/table.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,13 @@ export interface ITableProps extends IProps, IRowHeights, IColumnWidths {
*/
focusedCell?: IFocusedCellCoordinates;

/**
* If `true`, selection state changes will cause the component to re-render.
* If `false`, selection state is ignored when deciding to re-render.
* @default false
*/
forceRerenderOnSelectionChange?: boolean;

/**
* If defined, this callback will be invoked for each cell when the user
* attempts to copy a selection via `mod+c`. The returned data will be copied
Expand Down Expand Up @@ -420,6 +427,7 @@ export class Table extends AbstractComponent<ITableProps, ITableState> {
enableGhostCells: false,
enableMultipleSelection: true,
enableRowHeader: true,
forceRerenderOnSelectionChange: false,
loadingOptions: [],
minColumnWidth: 50,
minRowHeight: 20,
Expand Down Expand Up @@ -691,34 +699,54 @@ export class Table extends AbstractComponent<ITableProps, ITableState> {
defaultRowHeight,
enableFocusedCell,
focusedCell,
forceRerenderOnSelectionChange,
numRows,
rowHeights,
selectedRegions,
selectionModes,
} = nextProps;

const newChildArray = React.Children.toArray(children) as Array<React.ReactElement<IColumnProps>>;
const didChildrenChange = this.props.children !== nextProps.children;
const newChildArray = didChildrenChange
? (React.Children.toArray(children) as Array<React.ReactElement<IColumnProps>>)
: this.childrenArray;
const numCols = newChildArray.length;

// Try to maintain widths of columns by looking up the width of the
// column that had the same `ID` prop. If none is found, use the
// previous width at the same index.
const previousColumnWidths = newChildArray.map((child: React.ReactElement<IColumnProps>, index: number) => {
const mappedIndex = this.columnIdToIndex[child.props.id];
return this.state.columnWidths[mappedIndex != null ? mappedIndex : index];
});

// Make sure the width/height arrays have the correct length, but keep
// as many existing widths/heights when possible. Also, apply the
// sparse width/heights from props.
let shouldInvalidateGrid = false;
let newColumnWidths = this.state.columnWidths;
newColumnWidths = Utils.arrayOfLength(newColumnWidths, numCols, defaultColumnWidth);
newColumnWidths = Utils.assignSparseValues(newColumnWidths, previousColumnWidths);
newColumnWidths = Utils.assignSparseValues(newColumnWidths, columnWidths);
if (
defaultColumnWidth !== this.props.defaultColumnWidth ||
columnWidths !== this.props.columnWidths ||
didChildrenChange
) {
// Try to maintain widths of columns by looking up the width of the
// column that had the same `ID` prop. If none is found, use the
// previous width at the same index.
const previousColumnWidths = newChildArray.map((child: React.ReactElement<IColumnProps>, index: number) => {
const mappedIndex = this.columnIdToIndex[child.props.id];
return this.state.columnWidths[mappedIndex != null ? mappedIndex : index];
});

// Make sure the width/height arrays have the correct length, but keep
// as many existing widths/heights as possible. Also, apply the
// sparse width/heights from props.
newColumnWidths = Utils.arrayOfLength(newColumnWidths, numCols, defaultColumnWidth);
newColumnWidths = Utils.assignSparseValues(newColumnWidths, previousColumnWidths);
newColumnWidths = Utils.assignSparseValues(newColumnWidths, columnWidths);

shouldInvalidateGrid = true;
}

let newRowHeights = this.state.rowHeights;
newRowHeights = Utils.arrayOfLength(newRowHeights, numRows, defaultRowHeight);
newRowHeights = Utils.assignSparseValues(newRowHeights, rowHeights);
if (
defaultRowHeight !== this.props.defaultRowHeight ||
rowHeights !== this.props.rowHeights ||
numRows !== this.props.numRows
) {
newRowHeights = Utils.arrayOfLength(newRowHeights, numRows, defaultRowHeight);
newRowHeights = Utils.assignSparseValues(newRowHeights, rowHeights);
shouldInvalidateGrid = true;
}

if (
!CoreUtils.arraysEqual(newColumnWidths, this.state.columnWidths) ||
Expand All @@ -730,6 +758,9 @@ export class Table extends AbstractComponent<ITableProps, ITableState> {
}

let newSelectedRegions = selectedRegions;
if (forceRerenderOnSelectionChange && newSelectedRegions !== this.props.selectedRegions) {
shouldInvalidateGrid = true;
}
if (selectedRegions == null) {
// if we're in uncontrolled mode, filter out all selected regions that don't
// fit in the current new table dimensions
Expand All @@ -749,9 +780,13 @@ export class Table extends AbstractComponent<ITableProps, ITableState> {
newSelectedRegions,
);

this.childrenArray = newChildArray;
this.columnIdToIndex = Table.createColumnIdIndex(this.childrenArray);
this.invalidateGrid();
if (didChildrenChange) {
this.childrenArray = newChildArray;
this.columnIdToIndex = Table.createColumnIdIndex(this.childrenArray);
}
if (shouldInvalidateGrid) {
this.invalidateGrid();
}
this.setState({
columnWidths: newColumnWidths,
focusedCell: newFocusedCell,
Expand Down

1 comment on commit dcad422

@blueprint-bot
Copy link

Choose a reason for hiding this comment

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

[table] Perf improvements with selections (#2005)

Previews: documentation | landing | table

Please sign in to comment.