asdf2014 commented on code in PR #41933:
URL: https://github.com/apache/superset/pull/41933#discussion_r3559680852
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx:
##########
@@ -164,31 +169,35 @@ export default function EchartsMixedTimeseries({
formattedVal: xValueFormatter(data[0]),
});
}
- [
- ...(data && xAxis.type === AxisType.Category ? [xAxis.label] : []),
- ...(isFirst ? formData.groupby : formData.groupbyB),
- ].forEach((dimension, i) =>
+ if (data && xAxis.type === AxisType.Category && eventParams.name) {
drillToDetailFilters.push({
- col: dimension,
+ col: xAxis.label,
op: '==',
- val: values[i],
- formattedVal: String(values[i]),
- }),
- );
-
- [...(isFirst ? formData.groupby : formData.groupbyB)].forEach(
- (dimension, i) =>
+ val: eventParams.name,
+ formattedVal: eventParams.name,
+ });
Review Comment:
Adopted with a caveat: ECharts stringifies category names
(`convertOptionIdName` appends an empty string), so a numeric `0` reaches this
check as the truthy `'0'` — the reproduction as described cannot occur. The
truthiness check did, however, drop empty-string categories, which are legal
dimension values, so this was switched to an explicit null check in 2664dd0569.
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -810,14 +835,20 @@ export default function transformProps(
.forEach(key => {
const value = forecastValues[key];
// if there are no dimensions, key is a verbose name of a metric,
- // otherwise it is a comma separated string where the first part
is metric name
+ // otherwise it is a comma separated string where the first part
is metric name.
+ // The tooltip key is the rendered series name, so resolve it
through
+ // the display-keyed maps rather than the raw backend-keyed ones.
let formatterKey;
if (primarySeries.has(key)) {
formatterKey =
- groupby.length === 0 ? inverted[key] : labelMap[key]?.[0];
+ groupby.length === 0
+ ? inverted[key]
+ : displayLabelMap[key]?.[0];
} else {
formatterKey =
- groupbyB.length === 0 ? inverted[key] : labelMapB[key]?.[0];
+ groupbyB.length === 0
+ ? inverted[key]
+ : displayLabelMapB[key]?.[0];
Review Comment:
Adopted in 2664dd0569 — the no-groupby path now also resolves through the
display-keyed maps, with the verbose-name inversion kept as a fallback. Worth
noting `getCustomFormatter` short-circuits for a single saved metric, so the
miss only affected multi-metric queries without dimensions (when query
identifiers are shown or no verbose map exists).
--
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]
