rusackas commented on code in PR #44550:
URL: https://github.com/apache/superset/pull/44550#discussion_r4319715923
##########
superset-frontend/packages/superset-ui-chart-controls/src/operators/renameOperator.ts:
##########
@@ -75,7 +91,9 @@ export const renameOperator:
PostProcessingFactory<PostProcessingRename> = (
: [formData.comparison_type, metricOnly, metricWithOffset].join(
TIME_COMPARISON_SEPARATOR,
),
- metrics.length > 1 ? `${metricOnly}, ${offsetLabel}` : offsetLabel,
+ metricCountWithExtras > 1
Review Comment:
Good catch @sadpandajoe. `transformProps` now also treats a `label_map`
entry leading with `<metric>, <offset>` as a derived series, so the Percentage
case formats as `25%`. Added a grouped Percentage tooltip test using that
shape. Not run locally; relying on CI.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -484,6 +540,95 @@ 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;
Review Comment:
Agreed @sadpandajoe. Column sort now uses its own case-sensitive axis-value
comparator instead of the `Name` aggregate, so `B, a` stays put with multiple
metrics. Added a `sortRows` test covering both orderings. Not run locally;
relying on CI.
--
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]
