From 2cf1486f768145a2e29aa5b20ec69be6404fe42b Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Fri, 18 Oct 2024 16:26:44 +0100
Subject: [PATCH] Optimize pie charts rendering with many rows (#48670)
---
.../view/sidebars/ChartSettingsSidebar.jsx | 4 +-
.../src/metabase/query_builder/selectors.js | 4 +-
.../src/metabase/query_builder/utils/index.ts | 2 +-
.../settings/ChartSettingSeriesOrder.tsx | 2 +
.../visualizations/echarts/pie/model/index.ts | 24 ++++++++--
.../visualizations/echarts/pie/util/colors.ts | 44 +++----------------
.../CartesianChart/chart-definition-legacy.js | 2 +
.../visualizations/PieChart/PieRowsPicker.tsx | 30 +++++++++++--
.../PieChart/chart-definition.ts | 40 ++++++++++++-----
9 files changed, 90 insertions(+), 62 deletions(-)
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.jsx b/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.jsx
index bbbee7cc7ba..6b622974017 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.jsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/ChartSettingsSidebar.jsx
@@ -43,11 +43,11 @@ function ChartSettingsSidebar(props) {
const series = useMemo(() => {
return [
{
+ ...result,
card,
- data: result.data,
},
];
- }, [card, result.data]);
+ }, [card, result]);
return (
result && (
diff --git a/frontend/src/metabase/query_builder/selectors.js b/frontend/src/metabase/query_builder/selectors.js
index 34f67c055e2..d23ef313fab 100644
--- a/frontend/src/metabase/query_builder/selectors.js
+++ b/frontend/src/metabase/query_builder/selectors.js
@@ -697,9 +697,10 @@ export const getRawSeries = createSelector(
datasetQuery: lastRunDatasetQuery,
});
if (isShowingRawTable && rawSeries?.length > 0) {
- const [{ card, data }] = rawSeries;
+ const [{ card, ...rest }] = rawSeries;
return [
{
+ ...rest,
card: {
...card,
display: "table",
@@ -708,7 +709,6 @@ export const getRawSeries = createSelector(
"table.pivot": false,
},
},
- data,
},
];
}
diff --git a/frontend/src/metabase/query_builder/utils/index.ts b/frontend/src/metabase/query_builder/utils/index.ts
index 52d4904b9b0..4d1c282213a 100644
--- a/frontend/src/metabase/query_builder/utils/index.ts
+++ b/frontend/src/metabase/query_builder/utils/index.ts
@@ -158,11 +158,11 @@ export const createRawSeries = (options: {
return (
queryResult && [
{
+ ...queryResult,
card: {
...question.card(),
...(datasetQuery && { dataset_query: datasetQuery }),
},
- data: queryResult && queryResult.data,
},
]
);
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingSeriesOrder.tsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingSeriesOrder.tsx
index 863d2f5e302..2ce266ac9e3 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingSeriesOrder.tsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingSeriesOrder.tsx
@@ -20,6 +20,8 @@ import {
ChartSettingOrderedSimpleRoot,
} from "./ChartSettingOrderedSimple.styled";
+export { SortableChartSettingOrderedItem };
+
export interface SortableItem {
key: string;
enabled: boolean;
diff --git a/frontend/src/metabase/visualizations/echarts/pie/model/index.ts b/frontend/src/metabase/visualizations/echarts/pie/model/index.ts
index 1de858ad9c3..1ac3eeec665 100644
--- a/frontend/src/metabase/visualizations/echarts/pie/model/index.ts
+++ b/frontend/src/metabase/visualizations/echarts/pie/model/index.ts
@@ -28,7 +28,7 @@ import {
} from "../constants";
import { getDimensionFormatter } from "../format";
import { getArrayFromMapValues } from "../util";
-import { getColorForRing } from "../util/colors";
+import { createHexToAccentNumberMap, getRingColorAlias } from "../util/colors";
import type {
PieChartModel,
@@ -317,6 +317,23 @@ export function getPieChartModel(
return currTotal + value;
}, 0);
+ const hexToAccentColorMap = createHexToAccentNumberMap();
+
+ function getColorForRing(
+ hexColor: string,
+ ring: "inner" | "middle" | "outer",
+ hasMultipleRings: boolean,
+ ) {
+ if (!hasMultipleRings) {
+ return hexColor;
+ }
+ const accentNumber = hexToAccentColorMap.get(hexColor);
+ if (accentNumber == null) {
+ return hexColor;
+ }
+ return renderingContext.getColor(getRingColorAlias(accentNumber, ring));
+ }
+
// Create sliceTree, fill out the innermost slice ring
const sliceTree: SliceTree = new Map();
const [sliceTreeNodes, others] = _.chain(pieRowsWithValues)
@@ -335,7 +352,6 @@ export function getPieChartModel(
color,
"inner",
colDescs.middleDimensionDesc != null,
- renderingContext,
),
visible,
children: new Map(),
@@ -403,7 +419,7 @@ export function getPieChartModel(
colDescs.middleDimensionDesc,
formatMiddleDimensionValue,
dimensionNode,
- getColorForRing(dimensionNode.color, "middle", true, renderingContext),
+ getColorForRing(dimensionNode.color, "middle", true),
index,
total,
colDescs.outerDimensionDesc == null ? showWarning : undefined,
@@ -423,7 +439,7 @@ export function getPieChartModel(
colDescs.outerDimensionDesc,
formatOuterDimensionValue,
middleDimensionNode,
- getColorForRing(dimensionNode.color, "outer", true, renderingContext),
+ getColorForRing(dimensionNode.color, "outer", true),
index,
total,
showWarning,
diff --git a/frontend/src/metabase/visualizations/echarts/pie/util/colors.ts b/frontend/src/metabase/visualizations/echarts/pie/util/colors.ts
index 314572bafb2..ebd017d5c7b 100644
--- a/frontend/src/metabase/visualizations/echarts/pie/util/colors.ts
+++ b/frontend/src/metabase/visualizations/echarts/pie/util/colors.ts
@@ -1,13 +1,9 @@
import { aliases, colors } from "metabase/lib/colors";
import { checkNumber } from "metabase/lib/types";
-import type {
- ColorGetter,
- RenderingContext,
-} from "metabase/visualizations/types";
const ACCENT_KEY_PREFIX = "accent";
-function getAccentNumberFromHex(hexColor: string) {
+export function createHexToAccentNumberMap() {
const hexToAccentNumber = new Map<string, number>();
for (const [key, hex] of Object.entries(colors)) {
@@ -35,24 +31,13 @@ function getAccentNumberFromHex(hexColor: string) {
hexToAccentNumber.set(hex, accentNumber);
}
- return hexToAccentNumber.get(hexColor);
+ return hexToAccentNumber;
}
-export function getColorForRing(
- hexColor: string,
+export function getRingColorAlias(
+ accentColorNumber: number,
ring: "inner" | "middle" | "outer",
- hasMultipleRings: boolean,
- renderingContext: RenderingContext,
) {
- if (!hasMultipleRings) {
- return hexColor;
- }
-
- const accentNumber = getAccentNumberFromHex(hexColor);
- if (accentNumber == null) {
- return hexColor;
- }
-
let suffix = "";
if (ring === "inner") {
suffix = "-dark";
@@ -60,24 +45,9 @@ export function getColorForRing(
suffix = "-light";
}
- return renderingContext.getColor(
- `${ACCENT_KEY_PREFIX}${accentNumber}${suffix}`,
- );
+ return `${ACCENT_KEY_PREFIX}${accentColorNumber}${suffix}`;
}
-export function getColorForPicker(
- hexColor: string | undefined,
- hasMultipleRings: boolean,
- getColor: ColorGetter,
-) {
- if (!hasMultipleRings || hexColor == null) {
- return hexColor;
- }
-
- const accentNumber = getAccentNumberFromHex(hexColor);
- if (accentNumber == null) {
- return hexColor;
- }
-
- return getColor(`${ACCENT_KEY_PREFIX}${accentNumber}-dark`);
+export function getPickerColorAlias(accentNumber: number) {
+ return `${ACCENT_KEY_PREFIX}${accentNumber}-dark`;
}
diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/chart-definition-legacy.js b/frontend/src/metabase/visualizations/visualizations/CartesianChart/chart-definition-legacy.js
index e09fb0c90bc..b20908ab6fb 100644
--- a/frontend/src/metabase/visualizations/visualizations/CartesianChart/chart-definition-legacy.js
+++ b/frontend/src/metabase/visualizations/visualizations/CartesianChart/chart-definition-legacy.js
@@ -77,6 +77,7 @@ function transformSingleSeries(s, series, seriesIndex) {
}
return breakoutValues.map(breakoutValue => ({
+ ...s,
card: {
...card,
// if multiseries include the card title as well as the breakout value
@@ -133,6 +134,7 @@ function transformSingleSeries(s, series, seriesIndex) {
.join(": ");
return {
+ ...s,
card: {
...card,
name: name,
diff --git a/frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx b/frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx
index 1e939f0070f..70384a345d9 100644
--- a/frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/PieChart/PieRowsPicker.tsx
@@ -1,10 +1,15 @@
+import { useMemo } from "react";
+
import { color } from "metabase/lib/colors";
import {
ChartSettingSeriesOrder,
- type SortableItem,
+ type SortableChartSettingOrderedItem,
} from "metabase/visualizations/components/settings/ChartSettingSeriesOrder";
import type { PieRow } from "metabase/visualizations/echarts/pie/model/types";
-import { getColorForPicker } from "metabase/visualizations/echarts/pie/util/colors";
+import {
+ createHexToAccentNumberMap,
+ getPickerColorAlias,
+} from "metabase/visualizations/echarts/pie/util/colors";
import type { ComputedVisualizationSettings } from "metabase/visualizations/types";
import type { RawSeries } from "metabase-types/api";
@@ -22,10 +27,27 @@ export function PieRowsPicker({
onShowWidget: (widget: any, ref: any) => void;
}) {
const pieRows = settings["pie.rows"];
+ const hasMultipleRings = numRings > 1;
+
+ const hexToAccentNumberMap = useMemo(() => createHexToAccentNumberMap(), []);
+
if (pieRows == null) {
return null;
}
+ const handleGetColorForPicker = ({
+ color: hexColor,
+ }: SortableChartSettingOrderedItem) => {
+ if (!hasMultipleRings || hexColor == null) {
+ return hexColor;
+ }
+ const accentNumber = hexToAccentNumberMap.get(hexColor);
+ if (accentNumber == null) {
+ return hexColor;
+ }
+ return color(getPickerColorAlias(accentNumber));
+ };
+
const onChangeSeriesColor = (sliceKey: string, color: string) =>
onChangeSettings({
"pie.rows": pieRows.map(row => {
@@ -36,7 +58,7 @@ export function PieRowsPicker({
}),
});
- const onSortEnd = (newPieRows: SortableItem[]) =>
+ const onSortEnd = (newPieRows: SortableChartSettingOrderedItem[]) =>
onChangeSettings({
"pie.sort_rows": false,
"pie.rows": newPieRows as PieRow[],
@@ -56,7 +78,7 @@ export function PieRowsPicker({
? { dark: true, main: false, light: false, harmony: false }
: undefined
}
- getItemColor={item => getColorForPicker(item.color, numRings > 1, color)}
+ getItemColor={handleGetColorForPicker}
/>
);
}
diff --git a/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts b/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts
index f25f3aa5393..ec4087f9f25 100644
--- a/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts
+++ b/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts
@@ -39,6 +39,14 @@ import type { RawSeries, Series } from "metabase-types/api";
import { DimensionsWidget } from "./DimensionsWidget";
import { SliceNameWidget } from "./SliceNameWidget";
+const pieRowsReadDeps = [
+ "pie.dimension",
+ "pie.metric",
+ "pie.colors",
+ "pie.sort_rows",
+ "pie.slice_threshold",
+];
+
export const PIE_CHART_DEFINITION: VisualizationDefinition = {
uiName: t`Pie`,
identifier: "pie",
@@ -113,18 +121,26 @@ export const PIE_CHART_DEFINITION: VisualizationDefinition = {
}),
"pie.rows": {
hidden: true,
- getValue: (rawSeries, settings) => {
- return getPieRows(rawSeries, settings, (value, options) =>
- String(formatValue(value, options)),
- );
- },
- readDependencies: [
- "pie.dimension",
- "pie.metric",
- "pie.colors",
- "pie.sort_rows",
- "pie.slice_threshold",
- ],
+ getValue: _.memoize(
+ (series, settings) => {
+ return getPieRows(series, settings, (value, options) =>
+ String(formatValue(value, options)),
+ );
+ },
+ ([{ json_query, started_at }], settings) =>
+ JSON.stringify({
+ json_query,
+ started_at,
+ settings: _.pick(
+ settings,
+ ...pieRowsReadDeps,
+ "column",
+ "pie.rows",
+ "pie.sort_rows_dimension",
+ ),
+ }),
+ ),
+ readDependencies: pieRowsReadDeps,
writeDependencies: ["pie.sort_rows_dimension"],
},
"pie.sort_rows_dimension": {
--
GitLab