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

Victory 2298 Tooltip Bug Fix #3027

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ryansrofe
Copy link

Description

The problem occurs in voronoi-helpers.ts when calculating points for tooltips with y0 values. The issue is in the getDatasets method where the voronoi points are calculated. Currently, it calculates the voronoi point position as the midpoint between x and x0 for X coordinates, and y and y0 for Y coordinates.

// ... existing code ...
const { x, y, y0, x0 } = Helpers.getPoint(datum);
const voronoiX = (Number(x) + Number(x0)) / 2;
const voronoiY = (Number(y) + Number(y0)) / 2;
// ... existing code ...

This is problematic because for area charts using y0, the tooltip should activate based on the top line of the area (the y value), not the midpoint between y and y0. The current implementation causes tooltips to activate at points that are visually far from where users would expect them to be. This fix uses only the primary coordinates for voronoi calculations, ignoring the baseline coordinates.

// ... existing code ...
const { x, y } = Helpers.getPoint(datum);
const voronoiX = Number(x);
const voronoiY = Number(y);
// ... existing code ...

This would make the tooltips activate based on proximity to the actual data points rather than the midpoint between the top and bottom of areas, which is what users are expecting.

Fixes #2298

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • This change required a documentation update (StoryBook)

How Has This Been Tested?

I added the reporting user's code that includes the bug to Storybook and tested it in Storybook. Without this change the code produces the bug as described, with this change the bug is no longer present.

Checklist

  • I have included a changeset if this change will require a version change to one of the packages.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (Storybook)
  • I have run all builds, tests, and linting and all checks pass

The issue is in the getDatasets method where the voronoi points are calculated. Currently, it calculates the voronoi point position as the midpoint between x and x0 for X coordinates, and y and y0 for Y coordinates. The fix should be to use just the primary coordinates for voronoi calculations, ignoring the baseline coordinates.
Added a Voronoi section to Storybook to test the users bug example (voronoi-y0). I also added all the existing Voronoi examples to Storybook from the docs.
Copy link

changeset-bot bot commented Jan 9, 2025

🦋 Changeset detected

Latest commit: 9a50fc5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
victory-voronoi-container Minor
victory-create-container Minor
victory Minor
victory-area Minor
victory-axis Minor
victory-bar Minor
victory-box-plot Minor
victory-brush-container Minor
victory-brush-line Minor
victory-candlestick Minor
victory-canvas Minor
victory-chart Minor
victory-core Minor
victory-cursor-container Minor
victory-errorbar Minor
victory-group Minor
victory-histogram Minor
victory-legend Minor
victory-line Minor
victory-native Minor
victory-pie Minor
victory-polar-axis Minor
victory-scatter Minor
victory-selection-container Minor
victory-shared-events Minor
victory-stack Minor
victory-tooltip Minor
victory-vendor Minor
victory-voronoi Minor
victory-zoom-container Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Jan 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
victory ✅ Ready (Inspect) Visit Preview Jan 9, 2025 5:26pm

@ryansrofe ryansrofe self-assigned this Jan 9, 2025
Copy link
Contributor

github-actions bot commented Jan 9, 2025

Folder/File Previous size New size Difference
/packages/victory/dist/victory.min.js 386.05KB 386.01KB -0.04KB (-0.01%)
TOTAL -0.04KB

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.

Using y0 prop inside VictoryVoronoiContainer causes inaccurate tooltip activation
2 participants