FunnelChartNext follow ups

packages/polaris-viz/CHANGELOG.md

@@ -7,6 +7,10 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
 
+### Changed
+
+- Removed main percentage label from `<FunnelChartNext />`
+
 ### Fixed
 
 - Double formatting in Donut Chart label calculations

packages/polaris-viz/src/components/FunnelChartNext/Chart.tsx

@@ -14,15 +14,12 @@ import {
   FunnelChartConnectorGradient,
 } from '../shared/FunnelChartConnector';
 import {FunnelChartSegment} from '../shared';
-import {SingleTextLine} from '../Labels';
 import {ChartElements} from '../ChartElements';
 
 import {FunnelChartLabels} from './components';
 import {
   LABELS_HEIGHT,
-  PERCENTAGE_SUMMARY_HEIGHT,
   LINE_GRADIENT,
-  PERCENTAGE_COLOR,
   LINE_OFFSET,
   LINE_WIDTH,
   GAP,
@@ -50,6 +47,8 @@ export function Chart({
   const dataSeries = data[0].data;
   const xValues = dataSeries.map(({key}) => key) as string[];
   const yValues = dataSeries.map(({value}) => value) as [number, number];
+  const sanitizedYValues = yValues.map((value) => Math.max(0, value));
+  console.log('sanitizedYValues', sanitizedYValues);
 
   const {width: drawableWidth, height: drawableHeight} = containerBounds ?? {
     width: 0,
@@ -58,14 +57,15 @@ export function Chart({
     y: 0,
   };
 
-  const highestYValue = Math.max(...yValues);
+  const highestYValue = Math.max(...sanitizedYValues);
+
   const yScale = scaleLinear()
-    .range([0, drawableHeight - LABELS_HEIGHT - PERCENTAGE_SUMMARY_HEIGHT])
+    .range([0, drawableHeight - LABELS_HEIGHT])
     .domain([0, highestYValue]);
 
   const {getBarHeight, shouldApplyScaling} = useFunnelBarScaling({
     yScale,
-    values: yValues,
+    values: sanitizedYValues,
   });
 
   const labels = useMemo(
@@ -89,11 +89,12 @@ export function Chart({
   const barWidth = sectionWidth * SEGMENT_WIDTH_RATIO;
   const lineGradientId = useMemo(() => uniqueId('line-gradient'), []);
 
-  const lastPoint = dataSeries.at(-1);
   const firstPoint = dataSeries[0];
 
   const calculatePercentage = (value: number, total: number) => {
-    return total === 0 ? 0 : (value / total) * 100;
+    const sanitizedValue = Math.max(0, value);
+    const sanitizedTotal = Math.max(0, total);
+    return sanitizedTotal === 0 ? 0 : (sanitizedValue / sanitizedTotal) * 100;
   };
 
   const percentages = dataSeries.map((dataPoint) => {
@@ -107,10 +108,6 @@ export function Chart({
     return labelFormatter(dataPoint.value);
   });
 
-  const mainPercentage = percentageFormatter(
-    calculatePercentage(lastPoint?.value ?? 0, firstPoint?.value ?? 0),
-  );
-
   return (
     <ChartElements.Svg height={drawableHeight} width={drawableWidth}>
       <g>
@@ -124,17 +121,7 @@ export function Chart({
           y1="0%"
           y2="100%"
         />
-
-        <SingleTextLine
-          color={PERCENTAGE_COLOR}
-          fontWeight={600}
-          targetWidth={drawableWidth}
-          fontSize={20}
-          text={mainPercentage}
-          textAnchor="start"
-        />
-
-        <g transform={`translate(0,${PERCENTAGE_SUMMARY_HEIGHT})`}>
+        <g>
           <FunnelChartLabels
             formattedValues={formattedValues}
             labels={labels}
@@ -185,10 +172,10 @@ export function Chart({
                 </FunnelChartSegment>
                 {index > 0 && (
                   <rect
-                    y={PERCENTAGE_SUMMARY_HEIGHT}
+                    y={0}
                     x={x - (LINE_OFFSET - LINE_WIDTH)}
                     width={LINE_WIDTH}
-                    height={drawableHeight - PERCENTAGE_SUMMARY_HEIGHT}
+                    height={drawableHeight}
                     fill={`url(#${lineGradientId})`}
                   />
                 )}

packages/polaris-viz/src/components/FunnelChartNext/components/FunnelChartLabels.tsx → packages/polaris-viz/src/components/FunnelChartNext/components/FunnelChartLabels/FunnelChartLabels.tsx

@@ -3,12 +3,11 @@ import {Fragment, useMemo, useState} from 'react';
 import type {ScaleBand} from 'd3-scale';
 import {estimateStringWidth, useChartContext} from '@shopify/polaris-viz-core';
 
-import {LINE_HEIGHT} from '../../../constants';
-import {estimateStringWidthWithOffset} from '../../../utilities';
-import {SingleTextLine} from '../../Labels';
-
-import {ScaleIcon} from './ScaleIcon';
-import {ScaleIconTooltip} from './ScaleIconTooltip';
+import {LINE_HEIGHT} from '../../../../constants';
+import {estimateStringWidthWithOffset} from '../../../../utilities';
+import {SingleTextLine} from '../../../Labels';
+import {ScaleIcon} from '../ScaleIcon';
+import {ScaleIconTooltip} from '../ScaleIconTooltip';
 
 const LINE_GAP = 5;
 const LINE_PADDING = 10;

packages/polaris-viz/src/components/FunnelChartNext/components/FunnelChartLabels/index.ts

@@ -0,0 +1 @@
+export {FunnelChartLabels} from './FunnelChartLabels';

packages/polaris-viz/src/components/FunnelChartNext/components/FunnelChartLabels/tests/FunnelChartLabels.test.tsx

@@ -0,0 +1,162 @@
+import {mount} from '@shopify/react-testing';
+import {scaleBand} from 'd3-scale';
+import {ChartContext} from '@shopify/polaris-viz-core';
+
+import {SingleTextLine} from '../../../../Labels';
+import {ScaleIcon} from '../../ScaleIcon';
+import {ScaleIconTooltip} from '../../ScaleIconTooltip';
+import {FunnelChartLabels} from '../FunnelChartLabels';
+
+describe('<FunnelChartLabels />', () => {
+  const mockContext = {
+    characterWidths: new Map([['default', 10]]),
+    containerBounds: {
+      width: 500,
+      height: 300,
+      x: 0,
+      y: 0,
+    },
+  };
+
+  const mockProps = {
+    formattedValues: ['1,000', '750', '500'],
+    labels: ['Step 1', 'Step 2', 'Step 3'],
+    labelWidth: 150,
+    barWidth: 100,
+    percentages: ['100%', '75%', '50%'],
+    xScale: scaleBand().domain(['0', '1', '2']).range([0, 300]),
+    shouldApplyScaling: false,
+    renderScaleIconTooltipContent: () => <div>Tooltip content</div>,
+  };
+
+  const wrapper = (props = mockProps) => {
+    return mount(
+      <ChartContext.Provider value={mockContext}>
+        <FunnelChartLabels {...props} />
+      </ChartContext.Provider>,
+    );
+  };
+
+  describe('text elements', () => {
+    it('renders expected number of text elements', () => {
+      const component = wrapper();
+      const texts = component.findAll(SingleTextLine);
+      // 3 labels + 3 percentages + 3 values
+      expect(texts).toHaveLength(9);
+    });
+
+    it('renders labels, percentages, and values', () => {
+      const component = wrapper();
+
+      expect(component).toContainReactComponent(SingleTextLine, {
+        text: 'Step 1',
+      });
+      expect(component).toContainReactComponent(SingleTextLine, {
+        text: '100%',
+      });
+      expect(component).toContainReactComponent(SingleTextLine, {
+        text: '1,000',
+      });
+    });
+
+    it('hides formatted values when space is constrained', () => {
+      const propsWithNarrowWidth = {
+        ...mockProps,
+        labelWidth: 50,
+        barWidth: 50,
+      };
+
+      const component = wrapper(propsWithNarrowWidth);
+      const texts = component.findAll(SingleTextLine);
+
+      expect(texts).toHaveLength(6);
+
+      // Verify labels and percentages are still shown
+      expect(component).toContainReactComponent(SingleTextLine, {
+        text: 'Step 1',
+      });
+      expect(component).toContainReactComponent(SingleTextLine, {
+        text: '100%',
+      });
+    });
+  });
+
+  describe('scale icon', () => {
+    it('renders scale icon when shouldApplyScaling is true', () => {
+      const component = wrapper({
+        ...mockProps,
+        shouldApplyScaling: true,
+      });
+
+      expect(component).toContainReactComponent(ScaleIcon);
+    });
+
+    it('does not render scale icon when shouldApplyScaling is false', () => {
+      const component = wrapper({
+        ...mockProps,
+        shouldApplyScaling: false,
+      });
+
+      expect(component).not.toContainReactComponent(ScaleIcon);
+    });
+
+    it('shows tooltip when scale icon is hovered', () => {
+      const mockTooltipContent = () => <div>Tooltip content</div>;
+      const component = wrapper({
+        ...mockProps,
+        shouldApplyScaling: true,
+        renderScaleIconTooltipContent: mockTooltipContent,
+      });
+
+      // Get the second g element
+      const iconContainer = component.findAll('g')[1];
+      iconContainer.trigger('onMouseEnter');
+
+      expect(component).toContainReactComponent(ScaleIconTooltip);
+    });
+
+    it('hides tooltip when scale icon is unhovered', () => {
+      const mockTooltipContent = () => <div>Tooltip content</div>;
+      const component = wrapper({
+        ...mockProps,
+        shouldApplyScaling: true,
+        renderScaleIconTooltipContent: mockTooltipContent,
+      });
+
+      const iconContainer = component.findAll('g')[1];
+
+      iconContainer.trigger('onMouseEnter');
+      iconContainer.trigger('onMouseLeave');
+
+      expect(component).not.toContainReactComponent(ScaleIconTooltip);
+    });
+  });
+
+  describe('label font size', () => {
+    it('uses reduced font size when labels are too long', () => {
+      const propsWithLongLabels = {
+        ...mockProps,
+        labels: [
+          'Very Long Step Name 1',
+          'Very Long Step Name 2',
+          'Very Long Step Name 3',
+        ],
+        labelWidth: 50,
+      };
+
+      const component = wrapper(propsWithLongLabels);
+
+      expect(component.findAll(SingleTextLine)[0]).toHaveReactProps({
+        fontSize: 11,
+      });
+    });
+
+    it('uses default font size when labels fit', () => {
+      const component = wrapper();
+
+      expect(component.findAll(SingleTextLine)[0]).toHaveReactProps({
+        fontSize: 12,
+      });
+    });
+  });
+});

packages/polaris-viz/src/components/FunnelChartNext/components/index.ts

@@ -1,4 +1,4 @@
-export {FunnelChartLabels} from './FunnelChartLabels';
+export {FunnelChartLabels} from './FunnelChartLabels/FunnelChartLabels';
 export {FunnelTooltip} from './FunnelTooltip';
 export {TooltipWithPortal} from './TooltipWithPortal';
 export {ScaleIcon} from './ScaleIcon';

packages/polaris-viz/src/components/SparkFunnelChart/tests/Chart.test.tsx

@@ -0,0 +1,84 @@
+import {mount} from '@shopify/react-testing';
+import {ChartContext} from '@shopify/polaris-viz-core';
+import type {DataSeries} from '@shopify/polaris-viz-core';
+import React from 'react';
+
+import {Chart} from '../Chart';
+import {FunnelChartConnector, FunnelChartSegment} from '../../shared';
+
+const mockData: DataSeries[] = [
+  {
+    name: 'Group 1',
+    data: [
+      {key: 'Step 1', value: 100},
+      {key: 'Step 2', value: 75},
+      {key: 'Step 3', value: 50},
+    ],
+  },
+];
+
+const mockContext = {
+  containerBounds: {
+    width: 500,
+    height: 300,
+    x: 0,
+    y: 0,
+  },
+};
+
+const defaultProps = {
+  data: mockData,
+  accessibilityLabel: 'Funnel chart showing conversion',
+};
+
+describe('<Chart />', () => {
+  it('renders funnel segments for each data point', () => {
+    const chart = mount(
+      <ChartContext.Provider value={mockContext}>
+        <Chart {...defaultProps} />
+      </ChartContext.Provider>,
+    );
+
+    expect(chart).toContainReactComponentTimes(
+      FunnelChartSegment,
+      mockData[0].data.length,
+    );
+  });
+
+  it('renders n-1 connectors for n funnel segments, excluding the last segment', () => {
+    const chart = mount(
+      <ChartContext.Provider value={mockContext}>
+        <Chart {...defaultProps} />
+      </ChartContext.Provider>,
+    );
+
+    expect(chart).toContainReactComponentTimes(
+      FunnelChartConnector,
+      mockData[0].data.length - 1,
+    );
+  });
+
+  it('renders accessibility label when provided', () => {
+    const accessibilityLabel = 'Custom accessibility label';
+    const chart = mount(
+      <ChartContext.Provider value={mockContext}>
+        <Chart {...defaultProps} accessibilityLabel={accessibilityLabel} />
+      </ChartContext.Provider>,
+    );
+
+    expect(chart).toContainReactText(accessibilityLabel);
+  });
+
+  it('renders segments with expected aria labels', () => {
+    const chart = mount(
+      <ChartContext.Provider value={mockContext}>
+        <Chart {...defaultProps} />
+      </ChartContext.Provider>,
+    );
+
+    const firstSegment = chart.findAll(FunnelChartSegment)[0];
+    expect(firstSegment).toHaveReactProps({
+      ariaLabel: 'Step 1: 100',
+    });
+  });
+});