refactor solution

Author: mollerjorge

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

@@ -19,6 +19,8 @@ const LABEL_FONT_SIZE = 12;
 const VALUE_FONT_SIZE = 14;
 const VALUE_FONT_WEIGHT = 650;
 const TREND_INDICATOR_SPACING = 8;
+const VERTICAL_STACK_SPACING = 3;
+const MIN_CHART_WIDTH_FOR_RULE_3_PRIORITY = 400;
 export const LABEL_VERTICAL_OFFSET = 2;
 
 const TEXT_COLOR = 'rgba(31, 33, 36, 1)';
@@ -38,6 +40,12 @@ export interface FunnelChartLabelsProps {
   renderScaleIconTooltipContent?: () => ReactNode;
 }
 
+const LAYOUT_STRATEGY = {
+  ONE_LINE_ALL: 'one_line_all',
+  ONE_LINE_COUNTS_AND_TRENDS: 'one_line_counts_and_trends',
+  VERTICAL_STACKING: 'vertical_stacking',
+} as const;
+
 export function FunnelChartLabels({
   formattedValues,
   labels,
@@ -49,28 +57,24 @@ export function FunnelChartLabels({
   shouldApplyScaling,
   renderScaleIconTooltipContent,
 }: FunnelChartLabelsProps) {
-  const {characterWidths} = useChartContext();
+  const {characterWidths, containerBounds} = useChartContext();
+  const chartContainerWidth = containerBounds?.width ?? 0;
   const [showTooltip, setShowTooltip] = useState(false);
 
   const labelFontSize = useMemo(() => {
     const maxLabelWidth = Math.max(
       ...labels.map((label) => estimateStringWidth(label, characterWidths)),
     );
-
     return maxLabelWidth > labelWidth ? REDUCED_FONT_SIZE : LABEL_FONT_SIZE;
   }, [labels, characterWidths, labelWidth]);
 
   const {layoutStrategy} = useMemo(() => {
-    let allCanFitRule1 = true;
-    let allCanFitRule2 = true;
-    let anyTrendIndicatorExists = false;
-
-    for (let i = 0; i < labels.length; i++) {
+    // Check if all items can fit in one Main Percentage, Counts, and TI (if present) on a single line.
+    const canAllItemsFitRule1 = labels.every((_, i) => {
       const isLast = i === labels.length - 1;
       const currentTargetWidth = isLast
         ? barWidth - GROUP_OFFSET * 2
         : labelWidth - GROUP_OFFSET * 2;
-
       const currentPercentWidth = estimateStringWidthWithOffset(
         percentages[i],
         VALUE_FONT_SIZE,
@@ -85,34 +89,70 @@ export function FunnelChartLabels({
         trendIndicatorWidth: currentTrendWidth,
         trendIndicatorProps: currentTrendProps,
       } = getTrendIndicatorData(trends?.[i]?.reached);
-      if (currentTrendProps) anyTrendIndicatorExists = true;
 
-      // Check Rule 1 for current item
-      const canItemFitRule1 =
+      return (
         currentPercentWidth +
           LINE_PADDING +
           currentCountStringWidth +
           (currentTrendProps
             ? TREND_INDICATOR_SPACING + currentTrendWidth
             : 0) <
-        currentTargetWidth;
+        currentTargetWidth
+      );
+    });
+
+    if (canAllItemsFitRule1) {
+      // All elements on one line
+      return {layoutStrategy: LAYOUT_STRATEGY.ONE_LINE_ALL};
+    }
 
-      if (!canItemFitRule1) allCanFitRule1 = false;
+    // If chart width is very narrow, prioritize full stacking.
+    if (chartContainerWidth < MIN_CHART_WIDTH_FOR_RULE_3_PRIORITY) {
+      return {layoutStrategy: LAYOUT_STRATEGY.VERTICAL_STACKING};
+    }
+
+    // Check if all items can fit Rule 2: Main Percentage (L1), Counts + TI (L2, side-by-side with space).
+    const canAllItemsFitRule2 = labels.every((_, i) => {
+      const isLast = i === labels.length - 1;
+      const currentTargetWidth = isLast
+        ? barWidth - GROUP_OFFSET * 2
+        : labelWidth - GROUP_OFFSET * 2;
+      const currentCountStringWidth = estimateStringWidthWithOffset(
+        formattedValues[i],
+        VALUE_FONT_SIZE,
+        VALUE_FONT_WEIGHT,
+      );
+      const {
+        trendIndicatorWidth: currentTrendWidth,
+        trendIndicatorProps: currentTrendProps,
+      } = getTrendIndicatorData(trends?.[i]?.reached);
 
-      // Check Rule 2 for current item (only relevant if Rule 1 might fail for this item or others)
-      const canItemFitRule2 =
-        currentCountStringWidth + (currentTrendProps ? currentTrendWidth : 0) < // No TREND_INDICATOR_SPACING for Rule 2 adjacency
-        currentTargetWidth; // Rule 2 checks fit on a *new line*, so full currentTargetWidth is available
+      return (
+        currentCountStringWidth +
+          (currentTrendProps
+            ? TREND_INDICATOR_SPACING + currentTrendWidth
+            : 0) <
+        currentTargetWidth
+      );
+    });
 
-      if (!canItemFitRule2) allCanFitRule2 = false;
+    if (canAllItemsFitRule2) {
+      // Main% (L1), Counts + TI (L2, side-by-side with space)
+      return {layoutStrategy: LAYOUT_STRATEGY.ONE_LINE_COUNTS_AND_TRENDS};
     }
 
-    if (allCanFitRule1) return {layoutStrategy: 'rule1'};
-    if (allCanFitRule2) return {layoutStrategy: 'rule2'};
-    // If neither Rule 1 nor Rule 2 can be applied globally
-    // Rule 3: MainPerc on L1, Counts on L2, TI on L3 (if exists)
-    return {layoutStrategy: 'rule3plusCounts'};
-  }, [labels, percentages, formattedValues, trends, labelWidth, barWidth]);
+    // Fall back to vertical stacking.
+    // Main% (L1), Counts (L2), TI (L3)
+    return {layoutStrategy: LAYOUT_STRATEGY.VERTICAL_STACKING};
+  }, [
+    labels,
+    percentages,
+    formattedValues,
+    trends,
+    labelWidth,
+    barWidth,
+    chartContainerWidth,
+  ]);
 
   return (
     <Fragment>
@@ -172,20 +212,23 @@ export function FunnelChartLabels({
               x={showScaleIcon ? 20 : 0}
             />
 
+            {/* Group for Main Percentage, Counts, and Trend Indicator */}
             <g transform={`translate(0,${LINE_HEIGHT + LINE_GAP})`}>
               <SingleTextLine
                 color={TEXT_COLOR}
                 text={percentages[index]}
                 targetWidth={
-                  layoutStrategy === 'rule1' ? percentWidth : currentTargetWidth
+                  layoutStrategy === LAYOUT_STRATEGY.ONE_LINE_ALL
+                    ? percentWidth
+                    : currentTargetWidth
                 }
                 textAnchor="start"
                 fontSize={VALUE_FONT_SIZE}
                 fontWeight={VALUE_FONT_WEIGHT}
               />
 
               {(() => {
-                if (layoutStrategy === 'rule1') {
+                if (layoutStrategy === LAYOUT_STRATEGY.ONE_LINE_ALL) {
                   return (
                     <Fragment>
                       <SingleTextLine
@@ -214,9 +257,15 @@ export function FunnelChartLabels({
                       )}
                     </Fragment>
                   );
-                } else if (layoutStrategy === 'rule2') {
+                } else if (
+                  layoutStrategy === LAYOUT_STRATEGY.ONE_LINE_COUNTS_AND_TRENDS
+                ) {
                   return (
-                    <g transform={`translate(0, ${LINE_HEIGHT})`}>
+                    <g
+                      transform={`translate(0, ${
+                        LINE_HEIGHT + VERTICAL_STACK_SPACING
+                      })`}
+                    >
                       <SingleTextLine
                         color={VALUE_COLOR}
                         text={formattedValues[index]}
@@ -232,17 +281,25 @@ export function FunnelChartLabels({
                       />
                       {trendIndicatorProps && (
                         <g
-                          transform={`translate(${countStringWidth}, ${-LABEL_VERTICAL_OFFSET})`}
+                          transform={`translate(${
+                            countStringWidth + TREND_INDICATOR_SPACING
+                          }, ${-LABEL_VERTICAL_OFFSET})`}
                         >
                           <TrendIndicator {...trendIndicatorProps} />
                         </g>
                       )}
                     </g>
                   );
-                } else if (layoutStrategy === 'rule3plusCounts') {
+                } else if (
+                  layoutStrategy === LAYOUT_STRATEGY.VERTICAL_STACKING
+                ) {
                   return (
                     <Fragment>
-                      <g transform={`translate(0, ${LINE_HEIGHT})`}>
+                      <g
+                        transform={`translate(0, ${
+                          LINE_HEIGHT + VERTICAL_STACK_SPACING
+                        })`}
+                      >
                         <SingleTextLine
                           color={VALUE_COLOR}
                           text={formattedValues[index]}
@@ -254,7 +311,11 @@ export function FunnelChartLabels({
                         />
                       </g>
                       {trendIndicatorProps && (
-                        <g transform={`translate(0, ${LINE_HEIGHT * 2})`}>
+                        <g
+                          transform={`translate(0, ${
+                            LINE_HEIGHT * 2 + VERTICAL_STACK_SPACING * 2
+                          })`}
+                        >
                           <g
                             transform={`translate(0, ${-LABEL_VERTICAL_OFFSET})`}
                           >

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

@@ -98,7 +98,7 @@ describe('<FunnelChartLabels />', () => {
       const component = wrapper(propsWithNarrowWidth);
       const texts = component.findAll(SingleTextLine);
 
-      expect(texts).toHaveLength(6);
+      expect(texts).toHaveLength(9);
 
       // Verify labels and percentages are still shown
       expect(component).toContainReactComponent(SingleTextLine, {
@@ -159,7 +159,7 @@ describe('<FunnelChartLabels />', () => {
         barWidth: 50,
       });
 
-      expect(component).toContainReactComponentTimes('g', 3, {
+      expect(component).toContainReactComponentTimes('g', 0, {
         transform: expect.stringContaining(`${LINE_HEIGHT}`),
       });
     });