sadpandajoe commented on code in PR #38451:
URL: https://github.com/apache/superset/pull/38451#discussion_r3831177557
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -422,17 +424,23 @@ export default function transformProps(
let displayName: string;
- if (groupby.length > 0) {
- // When we have groupby, format as "metric, dimension"
- const metricPart: string = showQueryIdentifiers
- ? `${MetricDisplayNameA} (Query A)`
- : MetricDisplayNameA;
- displayName = entryName.includes(metricPart)
- ? entryName
- : `${metricPart}, ${entryName}`;
+ if (truncateMetric && groupby.length > 0) {
+ const groupbyValues = labelMap?.[seriesName] || [];
Review Comment:
A verbose column label that equals a group value can replace the value here:
`invert(verboseMap)` maps the series name back to the column key, so a `gender`
column labeled `boy` renders `gender` rather than `boy` when truncation is
enabled. Could this lookup prefer `entryName` before the inverted key?
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -422,17 +424,23 @@ export default function transformProps(
let displayName: string;
- if (groupby.length > 0) {
- // When we have groupby, format as "metric, dimension"
- const metricPart: string = showQueryIdentifiers
- ? `${MetricDisplayNameA} (Query A)`
- : MetricDisplayNameA;
- displayName = entryName.includes(metricPart)
- ? entryName
- : `${metricPart}, ${entryName}`;
+ if (truncateMetric && groupby.length > 0) {
+ const groupbyValues = labelMap?.[seriesName] || [];
+ displayName =
+ groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
Review Comment:
Agreed—when query identifiers are enabled, truncation drops the query
suffix, so identical values from A and B have indistinguishable legend and
tooltip labels. Could the truncated names retain the identifier?
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -495,17 +503,24 @@ export default function transformProps(
let displayName: string;
- if (groupbyB.length > 0) {
- // When we have groupby, format as "metric, dimension"
- const metricPart: string = showQueryIdentifiers
- ? `${MetricDisplayNameB} (Query B)`
- : MetricDisplayNameB;
- displayName = entryName.includes(metricPart)
- ? entryName
- : `${metricPart}, ${entryName}`;
+ if (truncateMetricB && groupbyB.length > 0) {
+ const groupbyValues =
+ labelMapB?.[seriesEntry] || labelMapB?.[entryName] || [];
+ displayName =
+ groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
Review Comment:
Agreed—Query B truncation reads unsuffixed keys while its formatter uses the
suffixed secondary-series key, so a response keyed by that series falls back to
the full name. Could this lookup include `seriesName`?
--
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]
