jesperct commented on code in PR #40220:
URL: https://github.com/apache/superset/pull/40220#discussion_r3364673570
##########
superset-frontend/src/chartCustomizations/components/DynamicGroupBy/types.ts:
##########
@@ -76,7 +76,6 @@ export const DEFAULT_FORM_DATA:
PluginFilterGroupByCustomizeProps = {
dataset: null,
column: null,
sortFilter: false,
- sortAscending: true,
canSelectMultiple: true,
Review Comment:
Intentional. With the default removed, an unset "Sort display control
values" toggle now follows the dataset's source order, while the toggle
controls A-Z / Z-A. This is documented in UPDATING.md: existing filters with
the toggle unset show source order, and enabling it restores alphabetical
ordering.
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.tsx:
##########
@@ -210,6 +212,18 @@ const DescriptionTooltip = ({ description }: {
description: string }) => (
</ToolTipContainer>
);
+// Sort display values by label: ascending when sortAscending is true,
descending
+// when false, and source order (no sort) when it is unset.
+export const createLabelSortComparator =
+ (sortAscending?: boolean) =>
+ (a: LabeledValue, b: LabeledValue): number => {
+ if (sortAscending === undefined) {
+ return 0;
+ }
+ const labelComparator = propertyComparator('label');
+ return sortAscending ? labelComparator(a, b) : labelComparator(b, a);
+ };
Review Comment:
Fair point, the comparator logic is the same. Leaving it as-is for now:
extracting a shared helper would make chartCustomizations depend on the
dashboard module, and this PR is already approved and waiting to merge. Happy
to do the dedup as a follow-up.
--
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]
