feat(Tooltip): allow offset prop to accept Coordinate object#6868
feat(Tooltip): allow offset prop to accept Coordinate object#6868ckifer merged 14 commits intorecharts:mainfrom
Conversation
WalkthroughThe PR widens the Tooltip Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client/App
participant Tooltip as Tooltip component
participant BB as TooltipBoundingBox
participant Util as translate util
participant DOM as Browser DOM
App->>Tooltip: render with offset (number or {x,y})
Tooltip->>BB: pass position and offset
BB->>Util: compute translate(offsetLeft, offsetTop, position, viewBox)
Util-->>BB: return transform(x, y) and flags
BB->>DOM: apply transform / render tooltip at computed coords
DOM-->>App: tooltip visible
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| type: { | ||
| summary: 'number | Coordinate', | ||
| detail: 'number | { x: number, y: number }', | ||
| }, |
There was a problem hiding this comment.
I couldn't find a way to support both number and object types in Storybook's control tab at the same time. Also, it seems like other props with object types don't get edited properly either. Am I missing something here?
There was a problem hiding this comment.
I think number alone is fine, it's a bit of a pain to set objects in Storybook anyway. We have direct code editor + stackblitz for people who want to dig deeper.
There was a problem hiding this comment.
Makes sense. Simplified the type to just number! cf60b97
There was a problem hiding this comment.
It looks like this:
There was a problem hiding this comment.
Please don't edit this file directly, it's autogenerated. Put the comments into Tooltip.tsx JSDoc and then run npm run omnidoc
There was a problem hiding this comment.
Thanks for the heads up! I wasn't aware this file was autogenerated. Updated the JSDoc in Tooltip.tsx and regenerated with omnidoc. 4ffd9d0
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/component/TooltipBoundingBox.spec.tsx (1)
63-78: Consider verifying actual offset behavior.The tests successfully verify that the component renders with different offset types (numeric, Coordinate with distinct values, negative Coordinate). However, they don't verify that the offsets are correctly applied to the tooltip positioning.
💡 Optional enhancement to verify offset behavior
Consider adding assertions that check the computed CSS transform or position to ensure the offset values are correctly applied. You could:
- Mock
getBoundingClientRectto return specific dimensions (as per the test helper intest/helper/MockGetBoundingClientRect.ts)- Assert on the
style.transformor CSS classes applied to the tooltip wrapperThis would provide stronger assurance that the offset logic works correctly beyond just rendering.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/component/Tooltip.tsxsrc/component/TooltipBoundingBox.tsxsrc/util/tooltip/translate.tsstorybook/stories/API/props/TooltipArgTypes.tsxtest/component/TooltipBoundingBox.spec.tsxtest/util/tooltip/translate.spec.tswww/src/docs/api/TooltipAPI.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx}: Never useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not useastype assertions in TypeScript; the only exception isas constAll imports from
rechartsmust use the public API entry point (e.g.,import { TooltipIndex } from 'recharts'). Imports from internal paths likerecharts/types/*orrecharts/src/*are not allowed and will fail the linter.
Files:
src/component/TooltipBoundingBox.tsxwww/src/docs/api/TooltipAPI.tsxtest/util/tooltip/translate.spec.tssrc/component/Tooltip.tsxtest/component/TooltipBoundingBox.spec.tsxstorybook/stories/API/props/TooltipArgTypes.tsxsrc/util/tooltip/translate.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
src/component/TooltipBoundingBox.tsxwww/src/docs/api/TooltipAPI.tsxtest/util/tooltip/translate.spec.tssrc/component/Tooltip.tsxtest/component/TooltipBoundingBox.spec.tsxstorybook/stories/API/props/TooltipArgTypes.tsxsrc/util/tooltip/translate.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired
Files:
src/component/TooltipBoundingBox.tsxsrc/component/Tooltip.tsxsrc/util/tooltip/translate.ts
test/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Aim for 100% unit test code coverage when writing new code
Files:
test/util/tooltip/translate.spec.tstest/component/TooltipBoundingBox.spec.tsx
test/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (test/README.md)
test/**/*.{test,spec}.{ts,tsx}: Aim for 100% unit test code coverage when writing new code
Prefer to use thecreateSelectorTestCasehelper function when writing or modifying tests
Use theexpectLastCalledWithhelper function instead ofexpect(spy).toHaveBeenLastCalledWith(...)for better typing and autocompletion
Verify the number of selector calls using the spy object fromcreateSelectorTestCaseto spot unnecessary re-renders and improve performance
MockgetBoundingClientRectin tests using the helper function provided intest/helper/MockGetBoundingClientRect.ts
Usevi.useFakeTimers()in all tests due to Redux autoBatchEnhancer dependency on timers andrequestAnimationFrame
Callvi.runOnlyPendingTimers()to advance timers after renders when not usingcreateSelectorTestCasehelper, and avoidvi.runAllTimers()to prevent infinite loops
UseuserEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })or theuserEventSetuphelper function fromtest/helper/userEventSetup.tswhen creating userEvent instances
When testing tooltips on hover, usevi.runOnlyPendingTimers()after eachuserEvent.hover()call or use theshowTooltiphelper function fromtooltipTestHelpers.tsto account for requestAnimationFrame delays
Files:
test/util/tooltip/translate.spec.tstest/component/TooltipBoundingBox.spec.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
When running unit tests, prefer to run a single test file using
npm run test -- path/to/TestFile.spec.tsxrather than running all tests withnpm testUnit tests should be placed in the
testdirectory, with some tests also allowed inwww/test. Test files follow the naming convention*.spec.tsx.
Files:
test/util/tooltip/translate.spec.tstest/component/TooltipBoundingBox.spec.tsx
test/component/**/*.spec.tsx
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use React Testing Library for testing component interactions and behavior upon rendering
Files:
test/component/TooltipBoundingBox.spec.tsx
🧠 Learnings (10)
📚 Learning: 2025-11-23T13:30:10.395Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.
Applied to files:
src/component/TooltipBoundingBox.tsxwww/src/docs/api/TooltipAPI.tsxtest/util/tooltip/translate.spec.tssrc/component/Tooltip.tsxtest/component/TooltipBoundingBox.spec.tsxstorybook/stories/API/props/TooltipArgTypes.tsxsrc/util/tooltip/translate.ts
📚 Learning: 2025-12-14T13:58:49.197Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6771
File: src/shape/Curve.tsx:39-40
Timestamp: 2025-12-14T13:58:49.197Z
Learning: In the recharts codebase, hooks like useAppSelector and other hooks (e.g., useChartLayout()) return undefined when used outside Redux Provider context, instead of throwing. This enables components that call these hooks, such as Curve, to operate in standalone mode by falling back to prop values. During code reviews, ensure TSX files gracefully handle undefined results from hooks and implement fallbacks (e.g., via default props or explicit prop-based values) rather than assuming the hook is always within Provider.
Applied to files:
src/component/TooltipBoundingBox.tsxsrc/component/Tooltip.tsx
📚 Learning: 2026-01-06T22:33:55.414Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6855
File: www/src/docs/api/AreaChartAPI.tsx:422-438
Timestamp: 2026-01-06T22:33:55.414Z
Learning: In Recharts v3, components can render inside any chart type as long as the parent provides the necessary cartesian context (e.g., AreaChart, BarChart, ComposedChart, LineChart, ScatterChart). This means API/tests describing cross-chart compatibility should reflect that Funnel-like components can render under multiple chart parents when Cartesian context is available. When reviewing code or docs, verify that the parent chart supplies the required context and that embedding is intentional for all relevant chart types; update tests/docs to avoid assuming stricter parent-child limitations from Recharts v2.
Applied to files:
www/src/docs/api/TooltipAPI.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : When testing tooltips on hover, use `vi.runOnlyPendingTimers()` after each `userEvent.hover()` call or use the `showTooltip` helper function from `tooltipTestHelpers.ts` to account for requestAnimationFrame delays
Applied to files:
test/util/tooltip/translate.spec.tstest/component/TooltipBoundingBox.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Mock `getBoundingClientRect` in tests using the helper function provided in `test/helper/MockGetBoundingClientRect.ts`
Applied to files:
test/util/tooltip/translate.spec.tstest/component/TooltipBoundingBox.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Verify the number of selector calls using the spy object from `createSelectorTestCase` to spot unnecessary re-renders and improve performance
Applied to files:
test/util/tooltip/translate.spec.tstest/component/TooltipBoundingBox.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Prefer to use the `createSelectorTestCase` helper function when writing or modifying tests
Applied to files:
test/util/tooltip/translate.spec.ts
📚 Learning: 2025-12-26T15:59:11.254Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-26T15:59:11.254Z
Learning: Applies to test-vr/**/*.spec.ts : Visual regression tests should follow the naming convention `*.spec.ts` and be placed in the `test-vr` directory. When updating snapshots, new files created in `test-vr/__snapshots__` should be committed to the repository.
Applied to files:
test/util/tooltip/translate.spec.ts
📚 Learning: 2025-11-25T01:22:59.729Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to test/component/**/*.spec.tsx : Use React Testing Library for testing component interactions and behavior upon rendering
Applied to files:
test/component/TooltipBoundingBox.spec.tsx
📚 Learning: 2025-12-16T08:12:06.809Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6783
File: test/util/ChartUtils.spec.tsx:15-16
Timestamp: 2025-12-16T08:12:06.809Z
Learning: In tests (files under any test directory, e.g., test/, __tests__/, etc.), allow importing internal module paths (e.g., ../../src/...) and do not require imports to use the public API entry point (src/index.ts). The public API import restriction should apply only to non-test code. During reviews, accept internal imports in test files and enforce the public API pattern only for non-test code.
Applied to files:
test/component/TooltipBoundingBox.spec.tsx
🧬 Code graph analysis (4)
src/component/TooltipBoundingBox.tsx (2)
src/util/types.ts (1)
Coordinate(128-131)src/util/tooltip/translate.ts (1)
getTooltipTranslate(106-168)
test/util/tooltip/translate.spec.ts (1)
src/util/tooltip/translate.ts (2)
getTooltipTranslateXY(33-88)getTooltipTranslate(106-168)
src/component/Tooltip.tsx (2)
src/util/types.ts (1)
Coordinate(128-131)src/index.ts (1)
Coordinate(138-138)
test/component/TooltipBoundingBox.spec.tsx (1)
src/component/TooltipBoundingBox.tsx (2)
render(78-149)TooltipBoundingBox(39-150)
🔇 Additional comments (11)
src/component/Tooltip.tsx (1)
183-183: LGTM! Type extension is backward compatible.The
offsetprop type has been successfully extended fromnumbertonumber | Coordinate, enabling per-axis offset control while maintaining backward compatibility with existing numeric offsets.storybook/stories/API/props/TooltipArgTypes.tsx (1)
26-40: LGTM! Clear and comprehensive documentation.The updated
offsetdocumentation clearly explains both usage patterns (numeric and Coordinate) with appropriate detail. The type summary and detail fields provide helpful guidance for users.src/util/tooltip/translate.ts (2)
33-88: LGTM! Clean refactor enabling per-axis offsets.The refactor from
offsetTopLeftto separateoffsetparameter (passed asoffsetLeftandoffsetTopfrom the caller) correctly enables independent horizontal and vertical offset handling. The arithmetic for both positive and negative offsets is correct.
106-168: LGTM! Consistent parameter naming.The signature update to use separate
offsetTopandoffsetLeftparameters aligns well with the refactoredgetTooltipTranslateXYfunction. The call sites correctly map these to theoffsetparameter.www/src/docs/api/TooltipAPI.tsx (1)
250-267: LGTM! Comprehensive and user-friendly documentation.The updated
offsetprop documentation clearly explains both usage patterns with appropriate detail. The explanation distinguishes between numeric (uniform) and Coordinate (per-axis) offsets, complete with a concrete code example.src/component/TooltipBoundingBox.tsx (3)
23-23: LGTM! Type signature enables per-axis offset control.The updated type signature correctly allows
offsetto be either a uniform numeric offset or aCoordinateobject with independentxandyvalues, maintaining backward compatibility while enabling the new feature.
99-101: LGTM! Clean type guard ensures backward compatibility.The extraction logic correctly handles both the legacy numeric offset (applied uniformly to both axes) and the new
Coordinateform (with independentxandyvalues). Thetypeofcheck is type-safe and maintains backward compatibility.
105-106: LGTM! Correctly passes per-axis offsets to translate utility.The separate
offsetLeftandoffsetTopparameters align with the refactoredgetTooltipTranslatesignature and enable independent horizontal and vertical positioning.test/util/tooltip/translate.spec.ts (3)
18-18: LGTM! Parameter rename aligns with refactored signature.The consistent update from
offsetTopLefttooffsetin allgetTooltipTranslateXYtest cases correctly reflects the function signature refactoring while maintaining test coverage.Also applies to: 35-35, 50-50, 65-65, 80-80, 97-97, 112-112, 129-129, 146-146, 163-163
259-260: LGTM! Existing tests updated for refactored signature.The split of
offsetTopLeftinto separateoffsetTopandoffsetLeftparameters (both set to 0) correctly maintains existing test coverage while adapting to the new function signature.Also applies to: 272-273, 285-286, 298-299, 315-316, 330-331, 348-349
360-394: LGTM! Excellent test coverage for per-axis offset feature.The new test cases provide comprehensive coverage of the independent offset functionality:
- Different positive
offsetTopandoffsetLeftvalues- Mixed negative/positive offsets
Transform calculations are correct and verify that each axis applies its respective offset independently.
PavelVanecek
left a comment
There was a problem hiding this comment.
Thanks for the PR! I have two requests for changes please, then we're good to merge
There was a problem hiding this comment.
Please don't edit this file directly, it's autogenerated. Put the comments into Tooltip.tsx JSDoc and then run npm run omnidoc
| describe('offset prop', () => { | ||
| it('should accept number offset', () => { | ||
| render(<TooltipBoundingBox {...defaultProps} offset={15} />); | ||
| expect(screen.getByText('Hello world!')).toBeInTheDocument(); |
There was a problem hiding this comment.
We need better assertions here - please look at what other tests are doing, there's showTooltip + mockGetBoundingClientRect + expectTooltipCoordinate helpers available.
There was a problem hiding this comment.
Oh, I wasn't aware of those helper functions. Thanks for letting me know!
I've added tests for the actual tooltip positioning with offset in e5e774b.
As for this test, it's essentially just verifying that offset accepts Coordinate type, which TypeScript already covers. I'm thinking of removing it - what do you think?
| type: { | ||
| summary: 'number | Coordinate', | ||
| detail: 'number | { x: number, y: number }', | ||
| }, |
There was a problem hiding this comment.
I think number alone is fine, it's a bit of a pain to set objects in Storybook anyway. We have direct code editor + stackblitz for people who want to dig deeper.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6868 +/- ##
=======================================
Coverage 93.98% 93.98%
=======================================
Files 539 539
Lines 53121 53128 +7
Branches 5158 5160 +2
=======================================
+ Hits 49924 49931 +7
Misses 3188 3188
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/component/Tooltip/Tooltip.offset.spec.tsx (2)
32-50: Consider adding edge case test for offset=0.The numeric offset tests are correct and well-documented. However, consider adding a test for
offset=0to ensure the tooltip appears at the exact data point position without any offset.📝 Optional edge case test
it('should position tooltip at { x: 20, y: 40 } with offset=20', () => { const { container } = renderTestCase(20); showTooltipOnCoordinate(container, lineChartMouseHoverTooltipSelector, { clientX: 20, clientY: 20 }); expectTooltipCoordinate(container, { x: 20, y: 40 }); }); + + it('should position tooltip at { x: 0, y: 20 } with offset=0', () => { + const { container } = renderTestCase(0); + showTooltipOnCoordinate(container, lineChartMouseHoverTooltipSelector, { clientX: 20, clientY: 20 }); + expectTooltipCoordinate(container, { x: 0, y: 20 }); + }); });
52-58: LGTM! Consider additional edge cases.The Coordinate offset test correctly validates per-axis offset behavior. Optionally, consider adding tests where one axis has zero offset (e.g.,
{ x: 0, y: 10 }or{ x: 10, y: 0 }) to ensure each axis is handled independently.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/component/Tooltip.tsxstorybook/stories/API/props/TooltipArgTypes.tsxtest/component/Tooltip/Tooltip.offset.spec.tsxwww/src/docs/api/TooltipAPI.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/component/Tooltip.tsx
- storybook/stories/API/props/TooltipArgTypes.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx}: Never useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not useastype assertions in TypeScript; the only exception isas constAll imports from
rechartsmust use the public API entry point (e.g.,import { TooltipIndex } from 'recharts'). Imports from internal paths likerecharts/types/*orrecharts/src/*are not allowed and will fail the linter.
Files:
test/component/Tooltip/Tooltip.offset.spec.tsxwww/src/docs/api/TooltipAPI.tsx
test/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Aim for 100% unit test code coverage when writing new code
Files:
test/component/Tooltip/Tooltip.offset.spec.tsx
test/component/**/*.spec.tsx
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use React Testing Library for testing component interactions and behavior upon rendering
Files:
test/component/Tooltip/Tooltip.offset.spec.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
test/component/Tooltip/Tooltip.offset.spec.tsxwww/src/docs/api/TooltipAPI.tsx
test/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (test/README.md)
test/**/*.{test,spec}.{ts,tsx}: Aim for 100% unit test code coverage when writing new code
Prefer to use thecreateSelectorTestCasehelper function when writing or modifying tests
Use theexpectLastCalledWithhelper function instead ofexpect(spy).toHaveBeenLastCalledWith(...)for better typing and autocompletion
Verify the number of selector calls using the spy object fromcreateSelectorTestCaseto spot unnecessary re-renders and improve performance
MockgetBoundingClientRectin tests using the helper function provided intest/helper/MockGetBoundingClientRect.ts
Usevi.useFakeTimers()in all tests due to Redux autoBatchEnhancer dependency on timers andrequestAnimationFrame
Callvi.runOnlyPendingTimers()to advance timers after renders when not usingcreateSelectorTestCasehelper, and avoidvi.runAllTimers()to prevent infinite loops
UseuserEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })or theuserEventSetuphelper function fromtest/helper/userEventSetup.tswhen creating userEvent instances
When testing tooltips on hover, usevi.runOnlyPendingTimers()after eachuserEvent.hover()call or use theshowTooltiphelper function fromtooltipTestHelpers.tsto account for requestAnimationFrame delays
Files:
test/component/Tooltip/Tooltip.offset.spec.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
When running unit tests, prefer to run a single test file using
npm run test -- path/to/TestFile.spec.tsxrather than running all tests withnpm testUnit tests should be placed in the
testdirectory, with some tests also allowed inwww/test. Test files follow the naming convention*.spec.tsx.
Files:
test/component/Tooltip/Tooltip.offset.spec.tsx
🧠 Learnings (12)
📓 Common learnings
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : When testing tooltips on hover, use `vi.runOnlyPendingTimers()` after each `userEvent.hover()` call or use the `showTooltip` helper function from `tooltipTestHelpers.ts` to account for requestAnimationFrame delays
Applied to files:
test/component/Tooltip/Tooltip.offset.spec.tsx
📚 Learning: 2025-11-23T13:30:10.395Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.
Applied to files:
test/component/Tooltip/Tooltip.offset.spec.tsxwww/src/docs/api/TooltipAPI.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Mock `getBoundingClientRect` in tests using the helper function provided in `test/helper/MockGetBoundingClientRect.ts`
Applied to files:
test/component/Tooltip/Tooltip.offset.spec.tsx
📚 Learning: 2025-12-16T08:12:06.809Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6783
File: test/util/ChartUtils.spec.tsx:15-16
Timestamp: 2025-12-16T08:12:06.809Z
Learning: In tests (files under any test directory, e.g., test/, __tests__/, etc.), allow importing internal module paths (e.g., ../../src/...) and do not require imports to use the public API entry point (src/index.ts). The public API import restriction should apply only to non-test code. During reviews, accept internal imports in test files and enforce the public API pattern only for non-test code.
Applied to files:
test/component/Tooltip/Tooltip.offset.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Verify the number of selector calls using the spy object from `createSelectorTestCase` to spot unnecessary re-renders and improve performance
Applied to files:
test/component/Tooltip/Tooltip.offset.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Prefer to use the `createSelectorTestCase` helper function when writing or modifying tests
Applied to files:
test/component/Tooltip/Tooltip.offset.spec.tsx
📚 Learning: 2025-11-25T01:22:59.729Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to test/component/**/*.spec.tsx : Use React Testing Library for testing component interactions and behavior upon rendering
Applied to files:
test/component/Tooltip/Tooltip.offset.spec.tsx
📚 Learning: 2025-11-25T01:23:14.977Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:23:14.977Z
Learning: Applies to src/**/*.{ts,tsx} : Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired
Applied to files:
www/src/docs/api/TooltipAPI.tsx
📚 Learning: 2025-12-26T15:59:11.254Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-26T15:59:11.254Z
Learning: Applies to **/*.{ts,tsx} : All imports from `recharts` must use the public API entry point (e.g., `import { TooltipIndex } from 'recharts'`). Imports from internal paths like `recharts/types/*` or `recharts/src/*` are not allowed and will fail the linter.
Applied to files:
www/src/docs/api/TooltipAPI.tsx
📚 Learning: 2026-01-06T22:33:55.414Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6855
File: www/src/docs/api/AreaChartAPI.tsx:422-438
Timestamp: 2026-01-06T22:33:55.414Z
Learning: In Recharts v3, components can render inside any chart type as long as the parent provides the necessary cartesian context (e.g., AreaChart, BarChart, ComposedChart, LineChart, ScatterChart). This means API/tests describing cross-chart compatibility should reflect that Funnel-like components can render under multiple chart parents when Cartesian context is available. When reviewing code or docs, verify that the parent chart supplies the required context and that embedding is intentional for all relevant chart types; update tests/docs to avoid assuming stricter parent-child limitations from Recharts v2.
Applied to files:
www/src/docs/api/TooltipAPI.tsx
📚 Learning: 2026-01-11T06:13:55.304Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6869
File: www/src/docs/api/useXAxisDomainAPI.tsx:6-20
Timestamp: 2026-01-11T06:13:55.304Z
Learning: In API documentation for hooks (e.g., useXAxisDomainAPI), it's acceptable to describe a parameter like xAxisId using both its literal value (0) and the constant name (defaultAxisId). The description can reference the literal to aid readability, while the defaultVal or equivalent should point to the constant name to aid maintainability. Ensure consistency across similar docs and clearly explain both perspectives for developers using the API.
Applied to files:
www/src/docs/api/TooltipAPI.tsx
🧬 Code graph analysis (1)
test/component/Tooltip/Tooltip.offset.spec.tsx (4)
test/helper/mockGetBoundingClientRect.ts (1)
mockGetBoundingClientRect(34-42)test/helper/createSelectorTestCase.tsx (1)
createSelectorTestCase(78-139)test/component/Tooltip/tooltipTestHelpers.tsx (2)
showTooltipOnCoordinate(78-85)expectTooltipCoordinate(172-178)test/component/Tooltip/tooltipMouseHoverSelectors.ts (1)
lineChartMouseHoverTooltipSelector(26-26)
🔇 Additional comments (4)
www/src/docs/api/TooltipAPI.tsx (1)
251-268: LGTM! Clear and accurate documentation.The type expansion to
Coordinate | numberis backward compatible, and the description clearly explains both usage modes.test/component/Tooltip/Tooltip.offset.spec.tsx (3)
9-12: Verify that fake timers are set up correctly.The coding guidelines require calling
vi.useFakeTimers()in all tests due to Redux autoBatchEnhancer dependency on timers andrequestAnimationFrame. WhilecreateSelectorTestCaseinternally handlesvi.runOnlyPendingTimers(), it doesn't set up fake timers.Verify that the tests run correctly without fake timers. If they fail or are flaky, add the setup:
describe('Tooltip offset', () => { + beforeAll(() => { + vi.useFakeTimers(); + }); + + afterAll(() => { + vi.useRealTimers(); + }); + beforeEach(() => { mockGetBoundingClientRect({ width: 10, height: 10 }); });As per coding guidelines: "Use
vi.useFakeTimers()in all tests due to Redux autoBatchEnhancer dependency on timers andrequestAnimationFrame"
14-30: LGTM! Well-structured test helper.The
renderTestCasehelper correctly usesReact.ComponentPropsfor type extraction and properly integrates withcreateSelectorTestCase.
60-66: LGTM! Excellent coverage of negative offset values.The test correctly validates negative offset behavior, which is important for positioning tooltips in opposite directions.
Added tests for offset=0, offset={ x: 0, y: 10 }, and offset={ x: 10, y: 0 } edge cases! 3e88bef |
|
thanks @bigsaigon333 |
Description
Allow the
offsetprop of the Tooltip component to accept either anumberor aCoordinateobject ({ x: number, y: number }). This enables specifying different horizontal and vertical offsets independently.Changes:
offsetprop type fromnumbertonumber | CoordinateinTooltipandTooltipBoundingBoxoffsetTopLeftparameter into separateoffsetLeftandoffsetTopin the translate utilityRelated Issue
closes #6643
Motivation and Context
Currently, the
offsetprop only accepts a single number that applies equally to both x and y axes. Users want to position tooltips with different offsets per axis (e.g., 20px horizontal offset but 0px vertical offset). This change allowsoffset={{ x: 20, y: 0 }}to achieve that.How Has This Been Tested?
npm test -- test/util/tooltip/translate.spec.ts test/component/TooltipBoundingBox.spec.tsx test/component/Tooltip/Tooltip.offset.spec.tsxScreenshots (if appropriate):
N/A
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.