From b7b1ef584b97abea72c4bc3815715138e0403d7d Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Mon, 18 Sep 2023 09:25:55 -0700 Subject: [PATCH 1/3] Revert "fix: do not emit state column in columnConfigsChange (#2403)" This reverts commit 1e8d426a6a1549ab86602c1ab0f3b14a771e23a4. --- projects/components/src/table/table.component.test.ts | 10 ++++++---- projects/components/src/table/table.component.ts | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/projects/components/src/table/table.component.test.ts b/projects/components/src/table/table.component.test.ts index ae7bdc3c6..919ea6c7a 100644 --- a/projects/components/src/table/table.component.test.ts +++ b/projects/components/src/table/table.component.test.ts @@ -464,10 +464,12 @@ describe('Table component', () => { // Hiding a column spectator.component.onHideColumn(columns[0]); - expect(mockColumnConfigsChange).toHaveBeenLastCalledWith([ - expect.objectContaining({ ...columns[0], visible: false }), - expect.objectContaining({ ...columns[1] }) - ]); + expect(mockColumnConfigsChange).toHaveBeenLastCalledWith( + expect.arrayContaining([ + expect.objectContaining({ ...columns[0], visible: false }), + expect.objectContaining({ ...columns[1] }) + ]) + ); flush(); })); diff --git a/projects/components/src/table/table.component.ts b/projects/components/src/table/table.component.ts index 9177d29c8..bbdc995cd 100644 --- a/projects/components/src/table/table.component.ts +++ b/projects/components/src/table/table.component.ts @@ -915,7 +915,7 @@ export class TableComponent private propagateUpdatedColumns(updatedColumns: TableColumnConfigExtended[]): void { this.columnConfigsSubject.next(updatedColumns); - this.columnConfigsChange.next(updatedColumns.filter(column => !this.isStateColumn(column))); + this.columnConfigsChange.next(updatedColumns); } private buildColumnConfigExtendeds(columnConfigs: TableColumnConfig[]): TableColumnConfigExtended[] { From 378d4871c0f9b19e54ae64d841fa9dd002023e2e Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Mon, 18 Sep 2023 09:26:08 -0700 Subject: [PATCH 2/3] Revert "fix: table component should emit column change when columns are updated (#2399)" This reverts commit 80619ef89f60d290294302a4f9d468d674904068. --- .../src/table/table.component.test.ts | 28 ------------------- .../components/src/table/table.component.ts | 15 ++++------ 2 files changed, 5 insertions(+), 38 deletions(-) diff --git a/projects/components/src/table/table.component.test.ts b/projects/components/src/table/table.component.test.ts index 919ea6c7a..a99b7a62c 100644 --- a/projects/components/src/table/table.component.test.ts +++ b/projects/components/src/table/table.component.test.ts @@ -445,34 +445,6 @@ describe('Table component', () => { flush(); })); - test('should emit correctly on column changes', fakeAsync(() => { - const mockColumnConfigsChange = jest.fn(); - const columns = buildColumns(); - const spectator = createHost( - ``, - { - hostProps: { - columnConfigs: columns, - data: buildData(), - selectionMode: TableSelectionMode.Multiple, - mode: TableMode.Flat, - mockColumnConfigsChange: mockColumnConfigsChange - } - } - ); - - // Hiding a column - spectator.component.onHideColumn(columns[0]); - expect(mockColumnConfigsChange).toHaveBeenLastCalledWith( - expect.arrayContaining([ - expect.objectContaining({ ...columns[0], visible: false }), - expect.objectContaining({ ...columns[1] }) - ]) - ); - flush(); - })); - test('should select only selected rows', fakeAsync(() => { const columns = buildColumns(); const rows = buildData(); diff --git a/projects/components/src/table/table.component.ts b/projects/components/src/table/table.component.ts index bbdc995cd..8c4a6e592 100644 --- a/projects/components/src/table/table.component.ts +++ b/projects/components/src/table/table.component.ts @@ -2,8 +2,8 @@ /* eslint-disable @angular-eslint/component-max-inline-declarations */ import { ModalService } from '../modal/modal.service'; import { - TableEditColumnsModalComponent, - TableEditColumnsModalConfig + TableEditColumnsModalConfig, + TableEditColumnsModalComponent } from './columns/table-edit-columns-modal.component'; import { CdkHeaderRow } from '@angular/cdk/table'; import { @@ -35,7 +35,7 @@ import { } from '@hypertrace/common'; import { isNil, without } from 'lodash-es'; import { BehaviorSubject, combineLatest, merge, Observable, Subject } from 'rxjs'; -import { filter, map, switchMap, take } from 'rxjs/operators'; +import { switchMap, take, filter, map } from 'rxjs/operators'; import { FilterAttribute } from '../filtering/filter/filter-attribute'; import { LoadAsyncConfig } from '../load-async/load-async.service'; import { PageEvent } from '../paginator/page.event'; @@ -722,7 +722,7 @@ export class TableComponent this.initialColumnConfigIdWidthMap = new Map(visibleColumns.map(column => [column.id, column.width ?? -1])); this.updateVisibleColumns(visibleColumns); - this.propagateUpdatedColumns(columnConfigurations); + this.columnConfigsSubject.next(columnConfigurations); } private checkColumnWidthCompatibilityOrThrow(width?: TableColumnWidth): void { @@ -827,7 +827,7 @@ export class TableComponent const updatedColumns = this.columnConfigsSubject.value; this.updateVisibleColumns(updatedColumns.filter(c => c.visible)); this.distributeWidthToColumns(TableColumnWidthUtil.getColWidthInPx(column.width)); - this.propagateUpdatedColumns(updatedColumns); + this.columnConfigsSubject.next(updatedColumns); } public showEditColumnsModal(): void { @@ -913,11 +913,6 @@ export class TableComponent return this.hasExpandableRows() ? index - 1 : index; } - private propagateUpdatedColumns(updatedColumns: TableColumnConfigExtended[]): void { - this.columnConfigsSubject.next(updatedColumns); - this.columnConfigsChange.next(updatedColumns); - } - private buildColumnConfigExtendeds(columnConfigs: TableColumnConfig[]): TableColumnConfigExtended[] { const stateColumns = []; From ef4e31bc026ca84df6b2e3ed770e7becfa93d094 Mon Sep 17 00:00:00 2001 From: Arjunlal B Date: Mon, 18 Sep 2023 14:12:27 -0700 Subject: [PATCH 3/3] fix: updates to table and table widget --- projects/components/src/public-api.ts | 1 + .../components/src/table/table.component.ts | 49 ++++++++++++++----- .../src/table/util/table-column.util.ts | 10 ++++ .../table/table-widget-renderer.component.ts | 7 ++- 4 files changed, 52 insertions(+), 15 deletions(-) create mode 100644 projects/components/src/table/util/table-column.util.ts diff --git a/projects/components/src/public-api.ts b/projects/components/src/public-api.ts index 3199504f5..2cf915c11 100644 --- a/projects/components/src/public-api.ts +++ b/projects/components/src/public-api.ts @@ -368,6 +368,7 @@ export * from './table/cells/data-parsers/table-cell-no-op-parser'; export * from './table/cells/data-parsers/table-cell-string-parser'; export * from './table/cells/data-parsers/table-cell-timestamp-parser'; export * from './table/cells/data-parsers/table-cell-icon-parser'; +export * from './table/util/table-column.util'; // Table Controls export * from './table/controls/table-controls.module'; diff --git a/projects/components/src/table/table.component.ts b/projects/components/src/table/table.component.ts index 8c4a6e592..4e4c20648 100644 --- a/projects/components/src/table/table.component.ts +++ b/projects/components/src/table/table.component.ts @@ -2,8 +2,8 @@ /* eslint-disable @angular-eslint/component-max-inline-declarations */ import { ModalService } from '../modal/modal.service'; import { - TableEditColumnsModalConfig, - TableEditColumnsModalComponent + TableEditColumnsModalComponent, + TableEditColumnsModalConfig } from './columns/table-edit-columns-modal.component'; import { CdkHeaderRow } from '@angular/cdk/table'; import { @@ -33,9 +33,9 @@ import { NumberCoercer, TypedSimpleChanges } from '@hypertrace/common'; -import { isNil, without } from 'lodash-es'; +import { isNil, isUndefined, without } from 'lodash-es'; import { BehaviorSubject, combineLatest, merge, Observable, Subject } from 'rxjs'; -import { switchMap, take, filter, map } from 'rxjs/operators'; +import { filter, map, switchMap, take } from 'rxjs/operators'; import { FilterAttribute } from '../filtering/filter/filter-attribute'; import { LoadAsyncConfig } from '../load-async/load-async.service'; import { PageEvent } from '../paginator/page.event'; @@ -68,6 +68,7 @@ import { ModalSize } from '../modal/modal'; import { DOCUMENT } from '@angular/common'; import { CdkDragDrop, moveItemInArray } from '@angular/cdk/drag-drop'; import { TableColumnWidthUtil } from './util/column-width.util'; +import { TableColumnUtil } from './util/table-column.util'; @Component({ selector: 'ht-table', @@ -547,7 +548,14 @@ export class TableComponent this.isTableFullPage = this.display === TableStyle.FullPage; } - if (changes.mode || changes.columnConfigs || changes.detailContent || changes.metadata) { + if ( + changes.columnConfigs && + this.hasNonStateColumnsChanged(changes.columnConfigs.previousValue, changes.columnConfigs.currentValue) + ) { + this.initializeColumns(); + } + + if (changes.mode || changes.detailContent || changes.metadata) { this.initializeColumns(); } @@ -572,6 +580,17 @@ export class TableComponent } } + private hasNonStateColumnsChanged(previousValue?: TableColumnConfig[], currentValue?: TableColumnConfig[]): boolean { + if (previousValue === undefined || currentValue === undefined) { + return false; + } + + const previousColumns = previousValue.filter(column => !this.isStateColumn(column)); + const currentColumns = currentValue.filter(column => !this.isStateColumn(column)); + + return !isEqualIgnoreFunctions(previousColumns, currentColumns); + } + public ngAfterViewInit(): void { setTimeout(() => { !this.dataSource && this.initializeData(); @@ -714,7 +733,7 @@ export class TableComponent this.checkColumnWidthCompatibilityOrThrow(column.width); this.checkColumnWidthCompatibilityOrThrow(column.minWidth); }); - const columnConfigurations = this.buildColumnConfigExtendeds(columnConfigs ?? this.columnConfigs ?? []); + const columnConfigurations = this.buildColumnConfigExtended(columnConfigs ?? this.columnConfigs ?? []); if (isNil(this.columnDefaultConfigs)) { this.columnDefaultConfigs = columnConfigurations; } @@ -723,6 +742,7 @@ export class TableComponent this.updateVisibleColumns(visibleColumns); this.columnConfigsSubject.next(columnConfigurations); + this.columnConfigsChange.next(columnConfigurations); } private checkColumnWidthCompatibilityOrThrow(width?: TableColumnWidth): void { @@ -789,12 +809,11 @@ export class TableComponent return this.showColumnDivider(columns, index); }; - public isSelectionStateColumn = (column?: TableColumnConfig): boolean => column?.id === '$$selected'; + protected isSelectionStateColumn = TableColumnUtil.isSelectionStateColumn; - public isExpansionStateColumn = (column?: TableColumnConfig): boolean => column?.id === '$$expanded'; + protected isExpansionStateColumn = TableColumnUtil.isExpansionStateColumn; - public isStateColumn = (column?: TableColumnConfig): boolean => - this.isSelectionStateColumn(column) || this.isExpansionStateColumn(column); + protected isStateColumn = TableColumnUtil.isStateColumn; public isLastStateColumn = (columns: TableColumnConfig[], index: number): boolean => { if (this.isStateColumn(columns[index]) && !this.isStateColumn(columns[index + 1])) { @@ -828,6 +847,7 @@ export class TableComponent this.updateVisibleColumns(updatedColumns.filter(c => c.visible)); this.distributeWidthToColumns(TableColumnWidthUtil.getColWidthInPx(column.width)); this.columnConfigsSubject.next(updatedColumns); + this.columnConfigsChange.next(updatedColumns); } public showEditColumnsModal(): void { @@ -913,14 +933,17 @@ export class TableComponent return this.hasExpandableRows() ? index - 1 : index; } - private buildColumnConfigExtendeds(columnConfigs: TableColumnConfig[]): TableColumnConfigExtended[] { + private buildColumnConfigExtended(columnConfigs: TableColumnConfig[]): TableColumnConfigExtended[] { const stateColumns = []; - if (this.hasMultiSelect()) { + if (this.hasMultiSelect() && columnConfigs.find(TableColumnUtil.isSelectionStateColumn) === undefined) { stateColumns.push(this.multiSelectRowColumnConfig); } - if (this.hasExpandableRows()) { + if ( + this.hasExpandableRows() && + isUndefined(columnConfigs.find(TableColumnUtil.isExpansionStateColumn)) === undefined + ) { stateColumns.push(this.expandableToggleColumnConfig); } diff --git a/projects/components/src/table/util/table-column.util.ts b/projects/components/src/table/util/table-column.util.ts new file mode 100644 index 000000000..056aa40e1 --- /dev/null +++ b/projects/components/src/table/util/table-column.util.ts @@ -0,0 +1,10 @@ +import { TableColumnConfig } from '../table-api'; + +export abstract class TableColumnUtil { + public static isSelectionStateColumn = (column?: TableColumnConfig): boolean => column?.id === '$$selected'; + + public static isExpansionStateColumn = (column?: TableColumnConfig): boolean => column?.id === '$$expanded'; + + public static isStateColumn = (column?: TableColumnConfig): boolean => + TableColumnUtil.isSelectionStateColumn(column) || TableColumnUtil.isExpansionStateColumn(column); +} diff --git a/projects/observability/src/shared/dashboard/widgets/table/table-widget-renderer.component.ts b/projects/observability/src/shared/dashboard/widgets/table/table-widget-renderer.component.ts index 6f5a137b6..e9bfd546c 100644 --- a/projects/observability/src/shared/dashboard/widgets/table/table-widget-renderer.component.ts +++ b/projects/observability/src/shared/dashboard/widgets/table/table-widget-renderer.component.ts @@ -18,6 +18,7 @@ import { TableCheckboxControlOption, TableCheckboxOptions, TableColumnConfig, + TableColumnUtil, TableControlOption, TableControlOptionType, TableDataSource, @@ -33,7 +34,7 @@ import { } from '@hypertrace/components'; import { WidgetRenderer } from '@hypertrace/dashboards'; import { Renderer } from '@hypertrace/hyperdash'; -import { RendererApi, RENDERER_API } from '@hypertrace/hyperdash-angular'; +import { RENDERER_API, RendererApi } from '@hypertrace/hyperdash-angular'; import { capitalize, isEmpty, isEqual, pick } from 'lodash-es'; import { BehaviorSubject, combineLatest, Observable, of, Subject } from 'rxjs'; import { @@ -517,7 +518,9 @@ export class TableWidgetRendererComponent this.getLocalPreferences().subscribe(preferences => this.setLocalPreferences({ ...preferences, - columns: columns.map(column => this.dehydratePersistedColumnConfig(column)) + columns: columns + .filter(column => !TableColumnUtil.isStateColumn(column)) + .map(column => this.dehydratePersistedColumnConfig(column)) }) ); }