Container | XY: Support secondary Y axis - #130
devgru wants to merge 1 commit into
Conversation
A component with `useSecondaryYScale` is bound to the container's second Y scale, `yScaleSecondary`, instead of `yScale`. The secondary scale keeps the primary one's range but gets its own domain, computed from just the components that opted in, or set explicitly with `ySecondaryDomain`. An Axis with the same flag becomes `yAxisSecondary` and is rendered against that scale, so a chart can carry two unrelated units at once — volume bars against a count axis on the right, yields against a percentage axis on the left. The domain pass, previously one loop over both dimensions, is extracted into `_applyScaleDomain` so the Y dimension can run twice: once for the primary components and once for the secondary ones. Without a secondary component it runs exactly as before.
|
Reviewed the core and React changes. The _applyScaleDomain extraction is clean, and the X-then-Y order is preserved, which matters because scaleByDomain reads the X domain when computing the Y extent. Type-check and lint on the changed files are clean on my side as well. Issues with the current state, most important first: 1. The Crosshair maps every series through the primary Y scale. The container builds crosshair.accessors.y from all components (xy-container/index.ts#L273-L278), and Crosshair.getCircleData and the CrosshairSnapMode.XY nearest search run those values through the crosshair's single yScale (crosshair/index.ts#L375, #L387, #L156). With the PR's own verification setup (a StackedBar on the secondary scale with domain [0, 850] and a Line on the primary with [85, 93]), the bar's crosshair circles are computed against the line's domain and land far outside the plot. The tooltip is unaffected. The crosshair needs a scale per accessor to be correct here. 2. The initial-render guard doesn't know about the secondary axis. The constructor renders only when xAxis, yAxis or component data are present (index.ts#L85-L91). A container configured with just yAxisSecondary and no data won't render until something else triggers it. 3. _isSecondaryY identity fallback. The c === this.config.yAxisSecondary check and the as unknown as T cast (index.ts#L306-L308) exist because the TS API path never sets the flag on the axis. Setting this.config.yAxisSecondary.config.useSecondaryYScale = true in updateContainer, next to the type = AxisType.Y assignment on L154, would let the predicate be just the flag. 4. Config naming and parity. yScaleSecondary uses a suffix while ySecondaryDomain uses an infix. The secondary domain also has no MinConstraint / MaxConstraint counterparts (_applyScaleDomain is called with undefined, undefined for them on L373), and yRange / yDirection are implicitly shared. Worth aligning the names and documenting what is shared before this becomes public API. 5. Two Y axes on the same side overlap. _setAutoMargin takes the max per side rather than the sum, and Axis.getOffset places both at the same origin. Since position defaults to left for both, forgetting position: 'right' on the secondary axis silently draws them on top of each other. A console.warn or at least a doc note would help. 6. Generated wrappers need pnpm generate. useSecondaryYScale was added to XYComponentConfigInterface, which changes every generated component. Angular lists its @Inputs explicitly, so the flag won't pass through there until the wrappers are regenerated. (The Svelte / Vue / Solid containers key axes by config[`${type}Axis`], so a second Y axis overwrites the first. I see that is on the checklist already.) 7. Nits.
|
Sorry, something went wrong.
|
Suggestion: let yAxis and yScale accept arrays instead of adding Secondary variants. The _applyScaleDomain extraction in this PR already does the hard part: it computes a domain for an arbitrary group of components. What is left hard-coded is that there are exactly two groups, and that the second one is addressed by a boolean and three *Secondary keys. Since the Y-axis-on-the-right case is really just "N Y scales", I'd propose expressing it as arrays from the start, so a third axis (or two on one side) needs no new API. Container config yScale?: ContinuousScale | ContinuousScale[]
yDomain?: Domain | Domain[] // same for yDomainMinConstraint / yDomainMaxConstraint
yAxis?: Axis<Datum> | Axis<Datum>[]The single-value forms stay valid and mean "index 0", so existing charts are untouched. One private normaliser turns them into arrays and everything below it works with arrays only (isArray from @/utils/data, per the repo rule; for yDomain the check is isArray(yDomain[0]), since a plain domain is a tuple of numbers). Component config useSecondaryYScale?: boolean becomes yScaleIndex?: number (default 0). Components pick their scale by index; yAxis[i] renders yScale[i], so the axis needs no flag at all: its position in the array is the binding. If ordering by array position feels fragile, an optional id on the Axis plus yScaleId on components is the same design with names instead of indices, but the array is the smaller change and already matches how updateComponents pairs configs to components by index. Container internals, mapped onto this diff
Wrappers They get simpler rather than more complex. React collects all vis-axis children with type === 'y' into one array instead of two .find calls (index.tsx#L60-L67); Angular uses filter instead of find; the Svelte / Vue / Solid containers push into config.yAxis instead of writing to a keyed ${type}Axis slot, which also removes the "second Y axis overwrites the first" problem. Child order defines the index, so <VisAxis type="y" /> followed by <VisAxis type="y" position="right" /> gives indices 0 and 1. Why now Nothing in _applyScaleDomain is Y-specific, so the same normaliser would later let xAxis / xScale take arrays for top-and-bottom X axes without another round of API. The MCP chart spec also maps onto yAxis: [...] directly, whereas a boolean does not. And once useSecondaryYScale / yScaleSecondary / ySecondaryDomain / yAxisSecondary ship, they have to be kept as aliases indefinitely, so switching to arrays is cheapest before this leaves draft. |
Sorry, something went wrong.
Lets one XY chart carry two unrelated units: volume bars measured against a count axis on the right, yield lines against a percentage axis on the left. Today the only way to get that is what the repo's own Dual Axis Chart dev example does — stack two absolutely-positioned XYContainers, each with its own data, margins, tooltip and crosshair, and keep them aligned by hand.
Opting a component in. useSecondaryYScale on any XY component config binds that component to the container's yScaleSecondary instead of yScale. The secondary scale shares the primary one's range, so both occupy the same plot area, but gets its own domain — computed from the opted-in components alone, or pinned explicitly with ySecondaryDomain. Components without the flag are unaffected, and a chart that sets it nowhere behaves exactly as before.
The axis. An Axis with type: 'y' and useSecondaryYScale becomes the container's yAxisSecondary and renders against the secondary scale; position: 'right' puts it opposite the primary one. It takes part in the auto-margin measurement pass, so its labels get their gutter like the primary axis's. The React container picks it out of the declared <VisAxis> children by the flag; the TS API accepts yAxisSecondary directly.
Domain calculation. _updateScalesDomain used to run one shared body over both dimensions. That body is extracted into _applyScaleDomain(dimension, domainComponents, targetComponents, …) so the Y dimension can run twice — once over the primary components, once over the secondary ones — while X is untouched. With no secondary component in the chart the call sequence is identical to before.
Verification. A StackedBar with useSecondaryYScale plus a plain Line, rendered from this branch in headless Chromium: the two components share one range ([408.35, 1]) and hold separate domains ([0, 850] for the bar, [85, 92.98] for the line), the two Y axes print those two tick sets, and auto-margin reserves 55.5px on the right for the secondary axis.
Draft for that reason: the core and the React wrapper are done, the remaining wrappers and the example/docs are not.
🤖 Generated with Claude Code