rusackas commented on code in PR #34759:
URL: https://github.com/apache/superset/pull/34759#discussion_r3358058003
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -1520,3 +1520,99 @@ test('should assign distinct dash patterns for multiple
time offsets consistentl
// must be different patterns
expect(symbol1).not.toEqual(symbol2);
});
+
+describe('Tooltip with long labels', () => {
+ test('should use axisValue for tooltip when available (richTooltip)', () => {
+ const longLabelData: ChartDataResponseResult[] = [
+ createTestQueryData([
+ {
+ 'This is a very long category name that would normally be
truncated': 100,
+ __timestamp: 599616000000,
+ },
+ {
+ 'Another extremely long category name for testing purposes': 200,
+ __timestamp: 599916000000,
+ },
+ ]),
+ ];
+
+ const chartProps = createTestChartProps({
+ formData: {
+ richTooltip: true,
+ },
+ queriesData: longLabelData,
+ });
+
+ const transformedProps = transformProps(chartProps);
+
+ // Get the tooltip formatter function
+ const tooltipFormatter = (transformedProps.echartOptions as any).tooltip
+ .formatter;
+
+ // Simulate params from ECharts with axisValue containing full label
+ // Use distinct values for axisValue and seriesName to verify axisValue is
used
+ const mockParams = [
+ {
+ axisValue:
+ 'This is a very long category name that would normally be truncated',
+ value: [599616000000, 100],
+ seriesName: 'Some Series Name',
+ },
+ ];
+
+ // Call the formatter and check it uses the full label from axisValue
+ const result = tooltipFormatter(mockParams);
+ expect(result).toContain(
+ 'This is a very long category name that would normally be truncated',
+ );
+ });
+
+ test('should fallback to value when axisValue is not available', () => {
+ const chartProps = createTestChartProps({
+ formData: {
+ richTooltip: true,
+ },
+ });
+
+ const transformedProps = transformProps(chartProps);
+
+ const tooltipFormatter = (transformedProps.echartOptions as any).tooltip
+ .formatter;
+
+ // Simulate params without axisValue
+ const mockParams = [
+ {
+ value: [599616000000, 1],
+ seriesName: 'San Francisco',
+ },
+ ];
+
+ // Should still work with fallback to value
+ const result = tooltipFormatter(mockParams);
+ expect(result).toBeDefined();
+ expect(typeof result).toBe('string');
Review Comment:
Good catch — fixed in 9ff0ed8. Both fallback tests now assert that the
x-value (`599616000000`) actually appears in the rendered tooltip title, so a
regression in the fallback path would be caught rather than silently passing on
the `typeof === 'string'` check alone.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -1520,3 +1520,99 @@ test('should assign distinct dash patterns for multiple
time offsets consistentl
// must be different patterns
expect(symbol1).not.toEqual(symbol2);
});
+
+describe('Tooltip with long labels', () => {
Review Comment:
I kept the `describe` block here intentionally for consistency — this test
file already groups its suites with `describe` throughout (e.g.
`EchartsTimeseries transformProps`, `legend sorting`, etc.), and the `Tooltip
with long labels` group keeps these related cases together. Converting the
whole file to flat `test()` calls would be a broader, file-wide refactor
outside the scope of this tooltip fix, so I will leave it matching the
surrounding convention.
--
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]
