sadpandajoe commented on code in PR #43113:
URL: https://github.com/apache/superset/pull/43113#discussion_r3917221772
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -606,6 +607,36 @@ export default function transformProps(
const array = ensureIsArray(chartProps.rawFormData?.time_compare);
const inverted = invert(verboseMap);
+ // A Percentage or Ratio time comparison replaces the derived series' values
with a
+ // dimensionless number, so that row is no longer in the source metric's
units and
+ // must not inherit its currency/D3 format. `renameOperator` names those
series with
+ // the offset alone or `<metric>, <offset>`, and a grouped chart appends its
dimension
+ // values on top ("1 week ago, East"). Recognise them with the same helper
the
+ // derived-series styling uses rather than matching exact names:
`getTimeOffset`
+ // covers every form except the bare offset of an ungrouped single-metric
chart,
+ // which the offsets themselves match.
+ const isDerivedComparisonSeries = (seriesKey: string) =>
+ array.includes(seriesKey) ||
+ getTimeOffset({ name: seriesKey }, array) !== undefined;
Review Comment:
A grouped base series can legitimately have a dimension value equal to the
configured offset (for example, `sum__num, 1 week ago`). `getTimeOffset` then
classifies that base series as derived, so Percentage mode formats its currency
value as a percent and Ratio mode drops its currency formatter. Could this use
the structured `label_map`/derived-series identity instead of matching the
rendered name?
--
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]
