Skip to content

Commit

Permalink
Label design changes
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelnesen committed Feb 14, 2025
1 parent c8f3f98 commit 858ec75
Show file tree
Hide file tree
Showing 9 changed files with 317 additions and 221 deletions.
22 changes: 18 additions & 4 deletions packages/polaris-viz/src/components/FunnelChartNext/Chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import {
uniqueId,
LinearGradientWithStops,
useChartContext,
clamp,
} from '@shopify/polaris-viz-core';

import {SCROLLBAR_WIDTH} from '../TooltipWrapper';
import {useFunnelBarScaling} from '../../hooks';
import {
FunnelChartConnector,
Expand Down Expand Up @@ -67,7 +69,7 @@ export function Chart({
const [tooltipIndex, setTooltipIndex] = useState<number | null>(null);
const {containerBounds} = useChartContext();
const dataSeries = data[0].data;
const calculatedTrends = useBuildFunnelTrends(data);
const calculatedTrends = useBuildFunnelTrends({data, percentageFormatter});
const trends = calculatedTrends?.trends;
const xValues = dataSeries.map(({key}) => key) as string[];
const yValues = dataSeries.map(({value}) => value) as [number, number];
Expand Down Expand Up @@ -255,13 +257,25 @@ export function Chart({

function getXPosition(activeDataSeries: DataPoint) {
const currentX = xScale(activeDataSeries.key.toString()) ?? 0;
let xPosition;

if (shouldPositionTooltipRight(activeDataSeries)) {
return chartX + currentX + barWidth + TOOLTIP_HORIZONTAL_OFFSET;
xPosition = chartX + currentX + barWidth + TOOLTIP_HORIZONTAL_OFFSET;
} else {
const xOffset = (barWidth - TOOLTIP_WIDTH) / 2;
xPosition = chartX + currentX + xOffset;
}

const xOffset = (barWidth - TOOLTIP_WIDTH) / 2;
return chartX + currentX + xOffset;
// Clamp the position to ensure tooltip stays within viewport
return clamp({
amount: xPosition,
min: TOOLTIP_HORIZONTAL_OFFSET,
max:
window.innerWidth -
TOOLTIP_WIDTH -
TOOLTIP_HORIZONTAL_OFFSET -
SCROLLBAR_WIDTH,
});
}

function getYPosition(activeDataSeries: DataPoint) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ const PERCENT_FONT_SIZE = 14;
const PERCENT_FONT_WEIGHT = 650;
const VALUE_FONT_SIZE = 11;
const TREND_INDICATOR_SPACING = 8;
const BUFFER_PADDING = 8;
const LABEL_VERTICAL_OFFSET = 2;
export const LABEL_VERTICAL_OFFSET = 2;

const TEXT_COLOR = 'rgba(31, 33, 36, 1)';
const VALUE_COLOR = 'rgba(97, 97, 97, 1)';
Expand Down Expand Up @@ -62,6 +61,62 @@ export function FunnelChartLabels({
return maxLabelWidth > labelWidth ? REDUCED_FONT_SIZE : LABEL_FONT_SIZE;
}, [labels, characterWidths, labelWidth]);

const canShowAllFormattedValues = useMemo(() => {
return labels.every((_, index) => {
const isLast = index === labels.length - 1;
const targetWidth = isLast
? barWidth - GROUP_OFFSET * 2
: labelWidth - GROUP_OFFSET * 2;

const percentWidth = estimateStringWidthWithOffset(
percentages[index],
PERCENT_FONT_SIZE,
PERCENT_FONT_WEIGHT,
);

const formattedValueWidth = estimateStringWidthWithOffset(
formattedValues[index],
VALUE_FONT_SIZE,
);

const trendIndicatorWidth = getTrendIndicatorData(
trends?.[index]?.reached,
).trendIndicatorWidth;

return (
percentWidth +
formattedValueWidth +
trendIndicatorWidth +
TREND_INDICATOR_SPACING * 2 <
targetWidth
);
});
}, [labels, barWidth, labelWidth, percentages, formattedValues, trends]);

const canShowAllTrendIndicatorsInline = useMemo(() => {
return labels.every((_, index) => {
const isLast = index === labels.length - 1;
const targetWidth = isLast
? barWidth - GROUP_OFFSET * 2
: labelWidth - GROUP_OFFSET * 2;

const percentWidth = estimateStringWidthWithOffset(
percentages[index],
PERCENT_FONT_SIZE,
PERCENT_FONT_WEIGHT,
);

const {trendIndicatorWidth} = getTrendIndicatorData(
trends?.[index]?.reached,
);

return (
percentWidth + TREND_INDICATOR_SPACING <
targetWidth - trendIndicatorWidth
);
});
}, [labels, barWidth, labelWidth, percentages, trends]);

return (
<Fragment>
{labels.map((label, index) => {
Expand All @@ -80,29 +135,12 @@ export function FunnelChartLabels({
PERCENT_FONT_WEIGHT,
);

const formattedValueWidth = estimateStringWidthWithOffset(
formattedValues[index],
VALUE_FONT_SIZE,
);

const {trendIndicatorProps, trendIndicatorWidth} =
getTrendIndicatorData(trends?.[index]?.reached);

// Position trend indicator at the right edge
const trendIndicatorX = targetWidth - trendIndicatorWidth;

// First check if we can show the trend indicator
const shouldShowTrendIndicator =
trendIndicatorProps != null &&
percentWidth + TREND_INDICATOR_SPACING < trendIndicatorX;

// Then check if we can show formatted value without overlapping trend indicator
const availableWidthForValue = shouldShowTrendIndicator
? trendIndicatorX - TREND_INDICATOR_SPACING - BUFFER_PADDING
: targetWidth;
const shouldShowFormattedValue =
percentWidth + formattedValueWidth < availableWidthForValue &&
(!trendIndicatorProps || shouldShowTrendIndicator);
const trendIndicatorX = canShowAllTrendIndicatorsInline
? targetWidth - trendIndicatorWidth
: 0;

return (
<g
Expand Down Expand Up @@ -145,7 +183,7 @@ export function FunnelChartLabels({
fontSize={PERCENT_FONT_SIZE}
fontWeight={PERCENT_FONT_WEIGHT}
/>
{shouldShowFormattedValue && (
{canShowAllFormattedValues && (
<SingleTextLine
color={VALUE_COLOR}
text={formattedValues[index]}
Expand All @@ -156,9 +194,13 @@ export function FunnelChartLabels({
fontSize={VALUE_FONT_SIZE}
/>
)}
{shouldShowTrendIndicator && (
{trendIndicatorProps && (
<g
transform={`translate(${trendIndicatorX}, ${-LABEL_VERTICAL_OFFSET})`}
transform={`translate(${trendIndicatorX}, ${
canShowAllTrendIndicatorsInline
? -LABEL_VERTICAL_OFFSET
: LINE_HEIGHT
})`}
>
<TrendIndicator {...trendIndicatorProps} />
</g>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import React from 'react';
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';
import {FunnelChartLabels, LABEL_VERTICAL_OFFSET} from '../FunnelChartLabels';
import {TrendIndicator} from '../../../../TrendIndicator';
import {LINE_HEIGHT} from '../../../../../constants';
import type {MetaDataTrendIndicator} from '../../../types';

describe('<FunnelChartLabels />', () => {
const mockContext = {
Expand All @@ -21,28 +25,53 @@ describe('<FunnelChartLabels />', () => {
const mockProps = {
formattedValues: ['1,000', '750', '500'],
labels: ['Step 1', 'Step 2', 'Step 3'],
labelWidth: 150,
barWidth: 100,
labelWidth: 200,
barWidth: 150,
percentages: ['100%', '75%', '50%'],
xScale: scaleBand().domain(['0', '1', '2']).range([0, 300]),
shouldApplyScaling: false,
renderScaleIconTooltipContent: () => <div>Tooltip content</div>,
trends: [
{
reached: {
value: '10%',
trend: 'positive',
direction: 'upward',
},
},
{
reached: {
value: '20%',
trend: 'negative',
direction: 'downward',
},
},
{
reached: {
value: '30%',
trend: 'neutral',
direction: 'upward',
},
},
] as MetaDataTrendIndicator,
};

const wrapper = (props = mockProps) => {
return mount(
<ChartContext.Provider value={mockContext}>
<FunnelChartLabels {...props} />
<svg>
<FunnelChartLabels {...props} />
</svg>
,
</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);

expect(component).toContainReactComponentTimes(SingleTextLine, 9);
});

it('renders labels, percentages, and values', () => {
Expand Down Expand Up @@ -79,6 +108,60 @@ describe('<FunnelChartLabels />', () => {
text: '100%',
});
});

it('hides all formatted values when any label has insufficient space', () => {
const propsWithMixedWidths = {
...mockProps,
labelWidth: 150, // Enough space for first two labels
barWidth: 50,
};

const component = wrapper(propsWithMixedWidths);

expect(component).toContainReactComponentTimes(SingleTextLine, 6);

// Verify no formatted values are shown
expect(component).not.toContainReactComponent(SingleTextLine, {
text: '1,000',
});
expect(component).not.toContainReactComponent(SingleTextLine, {
text: '750',
});
expect(component).not.toContainReactComponent(SingleTextLine, {
text: '500',
});
});
});

describe('trend indicators', () => {
it('renders trend indicators for all labels when provided', () => {
const component = wrapper(mockProps);
expect(component.findAll(TrendIndicator)).toHaveLength(3);
});

it('positions all trend indicators inline when there is enough space', () => {
const component = wrapper({
...mockProps,
labelWidth: 200,
barWidth: 200,
});

expect(component).toContainReactComponentTimes('g', 3, {
transform: expect.stringContaining(`${-LABEL_VERTICAL_OFFSET}`),
});
});

it('positions all trend indicators below when there is not enough space', () => {
const component = wrapper({
...mockProps,
labelWidth: 50,
barWidth: 50,
});

expect(component).toContainReactComponentTimes('g', 3, {
transform: expect.stringContaining(`${LINE_HEIGHT}`),
});
});
});

describe('scale icon', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export {FunnelChartNext} from './FunnelChartNext';
export type {FunnelChartNextProps} from './FunnelChartNext';
export {getFunnelBarHeight} from './utilities/get-funnel-bar-height';

This file was deleted.

This file was deleted.

Loading

0 comments on commit 858ec75

Please sign in to comment.