AG-18383 Treat an empty-string labelKey, sizeKey or colorKey as absent in scatter/bubble series - #8034
Conversation
… scatter/bubble QA found that a quadrant (or plain scatter/bubble) chart configured with labelKey: '' failed to render at all, throwing "didn't find property definition for [labelValue, ScatterSeries-1]" on every update. processData declares the optional labelValue/sizeValue/colorValue columns under a truthy guard (labelKey ?), but several consumers resolved them under a nullish one (labelKey == null ? ...), so a falsy-but-present key resolved a column that was never declared. Align every consumer on truthiness.
…est.ts The new describe's assertions must not use toMatchInlineSnapshot (vitest rejects it inside it.each), but vitest shares one snapshot counter per test between inline and image snapshots — so rewriting the pre-existing tests' inline assertions renumbered their image baselines to -1-snap and they failed as missing.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| colorDataValues: colorKey | ||
| ? dataModel.resolveColumnById(this, `colorValue`, processedData, 'number') | ||
| : undefined, |
There was a problem hiding this comment.
This now correctly omits colorDataValues for colorKey: '', but applyPerDatumStyle still checks ctx.colorKey != null at line 1180. When an item styler selects that path, every datum has an undefined colour value and is consequently assigned colorScale.missingDataFill. Thus an empty colour key is not treated as absent and can change all marker colours. Use the same truthiness condition downstream and add an enterprise-capable regression case for an empty colorKey with an item styler.
There was a problem hiding this comment.
Confirmed and fixed in 4f63959.
applyPerDatumStyle now guards on ctx.colorKey truthiness (bubbleSeries.ts:1183), and cacheable: colorKey == null two lines over became cacheable: !colorKey — the same inconsistency, and with colorKey: '' it was disabling the style cache for a pass that can no longer mutate fill.
Reproduced rather than reasoned: reverting just that guard turns the new test red with [ 'tomato', 'tomato', 'tomato', … ] to not include 'tomato'.
Enterprise test added as you asked — packages/ag-charts-enterprise/src/test/colorScale.test.ts, describe('AG-18383 empty-string colorKey does not trigger missingDataFill'). It lives there rather than in the community bubbleSeries.test.ts block because colorKey is enterprise-gated and gets cleared under a community-only harness. It supplies an itemStyler, which is the only route into that guard for a falsy key: updateDatumStyles skips the per-datum pass unless marker.itemStyler != null || colorScaleValid, and colorScaleValid is already false for colorKey: ''. A second case pins the converse, that a real colorKey still gets missingDataFill.
Left alone: the params object and the getContext passthrough — they carry the raw key to callbacks and tooltips and resolve no column.
Full suites green: community 4722 passed, enterprise 3163 passed.
|
✅ Codex review complete; no issues found View full reviewAG-18383 Treat an empty-string labelKey, sizeKey or colorKey as absent in scatter/bubble seriesPR: #8034 SummaryReviewed all three changed files, prioritising BubbleSeries key handling and both regression suites. The unresolved missing-data-fill concern is fixed in current code. No actionable issues found. Targeted tests could not run because the checkout lacks the FindingsP0: 0 | P1: 0 | P2: 0 | P3: 0 No issues found. VerdictAssessment: correct All optional-key consumers now use guards consistent with processData column declaration. The per-datum styling path treats an empty colorKey as absent, and regression coverage exercises rendering, tooltip, size, and enterprise colour styling behaviour. Required Actions: None - ready to merge |
✅ Live-test this PR in PlunkerPaste these two <script src="https://ag-grid.github.io/ag-charts/pr-8034/ag-charts-community.min.js"></script>
<script src="https://ag-grid.github.io/ag-charts/pr-8034/ag-charts-enterprise.min.js"></script>Bundles are removed automatically when the PR is closed. Updated on every push. |
ag-jira-agent-ci
Bot
added
the
aiw-applying-feedback
ag-jira-agent-ci
Bot
added
aiw-applying-feedback
ag-jira-agent-ci
Bot
removed
the
aiw-applying-feedback
https://ag-grid.atlassian.net/browse/AG-18383
Fix #AG-18383