YousufFFFF commented on code in PR #37712:
URL: https://github.com/apache/superset/pull/37712#discussion_r2775942636
##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx:
##########
@@ -146,37 +146,44 @@ export const xAxisSortControl = {
const columns = [controls?.x_axis?.value as QueryFormColumn].filter(
Boolean,
);
- const isSingleSortAvailable =
- ensureIsArray(controls?.groupby?.value).length === 0;
- const isMultiSortAvailable =
- !!ensureIsArray(controls?.groupby?.value).length ||
+ const hasGroupBy = ensureIsArray(controls?.groupby?.value).length > 0;
+ const hasMultipleMetrics =
ensureIsArray(controls?.metrics?.value).length > 1;
+ const isMultiSortAvailable = hasGroupBy || hasMultipleMetrics;
const metrics = [
...ensureIsArray(controls?.metrics?.value as QueryFormMetric),
controls?.timeseries_limit_metric?.value as QueryFormMetric,
].filter(Boolean);
const metricLabels = [...new Set(metrics.map(getMetricLabel))];
- const options = [
- ...(isSingleSortAvailable
- ? [
- ...columns.map(column => {
- const value = getColumnLabel(column);
- return { value, label: dataset?.verbose_map?.[value] || value
};
- }),
- ...metricLabels.map(value => ({
- value,
- label: dataset?.verbose_map?.[value] || value,
- })),
- ]
- : []),
- ...(isMultiSortAvailable
- ? SORT_SERIES_CHOICES.map(choice => ({
- value: choice[0],
- label: choice[1],
- }))
- : []),
+
+ // Base axis sort options: always include the explicit axis column and
metrics,
+ // regardless of whether a dimension (groupby) is present. This keeps the
+ // "X-Axis Sort By" dropdown consistent when toggling dimensions.
+ const axisSortOptions = [
+ ...columns.map(column => {
Review Comment:
Thanks for the note - agreed that in multi-series mode the ECharts pipeline
interprets
xAxisSort as a SortSeriesType, and supporting arbitrary metric/axis values
there would
require additional encoding/mapping and consumer changes.
To keep this PR scoped, the intent here is limited to keeping the “X-Axis
Sort By”
dropdown options consistent when toggling dimensions, without changing the
underlying
sorting semantics. Happy to follow up with a separate PR if we decide to
expand support
for metric-based sorting with dimensions.
--
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]
