Fix: Tooltip payload data include unrelated items#6773
Conversation
WalkthroughThis PR propagates a unique graphical item identifier (graphicalItemId) through tooltip and event handling systems across multiple chart types (Bar, Scatter, Funnel, Sankey, Sunburst, Treemap, RadialBar) to enable precise tooltip payload filtering when multiple chart components share a single chart. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/state/selectors/combiners/combineTooltipPayloadConfigurations.ts (1)
24-42: Switching item-tooltip filtering to graphicalItemId is correct; comment could be updatedFiltering
tooltipItemPayloadsbysettings?.graphicalItemIdinstead of dataKey is the right move to distinguish multiple series that share a dataKey and matches the newactiveGraphicalItemIdstate. The defaultIndex fallback behavior is preserved.The inline comment still mentions “we don't have a dataKey to filter by”; consider updating it to mention
graphicalItemIdto avoid confusion for future readers.test/cartesian/Scatter.spec.tsx (1)
626-678: Good test coverage for the core bug fix.This test directly addresses issue #6075 by verifying that tooltip payload only contains data from the hovered Scatter component when multiple Scatter series exist. The test structure is clear, and referencing the related PR and issue in comments is helpful.
However, per coding guidelines, tooltip hover tests should use
vi.runOnlyPendingTimers()after hover events to account forrequestAnimationFramedelays. The test usesfireEvent.mouseEnterwithout advancing timers.Consider adding timer handling after each
fireEvent.mouseEnter:const scatterPoints = container.querySelectorAll('.recharts-symbols'); fireEvent.mouseEnter(scatterPoints[0]); + vi.runOnlyPendingTimers(); const payloadFromFirstScatter = spy.mock.calls.at(-1)?.[0]; assertNotNull(payloadFromFirstScatter); expect(payloadFromFirstScatter).toEqual( expect.arrayContaining([expect.objectContaining({ payload: expect.objectContaining(data01[0]) })]), ); expect(payloadFromFirstScatter).not.toEqual( expect.arrayContaining([expect.objectContaining({ payload: expect.objectContaining(data02[0]) })]), ); fireEvent.mouseEnter(scatterPoints[2]); + vi.runOnlyPendingTimers(); const payloadFromSecondScatter = spy.mock.calls.at(-1)?.[0];As per coding guidelines, timer handling is needed for tooltip hover tests due to Redux autoBatchEnhancer's dependency on
requestAnimationFrame.src/cartesian/Bar.tsx (1)
608-623:idmay be undefined when passed to dispatch hooks.At line 608,
idis destructured with a fallback to an empty object (?? {}), which means ifsvgPropertiesNoEvents(props)returnsundefined,idwill beundefined. However,useMouseEnterItemDispatchanduseMouseClickItemDispatchexpect a non-optionalgraphicalItemId: string.Since
props.idis guaranteed to be aGraphicalItemId(required inInternalProps), consider extractingiddirectly from props before the SVG filtering:function BarRectangles({ data, props, }: { data: ReadonlyArray<BarRectangleItem> | undefined; props: BarRectanglesProps; }) { - const { id, ...baseProps } = svgPropertiesNoEvents(props) ?? {}; + const { id } = props; + const baseProps = svgPropertiesNoEvents(props) ?? {}; const { shape, dataKey, activeBar } = props;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/cartesian/Bar.tsx(2 hunks)src/cartesian/Funnel.tsx(2 hunks)src/cartesian/Scatter.tsx(2 hunks)src/chart/Sankey.tsx(17 hunks)src/chart/SunburstChart.tsx(5 hunks)src/chart/Treemap.tsx(6 hunks)src/context/tooltipContext.tsx(2 hunks)src/polar/RadialBar.tsx(6 hunks)src/state/keyboardEventsMiddleware.ts(0 hunks)src/state/selectors/combiners/combineTooltipPayloadConfigurations.ts(3 hunks)src/state/tooltipSlice.ts(6 hunks)src/state/touchEventsMiddleware.ts(1 hunks)test/cartesian/Bar.spec.tsx(1 hunks)test/cartesian/Scatter.spec.tsx(3 hunks)test/chart/Sankey.spec.tsx(17 hunks)test/chart/SunburstChart.spec.tsx(10 hunks)test/polar/Pie.spec.tsx(1 hunks)test/state/selectors/selectAxisRangeWithReverse.spec.tsx(1 hunks)test/state/selectors/selectAxisScale.spec.tsx(1 hunks)test/state/selectors/selectors.spec.tsx(17 hunks)
💤 Files with no reviewable changes (1)
- src/state/keyboardEventsMiddleware.ts
🧰 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 const
Files:
src/state/touchEventsMiddleware.tstest/polar/Pie.spec.tsxsrc/context/tooltipContext.tsxtest/state/selectors/selectAxisScale.spec.tsxsrc/cartesian/Bar.tsxsrc/cartesian/Scatter.tsxsrc/chart/Treemap.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxsrc/polar/RadialBar.tsxsrc/state/tooltipSlice.tssrc/cartesian/Funnel.tsxsrc/chart/SunburstChart.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Bar.spec.tsxsrc/chart/Sankey.tsxtest/cartesian/Scatter.spec.tsxsrc/state/selectors/combiners/combineTooltipPayloadConfigurations.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
src/state/touchEventsMiddleware.tstest/polar/Pie.spec.tsxsrc/context/tooltipContext.tsxtest/state/selectors/selectAxisScale.spec.tsxsrc/cartesian/Bar.tsxsrc/cartesian/Scatter.tsxsrc/chart/Treemap.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxsrc/polar/RadialBar.tsxsrc/state/tooltipSlice.tssrc/cartesian/Funnel.tsxsrc/chart/SunburstChart.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Bar.spec.tsxsrc/chart/Sankey.tsxtest/cartesian/Scatter.spec.tsxsrc/state/selectors/combiners/combineTooltipPayloadConfigurations.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/state/touchEventsMiddleware.tssrc/context/tooltipContext.tsxsrc/cartesian/Bar.tsxsrc/cartesian/Scatter.tsxsrc/chart/Treemap.tsxsrc/polar/RadialBar.tsxsrc/state/tooltipSlice.tssrc/cartesian/Funnel.tsxsrc/chart/SunburstChart.tsxsrc/chart/Sankey.tsxsrc/state/selectors/combiners/combineTooltipPayloadConfigurations.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
All imports from
rechartsmust use the public API entry point; imports from internal paths likerecharts/types/*orrecharts/src/*are not allowed
Files:
src/state/touchEventsMiddleware.tstest/polar/Pie.spec.tsxsrc/context/tooltipContext.tsxtest/state/selectors/selectAxisScale.spec.tsxsrc/cartesian/Bar.tsxsrc/cartesian/Scatter.tsxsrc/chart/Treemap.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxsrc/polar/RadialBar.tsxsrc/state/tooltipSlice.tssrc/cartesian/Funnel.tsxsrc/chart/SunburstChart.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Bar.spec.tsxsrc/chart/Sankey.tsxtest/cartesian/Scatter.spec.tsxsrc/state/selectors/combiners/combineTooltipPayloadConfigurations.ts
test/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Aim for 100% unit test code coverage when writing new code
Files:
test/polar/Pie.spec.tsxtest/state/selectors/selectAxisScale.spec.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Bar.spec.tsxtest/cartesian/Scatter.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/polar/Pie.spec.tsxtest/state/selectors/selectAxisScale.spec.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Bar.spec.tsxtest/cartesian/Scatter.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 test
Files:
test/polar/Pie.spec.tsxtest/state/selectors/selectAxisScale.spec.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Bar.spec.tsxtest/cartesian/Scatter.spec.tsx
🧠 Learnings (18)
📓 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/polar/Pie.spec.tsxsrc/context/tooltipContext.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Scatter.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/polar/Pie.spec.tsxsrc/context/tooltipContext.tsxtest/state/selectors/selectAxisScale.spec.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxsrc/polar/RadialBar.tsxsrc/state/tooltipSlice.tssrc/chart/SunburstChart.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Bar.spec.tsxsrc/chart/Sankey.tsxtest/cartesian/Scatter.spec.tsxsrc/state/selectors/combiners/combineTooltipPayloadConfigurations.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/context/tooltipContext.tsxsrc/cartesian/Bar.tsxsrc/cartesian/Scatter.tsxsrc/chart/Treemap.tsxsrc/polar/RadialBar.tsxsrc/cartesian/Funnel.tsxsrc/chart/SunburstChart.tsxsrc/chart/Sankey.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/state/selectors/selectAxisScale.spec.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Bar.spec.tsxtest/cartesian/Scatter.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/state/selectors/selectAxisScale.spec.tsxtest/state/selectors/selectAxisRangeWithReverse.spec.tsxtest/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/state/selectors/selectors.spec.tsxtest/cartesian/Scatter.spec.tsx
📚 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, `useAppSelector` and hooks like `useChartLayout()` are designed to return `undefined` when used outside Redux Provider context, rather than throwing errors. This allows components like `Curve` that call these hooks to work standalone by falling back to prop values.
Applied to files:
test/state/selectors/selectAxisScale.spec.tsxtest/cartesian/Scatter.spec.tsx
📚 Learning: 2025-11-19T14:08:01.728Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6659
File: www/src/components/GuideView/Performance/index.tsx:232-250
Timestamp: 2025-11-19T14:08:01.728Z
Learning: In Recharts version 3.4.2, object-as-prop optimizations were introduced to reduce unnecessary re-renders when new object references are passed as props. This affects the recommendation for the `react-perf/jsx-no-new-object-as-prop` ESLint rule.
Applied to files:
test/state/selectors/selectAxisScale.spec.tsxsrc/polar/RadialBar.tsxsrc/chart/SunburstChart.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} : Use the `expectLastCalledWith` helper function instead of `expect(spy).toHaveBeenLastCalledWith(...)` for better typing and autocompletion
Applied to files:
test/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/cartesian/Bar.spec.tsxtest/cartesian/Scatter.spec.tsx
📚 Learning: 2025-12-06T03:36:59.377Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.377Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All imports from `recharts` must use the public API entry point; imports from internal paths like `recharts/types/*` or `recharts/src/*` are not allowed
Applied to files:
test/chart/SunburstChart.spec.tsxsrc/polar/RadialBar.tsxsrc/chart/SunburstChart.tsxtest/cartesian/Bar.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/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/cartesian/Bar.spec.tsxtest/cartesian/Scatter.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/chart/SunburstChart.spec.tsxtest/chart/Sankey.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} : Use `vi.useFakeTimers()` in all tests due to Redux autoBatchEnhancer dependency on timers and `requestAnimationFrame`
Applied to files:
test/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/cartesian/Scatter.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} : Call `vi.runOnlyPendingTimers()` to advance timers after renders when not using `createSelectorTestCase` helper, and avoid `vi.runAllTimers()` to prevent infinite loops
Applied to files:
test/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/cartesian/Scatter.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 storybook/stories/**/*.stories.tsx : Use Storybook for smoke tests and add play functions with assertions for actual tests
Applied to files:
test/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsxtest/cartesian/Scatter.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} : Use `userEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })` or the `userEventSetup` helper function from `test/helper/userEventSetup.ts` when creating userEvent instances
Applied to files:
test/chart/SunburstChart.spec.tsxtest/chart/Sankey.spec.tsx
📚 Learning: 2025-12-08T08:23:26.261Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6750
File: src/state/selectors/axisSelectors.ts:593-602
Timestamp: 2025-12-08T08:23:26.261Z
Learning: In the recharts codebase, `DataKey<any>` is an intentional exception to the "no any" rule while proper typing is being developed. This should not be flagged in reviews.
Applied to files:
test/chart/Sankey.spec.tsxtest/cartesian/Bar.spec.tsx
📚 Learning: 2025-11-16T09:14:24.918Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6640
File: src/cartesian/Bar.tsx:156-159
Timestamp: 2025-11-16T09:14:24.918Z
Learning: In recharts, SSR (Server-Side Rendering) is not yet supported—charts don't render at all in SSR environments. The `isAnimationActive: 'auto'` mode is preparatory work for future SSR support, so testing of this mode should be deferred until SSR support is actually implemented.
Applied to files:
test/chart/Sankey.spec.tsx
🧬 Code graph analysis (15)
test/state/selectors/selectAxisScale.spec.tsx (1)
src/state/tooltipSlice.ts (1)
setActiveMouseOverItemIndex(387-395)
src/cartesian/Bar.tsx (1)
src/context/tooltipContext.tsx (3)
useMouseEnterItemDispatch(21-38)useMouseLeaveItemDispatch(40-48)useMouseClickItemDispatch(50-67)
src/cartesian/Scatter.tsx (1)
src/context/tooltipContext.tsx (3)
useMouseEnterItemDispatch(21-38)useMouseLeaveItemDispatch(40-48)useMouseClickItemDispatch(50-67)
src/chart/Treemap.tsx (1)
src/state/graphicalItemsSlice.ts (1)
GraphicalItemId(19-19)
test/state/selectors/selectAxisRangeWithReverse.spec.tsx (1)
src/state/tooltipSlice.ts (1)
setActiveMouseOverItemIndex(387-395)
test/chart/SunburstChart.spec.tsx (1)
src/state/tooltipSlice.ts (1)
TooltipInteractionState(119-163)
test/chart/Sankey.spec.tsx (2)
src/state/tooltipSlice.ts (1)
TooltipInteractionState(119-163)test/helper/expectLastCalledWith.ts (1)
expectLastCalledWith(14-34)
src/polar/RadialBar.tsx (2)
src/context/tooltipContext.tsx (3)
useMouseEnterItemDispatch(21-38)useMouseLeaveItemDispatch(40-48)useMouseClickItemDispatch(50-67)src/util/useUniqueId.ts (1)
WithIdRequired(33-35)
src/state/tooltipSlice.ts (2)
src/util/types.ts (2)
DataKey(65-65)Coordinate(100-103)src/state/graphicalItemsSlice.ts (1)
GraphicalItemId(19-19)
src/cartesian/Funnel.tsx (1)
src/context/tooltipContext.tsx (3)
useMouseEnterItemDispatch(21-38)useMouseLeaveItemDispatch(40-48)useMouseClickItemDispatch(50-67)
src/chart/SunburstChart.tsx (10)
src/state/tooltipSlice.ts (1)
TooltipIndex(39-39)src/index.ts (4)
TooltipIndex(17-17)Surface(6-6)Layer(8-8)SunburstChart(111-111)src/util/useUniqueId.ts (1)
WithIdRequired(33-35)src/util/resolveDefaultProps.tsx (2)
RequiresDefaultProps(91-108)resolveDefaultProps(16-53)src/container/Surface.tsx (1)
Surface(27-47)src/container/Layer.tsx (1)
Layer(13-22)src/state/RechartsStoreProvider.tsx (1)
RechartsStoreProvider(15-53)src/context/tooltipPortalContext.tsx (1)
TooltipPortalContext(3-3)src/chart/RechartsWrapper.tsx (1)
RechartsWrapper(211-402)src/context/RegisterGraphicalItemId.tsx (1)
RegisterGraphicalItemId(18-21)
test/state/selectors/selectors.spec.tsx (3)
src/state/tooltipSlice.ts (4)
setMouseOverAxisIndex(419-426)setActiveMouseOverItemIndex(387-395)setActiveClickItemIndex(410-418)setMouseClickAxisIndex(427-434)src/state/selectors/selectors.ts (2)
selectTooltipPayloadConfigurations(118-126)selectIsTooltipActive(188-198)src/state/selectors/tooltipSelectors.ts (1)
selectIsTooltipActive(468-471)
src/chart/Sankey.tsx (1)
src/state/graphicalItemsSlice.ts (1)
GraphicalItemId(19-19)
test/cartesian/Scatter.spec.tsx (2)
test/helper/createSelectorTestCase.tsx (1)
createSelectorTestCase(78-139)test/helper/assertNotNull.ts (1)
assertNotNull(1-5)
src/state/selectors/combiners/combineTooltipPayloadConfigurations.ts (1)
src/state/graphicalItemsSlice.ts (1)
GraphicalItemId(19-19)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build, Test, Pack
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (30)
test/polar/Pie.spec.tsx (1)
1085-1092: Pie hover state now correctly asserts graphicalItemIdAsserting
graphicalItemId: 'cy-pie'in the hover interaction state is consistent with the Pieidprop and the new tooltip id propagation; the expectation looks correct and precise.test/state/selectors/selectAxisScale.spec.tsx (1)
92-105: Axis scale stability test updated to reflect new tooltip payload shapeIncluding
activeGraphicalItemIdin the dispatchedsetActiveMouseOverItemIndexpayload keeps this test aligned with the real action type while still verifying thatselectAxisScaleignores tooltip item state. Looks good.src/state/touchEventsMiddleware.ts (1)
69-76: Touch item interactions now propagate graphical item idAdding
activeGraphicalItemId: graphicalItemIdto thesetActiveMouseOverItemIndexdispatch correctly aligns touch handling with the new graphicalItemId-based tooltip filtering, with existing guards ensuring only valid ids are used.test/cartesian/Bar.spec.tsx (1)
1653-1671: Bar stacked-hover tooltip state now asserts graphicalItemIdAsserting
hover.graphicalItemIdwith/^recharts-bar-.+/keeps the test in sync with the new id propagation and correctly verifies that item interactions carry the bar’s graphical id.src/cartesian/Scatter.tsx (1)
414-448: Scatter event wiring now passes graphical item id consistentlyExtracting
idfromallOtherScatterProps, using it in the tooltip dispatch hooks, and stamping it intoDATA_ITEM_GRAPHICAL_ITEM_ID_ATTRIBUTE_NAMEon each symbol is a clean way to integrate Scatter with the new graphicalItemId-based tooltip filtering. The rest of the rendering path remains unchanged.src/cartesian/Funnel.tsx (1)
129-132: Funnel trapezoid interactions now correctly carry graphical item idUpdating
allOtherFunnelPropstoInternalPropsand passingallOtherFunnelProps.idinto the mouse enter/click tooltip dispatch hooks cleanly integrates Funnel with the graphicalItemId-based tooltip logic, without changing rendering or label behavior.Also applies to: 204-229
src/chart/Treemap.tsx (1)
440-449: Treemap now supplies graphicalItemId across mouse and touch interactionsAdding
id: GraphicalItemIdto ContentItemProps, passing that id into the mouse enter/click tooltip actions inContentItemWithEvents, forwarding it fromTreemapItem, and including it in the touchsetActiveMouseOverItemIndexdispatch ensures Treemap’s tooltip behavior aligns with the new graphicalItemId-based filtering, without altering its rendering semantics.Also applies to: 516-548, 590-619, 888-917
src/context/tooltipContext.tsx (1)
21-37: LGTM! The graphicalItemId parameter is now correctly required.The change from optional to required ensures all graphical items must provide an identifier, which is essential for the tooltip filtering fix. The dispatch calls correctly propagate the
activeGraphicalItemIdto the Redux state.Also applies to: 50-66
src/polar/RadialBar.tsx (2)
117-131: LGTM! The graphicalItemId is correctly extracted and passed to dispatch hooks.The
idis properly destructured fromallOtherRadialBarPropsand passed as the third argument touseMouseEnterItemDispatchanduseMouseClickItemDispatch, aligning with the updated hook signatures that now require the graphical item identifier.
360-361: Good type definition for internal props.The
InternalPropstype alias correctly usesWithIdRequired<PropsWithDefaults>to ensure theidproperty is always present for internal component usage, while keeping the public API (RadialBarProps) unchanged.test/chart/Sankey.spec.tsx (3)
247-276: LGTM! Test correctly validates initial tooltip state with graphicalItemId.The test properly asserts that
graphicalItemIdisundefinedin the initial state for both click and hover interactions, which is the expected behavior before any user interaction.
284-355: LGTM! Comprehensive tooltip state transition testing.The test thoroughly validates the tooltip state transitions through hover, click, and leave events, correctly asserting that
graphicalItemIdmatches the expected pattern/^recharts-sankey-.+/when interactions are active. The use ofexpect.stringMatchingis appropriate for dynamically generated IDs.
18-18: No changes needed. The import ofTooltipInteractionStatefrom../../src/state/tooltipSliceat line 18 is consistent with the established pattern across 12+ other test files in the repository (Bar.spec.tsx, Tooltip.spec.tsx, ScatterChart.spec.tsx, etc.). While this type is not exported from the public API, importing internal types via relative paths in test files is a standard practice for verifying internal state. The coding guideline restricts absolute imports fromrecharts/types/*orrecharts/src/*, not relative internal imports used in tests.test/state/selectors/selectAxisRangeWithReverse.spec.tsx (1)
37-44: LGTM! Test correctly updated for new action payload shape.The dispatch call is properly updated to include the
activeGraphicalItemIdfield, which is now required by thesetActiveMouseOverItemIndexaction. This test correctly verifies that the axis range selectors remain stable and don't recompute when unrelated tooltip state changes.test/cartesian/Scatter.spec.tsx (1)
4-4: LGTM! Appropriate imports for the new test scenario.The added imports (
ZAxis,Tooltip,selectActiveTooltipPayload) are correctly sourced from the public API and internal selectors, supporting the new multi-Scatter tooltip filtering test.Also applies to: 16-16
test/chart/SunburstChart.spec.tsx (2)
165-167: LGTM! Improved type safety for the spy.The explicit
Mock<...>typing fortooltipStateSpyprovides better type safety and autocompletion when verifying tooltip state interactions. This aligns with the coding guideline to use theexpectLastCalledWithhelper function for better typing.
198-269: Well-structured assertions for graphicalItemId propagation.The test correctly verifies that:
graphicalItemIdisundefinedin click state before any click occurs (line 204)graphicalItemIdis populated with the auto-generated ID pattern after hover/click interactions- The ID persists correctly across different interaction states (hover, click, leave)
This thoroughly covers the fix for issue #6075.
src/chart/SunburstChart.tsx (4)
40-46: Good internal type design for ensuring tooltipIndex presence.The
SunburstNodeinterface extendingSunburstDatawith a requiredtooltipIndexis a clean pattern. The comment clearly explains why this internal type is needed - to track active nodes in the tooltip - while keeping the external API (SunburstData) flexible by not requiring users to provide tooltipIndex.
272-302: LGTM! Consistent graphicalItemId propagation in event handlers.The event handlers correctly:
- Accept
SunburstNodetype ensuringtooltipIndexis always available- Pass
activeGraphicalItemId: idin bothsetActiveMouseOverItemIndexandsetActiveClickItemIndexdispatches- Call user-provided callbacks before dispatching Redux actions
312-314: LGTM! Clean transformation to SunburstNode.The
nodeWithIndexcreation correctly transformsSunburstDatatoSunburstNodeby spreading the original data and explicitly adding the computedtooltipIndex. This ensures type safety when passing to event handlers.
383-421: LGTM! Well-structured component composition.The refactored
SunburstChartcorrectly:
- Resolves default props at the top level using
resolveDefaultProps- Manages tooltip portal state locally
- Uses
RegisterGraphicalItemIdto ensure a unique ID is always available- Passes the resolved
idtoSunburstChartImplvia the render prop patternsrc/state/tooltipSlice.ts (2)
269-301: Excellent type documentation for GraphicalItemTooltipActionPayload.The new
GraphicalItemTooltipActionPayloadtype is well-designed:
- Clear separation of concerns from axis and keyboard interactions
activeGraphicalItemIdis correctly marked as required (not optional) with documentation explaining that each graphical item must generate its own ID- Comprehensive JSDoc comments explain the purpose and behavior of each field
This directly addresses issue #6075 by enabling tooltip payload filtering based on graphical item identity.
387-433: LGTM! Correct separation of graphicalItemId handling between item and axis interactions.The reducer implementations correctly distinguish between:
- Item interactions (
setActiveMouseOverItemIndex,setActiveClickItemIndex): StoregraphicalItemIdsince these target specific chart elements- Axis interactions (
setMouseOverAxisIndex,setMouseClickAxisIndex): Don't includegraphicalItemIdsince axis interactions apply to all items at an index positionThis is the core fix for issue #6075 - when hovering over a specific Scatter component, only that component's data will be shown in the tooltip.
test/state/selectors/selectors.spec.tsx (4)
54-100: LGTM! Test fixture data correctly updated with graphicalItemId.The example tooltip payload configurations now include
graphicalItemIdin the settings object, which aligns with the updatedTooltipEntrySettingstype. This ensures tests accurately reflect the real-world data shape.
306-312: LGTM! Axis action dispatch correctly excludes graphicalItemId.The
setMouseOverAxisIndexdispatch call correctly matches theAxisTooltipActionPayloadtype, which doesn't includeactiveGraphicalItemId. This is intentional since axis interactions apply to all graphical items at a given index, not specific items.
388-395: LGTM! Item action dispatch correctly includes graphicalItemId matching the configuration.The
setActiveMouseOverItemIndexdispatch correctly includesactiveGraphicalItemIdthat matches thetooltipSettings.settings.graphicalItemId. This ensures the filtering logic in selectors can correctly identify which graphical item is being interacted with.
828-838: LGTM! Critical test for the bug fix.This test directly validates the fix for issue #6075. It verifies that when a user interacts with a graphical item whose
graphicalItemIddoesn't match any tooltip payload configuration, an empty array is returned instead of showing unrelated items. This is the core behavior that prevents multiple Scatter components from showing each other's data in the tooltip.src/chart/Sankey.tsx (3)
33-33: Tooltip configuration correctly tagged withGraphicalItemId.Importing
GraphicalItemIdand wiringgraphicalItemId: idintoSetSankeyTooltipEntrySettingsaligns Sankey’s tooltip configuration with the per-graphical-item matching used elsewhere in this PR. No issues spotted here.Also applies to: 545-561
718-772: Link event payloads now carry the correct graphical item identifier.
graphicalItemIdis consistently threaded throughAllSankeyLinkElementsintoSankeyLinkElementand dispatched asactiveGraphicalItemIdfor both hover and click, which should let the tooltip logic correctly scope payloads per Sankey instance. Implementation looks sound.Also applies to: 774-816
859-913: Node event payloads correctly includeactiveGraphicalItemId.The same
graphicalItemIdis forwarded fromAllNodeElementsintoNodeElementand attached asactiveGraphicalItemIdfor hover and click. This keeps node interactions in sync with link interactions for tooltip filtering. No problems here.Also applies to: 915-951
|
|
||
| // @ts-expect-error bar mouse events are not compatible with recharts mouse events | ||
| const onMouseEnterFromContext = useMouseEnterItemDispatch(onMouseEnterFromProps, dataKey); | ||
| const onMouseEnterFromContext = useMouseEnterItemDispatch(onMouseEnterFromProps, dataKey, allOtherBarProps.id); | ||
| // @ts-expect-error bar mouse events are not compatible with recharts mouse events | ||
| const onMouseLeaveFromContext = useMouseLeaveItemDispatch(onMouseLeaveFromProps); | ||
| // @ts-expect-error bar mouse events are not compatible with recharts mouse events | ||
| const onClickFromContext = useMouseClickItemDispatch(onItemClickFromProps, dataKey); | ||
| const onClickFromContext = useMouseClickItemDispatch(onItemClickFromProps, dataKey, allOtherBarProps.id); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check how BarBackground is called in the codebase
rg "BarBackground" -A 3 -B 3Repository: recharts/recharts
Length of output: 2000
🏁 Script executed:
# Check the Props type definition to see if id is really optional
rg "type Props" -A 20 "src/cartesian/Bar.tsx"Repository: recharts/recharts
Length of output: 600
🏁 Script executed:
# Check the type definitions of the dispatch functions
rg "useMouseEnterItemDispatch|useMouseClickItemDispatch" -B 5 "src/" -t ts -t tsxRepository: recharts/recharts
Length of output: 88
🏁 Script executed:
# Check the actual current implementation at lines 436-442
sed -n '430,450p' src/cartesian/Bar.tsxRepository: recharts/recharts
Length of output: 1061
🏁 Script executed:
# Let's find the definition of BarProps to see if id is optional or required
rg "type BarProps|interface BarProps" -A 30 "src/cartesian/Bar.tsx"Repository: recharts/recharts
Length of output: 1087
🏁 Script executed:
# Check BarSvgProps definition
rg "type BarSvgProps|interface BarSvgProps" -A 30 "src/cartesian/Bar.tsx"Repository: recharts/recharts
Length of output: 791
🏁 Script executed:
# Check if id is defined and where it comes from in the component
rg "id.*:" -B 2 -A 2 "src/cartesian/Bar.tsx" | head -60Repository: recharts/recharts
Length of output: 1559
🏁 Script executed:
# Let's look at the GraphicalItemId type
rg "GraphicalItemId" "src/" -t ts -A 3 | head -40Repository: recharts/recharts
Length of output: 2348
🏁 Script executed:
# Check the actual definition of useMouseEnterItemDispatch to see what types it accepts
ast-grep --pattern 'function useMouseEnterItemDispatch'Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Find the definition of useMouseEnterItemDispatch
rg "useMouseEnterItemDispatch" "src/" -B 5 -A 10 | head -80Repository: recharts/recharts
Length of output: 4874
🏁 Script executed:
# Let's check the GraphicalItemId type definition
rg "type GraphicalItemId|export.*GraphicalItemId" "src/state/graphicalItemsSlice.ts" -A 5Repository: recharts/recharts
Length of output: 269
🏁 Script executed:
# Check how this.props is being passed to BarBackground - look at the Bar class and BarWithState
sed -n '800,830p' src/cartesian/Bar.tsxRepository: recharts/recharts
Length of output: 985
🏁 Script executed:
# Let's check what type this.props has in the class
rg "class Bar extends|class BarWithState" -A 50 "src/cartesian/Bar.tsx" | head -80Repository: recharts/recharts
Length of output: 1498
🏁 Script executed:
# Let's get the actual function signature for useMouseEnterItemDispatch
rg "export.*useMouseEnterItemDispatch|const useMouseEnterItemDispatch" -A 15 "src/context/tooltipContext.tsx"Repository: recharts/recharts
Length of output: 684
🏁 Script executed:
# Also check useMouseClickItemDispatch
rg "export.*useMouseClickItemDispatch|const useMouseClickItemDispatch" -A 15 "src/context/tooltipContext.tsx"Repository: recharts/recharts
Length of output: 680
🏁 Script executed:
# Let's verify InternalBarProps definition
rg "type InternalBarProps|interface InternalBarProps" -A 20 "src/cartesian/Bar.tsx"Repository: recharts/recharts
Length of output: 585
🏁 Script executed:
# Check if BarBackground is exported or used elsewhere
rg "export.*BarBackground|BarBackground" "src/cartesian/Bar.tsx" | grep -v "BarBackgroundProps"Repository: recharts/recharts
Length of output: 172
🏁 Script executed:
# Let's check the full InternalBarProps definition to confirm id is there and required
sed -n '340,380p' src/cartesian/Bar.tsxRepository: recharts/recharts
Length of output: 1162
🏁 Script executed:
# Let's verify what type this.props has in BarWithState.render() to be absolutely sure
sed -n '800,825p' src/cartesian/Bar.tsx | cat -nRepository: recharts/recharts
Length of output: 1107
🏁 Script executed:
# Check if BarBackground is used anywhere else besides BarWithState
rg "BarBackground" "src/" -t ts -t tsxRepository: recharts/recharts
Length of output: 88
🏁 Script executed:
# Search for BarBackground without file type restriction
rg "BarBackground"Repository: recharts/recharts
Length of output: 480
Fix type safety for optional id in BarBackground dispatch calls.
The type annotation for BarBackgroundProps specifies allOtherBarProps: Props where id is optional (id?: string), but useMouseEnterItemDispatch and useMouseClickItemDispatch require a non-optional graphicalItemId: string parameter (lines 438, 442). While BarBackground is only called from BarWithState.render() where this.props has type InternalProps with a required id, the type mismatch should be resolved. Change BarBackgroundProps to use InternalProps or add a type assertion where the dispatch functions are called.
🤖 Prompt for AI Agents
In src/cartesian/Bar.tsx around lines 436 to 442, the calls to
useMouseEnterItemDispatch and useMouseClickItemDispatch pass allOtherBarProps.id
which is typed as optional on BarBackgroundProps, but the dispatch helpers
require a non-optional string; update the BarBackgroundProps type to accept
InternalProps (where id is required) so allOtherBarProps.id is typed as string,
or if you prefer a minimal change add a non-null assertion/type cast when
calling the dispatch functions (e.g. allOtherBarProps.id as string or
allOtherBarProps.id!) to satisfy the parameter type; ensure the chosen fix keeps
runtime safety by only applying the assertion when BarBackground is only ever
called with InternalProps.
|
|
||
| function SankeyImpl(props: InternalSankeyProps) { | ||
| const { className, style, children, ...others } = props; | ||
| const { className, style, children, id, ...others } = props; |
There was a problem hiding this comment.
id no longer forwarded to <Surface> – potential breaking DOM behavior.
By destructuring id out of props and excluding it from others, the id prop is no longer spread into attrs and thus is not applied to <Surface>. Previously, <Sankey id="my-chart" /> would result in the underlying <svg> element having id="my-chart", which consumers may rely on for CSS hooks, tests, or ARIA relationships. Now the id is used only as a graphicalItemId.
To preserve the existing DOM contract while still using id for graphical-item tracking, consider explicitly passing it to <Surface>:
function SankeyImpl(props: InternalSankeyProps) {
- const { className, style, children, id, ...others } = props;
+ const { className, style, children, id, ...others } = props;
const {
link,
@@
- const attrs = svgPropertiesNoEvents(others);
+ const attrs = svgPropertiesNoEvents(others);
@@
- <Surface {...attrs} width={width} height={height}>
+ <Surface {...attrs} id={id} width={width} height={height}>
{children}
<AllSankeyLinkElements
graphicalItemId={id}
@@
/>
<AllNodeElements
graphicalItemId={id}This keeps the new tooltip behavior while avoiding a subtle breaking change for existing users.
Also applies to: 990-990, 1087-1101
🤖 Prompt for AI Agents
In src/chart/Sankey.tsx around lines 971, 990, and 1087-1101, the id prop is
being destructured out of props and thus not forwarded to the underlying
<Surface>, breaking consumers who expect an id on the DOM element; fix by
ensuring the id is still applied to the <Surface> — either stop removing id from
the spread (include it in others) or explicitly pass id={id} to the <Surface>
element while continuing to use the same id value for any graphicalItemId logic
so both the DOM and internal tracking receive the id.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6773 +/- ##
=======================================
Coverage 93.72% 93.73%
=======================================
Files 516 516
Lines 43401 43438 +37
Branches 5008 5008
=======================================
+ Hits 40678 40716 +38
+ Misses 2717 2716 -1
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| id={id} | ||
| /> | ||
| {children} | ||
| </> |
There was a problem hiding this comment.
Quite possibly copy-paste from the other place
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
Bundle ReportChanges will increase total bundle size by 3.46kB (0.13%) ⬆️. This is within the configured threshold ✅ Detailed changes
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:
|
Description
Recreated #6537 but without the question marks. Tooltip matching is now graphical item ID only.
Related Issue
Fixes #6075
Screenshots (if appropriate):
Types of changes
Checklist:
Summary by CodeRabbit
Release Notes
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.