mirror of
https://github.com/apache/superset.git
synced 2026-04-19 08:04:53 +00:00
fix(table): align group headers correctly when filtering time compari… (#37236)
This commit is contained in:
@@ -710,9 +710,18 @@ export default function TableChart<D extends DataRecord = DataRecord>(
|
||||
);
|
||||
};
|
||||
|
||||
// Compute visible columns before groupHeaderColumns to ensure index consistency.
|
||||
// This filters out columns with config.visible === false.
|
||||
const visibleColumnsMeta = useMemo(
|
||||
() => filteredColumnsMeta.filter(col => col.config?.visible !== false),
|
||||
[filteredColumnsMeta],
|
||||
);
|
||||
|
||||
// Use visibleColumnsMeta for groupHeaderColumns to ensure indices match the actual
|
||||
// table columns. This fixes header misalignment when columns are filtered.
|
||||
const groupHeaderColumns = useMemo(
|
||||
() => getHeaderColumns(filteredColumnsMeta, isUsingTimeComparison),
|
||||
[filteredColumnsMeta, getHeaderColumns, isUsingTimeComparison],
|
||||
() => getHeaderColumns(visibleColumnsMeta, isUsingTimeComparison),
|
||||
[visibleColumnsMeta, getHeaderColumns, isUsingTimeComparison],
|
||||
);
|
||||
|
||||
const renderGroupingHeaders = (): JSX.Element => {
|
||||
@@ -720,12 +729,20 @@ export default function TableChart<D extends DataRecord = DataRecord>(
|
||||
const headers: any = [];
|
||||
let currentColumnIndex = 0;
|
||||
|
||||
Object.entries(groupHeaderColumns || {}).forEach(([key, value]) => {
|
||||
// Sort entries by their first column index to ensure correct left-to-right order.
|
||||
// Object.entries() maintains insertion order, but when columns are filtered,
|
||||
// the first occurrence of each metric might not match the visual column order.
|
||||
const sortedEntries = Object.entries(groupHeaderColumns || {}).sort(
|
||||
(a, b) => a[1][0] - b[1][0],
|
||||
);
|
||||
|
||||
sortedEntries.forEach(([key, value]) => {
|
||||
// Calculate the number of placeholder columns needed before the current header
|
||||
const startPosition = value[0];
|
||||
const colSpan = value.length;
|
||||
// Retrieve the originalLabel from the first column in this group
|
||||
const firstColumnInGroup = filteredColumnsMeta[startPosition];
|
||||
// Retrieve the originalLabel from the first column in this group.
|
||||
// Use visibleColumnsMeta to ensure consistent indexing with the actual table columns.
|
||||
const firstColumnInGroup = visibleColumnsMeta[startPosition];
|
||||
const originalLabel = firstColumnInGroup
|
||||
? columnsMeta.find(col => col.key === firstColumnInGroup.key)
|
||||
?.originalLabel || key
|
||||
@@ -1204,11 +1221,6 @@ export default function TableChart<D extends DataRecord = DataRecord>(
|
||||
],
|
||||
);
|
||||
|
||||
const visibleColumnsMeta = useMemo(
|
||||
() => filteredColumnsMeta.filter(col => col.config?.visible !== false),
|
||||
[filteredColumnsMeta],
|
||||
);
|
||||
|
||||
const columns = useMemo(
|
||||
() => visibleColumnsMeta.map(getColumnConfigs),
|
||||
[visibleColumnsMeta, getColumnConfigs],
|
||||
|
||||
@@ -680,6 +680,61 @@ describe('plugin-chart-table', () => {
|
||||
expectValidAriaLabels(container);
|
||||
});
|
||||
|
||||
test('should align group headers correctly when some comparison columns are hidden (#37074)', () => {
|
||||
// Test that group headers align correctly when columns have visible: false
|
||||
// This reproduces issue #37074 where headers became misaligned
|
||||
const props = transformProps(testData.comparisonWithHiddenColumns);
|
||||
|
||||
const { container } = render(<TableChart {...props} sticky={false} />);
|
||||
|
||||
// Get all header rows - first row contains group headers, second row contains column headers
|
||||
const headerRows = container.querySelectorAll('thead tr');
|
||||
expect(headerRows.length).toBe(2);
|
||||
|
||||
// Get group headers from the first row (th elements with colSpan > 1 or group headers)
|
||||
const groupHeaderRow = headerRows[0];
|
||||
const groupHeaders = groupHeaderRow.querySelectorAll('th');
|
||||
|
||||
// Extract group header text content (filter out empty placeholder headers)
|
||||
const groupHeaderTexts = Array.from(groupHeaders)
|
||||
.map(th => th.textContent?.trim())
|
||||
.filter(text => text && text.length > 0);
|
||||
|
||||
// Verify metric_1 group header appears before metric_2
|
||||
// With hidden columns: metric_1 has 2 visible columns (△, %), metric_2 has 4 (Main, #, △, %)
|
||||
const metric1Index = groupHeaderTexts.findIndex(
|
||||
text => text?.includes('metric_1') || text?.includes('Metric 1'),
|
||||
);
|
||||
const metric2Index = groupHeaderTexts.findIndex(
|
||||
text => text?.includes('metric_2') || text?.includes('Metric 2'),
|
||||
);
|
||||
|
||||
// Both headers should exist and metric_1 should come before metric_2
|
||||
expect(metric1Index).toBeGreaterThanOrEqual(0);
|
||||
expect(metric2Index).toBeGreaterThanOrEqual(0);
|
||||
expect(metric1Index).toBeLessThan(metric2Index);
|
||||
|
||||
// Verify colSpan values match the number of visible columns
|
||||
const metric1Header = Array.from(groupHeaders).find(
|
||||
th =>
|
||||
th.textContent?.includes('metric_1') ||
|
||||
th.textContent?.includes('Metric 1'),
|
||||
);
|
||||
const metric2Header = Array.from(groupHeaders).find(
|
||||
th =>
|
||||
th.textContent?.includes('metric_2') ||
|
||||
th.textContent?.includes('Metric 2'),
|
||||
);
|
||||
|
||||
// metric_1 should span 2 columns (△ and % are visible, Main and # are hidden)
|
||||
expect(metric1Header?.getAttribute('colspan')).toBe('2');
|
||||
// metric_2 should span 4 columns (all visible)
|
||||
expect(metric2Header?.getAttribute('colspan')).toBe('4');
|
||||
|
||||
// Verify ARIA labels are still valid after filtering
|
||||
expectValidAriaLabels(container);
|
||||
});
|
||||
|
||||
test('should set meaningful header IDs for regular table columns', () => {
|
||||
// Test regular (non-time-comparison) columns have proper IDs
|
||||
// Uses fallback to column.key since originalLabel is undefined
|
||||
|
||||
@@ -303,6 +303,81 @@ const comparisonWithConfig: TableChartProps = {
|
||||
emitCrossFilters: false,
|
||||
};
|
||||
|
||||
/**
|
||||
* Time comparison data with multiple metrics and some columns hidden.
|
||||
* Used to test that group headers align correctly when filtering columns.
|
||||
* Reproduces issue #37074.
|
||||
*/
|
||||
const comparisonWithHiddenColumns: TableChartProps = {
|
||||
...comparison,
|
||||
height: 400,
|
||||
width: 600,
|
||||
rawFormData: {
|
||||
...comparison.rawFormData,
|
||||
table_timestamp_format: 'smart_date',
|
||||
metrics: ['metric_1', 'metric_2'],
|
||||
percent_metrics: [],
|
||||
column_config: {
|
||||
// Hide Main and # columns for metric_1, only show △ and %
|
||||
'Main metric_1': { visible: false },
|
||||
'# metric_1': { visible: false },
|
||||
'△ metric_1': { d3NumberFormat: '.0f' },
|
||||
'% metric_1': { d3NumberFormat: '.2%' },
|
||||
// Show all columns for metric_2
|
||||
'Main metric_2': { d3NumberFormat: '.0f' },
|
||||
'# metric_2': { d3NumberFormat: '.0f' },
|
||||
'△ metric_2': { d3NumberFormat: '.0f' },
|
||||
'% metric_2': { d3NumberFormat: '.2%' },
|
||||
},
|
||||
time_compare: ['1 year ago'],
|
||||
comparison_color_enabled: true,
|
||||
comparison_type: ComparisonType.Values,
|
||||
},
|
||||
datasource: {
|
||||
...comparison.datasource,
|
||||
columnFormats: {},
|
||||
currencyFormats: {},
|
||||
verboseMap: { metric_1: 'Metric 1', metric_2: 'Metric 2' },
|
||||
},
|
||||
queriesData: [
|
||||
{
|
||||
...basicQueryResult,
|
||||
data: [
|
||||
{
|
||||
metric_1: 100,
|
||||
'metric_1__1 year ago': 80,
|
||||
metric_2: 200,
|
||||
'metric_2__1 year ago': 150,
|
||||
},
|
||||
],
|
||||
colnames: [
|
||||
'metric_1',
|
||||
'metric_1__1 year ago',
|
||||
'metric_2',
|
||||
'metric_2__1 year ago',
|
||||
],
|
||||
coltypes: [
|
||||
GenericDataType.Numeric,
|
||||
GenericDataType.Numeric,
|
||||
GenericDataType.Numeric,
|
||||
GenericDataType.Numeric,
|
||||
],
|
||||
},
|
||||
{
|
||||
...basicQueryResult,
|
||||
data: [{ rowcount: 1 }],
|
||||
},
|
||||
],
|
||||
filterState: { filters: {} },
|
||||
ownState: {},
|
||||
hooks: {
|
||||
onAddFilter: jest.fn(),
|
||||
setDataMask: jest.fn(),
|
||||
onContextMenu: jest.fn(),
|
||||
},
|
||||
emitCrossFilters: false,
|
||||
};
|
||||
|
||||
const raw = {
|
||||
...advanced,
|
||||
rawFormData: {
|
||||
@@ -402,6 +477,7 @@ export default {
|
||||
advancedWithCurrency,
|
||||
comparison,
|
||||
comparisonWithConfig,
|
||||
comparisonWithHiddenColumns,
|
||||
empty,
|
||||
raw,
|
||||
bigint,
|
||||
|
||||
Reference in New Issue
Block a user