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

[table] fix: re-render immediately on changes in batched mode #5284

Merged
merged 5 commits into from
May 4, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ export class TableSortableExample extends React.PureComponent<IExampleProps> {
numRows={numRows}
selectionModes={SelectionModes.COLUMNS_AND_CELLS}
getCellClipboardData={this.getCellData}
cellRendererDependencies={[this.state.sortedIndexMap]}
>
{columns}
</Table2>
Expand Down
2 changes: 1 addition & 1 deletion packages/table-dev-app/src/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ label.tbl-select-label {
margin-bottom: 7px;
}

.tbl-select-label .#{$ns}-select {
.tbl-select-label .#{$ns}-html-select {
float: right;
}

Expand Down
1 change: 1 addition & 0 deletions packages/table-dev-app/src/mutableTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ export class MutableTable extends React.Component<{}, IMutableTableState> {
selectionModes={this.getEnabledSelectionModes()}
selectedRegions={this.state.selectedRegions}
styledRegionGroups={this.getStyledRegionGroups()}
cellRendererDependencies={[this.state.cellContent]}
>
{this.renderColumns()}
</Table2>
Expand Down
4 changes: 2 additions & 2 deletions packages/table/src/docs/table-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ components are available in the __@blueprintjs/table__ package.

@## Table

The top-level component of the table is `Table`. You must at least define the
The top-level component of the table is `Table2`. You must at least define the
number of rows (`numRows` prop) as well as a set of `Column` children.

@interface ITableProps
@interface Table2Props

@### Instance methods

Expand Down
22 changes: 22 additions & 0 deletions packages/table/src/docs/table-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,30 @@ In the table below, try:
* Sorting with the menu in each column header
* Selecting cells and copying with the right-click context menu or with Cmd/Ctrl+C hotkeys

<div class="@ns-callout @ns-large @ns-intent-primary @ns-icon-info-sign">

This example utilizes `cellRendererDependencies`; [see why in the section below](#table/features.re-rendering-cells).
</div>

@reactExample TableSortableExample

@## Re-rendering cells

Sometimes you may need to re-render table cells based on new data or some user interaction,
like a sorting operation as demonstrated in the above example. In these cases, the typical
table props which tell the component to re-render (like `children`, `numRows`, `selectedRegions`, etc)
may not change, so the table will not re-run its `<Cell>` children render methods.

To work around this problem, `Table2` supports a dependency list in its `cellRendererDependencies` prop.
Dependency changes in this list (compared using shallow equality checks) trigger a re-render of
all cells in the table.

In the above sortable table example, we keep a `sortedIndexMap` value in state which is updated in
the column sort callback. This "map" is referenced in the cell renderers to determine which data to
render at each row index, so by including it as a dependency in `cellRendererDependencies`, we can
guarantee that cell renderers will be re-triggered after a sorting operation, and those renderers
will reference the up-to-date `sortedIndexMap` value.

@## Editing

To make your table editable, use the `EditableCell` and
Expand Down
3 changes: 2 additions & 1 deletion packages/table/src/docs/table.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ Do not forget to include `table.css` on your page:

<div class="@ns-callout @ns-large @ns-intent-success @ns-icon-star">

There is a new version of the table component compatible with the new hotkeys API, see [Table2](#table/table2).
There is an updated version of the table component with some new features and compatibility with the
[new hotkeys API](#core/components/hotkeys-target2): see [Table2](#table/table2).
</div>

### Features
Expand Down
2 changes: 0 additions & 2 deletions packages/table/src/table2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ import { clampNumFrozenColumns, clampNumFrozenRows, hasLoadingOption } from "./t

export interface Table2Props extends TableProps {
/**
* Warning: experimental feature!
*
* This dependency list may be used to trigger a re-render of all cells when one of its elements changes
* (compared using shallow equality checks). This is done by invalidating the grid, which forces
* TableQuadrantStack to re-render.
Expand Down
17 changes: 17 additions & 0 deletions packages/table/src/tableBodyCells.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ export class TableBodyCells extends AbstractComponent2<ITableBodyCellsProps> {

private batcher = new Batcher<JSX.Element>();

/**
* Set this flag to true in componentDidUpdate() when we call forceUpdate() to avoid an extra
* unnecessary update cycle.
*/
private didForceUpdate = false;

public componentDidMount() {
this.maybeInvokeOnCompleteRender();
}
Expand All @@ -113,11 +119,22 @@ export class TableBodyCells extends AbstractComponent2<ITableBodyCellsProps> {
}

public componentDidUpdate(prevProps: ITableBodyCellsProps) {
if (this.didForceUpdate) {
this.didForceUpdate = false;
return;
}

const shouldResetBatcher = !CoreUtils.shallowCompareKeys(prevProps, this.props, {
exclude: BATCHER_RESET_PROP_KEYS_DENYLIST,
});
if (shouldResetBatcher) {
this.batcher.reset();
// At this point, the batcher is reset, but it doesn't have a chance to re-run since render() is not called
// by default after this lifecycle method. This causes issues like https://github.com/palantir/blueprint/issues/5193.
// We can run forceUpdate() to re-render, but must take care to set a local flag indicating that we are doing so,
// so that this lifecycle method doesn't get re-run as well within the same forced update cycle.
this.didForceUpdate = true;
this.forceUpdate();
}
this.maybeInvokeOnCompleteRender();
}
Expand Down