Skip to content

Commit 3dfef4e

Browse files
authored
fix: DH-19693: Add null/empty formatting to constituent strings (#2464)
Part of DH-19693. Anything that renders as "empty" for empty strings, and "null" for null strings, should still render the same when rolledup. This adds the missing null and empty string formatting logic for tree table constituent values. Changes: - Checks if the result should show "null" or "empty" based on formatter
1 parent 35046d1 commit 3dfef4e

2 files changed

Lines changed: 159 additions & 1 deletion

File tree

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

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ 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 IrisGridTableModelTemplate from './IrisGridTableModelTemplate';
78
import IrisGridTreeTableModel, {
89
type UITreeRow,
910
} from './IrisGridTreeTableModel';
@@ -473,3 +474,143 @@ describe('IrisGridTreeTableModel textAlignForCell', () => {
473474
expect(result).toBe('left');
474475
});
475476
});
477+
478+
describe('IrisGridTreeTableModel textForCell', () => {
479+
let model: IrisGridTreeTableModel;
480+
481+
const columns = {
482+
string: irisGridTestUtils.makeColumn('StrCol', 'java.lang.String', 0),
483+
stringColWithStringConstituent: {
484+
...irisGridTestUtils.makeColumn('StrCol', 'java.lang.String', 0),
485+
constituentType: 'java.lang.String',
486+
},
487+
stringColWithIntConstituent: {
488+
...irisGridTestUtils.makeColumn('IntCol', 'int', 0),
489+
constituentType: 'int',
490+
} as DhType.Column,
491+
};
492+
493+
const rows = {
494+
leaf: {
495+
data: new Map(),
496+
hasChildren: false,
497+
isExpanded: false,
498+
depth: 2,
499+
} as UITreeRow,
500+
parent: {
501+
data: new Map(),
502+
hasChildren: true,
503+
isExpanded: false,
504+
depth: 1,
505+
} as UITreeRow,
506+
};
507+
508+
beforeEach(() => {
509+
const testColumns = irisGridTestUtils.makeColumns();
510+
const table = irisGridTestUtils.makeTreeTable(
511+
testColumns,
512+
testColumns.slice(0, 1),
513+
100,
514+
[]
515+
);
516+
model = new IrisGridTreeTableModel(dh, table);
517+
});
518+
519+
const mockTextCell = (
520+
value: unknown,
521+
column: DhType.Column,
522+
row: UITreeRow,
523+
displayText?: string
524+
) => {
525+
jest.spyOn(model, 'sourceColumn').mockReturnValue(column);
526+
jest.spyOn(model, 'row').mockReturnValue(row);
527+
jest.spyOn(model, 'valueForCell').mockReturnValue(value);
528+
if (displayText !== undefined) {
529+
jest.spyOn(model, 'displayString').mockReturnValue(displayText);
530+
}
531+
};
532+
533+
describe('leaf nodes with non-string constituent type', () => {
534+
it('handles null values', () => {
535+
jest
536+
.spyOn(model, 'columns', 'get')
537+
.mockReturnValue([columns.stringColWithIntConstituent]);
538+
mockTextCell(null, columns.stringColWithIntConstituent, rows.leaf, '');
539+
const result = model.textForCell(0, 0);
540+
expect(result).toBe('');
541+
});
542+
543+
it('handles normal values', () => {
544+
jest
545+
.spyOn(model, 'columns', 'get')
546+
.mockReturnValue([columns.stringColWithIntConstituent]);
547+
mockTextCell(21, columns.stringColWithIntConstituent, rows.leaf, '21');
548+
const result = model.textForCell(0, 0);
549+
expect(result).toBe('21');
550+
});
551+
});
552+
553+
describe('leaf nodes with formatter settings', () => {
554+
beforeEach(() => {
555+
jest.spyOn(model, 'columns', 'get').mockReturnValue([columns.string]);
556+
});
557+
558+
it('shows "null" when showNullStrings is true', () => {
559+
model.formatter.showNullStrings = true;
560+
mockTextCell(null, columns.stringColWithIntConstituent, rows.leaf, '');
561+
const result = model.textForCell(0, 0);
562+
expect(result).toBe('null');
563+
});
564+
565+
it('shows empty string when showNullStrings is false', () => {
566+
model.formatter.showNullStrings = false;
567+
mockTextCell(null, columns.stringColWithIntConstituent, rows.leaf, '');
568+
const result = model.textForCell(0, 0);
569+
expect(result).toBe('');
570+
});
571+
572+
it('shows "empty" when showEmptyStrings is true', () => {
573+
model.formatter.showEmptyStrings = true;
574+
mockTextCell('', columns.stringColWithIntConstituent, rows.leaf, '');
575+
const result = model.textForCell(0, 0);
576+
expect(result).toBe('empty');
577+
});
578+
579+
it('shows empty string when showEmptyStrings is false', () => {
580+
model.formatter.showEmptyStrings = false;
581+
mockTextCell('', columns.stringColWithIntConstituent, rows.leaf, '');
582+
const result = model.textForCell(0, 0);
583+
expect(result).toBe('');
584+
});
585+
});
586+
587+
describe('grouping column', () => {
588+
it('shows empty string for null values in rollup grouping columns', () => {
589+
Object.defineProperty(model.table, 'groupedColumns', {
590+
value: [],
591+
});
592+
jest
593+
.spyOn(model, 'getCachedGroupedColumnSet')
594+
.mockReturnValue(new Set([0]));
595+
mockTextCell(null, columns.string, { ...rows.parent, depth: 1 });
596+
const result = model.textForCell(0, 0);
597+
expect(result).toBe('');
598+
});
599+
});
600+
601+
describe('non-leaf nodes', () => {
602+
it('delegates to superClass for parent nodes', () => {
603+
jest
604+
.spyOn(IrisGridTableModelTemplate.prototype, 'textForCell')
605+
.mockReturnValue('parentString');
606+
mockTextCell(
607+
'parentString',
608+
columns.stringColWithIntConstituent,
609+
rows.parent,
610+
'parentString'
611+
);
612+
const result = model.textForCell(0, 0);
613+
expect(result).toBe('parentString');
614+
});
615+
});
616+
});

packages/iris-grid/src/IrisGridTreeTableModel.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,25 @@ class IrisGridTreeTableModel extends IrisGridTableModelTemplate<
163163
if (row != null && column != null) {
164164
if (!row.hasChildren && column.constituentType != null) {
165165
const value = this.valueForCell(x, y);
166-
return this.displayString(value, column.constituentType, column.name);
166+
const text = this.displayString(
167+
value,
168+
column.constituentType,
169+
column.name
170+
);
171+
172+
if (TableUtils.isTextType(this.columns[x]?.type)) {
173+
if (value === null) {
174+
return this.formatter.showNullStrings ? 'null' : '';
175+
}
176+
177+
if (text === '') {
178+
return this.formatter.showEmptyStrings ? 'empty' : '';
179+
}
180+
}
181+
182+
return text;
167183
}
184+
168185
// Show empty string instead of null in rollup grouping columns (issue #1483)
169186
if (
170187
row.hasChildren &&

0 commit comments

Comments
 (0)