feat: disable column sorting on unsupported types (#1390)

Closes #1380
deephaven · Jul 6, 2023 · 3a89bbf · 3a89bbf
1 parent c76730f
commit 3a89bbf
Show file tree

Hide file tree

Showing 9 changed files with 73 additions and 19 deletions.
diff --git a/packages/iris-grid/src/AdvancedFilterCreator.test.tsx b/packages/iris-grid/src/AdvancedFilterCreator.test.tsx
@@ -54,7 +54,7 @@ function makeAdvancedFilterCreatorWrapper(
       selectedValues: [],
     },
     model: irisGridTestUtils.makeModel(),
-    column: irisGridTestUtils.makeColumn(),
+    column: irisGridTestUtils.makeColumn('0'), // needs to match column name in makeModel so that columnIndex can be found when rendering
     formatter: new Formatter(dh),
   }
 ) {

diff --git a/packages/iris-grid/src/AdvancedFilterCreator.tsx b/packages/iris-grid/src/AdvancedFilterCreator.tsx
@@ -20,7 +20,11 @@ import {
 } from '@deephaven/jsapi-utils';
 import { Button, ContextActionUtils } from '@deephaven/components';
 import Log from '@deephaven/log';
-import { CancelablePromise, PromiseUtils } from '@deephaven/utils';
+import {
+  assertNotNull,
+  CancelablePromise,
+  PromiseUtils,
+} from '@deephaven/utils';
 import type { Column, FilterCondition, Table } from '@deephaven/jsapi-types';
 import shortid from 'shortid';
 import AdvancedFilterCreatorFilterItem from './AdvancedFilterCreatorFilterItem';
@@ -75,6 +79,8 @@ interface AdvancedFilterCreatorState {
 
   valuesTableError: null;
   valuesTable?: Table;
+
+  isSortable: boolean;
 }
 
 class AdvancedFilterCreator extends PureComponent<
@@ -119,7 +125,7 @@ class AdvancedFilterCreator extends PureComponent<
 
     this.focusTrapContainer = React.createRef();
 
-    const { options } = props;
+    const { model, column, options } = props;
     let { filterOperators, invertSelection, selectedValues } = options;
 
     // can be null or an empty array
@@ -142,6 +148,10 @@ class AdvancedFilterCreator extends PureComponent<
       selectedValues = [];
     }
 
+    const columnIndex = model.getColumnIndexByName(column.name);
+    assertNotNull(columnIndex);
+    const isSortable = model.isColumnSortable(columnIndex);
+
     this.state = {
       // Filter items
       filterItems,
@@ -155,6 +165,8 @@ class AdvancedFilterCreator extends PureComponent<
 
       valuesTableError: null,
       valuesTable: undefined,
+
+      isSortable,
     };
   }
 
@@ -202,9 +214,10 @@ class AdvancedFilterCreator extends PureComponent<
     );
     this.valuesTablePromise
       .then(valuesTable => {
-        const sort = valuesTable.columns[0].sort().asc();
-        valuesTable.applySort([sort]);
-
+        if (valuesTable.columns[0].isSortable ?? true) {
+          const sort = valuesTable.columns[0].sort().asc();
+          valuesTable.applySort([sort]);
+        }
         this.setState({ valuesTable });
       })
       .catch(error => {
@@ -384,7 +397,10 @@ class AdvancedFilterCreator extends PureComponent<
    */
   sortTable(direction: SortDirection, addToExisting = false): void {
     const { column, onSortChange } = this.props;
-    onSortChange(column, direction, addToExisting);
+    const { isSortable } = this.state;
+    if (isSortable) {
+      onSortChange(column, direction, addToExisting);
+    }
   }
 
   startUpdateTimer(): void {
@@ -454,6 +470,7 @@ class AdvancedFilterCreator extends PureComponent<
       selectedValues,
       valuesTable,
       valuesTableError,
+      isSortable,
     } = this.state;
     const { dh, isValuesTableAvailable } = model;
     const isBoolean = TableUtils.isBooleanType(column.type);
@@ -564,7 +581,10 @@ class AdvancedFilterCreator extends PureComponent<
                 })}
                 onClick={this.handleSortDown}
                 icon={dhSortAmountDown}
-                tooltip={`Sort ${column.name} Descending`}
+                tooltip={
+                  isSortable ? `Sort ${column.name} Descending` : 'Not sortable'
+                }
+                disabled={!isSortable}
               />
               <Button
                 kind="ghost"
@@ -575,7 +595,10 @@ class AdvancedFilterCreator extends PureComponent<
                 icon={
                   <FontAwesomeIcon icon={dhSortAmountDown} rotation={180} />
                 }
-                tooltip={`Sort ${column.name} Ascending`}
+                tooltip={
+                  isSortable ? `Sort ${column.name} Ascending` : 'Not sortable'
+                }
+                disabled={!isSortable}
               />
             </div>
           </div>

diff --git a/packages/iris-grid/src/ColumnStatistics.tsx b/packages/iris-grid/src/ColumnStatistics.tsx
@@ -2,7 +2,7 @@ import React, { Component, Key } from 'react';
 import classNames from 'classnames';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { Button, CopyButton, LoadingSpinner } from '@deephaven/components';
-import { dhFreeze, dhRefresh, vsLock } from '@deephaven/icons';
+import { dhFreeze, dhRefresh, dhSortSlash, vsLock } from '@deephaven/icons';
 import type {
   Column,
   ColumnStatistics as APIColumnStatistics,
@@ -203,6 +203,12 @@ class ColumnStatistics extends Component<
         {description != null && (
           <div className="column-statistics-description">{description}</div>
         )}
+        {columnIndex != null && !model.isColumnSortable(columnIndex) && (
+          <div className="column-statistics-status">
+            <FontAwesomeIcon icon={dhSortSlash} className="mr-1" />
+            Not sortable
+          </div>
+        )}
         {columnIndex != null && !model.isColumnMovable(columnIndex) && (
           <div className="column-statistics-status">
             <FontAwesomeIcon

diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx
@@ -2401,16 +2401,21 @@ export class IrisGrid extends Component<IrisGridProps, IrisGridState> {
     log.info('Toggling sort for column', columnIndex);
 
     const { model } = this.props;
-    const { sorts: currentSorts } = this.state;
     const modelColumn = this.getModelColumn(columnIndex);
     assertNotNull(modelColumn);
-    const sorts = TableUtils.toggleSortForColumn(
-      currentSorts,
-      model.columns,
-      modelColumn,
-      addToExisting
-    );
-    this.updateSorts(sorts);
+    if (model.isColumnSortable(columnIndex)) {
+      const { sorts: currentSorts } = this.state;
+      const sorts = TableUtils.toggleSortForColumn(
+        currentSorts,
+        model.columns,
+        modelColumn,
+        addToExisting
+      );
+
+      this.updateSorts(sorts);
+    } else {
+      log.debug('Column type was not sortable', model.columns[columnIndex]);
+    }
   }
 
   updateSorts(sorts: readonly Sort[]): void {

diff --git a/packages/iris-grid/src/IrisGridModel.ts b/packages/iris-grid/src/IrisGridModel.ts
@@ -291,6 +291,14 @@ abstract class IrisGridModel<
     return false;
   }
 
+  /**
+   * @param index The column index to check
+   * @returns Whether the column is sortable
+   */
+  isColumnSortable(index: ModelIndex): boolean {
+    return false;
+  }
+
   /**
    * @returns True if this model requires a filter to be set
    */

diff --git a/packages/iris-grid/src/IrisGridProxyModel.ts b/packages/iris-grid/src/IrisGridProxyModel.ts
@@ -273,6 +273,10 @@ class IrisGridProxyModel extends IrisGridModel {
     return this.model.isColumnFrozen(x);
   }
 
+  isColumnSortable(index: number): boolean {
+    return this.model.isColumnSortable(index);
+  }
+
   get hasExpandableRows(): boolean {
     if (isExpandableGridModel(this.model)) {
       return this.model.hasExpandableRows;

diff --git a/packages/iris-grid/src/IrisGridTableModel.ts b/packages/iris-grid/src/IrisGridTableModel.ts
@@ -269,6 +269,10 @@ class IrisGridTableModel extends IrisGridTableModelTemplate<Table, UIRow> {
     return this.frozenColumns.includes(this.columns[modelIndex].name);
   }
 
+  isColumnSortable(modelIndex: ModelIndex): boolean {
+    return this.columns[modelIndex].isSortable ?? true;
+  }
+
   async delete(ranges: GridRange[]): Promise<void> {
     if (!this.isDeletableRanges(ranges)) {
       throw new Error(`Undeletable ranges ${ranges}`);

diff --git a/packages/iris-grid/src/mousehandlers/IrisGridContextMenuHandler.tsx b/packages/iris-grid/src/mousehandlers/IrisGridContextMenuHandler.tsx
@@ -203,6 +203,7 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
       model.getColumnHeaderParentGroup(modelIndex, 0) === undefined &&
       !(isExpandableGridModel(model) && model.hasExpandableRows);
     const isColumnFrozen = model.isColumnFrozen(modelIndex);
+    const isColumnSortable = model.isColumnSortable(modelIndex);
     actions.push({
       title: 'Hide Column',
       group: IrisGridContextMenuHandler.GROUP_HIDE_COLUMNS,
@@ -274,6 +275,7 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
       group: IrisGridContextMenuHandler.GROUP_SORT,
       order: 10,
       actions: this.sortByActions(column, modelIndex, columnSort),
+      disabled: !isColumnSortable,
     });
     actions.push({
       title: 'Add Additional Sort',
@@ -289,7 +291,8 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
         (columnSort && modelSort.length === 1) ||
         (hasReverse && modelSort.length === 1) ||
         (columnSort && hasReverse && modelSort.length === 2) ||
-        modelSort.length === 0,
+        modelSort.length === 0 ||
+        !isColumnSortable,
       group: IrisGridContextMenuHandler.GROUP_SORT,
       order: 20,
       actions: this.additionalSortActions(column, modelIndex),

diff --git a/packages/jsapi-types/src/dh.types.ts b/packages/jsapi-types/src/dh.types.ts
@@ -558,6 +558,7 @@ export interface Column {
   readonly constituentType: string;
 
   readonly isPartitionColumn: boolean;
+  readonly isSortable?: boolean;
 
   filter(): FilterValue;
   sort(): Sort;