From 0b45a6c4b7f1180230138b0cf0f9a0b9b3849c0d Mon Sep 17 00:00:00 2001 From: mikebender Date: Thu, 16 Nov 2023 11:32:34 -0500 Subject: [PATCH 1/5] Remove duplicated function - It's already defined in IrisGridTableModelTemplate which this class extends, and was the exact same code --- packages/iris-grid/src/IrisGridTableModel.ts | 47 -------------------- 1 file changed, 47 deletions(-) diff --git a/packages/iris-grid/src/IrisGridTableModel.ts b/packages/iris-grid/src/IrisGridTableModel.ts index 96e275ebfd..146a4b99df 100644 --- a/packages/iris-grid/src/IrisGridTableModel.ts +++ b/packages/iris-grid/src/IrisGridTableModel.ts @@ -185,53 +185,6 @@ class IrisGridTableModel extends IrisGridTableModelTemplate { ); } - set totalsConfig(totalsConfig: UITotalsTableConfig | null) { - log.debug('set totalsConfig', totalsConfig); - - if (totalsConfig === this.totals) { - // Totals already set, or it will be set when the next model actually gets set - return; - } - - this.totals = totalsConfig; - this.formattedStringData = []; - - if (this.totalsTablePromise != null) { - this.totalsTablePromise.cancel(); - } - - this.setTotalsTable(null); - - if (totalsConfig == null) { - this.dispatchEvent(new EventShimCustomEvent(IrisGridModel.EVENT.UPDATED)); - return; - } - - this.totalsTablePromise = PromiseUtils.makeCancelable( - this.table.getTotalsTable(totalsConfig), - table => table.close() - ); - this.totalsTablePromise - .then(totalsTable => { - this.totalsTablePromise = null; - this.setTotalsTable(totalsTable); - }) - .catch(err => { - if (PromiseUtils.isCanceled(err)) { - return; - } - - log.error('Unable to set next totalsTable', err); - this.totalsTablePromise = null; - - this.dispatchEvent( - new EventShimCustomEvent(IrisGridModel.EVENT.REQUEST_FAILED, { - detail: err, - }) - ); - }); - } - get isFilterRequired(): boolean { return this.table.isUncoalesced; } From d754241116ec2105c7fb32009ea7f7709b38e110 Mon Sep 17 00:00:00 2001 From: mikebender Date: Thu, 16 Nov 2023 11:36:28 -0500 Subject: [PATCH 2/5] Use the `Skip` aggregation operation by default - Unless the user specifies it, we should just skip it --- packages/iris-grid/src/IrisGrid.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx index 664acd405d..7aec5ff6b1 100644 --- a/packages/iris-grid/src/IrisGrid.tsx +++ b/packages/iris-grid/src/IrisGrid.tsx @@ -1287,7 +1287,12 @@ export class IrisGrid extends Component { const operationMap = this.getOperationMap(columns, aggregations); const operationOrder = this.getOperationOrder(aggregations); - return { operationMap, operationOrder, showOnTop }; + return { + operationMap, + operationOrder, + showOnTop, + defaultOperation: AggregationOperation.SKIP, + }; } ); From bececf6364ec6bada6b2106303b5721ee37d5f38 Mon Sep 17 00:00:00 2001 From: mikebender Date: Thu, 16 Nov 2023 11:39:55 -0500 Subject: [PATCH 3/5] Clean up unused code --- packages/iris-grid/src/IrisGridTableModel.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/iris-grid/src/IrisGridTableModel.ts b/packages/iris-grid/src/IrisGridTableModel.ts index 146a4b99df..900d9eecdb 100644 --- a/packages/iris-grid/src/IrisGridTableModel.ts +++ b/packages/iris-grid/src/IrisGridTableModel.ts @@ -13,13 +13,9 @@ import type { } from '@deephaven/jsapi-types'; import Log from '@deephaven/log'; import { Formatter } from '@deephaven/jsapi-utils'; -import { - EventShimCustomEvent, - PromiseUtils, - assertNotNull, -} from '@deephaven/utils'; +import { EventShimCustomEvent, assertNotNull } from '@deephaven/utils'; import IrisGridModel from './IrisGridModel'; -import { ColumnName, UITotalsTableConfig, UIRow } from './CommonTypes'; +import { ColumnName, UIRow } from './CommonTypes'; import IrisGridTableModelTemplate from './IrisGridTableModelTemplate'; const log = Log.module('IrisGridTableModel'); From ce73663749f63a00c44c3b6a64155724d15a2e03 Mon Sep 17 00:00:00 2001 From: mikebender Date: Tue, 21 Nov 2023 17:07:43 -0500 Subject: [PATCH 4/5] Address review comments - Now if you deselect all of the columns for an aggregation, it will filter out that column from the config that it applies - Will also remove from the aggregations list entirely --- packages/iris-grid/src/IrisGrid.tsx | 48 +++++++++++++++++++---------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx index 7aec5ff6b1..a4b7d9688f 100644 --- a/packages/iris-grid/src/IrisGrid.tsx +++ b/packages/iris-grid/src/IrisGrid.tsx @@ -1275,12 +1275,17 @@ export class IrisGrid extends Component { config: UIRollupConfig | undefined, aggregationSettings: AggregationSettings ): UITotalsTableConfig | null => { - const { aggregations, showOnTop } = aggregationSettings; - // If we've got rollups, then aggregations are applied as part of that... - if ( - (config?.columns?.length ?? 0) > 0 || - (aggregations?.length ?? 0) === 0 - ) { + if ((config?.columns?.length ?? 0) > 0) { + // If we've got rollups, then aggregations are applied as part of that... + return null; + } + + // Filter out aggregations without any columns actually selected + const aggregations = aggregationSettings.aggregations.filter( + agg => agg.selected.length > 0 || agg.invert + ); + if (aggregations.length === 0) { + // We don't actually have any aggregations set, don't bother return null; } @@ -1290,7 +1295,7 @@ export class IrisGrid extends Component { return { operationMap, operationOrder, - showOnTop, + showOnTop: aggregationSettings.showOnTop, defaultOperation: AggregationOperation.SKIP, }; } @@ -3201,15 +3206,26 @@ export class IrisGrid extends Component { log.debug('handleAggregationChange', aggregation); this.startLoading(`Aggregating ${aggregation.operation}...`); - this.setState(({ aggregationSettings }) => ({ - selectedAggregation: aggregation, - aggregationSettings: { - ...aggregationSettings, - aggregations: aggregationSettings.aggregations.map(a => - a.operation === aggregation.operation ? aggregation : a - ), - }, - })); + this.setState(({ aggregationSettings }) => { + let newAggregations = [...aggregationSettings.aggregations]; + let aggregationIndex = newAggregations.findIndex( + a => a.operation === aggregation.operation + ); + if (aggregationIndex === -1) { + aggregationIndex = newAggregations.length; + } + newAggregations[aggregationIndex] = aggregation; + newAggregations = newAggregations.filter( + a => a.selected.length > 0 || a.invert + ); + return { + selectedAggregation: aggregation, + aggregationSettings: { + ...aggregationSettings, + aggregations: newAggregations, + }, + }; + }); } /** From ad9b81927507daa7e8556466d85cd67748a7ef5d Mon Sep 17 00:00:00 2001 From: mikebender Date: Wed, 22 Nov 2023 11:22:56 -0500 Subject: [PATCH 5/5] Remove empty aggregations only when editing is done - So the order stays stable --- packages/iris-grid/src/IrisGrid.tsx | 68 ++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx index a4b7d9688f..b4bd389e17 100644 --- a/packages/iris-grid/src/IrisGrid.tsx +++ b/packages/iris-grid/src/IrisGrid.tsx @@ -854,7 +854,7 @@ export class IrisGrid extends Component { this.startListening(model); } - componentDidUpdate(prevProps: IrisGridProps): void { + componentDidUpdate(prevProps: IrisGridProps, prevState: IrisGridState): void { const { inputFilters, isSelectingColumn, @@ -924,6 +924,18 @@ export class IrisGrid extends Component { }); } + const { openOptions } = this.state; + if (openOptions !== prevState.openOptions) { + const isAggEditType = (option: OptionItem): boolean => + option.type === OptionType.AGGREGATION_EDIT; + if ( + !openOptions.some(isAggEditType) && + prevState.openOptions.some(isAggEditType) + ) { + this.removeEmptyAggregations(); + } + } + this.sendStateChange(); } @@ -3206,26 +3218,16 @@ export class IrisGrid extends Component { log.debug('handleAggregationChange', aggregation); this.startLoading(`Aggregating ${aggregation.operation}...`); - this.setState(({ aggregationSettings }) => { - let newAggregations = [...aggregationSettings.aggregations]; - let aggregationIndex = newAggregations.findIndex( - a => a.operation === aggregation.operation - ); - if (aggregationIndex === -1) { - aggregationIndex = newAggregations.length; - } - newAggregations[aggregationIndex] = aggregation; - newAggregations = newAggregations.filter( - a => a.selected.length > 0 || a.invert - ); - return { - selectedAggregation: aggregation, - aggregationSettings: { - ...aggregationSettings, - aggregations: newAggregations, - }, - }; - }); + + this.setState(({ aggregationSettings }) => ({ + selectedAggregation: aggregation, + aggregationSettings: { + ...aggregationSettings, + aggregations: aggregationSettings.aggregations.map(a => + a.operation === aggregation.operation ? aggregation : a + ), + }, + })); } /** @@ -3355,6 +3357,30 @@ export class IrisGrid extends Component { } } + /** + * Aggregation editing has completed. Need to filter out any aggregations that have no columns selected + */ + removeEmptyAggregations(): void { + log.debug('removeEmptyAggregations'); + + this.setState(({ aggregationSettings }) => { + const { aggregations } = aggregationSettings; + const newAggregations = aggregations.filter( + a => a.selected.length > 0 || a.invert + ); + if (newAggregations.length !== aggregations.length) { + return { + selectedAggregation: null, + aggregationSettings: { + ...aggregationSettings, + aggregations: newAggregations, + }, + }; + } + return { selectedAggregation: null, aggregationSettings }; + }); + } + async seekRow(inputString: string, isBackwards = false): Promise { const { gotoValueSelectedColumnName: selectedColumnName,