fix(plugin-charts): name the scatter legend's series so its swatch stops reading as a stray point - #7398
Merged
Merged
Conversation
…ops reading as a stray point (#7248) The Chart Gallery scatter appeared to draw a seventh point below the x-axis. It was the legend swatch: ChartLegendContent resolves a label as config[nameKey || item.dataKey || 'value'], and a <Scatter> carries no dataKey, so the key collapsed to 'value', missed the config and rendered a colour swatch with no text. Measured in real Chromium: swatch at cy 341 against a plot area ending at cy 295, scale 4.835px/unit -> y = -9.5 at x ~ 45, matching the report's 'x~40, y~-10'. All six real marks were inside the plot area at every viewport width from 1440 down to 480, so the y domain is left untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ablation showed removing nameKey alone left every assertion green: the item.value fallback masks it on the label. The config lookup is still load-bearing for itemConfig.icon, which the fallback cannot stand in for, so pin that branch instead of shipping an unenforced change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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 #7248
The reported point is not a point — it is the legend swatch
The card reported the Chart Gallery scatter ("Estimate vs Progress") drawing a seventh
point below the x-axis at "x≈40, y≈-10", and asked for the y domain to include every
plotted value. Measured on the running showcase in real Chromium, that diagnosis is
false and the requested fix would have been in the wrong place.
ChartLegendContentresolves a label asconfig[nameKey || item.dataKey || 'value'].A
Scattercarries nodataKey— scatter's keys live on the XAxis/YAxis, not onthe mark — so the key collapsed to the literal string
'value', missed a config keyedby measure name, and the legend entry rendered its colour swatch with no text beside
it. An 8x8 square in
--chart-1, the same colour as the marks, sitting under thex-axis.
Captured from recharts 3.10.1 rather than assumed, the scatter's legend payload item is:
The series name was in
valuethe whole time; the legend never read it.Browser measurement — before and after
Showcase Chart Gallery,
/apps/com.example.showcase/dashboard/showcase_chart_gallery,real Chromium (Playwright 1.62.1), viewport 1440x1000, widget svg 510x350.
Before — plot area y 5..295, six marks, all inside it:
The seventh "point" was the legend swatch: cx 255, cy 341,
innerText === "". Thearithmetic closes the identification — the plot bottom (y=0) is cy 295 and the y scale
is 4.835 px per unit, so cy 341 is y = -9.5, at x ≈ 45. That is the card's
"x≈40, y≈-10", to the pixel.
Swept viewport widths 1440, 1100, 980, 900, 820, 760, 700, 640, 560, 480: zero marks
outside the plot area at any width, so the symptom was never width-dependent either.
After — plot area y 5..287, same six marks all inside (cy 209.5, 99, 5, 174.2, 146,
230.6), and the legend now reads
Avg Estimatebeside its swatch instead of painting ananonymous dot.
The y domain was not the defect and is unchanged
Clamping the y domain — the fix the card asked for — would have created the bug it
described. Two adversarial fixtures are pinned in this PR: mixed-sign data
(
ym: 40, -15, 25) draws 3 of 3 marks and all-negative data (ym: -40, -15) draws 2 of2, because recharts already extends the domain to cover negative values. Forcing
[0, 'auto']is exactly what would push a negative point below the baseline.The live data was read from the running showcase API and rules out the "bad row"
hypothesis independently: 10 seeded tasks, every
progressa finite non-negativenumber, no null / negative / string values, aggregating to the six buckets above.
Two changes
AdvancedChartImpl.tsx, scatter branch — passnameKey={scatterYKey}so thelegend resolves the measure's config entry, the same way the pie branch already
passes
nameKey={xAxisKey}.ChartContainerImpl.tsx,ChartLegendContent— fall back toitem.value, theseries name recharts itself put on the legend item, when the config lookup misses.
The second is a deliberate bounded in-place fix rather than a drive-by: it is the same
defect class, at the layer that decides whether a swatch gets a name, and it closes the
class instead of this one instance. The swatch renders unconditionally, so a config
miss must never leave it anonymous. The helper directly below it already documents the
pie form of this same bug ("legend swatches with NO text at all"); that one was closed
by making one family's key resolve, this closes the general case. Charts whose config
already resolves are unaffected — only a currently-empty label can change.
Reverse verification (ablation)
Each half was ablated independently from the committed state, with a restore trap on
absolute paths, the mutation proved on disk by HEAD-blob-hash comparison plus an anchor
count, and the tree confirmed clean (
git diff HEADempty) after every leg. Nodistis involved — the test imports
./AdvancedChartImpl, which imports./ChartContainerImpl, both relative source paths — so no rebuild leg applies.nameKeyonly640e2290 → 52a84b18, anchor count 0?? item.valueonly117306ba → 525ac855, anchor count 0The first run of this ablation found a real problem and changed the PR: removing
nameKeyalone left all assertions green, because theitem.valuefallback masksit on the label. Rather than ship an unenforced change, a pin was added for the one
branch the fallback cannot stand in for —
itemConfig.icon, which only resolves whenthe config lookup hits, so a config-declared legend icon is silently ignored for scatter
without
nameKeywhile every other family honours it. That pin is the single red inablation 1 above.
Per-facet answers to the dispatch
notice answers rows carrying no number; every gallery row has two. Pinned here: no
note and no refusal on this fixture. Note also that fix(plugin-charts): scatter says when it cannot place a row, instead of drawing an empty axis #7197 was already an ancestor of
the build the card was filed against (
67dadd602a3a), so it was never the missingpiece.
The
YAxisbindsdataKey={series[0].dataKey}for every series and eachScatterreceives the same unprojected
data, so every series does plot at the first series'y values. That is an axis-binding defect, not label resolution, and this card's chart
is single-series. Left to its own card.
type="number"axes and never reaches the categorical thinning path. Not reverted,not touched.
Gates
Run on the final commit
d2537532b:pnpm exec vitest run packages/plugin-charts/ --maxWorkers=2— 42 files, 390 testspassed (run from the repo root; the package-scoped
pnpm --filter ... testform isthe silent-green trap AGENTS.md bans)
pnpm --filter @object-ui/plugin-charts type-check— exit 0.tsc --listFilesconfirms both edited files and the new test are inside the program, so this green
actually covers the diff rather than excluding the test file
pnpm exec eslint .inpackages/plugin-charts— exit 0, 0 errors, 318 pre-existingwarnings, 55 files linted (count read from
--format json), all three touchedfiles present in the linted set
.changeset/7248-scatter-legend-names-its-series.md,@object-ui/plugin-chartspatchDeclared narrowing. Lint was run at package scope rather than repo-wide. The
population comes from eslint's own config resolution, the count from
--format json(55 files), and the config is not type-aware (no
parserOptions.project/projectService), so this diff cannot move the verdict on any untouched file. CI runsthe full farm regardless.
Verify-lock declaration, verbatim from the tool on every heavy run:
Out-of-scope findings filed
numeric axes get no domain padding). Measured here: plot x 53..505 with marks at
cx 53 and cx 505, so about half of each extreme symbol paints outside the plot area.
Real, different mechanism, and changing it would alter every scatter's appearance.
packages/components/src/ui/chart.tsxis an unreferenced duplicate ofthe plugin-charts chart primitives carrying this same legend hole. Imported by
nothing and absent from the barrel, so it is currently unreachable; left untouched
here rather than widening this PR across a second published package.
Generated by Claude Code