WhoamiI00 commented on code in PR #43113:
URL: https://github.com/apache/superset/pull/43113#discussion_r3964810112
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -2625,3 +2625,186 @@ describe('tooltip for metrics whose labels end in
forecast suffixes', () => {
expect(html).toContain('>ci<');
});
});
+test('tooltip formats each series with its own metric format instead of the
default formatter', () => {
+ // Two saved metrics with different formats: `pct_change` carries a
percentage
+ // D3 format, `count` carries a currency format. The series labels already
+ // honor each metric's format; the tooltip must do the same.
+ const chartProps = createTestChartProps({
+ formData: {
+ metrics: ['count', 'pct_change'],
+ richTooltip: true,
+ },
+ queriesData: [
+ createTestQueryData(
+ [{ count: 1000, pct_change: 0.1234, __timestamp: BASE_TIMESTAMP }],
+ { label_map: { count: ['count'], pct_change: ['pct_change'] } },
Review Comment:
Thanks — that matches how I'd been treating it. Each of the currency /
Percentage / Ratio / Difference cases reads as its own scenario with its own
fixture, and the shared shape between them is mostly the `createTestChartProps`
boilerplate rather than logic, so extracting a helper would have hidden exactly
the field that differs between them (`comparison_type`, the label_map shape,
the expected format). Happy to leave it flat.
Current state, since this thread is the most recent activity: all checks
green, 123 tests in `Timeseries/transformProps.test.ts`. The branch covers the
four things that came out of review — per-metric formats in the tooltip,
verbose-name resolution, grouped Percentage rows, and Ratio rows taking a
unitless format rather than the metric's currency — plus a guard that
Difference keeps the metric format, since `s - c` stays in the metric's units.
The derived-row detection ended up reading the offset from `label_map`
rather than the rendered series name, which is what stopped a base series like
`sum__num, 1 week ago` (a region genuinely named "1 week ago") being misread as
derived. That identity is captured during the `timeCompare` normalisation,
before the leading offset is shifted off the entry.
Ready for a merge decision whenever you have a moment.
--
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]
