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

Commit b6449e7

Browse files
mollerjorgemollerjorge
committed
refactor solution
1 parent a873470 commit b6449e7

File tree

2 files changed

+93
-58
lines changed

2 files changed

+93
-58
lines changed

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

Lines changed: 91 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ const LABEL_FONT_SIZE = 12;
1919
const VALUE_FONT_SIZE = 14;
2020
const VALUE_FONT_WEIGHT = 650;
2121
const TREND_INDICATOR_SPACING = 8;
22+
const VERTICAL_STACK_SPACING = 3;
23+
const MIN_CHART_WIDTH_FOR_RULE_3_PRIORITY = 400;
2224
export const LABEL_VERTICAL_OFFSET = 2;
2325

2426
const TEXT_COLOR = 'rgba(31, 33, 36, 1)';
@@ -38,6 +40,12 @@ export interface FunnelChartLabelsProps {
3840
renderScaleIconTooltipContent?: () => ReactNode;
3941
}
4042

43+
const LAYOUT_STRATEGY = {
44+
ONE_LINE_ALL: 'one_line_all',
45+
ONE_LINE_COUNTS_AND_TRENDS: 'one_line_counts_and_trends',
46+
VERTICAL_STACKING: 'vertical_stacking',
47+
} as const;
48+
4149
export function FunnelChartLabels({
4250
formattedValues,
4351
labels,
@@ -49,28 +57,24 @@ export function FunnelChartLabels({
4957
shouldApplyScaling,
5058
renderScaleIconTooltipContent,
5159
}: FunnelChartLabelsProps) {
52-
const {characterWidths} = useChartContext();
60+
const {characterWidths, containerBounds} = useChartContext();
61+
const chartContainerWidth = containerBounds?.width ?? 0;
5362
const [showTooltip, setShowTooltip] = useState(false);
5463

5564
const labelFontSize = useMemo(() => {
5665
const maxLabelWidth = Math.max(
5766
...labels.map((label) => estimateStringWidth(label, characterWidths)),
5867
);
59-
6068
return maxLabelWidth > labelWidth ? REDUCED_FONT_SIZE : LABEL_FONT_SIZE;
6169
}, [labels, characterWidths, labelWidth]);
6270

6371
const {layoutStrategy} = useMemo(() => {
64-
let allCanFitRule1 = true;
65-
let allCanFitRule2 = true;
66-
let anyTrendIndicatorExists = false;
67-
68-
for (let i = 0; i < labels.length; i++) {
72+
// Check if all items can fit in one Main Percentage, Counts, and TI (if present) on a single line.
73+
const canAllFitInOneLine = labels.every((_, i) => {
6974
const isLast = i === labels.length - 1;
7075
const currentTargetWidth = isLast
7176
? barWidth - GROUP_OFFSET * 2
7277
: labelWidth - GROUP_OFFSET * 2;
73-
7478
const currentPercentWidth = estimateStringWidthWithOffset(
7579
percentages[i],
7680
VALUE_FONT_SIZE,
@@ -85,34 +89,68 @@ export function FunnelChartLabels({
8589
trendIndicatorWidth: currentTrendWidth,
8690
trendIndicatorProps: currentTrendProps,
8791
} = getTrendIndicatorData(trends?.[i]?.reached);
88-
if (currentTrendProps) anyTrendIndicatorExists = true;
8992

90-
// Check Rule 1 for current item
91-
const canItemFitRule1 =
93+
return (
9294
currentPercentWidth +
9395
LINE_PADDING +
9496
currentCountStringWidth +
9597
(currentTrendProps
9698
? TREND_INDICATOR_SPACING + currentTrendWidth
9799
: 0) <
98-
currentTargetWidth;
100+
currentTargetWidth
101+
);
102+
});
103+
104+
if (canAllFitInOneLine) {
105+
// All elements in one line
106+
return {layoutStrategy: LAYOUT_STRATEGY.ONE_LINE_ALL};
107+
}
99108

100-
if (!canItemFitRule1) allCanFitRule1 = false;
109+
// If chart width is very narrow, prioritize full stacking.
110+
if (chartContainerWidth < MIN_CHART_WIDTH_FOR_RULE_3_PRIORITY) {
111+
return {layoutStrategy: LAYOUT_STRATEGY.VERTICAL_STACKING};
112+
}
113+
114+
// Check if counts and trends can fit in one line.
115+
const canCountsAndTrendsFitOneLine = labels.every((_, i) => {
116+
const isLast = i === labels.length - 1;
117+
const currentTargetWidth = isLast
118+
? barWidth - GROUP_OFFSET * 2
119+
: labelWidth - GROUP_OFFSET * 2;
120+
const currentCountStringWidth = estimateStringWidthWithOffset(
121+
formattedValues[i],
122+
VALUE_FONT_SIZE,
123+
VALUE_FONT_WEIGHT,
124+
);
125+
const {
126+
trendIndicatorWidth: currentTrendWidth,
127+
trendIndicatorProps: currentTrendProps,
128+
} = getTrendIndicatorData(trends?.[i]?.reached);
101129

102-
// Check Rule 2 for current item (only relevant if Rule 1 might fail for this item or others)
103-
const canItemFitRule2 =
104-
currentCountStringWidth + (currentTrendProps ? currentTrendWidth : 0) < // No TREND_INDICATOR_SPACING for Rule 2 adjacency
105-
currentTargetWidth; // Rule 2 checks fit on a *new line*, so full currentTargetWidth is available
130+
return (
131+
currentCountStringWidth +
132+
(currentTrendProps
133+
? TREND_INDICATOR_SPACING + currentTrendWidth
134+
: 0) <
135+
currentTargetWidth
136+
);
137+
});
106138

107-
if (!canItemFitRule2) allCanFitRule2 = false;
139+
if (canCountsAndTrendsFitOneLine) {
140+
return {layoutStrategy: LAYOUT_STRATEGY.ONE_LINE_COUNTS_AND_TRENDS};
108141
}
109142

110-
if (allCanFitRule1) return {layoutStrategy: 'rule1'};
111-
if (allCanFitRule2) return {layoutStrategy: 'rule2'};
112-
// If neither Rule 1 nor Rule 2 can be applied globally
113-
// Rule 3: MainPerc on L1, Counts on L2, TI on L3 (if exists)
114-
return {layoutStrategy: 'rule3plusCounts'};
115-
}, [labels, percentages, formattedValues, trends, labelWidth, barWidth]);
143+
// Fall back to vertical stacking.
144+
return {layoutStrategy: LAYOUT_STRATEGY.VERTICAL_STACKING};
145+
}, [
146+
labels,
147+
percentages,
148+
formattedValues,
149+
trends,
150+
labelWidth,
151+
barWidth,
152+
chartContainerWidth,
153+
]);
116154

117155
return (
118156
<Fragment>
@@ -172,20 +210,23 @@ export function FunnelChartLabels({
172210
x={showScaleIcon ? 20 : 0}
173211
/>
174212

213+
{/* Group for Main Percentage, Counts, and Trend Indicator */}
175214
<g transform={`translate(0,${LINE_HEIGHT + LINE_GAP})`}>
176215
<SingleTextLine
177216
color={TEXT_COLOR}
178217
text={percentages[index]}
179218
targetWidth={
180-
layoutStrategy === 'rule1' ? percentWidth : currentTargetWidth
219+
layoutStrategy === LAYOUT_STRATEGY.ONE_LINE_ALL
220+
? percentWidth
221+
: currentTargetWidth
181222
}
182223
textAnchor="start"
183224
fontSize={VALUE_FONT_SIZE}
184225
fontWeight={VALUE_FONT_WEIGHT}
185226
/>
186227

187228
{(() => {
188-
if (layoutStrategy === 'rule1') {
229+
if (layoutStrategy === LAYOUT_STRATEGY.ONE_LINE_ALL) {
189230
return (
190231
<Fragment>
191232
<SingleTextLine
@@ -214,9 +255,15 @@ export function FunnelChartLabels({
214255
)}
215256
</Fragment>
216257
);
217-
} else if (layoutStrategy === 'rule2') {
258+
} else if (
259+
layoutStrategy === LAYOUT_STRATEGY.ONE_LINE_COUNTS_AND_TRENDS
260+
) {
218261
return (
219-
<g transform={`translate(0, ${LINE_HEIGHT})`}>
262+
<g
263+
transform={`translate(0, ${
264+
LINE_HEIGHT + VERTICAL_STACK_SPACING
265+
})`}
266+
>
220267
<SingleTextLine
221268
color={VALUE_COLOR}
222269
text={formattedValues[index]}
@@ -232,17 +279,25 @@ export function FunnelChartLabels({
232279
/>
233280
{trendIndicatorProps && (
234281
<g
235-
transform={`translate(${countStringWidth}, ${-LABEL_VERTICAL_OFFSET})`}
282+
transform={`translate(${
283+
countStringWidth + TREND_INDICATOR_SPACING
284+
}, ${-LABEL_VERTICAL_OFFSET})`}
236285
>
237286
<TrendIndicator {...trendIndicatorProps} />
238287
</g>
239288
)}
240289
</g>
241290
);
242-
} else if (layoutStrategy === 'rule3plusCounts') {
291+
} else if (
292+
layoutStrategy === LAYOUT_STRATEGY.VERTICAL_STACKING
293+
) {
243294
return (
244295
<Fragment>
245-
<g transform={`translate(0, ${LINE_HEIGHT})`}>
296+
<g
297+
transform={`translate(0, ${
298+
LINE_HEIGHT + VERTICAL_STACK_SPACING
299+
})`}
300+
>
246301
<SingleTextLine
247302
color={VALUE_COLOR}
248303
text={formattedValues[index]}
@@ -254,7 +309,11 @@ export function FunnelChartLabels({
254309
/>
255310
</g>
256311
{trendIndicatorProps && (
257-
<g transform={`translate(0, ${LINE_HEIGHT * 2})`}>
312+
<g
313+
transform={`translate(0, ${
314+
LINE_HEIGHT * 2 + VERTICAL_STACK_SPACING * 2
315+
})`}
316+
>
258317
<g
259318
transform={`translate(0, ${-LABEL_VERTICAL_OFFSET})`}
260319
>

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

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ describe('<FunnelChartLabels />', () => {
9898
const component = wrapper(propsWithNarrowWidth);
9999
const texts = component.findAll(SingleTextLine);
100100

101-
expect(texts).toHaveLength(6);
101+
expect(texts).toHaveLength(9);
102102

103103
// Verify labels and percentages are still shown
104104
expect(component).toContainReactComponent(SingleTextLine, {
@@ -108,30 +108,6 @@ describe('<FunnelChartLabels />', () => {
108108
text: '100%',
109109
});
110110
});
111-
112-
it('hides all formatted values when any label has insufficient space', () => {
113-
const propsWithMixedWidths = {
114-
...mockProps,
115-
// Enough space for first two labels
116-
labelWidth: 150,
117-
barWidth: 50,
118-
};
119-
120-
const component = wrapper(propsWithMixedWidths);
121-
122-
expect(component).toContainReactComponentTimes(SingleTextLine, 6);
123-
124-
// Verify no formatted values are shown
125-
expect(component).not.toContainReactComponent(SingleTextLine, {
126-
text: '1,000',
127-
});
128-
expect(component).not.toContainReactComponent(SingleTextLine, {
129-
text: '750',
130-
});
131-
expect(component).not.toContainReactComponent(SingleTextLine, {
132-
text: '500',
133-
});
134-
});
135111
});
136112

137113
describe('trend indicators', () => {
@@ -159,7 +135,7 @@ describe('<FunnelChartLabels />', () => {
159135
barWidth: 50,
160136
});
161137

162-
expect(component).toContainReactComponentTimes('g', 3, {
138+
expect(component).toContainReactComponentTimes('g', 0, {
163139
transform: expect.stringContaining(`${LINE_HEIGHT}`),
164140
});
165141
});

0 commit comments

Comments
 (0)