sunburst improvements (#48316)

* sunburst improvements

* fix dimension picker styles

* simplify styles

* update e2e test

* add loki specs

* add missing newlines

e2e/test/scenarios/visualizations-charts/pie_chart.cy.spec.js

@@ -392,36 +392,43 @@ describe("scenarios > visualizations > pie chart", () => {
           color: "#51528D",
           name: "Saturday",
           value: "2,747",
+          secondaryValue: "14.64 %",
         },
         {
           color: "#ED8535",
           name: "Thursday",
           value: "2,698",
+          secondaryValue: "14.38 %",
         },
         {
           color: "#E75454",
           name: "Tuesday",
           value: "2,695",
+          secondaryValue: "14.37 %",
         },
         {
           color: "#689636",
           name: "Sunday",
           value: "2,671",
+          secondaryValue: "14.24 %",
         },
         {
           color: "#8A5EB0",
           name: "Monday",
           value: "2,664",
+          secondaryValue: "14.20 %",
         },
         {
           color: "#69C8C8",
           name: "Friday",
           value: "2,662",
+          secondaryValue: "14.19 %",
         },
         {
           color: "#F7C41F",
           name: "Wednesday",
           value: "2,623",
+          secondaryValue: "13.98 %",
         },
       ],
     });
@@ -464,18 +471,22 @@ describe("scenarios > visualizations > pie chart", () => {
         {
           name: "Doohickey",
           value: "606",
+          secondaryValue: "22.06 %",
         },
         {
           name: "Gadget",
           value: "740",
+          secondaryValue: "26.94 %",
         },
         {
           name: "Gizmo",
           value: "640",
+          secondaryValue: "23.30 %",
         },
         {
           name: "Widget",
           value: "761",
+          secondaryValue: "27.70 %",
         },
       ],
     });

frontend/src/metabase/components/Triggerable/Triggerable.jsx

@@ -157,7 +157,7 @@ const Triggerable = ComposedComponent =>
               !isOpen && triggerClassesClose,
               CS.noDecoration,
               {
-                [CS.cursorDefault]: this.props.disabled,
+                [CS.cursorInherit]: this.props.disabled,
               },
             )}
             aria-disabled={this.props.disabled}

frontend/src/metabase/css/core/cursor.module.css

@@ -9,3 +9,7 @@
 .cursorDefault {
   cursor: default;
 }
+
+.cursorInherit {
+  cursor: inherit;
+}

frontend/src/metabase/static-viz/components/PieChart/PieChart.stories.tsx

@@ -424,6 +424,18 @@ ThreeRingsNoLabels.args = {
   renderingContext,
 };
 
+export const ThreeRingsPercentagesAndLabels = Template.bind({});
+ThreeRingsPercentagesAndLabels.args = {
+  rawSeries: data.threeRingsPercentagesAndLabels as any,
+  renderingContext,
+};
+
+export const ThreeRingsPercentagesOnChart = Template.bind({});
+ThreeRingsPercentagesOnChart.args = {
+  rawSeries: data.threeRingsPercentagesOnChart as any,
+  renderingContext,
+};
+
 export const ThreeRingsOtherSlices = Template.bind({});
 ThreeRingsOtherSlices.args = {
   rawSeries: data.threeRingsOtherSlices as any,

frontend/src/metabase/static-viz/components/PieChart/stories-data/index.ts

@@ -39,6 +39,8 @@ import singleDimension from "./single-dimension.json";
 import smallMinimumSlicePercentage from "./small-min-slice-percentage.json";
 import threeRingsNoLabels from "./three-rings-no-labels.json";
 import threeRingsOtherSlices from "./three-rings-other-slices.json";
+import threeRingsPercentagesAndLabels from "./three-rings-percentages-and-labels.json";
+import threeRingsPercentagesOnChart from "./three-rings-percentages-on-chart.json";
 import threeRings from "./three-rings.json";
 import tinySlicesDisappear43766 from "./tiny-slices-disappear-43766.json";
 import truncatedTotal from "./truncated-total.json";
@@ -92,6 +94,8 @@ export const data = {
   threeRings,
   threeRingsNoLabels,
   threeRingsOtherSlices,
+  threeRingsPercentagesAndLabels,
+  threeRingsPercentagesOnChart,
   labelsWithPercent,
   labelsOnChart,
 };

frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx

@@ -8,8 +8,8 @@ import { getColumnKey } from "metabase-lib/v1/queries/utils/column-key";
 import {
   ChartSettingFieldPickerRoot,
   FieldPickerColorPicker,
+  GrabberHandle,
   SettingsButton,
-  SettingsIcon,
 } from "./ChartSettingFieldPicker.styled";
 import ChartSettingSelect from "./ChartSettingSelect";
 
@@ -51,9 +51,10 @@ const ChartSettingFieldPicker = ({
     <ChartSettingFieldPickerRoot
       className={className}
       disabled={options.length === 1 && options[0].value === value}
+      showDragHandle={showDragHandle}
       data-testid="chartsettings-field-picker"
     >
-      {showDragHandle && <SettingsIcon name="grabber" noPointer noMargin />}
+      {showDragHandle && <GrabberHandle name="grabber" noPointer noMargin />}
       {showColorPicker && seriesKey && (
         <FieldPickerColorPicker
           pillSize="small"

frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.styled.tsx

@@ -7,8 +7,28 @@ import { Icon } from "metabase/ui";
 
 import { ChartSettingColorPicker } from "./ChartSettingColorPicker";
 
+export const SettingsButton = styled(Button)`
+  margin-left: 0.75rem;
+  padding: 0;
+
+  &:hover {
+    background-color: unset;
+  }
+`;
+
+export const GrabberHandle = styled(Icon)`
+  color: var(--mb-color-text-medium);
+  cursor: inherit;
+`;
+
+export const FieldPickerColorPicker = styled(ChartSettingColorPicker)`
+  margin-bottom: 0;
+  margin-left: 0.25rem;
+`;
+
 interface ChartSettingFieldPickerRootProps {
   disabled: boolean;
+  showDragHandle: boolean;
 }
 
 export const ChartSettingFieldPickerRoot = styled.div<ChartSettingFieldPickerRootProps>`
@@ -19,6 +39,7 @@ export const ChartSettingFieldPickerRoot = styled.div<ChartSettingFieldPickerRoo
   padding-right: 1rem;
   padding-left: 0.5rem;
   background: var(--mb-color-bg-white);
+  cursor: ${props => (props.showDragHandle ? "grab" : "default")};
 
   ${Triggerable.Trigger} {
     flex: 1;
@@ -52,33 +73,10 @@ export const ChartSettingFieldPickerRoot = styled.div<ChartSettingFieldPickerRoo
   ${SelectButton.Root}:disabled {
     background-color: var(--mb-color-bg-white);
   }
-`;
-
-interface SettingsIconProps {
-  noPointer?: boolean;
-  noMargin?: boolean;
-}
-
-export const SettingsButton = styled(Button)<SettingsIconProps>`
-  margin-left: ${props => (props.noMargin ? "0" : "0.75rem")};
-  padding: 0;
-
-  &:hover {
-    background-color: unset;
-  }
-`;
-
-export const SettingsIcon = styled(Icon)<SettingsIconProps>`
-  margin-left: ${props => (props.noMargin ? "0" : "0.75rem")};
-  color: var(--mb-color-text-medium);
-  cursor: ${props => (props.noPointer ? "inherit" : "pointer")};
 
   &:hover {
-    color: var(--mb-color-brand);
+    ${GrabberHandle} {
+      color: var(--mb-color-brand);
+    }
   }
 `;
-
-export const FieldPickerColorPicker = styled(ChartSettingColorPicker)`
-  margin-bottom: 0;
-  margin-left: 0.25rem;
-`;

frontend/src/metabase/visualizations/echarts/pie/model/index.ts

@@ -128,14 +128,14 @@ function createOrUpdateNode(
       name: formatter(dimensionValue),
       value: metricValue,
       displayValue: metricValue,
-      normalizedPercentage: metricValue / total,
+      normalizedPercentage: 0, // placeholder
       color,
       visible: true,
       column: colDesc.column,
       rowIndex,
-      isOther: false,
+      isOther: false, // placeholder
       children: new Map(),
-      startAngle: 0,
+      startAngle: 0, // placeholders
       endAngle: 0,
     };
     parentNode.children.set(dimensionKey, dimensionNode);
@@ -144,7 +144,6 @@ function createOrUpdateNode(
     // to it.
     dimensionNode.value += metricValue;
     dimensionNode.displayValue += metricValue;
-    dimensionNode.normalizedPercentage = dimensionNode.value / total;
 
     showWarning?.(unaggregatedDataWarningPie(colDesc.column).text);
   }
@@ -152,16 +151,20 @@ function createOrUpdateNode(
   return dimensionNode;
 }
 
-function markOtherNodes(
+function calculatePercentageAndIsOther(
   node: SliceTreeNode,
   parent: SliceTreeNode,
   settings: ComputedVisualizationSettings,
 ) {
+  const relativePercentage = node.displayValue / parent.displayValue;
+
+  node.normalizedPercentage = relativePercentage;
   node.isOther =
-    node.displayValue / parent.displayValue <
-    (settings["pie.slice_threshold"] ?? 0) / 100;
+    relativePercentage < (settings["pie.slice_threshold"] ?? 0) / 100;
 
-  node.children.forEach(child => markOtherNodes(child, node, settings));
+  node.children.forEach(child =>
+    calculatePercentageAndIsOther(child, node, settings),
+  );
 }
 
 function aggregateSlices(
@@ -434,7 +437,9 @@ export function getPieChartModel(
   }
 
   sliceTree.forEach(node =>
-    node.children.forEach(child => markOtherNodes(child, node, settings)),
+    node.children.forEach(child =>
+      calculatePercentageAndIsOther(child, node, settings),
+    ),
   );
 
   // Only add "other" slice if there are slices below threshold with non-zero total

frontend/src/metabase/visualizations/visualizations/PieChart/DimensionsWidget.tsx

@@ -16,6 +16,7 @@ import { useState } from "react";
 import { t } from "ttag";
 
 import { Sortable } from "metabase/core/components/Sortable";
+import GrabberS from "metabase/css/components/grabber.module.css";
 import { Button, Text } from "metabase/ui";
 import ChartSettingFieldPicker from "metabase/visualizations/components/settings/ChartSettingFieldPicker";
 import { getOptionFromColumn } from "metabase/visualizations/lib/settings/utils";
@@ -128,12 +129,16 @@ export function DimensionsWidget({
   const [draggedDimensionIndex, setDraggedDimensionIndex] = useState<number>();
 
   const onDragStart = (event: DragStartEvent) => {
+    document.body.classList.add(GrabberS.grabbing);
+
     setDraggedDimensionIndex(
       dimensions.findIndex(d => d === String(event.active.id)),
     );
   };
 
   const onDragEnd = (event: DragEndEvent) => {
+    document.body.classList.remove(GrabberS.grabbing);
+
     setDraggedDimensionIndex(undefined);
 
     const over = event.over;

frontend/src/metabase/visualizations/visualizations/PieChart/use-chart-events.ts

@@ -9,10 +9,7 @@ import type {
   EChartsTooltipModel,
   EChartsTooltipRow,
 } from "metabase/visualizations/components/ChartTooltip/EChartsTooltip";
-import {
-  getPercent,
-  getTotalValue,
-} from "metabase/visualizations/components/ChartTooltip/StackedDataTooltip/utils";
+import { getTotalValue } from "metabase/visualizations/components/ChartTooltip/StackedDataTooltip/utils";
 import type { PieChartFormatters } from "metabase/visualizations/echarts/pie/format";
 import type { PieChartModel } from "metabase/visualizations/echarts/pie/model/types";
 import type { EChartsSunburstSeriesMouseEvent } from "metabase/visualizations/echarts/pie/types";
@@ -57,6 +54,7 @@ export const getTooltipModel = (
       color: nodes.length === 1 ? slice.color : undefined,
       formatter: formatters.formatMetric,
       key: slice.key,
+      normalizedPercentage: slice.normalizedPercentage,
     }));
   const rowsTotal = getTotalValue(rows);
 
@@ -70,7 +68,7 @@ export const getTooltipModel = (
       name: row.name,
       values: [
         row.formatter(row.value),
-        formatPercent(getPercent(chartModel.total, row.value) ?? 0),
+        formatPercent(row.normalizedPercentage),
       ],
     };
   });
@@ -88,10 +86,7 @@ export const getTooltipModel = (
       rows.length > 1
         ? {
             name: t`Total`,
-            values: [
-              formatters.formatMetric(rowsTotal),
-              formatPercent(getPercent(chartModel.total, rowsTotal) ?? 0),
-            ],
+            values: [formatters.formatMetric(rowsTotal), formatPercent(1)],
           }
         : undefined,
   };