Skip to content

Commit 4f63959

Browse files
AG-18383 Treat an empty-string colorKey as absent in the per-datum marker styler
1 parent 99b011e commit 4f63959

2 files changed

Lines changed: 64 additions & 3 deletions

File tree

packages/ag-charts-community/src/chart/series/cartesian/bubbleSeries.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,10 @@ export class BubbleSeries extends CartesianSeries<BubbleSeriesTypes> {
11771177
if (ctx.colorScaleValid && datum.colorValue != null) {
11781178
stylerStyle.fill = series.colorScale.convert(datum.colorValue);
11791179
} else if (
1180-
ctx.colorKey != null &&
1180+
// Truthy, not `!= null` — `processData` only declares the `colorValue` column for a
1181+
// truthy `colorKey`, so `colorKey: ''` must not be treated as a present colour key and
1182+
// paint every datum with `missingDataFill` (AG-18383).
1183+
ctx.colorKey &&
11811184
datum.colorValue == null &&
11821185
series.properties.colorScale.missingDataFill != null
11831186
) {
@@ -1221,7 +1224,7 @@ export class BubbleSeries extends CartesianSeries<BubbleSeriesTypes> {
12211224
return;
12221225
}
12231226

1224-
// colorKey forces cacheable=false: applyPerDatumStyle mutates stylerStyle.fill per datum.
1227+
// A truthy colorKey forces cacheable=false: applyPerDatumStyle mutates stylerStyle.fill per datum.
12251228
this.runMarkerStylePass<
12261229
BubbleStylerPassCtx,
12271230
BubbleScatterNodeDatum,
@@ -1232,7 +1235,7 @@ export class BubbleSeries extends CartesianSeries<BubbleSeriesTypes> {
12321235
isHighlight,
12331236
{ marker, params, isHighlight, colorScaleValid, colorKey },
12341237
{
1235-
cacheable: colorKey == null,
1238+
cacheable: !colorKey,
12361239
compute: BubbleSeries.computePerDatumStylerStyle,
12371240
apply: BubbleSeries.applyPerDatumStyle,
12381241
}

packages/ag-charts-enterprise/src/test/colorScale.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,3 +273,61 @@ describe('colorScale partial options — theme fills survive user partials', ()
273273
assertColorScalePopulated();
274274
});
275275
});
276+
277+
// `colorKey: ''` declares no `colorValue` column (bubbleSeries.ts `processData`), so every datum's
278+
// `colorValue` is undefined. `applyPerDatumStyle` must treat the empty key as absent — a `!= null`
279+
// check there paints every marker with `missingDataFill` (AG-18383).
280+
describe('AG-18383 empty-string colorKey does not trigger missingDataFill', () => {
281+
setupMockConsole();
282+
setupMockCanvas();
283+
284+
let chart: any;
285+
286+
afterEach(() => {
287+
if (chart) {
288+
chart.destroy();
289+
chart = undefined;
290+
}
291+
});
292+
293+
const missingDataFill = 'tomato';
294+
295+
const buildOptions = (colorKey: string) => {
296+
const capturedFills: unknown[] = [];
297+
const series: AgScatterSeriesOptions = {
298+
type: 'scatter',
299+
xKey: 'x',
300+
yKey: 'y',
301+
colorKey,
302+
colorScale: { fills, missingDataFill },
303+
// The per-datum styler pass is only taken when an itemStyler (or a valid colour scale)
304+
// is present, so the styler is what makes the guard reachable — and what observes it.
305+
itemStyler: (params) => {
306+
capturedFills.push(params.fill);
307+
return {};
308+
},
309+
};
310+
return { options: prepareEnterpriseTestOptions({ data, series: [series] }), capturedFills };
311+
};
312+
313+
it('leaves marker fills untouched for colorKey: ""', async () => {
314+
const { options, capturedFills } = buildOptions('');
315+
316+
chart = deproxy(AgCharts.create(options));
317+
await waitForChartStability(chart);
318+
319+
expect(capturedFills.length).toBeGreaterThan(0);
320+
expect(capturedFills).not.toContain(missingDataFill);
321+
expect(new Set(capturedFills).size).toBe(1);
322+
});
323+
324+
// Anti-vacuity: with a real colorKey the missing-colour datums still get missingDataFill.
325+
it('still applies missingDataFill for a present colorKey', async () => {
326+
const { options, capturedFills } = buildOptions('intensity');
327+
328+
chart = deproxy(AgCharts.create(options));
329+
await waitForChartStability(chart);
330+
331+
expect(capturedFills).toContain(missingDataFill);
332+
});
333+
});

0 commit comments

Comments
 (0)