Add expanding pinned tooltips and consistent callback contexts - #60
Merged
Conversation
📝 WalkthroughWalkthroughThis change adds expanding pinned energy-tooltip coverage, standardizes public callback context parameters, adds presentation-only focus points and dismissal handling, updates callback validation, and refreshes related documentation, examples, catalog metadata, and benchmark baselines. ChangesChart behavior and conformance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChartRenderer
participant Tooltip
participant Callback
User->>ChartRenderer: hover or focus point
ChartRenderer->>Tooltip: render transient tooltip with pinned=false
User->>ChartRenderer: click or press Enter
ChartRenderer->>Tooltip: render expanded tooltip with pinned=true
Tooltip->>Callback: provide shared tooltip context
User->>ChartRenderer: press Escape or click close
ChartRenderer->>Tooltip: dismiss and clear focus
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
tannerlinsley
changed the title
tannerlinsley
added
the
full-conformance
tannerlinsley
marked this pull request as ready for review
August 5, 2026 16:25
|
View your CI Pipeline Execution ↗ for commit f58bdb3
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/charts-core/docs/reference/focus-and-interaction.md (1)
139-353: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMove these documentation changes to the root
docs/tree.Direct edits under
packages/charts-core/docswill be overwritten by documentation synchronization. Update the matching root documentation files, then runpnpm docs:sync.
packages/charts-core/docs/reference/focus-and-interaction.md#L139-L353: remove the direct changes and regenerate this file from its rootdocs/source.packages/charts-core/docs/reference/marks/geo.md#L65-L65: remove the direct change and regenerate this file from its rootdocs/source.packages/charts-core/docs/reference/marks/text-frame-and-facet.md#L123-L134: remove the direct changes and regenerate this file from its rootdocs/source.As per coding guidelines, author public documentation only in the root
docs/tree and do not directly editpackages/charts-core/docs; usepnpm docs:sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/charts-core/docs/reference/focus-and-interaction.md` around lines 139 - 353, Move the documentation updates from packages/charts-core/docs/reference/focus-and-interaction.md lines 139-353, packages/charts-core/docs/reference/marks/geo.md line 65, and packages/charts-core/docs/reference/marks/text-frame-and-facet.md lines 123-134 into their corresponding source files under the root docs/ tree; remove the direct generated-file edits and run pnpm docs:sync to regenerate all three package documentation files.Source: Coding guidelines
🧹 Nitpick comments (6)
benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/tooltip-body.tsx (1)
59-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
consumptionBreakdownfor the four consumption detail rows.
consumptionBreakdown(month)inmodel.tsalready returnslabel,color, andvaluefor the same four segments, andview.tsxuses it for the nested chart. These fourDetailRowelements repeat those labels and colors. A future label or color change must then be applied in two places.♻️ Proposed refactor
- <DetailRow - color={energyColors.household} - label="Household" - value={formatEnergy(month.household)} - /> - <DetailRow - color={energyColors.heatPump} - label="Heat pump" - value={formatEnergy(month.heatPump)} - /> - <DetailRow - color={energyColors.hotWater} - label="Hot water" - value={formatEnergy(month.hotWater)} - /> - <DetailRow - color={energyColors.evCharging} - label="EV charging" - value={formatEnergy(month.evCharging)} - /> + {consumptionBreakdown(month).map((part) => ( + <DetailRow + key={part.id} + color={part.color} + label={part.label} + value={formatEnergy(part.value)} + /> + ))}Add the import:
import { consumptionBreakdown, energyColors, formatEnergy, formatPercent } from './model'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/tooltip-body.tsx` around lines 59 - 78, Update the tooltip component’s four consumption DetailRow elements to render by mapping over consumptionBreakdown(month), using each returned label, color, and value while preserving the existing DetailRow and formatting behavior. Reuse the existing consumptionBreakdown and related model imports instead of duplicating segment labels and colors.benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/view.tsx (1)
449-453: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
monthFromTargetinto the sharedmodel.tsmodule. Both renderers define the identical helper, which parses themonth:<id>conformance anchor. The shared root cause is that anchor parsing lives in each renderer instead of the shared model that already exportsisEnergyMonthId.
benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/view.tsx#L449-L453: remove the localmonthFromTargetdefinition and import it from./model.benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/recharts.ts#L782-L786: remove the duplicate definition and import the same helper from./model.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/view.tsx` around lines 449 - 453, Move monthFromTarget into the shared model.ts module, alongside isEnergyMonthId, and export it for both renderers. In benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/view.tsx lines 449-453, remove the local definition and import the shared helper; make the same removal and import change in recharts.ts lines 782-786, preserving the existing anchor parsing behavior.scripts/public-callback-contract.mjs (1)
782-812: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the category exemption out of the signature loop.
The
upstreamProtocolandserviceMethodtest at Lines 783 to 788 does not depend onparameters. It re-evaluates for every signature of the surface. Hoist it above the loop so the exemption reads as a surface-level rule.♻️ Proposed refactor
+ if ( + category === callbackCategories.upstreamProtocol || + category === callbackCategories.serviceMethod + ) { + continue + } + for (const parameters of surface.signatures) { - if ( - category === callbackCategories.upstreamProtocol || - category === callbackCategories.serviceMethod - ) { - continue - } if (parameters.length > 2) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/public-callback-contract.mjs` around lines 782 - 812, Move the upstreamProtocol/serviceMethod exemption from inside the signature loop to a surface-level conditional before iterating over surface.signatures, so those categories skip the entire signature validation once. Keep the existing positional-argument and second-argument checks unchanged for all other categories..changeset/brave-tooltips-expand.md (1)
7-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider recording the focused-rule change in this changeset.
The friction log entry F-213 states that
ruleXandruleYnow expose presentation-only focus points, sowhenFocused(ruleX(...), { match: "x" })renders. That is a user-visible behavior change in@tanstack/charts, but this changeset does not mention it. Add a short paragraph so the release notes describe it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/brave-tooltips-expand.md around lines 7 - 22, Add a short paragraph to the changeset documenting the user-visible focused-rule behavior: presentation-only focus points exposed by ruleX and ruleY now allow whenFocused(ruleX(...), { match: "x" }) to render. Keep the wording concise and scoped to this `@tanstack/charts` behavior.scripts/compare-plot-catalog.mjs (1)
1961-1979: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist the shared paint lookup and rename
patternId.
patternIdnow holds either a pattern id or a gradient id, andelement.ownerDocument.getElementById(patternId)runs twice on the gradient path. Resolve the referenced element once and rename the identifier to match its new meaning. Behavior stays the same.♻️ Proposed refactor
if (style.fill && style.fill !== 'none') { - const patternId = style.fill.match( + const paintId = style.fill.match( /url\((?:["'])?#([^"')]+)(?:["'])?\)/, )?.[1] - const patternPaint = patternId - ? element.ownerDocument - .getElementById(patternId) - ?.querySelector('rect, path') - : undefined + const paintElement = paintId + ? element.ownerDocument.getElementById(paintId) + : undefined + const patternPaint = paintElement?.querySelector('rect, path') if (patternPaint) return getComputedStyle(patternPaint).fill - const gradientStop = patternId - ? element.ownerDocument - .getElementById(patternId) - ?.querySelector('stop') - : undefined + const gradientStop = paintElement?.querySelector('stop') return gradientStop ? getComputedStyle(gradientStop).getPropertyValue('stop-color') : style.fill }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/compare-plot-catalog.mjs` around lines 1961 - 1979, In the fill-handling logic, rename patternId to a neutral reference identifier and resolve element.ownerDocument.getElementById(...) once into a shared referenced-element variable. Reuse that lookup for both the patternPaint and gradientStop queries, preserving the existing fallback and paint-resolution behavior.scripts/check-packed-consumers.mjs (1)
1092-1104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
formatGroupassertion to the packed tooltip contract.This fixture proves that packed declarations expose the shared context to
format. The changeset also changesformatGroup(points, context), but no fixture exercises it. Add aformatGroupcallback here so the packed-export gate covers both migrated tooltip formatters.♻️ Proposed addition
format(point, context) { point.datum.id.toUpperCase() point.xValue.toUpperCase() point.yValue.toFixed(0) context.pinned.valueOf() context.formatX(point.xValue) context.formatY(point.yValue) return point.datum.category }, + formatGroup(points, context) { + context.pinned.valueOf() + return points + .map((groupPoint) => context.formatY(groupPoint.yValue)) + .join(', ') + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-packed-consumers.mjs` around lines 1092 - 1104, Add a formatGroup callback alongside the existing tooltip.format fixture to exercise the packed tooltip contract, using the callback’s points and shared context to assert the migrated formatGroup signature and return a valid group result. Keep the existing format callback unchanged and ensure the fixture covers context access through formatGroup as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/view.tsx`:
- Around line 196-201: Update the interactive chart’s TooltipChart invocation to
pass chartWidth instead of input.width as initialWidth. Keep the existing
height, definition, and renderSvg wiring unchanged so the initial chart width
matches its container and energyDefinition’s tooltipPlacement calculation.
In `@docs/guides/migrating.md`:
- Around line 66-79: Update the breaking-change list in the migration guide to
include the tooltip content signature change from content(points) to
content(points, context). Explain that implementations should use the provided
context, including its pinned-state information when applicable, alongside the
existing format and formatGroup migration guidance.
In `@docs/reference/scales-guides-and-color.md`:
- Around line 453-460: Update the documentation sentence following the
ChartColorLegendContext callback signatures to distinguish the return values:
height returns a number, while only render returns one keyed SceneNode.
In `@packages/charts-core/docs/comparison.md`:
- Around line 15-22: Move the measured-revision table changes from
packages/charts-core/docs/comparison.md#L15-L22, the bundle-range change from
packages/charts-core/docs/comparison.md#L109, and the accessor documentation
from packages/charts-core/docs/concepts/data-and-channels.md#L45-L50 into the
corresponding root docs/comparison.md and docs/concepts/data-and-channels.md
sources, then run pnpm docs:sync to regenerate all copies; do not edit the
generated packages/charts-core/docs files directly.
In `@packages/charts-core/docs/guides/migrating.md`:
- Around line 77-79: Update the source migration guide under the root docs/ tree
to document that content receives ChartTooltipContentContext as its second
argument, alongside format and formatGroup; then run pnpm docs:sync to
regenerate this packages/charts-core/docs copy. Do not edit generated
documentation directly.
In `@packages/charts-core/docs/reference/scales-guides-and-color.md`:
- Around line 453-460: Move the legend contract text from
packages/charts-core/docs/reference/scales-guides-and-color.md lines 453-460
into its matching root docs/ source, and move the callback-contract text plus
remaining changes from packages/charts-core/docs/reference/types.md lines 16-22
into its matching root docs/ source. Regenerate both generated copies with pnpm
docs:sync; do not edit either generated file directly.
In `@packages/charts-core/src/tooltip.ts`:
- Around line 100-114: Update the native tooltip path’s
createTooltipContentContext call to pass the same options argument used by the
core renderer, ensuring callbacks receive consistent item-derived axis labels
across renderers. Add a native regression test covering the callback context
labels.
In `@packages/charts-core/src/types.ts`:
- Around line 790-791: Update the enclosing mark type and its focusPoints
declaration so focusPoints preserves the mark’s TDatum, TXValue, and TYValue
parameters instead of using the default ChartPoint type parameters; ensure the
renderer receives ChartPoint<TDatum, TXValue, TYValue>[] compatible with the
wrapped mark.
In `@scripts/public-callback-contract.mjs`:
- Around line 929-940: Guard the `checker.getTypeOfSymbolAtLocation` call inside
the `signature.parameters.map` callback with the same try/catch behavior used at
the other call sites, such as the logic around lines 607–611 and 650–657. When
type resolution throws for a synthetic or merged parameter symbol, treat that
parameter as a non-object bag or skip it so contract validation continues
instead of aborting.
In `@scripts/public-callback-contract.test.mjs`:
- Around line 17-37: Update the tests around inspectPublicCallableSurfaces so
the TypeScript inventory is computed once in shared beforeAll setup and reused
by this test and the existing test that uses the explicit 20-second timeout. Add
beforeAll to the Vitest import, store the resulting surfaces for both tests, and
remove the duplicate inspection call so the default-timeout test does not
rebuild the full program.
---
Outside diff comments:
In `@packages/charts-core/docs/reference/focus-and-interaction.md`:
- Around line 139-353: Move the documentation updates from
packages/charts-core/docs/reference/focus-and-interaction.md lines 139-353,
packages/charts-core/docs/reference/marks/geo.md line 65, and
packages/charts-core/docs/reference/marks/text-frame-and-facet.md lines 123-134
into their corresponding source files under the root docs/ tree; remove the
direct generated-file edits and run pnpm docs:sync to regenerate all three
package documentation files.
---
Nitpick comments:
In @.changeset/brave-tooltips-expand.md:
- Around line 7-22: Add a short paragraph to the changeset documenting the
user-visible focused-rule behavior: presentation-only focus points exposed by
ruleX and ruleY now allow whenFocused(ruleX(...), { match: "x" }) to render.
Keep the wording concise and scoped to this `@tanstack/charts` behavior.
In
`@benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/tooltip-body.tsx`:
- Around line 59-78: Update the tooltip component’s four consumption DetailRow
elements to render by mapping over consumptionBreakdown(month), using each
returned label, color, and value while preserving the existing DetailRow and
formatting behavior. Reuse the existing consumptionBreakdown and related model
imports instead of duplicating segment labels and colors.
In `@benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/view.tsx`:
- Around line 449-453: Move monthFromTarget into the shared model.ts module,
alongside isEnergyMonthId, and export it for both renderers. In
benchmarks/conformance/cases/84-pinned-nested-chart-tooltip/view.tsx lines
449-453, remove the local definition and import the shared helper; make the same
removal and import change in recharts.ts lines 782-786, preserving the existing
anchor parsing behavior.
In `@scripts/check-packed-consumers.mjs`:
- Around line 1092-1104: Add a formatGroup callback alongside the existing
tooltip.format fixture to exercise the packed tooltip contract, using the
callback’s points and shared context to assert the migrated formatGroup
signature and return a valid group result. Keep the existing format callback
unchanged and ensure the fixture covers context access through formatGroup as
well.
In `@scripts/compare-plot-catalog.mjs`:
- Around line 1961-1979: In the fill-handling logic, rename patternId to a
neutral reference identifier and resolve
element.ownerDocument.getElementById(...) once into a shared referenced-element
variable. Reuse that lookup for both the patternPaint and gradientStop queries,
preserving the existing fallback and paint-resolution behavior.
In `@scripts/public-callback-contract.mjs`:
- Around line 782-812: Move the upstreamProtocol/serviceMethod exemption from
inside the signature loop to a surface-level conditional before iterating over
surface.signatures, so those categories skip the entire signature validation
once. Keep the existing positional-argument and second-argument checks unchanged
for all other categories.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f6aae123-443d-4752-bee0-cf89cbc71086
📒 Files selected for processing (84)
.changeset/brave-tooltips-expand.mdAPI-FRICTION.mdCONTRIBUTING.mdPLAN.mdbenchmarks/bundle-size/universal-baseline.jsonbenchmarks/comparison/bundle-baseline.jsonbenchmarks/conformance/cases/31-linear-regression/tanstack.tsbenchmarks/conformance/cases/44-framed-scatter/tanstack.tsbenchmarks/conformance/cases/84-pinned-nested-chart-tooltip/case.jsonbenchmarks/conformance/cases/84-pinned-nested-chart-tooltip/model.test.tsbenchmarks/conformance/cases/84-pinned-nested-chart-tooltip/model.tsbenchmarks/conformance/cases/84-pinned-nested-chart-tooltip/recharts.tsbenchmarks/conformance/cases/84-pinned-nested-chart-tooltip/tooltip-body.tsxbenchmarks/conformance/cases/84-pinned-nested-chart-tooltip/view.tsxdocs/comparison.mddocs/concepts/data-and-channels.mddocs/concepts/grammar-of-graphics.mddocs/examples/interactive-charts.mddocs/guides/custom-marks-and-renderers.mddocs/guides/migrating.mddocs/guides/tooltips-and-focus.mddocs/reference/custom-extensions.mddocs/reference/focus-and-interaction.mddocs/reference/marks/geo.mddocs/reference/marks/text-frame-and-facet.mddocs/reference/scales-guides-and-color.mddocs/reference/types.mdexamples/sandbox/src/InteractionGeometryLab.test.tsexamples/sandbox/src/InteractionGeometryLab.tsxpackages/charts-core/docs/comparison.mdpackages/charts-core/docs/concepts/data-and-channels.mdpackages/charts-core/docs/concepts/grammar-of-graphics.mdpackages/charts-core/docs/examples/interactive-charts.mdpackages/charts-core/docs/guides/custom-marks-and-renderers.mdpackages/charts-core/docs/guides/migrating.mdpackages/charts-core/docs/guides/tooltips-and-focus.mdpackages/charts-core/docs/reference/custom-extensions.mdpackages/charts-core/docs/reference/focus-and-interaction.mdpackages/charts-core/docs/reference/marks/geo.mdpackages/charts-core/docs/reference/marks/text-frame-and-facet.mdpackages/charts-core/docs/reference/scales-guides-and-color.mdpackages/charts-core/docs/reference/types.mdpackages/charts-core/src/area-x.tspackages/charts-core/src/area.tspackages/charts-core/src/band.tspackages/charts-core/src/bar.tspackages/charts-core/src/dot.tspackages/charts-core/src/facet.test.tspackages/charts-core/src/facet.tspackages/charts-core/src/focus-disabled.tspackages/charts-core/src/focus-mark.test.tspackages/charts-core/src/focus.test.tspackages/charts-core/src/focus.tspackages/charts-core/src/geo.test.tspackages/charts-core/src/geo.tspackages/charts-core/src/hexagon.tspackages/charts-core/src/index.tspackages/charts-core/src/legend.tspackages/charts-core/src/line.tspackages/charts-core/src/link.test.tspackages/charts-core/src/mark.tspackages/charts-core/src/polar.tspackages/charts-core/src/rect.tspackages/charts-core/src/renderer.test.tspackages/charts-core/src/renderer.tspackages/charts-core/src/rule.tspackages/charts-core/src/runtime.test.tspackages/charts-core/src/scene.tspackages/charts-core/src/text.tspackages/charts-core/src/tooltip.tspackages/charts-core/src/type-contract.test.tspackages/charts-core/src/types.tspackages/charts-core/src/universal-types.tspackages/octane-charts/src/types.type-test.tspackages/react-charts-catalog/src/index.tspackages/react-charts/src/Chart.test.tsxpackages/react-native-charts/src/Tooltip.test.tspackages/react-native-charts/src/Tooltip.tsxpackages/react-native-charts/src/interaction.tsscripts/check-packed-consumers.mjsscripts/compare-plot-catalog.mjsscripts/measure-bundles.mjsscripts/public-callback-contract.mjsscripts/public-callback-contract.test.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
format(point, context),formatGroup(points, context), andcontent(points, context).Why
Built-in tooltip formatting and custom renderer content now receive the same pinned-state context. Callback contracts across the library use a consistent primary-value-plus-context shape.
Validation
pnpm validate— all 19 targets pass; 144 test files and 846 testspnpm benchmark:check— all 60 comparison bundles passpnpm conformance:quick -- --case=84-pinned-nested-chart-tooltip— visual, behavior, geometry, and type gates pass at both sizes and themes; 99.6% geometry similarity