PieChart: Refactoring & adding unsubscribe (#33432)

* Tweaks to piechart and theme

* Adds unsubscribe to events and move out to separate hook

* reverted constrast change

* Minor refactor after review feedback

* chain the subs

packages/grafana-ui/src/components/PieChart/PieChart.tsx

@@ -1,4 +1,4 @@
-import React, { FC, ReactNode, useState } from 'react';
+import React, { FC, useEffect, useState } from 'react';
 import {
   DataHoverClearEvent,
   DataHoverEvent,
@@ -6,9 +6,9 @@ import {
   FieldDisplay,
   formattedValueToString,
   getFieldDisplayValues,
-  GrafanaTheme,
+  GrafanaThemeV2,
 } from '@grafana/data';
-import { useStyles, useTheme } from '../../themes/ThemeContext';
+import { useStyles2, useTheme2 } from '../../themes/ThemeContext';
 import tinycolor from 'tinycolor2';
 import Pie, { PieArcDatum, ProvidedProps } from '@visx/shape/lib/shapes/Pie';
 import { Group } from '@visx/group';
@@ -33,6 +33,7 @@ import {
 import { getTooltipContainerStyles } from '../../themes/mixins';
 import { SeriesTable, SeriesTableRowProps, VizTooltipOptions } from '../VizTooltip';
 import { usePanelContext } from '../PanelChrome';
+import { Subscription } from 'rxjs';
 
 const defaultLegendOptions: PieChartLegendOptions = {
   displayMode: LegendDisplayMode.List,
@@ -44,45 +45,56 @@ const defaultLegendOptions: PieChartLegendOptions = {
 /**
  * @beta
  */
-export const PieChart: FC<PieChartProps> = ({
+export function PieChart(props: PieChartProps) {
+  const {
     data,
     timeZone,
     reduceOptions,
     fieldConfig,
     replaceVariables,
-  legendOptions = defaultLegendOptions,
     tooltipOptions,
     onSeriesColorChange,
     width,
     height,
     ...restProps
-}) => {
+  } = props;
-  const theme = useTheme();
-  const [highlightedTitle, setHighlightedTitle] = useState<string>();
-  const { eventBus } = usePanelContext();
 
-  if (eventBus) {
+  const theme = useTheme2();
-    const setHighlightedSlice = (event: DataHoverEvent) => {
+  const highlightedTitle = useSliceHighlightState();
-      if (eventBus.isOwnEvent(event)) {
+  const fieldDisplayValues = getFieldDisplayValues({
-        setHighlightedTitle(event.payload.dataId);
+    fieldConfig,
-      }
+    reduceOptions,
-    };
+    data,
+    theme: theme.v1,
+    replaceVariables,
+    timeZone,
+  });
 
-    const resetHighlightedSlice = (event: DataHoverClearEvent) => {
+  return (
-      if (eventBus.isOwnEvent(event)) {
+    <VizLayout width={width} height={height} legend={getLegend(props, fieldDisplayValues)}>
-        setHighlightedTitle(undefined);
+      {(vizWidth: number, vizHeight: number) => {
-      }
+        return (
-    };
+          <PieChartSvg
-
+            width={vizWidth}
-    eventBus.subscribe(DataHoverEvent, setHighlightedSlice);
+            height={vizHeight}
-    eventBus.subscribe(DataHoverClearEvent, resetHighlightedSlice);
+            highlightedTitle={highlightedTitle}
+            fieldDisplayValues={fieldDisplayValues}
+            tooltipOptions={tooltipOptions}
+            {...restProps}
+          />
+        );
+      }}
+    </VizLayout>
+  );
 }
 
-  const getLegend = (fields: FieldDisplay[], legendOptions: PieChartLegendOptions) => {
+function getLegend(props: PieChartProps, displayValues: FieldDisplay[]) {
+  const { legendOptions = defaultLegendOptions } = props;
+
   if (legendOptions.displayMode === LegendDisplayMode.Hidden) {
     return undefined;
   }
-    const values = fields.map((v) => v.display);
+  const values = displayValues.map((v) => v.display);
   const total = values.reduce((acc, item) => item.numeric + acc, 0);
 
   const legendItems = values.map<VizLegendItem>((value, idx) => {
@@ -119,53 +131,58 @@ export const PieChart: FC<PieChartProps> = ({
   return (
     <VizLegend
       items={legendItems}
-        onSeriesColorChange={onSeriesColorChange}
+      onSeriesColorChange={props.onSeriesColorChange}
       placement={legendOptions.placement}
       displayMode={legendOptions.displayMode}
     />
   );
+}
+
+function useSliceHighlightState() {
+  const [highlightedTitle, setHighlightedTitle] = useState<string>();
+  const { eventBus } = usePanelContext();
+
+  useEffect(() => {
+    if (!eventBus) {
+      return;
+    }
+
+    const setHighlightedSlice = (event: DataHoverEvent) => {
+      if (eventBus.isOwnEvent(event)) {
+        setHighlightedTitle(event.payload.dataId);
+      }
     };
 
-  const fieldDisplayValues = getFieldDisplayValues({
+    const resetHighlightedSlice = (event: DataHoverClearEvent) => {
-    fieldConfig,
+      if (eventBus.isOwnEvent(event)) {
-    reduceOptions,
+        setHighlightedTitle(undefined);
-    data,
+      }
-    theme,
-    replaceVariables,
-    timeZone,
-  });
-
-  return (
-    <VizLayout width={width} height={height} legend={getLegend(fieldDisplayValues, legendOptions)}>
-      {(vizWidth: number, vizHeight: number) => {
-        return (
-          <PieChartSvg
-            width={vizWidth}
-            height={vizHeight}
-            highlightedTitle={highlightedTitle}
-            fieldDisplayValues={fieldDisplayValues}
-            tooltipOptions={tooltipOptions}
-            {...restProps}
-          />
-        );
-      }}
-    </VizLayout>
-  );
     };
 
+    const subs = new Subscription()
+      .add(eventBus.subscribe(DataHoverEvent, setHighlightedSlice))
+      .add(eventBus.subscribe(DataHoverClearEvent, resetHighlightedSlice));
+
+    return () => {
+      subs.unsubscribe();
+    };
+  }, [setHighlightedTitle, eventBus]);
+
+  return highlightedTitle;
+}
+
 export const PieChartSvg: FC<PieChartSvgProps> = ({
   fieldDisplayValues,
   pieType,
   width,
   height,
   highlightedTitle,
-  useGradients = true,
   displayLabels = [],
   tooltipOptions,
 }) => {
-  const theme = useTheme();
+  const theme = useTheme2();
   const componentInstanceId = useComponentInstanceId('PieChart');
-  const styles = useStyles(getStyles);
+  const styles = useStyles2(getStyles);
   const tooltip = useTooltip<SeriesTableRowProps[]>();
   const { containerRef, TooltipInPortal } = useTooltipInPortal({
     detectBounds: true,
@@ -218,20 +235,11 @@ export const PieChartSvg: FC<PieChartSvgProps> = ({
             cornerRadius={3}
             padAngle={0.005}
           >
-            {(pie) => {
+            {(pie) => (
-              return pie.arcs.map((arc) => {
+              <>
-                const color = arc.data.display.color ?? FALLBACK_COLOR;
+                {pie.arcs.map((arc) => {
+                  let color = arc.data.display.color ?? FALLBACK_COLOR;
                   const highlighted = highlightedTitle === arc.data.display.title;
-                const label = showLabel ? (
-                  <PieLabel
-                    arc={arc}
-                    outerRadius={layout.outerRadius}
-                    innerRadius={layout.innerRadius}
-                    displayLabels={displayLabels}
-                    total={total}
-                    color={theme.colors.text}
-                  />
-                ) : undefined;
                   if (arc.data.hasLinks && arc.data.getLinks) {
                     return (
                       <DataLinksContextMenu config={arc.data.field} key={arc.index} links={arc.data.getLinks}>
@@ -244,9 +252,7 @@ export const PieChartSvg: FC<PieChartSvgProps> = ({
                             fill={getGradientColor(color)}
                             openMenu={api.openMenu}
                             tooltipOptions={tooltipOptions}
-                        >
+                          />
-                          {label}
-                        </PieSlice>
                         )}
                       </DataLinksContextMenu>
                     );
@@ -260,13 +266,24 @@ export const PieChartSvg: FC<PieChartSvgProps> = ({
                         pie={pie}
                         fill={getGradientColor(color)}
                         tooltipOptions={tooltipOptions}
-                    >
+                      />
-                      {label}
-                    </PieSlice>
                     );
                   }
-              });
+                })}
-            }}
+                {showLabel &&
+                  pie.arcs.map((arc) => (
+                    <PieLabel
+                      arc={arc}
+                      key={arc.index}
+                      outerRadius={layout.outerRadius}
+                      innerRadius={layout.innerRadius}
+                      displayLabels={displayLabels}
+                      total={total}
+                      color={theme.colors.text.primary}
+                    />
+                  ))}
+              </>
+            )}
           </Pie>
         </Group>
       </svg>
@@ -286,8 +303,7 @@ export const PieChartSvg: FC<PieChartSvgProps> = ({
   );
 };
 
-const PieSlice: FC<{
+interface SliceProps {
-  children: ReactNode;
   arc: PieArcDatum<FieldDisplay>;
   pie: ProvidedProps<FieldDisplay>;
   highlighted?: boolean;
@@ -295,9 +311,11 @@ const PieSlice: FC<{
   tooltip: UseTooltipParams<SeriesTableRowProps[]>;
   tooltipOptions: VizTooltipOptions;
   openMenu?: (event: React.MouseEvent<SVGElement>) => void;
-}> = ({ arc, children, pie, highlighted, openMenu, fill, tooltip, tooltipOptions }) => {
+}
-  const theme = useTheme();
+
-  const styles = useStyles(getStyles);
+function PieSlice({ arc, pie, highlighted, openMenu, fill, tooltip, tooltipOptions }: SliceProps) {
+  const theme = useTheme2();
+  const styles = useStyles2(getStyles);
 
   const onMouseMoveOverArc = (event: any) => {
     const coords = localPoint(event.target.ownerSVGElement, event);
@@ -316,20 +334,21 @@ const PieSlice: FC<{
       onMouseOut={tooltip.hideTooltip}
       onClick={openMenu}
     >
-      <path d={pie.path({ ...arc })!} fill={fill} stroke={theme.colors.panelBg} strokeWidth={1} />
+      <path d={pie.path({ ...arc })!} fill={fill} stroke={theme.colors.background.primary} strokeWidth={1} />
-      {children}
     </g>
   );
-};
+}
 
-const PieLabel: FC<{
+interface LabelProps {
   arc: PieArcDatum<FieldDisplay>;
   outerRadius: number;
   innerRadius: number;
   displayLabels: PieChartLabels[];
   total: number;
   color: string;
-}> = ({ arc, outerRadius, innerRadius, displayLabels, total, color }) => {
+}
+
+function PieLabel({ arc, outerRadius, innerRadius, displayLabels, total, color }: LabelProps) {
   const labelRadius = innerRadius === 0 ? outerRadius / 6 : innerRadius;
   const [labelX, labelY] = getLabelPos(arc, outerRadius, labelRadius);
   const hasSpaceForLabel = arc.endAngle - arc.startAngle >= 0.3;
@@ -371,7 +390,7 @@ const PieLabel: FC<{
       </text>
     </g>
   );
-};
+}
 
 function getTooltipData(
   pie: ProvidedProps<FieldDisplay>,
@@ -403,14 +422,14 @@ function getLabelPos(arc: PieArcDatum<FieldDisplay>, outerRadius: number, innerR
   return [Math.cos(a) * r, Math.sin(a) * r];
 }
 
-function getGradientColorFrom(color: string, theme: GrafanaTheme) {
+function getGradientColorFrom(color: string, theme: GrafanaThemeV2) {
   return tinycolor(color)
     .darken(20 * (theme.isDark ? 1 : -0.7))
     .spin(8)
     .toRgbString();
 }
 
-function getGradientColorTo(color: string, theme: GrafanaTheme) {
+function getGradientColorTo(color: string, theme: GrafanaThemeV2) {
   return tinycolor(color)
     .darken(10 * (theme.isDark ? 1 : -0.7))
     .spin(-8)
@@ -442,7 +461,7 @@ function getPieLayout(height: number, width: number, pieType: PieChartType, marg
   };
 }
 
-const getStyles = (theme: GrafanaTheme) => {
+const getStyles = (theme: GrafanaThemeV2) => {
   return {
     container: css`
       width: 100%;

packages/grafana-ui/src/components/VizTooltip/VizTooltipContainer.tsx

@@ -1,9 +1,9 @@
 import React, { useState, useLayoutEffect, useRef, HTMLAttributes, useMemo } from 'react';
 import { css, cx } from '@emotion/css';
-import { useStyles } from '../../themes';
+import { useStyles2 } from '../../themes';
 import { getTooltipContainerStyles } from '../../themes/mixins';
 import useWindowSize from 'react-use/lib/useWindowSize';
-import { Dimensions2D, GrafanaTheme } from '@grafana/data';
+import { Dimensions2D, GrafanaThemeV2 } from '@grafana/data';
 
 /**
  * @public
@@ -84,7 +84,7 @@ export const VizTooltipContainer: React.FC<VizTooltipContainerProps> = ({
     });
   }, [width, height, positionX, offsetX, positionY, offsetY, tooltipMeasurement.width, tooltipMeasurement.height]);
 
-  const styles = useStyles(getStyles);
+  const styles = useStyles2(getStyles);
 
   return (
     <div
@@ -105,7 +105,7 @@ export const VizTooltipContainer: React.FC<VizTooltipContainerProps> = ({
 
 VizTooltipContainer.displayName = 'VizTooltipContainer';
 
-const getStyles = (theme: GrafanaTheme) => ({
+const getStyles = (theme: GrafanaThemeV2) => ({
   wrapper: css`
     ${getTooltipContainerStyles(theme)}
   `,

packages/grafana-ui/src/themes/mixins.ts

@@ -63,11 +63,11 @@ export function getFocusStyles(theme: GrafanaThemeV2): CSSObject {
 }
 
 // max-width is set up based on .grafana-tooltip class that's used in dashboard
-export const getTooltipContainerStyles = (theme: GrafanaTheme) => `
+export const getTooltipContainerStyles = (theme: GrafanaThemeV2) => `
   overflow: hidden;
-  background: ${theme.colors.bg2};
+  background: ${theme.components.tooltip.background};
   max-width: 800px;
-  padding: ${theme.spacing.sm};
+  padding: ${theme.spacing(1)};
-  border-radius: ${theme.border.radius.sm};
+  border-radius: ${theme.shape.borderRadius()};
   z-index: ${theme.zIndex.tooltip};
 `;