Donut Chart UX updates

Shopify · Sep 25, 2023 · 9b9febc · 9b9febc
1 parent 68b0b2b
commit 9b9febc
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 103 deletions.
diff --git a/packages/polaris-viz-core/src/constants.ts b/packages/polaris-viz-core/src/constants.ts
@@ -504,7 +504,6 @@ export const ELLIPSIS = '…';
 export const HORIZONTAL_LABEL_MIN_WIDTH = 46;
 export const HORIZONTAL_LABEL_TARGET_HEIGHT = 80;
 export const DIAGONAL_LABEL_MIN_WIDTH = 30;
-export const DONUT_CHART_MAX_SERIES_COUNT = 5;
 export const MAX_DIAGONAL_LABEL_WIDTH = 100;
 // Visible height of a 100px wide label at 45deg
 export const MAX_DIAGONAL_VISIBLE_HEIGHT = 80;

diff --git a/packages/polaris-viz/src/components/DonutChart/Chart.tsx b/packages/polaris-viz/src/components/DonutChart/Chart.tsx
@@ -16,7 +16,6 @@ import type {
   Direction,
 } from '@shopify/polaris-viz-core';
 
-import {DONUT_CHART_MAX_SERIES_COUNT} from '../../constants';
 import {getContainerAlignmentForLegend} from '../../utilities';
 import {estimateLegendItemWidth} from '../Legend';
 import type {ComparisonMetricProps} from '../ComparisonMetric';
@@ -83,25 +82,17 @@ export function Chart({
   const seriesCount = clamp({
     amount: data.length,
     min: 1,
-    max: DONUT_CHART_MAX_SERIES_COUNT,
+    max: Infinity,
   });
 
-  const seriesData = data
-    .filter(({data}) => Number(data[0]?.value) > 0)
-    .sort(
-      (current, next) =>
-        Number(next.data[0].value) - Number(current.data[0].value),
-    )
-    .slice(0, seriesCount);
-
   const seriesColor = getSeriesColors(seriesCount, selectedTheme);
 
   const legendDirection: Direction =
     legendPosition === 'right' || legendPosition === 'left'
       ? 'vertical'
       : 'horizontal';
 
-  const longestLegendWidth = seriesData.reduce((previous, current) => {
+  const longestLegendWidth = data.reduce((previous, current) => {
     const estimatedLegendWidth = estimateLegendItemWidth(
       current.name ?? '',
       characterWidths,
@@ -124,7 +115,7 @@ export function Chart({
 
   const {height, width, legend, setLegendDimensions, isLegendMounted} =
     useLegend({
-      data: [{series: seriesData, shape: 'Bar'}],
+      data: [{series: data.slice(0, 5), shape: 'Bar'}],
       dimensions,
       showLegend,
       direction: legendDirection,
@@ -149,7 +140,7 @@ export function Chart({
   const diameter = Math.min(height, width);
   const radius = diameter / 2;
 
-  const points: DataPoint[] = seriesData.reduce(
+  const points: DataPoint[] = data.reduce(
     (prev: DataPoint[], {data}) => prev.concat(data),
     [],
   );
@@ -206,9 +197,8 @@ export function Chart({
                   ) : (
                     pieChartData.map(
                       ({data: pieData, startAngle, endAngle}, index) => {
-                        const color =
-                          seriesData[index]?.color ?? seriesColor[index];
-                        const name = seriesData[index].name;
+                        const color = data[index]?.color ?? seriesColor[index];
+                        const name = data[index].name;
                         const accessibilityLabel = `${name}: ${pieData.key} - ${pieData.value}`;
 
                         return (

diff --git a/packages/polaris-viz/src/components/DonutChart/DonutChart.scss b/packages/polaris-viz/src/components/DonutChart/DonutChart.scss
@@ -12,6 +12,8 @@
   position: relative;
   display: flex;
   align-items: center;
+  justify-content: center;
+  height: 100%;
 }
 
 .ContentWrapper {

diff --git a/packages/polaris-viz/src/components/DonutChart/components/InnerValue/InnerValue.tsx b/packages/polaris-viz/src/components/DonutChart/components/InnerValue/InnerValue.tsx
@@ -10,7 +10,7 @@ import {ComparisonMetric} from '../../../ComparisonMetric';
 import styles from '../../DonutChart.scss';
 
 interface Props {
-  activeValue: number | null;
+  activeValue: number | null | undefined;
   labelFormatter: LabelFormatter;
   isAnimated: boolean;
   totalValue: number;
@@ -47,7 +47,8 @@ export function InnerValue({
     </animated.span>
   );
 
-  const valueToDisplay = activeValue || animatedTotalValue;
+  const activeValueExists = activeValue !== null && activeValue !== undefined;
+  const valueToDisplay = activeValueExists ? activeValue : animatedTotalValue;
 
   const innerContent = renderInnerValueContent?.({
     activeValue,
@@ -61,7 +62,7 @@ export function InnerValue({
       >
         {valueToDisplay}
       </animated.p>
-      {comparisonMetric != null && !activeValue && (
+      {comparisonMetric != null && !activeValueExists && (
         <div className={styles.ComparisonMetric}>
           <ComparisonMetric
             metric={comparisonMetric.metric}

diff --git a/packages/polaris-viz/src/components/DonutChart/stories/CustomInnerValueContent.stories.tsx b/packages/polaris-viz/src/components/DonutChart/stories/CustomInnerValueContent.stories.tsx
@@ -8,51 +8,61 @@ import {DEFAULT_DATA, DEFAULT_PROPS, Template} from './data';
 
 const numberStyles = {
   fontWeight: 600,
-  fontFamily: "ui-monospace, SFMono-Regular, 'SF Mono', Consolas, 'Liberation Mono', Menlo, monospace",
-}
+  fontFamily:
+    "ui-monospace, SFMono-Regular, 'SF Mono', Consolas, 'Liberation Mono', Menlo, monospace",
+};
 
-export const CustomInnerValueContent: Story<DonutChartProps> = Template.bind({});
+export const CustomInnerValueContent: Story<DonutChartProps> = Template.bind(
+  {},
+);
 
 CustomInnerValueContent.args = {
   ...DEFAULT_PROPS,
   data: DEFAULT_DATA,
   renderInnerValueContent: ({activeValue, animatedTotalValue, totalValue}) => {
-    const activeValuePercentage = activeValue
-      ? `${(activeValue / totalValue * 100).toFixed(1)}%`
-      : null;
+    const activeValuePercentage =
+      activeValue === null || activeValue === undefined
+        ? null
+        : `${((activeValue / totalValue) * 100).toFixed(1)}%`;
 
     return (
-      <div style={{
-        display: 'flex',
-        flexDirection: 'column',
-        textAlign: 'center',
-      }}>
+      <div
+        style={{
+          display: 'flex',
+          flexDirection: 'column',
+          textAlign: 'center',
+        }}
+      >
         {activeValuePercentage && (
-          <p style={{
-            fontSize: 36,
-            margin: 0,
-            ...numberStyles,
-          }}>
+          <p
+            style={{
+              fontSize: 36,
+              margin: 0,
+              ...numberStyles,
+            }}
+          >
             {activeValuePercentage}
           </p>
         )}
-        <p style={{
-          display: 'flex',
-          alignItems: 'center',
-          gap: 4,
-          margin: '8px 0',
-        }}>
+        <p
+          style={{
+            display: 'flex',
+            alignItems: 'center',
+            gap: 4,
+            margin: '8px 0',
+          }}
+        >
           Total:
           <span
             style={{
               fontSize: 20,
-              ...numberStyles
+              ...numberStyles,
             }}
           >
             {animatedTotalValue}
           </span>
         </p>
       </div>
-    )
-  }
+    );
+  },
 };
diff --git a/packages/polaris-viz/src/components/DonutChart/tests/DonutChart.test.tsx b/packages/polaris-viz/src/components/DonutChart/tests/DonutChart.test.tsx
@@ -73,63 +73,6 @@ describe('<DonutChart />', () => {
       });
     });
 
-    it('truncates data to only show a maximum of 5 points', () => {
-      const chart = mount(
-        <DonutChart
-          {...mockProps}
-          data={[
-            ...mockProps.data,
-            {
-              name: 'Extra Data 1',
-              data: [{key: 'april - march', value: 100}],
-            },
-            {
-              name: 'Extra Data 2 ',
-              data: [{key: 'april - march', value: 10}],
-            },
-            {
-              name: 'Extra Data 3',
-              data: [{key: 'april - march', value: 10}],
-            },
-          ]}
-        />,
-      );
-
-      chart.act(() => {
-        requestAnimationFrame(() => {
-          expect(chart).toContainReactComponentTimes(Arc, 5);
-        });
-      });
-    });
-
-    it('filters out data with 0 or negative values', () => {
-      const chart = mount(
-        <DonutChart
-          {...mockProps}
-          data={[
-            ...mockProps.data,
-            {
-              name: 'Zero Value',
-              data: [{key: 'april - march', value: 0}],
-            },
-            {
-              name: 'Negative Value',
-              data: [{key: 'april - march', value: -100}],
-            },
-          ]}
-        />,
-      );
-
-      chart.act(() => {
-        requestAnimationFrame(() => {
-          expect(chart).toContainReactComponentTimes(
-            Arc,
-            mockProps.data.length,
-          );
-        });
-      });
-    });
-
     describe('<ComparisonMetric />', () => {
       it('does not render if comparisonMetric is not provided', () => {
         const chart = mount(

diff --git a/packages/polaris-viz/src/types.ts b/packages/polaris-viz/src/types.ts
@@ -208,7 +208,7 @@ export type RenderLegendContent = (
 export type SortedBarChartData = (number | null)[][];
 
 export interface InnerValueContents {
-  activeValue: number | null;
+  activeValue: number | null | undefined;
   animatedTotalValue: ReactNode;
   totalValue: number;
 }