From ab4ccb1b1a022e068953c2abc74894cecfe37110 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Mon, 3 Jun 2024 09:19:45 -0400 Subject: [PATCH] fix: DH-17076 LayoutHints on TreeTables were not being applied (#2041) - Related to DH-17076 - Needed a Core patch: https://github.com/deephaven/deephaven-core/pull/5555 - Tested using both the Groovy snippet from the ticket - Fixes #2035 --- .../src/IrisGridTableModelTemplate.ts | 19 ++++++-- packages/iris-grid/src/IrisGridTestUtils.ts | 4 +- .../src/IrisGridTreeTableModel.test.ts | 47 ++++++++++++++++++- .../iris-grid/src/IrisGridTreeTableModel.ts | 16 +++++++ 4 files changed, 79 insertions(+), 7 deletions(-) diff --git a/packages/iris-grid/src/IrisGridTableModelTemplate.ts b/packages/iris-grid/src/IrisGridTableModelTemplate.ts index c2f42e41b5..e7a662504b 100644 --- a/packages/iris-grid/src/IrisGridTableModelTemplate.ts +++ b/packages/iris-grid/src/IrisGridTableModelTemplate.ts @@ -177,6 +177,8 @@ class IrisGridTableModelTemplate< private _columnHeaderGroups: ColumnHeaderGroup[] = []; + private _isColumnHeaderGroupsInitialized = false; + private _movedColumns: MoveOperation[] | null = null; /** @@ -224,11 +226,6 @@ class IrisGridTableModelTemplate< // These rows can be sparse, so using a map instead of an array. this.pendingNewDataMap = new Map(); this.pendingNewRowCount = 0; - - this.columnHeaderGroups = IrisGridUtils.parseColumnHeaderGroups( - this, - this.layoutHints?.columnGroups ?? [] - ).groups; } close(): void { @@ -926,10 +923,12 @@ class IrisGridTableModelTemplate< } get columnHeaderGroupMap(): Map { + this.initializeColumnHeaderGroups(); return this._columnHeaderGroupMap; } get columnHeaderGroups(): ColumnHeaderGroup[] { + this.initializeColumnHeaderGroups(); return this._columnHeaderGroups; } @@ -952,6 +951,16 @@ class IrisGridTableModelTemplate< this.columnHeaderMaxDepth = maxDepth; this.columnHeaderParentMap = parentMap; this._columnHeaderGroupMap = groupMap; + this._isColumnHeaderGroupsInitialized = true; + } + + private initializeColumnHeaderGroups(): void { + if (!this._isColumnHeaderGroupsInitialized) { + this.columnHeaderGroups = IrisGridUtils.parseColumnHeaderGroups( + this, + this.layoutHints?.columnGroups ?? [] + ).groups; + } } row(y: ModelIndex): R | null { diff --git a/packages/iris-grid/src/IrisGridTestUtils.ts b/packages/iris-grid/src/IrisGridTestUtils.ts index 5c261a5618..c36cc371ce 100644 --- a/packages/iris-grid/src/IrisGridTestUtils.ts +++ b/packages/iris-grid/src/IrisGridTestUtils.ts @@ -100,7 +100,8 @@ class IrisGridTestUtils { columns = this.makeColumns(), groupedColumns: DhType.Column[] = [], size = 1000000000, - sort = [] + sort = [], + layoutHints?: Partial ): DhType.TreeTable { // eslint-disable-next-line @typescript-eslint/no-explicit-any const table = new (this.dh as any).TreeTable({ @@ -108,6 +109,7 @@ class IrisGridTestUtils { groupedColumns, size, sort, + layoutHints, }); table.copy = jest.fn(() => Promise.resolve(table)); return table; diff --git a/packages/iris-grid/src/IrisGridTreeTableModel.test.ts b/packages/iris-grid/src/IrisGridTreeTableModel.test.ts index 92d5a977ed..af7f11de42 100644 --- a/packages/iris-grid/src/IrisGridTreeTableModel.test.ts +++ b/packages/iris-grid/src/IrisGridTreeTableModel.test.ts @@ -4,7 +4,7 @@ import IrisGridTreeTableModel from './IrisGridTreeTableModel'; const irisGridTestUtils = new IrisGridTestUtils(dh); -describe('IrisGridTreeTableModel', () => { +describe('IrisGridTreeTableModel virtual columns', () => { const expectedVirtualColumn = expect.objectContaining({ name: '__DH_UI_GROUP__', displayName: 'Group', @@ -27,3 +27,48 @@ describe('IrisGridTreeTableModel', () => { } ); }); + +describe('IrisGridTreeTableModel layoutHints', () => { + test('null layout hints by default', () => { + const columns = irisGridTestUtils.makeColumns(); + const table = irisGridTestUtils.makeTreeTable(columns, columns); + const model = new IrisGridTreeTableModel(dh, table); + + expect(model.layoutHints).toEqual(null); + }); + + test('layoutHints set on tree table', () => { + const columns = irisGridTestUtils.makeColumns(); + const layoutHints = { hiddenColumns: ['X'], frozenColumns: ['Y'] }; + const table = irisGridTestUtils.makeTreeTable( + columns, + columns, + 100, + [], + layoutHints + ); + const model = new IrisGridTreeTableModel(dh, table); + + expect(model.layoutHints).toEqual(layoutHints); + }); + + test('layoutHints undefined (e.g. not set on the table)', () => { + const columns = irisGridTestUtils.makeColumns(); + const table = irisGridTestUtils.makeTreeTable(columns, columns, 100, []); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (table as any).layoutHints = undefined; + const model = new IrisGridTreeTableModel(dh, table); + + expect(model.layoutHints).toEqual(undefined); + }); + + test('layoutHints property does not exist should not crash', () => { + const columns = irisGridTestUtils.makeColumns(); + const table = irisGridTestUtils.makeTreeTable(columns, columns, 100, []); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + delete (table as any).layoutHints; + const model = new IrisGridTreeTableModel(dh, table); + + expect(model.layoutHints).toEqual(undefined); + }); +}); diff --git a/packages/iris-grid/src/IrisGridTreeTableModel.ts b/packages/iris-grid/src/IrisGridTreeTableModel.ts index e0bada6cdc..b89a3f7856 100644 --- a/packages/iris-grid/src/IrisGridTreeTableModel.ts +++ b/packages/iris-grid/src/IrisGridTreeTableModel.ts @@ -21,6 +21,15 @@ export interface UITreeRow extends UIRow { hasChildren: boolean; depth: number; } + +type LayoutTreeTable = DhType.TreeTable & { + layoutHints?: null | DhType.LayoutHints; +}; + +function isLayoutTreeTable(table: DhType.TreeTable): table is LayoutTreeTable { + return (table as LayoutTreeTable).layoutHints !== undefined; +} + class IrisGridTreeTableModel extends IrisGridTableModelTemplate< DhType.TreeTable, UITreeRow @@ -230,6 +239,13 @@ class IrisGridTreeTableModel extends IrisGridTableModelTemplate< return [this.virtualColumns.length, this.groupedColumns.length]; } + get layoutHints(): DhType.LayoutHints | null | undefined { + if (isLayoutTreeTable(this.table)) { + return this.table.layoutHints; + } + return undefined; + } + get hasExpandableRows(): boolean { return true; }