Skip to content
This repository was archived by the owner on Jun 6, 2025. It is now read-only.

Commit 97c5af0

Browse files
michaelnesenmichaelnesen
committed
FunnelChartNext follow ups
1 parent 8cf0484 commit 97c5af0

File tree

10 files changed

+396
-52
lines changed

10 files changed

+396
-52
lines changed

‎packages/polaris-viz/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
77

88
## Unreleased
99

10+
### Changed
11+
12+
- Removed main percentage label from `<FunnelChartNext />`
13+
1014
### Fixed
1115

1216
- Double formatting in Donut Chart label calculations

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

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,12 @@ import {
1414
FunnelChartConnectorGradient,
1515
} from '../shared/FunnelChartConnector';
1616
import {FunnelChartSegment} from '../shared';
17-
import {SingleTextLine} from '../Labels';
1817
import {ChartElements} from '../ChartElements';
1918

2019
import {FunnelChartLabels} from './components';
2120
import {
2221
LABELS_HEIGHT,
23-
PERCENTAGE_SUMMARY_HEIGHT,
2422
LINE_GRADIENT,
25-
PERCENTAGE_COLOR,
2623
LINE_OFFSET,
2724
LINE_WIDTH,
2825
GAP,
@@ -50,6 +47,7 @@ export function Chart({
5047
const dataSeries = data[0].data;
5148
const xValues = dataSeries.map(({key}) => key) as string[];
5249
const yValues = dataSeries.map(({value}) => value) as [number, number];
50+
const sanitizedYValues = yValues.map((value) => Math.max(0, value));
5351

5452
const {width: drawableWidth, height: drawableHeight} = containerBounds ?? {
5553
width: 0,
@@ -58,14 +56,15 @@ export function Chart({
5856
y: 0,
5957
};
6058

61-
const highestYValue = Math.max(...yValues);
59+
const highestYValue = Math.max(...sanitizedYValues);
60+
6261
const yScale = scaleLinear()
63-
.range([0, drawableHeight - LABELS_HEIGHT - PERCENTAGE_SUMMARY_HEIGHT])
62+
.range([0, drawableHeight - LABELS_HEIGHT])
6463
.domain([0, highestYValue]);
6564

6665
const {getBarHeight, shouldApplyScaling} = useFunnelBarScaling({
6766
yScale,
68-
values: yValues,
67+
values: sanitizedYValues,
6968
});
7069

7170
const labels = useMemo(
@@ -89,11 +88,12 @@ export function Chart({
8988
const barWidth = sectionWidth * SEGMENT_WIDTH_RATIO;
9089
const lineGradientId = useMemo(() => uniqueId('line-gradient'), []);
9190

92-
const lastPoint = dataSeries.at(-1);
9391
const firstPoint = dataSeries[0];
9492

9593
const calculatePercentage = (value: number, total: number) => {
96-
return total === 0 ? 0 : (value / total) * 100;
94+
const sanitizedValue = Math.max(0, value);
95+
const sanitizedTotal = Math.max(0, total);
96+
return sanitizedTotal === 0 ? 0 : (sanitizedValue / sanitizedTotal) * 100;
9797
};
9898

9999
const percentages = dataSeries.map((dataPoint) => {
@@ -107,10 +107,6 @@ export function Chart({
107107
return labelFormatter(dataPoint.value);
108108
});
109109

110-
const mainPercentage = percentageFormatter(
111-
calculatePercentage(lastPoint?.value ?? 0, firstPoint?.value ?? 0),
112-
);
113-
114110
return (
115111
<ChartElements.Svg height={drawableHeight} width={drawableWidth}>
116112
<g>
@@ -124,17 +120,7 @@ export function Chart({
124120
y1="0%"
125121
y2="100%"
126122
/>
127-
128-
<SingleTextLine
129-
color={PERCENTAGE_COLOR}
130-
fontWeight={600}
131-
targetWidth={drawableWidth}
132-
fontSize={20}
133-
text={mainPercentage}
134-
textAnchor="start"
135-
/>
136-
137-
<g transform={`translate(0,${PERCENTAGE_SUMMARY_HEIGHT})`}>
123+
<g>
138124
<FunnelChartLabels
139125
formattedValues={formattedValues}
140126
labels={labels}
@@ -185,10 +171,10 @@ export function Chart({
185171
</FunnelChartSegment>
186172
{index > 0 && (
187173
<rect
188-
y={PERCENTAGE_SUMMARY_HEIGHT}
174+
y={0}
189175
x={x - (LINE_OFFSET - LINE_WIDTH)}
190176
width={LINE_WIDTH}
191-
height={drawableHeight - PERCENTAGE_SUMMARY_HEIGHT}
177+
height={drawableHeight}
192178
fill={`url(#${lineGradientId})`}
193179
/>
194180
)}

packages/polaris-viz/src/components/FunnelChartNext/components/FunnelChartLabels.tsx renamed to packages/polaris-viz/src/components/FunnelChartNext/components/FunnelChartLabels/FunnelChartLabels.tsx

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@ import {Fragment, useMemo, useState} from 'react';
33
import type {ScaleBand} from 'd3-scale';
44
import {estimateStringWidth, useChartContext} from '@shopify/polaris-viz-core';
55

6-
import {LINE_HEIGHT} from '../../../constants';
7-
import {estimateStringWidthWithOffset} from '../../../utilities';
8-
import {SingleTextLine} from '../../Labels';
9-
10-
import {ScaleIcon} from './ScaleIcon';
11-
import {ScaleIconTooltip} from './ScaleIconTooltip';
6+
import {LINE_HEIGHT} from '../../../../constants';
7+
import {estimateStringWidthWithOffset} from '../../../../utilities';
8+
import {SingleTextLine} from '../../../Labels';
9+
import {ScaleIcon} from '../ScaleIcon';
10+
import {ScaleIconTooltip} from '../ScaleIconTooltip';
1211

1312
const LINE_GAP = 5;
1413
const LINE_PADDING = 10;

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export {FunnelChartLabels} from './FunnelChartLabels';

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

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
import {mount} from '@shopify/react-testing';
2+
import {scaleBand} from 'd3-scale';
3+
import {ChartContext} from '@shopify/polaris-viz-core';
4+
5+
import {SingleTextLine} from '../../../../Labels';
6+
import {ScaleIcon} from '../../ScaleIcon';
7+
import {ScaleIconTooltip} from '../../ScaleIconTooltip';
8+
import {FunnelChartLabels} from '../FunnelChartLabels';
9+
10+
describe('<FunnelChartLabels />', () => {
11+
const mockContext = {
12+
characterWidths: new Map([['default', 10]]),
13+
containerBounds: {
14+
width: 500,
15+
height: 300,
16+
x: 0,
17+
y: 0,
18+
},
19+
};
20+
21+
const mockProps = {
22+
formattedValues: ['1,000', '750', '500'],
23+
labels: ['Step 1', 'Step 2', 'Step 3'],
24+
labelWidth: 150,
25+
barWidth: 100,
26+
percentages: ['100%', '75%', '50%'],
27+
xScale: scaleBand().domain(['0', '1', '2']).range([0, 300]),
28+
shouldApplyScaling: false,
29+
renderScaleIconTooltipContent: () => <div>Tooltip content</div>,
30+
};
31+
32+
const wrapper = (props = mockProps) => {
33+
return mount(
34+
<ChartContext.Provider value={mockContext}>
35+
<FunnelChartLabels {...props} />
36+
</ChartContext.Provider>,
37+
);
38+
};
39+
40+
describe('text elements', () => {
41+
it('renders expected number of text elements', () => {
42+
const component = wrapper();
43+
const texts = component.findAll(SingleTextLine);
44+
// 3 labels + 3 percentages + 3 values
45+
expect(texts).toHaveLength(9);
46+
});
47+
48+
it('renders labels, percentages, and values', () => {
49+
const component = wrapper();
50+
51+
expect(component).toContainReactComponent(SingleTextLine, {
52+
text: 'Step 1',
53+
});
54+
expect(component).toContainReactComponent(SingleTextLine, {
55+
text: '100%',
56+
});
57+
expect(component).toContainReactComponent(SingleTextLine, {
58+
text: '1,000',
59+
});
60+
});
61+
62+
it('hides formatted values when space is constrained', () => {
63+
const propsWithNarrowWidth = {
64+
...mockProps,
65+
labelWidth: 50,
66+
barWidth: 50,
67+
};
68+
69+
const component = wrapper(propsWithNarrowWidth);
70+
const texts = component.findAll(SingleTextLine);
71+
72+
expect(texts).toHaveLength(6);
73+
74+
// Verify labels and percentages are still shown
75+
expect(component).toContainReactComponent(SingleTextLine, {
76+
text: 'Step 1',
77+
});
78+
expect(component).toContainReactComponent(SingleTextLine, {
79+
text: '100%',
80+
});
81+
});
82+
});
83+
84+
describe('scale icon', () => {
85+
it('renders scale icon when shouldApplyScaling is true', () => {
86+
const component = wrapper({
87+
...mockProps,
88+
shouldApplyScaling: true,
89+
});
90+
91+
expect(component).toContainReactComponent(ScaleIcon);
92+
});
93+
94+
it('does not render scale icon when shouldApplyScaling is false', () => {
95+
const component = wrapper({
96+
...mockProps,
97+
shouldApplyScaling: false,
98+
});
99+
100+
expect(component).not.toContainReactComponent(ScaleIcon);
101+
});
102+
103+
it('shows tooltip when scale icon is hovered', () => {
104+
const mockTooltipContent = () => <div>Tooltip content</div>;
105+
const component = wrapper({
106+
...mockProps,
107+
shouldApplyScaling: true,
108+
renderScaleIconTooltipContent: mockTooltipContent,
109+
});
110+
111+
// Get the second g element
112+
const iconContainer = component.findAll('g')[1];
113+
iconContainer.trigger('onMouseEnter');
114+
115+
expect(component).toContainReactComponent(ScaleIconTooltip);
116+
});
117+
118+
it('hides tooltip when scale icon is unhovered', () => {
119+
const mockTooltipContent = () => <div>Tooltip content</div>;
120+
const component = wrapper({
121+
...mockProps,
122+
shouldApplyScaling: true,
123+
renderScaleIconTooltipContent: mockTooltipContent,
124+
});
125+
126+
const iconContainer = component.findAll('g')[1];
127+
128+
iconContainer.trigger('onMouseEnter');
129+
iconContainer.trigger('onMouseLeave');
130+
131+
expect(component).not.toContainReactComponent(ScaleIconTooltip);
132+
});
133+
});
134+
135+
describe('label font size', () => {
136+
it('uses reduced font size when labels are too long', () => {
137+
const propsWithLongLabels = {
138+
...mockProps,
139+
labels: [
140+
'Very Long Step Name 1',
141+
'Very Long Step Name 2',
142+
'Very Long Step Name 3',
143+
],
144+
labelWidth: 50,
145+
};
146+
147+
const component = wrapper(propsWithLongLabels);
148+
149+
expect(component.findAll(SingleTextLine)[0]).toHaveReactProps({
150+
fontSize: 11,
151+
});
152+
});
153+
154+
it('uses default font size when labels fit', () => {
155+
const component = wrapper();
156+
157+
expect(component.findAll(SingleTextLine)[0]).toHaveReactProps({
158+
fontSize: 12,
159+
});
160+
});
161+
});
162+
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
export {FunnelChartLabels} from './FunnelChartLabels';
1+
export {FunnelChartLabels} from './FunnelChartLabels/FunnelChartLabels';
22
export {FunnelTooltip} from './FunnelTooltip';
33
export {TooltipWithPortal} from './TooltipWithPortal';
44
export {ScaleIcon} from './ScaleIcon';

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import {mount} from '@shopify/react-testing';
2+
import {ChartContext} from '@shopify/polaris-viz-core';
3+
import type {DataSeries} from '@shopify/polaris-viz-core';
4+
import React from 'react';
5+
6+
import {Chart} from '../Chart';
7+
import {FunnelChartConnector, FunnelChartSegment} from '../../shared';
8+
9+
const mockData: DataSeries[] = [
10+
{
11+
name: 'Group 1',
12+
data: [
13+
{key: 'Step 1', value: 100},
14+
{key: 'Step 2', value: 75},
15+
{key: 'Step 3', value: 50},
16+
],
17+
},
18+
];
19+
20+
const mockContext = {
21+
containerBounds: {
22+
width: 500,
23+
height: 300,
24+
x: 0,
25+
y: 0,
26+
},
27+
};
28+
29+
const defaultProps = {
30+
data: mockData,
31+
accessibilityLabel: 'Funnel chart showing conversion',
32+
};
33+
34+
describe('<Chart />', () => {
35+
it('renders funnel segments for each data point', () => {
36+
const chart = mount(
37+
<ChartContext.Provider value={mockContext}>
38+
<Chart {...defaultProps} />
39+
</ChartContext.Provider>,
40+
);
41+
42+
expect(chart).toContainReactComponentTimes(
43+
FunnelChartSegment,
44+
mockData[0].data.length,
45+
);
46+
});
47+
48+
it('renders n-1 connectors for n funnel segments, excluding the last segment', () => {
49+
const chart = mount(
50+
<ChartContext.Provider value={mockContext}>
51+
<Chart {...defaultProps} />
52+
</ChartContext.Provider>,
53+
);
54+
55+
expect(chart).toContainReactComponentTimes(
56+
FunnelChartConnector,
57+
mockData[0].data.length - 1,
58+
);
59+
});
60+
61+
it('renders accessibility label when provided', () => {
62+
const accessibilityLabel = 'Custom accessibility label';
63+
const chart = mount(
64+
<ChartContext.Provider value={mockContext}>
65+
<Chart {...defaultProps} accessibilityLabel={accessibilityLabel} />
66+
</ChartContext.Provider>,
67+
);
68+
69+
expect(chart).toContainReactText(accessibilityLabel);
70+
});
71+
72+
it('renders segments with expected aria labels', () => {
73+
const chart = mount(
74+
<ChartContext.Provider value={mockContext}>
75+
<Chart {...defaultProps} />
76+
</ChartContext.Provider>,
77+
);
78+
79+
const firstSegment = chart.findAll(FunnelChartSegment)[0];
80+
expect(firstSegment).toHaveReactProps({
81+
ariaLabel: 'Step 1: 100',
82+
});
83+
});
84+
});

0 commit comments

Comments
 (0)