Uh oh!
There was an error while loading. Please reload this page.
docs(charts): correct onChartClick wiring doc, pin combo's no-click state - #4705
Conversation
…tate AdvancedChartImpl's onChartClick doc comment claimed scatter/treemap/sankey were no-ops, but each already has a wired click handler; it also never mentioned that combo is the actual no-op, or that a chart's family can be silently derived to combo when a series' own type disagrees with the chart's family (effectiveChartFamily), turning off drill on the whole chart along with the mark change. Corrects the doc comment to the measured wiring and adds a pinning test for combo's current no-click-props state, with a positive control in the same file (an identically-shaped click on a plain bar chart still fires onChartClick) so the "not called" assertions are proof of the combo branch's silence rather than a broken harness. No renderer behavior changes: ComposedChart still receives no click props. Whether combo should drill is a product question left open. Part of #4692 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQ
The latest updates on your projects. Learn more about Vercel for GitHub. |
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
commented
Aug 15, 2026
Review verdict: ACCEPT (reviewer of record, PM session Verified against this PR, not the report: body's first line is Landing note (PM to-do, recorded per protocol): after merge, Will flip ready + enable auto-merge once Lint, Type Check, and Test shards 1–4 conclude Generated by Claude Code |
yinlianghui
commented
Aug 15, 2026
CI note (PM, reviewer of record): Failed jobs re-queued on the same head ( Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Part of #4692
What
AdvancedChartImpl'sonChartClickdoc comment claimed scatter/treemap/sankey chart types are click no-ops, but each already has a wired click handler (scatterClickProps/treemapClickProps/sankeyClickProps). The comment never mentioned that combo is the actual no-op: itsComposedChartbranch returns beforecartesianClickPropsis ever built, even though it renders the sameBar/Line/Areamarks the wired cartesian branch does.This corrects the comment to the measured wiring (bar/horizontal-bar/line/area/pie/donut/funnel/scatter/treemap/sankey are wired; radar and combo are not) and names a trap:
effectiveChartFamilyderivescombowhenever a series' owntypedisagrees with the chart's own family, so giving one series a different mark type silently turns off drill on the entire chart — nothing in the authored spec says drill was touched.Scope
This is the doc-truth half only, per the maintainer's dispatch ruling on #4692. It does not wire any click props onto
ComposedChart. Whether combo should drill is a product question left open on #4692 for a follow-up decision.Tests
Added
AdvancedChartImpl.comboClickNoop.test.tsx, pinning combo's current no-click-props state:chartType: "combo"fires nothing on a bar mark, a line mark, or the plot surface (axis-level);type: 'line'on an otherwise-bar chart) also fires nothing;onChartClick— proving the "not called" assertions above pin the combo branch's actual silence, not a broken test harness.No renderer behavior changes.
Verification (at
5ecbc4d71)Changeset:
.changeset/combo-drill-doc-truth.md(empty frontmatter — docs/test-only, releases nothing).Generated by Claude Code
Generated by Claude Code