Reviewing files that changed from the base of the PR and between 164d6b9 and adfbb6e.

• omnidoc/readProject.spec.ts (1 hunks)
• src/cartesian/Area.tsx (3 hunks)
• src/cartesian/Bar.tsx (5 hunks)
• src/cartesian/Funnel.tsx (16 hunks)
• src/cartesian/Line.tsx (5 hunks)
• src/cartesian/Scatter.tsx (7 hunks)
• src/chart/Sankey.tsx (6 hunks)
• src/chart/SunburstChart.tsx (6 hunks)
• src/chart/Treemap.tsx (7 hunks)
• src/component/DefaultTooltipContent.tsx (1 hunks)
• src/polar/Pie.tsx (7 hunks)
• src/polar/Radar.tsx (3 hunks)
• src/polar/RadialBar.tsx (2 hunks)
• src/state/selectors/funnelSelectors.ts (4 hunks)
• src/state/selectors/touchSelectors.ts (1 hunks)
• src/state/touchEventsMiddleware.ts (2 hunks)
• src/util/Constants.ts (1 hunks)
• test/cartesian/Area.spec.tsx (4 hunks)
• test/cartesian/Bar.spec.tsx (4 hunks)
• test/cartesian/Line.spec.tsx (1 hunks)
• test/cartesian/Scatter.spec.tsx (2 hunks)
• test/chart/LineChart.spec.tsx (2 hunks)
• test/chart/ScatterChart.spec.tsx (11 hunks)
• test/chart/Treemap.spec.tsx (3 hunks)
• test/component/Cursor.spec.tsx (2 hunks)
• test/component/DefaultTooltipContent.spec.tsx (2 hunks)
• test/component/Tooltip/Tooltip.content.spec.tsx (6 hunks)
• test/component/Tooltip/Tooltip.formatter.spec.tsx (6 hunks)
• test/component/Tooltip/Tooltip.payload.spec.tsx (13 hunks)
• test/component/Tooltip/Tooltip.visibility.spec.tsx (17 hunks)
• test/component/Tooltip/defaultIndex.spec.tsx (8 hunks)
• test/component/Tooltip/itemSorter.spec.tsx (81 hunks)
• test/helper/mockTouchingElement.ts (2 hunks)
• test/polar/Pie.spec.tsx (33 hunks)
• test/state/selectors/pieSelectors.spec.tsx (16 hunks)
• test/state/selectors/selectors.spec.tsx (20 hunks)

• GitHub Check: Build, Test, Pack
• GitHub Check: Analyze (javascript-typescript)

test/component/Cursor.spec.tsx (2)

11-11: LGTM: Import adds type safety to tests.

The import of TooltipPayload from the internal tooltipSlice module is appropriate for this test file and enables explicit typing of the payload variable in the tests below.

---

167-167: LGTM: Type annotation and graphicalItemId addition align with PR objectives.

The explicit TooltipPayload type annotation adds type safety, and the graphicalItemId property correctly implements the PR's goal of wiring graphical item IDs into tooltip payloads.

src/polar/RadialBar.tsx (1)

365-399: Tooltip payload wiring for graphicalItemId looks correct and consistently typed.
WithIdRequired<PropsWithDefaults> + settings.graphicalItemId: id + passing id={props.id} ensures the registered id is available downstream (selectors/payload generation).

Also applies to: 464-499

test/component/DefaultTooltipContent.spec.tsx (1)

3-7: Test updates correctly reflect the extended payload shape (graphicalItemId) and use the exported prop type.

Also applies to: 38-53

src/polar/Radar.tsx (1)

158-197: Radar tooltip entry settings now correctly include graphicalItemId sourced from the registered id.

Also applies to: 571-599

test/cartesian/Scatter.spec.tsx (1)

579-623: Tooltip payload assertions appropriately validate the presence of graphicalItemId without overfitting to an exact generated value.

test/cartesian/Line.spec.tsx (1)

457-481: Line tooltip payload test correctly asserts graphicalItemId presence using a stable pattern.

omnidoc/readProject.spec.ts (1)

79-86: Snapshot update for Funnel reflected public properties looks consistent with a narrowed runtime surface.

test/chart/Treemap.spec.tsx (1)

373-476: Treemap event payload tests now correctly assert the presence of the generated id while keeping existing behavioral checks intact.

Also applies to: 517-620, 644-742

test/cartesian/Bar.spec.tsx (1)

1587-1722: Payload graphicalItemId assertions are good; please sanity-check whether interaction state should also carry it.
Right now the test expects itemInteraction.hover.graphicalItemId to stay undefined even though payload entries have ids—if that’s intentional (scope-limited to payload only), all good; otherwise this is an easy place to thread it through.

src/util/Constants.ts (1)

28-40: Constant rename/addition looks consistent; consider tightening the doc comment
The new DATA_ITEM_GRAPHICAL_ITEM_ID_ATTRIBUTE_NAME and its usage intent are clear. Minor: the comment says “Unlike dataKey, or name, it is always unique” — might be worth explicitly saying “graphical item id” is unique per rendered item (and not per data point).

test/component/Tooltip/Tooltip.formatter.spec.tsx (1)

34-36: Good test stabilization: explicit Bar ids + graphicalItemId assertions
Adding id="bar-with-function" / id="bar-pv" makes the new graphicalItemId expectations deterministic and aligns with the PR objective.

Also applies to: 57-58, 77-78, 113-115, 150-151, 170-171

test/chart/LineChart.spec.tsx (1)

827-860: LGTM: tooltip integration test covers graphicalItemId end-to-end
Explicit id="line-y" plus the selector assertion should catch regressions in the tooltip payload pipeline.

src/polar/Pie.tsx (5)

43-44: Import/update to use graphical item id attribute looks consistent
No concerns with adding DATA_ITEM_GRAPHICAL_ITEM_ID_ATTRIBUTE_NAME alongside the index attribute for touch picking.

---

325-360: LGTM: Pie tooltip entry settings now carry graphicalItemId
Threading id into SetPieTooltipEntrySettings and writing it into tooltipEntrySettings.settings.graphicalItemId matches the intended pipeline.

---

551-573: Correct DOM wiring: sectors expose both index + graphical item id
Adding [DATA_ITEM_GRAPHICAL_ITEM_ID_ATTRIBUTE_NAME]: id should make item tooltips resolvable even when dataKey/name are ambiguous.

---

646-655: Good: sector tooltip payload includes graphicalItemId
This ensures consumers can disambiguate tooltip entries for identical dataKeys across multiple pies.

---

871-887: Good propagation: PieImpl passes the resolved id into tooltip settings
id={id} on SetPieTooltipEntrySettings keeps the id consistent between state and payload.

test/state/selectors/pieSelectors.spec.tsx (1)

74-88: Solid coverage: selector outputs now assert graphicalItemId consistently
The expectations validate graphicalItemId across initial render, after interaction (dataKey change), and with custom data props.

Also applies to: 122-136, 170-184, 218-232, 266-280, 314-328, 362-376, 423-437, 471-485, 519-533, 567-581, 615-629, 663-677, 711-725, 796-810, 843-857

src/cartesian/Bar.tsx (3)

87-88: Good: internal Bar id typed as GraphicalItemId
This aligns the Bar pipeline with the store’s id type and avoids accidental mixing with unrelated string ids.

Also applies to: 352-357

---

382-416: LGTM: Bar tooltip settings now include graphicalItemId
Setting tooltipEntrySettings.settings.graphicalItemId = id is the key wiring needed for the tooltip payload additions.

---

1042-1076: Good propagation: resolved id is passed into tooltip settings and Bar impl
RegisterGraphicalItemId → SetBarTooltipEntrySettings id={id} keeps tooltip identity stable and testable.

test/component/Tooltip/Tooltip.content.spec.tsx (1)

24-32: Good coverage: deterministic id → asserted graphicalItemId in payload.
This test does a nice job pinning the new linkage end-to-end for Area.

Also applies to: 77-182

test/chart/ScatterChart.spec.tsx (1)

1436-1456: LGTM: Scatter tooltip payload/config assertions now verify graphicalItemId.

Also applies to: 1511-1520, 2267-2274, 2442-2488

test/state/selectors/selectors.spec.tsx (2)

54-100: LGTM: selector expectations now propagate graphicalItemId end-to-end.

Also applies to: 261-509, 1246-1326

---

1-3: Tests: confirm vi.useFakeTimers() is applied for this file (global setup or add here).
Guidelines require fake timers due to Redux batching + rAF timing. If not globally enabled, add a beforeEach(() => vi.useFakeTimers()).

Also applies to: 694-699

src/cartesian/Scatter.tsx (2)

264-307: LGTM: Scatter tooltip settings now include graphicalItemId derived from id.

Also applies to: 758-768

---

439-448: Nice: DOM attribute is now based on graphicalItemId instead of dataKey.
This aligns touch/hover targeting with the new identity model.

src/chart/SunburstChart.tsx (2)

49-94: LGTM: id?: string added to Sunburst public props for deterministic identity.

---

129-163: Good: RegisterGraphicalItemId now drives settings.graphicalItemId for Sunburst tooltips.

Also applies to: 202-225, 365-380

test/component/Tooltip/Tooltip.payload.spec.tsx (2)

165-173: LGTM: tests now assert graphicalItemId for data-on-item LineChart payloads/configs.

Also applies to: 467-957, 969-1136, 1138-1212

---

442-446: Tests: confirm fake timers are enabled (global setup or add here).
If not already handled by createSelectorTestCase / test setup, add vi.useFakeTimers() per guidelines.

src/cartesian/Line.tsx (2)

221-256: Good: tooltip entry settings now carry graphicalItemId.
Adding graphicalItemId: id in TooltipPayloadConfiguration.settings is the right place to attach this identifier for downstream selectors/searchers.

---

835-869: Good: uses RegisterGraphicalItemId output to populate tooltip settings.
This ensures the tooltip gets the same id that’s used for state/graphical-item registration (and not the optional external prop directly).

src/state/selectors/funnelSelectors.ts (2)

10-23: Good: ResolvedFunnelSettings.id makes the identity explicit and typed.
This aligns funnel trapezoid computation with the new graphical-item-id plumbing.

---

30-83: Good: selector forwards graphicalItemId; verify that graphical item IDs are guaranteed unique per chart instance.

The implementation relies on IDs being unique for tooltip/event disambiguation—specifically, touchEventsMiddleware uses .find(item => item.id === graphicalItemId) which returns only the first match. If duplicate IDs exist, the wrong item could be selected. While the codebase documents that graphicalItemId should be unique (see src/state/graphicalItemsSlice.ts lines 15–26), the reducer does not validate uniqueness when items are added or updated. Confirm that user-provided or auto-generated IDs are guaranteed to be unique per chart instance, or add validation to prevent regressions.

test/component/Tooltip/defaultIndex.spec.tsx (3)

14-76: Good: BarChart expectations now assert graphicalItemId in tooltip payload (including hover update).
Nice use of showTooltip + expectLastCalledWith for stable assertions.

---

85-120: Good: LineChart (multi XAxes) payload now carries graphicalItemId and remains deterministic.

---

133-209: Good: PieChart payload assertions updated for graphicalItemId, including hover change.

src/cartesian/Area.tsx (1)

289-324: Good: graphicalItemId is added to Area tooltip entry settings, and id is enforced for the internal call.

src/state/selectors/touchSelectors.ts (1)

4-44: The selector implementation is correct and the concern in this review comment is unfounded.

Analysis: The code at line 31 properly matches tooltip configurations by settings.graphicalItemId. The graphicalItemId field is present in TooltipEntrySettings because it's part of the Payload interface (defined in DefaultTooltipContent.tsx). More importantly, the id prop is always guaranteed to be a non-null string because all graphical item components (Line, Bar, Pie, etc.) pass their id through the useUniqueId hook before calling SetTooltipEntrySettings. The hook generates a unique ID if none is provided, ensuring the value is never undefined.

Therefore: All tooltip configurations will reliably have graphicalItemId set, the selector matching will work as intended, and there is no risk of silent failures returning undefined. No backwards-compatibility fallback is needed.

test/polar/Pie.spec.tsx (5)

1-56: Import and setup changes look correct.

The updated imports properly bring in Mock type and the renamed DATA_ITEM_GRAPHICAL_ITEM_ID_ATTRIBUTE_NAME constant. The test file correctly imports from internal source paths which is acceptable for test files.

---

334-416: Test assertions properly validate the new graphicalItemId in activeShape payload.

The Mock<(props: PieSectorDataItem) => ReactNode> typing and @ts-expect-error comments indicate known typing gaps that should be addressed separately. The expectLastCalledWith helper is correctly used per coding guidelines, and the graphicalItemId: 'pie-y' assertion properly matches the id="pie-y" prop on the Pie component.

---

786-843: Tooltip integration tests correctly updated with graphicalItemId.

The test case properly:

• Adds id="cy-pie" to the Pie component
• Updates mockTouchingElement calls to use the new signature with graphicalItemId
• Verifies graphicalItemId: 'cy-pie' in tooltip state assertions

---

1863-1938: Event handler tests correctly verify graphicalItemId in tooltip payload.

The onClick test properly:

• Adds id="pie-uv" to the Pie component
• Verifies graphicalItemId: 'pie-uv' in the tooltipPayload array
• Uses expectLastCalledWith helper as per coding guidelines

---

1086-1092: Verify if setActiveMouseOverItemIndex from the touch middleware correctly populates graphicalItemId in the hover state.

The implementation in touchEventsMiddleware.ts (lines 56-67) does extract graphicalItemId from the element's DATA_ITEM_GRAPHICAL_ITEM_ID_ATTRIBUTE_NAME attribute via elementFromPoint. It then dispatches setActiveMouseOverItemIndex with this value.

However, the test expects graphicalItemId: undefined in the hover state. This inconsistency suggests one of the following:

1. The setActiveMouseOverItemIndex action is not correctly passing activeGraphicalItemId to the reducer, despite extracting it in the middleware
2. A different code path handles touch events that doesn't use setActiveMouseOverItemIndex
3. The test's mock setup with mockTouchingElement('2', 'cy-pie') is not properly simulating the actual touch event flow

The mouse hover equivalent works correctly because it sets graphicalItemId: 'cy-pie' (line 842), suggesting the inconsistency is real and needs investigation into which step of the touch event pipeline is dropping the graphicalItemId.

src/chart/Sankey.tsx (4)

31-32: Imports correctly added for ID registration pattern.

The WithIdRequired and RegisterGraphicalItemId imports align with the pattern used across other chart components in this PR.

---

533-563: SetSankeyTooltipEntrySettings correctly wires graphicalItemId into tooltip configuration.

The component properly:

• Accepts id in its props via the updated Pick type
• Includes graphicalItemId: id in the tooltip settings

This matches the pattern used in other components like SetFunnelTooltipEntrySettings.

---

951-955: Internal type definitions correctly establish ID-required contract.

The InternalSankeyProps type properly uses WithIdRequired<PropsWithResolvedDefaults> to ensure the internal implementation always receives a resolved ID, while the public Props type allows an optional ID.

---

1096-1154: Sankey component correctly implements the RegisterGraphicalItemId pattern.

The implementation:

• Extracts externalId from resolved props
• Wraps the content with RegisterGraphicalItemId using type "sankey"
• Passes the resolved id to both SetSankeyTooltipEntrySettings and SankeyImpl

This follows the established pattern from other components in this PR.

src/cartesian/Funnel.tsx (7)

48-50: Imports correctly added for graphical item ID wiring.

The imports for GraphicalItemId, RegisterGraphicalItemId, and WithIdRequired align with the pattern established across other chart components.

---

67-70: InternalFunnelProps correctly defines id as required.

Using GraphicalItemId type for id ensures type consistency across the codebase.

---

134-172: SetFunnelTooltipEntrySettings correctly integrates graphicalItemId.

The component properly:

• Accepts id via the updated Pick type (line 149)
• Includes graphicalItemId: id in tooltip settings (line 167)

This matches the pattern in other components.

---

224-232: Destructuring id from trapezoidProps prevents passing it to DOM elements.

This is a good practice to avoid React warnings about unknown DOM attributes. The id is destructured but not used here since it's not needed in the individual trapezoid rendering context.

---

476-537: computeFunnelTrapezoids correctly adds graphicalItemId to tooltip payload.

The function properly:

• Accepts graphicalItemId parameter (line 485)
• Includes it in the TooltipPayload array (line 536)

This ensures the tooltip payload carries the graphical item identifier for each funnel segment.

---

592-601: Funnel component correctly implements RegisterGraphicalItemId pattern.

The implementation:

• Extracts externalId from resolved props and spreads remaining props
• Wraps content with RegisterGraphicalItemId using type "funnel"
• Passes the resolved id to FunnelImpl

This is consistent with the pattern used in Sankey and other chart components.

---

381-427: FunnelImpl correctly uses the id in funnelSettings for selector consumption.

The id is properly:

• Included in the funnelSettings object (line 413)
• Added to the dependency array of useMemo (line 425)

This ensures the selector receives the graphical item ID for proper tooltip coordination.

test/component/Tooltip/Tooltip.visibility.spec.tsx (4)

4-35: Import change looks fine; ensure DefaultZIndexes is part of the intended test surface.

---

109-307: Good: explicit id props make graphicalItemId assertions deterministic.

---

441-448: Touch-mocking updated correctly for graphicalItemId.

---

767-801: Assertions updated consistently to include id/graphicalItemId.

Also applies to: 1060-1232

src/chart/Treemap.tsx (2)

42-43: LGTM: Treemap now supports an external id prop, consistent with other items.

Also applies to: 401-403

---

547-577: LGTM: graphicalItemId is now included in Treemap tooltip entry settings.

Also applies to: 919-926

test/component/Tooltip/itemSorter.spec.tsx (2)

51-62: Nice: explicit ids on composed items make itemSorter tests stable and exercise graphicalItemId.

---

78-436: Assertions updated thoroughly to include graphicalItemId across chart types.

Also applies to: 635-740, 914-959, 967-1276, 1278-1920, 1929-2314, 2321-2706

test/cartesian/Area.spec.tsx (2)

4-4: Import updates look consistent with the new tooltip payload shape.

Also applies to: 24-24

---

820-887: Good upgrade: strongly typed tooltipSpy + graphicalItemId assertion improves coverage.