Skip to content

Commit

Permalink
fix: Editing issues when key columns are not first columns (#2053)
Browse files Browse the repository at this point in the history
Resolves #2036

**Changes Implemented:**
- Changed check for key columns within range **to no longer check**
whether the _start of the range is > # of key columns_ and _end of range
is > # of key columns_ , **but rather**, iterate through and check _if
any of the columns are key columns_

- The check priorly was making it so that the key columns are always the
first columns, which was causing issues with editing columns if they
were not
  • Loading branch information
AkshatJawne authored Jun 7, 2024
1 parent 0943041 commit 1bbcc73
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 19 deletions.
4 changes: 4 additions & 0 deletions packages/grid/src/GridMetricCalculator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,10 @@ export class GridMetricCalculator {
const { model } = state;
const { columnCount, floatingRightColumnCount } = model;

if (columnCount === 0) {
return 0;
}

let lastLeft = Math.max(0, columnCount - floatingRightColumnCount - 1);
if (right != null) {
lastLeft = right;
Expand Down
8 changes: 8 additions & 0 deletions packages/iris-grid/src/IrisGridModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type IrisGridModelEventMap = {
};

const EMPTY_ARRAY: never[] = [];
const EMPTY_SET: Set<never> = new Set();

/**
* Abstract class that extends the GridModel to have more functionality, like filtering and sorting.
Expand Down Expand Up @@ -321,6 +322,13 @@ abstract class IrisGridModel<
return EMPTY_ARRAY;
}

/**
* @returns Names of key columns
*/
get keyColumnSet(): Set<ColumnName> {
return EMPTY_SET;
}

/**
* @returns Names of columns which should be frozen to the front and floating
*/
Expand Down
4 changes: 4 additions & 0 deletions packages/iris-grid/src/IrisGridProxyModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,10 @@ class IrisGridProxyModel extends IrisGridModel implements PartitionedGridModel {
return this.model.frozenColumns;
}

get keyColumnSet(): Set<ColumnName> {
return this.model.keyColumnSet;
}

getColumnHeaderGroup: IrisGridModel['getColumnHeaderGroup'] = (...args) =>
this.model.getColumnHeaderGroup(...args);

Expand Down
29 changes: 21 additions & 8 deletions packages/iris-grid/src/IrisGridTableModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
EventShimCustomEvent,
PromiseUtils,
assertNotNull,
EMPTY_ARRAY,
} from '@deephaven/utils';
import IrisGridModel from './IrisGridModel';
import { ColumnName, UIRow, UITotalsTableConfig } from './CommonTypes';
Expand Down Expand Up @@ -78,21 +79,32 @@ class IrisGridTableModel
return this.table.applyCustomColumns != null;
}

getMemoizedKeyColumnSet = memoize(
(inputTableKeys?: readonly ColumnName[]) =>
new Set(inputTableKeys ?? EMPTY_ARRAY)
);

get keyColumnSet(): Set<ColumnName> {
return this.getMemoizedKeyColumnSet(this.inputTable?.keys);
}

getMemoizedFrontColumns = memoize(
(layoutHintsFrontColumns: ColumnName[] | undefined) =>
layoutHintsFrontColumns ?? []
layoutHintsFrontColumns ?? EMPTY_ARRAY
);

get frontColumns(): ColumnName[] {
return this.getMemoizedFrontColumns(this.layoutHints?.frontColumns ?? []);
get frontColumns(): readonly ColumnName[] {
return this.getMemoizedFrontColumns(
this.layoutHints?.frontColumns ?? undefined
);
}

getMemoizedBackColumns = memoize(
(layoutHintsBackColumns: ColumnName[] | undefined) =>
layoutHintsBackColumns ?? []
layoutHintsBackColumns ?? EMPTY_ARRAY
);

get backColumns(): ColumnName[] {
get backColumns(): readonly ColumnName[] {
return this.getMemoizedBackColumns(
this.layoutHints?.backColumns ?? undefined
);
Expand All @@ -102,10 +114,11 @@ class IrisGridTableModel
(
layoutHintsFrozenColumns?: ColumnName[],
userFrozenColumns?: ColumnName[]
): ColumnName[] => userFrozenColumns ?? layoutHintsFrozenColumns ?? []
): readonly ColumnName[] =>
userFrozenColumns ?? layoutHintsFrozenColumns ?? EMPTY_ARRAY
);

get frozenColumns(): ColumnName[] {
get frozenColumns(): readonly ColumnName[] {
return this.getMemoizedFrozenColumns(
this.layoutHints?.frozenColumns ?? undefined,
this.userFrozenColumns
Expand Down Expand Up @@ -327,7 +340,7 @@ class IrisGridTableModel

assertNotNull(this.inputTable);
const { keyColumns } = this.inputTable;
if (keyColumns.length === 0) {
if (this.keyColumnSet.size === 0) {
throw new Error('No key columns to allow deletion');
}

Expand Down
28 changes: 17 additions & 11 deletions packages/iris-grid/src/IrisGridTableModelTemplate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -458,10 +458,7 @@ class IrisGridTableModelTemplate<
}

get isDeletable(): boolean {
if (this.inputTable !== null) {
return this.inputTable?.keyColumns.length > 0;
}
return false;
return this.keyColumnSet.size > 0;
}

get isViewportPending(): boolean {
Expand Down Expand Up @@ -1625,7 +1622,7 @@ class IrisGridTableModelTemplate<
}

isKeyColumn(x: ModelIndex): boolean {
return x < (this.inputTable?.keyColumns.length ?? 0);
return this.keyColumnSet.has(this.columns[x].name);
}

isRowMovable(): boolean {
Expand All @@ -1644,13 +1641,22 @@ class IrisGridTableModelTemplate<
// Pending rows are always editable
const isPendingRange =
this.isPendingRow(range.startRow) && this.isPendingRow(range.endRow);

let isKeyColumnInRange = false;
// Check if any of the columns in grid range are key columns
for (
let column = range.startColumn;
column <= range.endColumn;
column += 1
) {
if (this.isKeyColumn(column)) {
isKeyColumnInRange = true;
break;
}
}

if (
!(
isPendingRange ||
(this.inputTable.keyColumns.length !== 0 &&
range.startColumn >= this.inputTable.keyColumns.length &&
range.endColumn >= this.inputTable.keyColumns.length)
)
!(isPendingRange || (this.keyColumnSet.size !== 0 && !isKeyColumnInRange))
) {
return false;
}
Expand Down

0 comments on commit 1bbcc73

Please sign in to comment.