diff --git a/e2e/test/scenarios/visualizations-charts/pie_chart.cy.spec.js b/e2e/test/scenarios/visualizations-charts/pie_chart.cy.spec.js
index 024f8b9e315939c5665eb79233252a92d990bac1..0429baa831fb2d089dbd3067eaeb6e16c7b03c73 100644
--- a/e2e/test/scenarios/visualizations-charts/pie_chart.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-charts/pie_chart.cy.spec.js
@@ -172,12 +172,25 @@ describe("scenarios > visualizations > pie chart", () => {
changeRowLimit(4, 2);
ensurePieChartRendered(["Gadget", "Doohickey"]);
+ // Ensure row settings should show only two rows
+ cy.findByTestId("viz-settings-button").click();
+ getDraggableElements().should("have.length", 2);
+ getDraggableElements().contains("Woooget").should("not.exist");
+ getDraggableElements().contains("Gizmo").should("not.exist");
+
+ cy.findByTestId("Gadget-settings-button").click();
+ cy.findByDisplayValue("Gadget").type("{selectall}Katget").realPress("Tab");
+ moveDnDKitElement(getDraggableElements().contains("Katget"), {
+ vertical: 30,
+ });
+
changeRowLimit(2, 4);
- ensurePieChartRendered(["Woooget", "Gadget", "Gizmo", "Doohickey"]);
+ ensurePieChartRendered(["Doohickey", "Katget", "Gizmo", "Woooget"]);
chartPathWithFillColor("#509EE3").should("be.visible");
cy.findByTestId("chart-legend").within(() => {
- cy.get("li").eq(2).contains("Woooget");
+ cy.get("li").eq(1).contains("Katget");
+ cy.get("li").eq(3).contains("Woooget");
});
});
});
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingSeriesOrder.tsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingSeriesOrder.tsx
index 1e860a4660ee9d8e5424cee9684db2bf6c725ec0..9e8e0357d4296a77bf7b75c5f7a002413d5063a3 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingSeriesOrder.tsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingSeriesOrder.tsx
@@ -21,11 +21,14 @@ interface SortableItem {
enabled: boolean;
name: string;
color?: string;
+ hidden?: boolean;
}
interface ChartSettingSeriesOrderProps {
onChange: (rows: SortableItem[]) => void;
value: SortableItem[];
+ addButtonLabel: string;
+ searchPickerPlaceholder: string;
onShowWidget: (
widget: { props: { seriesKey: string } },
ref: HTMLElement | undefined,
@@ -39,6 +42,8 @@ interface ChartSettingSeriesOrderProps {
export const ChartSettingSeriesOrder = ({
onChange,
value: orderedItems,
+ addButtonLabel = t`Add another series`,
+ searchPickerPlaceholder = t`Select a series`,
onShowWidget,
hasEditSettings = true,
onChangeSeriesColor,
@@ -47,7 +52,11 @@ export const ChartSettingSeriesOrder = ({
const [isSeriesPickerVisible, setSeriesPickerVisible] = useState(false);
const [visibleItems, hiddenItems] = useMemo(
- () => _.partition(orderedItems, item => item.enabled),
+ () =>
+ _.partition(
+ orderedItems.filter(item => !item.hidden),
+ item => item.enabled,
+ ),
[orderedItems],
);
@@ -133,13 +142,15 @@ export const ChartSettingSeriesOrder = ({
<Button
variant="subtle"
onClick={() => setSeriesPickerVisible(true)}
- >{t`Add another series`}</Button>
+ >
+ {addButtonLabel}
+ </Button>
)}
{isSeriesPickerVisible && (
<Select
initiallyOpened
searchable
- placeholder={t`Select a series`}
+ placeholder={searchPickerPlaceholder}
data={hiddenItems.map(item => ({
value: item.key,
label: getItemTitle(item),
diff --git a/frontend/src/metabase/visualizations/echarts/pie/model/index.ts b/frontend/src/metabase/visualizations/echarts/pie/model/index.ts
index f4319e05feb0a196ee11e67c39510cadc65157cd..2a852bf71ca15de4a87b47dc60700a00a5509b8c 100644
--- a/frontend/src/metabase/visualizations/echarts/pie/model/index.ts
+++ b/frontend/src/metabase/visualizations/echarts/pie/model/index.ts
@@ -25,7 +25,7 @@ import type {
PieSliceData,
} from "./types";
-function getColDescs(
+export function getPieColumns(
rawSeries: RawSeries,
settings: ComputedVisualizationSettings,
): PieColumnDescriptors {
@@ -71,7 +71,7 @@ export function getPieChartModel(
data: { rows: dataRows },
},
] = rawSeries;
- const colDescs = getColDescs(rawSeries, settings);
+ const colDescs = getPieColumns(rawSeries, settings);
const rowIndiciesByKey = new Map<string | number, number>();
dataRows.forEach((row, index) => {
diff --git a/frontend/src/metabase/visualizations/shared/settings/pie.ts b/frontend/src/metabase/visualizations/shared/settings/pie.ts
index e014200defdb6099ce09dd7b542896bf30ad0d59..1e7dcb36286103f9781dfd34905a8b6c3f5af036 100644
--- a/frontend/src/metabase/visualizations/shared/settings/pie.ts
+++ b/frontend/src/metabase/visualizations/shared/settings/pie.ts
@@ -5,6 +5,7 @@ import { getColorsForValues } from "metabase/lib/colors/charts";
import { NULL_DISPLAY_VALUE } from "metabase/lib/constants";
import { checkNotNull, checkNumber, isNumber } from "metabase/lib/types";
import { SLICE_THRESHOLD } from "metabase/visualizations/echarts/pie/constants";
+import { getPieColumns } from "metabase/visualizations/echarts/pie/model";
import type { PieRow } from "metabase/visualizations/echarts/pie/model/types";
import type { ShowWarning } from "metabase/visualizations/echarts/types";
import { getNumberOr } from "metabase/visualizations/lib/settings/row-values";
@@ -148,23 +149,26 @@ export function getPieRows(
) {
const [
{
- data: { cols, rows: dataRows },
+ data: { rows: dataRows },
},
] = rawSeries;
+ if (!settings["pie.metric"] || !settings["pie.dimension"]) {
+ return [];
+ }
+
+ const hasSortDimensionChanged =
+ settings["pie.sort_rows_dimension"] != null &&
+ settings["pie.sort_rows_dimension"] !== settings["pie.dimension"];
+
+ const { metricDesc, dimensionDesc } = getPieColumns(rawSeries, settings);
+
const getColumnSettings = settings["column"];
if (!getColumnSettings) {
throw Error("`settings.column` is undefined");
}
- const dimensionCol = cols.find(c => c.name === settings["pie.dimension"]);
- if (dimensionCol == null) {
- throw Error(
- `Could not find column based on "pie.dimension setting with value ${settings["pie.dimension"]}`,
- );
- }
-
- const dimensionColSettings = getColumnSettings(dimensionCol);
+ const dimensionColSettings = getColumnSettings(dimensionDesc.column);
const formatDimensionValue = (value: RowValue) => {
if (value == null) {
@@ -174,13 +178,6 @@ export function getPieRows(
return formatter(value, dimensionColSettings);
};
- const dimensionIndex = cols.findIndex(
- col => col.name === settings["pie.dimension"],
- );
- const metricIndex = cols.findIndex(
- col => col.name === settings["pie.metric"],
- );
-
let colors = getColors(rawSeries, settings);
// `pie.colors` is a legacy setting used by old questions for their
// colors. We'll still read it to preserve those color selections, but
@@ -189,36 +186,41 @@ export function getPieRows(
if (settings["pie.colors"] != null) {
colors = { ...colors, ...settings["pie.colors"] };
}
- const savedPieRows = settings["pie.rows"] ?? [];
-
- const savedPieKeys = savedPieRows.map(pieRow => pieRow.key);
-
- const keyToSavedPieRow = new Map<PieRow["key"], PieRow>();
- savedPieRows.map(pieRow => keyToSavedPieRow.set(pieRow.key, pieRow));
const currentDataRows = getAggregatedRows(
dataRows,
- dimensionIndex,
- metricIndex,
+ dimensionDesc.index,
+ metricDesc.index,
+ );
+ const keyToCurrentDataRow = new Map<PieRow["key"], RowValues>(
+ currentDataRows.map(dataRow => [
+ getKeyFromDimensionValue(dataRow[dimensionDesc.index]),
+ dataRow,
+ ]),
);
+ const currentDataKeys = Array.from(keyToCurrentDataRow.keys());
- const keyToCurrentDataRow = new Map<PieRow["key"], RowValues>();
- const currentDataKeys = currentDataRows.map(dataRow => {
- const key = getKeyFromDimensionValue(dataRow[dimensionIndex]);
- keyToCurrentDataRow.set(key, dataRow);
+ const savedPieRows = hasSortDimensionChanged
+ ? []
+ : (settings["pie.rows"] ?? []);
- return key;
- });
+ const savedPieKeys = savedPieRows.map(pieRow => pieRow.key);
+ const keyToSavedPieRow = new Map<PieRow["key"], PieRow>(
+ savedPieRows.map(pieRow => [pieRow.key, pieRow]),
+ );
const removed = _.difference(savedPieKeys, currentDataKeys);
let newPieRows: PieRow[] = [];
// Case 1: Auto sorted, sort existing and new rows together
if (settings["pie.sort_rows"]) {
- const sortedCurrentDataRows = getSortedRows(currentDataRows, metricIndex);
+ const sortedCurrentDataRows = getSortedRows(
+ currentDataRows,
+ metricDesc.index,
+ );
newPieRows = sortedCurrentDataRows.map(dataRow => {
- const dimensionValue = dataRow[dimensionIndex];
+ const dimensionValue = dataRow[dimensionDesc.index];
const key = getKeyFromDimensionValue(dimensionValue);
// Historically we have used the dimension value in the `pie.colors`
// setting instead of the key computed above. For compatibility with
@@ -278,11 +280,11 @@ export function getPieRows(
return dataRow;
});
- const sortedAddedRows = getSortedRows(addedRows, metricIndex);
+ const sortedAddedRows = getSortedRows(addedRows, metricDesc.index);
newPieRows.push(
...sortedAddedRows.map(addedDataRow => {
- const dimensionValue = addedDataRow[dimensionIndex];
+ const dimensionValue = addedDataRow[dimensionDesc.index];
const color = Color(colors[String(dimensionValue)]).hex();
const key = getKeyFromDimensionValue(dimensionValue);
@@ -323,7 +325,7 @@ export function getPieRows(
return (
currTotal +
checkNumber(
- checkNotNull(keyToCurrentDataRow.get(pieRow.key))[metricIndex],
+ checkNotNull(keyToCurrentDataRow.get(pieRow.key))[metricDesc.index],
)
);
}, 0);
@@ -335,7 +337,7 @@ export function getPieRows(
}
const metricValue = checkNumber(
- checkNotNull(keyToCurrentDataRow.get(pieRow.key))[metricIndex],
+ checkNotNull(keyToCurrentDataRow.get(pieRow.key))[metricDesc.index],
);
const normalizedPercentage = metricValue / total;
@@ -359,4 +361,10 @@ export function getPieRows(
return newPieRows;
}
+export const getPieSortRowsDimensionSetting = (
+ settings: ComputedVisualizationSettings,
+) => {
+ return settings["pie.dimension"];
+};
+
export const getDefaultSortRows = () => true;
diff --git a/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts b/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts
index d9ae8e4c7da3c09b176af1abc4b03270618322a0..4af837092b225baa546f6b8ed81ecc6907e2ed9e 100644
--- a/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts
+++ b/frontend/src/metabase/visualizations/visualizations/PieChart/chart-definition.ts
@@ -20,6 +20,7 @@ import {
getDefaultSliceThreshold,
getDefaultSortRows,
getPieRows,
+ getPieSortRowsDimensionSetting,
} from "metabase/visualizations/shared/settings/pie";
import { SERIES_SETTING_KEY } from "metabase/visualizations/shared/settings/series";
import { getDefaultShowTotal } from "metabase/visualizations/shared/settings/waterfall";
@@ -107,6 +108,8 @@ export const PIE_CHART_DEFINITION: VisualizationDefinition = {
onChangeSettings,
) => {
return {
+ addButtonLabel: t`Add another row`,
+ searchPickerPlaceholder: t`Select a row`,
onChangeSeriesColor: (sliceKey: string, color: string) => {
const pieRows = vizSettings["pie.rows"];
if (pieRows == null) {
@@ -126,6 +129,8 @@ export const PIE_CHART_DEFINITION: VisualizationDefinition = {
onChangeSettings({
"pie.sort_rows": false,
"pie.rows": newPieRows,
+ "pie.sort_rows_dimension":
+ getPieSortRowsDimensionSetting(vizSettings),
}),
};
},
@@ -136,6 +141,13 @@ export const PIE_CHART_DEFINITION: VisualizationDefinition = {
"pie.sort_rows",
"pie.slice_threshold",
],
+ writeDependencies: ["pie.sort_rows_dimension"],
+ },
+ "pie.sort_rows_dimension": {
+ getValue: (_series, settings) => getPieSortRowsDimensionSetting(settings),
+ // This read dependency is set so that "pie.sort_rows" is computed *before* this value, ensuring that
+ // that it uses the stored value if one exists. This is needed to check if the dimension has actually changed
+ readDependencies: ["pie.sort_rows", "pie.dimension"],
},
"pie.sort_rows": {
hidden: true,