Uh oh!
There was an error while loading. Please reload this page.
fix(charts): build the cartesian drill event from the payload recharts 3 actually sends - #4680
Merged
Merged
Conversation
…s 3 sends `AdvancedChartImpl.handleCartesianClick` read `payload.activePayload[0]` for both the clicked series and its value. That is a recharts 2 field; recharts 3 hands a chart-level `onClick` a `MouseHandlerDataParam` and nothing else, so the read was `undefined` on every cartesian click and every bar/line/area drill event carried `series: undefined, value: undefined`. The call site types the payload `any`, so nothing went red. The value is now read off the clicked row (`data[activeTooltipIndex]`) for the resolved measure; the series comes from `activeDataKey` when the payload carries one, and from the chart's own series list when it plots exactly one. A multi-series chart under the shared cursor is left unresolved rather than guessed. Also: a click with no active tick reports a NULL index, and `Number(null)` is 0, so such a click resolved to bucket zero and drilled records the user never clicked. Only a real index selects a row now. Part of objectui#4672
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 15, 2026 08:30
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Part of #4672
Part of, notFixes: the card's headline harm — the dead click on a PIVOTEDdataset chart — is not resolved here, and the card stays open for it. What is
resolved is everything the recharts 3 payload can answer; the half it cannot is
measured, argued and escalated below.
What was wrong
AdvancedChartImpl.handleCartesianClickbuilt its drill event frompayload.activePayload[0]— a recharts 2 field. This package is on recharts3.10.1, whose chart-level
onClickreceives aMouseHandlerDataParam:{ activeCoordinate, activeDataKey, activeIndex, activeLabel, activeTooltipIndex, isTooltipActive }and nothing else. Re-verified onorigin/mainat9ce096fb0: the read is still there, andactivePayloadappears nowhere in theshipped
lib/ortypes/except an unrelated local incomponent/Cursor.js. Sothe read was
undefinedon every cartesian click, and every bar / line / areadrill event carried
series: undefined, value: undefined. Nothing went red — thepayload is typed
anyat the call site, and every existing drill test eithercalls the pure lookup directly or stubs the chart component.
What this PR changes
data[activeTooltipIndex], thiscomponent's own array — for the resolved measure, the same way the bucket
identity has been read since The chart null-category bucket has two identity collisions: it merges a null group with an empty-string group, and collides with a stored value spelling the label #4508.
activeDataKeywhen the payload carries one, andotherwise from the chart's own series list when it plots exactly one
series, where the clicked column can belong to nothing else.
zero — see "Adjacent fix" below.
The measurement (recharts 3.10.1, as installed)
Driving real recharts charts in the DOM and reading the payload the chart-level
handler is actually given:
activeDataKeyactiveTooltipIndex"1""0""1""1""1"Tooltipelement at all"1"Tooltip shared={false})"b""1"The mechanism, not just the observation: a chart-level cartesian click is an
axis interaction, and recharts dispatches those with
activeDataKey: undefinedhard-coded —setMouseClickAxisIndex({ activeIndex, activeDataKey: undefined, … })inlib/state/mouseEventsMiddleware.js, mirrored for hover insetMouseOverAxisIndex. Only an item interaction carries adataKey(
useMouseClickItemDispatchinlib/context/tooltipContext.js), and theselector only reads item state when the tooltip event type is
item, i.e. whenshared={false}. These charts render the shared cursor. recharts says so itselfin
lib/cartesian/Bar.js: "With shared Tooltip, the activeDataKey isundefined."
So the card's premise held with a wider radius than filed:
activeDataKeyisnot merely ambiguous under a shared tooltip, it is absent for every cartesian
click these charts produce, single-series included.
Also measured (the index arrives as a string,
"1"not1, and is null— not absent — when no tick is active). Both are pinned in the tests.
What is still open (the reason this is
Part of)A chart plotting several series under the shared cursor: the payload names no
series, and the pivoted drill lookup requires one —
findChartSeriesRow's pivot arm isString(r[gDim] ?? '') === s. Measured onthe card's repro shape (
dimensions: ['status','priority'], values: ['est_hours']):series=undefinedreturns-1(the dead click),series='Low'returns3,series='High'returns2.The series is left unresolved rather than guessed. Filling it with
series[0]would turn a dead click into a click that drills another group'srecords — a wrong answer is worse than no answer, and the user did click
something specific. Resolving it needs the clicked mark (an item-level
onClickon eachBar/Line/Area), which changes what a cartesian drillmeans — hit targets, the plot-area fallback, double-fire suppression. That is a
contract decision, not an implementation detail, so it is escalated on the card
rather than guessed here.
Adjacent fix, named (same handler, same defect class)
A click with no active tick — plot margins, an axis label — reports
activeTooltipIndex: null, andNumber(null)is0. The index read (landed by#4677) therefore resolved such a click to bucket zero and drilled the first
bucket's records. Evidence it is wrong rather than intended:
AdvancedChartImpl.bucketIdentity.test.tsxstates the intended behaviour inprose — "No active tick (a click on empty plot area) resolves to no identity
rather than to bucket zero" — but asserts it with
undefined, which takes theNaNpath and never exercisesnull, which is what recharts actually sends.Fixed in place (same file, same handler, same gate families, mechanical): only a
non-null index selects a row. Pinned by test 5 below.
Tests
New:
packages/plugin-charts/src/AdvancedChartImpl.cartesianClickPayload.test.tsxAdvancedChartImpl— nostub between the click and the assertion — reports
series: 'est_hours'andvalue: 30.value: 10(the value follows thebucket, which is why it is read out of
data[activeTooltipIndex]).activeDataKeyarm, over a payload captured verbatim from a realper-series-cursor click, on a pivoted chart:
series: 'Low',value: 11.undefined— the non-guessing contract, pinned so a later "helpful" defaultcannot land silently.
activeTooltipIndex: null) over the The chart null-category bucket has two identity collisions: it merges a null group with an empty-string group, and collides with a stored value spelling the label #4508 collision fixture— the only fixture where "bucket zero" and "no bucket" are distinguishable —
reports no identity.
multi-series click reports an index and a label and no
activeDataKey,and no
activePayloadat all. This is the premise the open half rests on, andthe thing to re-measure on a recharts upgrade.
The tests flush a
requestAnimationFramebetween the pointer move and the click,because recharts throttles pointer moves through rAF (
eventSettingsSlice:throttleDelay: 'raf') and the click reads the state that move left behind. Areal browser never sees the un-flushed shape; a test that skips it measures
activeTooltipIndex: nullfor every click and proves nothing.Reverse verification (direction predicted before running: red on 1, 2, 3, 5;
green on 4 and 6, since neither depends on this change). Restoring the
activePayloadhandler on top of the new tests:Tests 4 failed | 2 passed (6)— exactly those four, with test 5 failing as
expected '["(None)"]' to be undefined, i.e. the wrong-bucket drill made visible. Restored byte-identicallyfrom the commit (empty
git diff HEAD) and re-run green.Local verification — all at
0440c7654(this PR's head)plugin-dashboardis included becauseDatasetWidget.handleChartDrillis thisevent's production consumer. It needed no change: it already reads
ev.seriesand
ev.categoryId, and the fix only changes what reaches it.Scope
AdvancedChartImpl.tsx(thehandleCartesianClickregion and one module-localhelper beside it) plus the new test and the changeset. No core files — the
ChartSegmentClickEventcontract is unchanged and needed no new field. The pie,funnel, scatter, treemap and sankey paths are untouched; they build their events
from the clicked entry, not from this payload.
Generated by Claude Code
Generated by Claude Code