This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 7d77dc4fd2 fix: Time Comparison Feature Reverts Metric Labels to
Metric Keys in Table Charts (#32665)
7d77dc4fd2 is described below
commit 7d77dc4fd2001bfa1a752fb9a2da0cdc7349f886
Author: Fardin Mustaque <[email protected]>
AuthorDate: Tue Mar 25 17:52:15 2025 +0530
fix: Time Comparison Feature Reverts Metric Labels to Metric Keys in Table
Charts (#32665)
Co-authored-by: Fardin Mustaque <[email protected]>
---
.../plugins/plugin-chart-table/src/TableChart.tsx | 8 ++-
.../plugin-chart-table/src/transformProps.ts | 5 ++
.../plugins/plugin-chart-table/src/types.ts | 2 +
.../plugin-chart-table/test/TableChart.test.tsx | 80 ++++++++++++++++++++++
4 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
index 77905ea562..95121a317d 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
@@ -605,6 +605,8 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
// 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 originalLabel = columnsMeta[value[0]]?.originalLabel || key;
// Add placeholder <th> for columns before this header
for (let i = currentColumnIndex; i < startPosition; i += 1) {
@@ -620,7 +622,7 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
// Add the current header <th>
headers.push(
<th key={`header-${key}`} colSpan={colSpan} style={{ borderBottom: 0
}}>
- {key}
+ {originalLabel}
<span
css={css`
float: right;
@@ -975,7 +977,7 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
),
Footer: totals ? (
i === 0 ? (
- <th>
+ <th key={`footer-summary-${i}`}>
<div
css={css`
display: flex;
@@ -997,7 +999,7 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
</div>
</th>
) : (
- <td style={sharedStyle}>
+ <td key={`footer-total-${i}`} style={sharedStyle}>
<strong>{formatColumnValue(column, totals[key])[1]}</strong>
</td>
)
diff --git a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
index 48871e4ea4..d62d9cb92c 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
@@ -347,6 +347,7 @@ const processComparisonColumns = (
} = props;
const savedFormat = columnFormats?.[col.key];
const savedCurrency = currencyFormats?.[col.key];
+ const originalLabel = col.label;
if (
(col.isMetric || col.isPercentMetric) &&
!col.key.includes(comparisonSuffix) &&
@@ -355,6 +356,7 @@ const processComparisonColumns = (
return [
{
...col,
+ originalLabel,
label: t('Main'),
key: `${t('Main')} ${col.key}`,
config: getComparisonColConfig(t('Main'), col.key, columnConfig),
@@ -368,6 +370,7 @@ const processComparisonColumns = (
},
{
...col,
+ originalLabel,
label: `#`,
key: `# ${col.key}`,
config: getComparisonColConfig(`#`, col.key, columnConfig),
@@ -381,6 +384,7 @@ const processComparisonColumns = (
},
{
...col,
+ originalLabel,
label: `△`,
key: `△ ${col.key}`,
config: getComparisonColConfig(`△`, col.key, columnConfig),
@@ -394,6 +398,7 @@ const processComparisonColumns = (
},
{
...col,
+ originalLabel,
label: `%`,
key: `% ${col.key}`,
config: getComparisonColConfig(`%`, col.key, columnConfig),
diff --git a/superset-frontend/plugins/plugin-chart-table/src/types.ts
b/superset-frontend/plugins/plugin-chart-table/src/types.ts
index 1ec3cbe29d..62a666a88e 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/types.ts
+++ b/superset-frontend/plugins/plugin-chart-table/src/types.ts
@@ -56,6 +56,8 @@ export interface DataColumnMeta {
key: string;
// `label` is verbose column name used for rendering
label: string;
+ // `originalLabel` preserves the original label when time comparison
transforms the labels
+ originalLabel?: string;
dataType: GenericDataType;
formatter?:
| TimeFormatter
diff --git
a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
index b21a657b81..b74e1ffccf 100644
--- a/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx
@@ -175,6 +175,75 @@ describe('plugin-chart-table', () => {
?.formatter?.(0.123456);
expect(formattedPercentMetric).toBe('0.123');
});
+
+ it('should set originalLabel for comparison columns when time_compare and
comparison_type are set', () => {
+ const transformedProps = transformProps(testData.comparison);
+
+ // Check if comparison columns are processed
+ const comparisonColumns = transformedProps.columns.filter(
+ col =>
+ col.label === 'Main' ||
+ col.label === '#' ||
+ col.label === '△' ||
+ col.label === '%',
+ );
+
+ expect(comparisonColumns.length).toBeGreaterThan(0);
+ expect(comparisonColumns.some(col => col.label === 'Main')).toBe(true);
+ expect(comparisonColumns.some(col => col.label === '#')).toBe(true);
+ expect(comparisonColumns.some(col => col.label === '△')).toBe(true);
+ expect(comparisonColumns.some(col => col.label === '%')).toBe(true);
+
+ // Verify originalLabel for metric_1 comparison columns
+ const mainMetric1 = transformedProps.columns.find(
+ col => col.key === 'Main metric_1',
+ );
+ expect(mainMetric1).toBeDefined();
+ expect(mainMetric1?.originalLabel).toBe('metric_1');
+
+ const hashMetric1 = transformedProps.columns.find(
+ col => col.key === '# metric_1',
+ );
+ expect(hashMetric1).toBeDefined();
+ expect(hashMetric1?.originalLabel).toBe('metric_1');
+
+ const deltaMetric1 = transformedProps.columns.find(
+ col => col.key === '△ metric_1',
+ );
+ expect(deltaMetric1).toBeDefined();
+ expect(deltaMetric1?.originalLabel).toBe('metric_1');
+
+ const percentMetric1 = transformedProps.columns.find(
+ col => col.key === '% metric_1',
+ );
+ expect(percentMetric1).toBeDefined();
+ expect(percentMetric1?.originalLabel).toBe('metric_1');
+
+ // Verify originalLabel for metric_2 comparison columns
+ const mainMetric2 = transformedProps.columns.find(
+ col => col.key === 'Main metric_2',
+ );
+ expect(mainMetric2).toBeDefined();
+ expect(mainMetric2?.originalLabel).toBe('metric_2');
+
+ const hashMetric2 = transformedProps.columns.find(
+ col => col.key === '# metric_2',
+ );
+ expect(hashMetric2).toBeDefined();
+ expect(hashMetric2?.originalLabel).toBe('metric_2');
+
+ const deltaMetric2 = transformedProps.columns.find(
+ col => col.key === '△ metric_2',
+ );
+ expect(deltaMetric2).toBeDefined();
+ expect(deltaMetric2?.originalLabel).toBe('metric_2');
+
+ const percentMetric2 = transformedProps.columns.find(
+ col => col.key === '% metric_2',
+ );
+ expect(percentMetric2).toBeDefined();
+ expect(percentMetric2?.originalLabel).toBe('metric_2');
+ });
});
describe('TableChart', () => {
@@ -400,6 +469,17 @@ describe('plugin-chart-table', () => {
);
expect(getComputedStyle(screen.getByText('N/A')).background).toBe('');
});
+ it('should display originalLabel in grouped headers', () => {
+ render(
+ <ThemeProvider theme={supersetTheme}>
+ <TableChart {...transformProps(testData.comparison)} sticky={false}
/>
+ </ThemeProvider>,
+ );
+
+ const groupHeaders = screen.getAllByRole('columnheader');
+ expect(groupHeaders[0]).toHaveTextContent('metric_1');
+ expect(groupHeaders[1]).toHaveTextContent('metric_2');
+ });
});
it('render cell bars properly, and only when it is toggled on in both
regular and percent metrics', () => {
