Fix: scale tooltip coordinates during sync between different size charts#6387
Merged
ckifer merged 6 commits intorecharts:mainfrom Oct 3, 2025
Merged
Conversation
Member
|
looks like this is failing typecheck |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes tooltip coordinate synchronization between charts of different sizes by including the source chart's viewBox in sync events and scaling coordinates proportionally.
- Adds
sourceViewBoxproperty to sync events to enable coordinate scaling between different-sized charts - Updates coordinate calculation logic to scale based on source and target viewBox dimensions instead of just bounding
- Adds comprehensive test coverage for coordinate ratio validation across different chart types
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/synchronisation/useChartSynchronisation.tsx | Implements coordinate scaling logic and includes sourceViewBox in sync events |
| src/state/tooltipSlice.ts | Adds sourceViewBox field to TooltipSyncState type and initial state |
| test/synchronisation/useChartSynchronisation.spec.tsx | Updates test expectations to include sourceViewBox in sync events |
| test/synchronisation/eventCenter.spec.ts | Updates test expectations to include sourceViewBox parameter |
| test/component/Tooltip/Tooltip.sync.spec.tsx | Adds coordinate ratio testing and updates expected call counts |
| test/component/Tooltip/Tooltip.visibility.spec.tsx | Updates test expectations for sourceViewBox field |
| test/chart/ScatterChart.spec.tsx | Updates test expectations for sourceViewBox field |
| test/cartesian/Bar.spec.tsx | Updates test expectations for sourceViewBox field |
| storybook/stories/Examples/AreaChart.stories.tsx | Updates story to demonstrate synchronization with different chart sizes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6387 +/- ##
=======================================
Coverage 93.43% 93.43%
=======================================
Files 412 412
Lines 37876 37878 +2
Branches 4424 4426 +2
=======================================
+ Hits 35391 35393 +2
Misses 2458 2458
Partials 27 27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
013e902 to
8b54d78
Compare
Contributor
Author
|
Checks should be passing now |
ckifer
approved these changes
Oct 1, 2025
…oordinates-during-sync
Contributor
Author
|
Merged with main to resolve a conflict. Ready again 😄 |
Bundle ReportChanges will decrease total bundle size by 480 bytes (-0.02%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
|
ckifer
approved these changes
Oct 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Sync event needed information about the viewbox of the source chart to be able to correctly sync tooltip position when the chart sizes differ.
There is a side effect from including the sourceViewBox that I noticed in unit tests and that's additional sync events being dispatched. Not sure if it's important, so please comment if there is an alternative approach you can think of.
Related Issue
Hopefully finally fixes: #6253
Motivation and Context
See linked issue.
How Has This Been Tested?
Tested by adding unit tests for testing tooltip position ratio, updated Storybook example and doing manual checks.
Screenshots (if appropriate):
Types of changes
Checklist: