You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ri...@apache.org on 2026/02/10 23:19:04 UTC
(superset) 01/01: fix: address PR review - alphabetize icons and extract shared orderby helper
This is an automated email from the ASF dual-hosted git repository.
richardfogaca pushed a commit to branch feat/auto-refresh-dashboard
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8bb6d93e3c3c5639aefd086ff0a779deffa0d60c
Author: richard <ri...@richards-MacBook-Pro-2.local>
AuthorDate: Tue Feb 10 20:17:38 2026 -0300
fix: address PR review - alphabetize icons and extract shared orderby helper
---
.../src/components/Icons/AntdEnhanced.tsx | 16 ++++----
.../plugin-chart-echarts/src/Graph/buildQuery.ts | 20 +++++----
.../plugin-chart-echarts/src/Tree/buildQuery.ts | 19 ++-------
.../plugin-chart-echarts/src/Treemap/buildQuery.ts | 17 ++------
.../plugin-chart-echarts/src/utils/orderby.ts | 47 ++++++++++++++++++++++
5 files changed, 71 insertions(+), 48 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx b/superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx
index 58e4d8c02cd..b1914d3e267 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/components/Icons/AntdEnhanced.tsx
@@ -40,9 +40,6 @@ import {
CaretLeftOutlined,
CaretRightOutlined,
CaretRightFilled,
- PauseOutlined,
- PauseCircleOutlined,
- PlayCircleOutlined,
CalendarOutlined,
CheckOutlined,
CheckCircleOutlined,
@@ -109,8 +106,11 @@ import {
MoreOutlined,
OrderedListOutlined,
PartitionOutlined,
- PieChartOutlined,
+ PauseCircleOutlined,
+ PauseOutlined,
PicCenterOutlined,
+ PieChartOutlined,
+ PlayCircleOutlined,
PlusCircleOutlined,
PlusSquareOutlined,
PlusOutlined,
@@ -191,9 +191,6 @@ const AntdIcons = {
CaretLeftOutlined,
CaretRightOutlined,
CaretRightFilled,
- PauseOutlined,
- PauseCircleOutlined,
- PlayCircleOutlined,
CalendarOutlined,
CheckOutlined,
CheckCircleOutlined,
@@ -264,8 +261,11 @@ const AntdIcons = {
MoreOutlined,
OrderedListOutlined,
PartitionOutlined,
- PieChartOutlined,
+ PauseCircleOutlined,
+ PauseOutlined,
PicCenterOutlined,
+ PieChartOutlined,
+ PlayCircleOutlined,
PlusCircleOutlined,
PlusSquareOutlined,
PlusOutlined,
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts
index f2ee0c8be3f..78e431ac707 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/buildQuery.ts
@@ -16,21 +16,19 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
+import { buildQueryContext } from '@superset-ui/core';
import { EchartsGraphFormData } from './types';
+import { buildColumnsOrderBy, applyOrderBy } from '../utils/orderby';
export default function buildQuery(formData: EchartsGraphFormData) {
const { source, target, source_category, target_category, row_limit } =
formData;
- const orderby: QueryFormOrderBy[] = [];
- const shouldApplyOrderBy =
- row_limit !== undefined && row_limit !== null && row_limit !== 0;
-
- [source, target, source_category, target_category].forEach(column => {
- if (column) {
- orderby.push([column, true]);
- }
- });
+ const orderby = buildColumnsOrderBy([
+ source,
+ target,
+ source_category,
+ target_category,
+ ]);
return buildQueryContext(formData, {
queryFields: {
@@ -42,7 +40,7 @@ export default function buildQuery(formData: EchartsGraphFormData) {
buildQuery: baseQueryObject => [
{
...baseQueryObject,
- ...(shouldApplyOrderBy && orderby.length > 0 && { orderby }),
+ ...applyOrderBy(orderby, row_limit),
},
],
});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts
index d7a36e04508..09d99b28dd9 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Tree/buildQuery.ts
@@ -16,24 +16,13 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
+import { buildQueryContext } from '@superset-ui/core';
import { EchartsTreeFormData } from './types';
+import { buildColumnsOrderBy, applyOrderBy } from '../utils/orderby';
export default function buildQuery(formData: EchartsTreeFormData) {
const { id, parent, name, row_limit } = formData;
- const orderby: QueryFormOrderBy[] = [];
- const shouldApplyOrderBy =
- row_limit !== undefined && row_limit !== null && row_limit !== 0;
- const orderbyColumns: string[] = [];
-
- [parent, id, name].forEach(column => {
- if (column) {
- orderbyColumns.push(column);
- }
- });
- orderbyColumns.forEach(column => {
- orderby.push([column, true]);
- });
+ const orderby = buildColumnsOrderBy([parent, id, name]);
return buildQueryContext(formData, {
queryFields: {
@@ -44,7 +33,7 @@ export default function buildQuery(formData: EchartsTreeFormData) {
buildQuery: baseQueryObject => [
{
...baseQueryObject,
- ...(shouldApplyOrderBy && orderby.length > 0 && { orderby }),
+ ...applyOrderBy(orderby, row_limit),
},
],
});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts
index 84172b5b7e8..e6e76ac7ad3 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Treemap/buildQuery.ts
@@ -21,31 +21,20 @@ import {
QueryFormData,
QueryFormOrderBy,
} from '@superset-ui/core';
+import { buildColumnsOrderBy, applyOrderBy } from '../utils/orderby';
export default function buildQuery(formData: QueryFormData) {
const { metric, sort_by_metric, groupby = [], row_limit } = formData;
const orderby: QueryFormOrderBy[] = [];
- const shouldApplyOrderBy =
- row_limit !== undefined && row_limit !== null && row_limit !== 0;
-
if (sort_by_metric && metric) {
orderby.push([metric, false]);
}
- if (!sort_by_metric && groupby.length > 0) {
- groupby.forEach(column => {
- orderby.push([column, true]);
- });
- }
- if (sort_by_metric && groupby.length > 0) {
- groupby.forEach(column => {
- orderby.push([column, true]);
- });
- }
+ orderby.push(...buildColumnsOrderBy(groupby));
return buildQueryContext(formData, baseQueryObject => [
{
...baseQueryObject,
- ...(shouldApplyOrderBy && orderby.length > 0 && { orderby }),
+ ...applyOrderBy(orderby, row_limit),
},
]);
}
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts
new file mode 100644
index 00000000000..e8bca2f926a
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/orderby.ts
@@ -0,0 +1,47 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { QueryFormOrderBy } from '@superset-ui/core';
+
+/**
+ * Builds ascending orderby clauses from a list of columns, filtering out
+ * any null/undefined values. This ensures deterministic row ordering so
+ * that chart elements maintain stable positions across auto-refreshes.
+ */
+export function buildColumnsOrderBy(
+ columns: (string | undefined | null)[],
+ ascending: boolean = true,
+): QueryFormOrderBy[] {
+ return columns
+ .filter((col): col is string => !!col)
+ .map(col => [col, ascending]);
+}
+
+/**
+ * Conditionally applies orderby to a query object spread. Returns the
+ * orderby field only when row_limit is set (non-zero, non-null) and
+ * there are orderby entries to apply.
+ */
+export function applyOrderBy(
+ orderby: QueryFormOrderBy[],
+ rowLimit: string | number | undefined | null,
+): { orderby: QueryFormOrderBy[] } | Record<string, never> {
+ const shouldApply =
+ rowLimit !== undefined && rowLimit !== null && rowLimit !== 0;
+ return shouldApply && orderby.length > 0 ? { orderby } : {};
+}