diff --git a/.changeset/combo-mark-drill.md b/.changeset/combo-mark-drill.md new file mode 100644 index 0000000000..5feb5feaea --- /dev/null +++ b/.changeset/combo-mark-drill.md @@ -0,0 +1,36 @@ +--- +'@object-ui/plugin-charts': patch +--- + +Combo charts drill from their marks + +objectui#4692, ruled Option B. `AdvancedChartImpl` built `cartesianClickProps` once and +applied it to exactly one element — the final cartesian `ChartComponent`. The `combo` +branch returns earlier, from its own `ComposedChart`, which was rendered with `data` and +no click props at all, so a combo chart fired `onChartClick` never: not on a mark, not on +the axis. Its marks are the same `Bar` / `Line` / `Area` components the drillable branch +renders. + +The trap that made this worth fixing rather than documenting is that the family is +**derived**, not only authored: `effectiveChartFamily` resolves a chart to `combo` +whenever its series declare different families (objectui#2945), so adding `type: 'line'` +to one series of a drillable bar chart silently turned that chart's drill-through off — +nothing in the authored spec said drill had been touched, and nothing errored. + +A combo's `Bar` / `Line` / `Area` marks now emit `{ category, categoryId, series, value }` +with the same semantics the plain cartesian branch gives, reusing the item-level +series-identity machinery from objectui#4672 / objectui#4682: the mark handler records the +series it was rendered with, the chart-level handler composes the one event, so a gesture +still produces exactly one `onChartClick`. Retyping one series now changes that series' +mark and nothing else. + +**Only the marks drill.** A click on a combo's plot surface or axis stays silent, where +the plain cartesian branch falls back to its axis-level answer. A combo plots several +measures on one plot, so a surface click there has no single series to report and the +fallback would have to invent one — the same reasoning objectui#4672's ruling gave the +pivoted case. Combo also carries no chart-wide pointer cursor for that reason; the +affordance sits on the marks that answer. + +Radar is now the one cartesian-adjacent family with no click wiring. The `onChartClick` +doc comment, corrected in objectui#4705 to say combo was a no-op, states the new rule and +its one deliberate exception. diff --git a/packages/plugin-charts/src/AdvancedChartImpl.comboClickNoop.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.comboClickNoop.test.tsx deleted file mode 100644 index e5dec3129d..0000000000 --- a/packages/plugin-charts/src/AdvancedChartImpl.comboClickNoop.test.tsx +++ /dev/null @@ -1,161 +0,0 @@ -/** - * ObjectUI - * Copyright (c) 2024-present ObjectStack Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * Doc-truth pin: a combo chart's `ComposedChart` branch returns before - * `cartesianClickProps` is ever built (see `onChartClick`'s doc comment on - * `AdvancedChartImplProps`), so no click on a combo mark reaches - * `onChartClick` today — not the mark handler, not the axis handler. This - * file pins that state as it actually is, for both an EXPLICITLY authored - * combo and one DERIVED from mixed series families, and carries a positive - * control in the same file: an identically-shaped click on a plain bar chart - * DOES reach `onChartClick`, so the "not called" assertions below are proof - * of the combo branch's silence rather than of a broken click harness. - * - * Whether combo SHOULD drill is a product question this file takes no - * position on (see the linked issue) — it only pins what the renderer does - * today, so a future change to that answer has to touch this file on - * purpose rather than drift past it unnoticed. - */ - -import React from 'react'; -import { describe, it, expect, afterEach, vi } from 'vitest'; -import { render, cleanup, fireEvent, act } from '@testing-library/react'; -import type { ChartSegmentClickEvent } from '@object-ui/core'; - -// Recharts' ResponsiveContainer measures via ResizeObserver, which reports -// 0x0 under the headless DOM, so nothing paints. Fix its size. -vi.mock('recharts', async () => { - const actual = await vi.importActual('recharts'); - return { - ...actual, - ResponsiveContainer: ({ children }: any) => - React.cloneElement(children, { width: 480, height: 320 }), - }; -}); - -import AdvancedChartImpl from './AdvancedChartImpl'; - -afterEach(cleanup); - -/** - * recharts throttles pointer moves through `requestAnimationFrame`, and a - * click reads the tooltip state that move left behind — the same idiom the - * sibling click-payload test files use. - */ -const flushFrame = async () => { - await act(async () => { - await new Promise((r) => requestAnimationFrame(() => r())); - await new Promise((r) => setTimeout(r, 0)); - }); -}; - -const clickAt = async (el: Element, clientX: number, clientY: number) => { - fireEvent.mouseMove(el, { clientX, clientY }); - await flushFrame(); - fireEvent.click(el, { clientX, clientY }); - await flushFrame(); -}; - -const clickRect = async (rect: Element) => { - const x = Number(rect.getAttribute('x')) + Number(rect.getAttribute('width')) / 2; - const y = Number(rect.getAttribute('y')) + Number(rect.getAttribute('height')) / 2; - await clickAt(rect, x, y); -}; - -const DATA = [ - { month: 'Jan', revenue: 120, margin: 0.4 }, - { month: 'Feb', revenue: 80, margin: 0.9 }, -]; - -describe('AdvancedChartImpl — combo has no click wiring today (doc-truth pin)', () => { - it('fires nothing for a click on an EXPLICITLY authored combo\'s bar mark', async () => { - const clicks: ChartSegmentClickEvent[] = []; - const { container } = render( - clicks.push(ev)} - />, - ); - - const bar = container.querySelector('.recharts-bar .recharts-rectangle'); - expect(bar, 'a bar mark must actually render for this click to mean anything').toBeTruthy(); - await clickRect(bar!); - - const line = container.querySelector('.recharts-line-curve'); - expect(line, 'a line mark must actually render for this click to mean anything').toBeTruthy(); - await clickAt(line!, 300, 120); - - // The chart surface itself — an axis-level click — is checked too: combo's - // `ComposedChart` carries no `onClick` at all, so this is not expected to - // resolve to a category-only event the way a wired cartesian chart's does. - const surface = container.querySelector('.recharts-surface')!; - await clickAt(surface, 200, 30); - - expect(clicks).toHaveLength(0); - }); - - it('fires nothing for a click on a combo DERIVED from mixed series families', async () => { - // No `chartType: 'combo'` authored here — one series declares `type: - // 'line'` on an otherwise-bar chart, and `effectiveChartFamily` derives - // combo from that disagreement. This is the derived-family trap the doc - // comment now names: an author who only meant to change one series' mark - // loses drill on the whole chart along with it. - const clicks: ChartSegmentClickEvent[] = []; - const { container } = render( - clicks.push(ev)} - />, - ); - - // Confirm this really did derive to combo: one bar series, one line - // series — not two bars, which is what a non-derived read would draw. - expect(container.querySelectorAll('.recharts-bar').length).toBe(1); - expect(container.querySelectorAll('.recharts-line').length).toBe(1); - - const bar = container.querySelector('.recharts-bar .recharts-rectangle'); - expect(bar).toBeTruthy(); - await clickRect(bar!); - - expect(clicks).toHaveLength(0); - }); - - it('POSITIVE CONTROL: the identically-shaped click DOES fire on a plain (non-combo) bar chart', async () => { - // Same data, same click helper, same mark shape as the first case above — - // only the family differs (mixed -> combo vs. uniform -> bar). If this - // control failed too, the "not called" assertions above would be - // meaningless: they would just mean the click harness is broken, not - // that combo specifically has no wiring. - const clicks: ChartSegmentClickEvent[] = []; - const { container } = render( - clicks.push(ev)} - />, - ); - - const bar = container.querySelector('.recharts-bar .recharts-rectangle'); - expect(bar).toBeTruthy(); - await clickRect(bar!); - - expect(clicks).toHaveLength(1); - expect(clicks[0].category).toBe('Jan'); - expect(clicks[0].series).toBe('revenue'); - }); -}); diff --git a/packages/plugin-charts/src/AdvancedChartImpl.comboMarkClick.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.comboMarkClick.test.tsx new file mode 100644 index 0000000000..360c1903ed --- /dev/null +++ b/packages/plugin-charts/src/AdvancedChartImpl.comboMarkClick.test.tsx @@ -0,0 +1,327 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * objectui#4692's ruled half — a combo chart drills AT ITS MARKS, AND ONLY + * THERE. + * + * This file was `AdvancedChartImpl.comboClickNoop.test.tsx`: it pinned the + * state that card measured, where `ComposedChart` received no click props at + * all and every combo click was silent. The maintainer ruled Option B, so the + * pin is inverted here on purpose rather than deleted — a pin whose subject + * changes is the one place a behaviour change has to be argued explicitly. + * + * What is pinned now, in three layers: + * + * 1. **Marks emit.** A `Bar` / `Line` mark on a combo emits + * `{ category, categoryId, series, value }` with the same semantics the + * plain cartesian branch gives, reusing objectui#4672's item-level + * series-identity machinery. Covered for BOTH shapes the branch is + * reachable in — an explicitly authored `chartType: 'combo'`, and one + * DERIVED by `effectiveChartFamily` from series whose families disagree. + * The derived shape is the card's actual harm: an author who adds + * `type: 'line'` to one series of a drillable chart never wrote the word + * `combo`, and used to lose drill on the whole chart for it. + * 2. **The surface stays silent** (guard 1 of the ruling). A combo plots + * several measures on one plot, so an axis/surface click has no single + * series to report and the plain branch's axis fallback would have to + * invent one. Its own control sits beside it: the identically-shaped + * surface click on a plain bar chart DOES fire, so the silence below is a + * fact about the combo branch and not about a surface click this harness + * cannot deliver. + * 3. **The positive control from the no-op era is kept** — an identically + * shaped mark click on a plain (non-combo) bar chart still fires, so a + * regression that killed clicks everywhere reads as a broad failure here + * rather than as a combo-shaped one. + */ + +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup, fireEvent, act } from '@testing-library/react'; +import { + buildChartSeries, + findChartSeriesRow, + type ChartSegmentClickEvent, +} from '@object-ui/core'; + +// Recharts' ResponsiveContainer measures via ResizeObserver, which reports +// 0x0 under the headless DOM, so nothing paints. Fix its size. +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +import AdvancedChartImpl from './AdvancedChartImpl'; + +afterEach(cleanup); + +/** + * recharts throttles pointer moves through `requestAnimationFrame`, and a + * click reads the tooltip state that move left behind — the same idiom the + * sibling click-payload test files use. + */ +const flushFrame = async () => { + await act(async () => { + await new Promise((r) => requestAnimationFrame(() => r())); + await new Promise((r) => setTimeout(r, 0)); + }); +}; + +const clickAt = async (el: Element, clientX: number, clientY: number) => { + fireEvent.mouseMove(el, { clientX, clientY }); + await flushFrame(); + fireEvent.click(el, { clientX, clientY }); + await flushFrame(); +}; + +const clickRect = async (rect: Element) => { + const x = Number(rect.getAttribute('x')) + Number(rect.getAttribute('width')) / 2; + const y = Number(rect.getAttribute('y')) + Number(rect.getAttribute('height')) / 2; + await clickAt(rect, x, y); +}; + +/** + * A click on the plot SURFACE, above the marks — the axis-level gesture guard 1 + * rules silent on a combo. The y is kept in the header band so it cannot land + * on a bar by accident; the x is a real tick's, so a wired chart resolves a + * category from it (which the control below relies on). + */ +const clickSurface = async (container: HTMLElement) => { + const surface = container.querySelector('.recharts-surface'); + expect(surface, 'the plot surface must exist for this click to mean anything').toBeTruthy(); + await clickAt(surface!, 200, 30); +}; + +const DATA = [ + { month: 'Jan', revenue: 120, margin: 0.4 }, + { month: 'Feb', revenue: 80, margin: 0.9 }, +]; + +/** An EXPLICITLY authored combo — `chartType: 'combo'`, marks named per series. */ +const renderExplicitCombo = (onChartClick: (ev: ChartSegmentClickEvent) => void) => + render( + , + ); + +/** + * A DERIVED combo, built the way the card's trap is sprung: an ordinary + * two-dimension pivot (the shape ADR-0021 introduced, whose rows carry the + * bucket identity a drill lookup needs), with ONE series given a different + * mark. Nothing here says `combo`; `effectiveChartFamily` derives it. + */ +const PIVOT_RAW = [ + { status: 'Open', priority: 'High', est_hours: 3 }, + { status: 'Open', priority: 'Low', est_hours: 5 }, + { status: 'Done', priority: 'High', est_hours: 7 }, + { status: 'Done', priority: 'Low', est_hours: 11 }, +]; +const DIMS = ['status', 'priority']; +const VALS = ['est_hours']; + +function renderDerivedCombo(onChartClick: (ev: ChartSegmentClickEvent) => void) { + const { data, xAxisKey, series } = buildChartSeries(PIVOT_RAW, DIMS, VALS); + // The whole edit an author makes: one series' mark, on a chart authored + // `bar`. `series[0]` stays un-annotated, exactly as `buildChartSeries` left + // it — the disagreement is what derives the family. + const mixed = series.map((s: any, i: number) => (i === 1 ? { ...s, chartType: 'line' } : s)); + const { container } = render( + , + ); + return { container, series: mixed }; +} + +describe('AdvancedChartImpl — an EXPLICIT combo drills at its marks (objectui#4692)', () => { + it('a bar mark emits { category, series, value } — was silent before the ruling', async () => { + const clicks: ChartSegmentClickEvent[] = []; + const { container } = renderExplicitCombo((ev) => clicks.push(ev)); + + const bar = container.querySelector('.recharts-bar .recharts-rectangle'); + expect(bar, 'a bar mark must actually render for this click to mean anything').toBeTruthy(); + await clickRect(bar!); + + expect(clicks).toHaveLength(1); + expect(clicks[0].category).toBe('Jan'); + expect(clicks[0].series).toBe('revenue'); + expect(clicks[0].value).toBe(120); + }); + + it('a line mark emits ITS OWN series, not the bar it shares the plot with', async () => { + // The discriminating case for a combo specifically: two marks of different + // families over one tick. An implementation that emitted the axis answer + // would name `revenue` (or nothing) for this click. + const clicks: ChartSegmentClickEvent[] = []; + const { container } = renderExplicitCombo((ev) => clicks.push(ev)); + + const line = container.querySelector('.recharts-line-curve'); + expect(line, 'a line mark must actually render for this click to mean anything').toBeTruthy(); + await clickAt(line!, 300, 120); + + expect(clicks).toHaveLength(1); + expect(clicks[0].series).toBe('margin'); + // The category and the value are read from different places — the tick from + // the chart-level payload, the cell from the row it selects — so pinning + // that they agree is worth more than pinning either alone. + const row = DATA.find((d) => d.month === clicks[0].category); + expect(row, `category ${String(clicks[0].category)} must be a real tick`).toBeTruthy(); + expect(clicks[0].value).toBe(row!.margin); + }); + + it('emits exactly ONE event per gesture — item records, chart emits', async () => { + // Both handlers fire for one mark click (item first, chart second — + // measured on recharts 3.10.1). Wiring combo the naive way (emitting from + // the item handler as well) would double every drill. + const clicks: ChartSegmentClickEvent[] = []; + const { container } = renderExplicitCombo((ev) => clicks.push(ev)); + + await clickRect(container.querySelector('.recharts-bar .recharts-rectangle')!); + + expect(clicks).toHaveLength(1); + }); + + it('NEGATIVE PIN (guard 1): a click on the surface / axis stays silent', async () => { + // Ruled, not incidental: a combo plots several measures on one plot, so the + // axis-level fallback the plain branch keeps has no single series to name + // here. See the control at the bottom of this file — the same gesture on a + // plain bar chart DOES fire, so this zero is combo's rule, not the + // harness's limit. + const clicks: ChartSegmentClickEvent[] = []; + const { container } = renderExplicitCombo((ev) => clicks.push(ev)); + + await clickSurface(container); + + expect(clicks).toHaveLength(0); + }); +}); + +describe('AdvancedChartImpl — a DERIVED combo drills too (objectui#4692, the card’s trap)', () => { + it('resolves the drill end to end after ONE series changed its mark', async () => { + const clicks: ChartSegmentClickEvent[] = []; + const { container, series } = renderDerivedCombo((ev) => clicks.push(ev)); + + // Stated, not assumed: the second dimension became the series axis, and the + // mixed families really did derive a combo — one bar group and one line + // group, where a non-derived read would have drawn two bars. + expect(series.map((s: any) => s.dataKey)).toEqual(['High', 'Low']); + expect(container.querySelectorAll('.recharts-bar')).toHaveLength(1); + expect(container.querySelectorAll('.recharts-line')).toHaveLength(1); + + // The High segment of the Done column — 7 hours. + const bars = container.querySelectorAll('.recharts-bar .recharts-rectangle'); + expect(bars).toHaveLength(2); + await clickRect(bars[1]); + + expect(clicks).toHaveLength(1); + expect(clicks[0].category).toBe('Done'); + expect(clicks[0].series).toBe('High'); + expect(clicks[0].value).toBe(7); + + // The harm the card reports is a dead DRILL, so the lookup is part of the + // assertion — the same end-to-end check objectui#4672's file makes for the + // plain branch. Before this change there was no event to look anything up + // with. + const idx = findChartSeriesRow( + PIVOT_RAW, + DIMS, + VALS, + clicks[0].category, + clicks[0].series, + { bucketId: clicks[0].categoryId }, + ); + expect(idx).toBe(2); + expect(PIVOT_RAW[idx]).toEqual({ status: 'Done', priority: 'High', est_hours: 7 }); + }); + + it('the line series that DERIVED the combo is itself drillable', async () => { + // The series the author retyped is the one most likely to be clicked next, + // and it is the one whose mark family changed under it. + const clicks: ChartSegmentClickEvent[] = []; + const { container } = renderDerivedCombo((ev) => clicks.push(ev)); + + const line = container.querySelector('.recharts-line-curve'); + expect(line).toBeTruthy(); + await clickAt(line!, 300, 120); + + expect(clicks).toHaveLength(1); + expect(clicks[0].series).toBe('Low'); + }); + + it('NEGATIVE PIN (guard 1): the derived combo’s surface is silent too', async () => { + const clicks: ChartSegmentClickEvent[] = []; + const { container } = renderDerivedCombo((ev) => clicks.push(ev)); + + await clickSurface(container); + + expect(clicks).toHaveLength(0); + }); +}); + +describe('AdvancedChartImpl — combo click controls (both directions)', () => { + it('POSITIVE CONTROL: a mark click on a plain (non-combo) bar chart fires', async () => { + // Kept from the no-op era of this file. If this failed alongside the combo + // cases above, those would be measuring a broken harness rather than the + // combo branch. + const clicks: ChartSegmentClickEvent[] = []; + const { container } = render( + clicks.push(ev)} + />, + ); + + const bar = container.querySelector('.recharts-bar .recharts-rectangle'); + expect(bar).toBeTruthy(); + await clickRect(bar!); + + expect(clicks).toHaveLength(1); + expect(clicks[0].category).toBe('Jan'); + expect(clicks[0].series).toBe('revenue'); + }); + + it('CONTROL for the negative pins: the SAME surface click DOES fire on a plain bar chart', async () => { + // Without this, the two `toHaveLength(0)` pins above would pass for the + // empty reason — a surface click that reaches nothing on ANY chart. The + // plain branch answers it with the axis-level event (category + identity), + // which is exactly the answer guard 1 withholds from a multi-measure combo. + const clicks: ChartSegmentClickEvent[] = []; + const { container } = render( + clicks.push(ev)} + />, + ); + + await clickSurface(container); + + expect(clicks).toHaveLength(1); + expect(clicks[0].category).toBe('Jan'); + }); +}); diff --git a/packages/plugin-charts/src/AdvancedChartImpl.tsx b/packages/plugin-charts/src/AdvancedChartImpl.tsx index c2f3d818b2..baf47ce1ee 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -248,18 +248,30 @@ export interface AdvancedChartImplProps { * Optional drill-down click handler. Fires when a chart segment is clicked * with `{ category, categoryId, series, value }`. Wired for * bar/horizontal-bar/line/area/pie/donut/funnel/scatter/treemap/sankey — - * each has its own click handler attached to its mark(s) below. Radar and - * combo are no-ops in L1: neither branch attaches a click handler anywhere, - * even though combo renders the same `Bar`/`Line`/`Area` marks the wired - * cartesian branch does — its `ComposedChart` element (see the - * `chartType === 'combo'` branch) is built and returned before - * `cartesianClickProps` is applied to anything. + * each has its own click handler attached to its mark(s) below — and for + * `combo`, whose `Bar`/`Line`/`Area` marks are the same components the plain + * cartesian branch renders and now emit the same event with the same + * semantics (ruled on objectui#4692). * - * The combo branch is reachable without authoring `chartType: 'combo'`: - * `effectiveChartFamily` derives it whenever a series' own family disagrees - * with the chart's own (see its doc comment), so giving one series of an - * otherwise-drillable chart a different `type` silently turns that chart's - * drill off too — nothing in the authored spec says drill was touched. + * `combo` differs from the plain cartesian branch in exactly ONE way, and + * deliberately: **only its marks drill.** A click on the plot surface or an + * axis stays silent there, where the plain branch would fall back to its + * axis-level answer. A combo plots several measures on one plot, so a + * surface click has no single series answer to give — the same reasoning + * objectui#4672's ruling used for the pivoted case. The plain branch keeps + * that fallback because its own marks share one measure, so its axis answer + * is either unambiguous or explicitly named by recharts. + * + * That combo drills at all is what keeps its DERIVATION from costing an + * interaction. The branch is reachable without authoring + * `chartType: 'combo'`: `effectiveChartFamily` derives it whenever a series' + * own family disagrees with the chart's own (see its doc comment), so giving + * one series of a drillable chart a different `type` used to turn that + * chart's drill off with nothing in the authored spec saying drill was + * touched. It now changes the mark and nothing else. + * + * Radar is the one remaining no-op in L1 — its branch attaches no click + * handler anywhere. */ onChartClick?: (event: ChartSegmentClickEvent) => void; /** @@ -480,7 +492,22 @@ function AdvancedChartImplInner({ ? (s: NormalizedSeries) => ({ onClick: handleMarkClick(s) }) : () => ({}); - const handleCartesianClick = React.useCallback((payload: any, event?: any) => { + // Compose-and-emit for a chart-level cartesian click. TWO branches reach it, + // and they differ in one rule only — `requireMark` (objectui#4692): + // + // - the plain branch (bar / horizontal-bar / line / area) emits for EVERY + // click, falling back to the axis answer described above when the gesture + // landed on no mark; + // - `combo` emits ONLY for a gesture that landed on a mark. Its plot carries + // several measures, so a surface/axis click there has no single series to + // report and the fallback would have to invent one — the same reasoning + // objectui#4672's ruling gave the pivoted case. + // + // Sharing the composer rather than writing combo its own is the point: a + // combo mark's click IS a cartesian mark's click (same components, same + // recorded series identity), so there is one event shape and one emit site, + // and the two branches disagree about reachability alone. + const emitCartesianClick = React.useCallback((payload: any, event: any, requireMark: boolean) => { if (!onChartClick || !payload) return; // A click with no active tick (the plot margins, an axis label) reports a // NULL index, not an absent one — and `Number(null)` is 0, which would @@ -496,6 +523,9 @@ function AdvancedChartImplInner({ clickedMark.current = null; const gesture = gestureIdOf(event); const onMark = mark != null && gesture !== undefined && mark.gesture === gesture; + // The record is cleared above whether or not it is used, so a combo's + // silent surface click cannot leave a stale series behind for the next one. + if (requireMark && !onMark) return; const clickedKey = onMark ? mark!.dataKey : resolveClickedSeriesKey(payload.activeDataKey, series); @@ -511,6 +541,14 @@ function AdvancedChartImplInner({ }); }, [onChartClick, data, series]); + const handleCartesianClick = React.useCallback((payload: any, event?: any) => { + emitCartesianClick(payload, event, false); + }, [emitCartesianClick]); + + const handleComboClick = React.useCallback((payload: any, event?: any) => { + emitCartesianClick(payload, event, true); + }, [emitCartesianClick]); + // A pie sector's `payload` is a SPREAD COPY of the data row (recharts builds // it as `{...entry, ...cellProps}`), which is precisely why the bucket // identity is an ordinary enumerable property — see `CHART_BUCKET_ID_KEY`. @@ -531,6 +569,14 @@ function AdvancedChartImplInner({ }, [onChartClick, xAxisKey, series]); const cartesianClickProps = onChartClick ? { onClick: handleCartesianClick, style: { cursor: 'pointer' as const } } : {}; + // Combo carries NO chart-wide pointer cursor, unlike every other wired + // family: on this plot only the marks answer a click, and a surface-wide + // pointer would promise a drill the axis deliberately does not perform. The + // affordance rides on the marks instead — see `comboMarkClickProps`. + const comboClickProps = onChartClick ? { onClick: handleComboClick } : {}; + const comboMarkClickProps = onChartClick + ? (s: NormalizedSeries) => ({ ...markClickProps(s), cursor: 'pointer' as const }) + : () => ({}); const pieClickProps = onChartClick ? { onClick: handlePieClick, style: { cursor: 'pointer' as const } } : {}; // Per-category colour: a select/lookup dimension's option colour (passed via @@ -1111,7 +1157,13 @@ function AdvancedChartImplInner({ host mixed marks. Under `BarChart` an `` child renders nothing at all, so the `seriesType === 'area'` arm below was unreachable — an authored combo with an `area` series drew a blank series. */} - + {/* `comboClickProps`, not `cartesianClickProps`: the chart-level + handler here emits ONLY for a gesture a mark recorded, so an axis / + surface click stays silent (objectui#4692's ruling). The emit still + happens at chart level because that is the only place the CATEGORY + is known — a line/area item handler is handed the curve's props and + no datum. */} + @@ -1148,20 +1200,20 @@ function AdvancedChartImplInner({ if (seriesType === 'line') { return ( - + {dataLabel(valueFormatter)} ); } if (seriesType === 'area') { return ( - + {dataLabel(valueFormatter)} ); } return ( - + {dataLabel(valueFormatter)} );