From 5156a4abaf73f8f4be2567dccbdc7507f01af52b Mon Sep 17 00:00:00 2001 From: Akshat Jawne <69530774+AkshatJawne@users.noreply.github.com> Date: Mon, 29 Jul 2024 09:17:58 -0600 Subject: [PATCH 1/4] fix: Input Tables cannot paste more rows than number of visible rows (#2152) Resolves https://github.com/deephaven/web-client-ui/issues/2089 --- packages/grid/src/Grid.tsx | 11 +--- packages/iris-grid/src/IrisGridTableModel.js | 59 +++++++++++++------- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/packages/grid/src/Grid.tsx b/packages/grid/src/Grid.tsx index 8d32ed7a3f..b5568526b0 100644 --- a/packages/grid/src/Grid.tsx +++ b/packages/grid/src/Grid.tsx @@ -1326,6 +1326,7 @@ class Grid extends PureComponent { const { columnCount, rowCount } = model; let ranges = selectedRanges; // If each cell is a single selection, we need to update the selection to map to the newly pasted data + // Check for if ( ranges.every( range => @@ -1347,16 +1348,6 @@ class Grid extends PureComponent { this.setSelectedRanges(ranges); } - if ( - !ranges.every( - range => - GridRange.rowCount([range]) === tableHeight && - GridRange.columnCount([range]) === tableWidth - ) - ) { - throw new PasteError('Copy and paste area are not same size.'); - } - const edits: EditOperation[] = []; ranges.forEach(range => { for (let x = 0; x < tableWidth; x += 1) { diff --git a/packages/iris-grid/src/IrisGridTableModel.js b/packages/iris-grid/src/IrisGridTableModel.js index ceacd2642d..5ed9cac20f 100644 --- a/packages/iris-grid/src/IrisGridTableModel.js +++ b/packages/iris-grid/src/IrisGridTableModel.js @@ -37,9 +37,8 @@ class IrisGridTableModel extends IrisGridModel { this.handleTableUpdate = this.handleTableUpdate.bind(this); this.handleTotalsUpdate = this.handleTotalsUpdate.bind(this); this.handleRequestFailed = this.handleRequestFailed.bind(this); - this.handleCustomColumnsChanged = this.handleCustomColumnsChanged.bind( - this - ); + this.handleCustomColumnsChanged = + this.handleCustomColumnsChanged.bind(this); this.setViewport = throttle( this.setViewport.bind(this), SET_VIEWPORT_THROTTLE @@ -654,7 +653,7 @@ class IrisGridTableModel extends IrisGridModel { */ pendingRow(y) { const pendingRow = y - this.floatingTopRowCount - this.table.size; - if (pendingRow >= 0 && pendingRow < this.pendingNewRowCount) { + if (pendingRow >= 0) { return pendingRow; } @@ -726,10 +725,8 @@ class IrisGridTableModel extends IrisGridModel { const operationMap = this.totals?.operationMap; for (let c = 0; c < columns.length; c += 1) { const column = columns[c]; - const [ - name, - operation = operationMap?.[name]?.[0] ?? defaultOperation, - ] = column.name.split('__'); + const [name, operation = operationMap?.[name]?.[0] ?? defaultOperation] = + column.name.split('__'); if (!dataMap.has(operation)) { dataMap.set(operation, { data: new Map() }); } @@ -1260,18 +1257,40 @@ class IrisGridTableModel extends IrisGridModel { } isEditableRange(range) { - return ( - this.inputTable != null && - GridRange.isBounded(range) && - ((this.isPendingRow(range.startRow) && this.isPendingRow(range.endRow)) || - (range.startColumn >= this.inputTable.keyColumns.length && - range.endColumn >= this.inputTable.keyColumns.length)) && - range.startRow >= this.floatingTopRowCount && - range.startRow < - this.floatingTopRowCount + this.table.size + this.pendingRowCount && - range.endRow < - this.floatingTopRowCount + this.table.size + this.pendingRowCount - ); + // Make sure we have an input table and a valid range + if ( + this.inputTable == null || + range.startRow == null || + range.endRow == null + ) { + return false; + } + + // Check that the edit is in the editable range + // If an input table has keyed columns, the non-key columns are always editable + // If an input table does not have key columns, it is append only and existing rows cannot be editable + // 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 + const bound = range.endColumn ?? this.table.size; + for (let column = range.startColumn; column <= bound; column += 1) { + if (this.isKeyColumn(column)) { + isKeyColumnInRange = true; + break; + } + } + + if ( + !(isPendingRange || (this.keyColumnSet.size !== 0 && !isKeyColumnInRange)) + ) { + return false; + } + + return true; } isDeletableRange(range) { From b03df232c5e14371bd8390a3d4c087452e1b60dc Mon Sep 17 00:00:00 2001 From: Akshat Jawne <69530774+AkshatJawne@users.noreply.github.com> Date: Fri, 7 Jun 2024 11:31:49 -0600 Subject: [PATCH 2/4] fix: Editing issues when key columns are not first columns (#2053) Resolves https://github.com/deephaven/web-client-ui/issues/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 --- packages/grid/src/GridMetricCalculator.ts | 8 ++++++++ packages/iris-grid/src/IrisGridModel.ts | 13 +++++++++++++ packages/iris-grid/src/IrisGridTableModel.js | 19 ++++++++++++++----- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/packages/grid/src/GridMetricCalculator.ts b/packages/grid/src/GridMetricCalculator.ts index 1cfac1f455..3d16fac84e 100644 --- a/packages/grid/src/GridMetricCalculator.ts +++ b/packages/grid/src/GridMetricCalculator.ts @@ -764,7 +764,15 @@ export class GridMetricCalculator { const { model } = state; const { columnCount } = model; +<<<<<<< HEAD let lastLeft = columnCount - 1; +======= + if (columnCount === 0) { + return 0; + } + + let lastLeft = Math.max(0, columnCount - floatingRightColumnCount - 1); +>>>>>>> 1bbcc73dda (fix: Editing issues when key columns are not first columns (#2053)) if (right != null) { lastLeft = right; } diff --git a/packages/iris-grid/src/IrisGridModel.ts b/packages/iris-grid/src/IrisGridModel.ts index a597c84693..c0600c0a49 100644 --- a/packages/iris-grid/src/IrisGridModel.ts +++ b/packages/iris-grid/src/IrisGridModel.ts @@ -30,6 +30,12 @@ type IrisGridModelEventMap = { [E in IrisGridModelEventNames]: Event; }; +<<<<<<< HEAD +======= +const EMPTY_ARRAY: never[] = []; +const EMPTY_SET: Set = new Set(); + +>>>>>>> 1bbcc73dda (fix: Editing issues when key columns are not first columns (#2053)) /** * Abstract class that extends the GridModel to have more functionality, like filtering and sorting. * For use from IrisGrid. @@ -272,6 +278,13 @@ abstract class IrisGridModel< return []; } + /** + * @returns Names of key columns + */ + get keyColumnSet(): Set { + return EMPTY_SET; + } + /** * @returns Names of columns which should be frozen to the front and floating */ diff --git a/packages/iris-grid/src/IrisGridTableModel.js b/packages/iris-grid/src/IrisGridTableModel.js index 5ed9cac20f..3adc38085b 100644 --- a/packages/iris-grid/src/IrisGridTableModel.js +++ b/packages/iris-grid/src/IrisGridTableModel.js @@ -15,6 +15,7 @@ const log = Log.module('IrisGridTableModel'); const SET_VIEWPORT_THROTTLE = 150; const APPLY_VIEWPORT_THROTTLE = 0; +const EMPTY_ARRAY = Object.freeze([]); /** * Model for a grid showing an iris data table @@ -543,8 +544,16 @@ class IrisGridTableModel extends IrisGridModel { return this.getMemoizedColumnMap(this.table.columns); } + getMemoizedKeyColumnSet = memoize( + inputTableKeys => new Set(inputTableKeys ?? EMPTY_ARRAY) + ); + + get keyColumnSet() { + return this.getMemoizedKeyColumnSet(this.inputTable?.keys); + } + getMemoizedFrontColumns = memoize( - layoutHintsFrontColumns => layoutHintsFrontColumns ?? [] + layoutHintsFrontColumns => layoutHintsFrontColumns ?? EMPTY_ARRAY ); get frontColumns() { @@ -552,7 +561,7 @@ class IrisGridTableModel extends IrisGridModel { } getMemoizedBackColumns = memoize( - layoutHintsBackColumns => layoutHintsBackColumns ?? [] + layoutHintsBackColumns => layoutHintsBackColumns ?? EMPTY_ARRAY ); get backColumns() { @@ -561,7 +570,7 @@ class IrisGridTableModel extends IrisGridModel { getMemoizedFrozenColumns = memoize( (layoutHintsFrozenColumns, userFrozenColumns) => - userFrozenColumns ?? layoutHintsFrozenColumns ?? [] + userFrozenColumns ?? layoutHintsFrozenColumns ?? EMPTY_ARRAY ); get frozenColumns() { @@ -580,7 +589,7 @@ class IrisGridTableModel extends IrisGridModel { } get groupedColumns() { - return []; + return EMPTY_ARRAY; } get description() { @@ -1249,7 +1258,7 @@ class IrisGridTableModel extends IrisGridModel { } isKeyColumn(x) { - return x < (this.inputTable?.keyColumns.length ?? 0); + return this.keyColumnSet.has(this.columns[x].name); } isRowMovable() { From 87de74660c7f75641f3c7360154748a811eccb41 Mon Sep 17 00:00:00 2001 From: mikebender Date: Wed, 19 Feb 2025 17:13:32 -0500 Subject: [PATCH 3/4] Fix conflicts --- packages/grid/src/GridMetricCalculator.ts | 20 +++++--------------- packages/iris-grid/src/IrisGridModel.ts | 19 +++++++++---------- 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/packages/grid/src/GridMetricCalculator.ts b/packages/grid/src/GridMetricCalculator.ts index 3d16fac84e..aa30384668 100644 --- a/packages/grid/src/GridMetricCalculator.ts +++ b/packages/grid/src/GridMetricCalculator.ts @@ -762,17 +762,13 @@ export class GridMetricCalculator { visibleWidth: number = this.getVisibleWidth(state) ): VisibleIndex { const { model } = state; - const { columnCount } = model; + const { columnCount, floatingRightColumnCount } = model; -<<<<<<< HEAD - let lastLeft = columnCount - 1; -======= if (columnCount === 0) { return 0; } let lastLeft = Math.max(0, columnCount - floatingRightColumnCount - 1); ->>>>>>> 1bbcc73dda (fix: Editing issues when key columns are not first columns (#2053)) if (right != null) { lastLeft = right; } @@ -971,11 +967,8 @@ export class GridMetricCalculator { treePaddingX: number = this.calculateTreePaddingX(state) ): SizeMap { const { model } = state; - const { - columnCount, - floatingLeftColumnCount, - floatingRightColumnCount, - } = model; + const { columnCount, floatingLeftColumnCount, floatingRightColumnCount } = + model; const columnWidths = new Map(); for (let i = 0; i < floatingLeftColumnCount && i < columnCount; i += 1) { @@ -1044,11 +1037,8 @@ export class GridMetricCalculator { maxX: Coordinate ): CoordinateMap { const { model } = state; - const { - columnCount, - floatingLeftColumnCount, - floatingRightColumnCount, - } = model; + const { columnCount, floatingLeftColumnCount, floatingRightColumnCount } = + model; return getFloatingCoordinates( floatingLeftColumnCount, diff --git a/packages/iris-grid/src/IrisGridModel.ts b/packages/iris-grid/src/IrisGridModel.ts index c0600c0a49..b7b76f651d 100644 --- a/packages/iris-grid/src/IrisGridModel.ts +++ b/packages/iris-grid/src/IrisGridModel.ts @@ -23,19 +23,18 @@ import type { import { Formatter } from '@deephaven/jsapi-utils'; type RowIndex = ModelIndex; +type ColumnName = string; -type IrisGridModelEventNames = typeof IrisGridModel.EVENT[keyof typeof IrisGridModel.EVENT]; +type IrisGridModelEventNames = + (typeof IrisGridModel.EVENT)[keyof typeof IrisGridModel.EVENT]; type IrisGridModelEventMap = { [E in IrisGridModelEventNames]: Event; }; -<<<<<<< HEAD -======= const EMPTY_ARRAY: never[] = []; const EMPTY_SET: Set = new Set(); ->>>>>>> 1bbcc73dda (fix: Editing issues when key columns are not first columns (#2053)) /** * Abstract class that extends the GridModel to have more functionality, like filtering and sorting. * For use from IrisGrid. @@ -48,7 +47,7 @@ abstract class IrisGridModel< string, Event >, - TMode extends 'standard' | 'strict' = 'standard' + TMode extends 'standard' | 'strict' = 'standard', > extends GridModel { static EVENT = Object.freeze({ UPDATED: 'UPDATED', @@ -140,12 +139,12 @@ abstract class IrisGridModel< /** List of column movements defined by the model. Used as initial movements for IrisGrid */ get movedColumns(): MoveOperation[] { - return []; + return EMPTY_ARRAY; } /** List of row movements defined by the model. Used as initial movements for IrisGrid */ get movedRows(): MoveOperation[] { - return []; + return EMPTY_ARRAY; } /** @@ -268,14 +267,14 @@ abstract class IrisGridModel< * @returns Names of columns which should be locked to the front, but not floating */ get frontColumns(): string[] { - return []; + return EMPTY_ARRAY; } /** * @returns Names of columns which should be locked to the back, but not floating */ get backColumns(): string[] { - return []; + return EMPTY_ARRAY; } /** @@ -289,7 +288,7 @@ abstract class IrisGridModel< * @returns Names of columns which should be frozen to the front and floating */ get frozenColumns(): string[] { - return []; + return EMPTY_ARRAY; } /** From 63aabbe21da4e96750d16491f996f99b6150b3f7 Mon Sep 17 00:00:00 2001 From: mikebender Date: Thu, 20 Feb 2025 12:57:31 -0500 Subject: [PATCH 4/4] Fix formatting --- packages/grid/src/GridMetricCalculator.ts | 14 ++++++++++---- packages/iris-grid/src/IrisGridModel.ts | 5 ++--- packages/iris-grid/src/IrisGridTableModel.js | 11 +++++++---- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/grid/src/GridMetricCalculator.ts b/packages/grid/src/GridMetricCalculator.ts index aa30384668..b929819701 100644 --- a/packages/grid/src/GridMetricCalculator.ts +++ b/packages/grid/src/GridMetricCalculator.ts @@ -967,8 +967,11 @@ export class GridMetricCalculator { treePaddingX: number = this.calculateTreePaddingX(state) ): SizeMap { const { model } = state; - const { columnCount, floatingLeftColumnCount, floatingRightColumnCount } = - model; + const { + columnCount, + floatingLeftColumnCount, + floatingRightColumnCount, + } = model; const columnWidths = new Map(); for (let i = 0; i < floatingLeftColumnCount && i < columnCount; i += 1) { @@ -1037,8 +1040,11 @@ export class GridMetricCalculator { maxX: Coordinate ): CoordinateMap { const { model } = state; - const { columnCount, floatingLeftColumnCount, floatingRightColumnCount } = - model; + const { + columnCount, + floatingLeftColumnCount, + floatingRightColumnCount, + } = model; return getFloatingCoordinates( floatingLeftColumnCount, diff --git a/packages/iris-grid/src/IrisGridModel.ts b/packages/iris-grid/src/IrisGridModel.ts index b7b76f651d..b0027b8e36 100644 --- a/packages/iris-grid/src/IrisGridModel.ts +++ b/packages/iris-grid/src/IrisGridModel.ts @@ -25,8 +25,7 @@ import { Formatter } from '@deephaven/jsapi-utils'; type RowIndex = ModelIndex; type ColumnName = string; -type IrisGridModelEventNames = - (typeof IrisGridModel.EVENT)[keyof typeof IrisGridModel.EVENT]; +type IrisGridModelEventNames = typeof IrisGridModel.EVENT[keyof typeof IrisGridModel.EVENT]; type IrisGridModelEventMap = { [E in IrisGridModelEventNames]: Event; @@ -47,7 +46,7 @@ abstract class IrisGridModel< string, Event >, - TMode extends 'standard' | 'strict' = 'standard', + TMode extends 'standard' | 'strict' = 'standard' > extends GridModel { static EVENT = Object.freeze({ UPDATED: 'UPDATED', diff --git a/packages/iris-grid/src/IrisGridTableModel.js b/packages/iris-grid/src/IrisGridTableModel.js index 3adc38085b..ebfd30de58 100644 --- a/packages/iris-grid/src/IrisGridTableModel.js +++ b/packages/iris-grid/src/IrisGridTableModel.js @@ -38,8 +38,9 @@ class IrisGridTableModel extends IrisGridModel { this.handleTableUpdate = this.handleTableUpdate.bind(this); this.handleTotalsUpdate = this.handleTotalsUpdate.bind(this); this.handleRequestFailed = this.handleRequestFailed.bind(this); - this.handleCustomColumnsChanged = - this.handleCustomColumnsChanged.bind(this); + this.handleCustomColumnsChanged = this.handleCustomColumnsChanged.bind( + this + ); this.setViewport = throttle( this.setViewport.bind(this), SET_VIEWPORT_THROTTLE @@ -734,8 +735,10 @@ class IrisGridTableModel extends IrisGridModel { const operationMap = this.totals?.operationMap; for (let c = 0; c < columns.length; c += 1) { const column = columns[c]; - const [name, operation = operationMap?.[name]?.[0] ?? defaultOperation] = - column.name.split('__'); + const [ + name, + operation = operationMap?.[name]?.[0] ?? defaultOperation, + ] = column.name.split('__'); if (!dataMap.has(operation)) { dataMap.set(operation, { data: new Map() }); }