Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Refactor the tooltip positioning logic #1799

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

envex
Copy link
Collaborator

@envex envex commented Feb 4, 2025

What does this implement/fix?

See if we can move away from positioning the tooltip based a bunch of math around the data and SVG points.

I think we should be able to use data attributes to get all the data we need to render and position the tooltips. We already have it available when we render the chart elements, why not just use that and skip all the getXPosition and getAlteredX functions we have for each chart.

This should make adding tooltips to new charts way easier.

The basic idea is that the TooltipWrapper is always watching mouse movements for a chart. If it hovers over an element with a set of data-attributes specific to rendering charts x position, y position and activeIndex we will render a tooltip for that specific point/index.

Storybook link

https://6062ad4a2d14cd0021539c1b-oanoehqfpg.chromatic.com/?path=/story/polaris-viz-playground-tooltip-o-rama--tooltip-orama

Before merging

  • Check your changes on a variety of browsers and devices.

  • Update the Changelog's Unreleased section with your changes.

  • Update relevant documentation, tests, and Storybook.

  • Make sure you're exporting any new shared Components, Types and Utilities from the top level index file of the package

@envex envex force-pushed the envex/better-tooltip-positioning branch 10 times, most recently from d640ee4 to 1f6d713 Compare February 10, 2025 15:00
@envex envex force-pushed the envex/better-tooltip-positioning branch from 1f6d713 to 406caf4 Compare February 10, 2025 15:23
@envex envex changed the title WIP: Simplify Tooltips WIP: Refactor the tooltip positioning logic Feb 10, 2025
@@ -0,0 +1,281 @@
import type {Story} from '@storybook/react';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This story won't stay around, it's just for testing/development

yScale: yScale as ScaleLinear<number, number>,
});
(event: MouseEvent | TouchEvent) => {
const resetPosition = {
Copy link
Collaborator Author

@envex envex Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the first part of the logic starts.

We watch mousemove and if we're hovering over an element that provides us (at least) the activeIndex, we will render the tooltip.

Most likely the element will also provide x and y, but at minimum the activeIndex will render a tooltip for that index.

After we have x/y positions, we check if the tooltip needs to be repositioned.

currentX,
currentY,
position,
return repositionTooltip({
Copy link
Collaborator Author

@envex envex Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the second part of the logic.

Once we have the initial x/y of the tooltip, we need to check if the position needs to be changed.

This can either be:

  • The tooltip will render outside the bounds of the browser window.
  • If the tooltip was repositioned back into the window, we check to see if it would be covering the data point and we try to reposition it again.

This logic is located in the useRepositonTooltip() hook.

The reason we do this here and not in TooltipWrapper is because we need access to the height & width of the tooltip. We don't have access to the actual sizing of the tooltip until we've rendered this component.

@@ -0,0 +1,337 @@
import type {
Copy link
Collaborator Author

@envex envex Feb 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For charts that always update their tooltip position (LineChart, DonutChart) the only repositioning we do it to keep the tooltip inside the bounds of the browser window and away from covering the cursor.

// outside the browser window, or cover the data series,
// we want to reposition it again around the series bounds.
if (isOutsideWindow && seriesBounds != null) {
const positions: Position[] = [
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For charts that don't update their position, we likely pass a seriesBounds prop.

This prop is a set of x, y, height and width values that we use to position the tooltip around.

For example, in a BarChart, if the tooltip has been repositioned into the seriesBounds we will try and reposition the tooltip to the left of the seriesBounds so that the tooltip does not cover the bars.

longestSeriesLength,
}: Props) {
return (
<Fragment>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how we trigger the tooltip for a LineChart.

We render a rect for each data point which acts as the hover zone. These rects are rendered as red boxes with blue outlines below.

image

Each rect contains the data for it's activeIndex as well as the expected x/y for the tooltip.

When using a mouse, the x/y values here are ignored because the tooltip follows the cursor, but are used when using the Tab key to tab through the active points.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant