Skip to content

Commit 44edc02

Browse files
vbabichvbabich
authored
fix: Cherry-pick DH-21258: Fix column group separators (#2599)
Cherry-pick #2597 Update GridUtils.getColumnSeparatorIndex to take column group depth into account. BREAKING CHANGE: getColumnSeparatorIndex now requires a model argument.
1 parent bb592e9 commit 44edc02

File tree

3 files changed

+211
-10
lines changed

3 files changed

+211
-10
lines changed

‎packages/grid/src/GridUtils.test.ts‎

Lines changed: 150 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
import { AxisRange } from './GridAxisRange';
2-
import GridMetrics, { ModelIndex, MoveOperation } from './GridMetrics';
3-
import GridRange, { GridRangeIndex } from './GridRange';
4-
import GridUtils, { Token, TokenBox } from './GridUtils';
5-
import type { BoundedAxisRange } from './GridAxisRange';
1+
import { TestUtils } from '@deephaven/utils';
2+
import { type AxisRange, type BoundedAxisRange } from './GridAxisRange';
3+
import { type ModelIndex, type MoveOperation } from './GridMetrics';
4+
import type GridMetrics from './GridMetrics';
5+
import type GridModel from './GridModel';
6+
import GridRange, { type GridRangeIndex } from './GridRange';
7+
import GridTheme from './GridTheme';
8+
import GridUtils, { type Token, type TokenBox } from './GridUtils';
69

710
function expectModelIndexes(
811
movedItems: MoveOperation[],
@@ -1065,3 +1068,145 @@ describe('translateTokenBox', () => {
10651068
expect(GridUtils.translateTokenBox(input, metrics)).toEqual(expectedValue);
10661069
});
10671070
});
1071+
1072+
describe('getColumnSeparatorIndex', () => {
1073+
const mockTheme = {
1074+
...GridTheme,
1075+
allowColumnResize: true,
1076+
headerSeparatorHandleSize: 10,
1077+
columnHeaderHeight: 30,
1078+
};
1079+
1080+
const createMockMetrics = (
1081+
columnHeaderMaxDepth = 1
1082+
): Partial<GridMetrics> => ({
1083+
rowHeaderWidth: 50,
1084+
columnHeaderHeight: 30,
1085+
columnHeaderMaxDepth,
1086+
floatingColumns: [],
1087+
floatingLeftWidth: 0,
1088+
visibleColumns: [0, 1, 2, 3],
1089+
allColumnXs: new Map([
1090+
[0, 0],
1091+
[1, 100],
1092+
[2, 200],
1093+
[3, 300],
1094+
]),
1095+
allColumnWidths: new Map([
1096+
[0, 100],
1097+
[1, 100],
1098+
[2, 100],
1099+
[3, 100],
1100+
]),
1101+
modelColumns: new Map([
1102+
[0, 0],
1103+
[1, 1],
1104+
[2, 2],
1105+
[3, 3],
1106+
]),
1107+
// Additional properties needed for getRowAtY (called by getColumnHeaderDepthAtY)
1108+
gridY: 30,
1109+
floatingTopRowCount: 0,
1110+
floatingBottomRowCount: 0,
1111+
rowCount: 100,
1112+
visibleRows: [],
1113+
allRowYs: new Map(),
1114+
allRowHeights: new Map(),
1115+
});
1116+
1117+
/**
1118+
* Creates a mock GridModel with grouped column headers for testing
1119+
*/
1120+
const createMockGroupedGridModel = (
1121+
headerGroups: Map<number, Map<number, string>>
1122+
): GridModel =>
1123+
TestUtils.createMockProxy<GridModel>({
1124+
columnCount: 4,
1125+
rowCount: 100,
1126+
columnHeaderMaxDepth: headerGroups.size,
1127+
textForColumnHeader: (column: ModelIndex, depth = 0) =>
1128+
headerGroups.get(depth)?.get(column) ?? '',
1129+
});
1130+
1131+
const singleLevelHeaderGroups = new Map([
1132+
[
1133+
0,
1134+
new Map([
1135+
[0, 'A'],
1136+
[1, 'B'],
1137+
[2, 'C'],
1138+
[3, 'D'],
1139+
]),
1140+
],
1141+
]);
1142+
1143+
const multiLevelHeaderGroups = new Map([
1144+
[
1145+
0,
1146+
new Map([
1147+
[0, 'A'],
1148+
[1, 'B'],
1149+
[2, 'C'],
1150+
[3, 'D'],
1151+
]),
1152+
],
1153+
[
1154+
1,
1155+
new Map([
1156+
[0, 'Group1'],
1157+
[1, 'Group1'],
1158+
[2, 'Group2'],
1159+
[3, 'Group2'],
1160+
]),
1161+
],
1162+
]);
1163+
1164+
it.each([
1165+
{
1166+
description: 'detects separator at column boundary',
1167+
x: 150, // At boundary between column 0 and 1 (100 + 50)
1168+
y: 15, // Middle of the top header (maxDepth - 1)
1169+
headerGroups: singleLevelHeaderGroups,
1170+
maxDepth: 1,
1171+
expected: 0,
1172+
},
1173+
{
1174+
description: 'detects there is no separator within the column',
1175+
x: 120, // Within column 1
1176+
y: 15,
1177+
headerGroups: singleLevelHeaderGroups,
1178+
maxDepth: 1,
1179+
expected: null,
1180+
},
1181+
{
1182+
description:
1183+
'should return null at depth 1 when no separator exists (columns in same group)',
1184+
x: 150, // Between column 0 and 1
1185+
y: 15, // Middle of the top header (maxDepth - 1)
1186+
headerGroups: multiLevelHeaderGroups,
1187+
maxDepth: 2,
1188+
expected: null,
1189+
},
1190+
{
1191+
description: 'should detect separator at depth 1 when groups differ',
1192+
x: 250, // Between Group1 and Group2
1193+
y: 15,
1194+
headerGroups: multiLevelHeaderGroups,
1195+
maxDepth: 2,
1196+
expected: 1,
1197+
},
1198+
])('$description', ({ x, y, headerGroups, maxDepth, expected }) => {
1199+
const metrics = createMockMetrics(maxDepth) as GridMetrics;
1200+
const model = createMockGroupedGridModel(headerGroups);
1201+
1202+
const result = GridUtils.getColumnSeparatorIndex(
1203+
x,
1204+
y,
1205+
metrics,
1206+
mockTheme,
1207+
model
1208+
);
1209+
1210+
expect(result).toBe(expected);
1211+
});
1212+
});

‎packages/grid/src/GridUtils.ts‎

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ import {
2424
Range,
2525
} from './GridAxisRange';
2626
import { isExpandableGridModel } from './ExpandableGridModel';
27-
import { GridRenderState } from './GridRendererTypes';
27+
import { type GridRenderState } from './GridRendererTypes';
28+
import type GridModel from './GridModel';
2829

2930
export type GridPoint = {
3031
x: Coordinate;
@@ -453,19 +454,53 @@ export class GridUtils {
453454
);
454455
}
455456

457+
/**
458+
* Check if a separator exists between a column and the next column at a given depth.
459+
* A separator exists if adjacent columns have different header text at the specified depth.
460+
*
461+
* @param model The grid model
462+
* @param depth The header depth to check at
463+
* @param columnIndex The current model column index
464+
* @param nextColumnIndex The next model column index (undefined for last column)
465+
* @returns true if a separator should be shown, false otherwise
466+
*/
467+
static hasColumnSeparatorAtDepth(
468+
model: GridModel,
469+
depth: number | undefined,
470+
columnIndex: ModelIndex | undefined,
471+
nextColumnIndex: ModelIndex | undefined
472+
): boolean {
473+
if (depth == null || columnIndex == null) {
474+
return false;
475+
}
476+
477+
// Always show separator for the last column
478+
if (nextColumnIndex == null) {
479+
return true;
480+
}
481+
482+
// A separator exists if adjacent columns have different header text at this depth
483+
return (
484+
model.textForColumnHeader(columnIndex, depth) !==
485+
model.textForColumnHeader(nextColumnIndex, depth)
486+
);
487+
}
488+
456489
/**
457490
* Gets the column index if the x/y coordinates provided are close enough to the separator, otherwise null
458491
* @param x Mouse x coordinate
459492
* @param y Mouse y coordinate
460493
* @param metrics The grid metrics
461494
* @param theme The grid theme with potential user overrides
495+
* @param model The grid model
462496
* @returns Index of the column separator at the coordinates provided, or null if none match
463497
*/
464498
static getColumnSeparatorIndex(
465499
x: Coordinate,
466500
y: Coordinate,
467501
metrics: GridMetrics,
468-
theme: GridTheme
502+
theme: GridTheme,
503+
model: GridModel
469504
): VisibleIndex | null {
470505
const {
471506
rowHeaderWidth,
@@ -476,6 +511,7 @@ export class GridUtils {
476511
allColumnXs,
477512
allColumnWidths,
478513
columnHeaderMaxDepth,
514+
modelColumns,
479515
} = metrics;
480516
const { allowColumnResize, headerSeparatorHandleSize } = theme;
481517

@@ -489,6 +525,7 @@ export class GridUtils {
489525

490526
const gridX = x - rowHeaderWidth;
491527
const halfSeparatorSize = headerSeparatorHandleSize * 0.5;
528+
const depth = GridUtils.getColumnHeaderDepthAtY(y, metrics);
492529

493530
// Iterate through the floating columns first since they're on top
494531
let isPreviousColumnHidden = false;
@@ -507,7 +544,16 @@ export class GridUtils {
507544

508545
const minX = midX - halfSeparatorSize;
509546
const maxX = midX + halfSeparatorSize;
510-
if (minX <= gridX && gridX <= maxX) {
547+
if (
548+
minX <= gridX &&
549+
gridX <= maxX &&
550+
GridUtils.hasColumnSeparatorAtDepth(
551+
model,
552+
depth,
553+
modelColumns.get(column),
554+
modelColumns.get(column + 1)
555+
)
556+
) {
511557
return column;
512558
}
513559

@@ -538,7 +584,16 @@ export class GridUtils {
538584

539585
const minX = midX - halfSeparatorSize;
540586
const maxX = midX + halfSeparatorSize;
541-
if (minX <= gridX && gridX <= maxX) {
587+
if (
588+
minX <= gridX &&
589+
gridX <= maxX &&
590+
GridUtils.hasColumnSeparatorAtDepth(
591+
model,
592+
depth,
593+
modelColumns.get(column),
594+
modelColumns.get(column + 1)
595+
)
596+
) {
542597
return column;
543598
}
544599

‎packages/grid/src/mouse-handlers/GridColumnSeparatorMouseHandler.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ class GridColumnSeparatorMouseHandler extends GridSeparatorMouseHandler {
2828
x,
2929
y,
3030
metrics,
31-
theme
31+
theme,
32+
model
3233
);
3334

3435
if (separatorIndex == null || depth == null) {

0 commit comments

Comments
 (0)