bito-code-review[bot] commented on code in PR #40220:
URL: https://github.com/apache/superset/pull/40220#discussion_r3364674380
##########
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:
<!-- Bito Reply -->
The user's explanation clarifies that the removal of the default sort
behavior is intentional. By removing the default, the component now correctly
respects the dataset's source order when the toggle is unset, while the toggle
itself continues to control alphabetical sorting as documented in the project's
update guidelines. This aligns with the intended behavior for existing filters.
##########
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:
<!-- Bito Reply -->
Acknowledged. Since the PR is already approved and waiting to merge,
deferring the refactoring of the comparator logic to a follow-up task is a
reasonable approach to avoid unnecessary churn and dependency changes.
--
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]
