Skip to content

Commit f5536d3

Browse files
committed
fix(engine): gate theme-derived colors on isOpaqueColor, not the literal 'transparent'
Three sites decided "is this background really a color?" by comparing against the string 'transparent', which misses 'none' and alpha-zero rgba() -- the exact truthy-but-invisible values the export-canvas fix (529ebb8) already hardened against. All three now share core's existing isOpaqueColor(): - charts/scatter/compute.ts: the default point stroke fell through to the raw background string, so a theme background of 'none' became stroke='none'. SVG drops the separator halo; canvas is worse, since an invalid strokeStyle assignment is silently ignored and the previous color keeps painting. - theme/resolve.ts isDarkBackground(): a non-opaque background parsed to luminance 0 and was treated as dark, inverting chrome text on an effectively transparent (likely light) host. The existing comment already stated the intent -- no intrinsic luminance -- for the 'transparent' literal; now it holds for every non-opaque value. - theme/dark-mode.ts adaptTheme(): same equality check, same widening. No behavior change here (unparseable inputs already fell into the alreadyDark branch via luminance 0); this is convergence on one definition of transparency. Also documents the rc.19 stroke-default change in migrating-v8.md (section 20). It shipped in 644e0f8, whose non-conventional message ("bug: fix") kept it out of every generated changelog.
1 parent 1dfaaae commit f5536d3

5 files changed

Lines changed: 63 additions & 10 deletions

File tree

docs/migrating-v8.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,32 @@ escape hatch here. Charts rendering points on canvas get a much higher default
907907

908908
---
909909

910+
## 20. Scatter point stroke follows the theme background
911+
912+
Scatter and bubble points used to draw a white separator stroke unconditionally.
913+
The white halo is what gives overlapping dots separation on a light ground, but
914+
on a dark canvas it reads as a bright grid of rings. The default stroke now
915+
matches the resolved theme background when that background is an opaque color:
916+
917+
```
918+
before: stroke = mark.stroke ?? '#ffffff'
919+
after: stroke = mark.stroke ?? (opaque theme background ? background : '#ffffff')
920+
```
921+
922+
**What changes:** scatter charts on themes with an opaque dark background get
923+
dark separator strokes instead of white ones. The default theme background is
924+
`'transparent'`, so charts without a custom theme are unaffected, as is any
925+
theme whose background does not parse to an opaque color (`'none'`, alpha
926+
`rgba()`).
927+
928+
**If you want the white halo back** on a dark theme, set it explicitly:
929+
930+
```js
931+
{ mark: { type: 'point', stroke: '#ffffff' } }
932+
```
933+
934+
---
935+
910936
## Verification
911937

912938
After applying the changes above, run a build and check the console output.
@@ -928,3 +954,6 @@ warning at compile time with the exact fix. Items with no runtime warning:
928954
global reset, test table pagination buttons and search inputs first.
929955
- **Line chart zero (section 15):** silent behavior change. Audit line chart
930956
specs manually; y-axis domains will be tighter.
957+
- **Scatter stroke default (section 20):** silent visual change, and only on
958+
themes with an opaque dark background. Set `mark.stroke` explicitly to opt
959+
out.

packages/core/src/theme/dark-mode.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
*/
77

88
import { hsl, rgb } from 'd3-color';
9-
import { contrastRatio } from '../colors/contrast';
9+
import { contrastRatio, isOpaqueColor } from '../colors/contrast';
1010
import { ACHROMATIC_RAMP } from '../colors/palettes';
1111
import type { ResolvedTheme } from '../types/theme';
1212
import { DEFAULT_THEME } from './defaults';
@@ -108,9 +108,10 @@ function _luminanceFromHex(color: string): number {
108108
export function adaptTheme(theme: ResolvedTheme): ResolvedTheme {
109109
const pairs = theme._tokenPairs;
110110
const inputBg = theme.colors.background;
111-
// "transparent" preserves the background token but must still adopt dark
112-
// text/gridline/axis colors — the container is dark, not the chart canvas.
113-
const isTransparent = inputBg === 'transparent';
111+
// A non-opaque background ('transparent', 'none', alpha-zero rgba) preserves
112+
// the background token but must still adopt dark text/gridline/axis colors —
113+
// the container is dark, not the chart canvas.
114+
const isTransparent = !isOpaqueColor(inputBg);
114115
const alreadyDark = isTransparent || _luminanceFromHex(inputBg) < 0.2;
115116

116117
// Preserve user-supplied background (including transparent) but always

packages/core/src/theme/resolve.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* enabling darkMode.
1111
*/
1212

13+
import { isOpaqueColor } from '../colors/contrast';
1314
import type { ChromeThemeOverride, ThemeConfig } from '../types/spec';
1415
import type { ChromeDefaults, ResolvedTheme, Theme, TokenValue } from '../types/theme';
1516
import { DEFAULT_THEME } from './defaults';
@@ -192,9 +193,10 @@ function relativeLuminance(hex: string): number {
192193

193194
/** Returns true if the hex color is perceptually dark (luminance < 0.2). */
194195
function isDarkBackground(hex: string): boolean {
195-
// Transparent has no intrinsic luminance — don't treat it as dark so
196-
// light-mode chrome text defaults don't get inverted.
197-
if (hex === 'transparent') return false;
196+
// A non-opaque background ('transparent', 'none', alpha-zero rgba) has no
197+
// intrinsic luminance — don't treat it as dark so light-mode chrome text
198+
// defaults don't get inverted.
199+
if (!isOpaqueColor(hex)) return false;
198200
return relativeLuminance(hex) < 0.2;
199201
}
200202

packages/engine/src/charts/scatter/__tests__/compute.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,23 @@ describe('computeScatterMarks', () => {
501501
expect(marks.every((m) => m.stroke === '#ffffff')).toBe(true);
502502
});
503503

504+
it('keeps the white stroke for any non-opaque background, not just the literal "transparent"', () => {
505+
// 'none' and alpha-zero rgba are truthy but paint nothing. Passing them
506+
// through as the stroke would be invisible in SVG and worse on canvas,
507+
// where an invalid strokeStyle is silently ignored and the previous
508+
// color keeps painting.
509+
const spec: NormalizedChartSpec = {
510+
...baseSpec,
511+
markDef: { type: 'point' },
512+
};
513+
const scales = computeScales(spec, chartArea, spec.data);
514+
for (const background of ['none', 'rgba(0, 0, 0, 0)', 'rgba(9, 9, 11, 0.5)']) {
515+
const theme = { colors: { background } } as ResolvedTheme;
516+
const marks = computeScatterMarks(spec, scales, chartArea, fullStrategy, theme);
517+
expect(marks.every((m) => m.stroke === '#ffffff')).toBe(true);
518+
}
519+
});
520+
504521
it('lets an explicit mark.stroke override the theme default', () => {
505522
const spec: NormalizedChartSpec = {
506523
...baseSpec,

packages/engine/src/charts/scatter/compute.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {
1717
Rect,
1818
ResolvedTheme,
1919
} from '@opendata-ai/openchart-core';
20+
import { isOpaqueColor } from '@opendata-ai/openchart-core';
2021
import type { ScaleBand, ScaleLinear, ScalePoint, ScaleTime } from 'd3-scale';
2122
import { buildSizeScale, SIZE_SCALE_DEFAULTS } from '../../compile/size-scale';
2223
import { dedupeKeys, serializeKeyValue } from '../../compiler/keys';
@@ -133,10 +134,13 @@ export function computeScatterMarks(
133134
// Default point separator stroke. The classic white halo reads as a bright
134135
// grid of rings on a dark canvas, so match the stroke to the resolved theme
135136
// background when it's opaque enough to sit behind the dots (dark mode ->
136-
// dark stroke). A transparent background (light mode) keeps the white halo,
137-
// which is what gives dots separation on a light ground.
137+
// dark stroke). A non-opaque background ('transparent', 'none', alpha-zero
138+
// rgba) keeps the white halo: it has no color of its own to match, and
139+
// passing a value like 'none' through as a stroke is worse than cosmetic --
140+
// canvas silently ignores invalid strokeStyle assignments and keeps painting
141+
// with whatever color was set last.
138142
const bg = theme?.colors?.background;
139-
const defaultStroke = bg && bg !== 'transparent' ? bg : '#ffffff';
143+
const defaultStroke = bg && isOpaqueColor(bg) ? bg : '#ffffff';
140144

141145
const keyEnc = encoding.key && 'field' in encoding.key ? encoding.key : undefined;
142146
const keyField = keyEnc?.field;

0 commit comments

Comments
 (0)