rusackas commented on code in PR #38522:
URL: https://github.com/apache/superset/pull/38522#discussion_r3523801930
##########
superset-frontend/plugins/plugin-chart-echarts/src/Heatmap/transformProps.ts:
##########
@@ -370,20 +370,32 @@ export default function transformProps(
metricLabel,
);
const paramsValue = params.value as (string | number)[];
- const x = paramsValue?.[0];
- const y = paramsValue?.[1];
+ // paramsValue contains [xIndex, yIndex, metricValue, rankValue?]
+ // We need to look up the actual axis values from the sorted arrays
+ const xIndex = paramsValue?.[0] as number;
+ const yIndex = paramsValue?.[1] as number;
const value = paramsValue?.[2] as number | null | undefined;
- const formattedX = xAxisFormatter(x);
- const formattedY = yAxisFormatter(y);
+ const xValue = sortedXAxisValues[xIndex];
+ const yValue = sortedYAxisValues[yIndex];
+ // Format the axis values for display (handle null/undefined with
empty string fallback)
+ // Convert to string/number for formatter compatibility
+ const formattedX =
+ xValue !== null && xValue !== undefined
+ ? xAxisFormatter(xValue as string | number)
+ : '';
+ const formattedY =
+ yValue !== null && yValue !== undefined
+ ? yAxisFormatter(yValue as string | number)
+ : '';
const formattedValue = valueFormatter(value);
let percentage = 0;
let suffix = 'heatmap';
if (typeof value === 'number') {
if (normalizeAcross === 'x') {
- percentage = value / totals.x[x];
+ percentage = value / totals.x[String(xValue)];
suffix = formattedX;
} else if (normalizeAcross === 'y') {
- percentage = value / totals.y[y];
+ percentage = value / totals.y[String(yValue)];
suffix = formattedY;
} else {
percentage = value / totals.total;
Review Comment:
The division by zero predates this PR and only bites when an entire row or
column sums to zero... fine as a follow-up guard, not something this fix needs
to solve.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Heatmap/transformProps.ts:
##########
@@ -370,20 +370,32 @@ export default function transformProps(
metricLabel,
);
const paramsValue = params.value as (string | number)[];
- const x = paramsValue?.[0];
- const y = paramsValue?.[1];
+ // paramsValue contains [xIndex, yIndex, metricValue, rankValue?]
+ // We need to look up the actual axis values from the sorted arrays
+ const xIndex = paramsValue?.[0] as number;
+ const yIndex = paramsValue?.[1] as number;
Review Comment:
ECharts hands us numeric indices here, and even if a string slipped through,
`arr['1']` and `arr[1]` hit the same element in JS... no runtime difference to
fix.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts:
##########
@@ -359,4 +359,171 @@ describe('Heatmap transformProps', () => {
);
expect((resultWithoutLegend.echartOptions.legend as any).show).toBe(false);
});
+
+ test('tooltip formatter should display actual axis values, not indices', ()
=> {
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+ expect(typeof tooltipFormatter).toBe('function');
+
+ // Simulate tooltip data: [xIndex, yIndex, value]
+ // With alpha_asc sorting: xAxis = ['Friday', 'Monday', 'Thursday',
'Tuesday', 'Wednesday']
+ // yAxis = [9, 10, 11, 14, 15, 16]
+ // So index [1, 2, 15] should map to 'Monday' and hour 11
+ const mockParams = {
+ value: [1, 2, 15],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ // Tooltip should contain the actual day name 'Monday', not the index '1'
+ expect(tooltipHtml).toContain('Monday');
+ // Tooltip should contain the actual hour '11', not the index '2'
+ expect(tooltipHtml).toContain('11');
+ // Should not contain raw indices
+ expect(tooltipHtml).not.toMatch(/\b1\s*\(/);
+ expect(tooltipHtml).not.toMatch(/\(\s*2\b/);
+ });
+
+ test('tooltip formatter should work with different sort orders', () => {
+ // Test with descending sort
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_desc',
+ sortYAxis: 'alpha_desc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With alpha_desc sorting: xAxis = ['Wednesday', 'Tuesday', 'Thursday',
'Monday', 'Friday']
+ // yAxis = [16, 15, 14, 11, 10, 9]
+ // So index [4, 5, 20] should map to 'Friday' and hour 9
+ const mockParams = {
+ value: [4, 5, 20],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Friday');
+ expect(tooltipHtml).toContain('9');
+ });
+
+ test('tooltip formatter should work with value-based sorting', () => {
+ const chartProps = createChartProps({
+ sortXAxis: 'value_asc',
+ sortYAxis: 'value_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With value_asc sorting on X: ['Wednesday', 'Tuesday', 'Thursday',
'Friday', 'Monday']
+ // With value_asc sorting on Y: [11, 9, 10, 14, 15, 16]
+ // So index [0, 0, 8] should map to 'Wednesday' and hour 11
+ const mockParams = {
+ value: [0, 0, 8],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Wednesday');
+ expect(tooltipHtml).toContain('11');
+ });
+
+ test('tooltip percentage calculation should use actual values, not indices',
() => {
+ const testData = [
+ { day_of_week: 'Monday', hour: 9, count: 10 },
+ { day_of_week: 'Monday', hour: 10, count: 20 },
+ { day_of_week: 'Tuesday', hour: 9, count: 30 },
+ ];
+
+ // Test normalizeAcross: 'x'
+ const chartPropsX = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'x',
+ },
+ testData,
+ );
+ const resultX = transformProps(chartPropsX as HeatmapChartProps);
+ const tooltipFormatterX = (resultX.echartOptions.tooltip as any).formatter;
+
+ // With alpha_asc: xAxis = ['Monday', 'Tuesday'], yAxis = [9, 10]
+ // Monday total = 30, Tuesday total = 30
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/30 = 33.33%, not based on index
+ const mockParamsX = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlX = tooltipFormatterX(mockParamsX);
+ expect(tooltipHtmlX).toContain('33');
+ expect(tooltipHtmlX).toContain('%');
+
+ // Test normalizeAcross: 'y'
+ const chartPropsY = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'y',
+ },
+ testData,
+ );
+ const resultY = transformProps(chartPropsY as HeatmapChartProps);
+ const tooltipFormatterY = (resultY.echartOptions.tooltip as any).formatter;
+
+ // Hour 9 total = 40 (10 + 30)
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/40 = 25%
+ const mockParamsY = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlY = tooltipFormatterY(mockParamsY);
+ expect(tooltipHtmlY).toContain('25');
+ expect(tooltipHtmlY).toContain('%');
+ });
+
+ test('tooltip formatter should handle numeric axes correctly', () => {
+ const numericData = [
+ { year: 2020, quarter: 1, revenue: 100 },
+ { year: 2021, quarter: 2, revenue: 150 },
+ { year: 2022, quarter: 3, revenue: 200 },
+ ];
+
+ const chartProps = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ xAxis: 'year',
+ groupby: ['quarter'],
+ },
+ numericData,
+ );
+
+ (chartProps as any).queriesData[0].colnames = [
+ 'year',
+ 'quarter',
+ 'revenue',
+ ];
+
+ const result = transformProps(chartProps as HeatmapChartProps);
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With alpha_asc: xAxis = [2020, 2021, 2022], yAxis = [1, 2, 3]
+ // Index [1, 1, 150] should map to year 2021 and quarter 2
+ const mockParams = {
+ value: [1, 1, 150],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('2021');
+ expect(tooltipHtml).toContain('2');
+ });
Review Comment:
This one's real... the numeric-axes test should set `metric: 'revenue'` so
totals aren't computed off a missing column. Same fix Joe's assertion note
points at.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts:
##########
@@ -359,4 +359,171 @@ describe('Heatmap transformProps', () => {
);
expect((resultWithoutLegend.echartOptions.legend as any).show).toBe(false);
});
+
+ test('tooltip formatter should display actual axis values, not indices', ()
=> {
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+ expect(typeof tooltipFormatter).toBe('function');
+
+ // Simulate tooltip data: [xIndex, yIndex, value]
+ // With alpha_asc sorting: xAxis = ['Friday', 'Monday', 'Thursday',
'Tuesday', 'Wednesday']
+ // yAxis = [9, 10, 11, 14, 15, 16]
+ // So index [1, 2, 15] should map to 'Monday' and hour 11
+ const mockParams = {
+ value: [1, 2, 15],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ // Tooltip should contain the actual day name 'Monday', not the index '1'
+ expect(tooltipHtml).toContain('Monday');
+ // Tooltip should contain the actual hour '11', not the index '2'
+ expect(tooltipHtml).toContain('11');
+ // Should not contain raw indices
+ expect(tooltipHtml).not.toMatch(/\b1\s*\(/);
+ expect(tooltipHtml).not.toMatch(/\(\s*2\b/);
+ });
+
+ test('tooltip formatter should work with different sort orders', () => {
+ // Test with descending sort
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_desc',
+ sortYAxis: 'alpha_desc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With alpha_desc sorting: xAxis = ['Wednesday', 'Tuesday', 'Thursday',
'Monday', 'Friday']
+ // yAxis = [16, 15, 14, 11, 10, 9]
+ // So index [4, 5, 20] should map to 'Friday' and hour 9
+ const mockParams = {
+ value: [4, 5, 20],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Friday');
+ expect(tooltipHtml).toContain('9');
+ });
+
+ test('tooltip formatter should work with value-based sorting', () => {
+ const chartProps = createChartProps({
+ sortXAxis: 'value_asc',
+ sortYAxis: 'value_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With value_asc sorting on X: ['Wednesday', 'Tuesday', 'Thursday',
'Friday', 'Monday']
+ // With value_asc sorting on Y: [11, 9, 10, 14, 15, 16]
+ // So index [0, 0, 8] should map to 'Wednesday' and hour 11
+ const mockParams = {
+ value: [0, 0, 8],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Wednesday');
+ expect(tooltipHtml).toContain('11');
+ });
+
+ test('tooltip percentage calculation should use actual values, not indices',
() => {
+ const testData = [
+ { day_of_week: 'Monday', hour: 9, count: 10 },
+ { day_of_week: 'Monday', hour: 10, count: 20 },
+ { day_of_week: 'Tuesday', hour: 9, count: 30 },
+ ];
+
+ // Test normalizeAcross: 'x'
+ const chartPropsX = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'x',
+ },
+ testData,
+ );
+ const resultX = transformProps(chartPropsX as HeatmapChartProps);
+ const tooltipFormatterX = (resultX.echartOptions.tooltip as any).formatter;
+
+ // With alpha_asc: xAxis = ['Monday', 'Tuesday'], yAxis = [9, 10]
+ // Monday total = 30, Tuesday total = 30
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/30 = 33.33%, not based on index
+ const mockParamsX = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlX = tooltipFormatterX(mockParamsX);
+ expect(tooltipHtmlX).toContain('33');
+ expect(tooltipHtmlX).toContain('%');
+
+ // Test normalizeAcross: 'y'
+ const chartPropsY = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'y',
+ },
+ testData,
+ );
+ const resultY = transformProps(chartPropsY as HeatmapChartProps);
+ const tooltipFormatterY = (resultY.echartOptions.tooltip as any).formatter;
+
+ // Hour 9 total = 40 (10 + 30)
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/40 = 25%
+ const mockParamsY = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlY = tooltipFormatterY(mockParamsY);
+ expect(tooltipHtmlY).toContain('25');
+ expect(tooltipHtmlY).toContain('%');
+ });
+
+ test('tooltip formatter should handle numeric axes correctly', () => {
+ const numericData = [
+ { year: 2020, quarter: 1, revenue: 100 },
+ { year: 2021, quarter: 2, revenue: 150 },
+ { year: 2022, quarter: 3, revenue: 200 },
+ ];
+
+ const chartProps = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ xAxis: 'year',
+ groupby: ['quarter'],
+ },
+ numericData,
+ );
Review Comment:
Same point as the CodeAnt thread above (tracking it there)... though the
assertions here don't actually fail since they only touch axis labels.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Heatmap/transformProps.ts:
##########
@@ -365,8 +358,8 @@ export default function transformProps(
formatter: (params: CallbackDataParams) => {
const totals = calculateTotals(
data,
- xAxisLabel,
- yAxisLabel,
+ xAxisColumnName,
+ yAxisColumnName,
Review Comment:
Good catch... the SUMMARY still says test-only from before the
`calculateTotals` change landed on this branch. Description should be updated
to match the title.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts:
##########
@@ -359,4 +359,171 @@ describe('Heatmap transformProps', () => {
);
expect((resultWithoutLegend.echartOptions.legend as any).show).toBe(false);
});
+
+ test('tooltip formatter should display actual axis values, not indices', ()
=> {
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+ expect(typeof tooltipFormatter).toBe('function');
+
+ // Simulate tooltip data: [xIndex, yIndex, value]
+ // With alpha_asc sorting: xAxis = ['Friday', 'Monday', 'Thursday',
'Tuesday', 'Wednesday']
+ // yAxis = [9, 10, 11, 14, 15, 16]
+ // So index [1, 2, 15] should map to 'Monday' and hour 11
+ const mockParams = {
+ value: [1, 2, 15],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ // Tooltip should contain the actual day name 'Monday', not the index '1'
+ expect(tooltipHtml).toContain('Monday');
+ // Tooltip should contain the actual hour '11', not the index '2'
Review Comment:
Fair on robustness, but the fixture is local to this file and the hardcoded
orders double as documentation of the sort behavior... fine as-is.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts:
##########
@@ -359,4 +359,171 @@ describe('Heatmap transformProps', () => {
);
expect((resultWithoutLegend.echartOptions.legend as any).show).toBe(false);
});
+
+ test('tooltip formatter should display actual axis values, not indices', ()
=> {
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+ expect(typeof tooltipFormatter).toBe('function');
+
+ // Simulate tooltip data: [xIndex, yIndex, value]
+ // With alpha_asc sorting: xAxis = ['Friday', 'Monday', 'Thursday',
'Tuesday', 'Wednesday']
+ // yAxis = [9, 10, 11, 14, 15, 16]
+ // So index [1, 2, 15] should map to 'Monday' and hour 11
+ const mockParams = {
+ value: [1, 2, 15],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ // Tooltip should contain the actual day name 'Monday', not the index '1'
+ expect(tooltipHtml).toContain('Monday');
+ // Tooltip should contain the actual hour '11', not the index '2'
+ expect(tooltipHtml).toContain('11');
+ // Should not contain raw indices
+ expect(tooltipHtml).not.toMatch(/\b1\s*\(/);
+ expect(tooltipHtml).not.toMatch(/\(\s*2\b/);
+ });
+
+ test('tooltip formatter should work with different sort orders', () => {
+ // Test with descending sort
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_desc',
+ sortYAxis: 'alpha_desc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With alpha_desc sorting: xAxis = ['Wednesday', 'Tuesday', 'Thursday',
'Monday', 'Friday']
+ // yAxis = [16, 15, 14, 11, 10, 9]
+ // So index [4, 5, 20] should map to 'Friday' and hour 9
+ const mockParams = {
+ value: [4, 5, 20],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Friday');
+ expect(tooltipHtml).toContain('9');
+ });
+
+ test('tooltip formatter should work with value-based sorting', () => {
+ const chartProps = createChartProps({
+ sortXAxis: 'value_asc',
+ sortYAxis: 'value_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With value_asc sorting on X: ['Wednesday', 'Tuesday', 'Thursday',
'Friday', 'Monday']
+ // With value_asc sorting on Y: [11, 9, 10, 14, 15, 16]
+ // So index [0, 0, 8] should map to 'Wednesday' and hour 11
+ const mockParams = {
+ value: [0, 0, 8],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Wednesday');
+ expect(tooltipHtml).toContain('11');
+ });
+
+ test('tooltip percentage calculation should use actual values, not indices',
() => {
+ const testData = [
+ { day_of_week: 'Monday', hour: 9, count: 10 },
+ { day_of_week: 'Monday', hour: 10, count: 20 },
+ { day_of_week: 'Tuesday', hour: 9, count: 30 },
+ ];
+
+ // Test normalizeAcross: 'x'
+ const chartPropsX = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'x',
+ },
+ testData,
+ );
+ const resultX = transformProps(chartPropsX as HeatmapChartProps);
+ const tooltipFormatterX = (resultX.echartOptions.tooltip as any).formatter;
+
+ // With alpha_asc: xAxis = ['Monday', 'Tuesday'], yAxis = [9, 10]
+ // Monday total = 30, Tuesday total = 30
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/30 = 33.33%, not based on index
+ const mockParamsX = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlX = tooltipFormatterX(mockParamsX);
+ expect(tooltipHtmlX).toContain('33');
+ expect(tooltipHtmlX).toContain('%');
Review Comment:
Agreed they could be tighter... Joe flagged the same class of issue, so one
pass tightening the assertions covers this. Tracking it on his thread.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts:
##########
@@ -359,4 +359,171 @@ describe('Heatmap transformProps', () => {
);
expect((resultWithoutLegend.echartOptions.legend as any).show).toBe(false);
});
+
+ test('tooltip formatter should display actual axis values, not indices', ()
=> {
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+ expect(typeof tooltipFormatter).toBe('function');
+
+ // Simulate tooltip data: [xIndex, yIndex, value]
+ // With alpha_asc sorting: xAxis = ['Friday', 'Monday', 'Thursday',
'Tuesday', 'Wednesday']
+ // yAxis = [9, 10, 11, 14, 15, 16]
+ // So index [1, 2, 15] should map to 'Monday' and hour 11
+ const mockParams = {
+ value: [1, 2, 15],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ // Tooltip should contain the actual day name 'Monday', not the index '1'
+ expect(tooltipHtml).toContain('Monday');
+ // Tooltip should contain the actual hour '11', not the index '2'
+ expect(tooltipHtml).toContain('11');
+ // Should not contain raw indices
+ expect(tooltipHtml).not.toMatch(/\b1\s*\(/);
+ expect(tooltipHtml).not.toMatch(/\(\s*2\b/);
+ });
+
+ test('tooltip formatter should work with different sort orders', () => {
+ // Test with descending sort
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_desc',
+ sortYAxis: 'alpha_desc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With alpha_desc sorting: xAxis = ['Wednesday', 'Tuesday', 'Thursday',
'Monday', 'Friday']
+ // yAxis = [16, 15, 14, 11, 10, 9]
+ // So index [4, 5, 20] should map to 'Friday' and hour 9
+ const mockParams = {
+ value: [4, 5, 20],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Friday');
+ expect(tooltipHtml).toContain('9');
+ });
+
+ test('tooltip formatter should work with value-based sorting', () => {
+ const chartProps = createChartProps({
+ sortXAxis: 'value_asc',
+ sortYAxis: 'value_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With value_asc sorting on X: ['Wednesday', 'Tuesday', 'Thursday',
'Friday', 'Monday']
+ // With value_asc sorting on Y: [11, 9, 10, 14, 15, 16]
+ // So index [0, 0, 8] should map to 'Wednesday' and hour 11
+ const mockParams = {
+ value: [0, 0, 8],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Wednesday');
+ expect(tooltipHtml).toContain('11');
+ });
+
+ test('tooltip percentage calculation should use actual values, not indices',
() => {
+ const testData = [
+ { day_of_week: 'Monday', hour: 9, count: 10 },
+ { day_of_week: 'Monday', hour: 10, count: 20 },
+ { day_of_week: 'Tuesday', hour: 9, count: 30 },
+ ];
+
+ // Test normalizeAcross: 'x'
+ const chartPropsX = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'x',
+ },
+ testData,
+ );
+ const resultX = transformProps(chartPropsX as HeatmapChartProps);
+ const tooltipFormatterX = (resultX.echartOptions.tooltip as any).formatter;
+
+ // With alpha_asc: xAxis = ['Monday', 'Tuesday'], yAxis = [9, 10]
+ // Monday total = 30, Tuesday total = 30
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/30 = 33.33%, not based on index
+ const mockParamsX = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlX = tooltipFormatterX(mockParamsX);
+ expect(tooltipHtmlX).toContain('33');
+ expect(tooltipHtmlX).toContain('%');
+
+ // Test normalizeAcross: 'y'
+ const chartPropsY = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'y',
+ },
+ testData,
+ );
+ const resultY = transformProps(chartPropsY as HeatmapChartProps);
+ const tooltipFormatterY = (resultY.echartOptions.tooltip as any).formatter;
+
+ // Hour 9 total = 40 (10 + 30)
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/40 = 25%
+ const mockParamsY = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlY = tooltipFormatterY(mockParamsY);
+ expect(tooltipHtmlY).toContain('25');
+ expect(tooltipHtmlY).toContain('%');
Review Comment:
Same as the thread above, covered by the same tightening pass.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts:
##########
@@ -359,4 +359,171 @@ describe('Heatmap transformProps', () => {
);
expect((resultWithoutLegend.echartOptions.legend as any).show).toBe(false);
});
+
+ test('tooltip formatter should display actual axis values, not indices', ()
=> {
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+ expect(typeof tooltipFormatter).toBe('function');
+
+ // Simulate tooltip data: [xIndex, yIndex, value]
+ // With alpha_asc sorting: xAxis = ['Friday', 'Monday', 'Thursday',
'Tuesday', 'Wednesday']
+ // yAxis = [9, 10, 11, 14, 15, 16]
+ // So index [1, 2, 15] should map to 'Monday' and hour 11
+ const mockParams = {
+ value: [1, 2, 15],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ // Tooltip should contain the actual day name 'Monday', not the index '1'
+ expect(tooltipHtml).toContain('Monday');
+ // Tooltip should contain the actual hour '11', not the index '2'
+ expect(tooltipHtml).toContain('11');
+ // Should not contain raw indices
+ expect(tooltipHtml).not.toMatch(/\b1\s*\(/);
+ expect(tooltipHtml).not.toMatch(/\(\s*2\b/);
+ });
+
+ test('tooltip formatter should work with different sort orders', () => {
+ // Test with descending sort
+ const chartProps = createChartProps({
+ sortXAxis: 'alpha_desc',
+ sortYAxis: 'alpha_desc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With alpha_desc sorting: xAxis = ['Wednesday', 'Tuesday', 'Thursday',
'Monday', 'Friday']
+ // yAxis = [16, 15, 14, 11, 10, 9]
+ // So index [4, 5, 20] should map to 'Friday' and hour 9
+ const mockParams = {
+ value: [4, 5, 20],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Friday');
+ expect(tooltipHtml).toContain('9');
+ });
+
+ test('tooltip formatter should work with value-based sorting', () => {
+ const chartProps = createChartProps({
+ sortXAxis: 'value_asc',
+ sortYAxis: 'value_asc',
+ });
+ const result = transformProps(chartProps as HeatmapChartProps);
+
+ const tooltipFormatter = (result.echartOptions.tooltip as any).formatter;
+
+ // With value_asc sorting on X: ['Wednesday', 'Tuesday', 'Thursday',
'Friday', 'Monday']
+ // With value_asc sorting on Y: [11, 9, 10, 14, 15, 16]
+ // So index [0, 0, 8] should map to 'Wednesday' and hour 11
+ const mockParams = {
+ value: [0, 0, 8],
+ };
+
+ const tooltipHtml = tooltipFormatter(mockParams);
+
+ expect(tooltipHtml).toContain('Wednesday');
+ expect(tooltipHtml).toContain('11');
+ });
+
+ test('tooltip percentage calculation should use actual values, not indices',
() => {
+ const testData = [
+ { day_of_week: 'Monday', hour: 9, count: 10 },
+ { day_of_week: 'Monday', hour: 10, count: 20 },
+ { day_of_week: 'Tuesday', hour: 9, count: 30 },
+ ];
+
+ // Test normalizeAcross: 'x'
+ const chartPropsX = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'x',
+ },
+ testData,
+ );
+ const resultX = transformProps(chartPropsX as HeatmapChartProps);
+ const tooltipFormatterX = (resultX.echartOptions.tooltip as any).formatter;
+
+ // With alpha_asc: xAxis = ['Monday', 'Tuesday'], yAxis = [9, 10]
+ // Monday total = 30, Tuesday total = 30
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/30 = 33.33%, not based on index
+ const mockParamsX = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlX = tooltipFormatterX(mockParamsX);
+ expect(tooltipHtmlX).toContain('33');
+ expect(tooltipHtmlX).toContain('%');
+
+ // Test normalizeAcross: 'y'
+ const chartPropsY = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ showPercentage: true,
+ normalizeAcross: 'y',
+ },
+ testData,
+ );
+ const resultY = transformProps(chartPropsY as HeatmapChartProps);
+ const tooltipFormatterY = (resultY.echartOptions.tooltip as any).formatter;
+
+ // Hour 9 total = 40 (10 + 30)
+ // Point [0, 0, 10] = Monday/9 with value 10
+ // Percentage should be 10/40 = 25%
+ const mockParamsY = {
+ value: [0, 0, 10],
+ };
+
+ const tooltipHtmlY = tooltipFormatterY(mockParamsY);
+ expect(tooltipHtmlY).toContain('25');
+ expect(tooltipHtmlY).toContain('%');
+ });
+
+ test('tooltip formatter should handle numeric axes correctly', () => {
+ const numericData = [
+ { year: 2020, quarter: 1, revenue: 100 },
+ { year: 2021, quarter: 2, revenue: 150 },
+ { year: 2022, quarter: 3, revenue: 200 },
+ ];
+
+ const chartProps = createChartProps(
+ {
+ sortXAxis: 'alpha_asc',
+ sortYAxis: 'alpha_asc',
+ xAxis: 'year',
+ groupby: ['quarter'],
Review Comment:
Duplicate of the CodeAnt metric thread, same `metric: 'revenue'` fix...
tracking there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
