sadpandajoe commented on code in PR #44550:
URL: https://github.com/apache/superset/pull/44550#discussion_r4143761190
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -483,6 +505,35 @@ export default function transformProps(
);
const isMultiSeries = groupBy.length || metrics?.length > 1;
+ // `x_axis_sort` stores either a series aggregate (a `SortSeriesType`) or
+ // the label of the x-axis column or of a metric. A single-series chart is
+ // sorted by the backend sort operator; with several series the rows are
+ // ordered here instead: by axis value, by an aggregate over every series,
+ // or by the sum of the chosen metric's columns. The field is carried
+ // through as-is, so a column or metric that happens to be named like an
+ // aggregate is never guessed at.
+ const sortSeriesTypes = new Set<string>(Object.values(SortSeriesType));
+ const resolveXAxisSortSeries = (): XAxisSortSeries | undefined => {
+ if (!isMultiSeries || typeof xAxisSort !== 'string') {
+ return undefined;
+ }
+ if (sortSeriesTypes.has(xAxisSort)) {
+ return xAxisSort as SortSeriesType;
+ }
+ if (
+ xAxisSort === xAxisLabel ||
+ xAxisSort === getXAxisLabel(chartProps.rawFormData)
+ ) {
+ return SortSeriesType.Name;
+ }
+ const dataColumns = new Set(Object.keys(rebasedData[0] ?? {}));
Review Comment:
When a chart has one displayed metric with a dimension and Truncate Metric
enabled, its pivoted columns are `PS4`/`XOne`, so this lookup finds no columns
for the newly offered metric selection and leaves the axis in query order.
Could we retain or map those metric columns for this case and cover selecting
the displayed metric?
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -557,8 +608,10 @@ export default function transformProps(
isHorizontal,
sortSeriesType,
sortSeriesAscending,
- xAxisSortSeries: isMultiSeries ? xAxisSort : undefined,
- xAxisSortSeriesAscending: isMultiSeries ? xAxisSortAsc : undefined,
+ xAxisSortSeries,
Review Comment:
When this client-side sort changes row order, `thresholdValues` remains in
query order while the series data is reordered, so percentage-threshold value
labels are evaluated against another category's total. Could this be permuted
with the sorted rows and covered by a reordered stacked case?
--
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]
