Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FunnelChartNext follow ups #1788

Merged
merged 1 commit into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/polaris-viz/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 11 additions & 25 deletions packages/polaris-viz/src/components/FunnelChartNext/Chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -50,6 +47,7 @@ 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));

const {width: drawableWidth, height: drawableHeight} = containerBounds ?? {
width: 0,
Expand All @@ -58,14 +56,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(
Expand All @@ -89,11 +88,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) => {
Expand All @@ -107,10 +107,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>
Expand All @@ -124,17 +120,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}
Expand Down Expand Up @@ -185,10 +171,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})`}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export {FunnelChartLabels} from './FunnelChartLabels';
Original file line number Diff line number Diff line change
@@ -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,
});
});
});
});
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
Original file line number Diff line number Diff line change
@@ -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',
});
});
});
Loading
Loading