[Table] Better column reordering (#1462)

* Get rid of unneeded TABLE_DRAGGABLE class

* Always show reorder handle if column header cell is reorderable

* Add new state classes

* Enable column reordering in example

* Style the reorder handle

* Newly reordered region becomes the only selection

* Fix reordering in example when selection disabled

* Fix editable-cell border in column header

* Fix reordering tests

* Try to fix stylelint errors

* Try again

* Shorter CSS classes

* [Experimental] On handle mousedown, select only the region that will be reordered

* Revert "[Experimental] On handle mousedown, select only the region that will be reordered"

This reverts commit bdcc845.

Actually, this leads to some buggy behavior with overlapping regions.
Will consider this again later.

* Square off icon margin

* Respond to CR feedback

* Cleaner test setup now that handle is wider

* Fix reordering docs

packages/table/preview/index.tsx

@@ -163,7 +163,7 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
             enableCellSelection: true,
             enableCellTruncation: false,
             enableColumnNameEditing: false,
-            enableColumnReordering: false,
+            enableColumnReordering: true,
             enableColumnResizing: false,
             enableColumnSelection: true,
             enableContextMenu: false,
@@ -637,11 +637,13 @@ class MutableTable extends React.Component<{}, IMutableTableState> {
     private onColumnsReordered = (oldIndex: number, newIndex: number, length: number) => {
         this.maybeLogCallback(`[onColumnsReordered] oldIndex = ${oldIndex} newIndex = ${newIndex} length = ${length}`);
         this.store.reorderColumns(oldIndex, newIndex, length);
+        this.forceUpdate();
     }
 
     private onRowsReordered = (oldIndex: number, newIndex: number, length: number) => {
         this.maybeLogCallback(`[onRowsReordered] oldIndex = ${oldIndex} newIndex = ${newIndex} length = ${length}`);
         this.store.reorderRows(oldIndex, newIndex, length);
+        this.forceUpdate();
     }
 
     private onColumnWidthChanged = (index: number, size: number) => {

packages/table/src/common/classes.ts

@@ -17,13 +17,15 @@ export const TABLE_CELL_LEDGER_EVEN = "bp-table-cell-ledger-even";
 export const TABLE_CELL_LEDGER_ODD = "bp-table-cell-ledger-odd";
 export const TABLE_COLUMN_HEADER_TR = "bp-table-column-header-tr";
 export const TABLE_COLUMN_HEADERS = "bp-table-column-headers";
+export const TABLE_COLUMN_HEADER_CELL = "bp-table-column-header-cell";
 export const TABLE_COLUMN_NAME = "bp-table-column-name";
 export const TABLE_COLUMN_NAME_TEXT = "bp-table-column-name-text";
 export const TABLE_CONTAINER = "bp-table-container";
-export const TABLE_DRAGGABLE = "bp-table-draggable";
 export const TABLE_DRAGGING = "bp-table-dragging";
 export const TABLE_EDITABLE_NAME = "bp-table-editable-name";
 export const TABLE_FOCUS_REGION = "bp-table-focus-region";
+export const TABLE_HAS_INTERACTION_BAR = "bp-table-has-interaction-bar";
+export const TABLE_HAS_REORDER_HANDLE = "bp-table-has-reorder-handle";
 export const TABLE_HEADER = "bp-table-header";
 export const TABLE_HEADER_ACTIVE = "bp-table-header-active";
 export const TABLE_HEADER_CONTENT = "bp-table-header-content";

packages/table/src/docs.md

@@ -96,21 +96,51 @@ regular expression (`[a-zA-Z]`). If the content is invalid, a
 
 @### Reordering
 
-The table supports column and row reordering via the `isColumnReorderable` and `isRowReorderable`
-props, respectively. The table also requires the `FULL_COLUMNS` selection mode to be enabled for
-column reordering and the `FULL_ROWS` selection mode to be enabled for row reordering; these can be
-set via the `selectionModes` prop.
-
-To reorder a single row or column, first click its header cell to select it, then click and drag the
-header cell again to move it elsewhere. Likewise, to reorder multiple consecutive rows or columns at
-once, click and drag across multiple header cells to select the range, then click and drag anywhere in
-the selected header cells to move them as a group.
-
-<div class="pt-callout pt-intent-primary pt-icon-info-sign">
-    <h5>Column reordering with interaction bar enabled</h5>
-    When the interaction bar is enabled, the table will show handle icons in the interaction bar that
-    you can drag directly without having to make a selection first.
-</div>
+The table supports drag-reordering of columns and rows via the `isColumnReorderable` and `isRowReorderable`
+props, respectively.
+
+#### Reordering columns
+
+When `isColumnReorderable={true}`, a drag handle will appear in the column header (or in the
+interaction bar, if `useInteractionBar={true}`).
+
+##### Single column
+
+To reorder a single column, click and drag the desired column's drag handle to the left or right,
+then release. This will work whether or not column selection is enabled.
+
+##### Multiple columns
+
+To allow reordering of multiple contiguous columns at once, first set the following additional
+props:
+
+- `allowMultipleSelection={true}`
+- `selectionModes={[RegionCardinality.FULL_COLUMNS, ...]}`
+
+Then drag-select the desired columns into a single selection, and grab any selected column's drag
+handle to reorder the entire selected block.
+
+##### Edge cases
+
+With disjoint column selections (specified via <kbd>Cmd</kbd> or <kbd>Ctrl</kbd> + click),
+only the selection containing the target drag handle will be reordered. All other
+selections will be cleared afterward.
+
+Reordering a column contained in two overlapping selections will currently result in undefined
+behavior.
+
+#### Reordering rows
+
+Rows do not have a drag handle, so they must be selected before reordering. To reorder a selection
+of one or more rows, simply click and drag anywhere in a selected row header, then release. Note
+that the following props must be set for row reordering to work:
+
+- `isRowHeaderShown={true}`
+- `isRowReorderable={true}`
+- `selectionModes={[RegionCardinality.FULL_ROWS, ...]}`
+- `allowMultipleSelection={true}` (to optionally enable multi-row reordering)
+
+#### Example
 
 @reactExample TableReorderableExample
 

packages/table/src/headers/columnHeader.tsx

@@ -116,9 +116,7 @@ private wrapCells = (cells: Array<React.ReactElement<any>>) => {
         width: tableWidth - scrollLeftCorrection,
     };
 
-    const classes = classNames(Classes.TABLE_THEAD, Classes.TABLE_COLUMN_HEADER_TR, {
-        [Classes.TABLE_DRAGGABLE] : (this.props.onSelection != null),
-    });
+    const classes = classNames(Classes.TABLE_THEAD, Classes.TABLE_COLUMN_HEADER_TR);
 
     // add a wrapper set to the full-table width to ensure container styles stretch from the first
     // cell all the way to the last

packages/table/src/headers/columnHeaderCell.tsx

@@ -123,11 +123,17 @@ export class ColumnHeaderCell extends AbstractComponent<IColumnHeaderCellProps,
             ...spreadableProps,
         } = this.props;
 
+        const classes = classNames(spreadableProps.className, Classes.TABLE_COLUMN_HEADER_CELL, {
+            [Classes.TABLE_HAS_INTERACTION_BAR]: useInteractionBar,
+            [Classes.TABLE_HAS_REORDER_HANDLE]: this.props.reorderHandle != null,
+        });
+
         return (
             <HeaderCell
                 isReorderable={this.props.isColumnReorderable}
                 isSelected={this.props.isColumnSelected}
                 {...spreadableProps}
+                className={classes}
             >
                 {this.renderName()}
                 {this.maybeRenderContent()}
@@ -146,7 +152,7 @@ export class ColumnHeaderCell extends AbstractComponent<IColumnHeaderCellProps,
     }
 
     private renderName() {
-        const { index, loading, name, renderName, useInteractionBar } = this.props;
+        const { index, loading, name, renderName, reorderHandle, useInteractionBar } = this.props;
 
         const dropdownMenu = this.maybeRenderDropdownMenu();
         const defaultName = <div className={Classes.TABLE_TRUNCATED_TEXT}>{name}</div>;
@@ -161,7 +167,7 @@ export class ColumnHeaderCell extends AbstractComponent<IColumnHeaderCellProps,
             return (
                 <div className={Classes.TABLE_COLUMN_NAME} title={name}>
                     <div className={Classes.TABLE_INTERACTION_BAR}>
-                        {this.props.reorderHandle}
+                        {reorderHandle}
                         {dropdownMenu}
                     </div>
                     <HorizontalCellDivider />
@@ -171,6 +177,7 @@ export class ColumnHeaderCell extends AbstractComponent<IColumnHeaderCellProps,
         } else {
             return (
                 <div className={Classes.TABLE_COLUMN_NAME} title={name}>
+                    {reorderHandle}
                     {dropdownMenu}
                     <div className={Classes.TABLE_COLUMN_NAME_TEXT}>{nameComponent}</div>
                 </div>

packages/table/src/headers/header.tsx

@@ -303,16 +303,15 @@ export class Header extends React.Component<IInternalHeaderProps, IHeaderState>
     }
 
     private renderCell = (index: number, extremaClasses: string[]) => {
-        const { getIndexClass, onSelection, selectedRegions } = this.props;
+        const { getIndexClass, selectedRegions } = this.props;
 
         const cell = this.props.renderHeaderCell(index);
 
         const isLoading = cell.props.loading != null ? cell.props.loading : this.props.loading;
         const isSelected = this.props.isCellSelected(index);
-        const isEntireCellTargetReorderable = this.isEntireCellTargetReorderable(cell, isSelected);
+        const isEntireCellTargetReorderable = this.isEntireCellTargetReorderable(isSelected);
 
         const className = classNames(extremaClasses, {
-            [Classes.TABLE_DRAGGABLE]: onSelection != null,
             [Classes.TABLE_HEADER_REORDERABLE]: isEntireCellTargetReorderable,
         }, this.props.getCellIndexClass(index), cell.props.className);
 
@@ -322,7 +321,7 @@ export class Header extends React.Component<IInternalHeaderProps, IHeaderState>
             [this.props.headerCellIsSelectedPropName]: isSelected,
             [this.props.headerCellIsReorderablePropName]: isEntireCellTargetReorderable,
             loading: isLoading,
-            reorderHandle: this.maybeRenderReorderHandle(cell, index),
+            reorderHandle: this.maybeRenderReorderHandle(index),
         };
 
         const modifiedHandleSizeChanged = (size: number) => this.props.handleSizeChanged(index, size);
@@ -333,7 +332,7 @@ export class Header extends React.Component<IInternalHeaderProps, IHeaderState>
             <DragSelectable
                 allowMultipleSelection={this.props.allowMultipleSelection}
                 disabled={isEntireCellTargetReorderable}
-                ignoredSelectors={[`.${Classes.TABLE_REORDER_HANDLE}`]}
+                ignoredSelectors={[`.${Classes.TABLE_REORDER_HANDLE_TARGET}`]}
                 key={getIndexClass(index)}
                 locateClick={this.locateClick}
                 locateDrag={this.locateDragForSelection}
@@ -359,18 +358,18 @@ export class Header extends React.Component<IInternalHeaderProps, IHeaderState>
             </DragSelectable>
         );
 
-        return this.isReorderHandleEnabled(cell)
+        return this.isReorderHandleEnabled()
             ? baseChildren // reordering will be handled by interacting with the reorder handle
             : this.wrapInDragReorderable(index, baseChildren, !isEntireCellTargetReorderable);
     }
 
-    private isReorderHandleEnabled(cell: JSX.Element) {
+    private isReorderHandleEnabled() {
         // the reorder handle can only appear in the column interaction bar
-        return this.isColumnHeader() && cell.props.useInteractionBar && this.props.isReorderable;
+        return this.isColumnHeader() && this.props.isReorderable;
     }
 
-    private maybeRenderReorderHandle(cell: JSX.Element, index: number) {
-        return !this.isReorderHandleEnabled(cell)
+    private maybeRenderReorderHandle(index: number) {
+        return !this.isReorderHandleEnabled()
             ? undefined
             : this.wrapInDragReorderable(index,
                 <div className={Classes.TABLE_REORDER_HANDLE_TARGET}>
@@ -412,7 +411,7 @@ export class Header extends React.Component<IInternalHeaderProps, IHeaderState>
         this.setState({ hasSelectionEnded: true });
     }
 
-    private isEntireCellTargetReorderable = (cell: JSX.Element, isSelected: boolean) => {
+    private isEntireCellTargetReorderable = (isSelected: boolean) => {
         const { selectedRegions } = this.props;
         // although reordering may be generally enabled for this row/column (via props.isReorderable), the
         // row/column shouldn't actually become reorderable from a user perspective until a few other
@@ -430,7 +429,7 @@ export class Header extends React.Component<IInternalHeaderProps, IHeaderState>
             // both selection and reordering behavior.
             && selectedRegions.length === 1
             // columns are reordered via a reorder handle, so drag-selection needn't be disabled
-            && !this.isReorderHandleEnabled(cell);
+            && !this.isReorderHandleEnabled();
     }
 }
 

packages/table/src/interactions/_interactions.scss

@@ -9,6 +9,10 @@ $resize-handle-padding: 2px !default;
 $resize-handle-color: $pt-intent-primary !default;
 $resize-handle-dragging-color: $pt-intent-primary !default;
 
+// we want to square off the margin around the handle
+// so we need a special value based on the handle icon shape
+$reorder-handle-width: 22px !default;
+
 @mixin grabbable() {
   cursor: grab;
 
@@ -48,24 +52,35 @@ $resize-handle-dragging-color: $pt-intent-primary !default;
   }
 }
 
+.bp-table-column-header-cell.bp-table-has-reorder-handle:not(.bp-table-has-interaction-bar) {
+  .bp-table-column-name-text {
+    padding-left: $reorder-handle-width;
+  }
+
+  // keep the editable cell input flush with the cell's left border
+  .bp-table-editable-name::before {
+    left: -$reorder-handle-width;
+  }
+}
+
 .bp-table-reorder-handle-target {
   @include grabbable();
+  display: flex;
+  align-items: center;
+  justify-content: center;
   position: absolute;
   top: 0;
+  bottom: 0;
   left: 0;
-  color: $gray3;
+  width: $reorder-handle-width;
+  color: $pt-text-color-disabled;
 
   &:hover {
-    color: $pt-text-color-muted;
+    color: $pt-icon-color-hover;
   }
 
   &:active {
-    color: $pt-text-color;
-  }
-
-  .bp-table-reorder-handle {
-    padding: 3px 6px 3px 1px;
-    line-height: 14px;
+    color: $pt-intent-primary;
   }
 }
 

packages/table/src/interactions/reorderable.tsx

@@ -155,8 +155,9 @@ export class DragReorderable extends React.Component<IDragReorderable, {}> {
         const reorderedIndex = Utils.guideIndexToReorderedIndex(oldIndex, guideIndex, length);
         this.props.onReordered(oldIndex, reorderedIndex, length);
 
+        // the newly reordered region becomes the only selection
         const newRegion = this.props.toRegion(reorderedIndex, reorderedIndex + length - 1);
-        this.props.onSelection(Regions.update(this.props.selectedRegions, newRegion));
+        this.props.onSelection(Regions.update([], newRegion));
 
         // resetting is not strictly required, but it's cleaner
         this.selectedRegionStartIndex = undefined;

packages/table/test/columnHeaderCellTests.tsx

@@ -143,22 +143,18 @@ describe("<ColumnHeaderCell>", () => {
     });
 
     describe("Reorder handle", () => {
-        const REORDER_HANDLE_CLASS = "reorder-handle";
+        const REORDER_HANDLE_CLASS = Classes.TABLE_REORDER_HANDLE_TARGET;
 
         it("shows reorder handle in interaction bar if reordering and interaction bar are enabled", () => {
             const element = mount({ useInteractionBar: true, isColumnReorderable: true });
-            expect(doesReorderHandleExist(element)).to.be.true;
+            expect(element.find(`.${Classes.TABLE_INTERACTION_BAR} .${REORDER_HANDLE_CLASS}`).exists()).to.be.true;
         });
 
-        it("hides reorder handle if reordering enabled but interaction bar disabled", () => {
+        it("shows reorder handle next to column name if reordering enabled but interaction bar disabled", () => {
             const element = mount({ useInteractionBar: false, isColumnReorderable: true });
-            expect(doesReorderHandleExist(element)).to.be.false;
+            expect(element.find(`.${Classes.TABLE_COLUMN_NAME} .${REORDER_HANDLE_CLASS}`).exists()).to.be.true;
         });
 
-        function doesReorderHandleExist(element: ElementHarness) {
-            return element.find(`.${REORDER_HANDLE_CLASS}`).exists();
-        }
-
         function mount(props: Partial<IColumnHeaderCellProps> & object) {
             const element = harness.mount(
                 <ColumnHeaderCell