From 3f35f468a72ed318f8f1e10d807d7e7f609ed0bc Mon Sep 17 00:00:00 2001 From: Pablo Assanelli Date: Tue, 19 Nov 2024 17:07:33 -0300 Subject: [PATCH 1/2] fix, Tooltip should be render inside polaris_viz_tooltip_root --- .../polaris-viz/src/components/Grid/Grid.tsx | 60 ++++++++---- .../components/Grid/components/Tooltip.scss | 4 + .../components/Grid/components/Tooltip.tsx | 97 +++++++++++-------- .../TooltipWrapper/TooltipWrapper.tsx | 3 +- packages/polaris-viz/src/constants.ts | 1 + 5 files changed, 100 insertions(+), 65 deletions(-) diff --git a/packages/polaris-viz/src/components/Grid/Grid.tsx b/packages/polaris-viz/src/components/Grid/Grid.tsx index 6bbda285c..52447681d 100644 --- a/packages/polaris-viz/src/components/Grid/Grid.tsx +++ b/packages/polaris-viz/src/components/Grid/Grid.tsx @@ -1,4 +1,11 @@ -import React, {useCallback, useEffect, useMemo, useState} from 'react'; +import React, { + useCallback, + useEffect, + useLayoutEffect, + useMemo, + useRef, + useState, +} from 'react'; import type {LabelFormatter} from '@shopify/polaris-viz-core'; import { DEFAULT_CHART_PROPS, @@ -54,6 +61,15 @@ export function Grid(props: GridProps) { const [isSmallContainer, setIsSmallContainer] = useState(false); const [groupSelected, setGroupSelected] = useState(null); + const tooltipRef = useRef(null); + const [tooltipDimensions, setTooltipDimensions] = useState<{ + width: number; + height: number; + }>({ + width: TOOLTIP_WIDTH, + height: TOOLTIP_HEIGHT, + }); + const { cellGroups = [], xAxisOptions = {}, @@ -89,6 +105,13 @@ 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 = @@ -105,21 +128,17 @@ export function Grid(props: GridProps) { let y: number; let placement: Placement; - if (leftSpace >= TOOLTIP_WIDTH) { - x = - rect.left - - containerRect.left - - TOOLTIP_WIDTH - - TOOLTIP_HORIZONTAL_OFFSET; - y = rect.top - containerRect.top; + if (leftSpace >= tooltipDimensions.width) { + x = rect.left - tooltipDimensions.width - TOOLTIP_HORIZONTAL_OFFSET; + y = rect.top; placement = 'left'; - } else if (bottomSpace >= TOOLTIP_HEIGHT) { - x = rect.left - containerRect.left; - y = rect.bottom - containerRect.top + TOOLTIP_HORIZONTAL_OFFSET; + } else if (bottomSpace >= tooltipDimensions.height) { + x = rect.left; + y = rect.bottom + TOOLTIP_HORIZONTAL_OFFSET; placement = 'bottom'; } else { - x = rect.left - containerRect.left; - y = rect.top - containerRect.top - TOOLTIP_VERTICAL_OFFSET; + x = rect.left; + y = rect.top - TOOLTIP_VERTICAL_OFFSET; placement = 'top'; } @@ -130,7 +149,7 @@ export function Grid(props: GridProps) { group, }; }, - [entry], + [entry, tooltipDimensions], ); const handleGroupHover = useCallback( @@ -323,13 +342,12 @@ export function Grid(props: GridProps) { setXAxisHeight={setXAxisHeight} /> - {tooltipInfo && ( - - )} + ); } diff --git a/packages/polaris-viz/src/components/Grid/components/Tooltip.scss b/packages/polaris-viz/src/components/Grid/components/Tooltip.scss index d41ac13d4..86367db52 100644 --- a/packages/polaris-viz/src/components/Grid/components/Tooltip.scss +++ b/packages/polaris-viz/src/components/Grid/components/Tooltip.scss @@ -46,6 +46,8 @@ .Tooltip { font-size: 12px; + font-family: Inter, -apple-system, system-ui, 'San Francisco', 'Segoe UI', + Roboto, 'Helvetica Neue', sans-serif; line-height: 1.2; padding: 12px; background: $color-white; @@ -57,6 +59,8 @@ .TooltipContainer { position: absolute; z-index: 1; + top: 0; + left: 0; width: 250px; outline: none; @container (max-width: #{$grid-small-breakpoint}) { diff --git a/packages/polaris-viz/src/components/Grid/components/Tooltip.tsx b/packages/polaris-viz/src/components/Grid/components/Tooltip.tsx index b0c187120..f6a53b937 100644 --- a/packages/polaris-viz/src/components/Grid/components/Tooltip.tsx +++ b/packages/polaris-viz/src/components/Grid/components/Tooltip.tsx @@ -1,4 +1,9 @@ +import {forwardRef} from 'react'; +import {createPortal} from 'react-dom'; + import type {CellGroup} from '../types'; +import {useRootContainer} from '../../../hooks/useRootContainer'; +import {TOOLTIP_ID} from '../../../constants'; import styles from './Tooltip.scss'; @@ -8,47 +13,55 @@ interface TooltipProps { group: CellGroup | null; } -export function Tooltip({x, y, group}: TooltipProps) { - if (!group) return null; +export const Tooltip = forwardRef( + function Tooltip({x, y, group}, ref) { + const container = useRootContainer(TOOLTIP_ID); - return ( -
-
-
{group.name}
- {group.metricInformation && ( -
- {group.metricInformation} -
- )} - {group.description && ( -
{group.description}
- )} - {group.goal && ( -
- - - -

{group.goal}

+ return createPortal( + !group ? null : ( +
+
+
{group.name}
+ {group.metricInformation && ( +
+ {group.metricInformation} +
+ )} + {group.description && ( +
+ {group.description} +
+ )} + {group.goal && ( +
+ + + +

{group.goal}

+
+ )}
- )} -
-
- ); -} +
+ ), + container, + ); + }, +); diff --git a/packages/polaris-viz/src/components/TooltipWrapper/TooltipWrapper.tsx b/packages/polaris-viz/src/components/TooltipWrapper/TooltipWrapper.tsx index 0e2fdae01..8802b7a74 100644 --- a/packages/polaris-viz/src/components/TooltipWrapper/TooltipWrapper.tsx +++ b/packages/polaris-viz/src/components/TooltipWrapper/TooltipWrapper.tsx @@ -7,6 +7,7 @@ import {createPortal} from 'react-dom'; import {useRootContainer} from '../../hooks/useRootContainer'; import type {Margin} from '../../types'; import {SwallowErrors} from '../SwallowErrors'; +import {TOOLTIP_ID} from '../../constants'; import {shouldBlockTooltipEvents} from './utilities/shouldBlockTooltipEvents'; import type {TooltipPosition, TooltipPositionParams} from './types'; @@ -14,8 +15,6 @@ import {DEFAULT_TOOLTIP_POSITION} from './constants'; import {TooltipAnimatedContainer} from './components/TooltipAnimatedContainer'; import type {AlteredPosition} from './utilities'; -const TOOLTIP_ID = 'polaris_viz_tooltip_root'; - interface BaseProps { chartBounds: BoundingRect; getMarkup: (index: number) => ReactNode; diff --git a/packages/polaris-viz/src/constants.ts b/packages/polaris-viz/src/constants.ts index 3bb8cff49..b9039d94e 100644 --- a/packages/polaris-viz/src/constants.ts +++ b/packages/polaris-viz/src/constants.ts @@ -62,3 +62,4 @@ export const WARN_FOR_DEVELOPMENT = IS_DEVELOPMENT && !IS_TEST; export const HOVER_TARGET_ZONE = 48; export const CROSSHAIR_ID = 'Crosshair'; export const DEFAULT_ANIMATION_DELAY = 100; +export const TOOLTIP_ID = 'polaris_viz_tooltip_root'; From 96087355f8c4d6eeaab5dbfb44ea9947a1554f69 Mon Sep 17 00:00:00 2001 From: George Date: Wed, 20 Nov 2024 09:54:08 -0300 Subject: [PATCH 2/2] Address pr feedback --- packages/polaris-viz/CHANGELOG.md | 5 + .../polaris-viz/src/components/Grid/Grid.tsx | 83 +++++++--------- .../components/Grid/components/Tooltip.tsx | 98 +++++++++---------- .../playground/ExternalTooltip.stories.tsx | 49 ++++++++++ .../playground/TooltipOverflow.stories.tsx | 62 ++++++++++++ .../components/Grid/utilities/constants.ts | 3 +- 6 files changed, 201 insertions(+), 99 deletions(-) create mode 100644 packages/polaris-viz/src/components/Grid/stories/playground/ExternalTooltip.stories.tsx create mode 100644 packages/polaris-viz/src/components/Grid/stories/playground/TooltipOverflow.stories.tsx diff --git a/packages/polaris-viz/CHANGELOG.md b/packages/polaris-viz/CHANGELOG.md index 31dfc437a..cd412fb60 100644 --- 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 index 52447681d..ab718a1cc 100644 --- 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(null); - const tooltipRef = useRef(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,12 +260,12 @@ 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') { @@ -284,7 +273,7 @@ export function Grid(props: GridProps) { 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) => ( @@ -342,12 +331,14 @@ export function Grid(props: GridProps) { setXAxisHeight={setXAxisHeight} /> - + + {tooltipInfo && ( + + )}
); } diff --git a/packages/polaris-viz/src/components/Grid/components/Tooltip.tsx b/packages/polaris-viz/src/components/Grid/components/Tooltip.tsx index f6a53b937..e43f5ec25 100644 --- 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( - 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 : ( -
-
-
{group.name}
- {group.metricInformation && ( -
- {group.metricInformation} -
- )} - {group.description && ( -
- {group.description} -
- )} - {group.goal && ( -
- - - -

{group.goal}

-
- )} + if (!group) { + return null; + } + + return createPortal( +
+
+
{group.name}
+ {group.metricInformation && ( +
+ {group.metricInformation} +
+ )} + {group.description && ( +
{group.description}
+ )} + {group.goal && ( +
+ + + +

{group.goal}

-
- ), - container, - ); - }, -); + )} +
+
, + 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 new file mode 100644 index 000000000..322376893 --- /dev/null +++ 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 ( +
+ +
+ ); +} + +const Template: Story = (args: GridProps) => { + return ( +
+ + +
+ ); +}; + +export const MultipleGridsTooltipPosition = Template.bind({}); + +MultipleGridsTooltipPosition.args = { + cellGroups: CELL_GROUPS, + xAxisOptions: { + label: 'Recency', + }, + yAxisOptions: { + label: 'Frequency', + }, +}; diff --git a/packages/polaris-viz/src/components/Grid/stories/playground/TooltipOverflow.stories.tsx b/packages/polaris-viz/src/components/Grid/stories/playground/TooltipOverflow.stories.tsx new file mode 100644 index 000000000..6fdc6cbba --- /dev/null +++ b/packages/polaris-viz/src/components/Grid/stories/playground/TooltipOverflow.stories.tsx @@ -0,0 +1,62 @@ +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 ( +
+ +
+ ); +} + +const Template: Story = (args: GridProps) => { + return ( +
+
+ +
+
+ ); +}; + +export const TooltipOverflow = Template.bind({}); + +TooltipOverflow.args = { + cellGroups: CELL_GROUPS, + xAxisOptions: { + label: 'Recency', + }, + yAxisOptions: { + label: 'Frequency', + }, +}; + +TooltipOverflow.parameters = { + docs: { + description: { + story: + 'This story tests that tooltips render correctly when the Grid is inside a container with overflow: hidden.', + }, + }, +}; diff --git a/packages/polaris-viz/src/components/Grid/utilities/constants.ts b/packages/polaris-viz/src/components/Grid/utilities/constants.ts index 9df773584..c4ce984f9 100644 --- a/packages/polaris-viz/src/components/Grid/utilities/constants.ts +++ b/packages/polaris-viz/src/components/Grid/utilities/constants.ts @@ -1,7 +1,6 @@ export const TOOLTIP_WIDTH = 250; -export const TOOLTIP_HEIGHT = 120; +export const TOOLTIP_HEIGHT = 155; export const TOOLTIP_HORIZONTAL_OFFSET = 10; -export const TOOLTIP_VERTICAL_OFFSET = 155; export const Y_LABEL_OFFSET = 25; export const Y_AXIS_LABEL_WIDTH = 50;