Address initial comments

Shopify · Dec 13, 2024 · c76072e · c76072e
1 parent 2e92b22
commit c76072e
Show file tree

Hide file tree

Showing 15 changed files with 265 additions and 109 deletions.
diff --git a/packages/polaris-viz-core/src/constants.ts b/packages/polaris-viz-core/src/constants.ts
@@ -11,7 +11,6 @@ export const SMALL_CHART_HEIGHT = 125;
 export const FONT_SIZE = 11;
 export const TOUCH_FONT_SIZE = 12;
 
-export const FONT_WEIGHT = 300;
 export const FONT_FAMILY =
   'Inter, -apple-system, "system-ui", "San Francisco", "Segoe UI", Roboto, "Helvetica Neue", sans-serif';
 

diff --git a/packages/polaris-viz-core/src/index.ts b/packages/polaris-viz-core/src/index.ts
@@ -12,7 +12,6 @@ export {
   EMPTY_STATE_CHART_MAX,
   EMPTY_STATE_CHART_MIN,
   FONT_SIZE,
-  FONT_WEIGHT,
   HORIZONTAL_BAR_LABEL_HEIGHT,
   HORIZONTAL_BAR_LABEL_OFFSET,
   HORIZONTAL_GROUP_LABEL_HEIGHT,

diff --git a/packages/polaris-viz/src/components/FunnelChartNext/Chart.tsx b/packages/polaris-viz/src/components/FunnelChartNext/Chart.tsx
@@ -11,11 +11,8 @@ import {
   LinearGradientWithStops,
   useChartContext,
 } from '@shopify/polaris-viz-core';
-import {createPortal} from 'react-dom';
 
-import {TOOLTIP_ID} from '../../constants';
 import {useFunnelBarScaling} from '../../hooks';
-import {useRootContainer} from '../../hooks/useRootContainer';
 import {
   FunnelChartConnector,
   FunnelChartConnectorGradient,
@@ -24,7 +21,12 @@ import {FunnelChartSegment} from '../shared';
 import {SingleTextLine} from '../Labels';
 import {ChartElements} from '../ChartElements';
 
-import {FunnelChartXAxisLabels, Tooltip, FunnelTooltip} from './components';
+import {
+  FunnelChartXAxisLabels,
+  Tooltip,
+  FunnelTooltip,
+  TooltipWithPortal,
+} from './components';
 import type {FunnelChartNextProps} from './FunnelChartNext';
 import {
   TOOLTIP_WIDTH,
@@ -37,6 +39,8 @@ import {
   GAP,
   SHORT_TOOLTIP_HEIGHT,
   TOOLTIP_HEIGHT,
+  SEGMENT_WIDTH_RATIO,
+  TOOLTIP_HORIZONTAL_OFFSET,
 } from './constants';
 
 export interface ChartProps {
@@ -45,6 +49,7 @@ export interface ChartProps {
   xAxisOptions: Required<XAxisOptions>;
   yAxisOptions: Required<YAxisOptions>;
   enableScaling: boolean;
+  renderScaleIconTooltipContent?: () => ReactNode;
 }
 
 export function Chart({
@@ -53,6 +58,7 @@ export function Chart({
   xAxisOptions,
   yAxisOptions,
   enableScaling,
+  renderScaleIconTooltipContent,
 }: ChartProps) {
   const [tooltipIndex, setTooltipIndex] = useState<number | null>(null);
   const {containerBounds} = useChartContext();
@@ -96,7 +102,7 @@ export function Chart({
     .domain(labels.map((_, index) => index.toString()));
 
   const sectionWidth = xScale.bandwidth();
-  const barWidth = sectionWidth * 0.75;
+  const barWidth = sectionWidth * SEGMENT_WIDTH_RATIO;
   const lineGradientId = useMemo(() => uniqueId('line-gradient'), []);
 
   const lastPoint = dataSeries.at(-1);
@@ -162,6 +168,7 @@ export function Chart({
               percentages={percentages}
               xScale={labelXScale}
               shouldApplyScaling={shouldApplyScaling}
+              renderScaleIconTooltipContent={renderScaleIconTooltipContent}
             />
           </g>
         )}
@@ -196,7 +203,7 @@ export function Chart({
                       height={drawableHeight}
                       index={index}
                       nextX={
-                        (xScale(nextPoint?.key as string) ?? 0) - LINE_OFFSET
+                        (xScale(nextPoint?.key.toString()) ?? 0) - LINE_OFFSET
                       }
                       nextY={drawableHeight - nextBarHeight}
                       startX={x + barWidth + GAP}
@@ -256,11 +263,9 @@ export function Chart({
 
     function getXPosition() {
       if (tooltipIndex === 0) {
-        // Push the tooltip beside the bar
-        return chartX + barWidth + 10;
+        return chartX + barWidth + TOOLTIP_HORIZONTAL_OFFSET;
       }
 
-      // Center the tooltip over the bar
       const xOffset = (barWidth - TOOLTIP_WIDTH) / 2;
       return chartX + (xScale(activeDataSeries.key.toString()) ?? 0) + xOffset;
     }
@@ -281,9 +286,3 @@ export function Chart({
     return `${yAxisOptions.labelFormatter(isNaN(value) ? 0 : value)}%`;
   }
 }
-
-function TooltipWithPortal({children}: {children: ReactNode}) {
-  const container = useRootContainer(TOOLTIP_ID);
-
-  return createPortal(children, container);
-}
diff --git a/packages/polaris-viz/src/components/FunnelChartNext/FunnelChartNext.tsx b/packages/polaris-viz/src/components/FunnelChartNext/FunnelChartNext.tsx
@@ -8,6 +8,7 @@ import {
   ChartState,
   useChartContext,
 } from '@shopify/polaris-viz-core';
+import type {ReactNode} from 'react';
 
 import {ChartContainer} from '../../components/ChartContainer';
 import {
@@ -26,6 +27,7 @@ export type FunnelChartNextProps = {
   xAxisOptions?: Pick<XAxisOptions, 'hide'>;
   yAxisOptions?: Pick<YAxisOptions, 'labelFormatter'>;
   enableScaling?: boolean;
+  renderScaleIconTooltipContent?: () => ReactNode;
 } & ChartProps;
 
 export function FunnelChartNext(props: FunnelChartNextProps) {
@@ -41,8 +43,9 @@ export function FunnelChartNext(props: FunnelChartNextProps) {
     isAnimated,
     state,
     errorText,
-    onError,
     tooltipLabels,
+    onError,
+    renderScaleIconTooltipContent,
   } = {
     ...DEFAULT_CHART_PROPS,
     ...props,
@@ -76,6 +79,7 @@ export function FunnelChartNext(props: FunnelChartNextProps) {
           xAxisOptions={xAxisOptionsForChart}
           yAxisOptions={yAxisOptionsForChart}
           enableScaling={enableScaling}
+          renderScaleIconTooltipContent={renderScaleIconTooltipContent}
         />
       )}
     </ChartContainer>

diff --git a/packages/polaris-viz/src/components/FunnelChartNext/components/FunnelChartXAxisLabels.tsx b/packages/polaris-viz/src/components/FunnelChartNext/components/FunnelChartXAxisLabels.tsx
@@ -1,4 +1,5 @@
-import {Fragment, useMemo} from 'react';
+import type {ReactNode} from 'react';
+import {Fragment, useMemo, useState} from 'react';
 import type {ScaleBand} from 'd3-scale';
 import {estimateStringWidth, useChartContext} from '@shopify/polaris-viz-core';
 
@@ -7,6 +8,7 @@ import {estimateStringWidthWithOffset} from '../../../utilities';
 import {SingleTextLine} from '../../Labels';
 
 import {ScaleIcon} from './ScaleIcon';
+import {ScaleIconTooltip} from './ScaleIconTooltip';
 
 const LINE_GAP = 5;
 const LINE_PADDING = 10;
@@ -28,6 +30,7 @@ export interface FunnelChartXAxisLabelsProps {
   percentages: string[];
   xScale: ScaleBand<string>;
   shouldApplyScaling: boolean;
+  renderScaleIconTooltipContent?: () => ReactNode;
 }
 
 export function FunnelChartXAxisLabels({
@@ -37,17 +40,17 @@ export function FunnelChartXAxisLabels({
   percentages,
   xScale,
   shouldApplyScaling,
+  renderScaleIconTooltipContent,
 }: FunnelChartXAxisLabelsProps) {
   const {characterWidths} = useChartContext();
+  const [showTooltip, setShowTooltip] = useState(false);
   const targetWidth = labelWidth - GROUP_OFFSET * 3;
 
   const labelFontSize = useMemo(() => {
-    // Find the widest label
     const maxLabelWidth = Math.max(
       ...labels.map((label) => estimateStringWidth(label, characterWidths)),
     );
 
-    // If any label is too wide, reduce font size for all
     return maxLabelWidth > labelWidth ? REDUCED_FONT_SIZE : LABEL_FONT_SIZE;
   }, [labels, characterWidths, labelWidth]);
 
@@ -80,8 +83,19 @@ export function FunnelChartXAxisLabels({
             key={index}
           >
             {showScaleIcon && (
-              <g transform="translate(0, -3)">
+              <g
+                transform="translate(0, -3)"
+                onMouseEnter={() => setShowTooltip(true)}
+                onMouseLeave={() => setShowTooltip(false)}
+              >
                 <ScaleIcon />
+                {showTooltip && renderScaleIconTooltipContent && (
+                  <ScaleIconTooltip
+                    renderScaleIconTooltipContent={
+                      renderScaleIconTooltipContent
+                    }
+                  />
+                )}
               </g>
             )}
             <SingleTextLine

diff --git a/packages/polaris-viz/src/components/FunnelChartNext/components/ScaleIconTooltip.tsx b/packages/polaris-viz/src/components/FunnelChartNext/components/ScaleIconTooltip.tsx
@@ -0,0 +1,36 @@
+import {DEFAULT_THEME_NAME, useChartContext} from '@shopify/polaris-viz-core';
+import type {ReactNode} from 'react';
+import {Fragment} from 'react';
+
+import {FunnelTooltip, TooltipWithPortal} from '../components';
+import {TooltipContentContainer} from '../../../components/TooltipContent';
+
+const TOOLTIP_VERTICAL_OFFSET = 65;
+
+interface ScaleIconTooltipProps {
+  renderScaleIconTooltipContent: () => ReactNode;
+}
+
+export function ScaleIconTooltip({
+  renderScaleIconTooltipContent,
+}: ScaleIconTooltipProps) {
+  const {containerBounds} = useChartContext();
+  const {x, y} = containerBounds ?? {
+    x: 0,
+    y: 0,
+  };
+
+  return (
+    <TooltipWithPortal>
+      <FunnelTooltip x={x} y={y - TOOLTIP_VERTICAL_OFFSET}>
+        <TooltipContentContainer
+          maxWidth={200}
+          theme={DEFAULT_THEME_NAME}
+          color="white"
+        >
+          {() => <Fragment>{renderScaleIconTooltipContent()}</Fragment>}
+        </TooltipContentContainer>
+      </FunnelTooltip>
+    </TooltipWithPortal>
+  );
+}
diff --git a/packages/polaris-viz/src/components/FunnelChartNext/components/TooltipWithPortal.tsx b/packages/polaris-viz/src/components/FunnelChartNext/components/TooltipWithPortal.tsx
@@ -0,0 +1,11 @@
+import {createPortal} from 'react-dom';
+import type {ReactNode} from 'react';
+
+import {useRootContainer} from '../../../hooks/useRootContainer';
+import {TOOLTIP_ID} from '../../../constants';
+
+export function TooltipWithPortal({children}: {children: ReactNode}) {
+  const container = useRootContainer(TOOLTIP_ID);
+
+  return createPortal(children, container);
+}
diff --git a/packages/polaris-viz/src/components/FunnelChartNext/components/index.ts b/packages/polaris-viz/src/components/FunnelChartNext/components/index.ts
@@ -1,3 +1,4 @@
 export {FunnelChartXAxisLabels} from './FunnelChartXAxisLabels';
 export {Tooltip} from './Tooltip';
 export {FunnelTooltip} from './FunnelTooltip';
+export {TooltipWithPortal} from './TooltipWithPortal';
diff --git a/packages/polaris-viz/src/components/FunnelChartNext/constants.ts b/packages/polaris-viz/src/components/FunnelChartNext/constants.ts
@@ -1,6 +1,7 @@
 export const FUNNEL_CONNECTOR_Y_OFFSET = 30;
 export const TOOLTIP_WIDTH = 250;
-
+export const SEGMENT_WIDTH_RATIO = 0.75;
+export const TOOLTIP_HORIZONTAL_OFFSET = 10;
 export const LINE_OFFSET = 3;
 export const LINE_WIDTH = 1;
 export const TOOLTIP_HEIGHT = 90;

diff --git a/packages/polaris-viz/src/components/FunnelChartNext/stories/Default.stories.tsx b/packages/polaris-viz/src/components/FunnelChartNext/stories/Default.stories.tsx
@@ -5,6 +5,7 @@ export {META as default} from './meta';
 import type {FunnelChartNextProps} from '../FunnelChartNext';
 
 import {DEFAULT_DATA, Template} from './data';
+import {Fragment} from 'react';
 
 export const Default: Story<FunnelChartNextProps> = Template.bind({});
 
@@ -24,5 +25,12 @@ Default.args = {
     reached: 'Reached this step',
     dropped: 'Dropped off',
   },
-  enableScaling: true,
+  renderScaleIconTooltipContent: () => (
+    <Fragment>
+      <div>Truncated Sessions</div>{' '}
+      <p style={{color: 'black', fontSize: '12px', lineHeight: '12px'}}>
+        Sessions were drawn to scale to better represent the funnel
+      </p>
+    </Fragment>
+  ),
 };
diff --git a/packages/polaris-viz/src/components/FunnelChartNext/stories/data.tsx b/packages/polaris-viz/src/components/FunnelChartNext/stories/data.tsx
@@ -32,7 +32,7 @@ export const Template: Story<FunnelChartNextProps> = (
   args: FunnelChartNextProps,
 ) => {
   return (
-    <div style={{height: 400}}>
+    <div style={{height: 400, marginTop: 20}}>
       <FunnelChartNext {...args} />
     </div>
   );

diff --git a/packages/polaris-viz/src/components/Labels/SingleTextLine.tsx b/packages/polaris-viz/src/components/Labels/SingleTextLine.tsx
@@ -1,7 +1,6 @@
 import {Fragment} from 'react';
 import {
   FONT_FAMILY,
-  FONT_WEIGHT,
   LINE_HEIGHT,
   useChartContext,
 } from '@shopify/polaris-viz-core';
@@ -21,12 +20,13 @@ interface SingleTextLineProps {
   textAnchor?: 'start' | 'middle' | 'end';
 }
 
+const DEFAULT_LABEL_FONT_WEIGHT = 400;
 export function SingleTextLine({
   ariaHidden = false,
   color,
   dominantBaseline = 'hanging',
   fontSize,
-  fontWeight = FONT_WEIGHT,
+  fontWeight = DEFAULT_LABEL_FONT_WEIGHT,
   targetWidth,
   text,
   textAnchor = 'middle',

diff --git a/.../components/TooltipContent/components/TooltipContentContainer/TooltipContentContainer.tsx b/.../components/TooltipContent/components/TooltipContentContainer/TooltipContentContainer.tsx
@@ -21,9 +21,15 @@ interface Props {
   }) => ReactNode;
   maxWidth: number;
   theme: string;
+  color?: string;
 }
 
-export function TooltipContentContainer({children, maxWidth, theme}: Props) {
+export function TooltipContentContainer({
+  children,
+  maxWidth,
+  theme,
+  color,
+}: Props) {
   const {isFirefox} = useBrowserCheck();
 
   const selectedTheme = useTheme(theme);
@@ -39,10 +45,12 @@ export function TooltipContentContainer({children, maxWidth, theme}: Props) {
     <div
       className={styles.Container}
       style={{
-        background: changeColorOpacity(
-          selectedTheme.tooltip.backgroundColor,
-          isFirefox ? 1 : TOOLTIP_BG_OPACITY,
-        ),
+        background: color
+          ? color
+          : changeColorOpacity(
+              selectedTheme.tooltip.backgroundColor,
+              isFirefox ? 1 : TOOLTIP_BG_OPACITY,
+            ),
         maxWidth,
         fontFamily: FONT_FAMILY,
       }}

diff --git a/packages/polaris-viz/src/components/shared/FunnelChartConnector/FunnelChartConnector.tsx b/packages/polaris-viz/src/components/shared/FunnelChartConnector/FunnelChartConnector.tsx
@@ -26,7 +26,7 @@ export function FunnelChartConnector({
   startY,
 }: ConnectorProps) {
   const springConfig = useBarSpringConfig({
-    animationDelay: index * ANIMATION_DELAY,
+    animationDelay: (index || 1) * ANIMATION_DELAY,
   });
 
   const {animatedStartY, animatedNextY} = useSpring({