feat(charts): ObjectChart honors the spec ChartConfig author shape (#2880) - #2883
Merged
Merged
Conversation
…bjectui#2880) ChartConfigSchema is the chart protocol, but the renderer only ever read a Recharts-flavoured internal shape — chartType, xAxisKey, series[].dataKey. Everything an author wrote in the SPEC shape reached the renderer and was silently dropped, which is what ADR-0078 forbids. framework#3725 documented the gap by trimming the published contract to the props that actually worked; this closes it the other way round. S1 — one normalization boundary. normalizeChartSchema translates the author shape into the internal pipeline contract in a single place rather than scattering `??` fallbacks through the render tree (PD #12: one translation is a contract mapping, N fallbacks are a second dialect). `type`→`chartType`, `xAxis:{field}`→`xAxisKey`, `series:[{name}]`→`series:[{dataKey}]`; the report surface's bare-string axes resolve too; `yAxis:[{field}]` alone plots with no series declared. Internal props win, so DashboardRenderer, ObjectView and the dataset path are byte-for-byte unaffected. The `type` collision: ChartConfig.type is the chart family, but on any surface that flattens chart config into a props bag `type` is already the SDUI envelope's component discriminator. Spreading props last let an author's type="bar" replace object-chart so the block stopped resolving; stamping the discriminator last ate the author's value instead. The react-page wrapper now keeps both — the discriminator wins the `type` slot, the author's value is preserved beside it as `specType`, and the normalizer reads it back. S2 — axis presentation. ChartAxis.format drives the tick formatter (via Intl.NumberFormat, no new dependency), min/max pin the domain, logarithmic swaps the scale, title labels the axis, showGridLines is honored. A second yAxis entry (or position:'right') turns on the secondary axis that series[].yAxis binds to; in combo charts an explicit binding beats the family-derived bar→left/line→right guess. showLegend is honored, and title/subtitle render above the plot instead of only titling the drill drawer. S3 — series[].stack passes through as Recharts' stackId; annotations render as ReferenceLine/ReferenceArea with the declared axis, colour, style and label; interaction.tooltips:false suppresses the hover card, interaction.brush:true adds the range selector, and showDataLabels prints values on the marks. interaction.zoom has no Recharts primitive behind it and stays unimplemented rather than faked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UMHZBHTjH4rw8xmDYirFC7
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
July 28, 2026 02:00
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
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.
Closes #2880. Framework umbrella: objectstack-ai/objectstack#3729. Companion framework PR: objectstack-ai/objectstack#3737 (merge this one first — the contract flip depends on this behaviour).
Context
ChartConfigSchemais the chart protocol, but the renderer only ever read a Recharts-flavoured internal shape —chartType,xAxisKey,series[].dataKey. Everything an author wrote in the SPEC shape reached the renderer and was silently dropped, which is exactly what ADR-0078 forbids. framework#3725 documented the gap by trimming the published contract down to the props that actually worked; this closes it the other way round.S1 — one normalization boundary
normalizeChartSchematranslates the author shape into the internal pipeline contract in a single place, rather than scattering??fallbacks through the render tree. That distinction is the point (framework PD #12): one translation is a contract mapping, N fallbacks are a second de-facto dialect.type→chartType,xAxis: { field }→xAxisKey,series: [{ name }]→series: [{ dataKey }]xAxis/yAxisresolve tooyAxis: [{ field }]alone plots, with noseriesdeclaredDashboardRenderer,ObjectViewand the dataset path are byte-for-byte unaffected — there is no migrationThe
typecollisionChartConfig.typeis the chart family, but on any surface that flattens chart config into a props bagtypeis already the SDUI envelope's component discriminator ({ type: 'object-chart', … }). The react-page wrapper spread props last, so an author'stype="bar"replaced the discriminator and the block stopped resolving; stamping the discriminator last would have silently eaten the author's value instead. Neither is acceptable under ADR-0078, so the wrapper now keeps both: the discriminator wins thetypeslot and the author's value is preserved beside it asspecType, which the normalizer reads back.This is why framework#3729's checklist item "
type取代 overlay 的chartType" could not be done as written — the collision had to be resolved at the flattening site first.S2 — axis presentation
ChartAxis.formatIntl.NumberFormat— no new dependency; an unrecognized format keeps the existing default rather than guessing)ChartAxis.min/maxdomain+allowDataOverflow(so an explicit domain actually clips)ChartAxis.logarithmicscale="log"ChartAxis.titleChartAxis.showGridLinesCartesianGridper orientationyAxisentry /position: 'right'series[].yAxis— in combo charts an explicit binding now beats the family-derived bar→left/line→right guessshowLegendtitle/subtitletitlepreviously only titled the drill-down drawer)Horizontal bars get the y-axis config applied to their x axis, since that is the value axis there.
S3 — stack, annotations, interaction
series[].stack(prioritized per the issue): the author's group name passes through as Recharts'stackId. Grouped vs stacked is a series property, not a chart family — which is whystacked-bar/stacked-areadon't need to come back into the taxonomy (corrected in the framework PR).annotations:type: 'line'→ReferenceLine,type: 'region'→ReferenceArea, honoringaxis,value/endValue,color,styleandlabel.interaction:tooltips: falsesuppresses the hover card,brush: trueadds the range selector.showDataLabelsprints values on the marks.interaction.zoomis deliberately still unimplemented — Recharts has no zoom primitive and faking it would be the exact declared≠delivered problem this PR exists to fix. Flagged in the framework PR by marking it declared-not-delivered in its own schema description.Verification
normalizeChartSchema.test.ts— 20 tests: spec shape, internal-wins (the no-migration guarantee), thetypecollision,formatterFor,domainForAdvancedChartImpl.specConfig.test.tsx— 12 tests pinning the wiring at the DOM level (title/subtitle, legend off, currency ticks, axis title, second y-axis, stacked-vs-grouped bar geometry, reference line/area, brush)type-checkclean onplugin-chartsandcomponents🤖 Generated with Claude Code
https://claude.ai/code/session_01UMHZBHTjH4rw8xmDYirFC7
Generated by Claude Code