Repository navigation
fix(plugin-charts): the radar mark consults the series (objectui#8157) - #9864
Merged
Merged
Conversation
The radar arm of `AdvancedChartImpl` rendered a bare `fillOpacity={0.6}`
and read nothing off the series, while every cartesian arm resolved its
presentation through the shared `seriesStyle` helper. Three things
`@objectstack/spec` declares for every `ChartSeries` were therefore inert
on `chartType: 'radar'` at once: `opacity` and `dashArray` (both read by
`normalizeSeries`, then discarded) and the whole `variant: 'comparison'`
treatment, which is declared and documented with no family restriction.
The control that made the absence a reading: `seriesStyle(` has five call
sites in this file, one per cartesian arm, and had zero on the radar path.
The radar arm now calls the helper like the cartesian arms do, and passes
`strokeOpacity` and `strokeDasharray` through to the mark as well — radar
is a stroked mark, so unlike a bar or a scatter this is a family where a
dash actually paints.
The default face does not move. `0.6` was a DEFAULT as much as a bug (a
radar fill needs some opacity or an overlay is unreadable), so it became
the fallback — `fillOpacity={pres.fillOpacity ?? 0.6}` — rather than
being deleted. A radar series declaring none of the three keys paints
exactly what it painted before: fill-opacity 0.6, no stroke fade, no
dash. That control is pinned on both sides in the new test.
No cartesian arm changed. The helper gained `'radar'` in its `kind` union
and an `areaLike` binding which, for every cartesian kind, is exactly
`kind === 'area'`; radar takes the area family's muted defaults because
it is the other mark this renderer both strokes and fills, so no new
numbers were invented for it.
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012EpHzwH4wTy5sd7ibkD2yq
Contributor
|
changeset-claim-re-read
|
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
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
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.
Fixes #8157
The radar arm of
AdvancedChartImpl.tsxrendered a barefillOpacity={0.6}and readnothing off the series, while every cartesian arm resolved its presentation through the
shared
seriesStylehelper. Three things@objectstack/specdeclares for everyChartSerieswere therefore inert onchartType: 'radar'at once —opacityanddashArray(both read bynormalizeSeries, then discarded) and the wholevariant: 'comparison'treatment, which is declared and documented with no familyrestriction.
Anchors re-derived by content, because they had already moved twice
The card measured
c14d3a090, triage re-measured83d4fda16, the dispatch re-measured592402ea3— and on592402ea3the helper had moved once more. Everything below wasfound by content, not by address:
592402ea3const seriesStyle = (s: any, kind:grep -c "seriesStyle("chartType === 'radar'branchfillOpacity={0.6}The control the dispatch called the sharp fact holds exactly as stated: five cartesian
call sites, zero on the radar path. One premise did move again and is reported below.
What changed
seriesStyle(s, 'radar')and passesstrokeOpacityandstrokeDasharraythrough to the mark as well asfillOpacity. Radar is a strokedmark (
stroke={color}sits right above the literal), so unlike a bar or a scatterthis is a family where a dash actually paints.
seriesStylegained'radar'in itskindunion and anareaLikebinding. Radartakes the area family's muted defaults because it is the other mark this renderer
both strokes and fills — no new numbers were invented for it.
content/docs/plugins/plugin-charts.mdx: three rows of the per-series options tablesaid this renderer strokes
lineandareamarks. That sentence is falsified by thischange, so radar joins it there, and in the
opacityandvariantrows.The fallback, which is the decision this card actually asks for
0.6was a DEFAULT as much as a bug — a radar fill needs some opacity or an overlay isunreadable — so the literal became the fallback rather than being deleted:
⇒ a radar series declaring none of the three keys paints exactly what it painted
before:
fill-opacity0.6, no stroke fade, no dash. That is the answer triage asked aclaimant to state, and it is pinned from both sides in the new test. Already-drawn radar
charts that author nothing are untouched; only a chart that authors one of the keys
changes, which is the repair.
No cartesian arm changed
areaLikeiskind === 'area' || kind === 'radar', so for every cartesian kind it isexactly
kind === 'area'and every existing branch keeps its own value. Measured ratherthan argued:
ChartRenderer.seriesPresentation.test.tsx(the objectui#7698 pin, 13 casesover bar / line / area / scatter / combo, both variants) is green on both legs of the
reverse verification below — with the helper edit and without it.
Pin — three reds, with the control green on both sides
packages/plugin-charts/src/ChartRenderer.radarSeriesPresentation-8157.test.tsx. Itasserts on the rendered polygon's attributes, never on the helper's return value: the
helper was not on this path at all, which was the entire defect, so a unit test of the
helper would have passed on the broken tree.
Reverse verification, run from the committed state (
git checkout 592402ea3 -- ...restores the pre-fix file; restore leg is
git checkout HEAD -- ..., proved by the blobhash returning to the HEAD blob and by an empty
git diff HEAD). The mutation was provedto have landed on disk before the run was read:
seriesStyle(s, 'radar')count 1 to 0,and the mutated blob hash differs from the HEAD blob hash.
opacityreaches both channels0.6/ absent0.25/0.25dashArraydashes the outlinestroke-dasharrayabsent2 6variant: 'comparison'mutes the overlay0.2/0.6/4 40.9/0.9/8 40.6, no fade, no dashAblated run:
Tests 4 failed | 15 passed (19)— the 4 reds, and 15 green made up of the2 controls plus the whole objectui#7698 cartesian pin.
Verification
All readings below are from the final head
869c3160a(afterorigin/mainwas merged in).pnpm exec vitest run packages/plugin-charts/Test Files 64 passed (64)·Tests 569 passed (569)pnpm --filter @object-ui/plugin-charts type-checktsc --noEmitandtsconfig.test.json)pnpm --filter @object-ui/plugin-charts lint358 problems (0 errors, 358 warnings), all pre-existingnode scripts/check-changeset-presence.mjs.changeset/8157-radar-series-presentation.mdcheck:control-bytes·check:new-line-citations·check:changeset-claimscheck:doc-fences·check:doc-types·check:test-path-rootscheck:vi-mock-specifiers·check:vi-mock-inherit·check:vi-mock-override-shapecheck:pending-changeset-literalsnode scripts/check-governed-queue-guard.mjs --test(the 4 paths)NOT GOVERNEDThe dependency closure was built first (
pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-charts^...' build); before it,type-checkreported onlyCannot find module '@object-ui/core'and friends, which is an unbuilt tree rather than afinding.
Declared narrowing.
check:doc-snippetsandcheck:doc-examplesare NOT MEASUREDhere: both refuse to run until 34 packages are built, and building them locally for a
documentation check costs a long hold on the shared serialisation lock. The narrowing is
provable rather than hopeful —
git diff -U0on the.mdxshows three markdown tablerows changed and zero fence lines touched, so no TypeScript block in any document moved.
CI owns both gates.
Relationship to objectui#7698 and objectui#7546
variantgate and the Bar / Scatter marks inside the helper's owncoverage; its census classified this literal as "a literal constant", deliberately
outside its measured surface. It is untouched here and stays as it is.
check:changeset-claimsflagged one pending changeset that names this file —.changeset/7546-chart-series-keys-declared.md, whose body says the six keys "do realwork in
AdvancedChartImpl.tsx", withopacity/dashArraysetting stroke and fill.Read as a paragraph: this change makes that sentence more true, not less — before
it, the claim was silently overstated for one family. No correction is needed and none
was made.
Acceptance notes
Noted while in this file, deliberately not filed and not fixed here:
packages/types/src/data-display.tscarries cross-file line-address citations intothis package's
AdvancedChartImpl.tsxandnormalizeChartSchema.ts, and.changeset/7546-chart-series-keys-declared.mdcarries another intonormalizeChartSchema.ts. The addresses are deliberately not reproduced here — one ofthem points into a span this diff shifts, which is the rot AGENTS.md [WIP] Update documentation for project #11 describes.
That rule bans the form outright but rules the repair opportunistic, never a sweep,
so it belongs to whoever next edits those files rather than to this PR, and no card is
warranted by the rule's own wording. This change adds none:
check:new-line-citationsreports0 new citation(s).opacity,dashArrayorvariant: 'comparison'finds none in this repo's docs, fixtures orexamples, so the p3 grade stands.
Generated by Claude Code