Address pr feedback

Shopify · Nov 20, 2024 · 9608735 · 9608735
1 parent 3f35f46
commit 9608735
Show file tree

Hide file tree

Showing 6 changed files with 201 additions and 99 deletions.
diff --git a/packages/polaris-viz/CHANGELOG.md b/packages/polaris-viz/CHANGELOG.md
@@ -7,6 +7,11 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
 
+### Fixed
+
+- Tooltip positioning when the grid is inside a container with overflow: hidden.
+- Tooltip positioning when the grid has multiple instances.
+
 ### Changed
 
 - Changed `TOUCH_FONT_SIZE` constant to `12px`.

diff --git a/packages/polaris-viz/src/components/Grid/Grid.tsx b/packages/polaris-viz/src/components/Grid/Grid.tsx
@@ -1,15 +1,9 @@
-import React, {
-  useCallback,
-  useEffect,
-  useLayoutEffect,
-  useMemo,
-  useRef,
-  useState,
-} from 'react';
+import React, {useCallback, useEffect, useMemo, useState} from 'react';
 import type {LabelFormatter} from '@shopify/polaris-viz-core';
 import {
   DEFAULT_CHART_PROPS,
   useChartPositions,
+  useUniqueId,
 } from '@shopify/polaris-viz-core';
 import {scaleLinear} from 'd3-scale';
 
@@ -26,7 +20,6 @@ import {
   TOOLTIP_WIDTH,
   TOOLTIP_HEIGHT,
   TOOLTIP_HORIZONTAL_OFFSET,
-  TOOLTIP_VERTICAL_OFFSET,
   Y_AXIS_LABEL_WIDTH,
   X_AXIS_HEIGHT,
   DEFAULT_GROUP_COLOR,
@@ -61,15 +54,6 @@ export function Grid(props: GridProps) {
   const [isSmallContainer, setIsSmallContainer] = useState(false);
   const [groupSelected, setGroupSelected] = useState<CellGroup | null>(null);
 
-  const tooltipRef = useRef<HTMLDivElement>(null);
-  const [tooltipDimensions, setTooltipDimensions] = useState<{
-    width: number;
-    height: number;
-  }>({
-    width: TOOLTIP_WIDTH,
-    height: TOOLTIP_HEIGHT,
-  });
-
   const {
     cellGroups = [],
     xAxisOptions = {},
@@ -88,11 +72,23 @@ export function Grid(props: GridProps) {
     [entry],
   );
 
+  const gridId = useUniqueId('grid');
+
+  const uniqueGroups = useMemo(() => {
+    return cellGroups.map((group) => ({
+      ...group,
+      id: `${group.id}-${gridId}`,
+      connectedGroups: group.connectedGroups?.map(
+        (connectedId) => `${connectedId}-${gridId}`,
+      ),
+    }));
+  }, [cellGroups, gridId]);
+
   const gridDimensions = useMemo(() => {
-    const maxRow = Math.max(...cellGroups.map((group) => group.end.row)) + 1;
-    const maxCol = Math.max(...cellGroups.map((group) => group.end.col)) + 1;
+    const maxRow = Math.max(...uniqueGroups.map((group) => group.end.row)) + 1;
+    const maxCol = Math.max(...uniqueGroups.map((group) => group.end.col)) + 1;
     return {rows: maxRow, cols: maxCol};
-  }, [cellGroups]);
+  }, [uniqueGroups]);
 
   const fullChartWidth = dimensions.width - Y_AXIS_LABEL_WIDTH;
   const fullChartHeight = dimensions.height - X_AXIS_HEIGHT;
@@ -105,13 +101,6 @@ export function Grid(props: GridProps) {
     return new Set([group.id, ...(group.connectedGroups ?? [])]);
   };
 
-  useLayoutEffect(() => {
-    if (tooltipRef.current && (hoveredGroup || groupSelected)) {
-      const {width, height} = tooltipRef.current.getBoundingClientRect();
-      setTooltipDimensions({width, height});
-    }
-  }, [hoveredGroup, groupSelected]);
-
   const getTooltipInfo = useCallback(
     (group: CellGroup): TooltipInfo | null => {
       const rect =
@@ -128,17 +117,17 @@ export function Grid(props: GridProps) {
       let y: number;
       let placement: Placement;
 
-      if (leftSpace >= tooltipDimensions.width) {
-        x = rect.left - tooltipDimensions.width - TOOLTIP_HORIZONTAL_OFFSET;
+      if (leftSpace >= TOOLTIP_WIDTH) {
+        x = rect.left - TOOLTIP_WIDTH - TOOLTIP_HORIZONTAL_OFFSET;
         y = rect.top;
         placement = 'left';
-      } else if (bottomSpace >= tooltipDimensions.height) {
+      } else if (bottomSpace >= TOOLTIP_HEIGHT) {
         x = rect.left;
         y = rect.bottom + TOOLTIP_HORIZONTAL_OFFSET;
         placement = 'bottom';
       } else {
         x = rect.left;
-        y = rect.top - TOOLTIP_VERTICAL_OFFSET;
+        y = rect.top - TOOLTIP_HEIGHT;
         placement = 'top';
       }
 
@@ -149,7 +138,7 @@ export function Grid(props: GridProps) {
         group,
       };
     },
-    [entry, tooltipDimensions],
+    [entry],
   );
 
   const handleGroupHover = useCallback(
@@ -271,20 +260,20 @@ export function Grid(props: GridProps) {
     (event: React.KeyboardEvent) => {
       if (event.key === 'Tab') {
         event.preventDefault();
-        const currentIndex = cellGroups.findIndex(
+        const currentIndex = uniqueGroups.findIndex(
           (group) => group.id === groupSelected?.id,
         );
         const nextIndex =
-          currentIndex === -1 ? 0 : (currentIndex + 1) % cellGroups.length;
-        const nextGroup = cellGroups[nextIndex];
+          currentIndex === -1 ? 0 : (currentIndex + 1) % uniqueGroups.length;
+        const nextGroup = uniqueGroups[nextIndex];
         setGroupSelected(nextGroup);
         handleGroupHover(nextGroup);
       } else if (event.key === 'Escape') {
         setGroupSelected(null);
         handleGroupHover(null);
       }
     },
-    [cellGroups, groupSelected, handleGroupHover],
+    [uniqueGroups, groupSelected, handleGroupHover],
   );
 
   return (
@@ -304,10 +293,10 @@ export function Grid(props: GridProps) {
             cellHeight={cellHeight}
             xScale={xScale}
           />
-          {cellGroups.map((group, index) => (
+          {uniqueGroups.map((group, index) => (
             <GroupCell
               index={index}
-              key={`group-${index}`}
+              key={group.id}
               group={group}
               xScale={xScale}
               cellHeight={cellHeight}
@@ -326,7 +315,7 @@ export function Grid(props: GridProps) {
           ))}
           <Arrows
             hoveredGroup={hoveredGroup}
-            cellGroups={cellGroups}
+            cellGroups={uniqueGroups}
             xScale={xScale}
             cellHeight={cellHeight}
           />
@@ -342,12 +331,14 @@ export function Grid(props: GridProps) {
           setXAxisHeight={setXAxisHeight}
         />
       </svg>
-      <Tooltip
-        ref={tooltipRef}
-        x={tooltipInfo?.x ?? 0}
-        y={tooltipInfo?.y ?? 0}
-        group={groupSelected || hoveredGroup}
-      />
+
+      {tooltipInfo && (
+        <Tooltip
+          x={tooltipInfo.x}
+          y={tooltipInfo.y}
+          group={groupSelected || hoveredGroup}
+        />
+      )}
     </div>
   );
 }

diff --git a/packages/polaris-viz/src/components/Grid/components/Tooltip.tsx b/packages/polaris-viz/src/components/Grid/components/Tooltip.tsx
@@ -1,4 +1,3 @@
-import {forwardRef} from 'react';
 import {createPortal} from 'react-dom';
 
 import type {CellGroup} from '../types';
@@ -13,55 +12,52 @@ interface TooltipProps {
   group: CellGroup | null;
 }
 
-export const Tooltip = forwardRef<HTMLDivElement, TooltipProps>(
-  function Tooltip({x, y, group}, ref) {
-    const container = useRootContainer(TOOLTIP_ID);
+export function Tooltip({x, y, group}: TooltipProps) {
+  const container = useRootContainer(TOOLTIP_ID);
 
-    return createPortal(
-      !group ? null : (
-        <div
-          ref={ref}
-          className={styles.TooltipContainer}
-          style={{
-            transform: `translate(${x}px, ${y}px)`,
-          }}
-          aria-label={group.name}
-        >
-          <div className={styles.Tooltip}>
-            <div className={styles.TooltipTitle}>{group.name}</div>
-            {group.metricInformation && (
-              <div className={styles.TooltipMetricInformation}>
-                {group.metricInformation}
-              </div>
-            )}
-            {group.description && (
-              <div className={styles.TooltipDescription}>
-                {group.description}
-              </div>
-            )}
-            {group.goal && (
-              <div className={styles.TooltipGoal}>
-                <svg
-                  xmlns="http://www.w3.org/2000/svg"
-                  fill="none"
-                  viewBox="0 0 24 24"
-                  strokeWidth="2.5"
-                  stroke="currentColor"
-                  className={styles.TooltipIcon}
-                >
-                  <path
-                    strokeLinecap="round"
-                    strokeLinejoin="round"
-                    d="M13.5 4.5 21 12m0 0-7.5 7.5M21 12H3"
-                  />
-                </svg>
-                <p className={styles.GroupGoal}>{group.goal}</p>
-              </div>
-            )}
+  if (!group) {
+    return null;
+  }
+
+  return createPortal(
+    <div
+      className={styles.TooltipContainer}
+      style={{
+        transform: `translate(${x}px, ${y}px)`,
+      }}
+      aria-label={group.name}
+    >
+      <div className={styles.Tooltip}>
+        <div className={styles.TooltipTitle}>{group.name}</div>
+        {group.metricInformation && (
+          <div className={styles.TooltipMetricInformation}>
+            {group.metricInformation}
+          </div>
+        )}
+        {group.description && (
+          <div className={styles.TooltipDescription}>{group.description}</div>
+        )}
+        {group.goal && (
+          <div className={styles.TooltipGoal}>
+            <svg
+              xmlns="http://www.w3.org/2000/svg"
+              fill="none"
+              viewBox="0 0 24 24"
+              strokeWidth="2.5"
+              stroke="currentColor"
+              className={styles.TooltipIcon}
+            >
+              <path
+                strokeLinecap="round"
+                strokeLinejoin="round"
+                d="M13.5 4.5 21 12m0 0-7.5 7.5M21 12H3"
+              />
+            </svg>
+            <p className={styles.GroupGoal}>{group.goal}</p>
           </div>
-        </div>
-      ),
-      container,
-    );
-  },
-);
+        )}
+      </div>
+    </div>,
+    container,
+  );
+}
diff --git a/packages/polaris-viz/src/components/Grid/stories/playground/ExternalTooltip.stories.tsx b/packages/polaris-viz/src/components/Grid/stories/playground/ExternalTooltip.stories.tsx
@@ -0,0 +1,49 @@
+import type {Story} from '@storybook/react';
+
+import {Grid} from '../../Grid';
+import type {GridProps} from '../../Grid';
+import {CELL_GROUPS} from '../data';
+import {META} from '../meta';
+
+export default {
+  ...META,
+  title: `${META.title}/Playground`,
+  decorators: [],
+};
+
+function GridCard(args: GridProps) {
+  return (
+    <div
+      style={{
+        height: 400,
+        width: 800,
+        background: 'white',
+        borderRadius: '8px',
+        padding: 10,
+      }}
+    >
+      <Grid {...args} />
+    </div>
+  );
+}
+
+const Template: Story<GridProps> = (args: GridProps) => {
+  return (
+    <div style={{display: 'flex', gap: '20px'}}>
+      <GridCard {...args} />
+      <GridCard {...args} />
+    </div>
+  );
+};
+
+export const MultipleGridsTooltipPosition = Template.bind({});
+
+MultipleGridsTooltipPosition.args = {
+  cellGroups: CELL_GROUPS,
+  xAxisOptions: {
+    label: 'Recency',
+  },
+  yAxisOptions: {
+    label: 'Frequency',
+  },
+};