rusackas commented on code in PR #43238:
URL: https://github.com/apache/superset/pull/43238#discussion_r4087115817
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -480,11 +480,57 @@ export default function transformProps(
? [minMarkerSize, maxMarkerSize]
: [maxMarkerSize, minMarkerSize];
+ // When stackDimension is configured, each series is assigned to a separate
+ // ECharts stack group keyed by the dimension value. Compute this mapping
+ // before calling extractShowValueIndexes so each group's topmost series is
+ // tracked independently (fixing the bug where only the last series across
+ // all groups was flagged to show the total label).
+ const idxSelectedDimension =
+ stack === StackControlsValue.Stack &&
+ stackDimension &&
+ chartProps.rawFormData?.groupby
+ ? formData.metrics.length > 1
+ ? 1
+ : 0 + chartProps.rawFormData.groupby.indexOf(stackDimension)
Review Comment:
Looks like this is covered at head. The offset is `(metrics.length > 1 ? 1 :
0) + groupby.indexOf(stackDimension)`, guarded by an `includes` check, and
there is a two-metric, two-groupby transformProps test asserting the stack and
the per-group totals follow the second dimension. Resolving.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -109,66 +109,66 @@ import {
transformIntervalAnnotation,
transformSeries,
transformTimeseriesAnnotation,
-} from './transformers';
+} from "./transformers";
import {
OpacityEnum,
StackControlsValue,
TIMEGRAIN_TO_TIMESTAMP,
TIMESERIES_CONSTANTS,
-} from '../constants';
-import { getDefaultTooltip } from '../utils/tooltip';
+} from "../constants";
+import { getDefaultTooltip } from "../utils/tooltip";
import {
getPercentFormatter,
getTooltipTimeFormatter,
getXAxisFormatter,
getYAxisFormatter,
-} from '../utils/formatters';
-import { safeParseEChartOptions } from '../utils/safeEChartOptionsParser';
-import { mergeCustomEChartOptions } from '../utils/mergeCustomEChartOptions';
+} from "../utils/formatters";
+import { safeParseEChartOptions } from "../utils/safeEChartOptionsParser";
+import { mergeCustomEChartOptions } from "../utils/mergeCustomEChartOptions";
-const visibleDashPatterns: ([number, number] | 'dashed' | 'dotted')[] = [
- 'dashed',
- 'dotted',
+const visibleDashPatterns: ([number, number] | "dashed" | "dotted")[] = [
+ "dashed",
+ "dotted",
[6, 15], // narrow dashed
[2, 10], // wide dotted
[20, 3], // wide dashed
];
const visibleSymbols = [
- 'rect',
- 'triangle',
- 'diamond',
- 'roundRect',
- 'pin',
+ "rect",
+ "triangle",
+ "diamond",
+ "roundRect",
+ "pin",
] as const;
function getSymbolMarker(symbol: string, color: string) {
const size = 10;
switch (symbol) {
- case 'circle':
+ case "circle":
return `<span style="
display:inline-block;width:${size}px;height:${size}px;
border-radius:50%;background:${color};margin-right:5px"></span>`;
- case 'rect':
+ case "rect":
return `<span style="
display:inline-block;width:${size}px;height:${size}px;
background:${color};margin-right:5px"></span>`;
- case 'roundRect':
+ case "roundRect":
return `<span style="
display:inline-block;width:${size}px;height:${size}px;border-radius:2px;
background:${color};margin-right:5px"></span>`;
- case 'triangle':
+ case "triangle":
return `<span style="
display:inline-block;width:0;height:0;
border-left:${size / 2}px solid transparent;
border-right:${size / 2}px solid transparent;
border-bottom:${size}px solid ${color};
margin-right:5px"></span>`;
- case 'diamond':
+ case "diamond":
Review Comment:
This is pre-existing code the PR does not touch (identical to `master`), so
it is out of scope here. Resolving to keep the thread focused.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -1108,9 +1159,9 @@ export default function transformProps(
// at the axis boundary.
...(showMaxLabel && {
showMaxLabel: true,
- alignMaxLabel: 'right',
+ alignMaxLabel: "right",
showMinLabel: true,
- alignMinLabel: 'left',
+ alignMinLabel: "left",
Review Comment:
Same as above, this block is untouched by the PR and matches `master`, so
not something to sort out here. Resolving.
--
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]
