Skip to content

Commit 1c9a8ed

Browse files
authored
fix: DH-19614: Add tree table formatting logic to tree model (#2461)
Part of DH-19614. Fixes an issue with rollups where tree table aggregated constituents were not being rendered based on the original column type. This change ensures that leaf nodes in tree tables use the appropriate column type for formatting and alignment. Changes: - Added tree table specific overrides to `IrisGridTreeTableModel`: - Updated `colorForCell` to use `column.constituentType` instead of `column,type` for leaf node values - Updated `textAlignForCell` to use `column.constituentType` for leaf node values - Added type guard `isUITreeRow` function in `IrisGridTreeTableModel.ts`
1 parent 9c39986 commit 1c9a8ed

5 files changed

Lines changed: 413 additions & 27 deletions

File tree

packages/iris-grid/src/IrisGridTableModelTemplate.ts

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -630,22 +630,12 @@ class IrisGridTableModelTemplate<
630630
// Fallback to formatting based on the value/type of the cell
631631
if (value != null) {
632632
const column = this.sourceColumn(x, y);
633-
if (TableUtils.isDateType(column.type) || column.name === 'Date') {
634-
assertNotNull(theme.dateColor);
635-
return theme.dateColor;
636-
}
637-
if (TableUtils.isNumberType(column.type)) {
638-
if ((value as number) > 0) {
639-
assertNotNull(theme.positiveNumberColor);
640-
return theme.positiveNumberColor;
641-
}
642-
if ((value as number) < 0) {
643-
assertNotNull(theme.negativeNumberColor);
644-
return theme.negativeNumberColor;
645-
}
646-
assertNotNull(theme.zeroNumberColor);
647-
return theme.zeroNumberColor;
648-
}
633+
return IrisGridUtils.colorForValue(
634+
theme,
635+
column.type,
636+
column.name,
637+
value
638+
);
649639
}
650640
} else if (this.isPendingRow(y) && this.isKeyColumn(x)) {
651641
assertNotNull(theme.errorTextColor);
@@ -663,17 +653,10 @@ class IrisGridTableModelTemplate<
663653
return this.formatForCell(x, y)?.backgroundColor ?? null;
664654
}
665655

666-
textAlignForCell(x: ModelIndex): CanvasTextAlign {
667-
const column = this.columns[x];
668-
const { type } = column;
656+
textAlignForCell(x: ModelIndex, y: ModelIndex): CanvasTextAlign {
657+
const column = this.sourceColumn(x, y);
669658

670-
if (TableUtils.isNumberType(type)) {
671-
return 'right';
672-
}
673-
if (TableUtils.isDateType(type) || column.name === 'Date') {
674-
return 'center';
675-
}
676-
return 'left';
659+
return IrisGridUtils.textAlignForValue(column.type, column.name);
677660
}
678661

679662
textForColumnHeader(x: ModelIndex, depth = 0): string | undefined {

packages/iris-grid/src/IrisGridTreeTableModel.test.ts

Lines changed: 158 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import { type dh as DhType } from '@deephaven/jsapi-types';
44
import { TestUtils } from '@deephaven/test-utils';
55
import { act } from 'react-dom/test-utils';
66
import IrisGridTestUtils from './IrisGridTestUtils';
7-
import IrisGridTreeTableModel from './IrisGridTreeTableModel';
7+
import IrisGridTreeTableModel, {
8+
type UITreeRow,
9+
} from './IrisGridTreeTableModel';
10+
import type { IrisGridThemeType } from './IrisGridTheme';
811

912
const irisGridTestUtils = new IrisGridTestUtils(dh);
1013

@@ -160,3 +163,157 @@ describe('IrisGridTreeTableModel snapshot', () => {
160163
}).not.toThrow();
161164
});
162165
});
166+
167+
describe('IrisGridTreeTableModel colorForCell and textAlignForCell', () => {
168+
let model: IrisGridTreeTableModel;
169+
170+
const mockTheme: IrisGridThemeType = {
171+
textColor: '#text-color',
172+
dateColor: '#date-color',
173+
positiveNumberColor: '#positive-color',
174+
negativeNumberColor: '#negative-color',
175+
zeroNumberColor: '#zero-color',
176+
nullStringColor: '#null-string-color',
177+
} as IrisGridThemeType;
178+
179+
const columns = {
180+
string: irisGridTestUtils.makeColumn('StrCol', 'java.lang.String', 0),
181+
number: irisGridTestUtils.makeColumn('NumCol', 'int', 0),
182+
date: irisGridTestUtils.makeColumn(
183+
'DateCol',
184+
'io.deephaven.time.DateTime',
185+
0
186+
),
187+
dateNamed: irisGridTestUtils.makeColumn('Date', 'java.lang.String', 0),
188+
};
189+
190+
const rows = {
191+
default: {
192+
data: new Map(),
193+
hasChildren: false,
194+
isExpanded: false,
195+
depth: 1,
196+
} as UITreeRow,
197+
parent: {
198+
data: new Map(),
199+
hasChildren: true,
200+
isExpanded: false,
201+
depth: 1,
202+
} as UITreeRow,
203+
leaf: {
204+
data: new Map(),
205+
hasChildren: false,
206+
isExpanded: false,
207+
depth: 2,
208+
} as UITreeRow,
209+
};
210+
211+
beforeEach(() => {
212+
const testColumns = irisGridTestUtils.makeColumns();
213+
const table = irisGridTestUtils.makeTreeTable(
214+
testColumns,
215+
testColumns.slice(0, 1),
216+
100,
217+
[]
218+
);
219+
model = new IrisGridTreeTableModel(dh, table);
220+
221+
// Setup the basic mocks needed for assertNotNull checks
222+
jest.spyOn(model, 'sourceColumn').mockReturnValue(columns.string);
223+
jest.spyOn(model, 'row').mockReturnValue(rows.default);
224+
});
225+
226+
const mockCell = (
227+
value: unknown,
228+
format?: { color?: string },
229+
column?: DhType.Column,
230+
row?: UITreeRow
231+
) => {
232+
if (column) jest.spyOn(model, 'sourceColumn').mockReturnValue(column);
233+
if (row) jest.spyOn(model, 'row').mockReturnValue(row);
234+
jest.spyOn(model, 'dataForCell').mockReturnValue({ value, format });
235+
};
236+
237+
describe('colorForCell', () => {
238+
it('returns nullStringColor for null values', () => {
239+
mockCell(null);
240+
const result = model.colorForCell(0, 0, mockTheme);
241+
expect(result).toBe(mockTheme.nullStringColor);
242+
});
243+
244+
it('returns nullStringColor for empty strings', () => {
245+
mockCell('');
246+
const result = model.colorForCell(0, 0, mockTheme);
247+
expect(result).toBe(mockTheme.nullStringColor);
248+
});
249+
250+
it('returns custom color when available', () => {
251+
const customColor = '#e0e0e0';
252+
mockCell('test', { color: customColor });
253+
const result = model.colorForCell(0, 0, mockTheme);
254+
expect(result).toBe(customColor);
255+
});
256+
257+
it('delegates to IrisGridUtils.colorForValue for standard color logic', () => {
258+
mockCell(42, undefined, columns.number);
259+
const result = model.colorForCell(0, 0, mockTheme);
260+
expect(result).toBe(mockTheme.positiveNumberColor);
261+
});
262+
});
263+
264+
describe('colorForCell constituent type handling', () => {
265+
it('uses constituent type for tree table leaf nodes', () => {
266+
const constituentColumn = {
267+
...irisGridTestUtils.makeColumn('TestCol', 'java.lang.String', 0),
268+
constituentType: 'int',
269+
} as DhType.Column;
270+
271+
mockCell(1, undefined, constituentColumn, rows.leaf);
272+
const result = model.colorForCell(0, 0, mockTheme);
273+
expect(result).toBe(mockTheme.positiveNumberColor);
274+
});
275+
276+
it('ignores constituent type for non-leaf nodes', () => {
277+
const constituentColumn = {
278+
...irisGridTestUtils.makeColumn('TestCol', 'java.lang.String', 0),
279+
constituentType: 'int',
280+
} as DhType.Column;
281+
282+
mockCell('test', undefined, constituentColumn, rows.parent);
283+
const result = model.colorForCell(0, 0, mockTheme);
284+
expect(result).toBe(mockTheme.textColor);
285+
});
286+
});
287+
288+
describe('textAlignForCell', () => {
289+
it('delegates to IrisGridUtils.textAlignForValue for standard alignment logic', () => {
290+
mockCell('ignored', undefined, columns.number);
291+
const result = model.textAlignForCell(0, 0);
292+
expect(result).toBe('right');
293+
});
294+
});
295+
296+
describe('textAlignForCell constituent type handling', () => {
297+
it('uses constituent type for tree table leaf nodes', () => {
298+
const constituentColumn = {
299+
...irisGridTestUtils.makeColumn('TestCol', 'java.lang.String', 0),
300+
constituentType: 'double',
301+
} as DhType.Column;
302+
303+
mockCell('ignored', undefined, constituentColumn, rows.leaf);
304+
const result = model.textAlignForCell(0, 0);
305+
expect(result).toBe('right');
306+
});
307+
308+
it('ignores constituent type for non-leaf nodes', () => {
309+
const constituentColumn = {
310+
...irisGridTestUtils.makeColumn('TestCol', 'java.lang.String', 0),
311+
constituentType: 'int',
312+
} as DhType.Column;
313+
314+
mockCell('ignored', undefined, constituentColumn, rows.parent);
315+
const result = model.textAlignForCell(0, 0);
316+
expect(result).toBe('left');
317+
});
318+
});
319+
});

packages/iris-grid/src/IrisGridTreeTableModel.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import { assertNotNull, EventShimCustomEvent } from '@deephaven/utils';
1313
import { type UIRow, type ColumnName } from './CommonTypes';
1414
import IrisGridTableModelTemplate from './IrisGridTableModelTemplate';
1515
import IrisGridModel, { type DisplayColumn } from './IrisGridModel';
16+
import { type IrisGridThemeType } from './IrisGridTheme';
17+
import IrisGridUtils from './IrisGridUtils';
1618

1719
const log = Log.module('IrisGridTreeTableModel');
1820

@@ -55,6 +57,24 @@ export interface UITreeRow extends UIRow {
5557
depth: number;
5658
}
5759

60+
/**
61+
* Check if a row is a UITreeRow
62+
* @param row The row to check
63+
* @returns True if the row is a UITreeRow and false otherwise
64+
*/
65+
export function isUITreeRow(row: unknown): row is UITreeRow {
66+
return (
67+
row != null &&
68+
typeof row === 'object' &&
69+
'hasChildren' in row &&
70+
'isExpanded' in row &&
71+
'depth' in row &&
72+
typeof row.hasChildren === 'boolean' &&
73+
typeof row.isExpanded === 'boolean' &&
74+
typeof row.depth === 'number'
75+
);
76+
}
77+
5878
type LayoutTreeTable = DhType.TreeTable & {
5979
layoutHints?: null | DhType.LayoutHints;
6080
};
@@ -155,6 +175,66 @@ class IrisGridTreeTableModel extends IrisGridTableModelTemplate<
155175
return super.textForCell(x, y);
156176
}
157177

178+
colorForCell(x: ModelIndex, y: ModelIndex, theme: IrisGridThemeType): string {
179+
const data = this.dataForCell(x, y);
180+
if (data) {
181+
const { format, value } = data;
182+
if (value == null || value === '') {
183+
assertNotNull(theme.nullStringColor);
184+
return theme.nullStringColor;
185+
}
186+
if (format?.color != null && format.color !== '') {
187+
return format.color;
188+
}
189+
190+
// Fallback to formatting based on the value/type of the cell
191+
if (value != null) {
192+
const column = this.sourceColumn(x, y);
193+
const row = this.row(y);
194+
assertNotNull(row);
195+
196+
let columnTypeForFormatting = column.type;
197+
198+
// For tree table leaf nodes, use the constituent type for formatting
199+
if (
200+
isUITreeRow(row) &&
201+
row.hasChildren === false &&
202+
column.constituentType != null
203+
) {
204+
columnTypeForFormatting = column.constituentType;
205+
}
206+
207+
return IrisGridUtils.colorForValue(
208+
theme,
209+
columnTypeForFormatting,
210+
column.name,
211+
value
212+
);
213+
}
214+
}
215+
216+
return theme.textColor;
217+
}
218+
219+
textAlignForCell(x: ModelIndex, y: ModelIndex): CanvasTextAlign {
220+
const column = this.sourceColumn(x, y);
221+
const row = this.row(y);
222+
assertNotNull(row);
223+
224+
let typeForFormatting = column.type;
225+
226+
// For tree table leaf nodes, use the constituent type for formatting
227+
if (
228+
isUITreeRow(row) &&
229+
row.hasChildren === false &&
230+
column.constituentType != null
231+
) {
232+
typeForFormatting = column.constituentType;
233+
}
234+
235+
return IrisGridUtils.textAlignForValue(typeForFormatting, column.name);
236+
}
237+
158238
extractViewportRow(row: DhType.TreeRow, columns: DhType.Column[]): UITreeRow {
159239
const { isExpanded, hasChildren, depth } = row;
160240
const extractedRow = super.extractViewportRow(row, columns);

0 commit comments

Comments
 (0)