Merge pull request #1774 from Shopify/fix-tab-tooltip-position

Fix mis-positioned LineChart tooltips while tabbing points
Shopify · Dec 5, 2024 · 43f6b72 · 43f6b72
2 parents 8d1c79f + 31d6fe3
commit 43f6b72
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 19 deletions.
diff --git a/packages/polaris-viz/CHANGELOG.md b/packages/polaris-viz/CHANGELOG.md
@@ -11,6 +11,10 @@ and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 - Added dashed `lineStyle` support for custom legends in `<LineChartRelational />`
 
+### Fixed
+
+- Fixed issue where tooltips were not positioned correctly when tabbing through points in `<LineChart />`.
+
 ## [15.3.4] - 2024-12-03
 
 

diff --git a/packages/polaris-viz/src/components/LineChart/stories/playground/ExternalTooltip.stories.tsx b/packages/polaris-viz/src/components/LineChart/stories/playground/ExternalTooltip.stories.tsx
@@ -63,7 +63,10 @@ const TemplateWithFrame: Story<LineChartProps> = (args: LineChartProps) => {
   return (
     <div style={{overflow: 'hidden', position: 'fixed', inset: 0}}>
       <div style={{height: 100, background: 'black', width: '100%'}}></div>
-      <div style={{overflow: 'auto', height: '100vh'}} ref={setRef}>
+      <div
+        style={{overflow: 'auto', height: 'calc(100vh - 100px)'}}
+        ref={setRef}
+      >
         <Card {...props} />
         <div style={{height: 700, width: 10}} />
         <div style={{display: 'flex', justifyContent: 'space-between'}}>

diff --git a/packages/polaris-viz/src/components/TooltipWrapper/TooltipWrapper.tsx b/packages/polaris-viz/src/components/TooltipWrapper/TooltipWrapper.tsx
@@ -57,7 +57,7 @@ function TooltipWrapperRaw(props: BaseProps) {
     xScale,
     yScale,
   } = props;
-  const {scrollContainer, isTouchDevice} = useChartContext();
+  const {scrollContainer, isTouchDevice, containerBounds} = useChartContext();
   const [position, setPosition] = useState<TooltipPosition>({
     x: 0,
     y: 0,
@@ -92,18 +92,14 @@ function TooltipWrapperRaw(props: BaseProps) {
       event?: MouseEvent | TouchEvent;
       index?: number;
     }) => {
-      const containerBounds = {
-        x: parentElement?.getBoundingClientRect().x ?? 0,
-        y:
-          Number(parentElement?.getBoundingClientRect().y ?? 0) +
-          Number(scrollContainer?.scrollTop ?? 0),
-        width: parentElement?.getBoundingClientRect().width ?? 0,
-        height: parentElement?.getBoundingClientRect().height ?? 0,
-      };
+      const scrollY = scrollContainer == null ? 0 : scrollContainer.scrollTop;
+
       switch (chartType) {
         case InternalChartType.Line:
           return getLineChartTooltipPosition({
             chartBounds,
+            containerBounds,
+            scrollY,
             data,
             event,
             eventType,
@@ -140,14 +136,14 @@ function TooltipWrapperRaw(props: BaseProps) {
     },
     [
       chartBounds,
+      containerBounds,
       chartType,
       data,
       longestSeriesIndex,
-      parentElement,
-      scrollContainer?.scrollTop,
       type,
       xScale,
       yScale,
+      scrollContainer,
     ],
   );
 
@@ -192,7 +188,7 @@ function TooltipWrapperRaw(props: BaseProps) {
     (event: MouseEvent | TouchEvent) => {
       window.clearTimeout(touchStartTimer.current);
 
-      if (event instanceof TouchEvent) {
+      if (typeof TouchEvent !== 'undefined' && event instanceof TouchEvent) {
         if (isLongTouch.current === true) {
           // prevents scrolling after long touch (since it is supposed to move the tooltip/datapoint vs scroll)
           event?.preventDefault();

diff --git a/packages/polaris-viz/src/components/TooltipWrapper/utilities/eventPoint.ts b/packages/polaris-viz/src/components/TooltipWrapper/utilities/eventPoint.ts
@@ -72,7 +72,7 @@ export function eventPointNative(event: MouseEvent | TouchEvent) {
 }
 
 export function getXYFromEventType(event: MouseEvent | TouchEvent): Position {
-  return event instanceof TouchEvent
+  return 'touches' in event
     ? {x: event.touches[0].pageX, y: event.touches[0].pageY}
     : {x: event.pageX, y: event.pageY};
 }
@@ -86,5 +86,7 @@ export function isMouseEvent(
 export function isTouchEvent(
   event: React.SyntheticEvent,
 ): event is React.TouchEvent {
-  return event.nativeEvent instanceof TouchEvent;
+  return (
+    typeof TouchEvent !== 'undefined' && event.nativeEvent instanceof TouchEvent
+  );
 }
diff --git a/packages/polaris-viz/src/components/TooltipWrapper/utilities/getAlteredLineChartPosition.ts b/packages/polaris-viz/src/components/TooltipWrapper/utilities/getAlteredLineChartPosition.ts
@@ -18,7 +18,7 @@ export type AlteredPosition = (
 export function getAlteredLineChartPosition(
   props: AlteredPositionProps,
 ): AlteredPositionReturn {
-  const {currentX, currentY, chartBounds, scrollContainer} = props;
+  const {currentX, currentY, containerBounds, scrollContainer} = props;
 
   let x = currentX;
   let y = currentY;
@@ -28,7 +28,7 @@ export function getAlteredLineChartPosition(
   //
 
   if (props.isPerformanceImpacted) {
-    y = chartBounds.y - (scrollContainer?.scrollTop ?? 0);
+    y = containerBounds.y - (scrollContainer?.scrollTop ?? 0);
   }
 
   //

diff --git a/packages/polaris-viz/src/components/TooltipWrapper/utilities/getLineChartTooltipPosition.ts b/packages/polaris-viz/src/components/TooltipWrapper/utilities/getLineChartTooltipPosition.ts
@@ -1,3 +1,4 @@
+import type {BoundingRect} from '@shopify/polaris-viz-core';
 import {clamp} from '@shopify/polaris-viz-core';
 import type {ScaleLinear} from 'd3-scale';
 
@@ -7,16 +8,20 @@ import {TOOLTIP_POSITION_DEFAULT_RETURN} from '../constants';
 import {getXYFromEventType, eventPointNative} from './eventPoint';
 
 interface Props extends Omit<TooltipPositionParams, 'xScale'> {
+  containerBounds: BoundingRect;
+  scrollY: number;
   xScale: ScaleLinear<number, number>;
 }
 
 export function getLineChartTooltipPosition({
+  containerBounds,
   chartBounds,
   data,
   event,
   eventType,
   index,
   longestSeriesIndex,
+  scrollY,
   xScale,
 }: Props) {
   if (eventType === 'mouse') {
@@ -53,8 +58,8 @@ export function getLineChartTooltipPosition({
     const x = xScale?.(activeIndex) ?? 0;
 
     return {
-      x: x + chartBounds.x,
-      y: chartBounds.y,
+      x: x + containerBounds.x,
+      y: containerBounds.y - scrollY,
       activeIndex,
     };
   }

diff --git a/...ris-viz/src/components/TooltipWrapper/utilities/tests/getAlteredLineChartPosition.test.ts b/...ris-viz/src/components/TooltipWrapper/utilities/tests/getAlteredLineChartPosition.test.ts
@@ -16,6 +16,7 @@ const BASE_PROPS: AlteredPositionProps = {
     horizontal: TooltipHorizontalOffset.Left,
     vertical: TooltipVerticalOffset.Center,
   },
+  containerBounds: {height: 100, width: 200, x: 0, y: 100},
 };
 
 let windowSpy;