fix: bound tooltip coordinates to chart container in index sync method by shreedharbhat98 · Pull Request #6263 · recharts/recharts

shreedharbhat98 · 2025-08-30T12:44:36Z

Fix: Bound tooltip coordinates to chart container in index sync method

Ensure synchronized tooltips stay within chart boundaries by clamping coordinates to the receiving chart's viewBox dimensions. Fixes tooltip positioning issues when synchronizing charts with different widths.

Description

When using syncMethod='index' for chart synchronization, tooltip coordinates were passed through directly without considering the receiving chart's container boundaries. This caused tooltips to appear outside chart bounds when synchronizing between charts with different dimensions.

This PR adds coordinate bounding to the index sync method:

X coordinates are clamped between viewBox.x and viewBox.x + viewBox.width
Y coordinates are clamped between viewBox.y and viewBox.y + viewBox.height
All other coordinate properties (for radial charts: angle, clockWise, cx, cy, etc.) are preserved

The fix ensures tooltips always appear within their respective chart containers when synchronizing between charts of different sizes, while maintaining compatibility with all chart types including RadialBarChart.

Related Issue

Fixes #6253 - Tooltip sync for different chart widths

Motivation and Context

When multiple synchronized charts have different dimensions, the tooltip coordinates from one chart may fall outside the boundaries of another chart. This results in tooltips appearing in incorrect positions or outside the visible chart area.

The issue was particularly problematic when:

Synchronizing charts with significantly different widths
Charts positioned at different locations on the page
Using the default syncMethod='index' which previously did no coordinate validation

How Has This Been Tested?

Manual Testing: Verified with the provided CodeSandbox example (https://codesandbox.io/p/sandbox/lm684y) showing tooltip synchronization between charts of different widths

Unit Tests Added

Comprehensive test suite covering:

All Chart Types: LineChart, AreaChart, BarChart, ComposedChart, RadarChart, RadialBarChart
Coordinate Bounding Scenarios:
- Coordinates exceeding chart bounds are clamped to chart dimensions
- Coordinates within bounds remain unchanged
- Property preservation for complex coordinate objects (RadialBarChart)
Cross-Chart Type Synchronization:
- AreaChart ↔ LineChart synchronization with different dimensions
- LineChart → RadialBarChart with property preservation
- BarChart → ComposedChart coordinate bounding
- ComposedChart → AreaChart synchronization
- RadarChart → LineChart coordinate transformation
- RadialBarChart → BarChart with property preservation
- AreaChart → RadarChart coordinate bounding
- Multi-chart synchronization with mixed chart types (LineChart + ComposedChart)
Edge Cases:
- Undefined coordinates handled gracefully
- Zero-sized viewBox handling
- Identical chart sizes (should behave normally)
- Extreme coordinate values with large dimension differences

Screenshots (if appropriate):

Before	After

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
I have added a storybook story or VR test, or extended an existing story or VR test to show my changes

Ensure synchronized tooltips stay within chart boundaries by clamping coordinates to the receiving chart's viewBox dimensions. Fixes tooltip positioning issues when synchronizing charts with different

shreedharbhat98 · 2025-08-30T12:45:17Z

@ckifer Hey, Feel free to take a look when available.
Thanks in advance

codecov · 2025-08-30T12:52:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.62%. Comparing base (9e7fee9) to head (6168e1f).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6263      +/-   ##
==========================================
- Coverage   96.63%   96.62%   -0.02%     
==========================================
  Files         221      221              
  Lines       20147    20178      +31     
  Branches     4129     4136       +7     
==========================================
+ Hits        19470    19496      +26     
- Misses        670      675       +5     
  Partials        7        7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

PavelVanecek · 2025-08-30T13:44:37Z

Hi @shreedharbhat98 , thanks for the PR. Please add tests - the more the better. There's tons of edge cases here to cover.

shreedharbhat98 · 2025-08-30T15:32:16Z

Hi @shreedharbhat98 , thanks for the PR. Please add tests - the more the better. There's tons of edge cases here to cover.

@PavelVanecek Have added some tests to cover all kinds of charts. Feel free to mention if I left any.

Thanks

vmizg · 2025-09-03T10:20:42Z

test/component/Tooltip/Tooltip.sync.spec.tsx

+        if (lastCallA && typeof lastCallA === 'object' && 'x' in lastCallA) {
+          expect(lastCallA.x).toBeLessThanOrEqual(200);
+          expect(lastCallA.x).toBeGreaterThanOrEqual(0);
+        }


let's avoid conditionals in tests: if for whatever reason the call is falsy or any of the conditions fail, the test will continue to pass.

not sure what's the preferred method in the codebase, but I'd do something like this:

const lastCallA = spyA.mock.calls[spyA.mock.calls.length - 1][0]; // assuming lastCallA is of type `any` expect(lastCallA?.x).toBeLessThanOrEqual(200); expect(lastCallA?.x).toBeGreaterThanOrEqual(0);

...as well as in all the rest of the test cases

the exception would be of course if we're testing the opposite behaviour, i.e. if we want it to be called without arguments or with undefined, I'd then separate that into a different set of test cases that assert expect(lastCallA.x).toBeUndefined(), etc

+1. If the conditions are necessary for typescript then please use type assertions and throw. See assertNotNull for an example.

@PavelVanecek I have refactored the tests and also the logic. Back to you for now. 🏓

…ove coordinate bounding - Replace conditional assertions with explicit expect statements - Remove all if/else blocks that could cause silent test failures - Use expect().toBeDefined(), expect().not.toBeNull(), and expect().toHaveProperty() for robust assertions

Copilot

Pull Request Overview

This PR fixes tooltip positioning issues during chart synchronization by ensuring tooltip coordinates are bounded within the receiving chart's container dimensions. The fix specifically addresses the syncMethod='index' case where tooltips could appear outside chart boundaries when synchronizing between charts of different sizes.

Key changes:

Added coordinate bounding logic for x and y coordinates in the index sync method
Preserved all other coordinate properties for compatibility with radial charts
Added comprehensive test coverage for various chart type combinations and edge cases

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/synchronisation/useChartSynchronisation.tsx	Implements coordinate bounding logic to clamp x/y coordinates within viewBox dimensions while preserving other properties
test/component/Tooltip/Tooltip.sync.spec.tsx	Adds extensive test suite covering coordinate bounding for all chart types and cross-chart synchronization scenarios

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-05T12:26:25Z

src/synchronisation/useChartSynchronisation.tsx

+          const boundedCoordinate = {
+            ...otherCoordinateProps,
+            ...(typeof x === 'number' && {
+              x: Math.max(viewBox.x, Math.min(x, viewBox.x + viewBox.width)),


The x coordinate bounding logic is incorrect. It should clamp between viewBox.x and viewBox.x + viewBox.width - 1, or use <= comparison, since viewBox.x + viewBox.width represents the first pixel outside the bounds.

Suggested change

x: Math.max(viewBox.x, Math.min(x, viewBox.x + viewBox.width)),

x: Math.max(viewBox.x, Math.min(x, viewBox.x + viewBox.width - 1)),

Copilot · 2025-09-05T12:26:26Z

src/synchronisation/useChartSynchronisation.tsx

+              x: Math.max(viewBox.x, Math.min(x, viewBox.x + viewBox.width)),
+            }),
+            ...(typeof y === 'number' && {
+              y: Math.max(viewBox.y, Math.min(y, viewBox.y + viewBox.height)),


The y coordinate bounding logic is incorrect. It should clamp between viewBox.y and viewBox.y + viewBox.height - 1, or use <= comparison, since viewBox.y + viewBox.height represents the first pixel outside the bounds.

Suggested change

y: Math.max(viewBox.y, Math.min(y, viewBox.y + viewBox.height)),

y: Math.max(viewBox.y, Math.min(y, viewBox.y + viewBox.height - 1)),

PavelVanecek · 2025-09-05T13:38:19Z

Is the -1 important as Copilot says? Tooltip will bleed into the margins a little I suppose.

ckifer · 2025-09-05T13:52:31Z

Should be an improvement either way for now

shreedharbhat98 · 2025-09-05T14:04:16Z

Is the -1 important as Copilot says? Tooltip will bleed into the margins a little I suppose.

@PavelVanecek Not sure, While testing it, I did not feel it. Tooltip perfectly appeared with in the viewbox itself.
Let me know if you want me to dig deeper and test it out again. Would be happy to do that.
Thanks!

PavelVanecek · 2025-09-05T14:48:28Z

If you couldn't see it while testing then it's all good 👍 the AI is often hallucinating.

fix: bound tooltip coordinates to chart container in index sync method

cb5ffcf

Ensure synchronized tooltips stay within chart boundaries by clamping coordinates to the receiving chart's viewBox dimensions. Fixes tooltip positioning issues when synchronizing charts with different

Added tests to cover different types of charts

2671033

Added cross-chart type synchronization tests

cceb237

vmizg reviewed Sep 3, 2025

View reviewed changes

shreedharbhat98 added 2 commits September 5, 2025 13:42

Fix coordinate bounding logic to handle undefined x/y properties

3788165

PavelVanecek requested a review from Copilot September 5, 2025 12:25

Copilot AI reviewed Sep 5, 2025

View reviewed changes

PavelVanecek approved these changes Sep 5, 2025

View reviewed changes

ckifer approved these changes Sep 5, 2025

View reviewed changes

ckifer merged commit 21a7bf5 into recharts:main Sep 5, 2025
19 of 20 checks passed

shreedharbhat98 deleted the fix/tooltip-sync-different-chart-widths-6253 branch September 5, 2025 14:06

	x: Math.max(viewBox.x, Math.min(x, viewBox.x + viewBox.width)),
	x: Math.max(viewBox.x, Math.min(x, viewBox.x + viewBox.width - 1)),

	y: Math.max(viewBox.y, Math.min(y, viewBox.y + viewBox.height)),
	y: Math.max(viewBox.y, Math.min(y, viewBox.y + viewBox.height - 1)),

Uh oh!

Conversation

shreedharbhat98 commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix: Bound tooltip coordinates to chart container in index sync method

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Unit Tests Added

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

shreedharbhat98 commented Aug 30, 2025

Uh oh!

codecov bot commented Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PavelVanecek commented Aug 30, 2025

Uh oh!

shreedharbhat98 commented Aug 30, 2025

Uh oh!

vmizg Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PavelVanecek Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

shreedharbhat98 Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

PavelVanecek commented Sep 5, 2025

Uh oh!

ckifer commented Sep 5, 2025

Uh oh!

Uh oh!

shreedharbhat98 commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PavelVanecek commented Sep 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

shreedharbhat98 commented Aug 30, 2025 •

edited

Loading

codecov bot commented Aug 30, 2025 •

edited

Loading

vmizg Sep 3, 2025 •

edited

Loading

shreedharbhat98 commented Sep 5, 2025 •

edited

Loading