Skip to content

Commit e59c76a

Browse files
ljodeaclaudegka
authored
fix: give usePlot().scales.fx/fy valid ranges for facet-aware positioning (#500)
## Summary - `usePlot().scales.fx/fy` had empty ranges (`[]`), making `fn()` calls return `NaN` — preventing custom overlay components from using fx/fy scales for facet-aware positioning (labels, tooltips, annotations) - After `computeScales()` returns in `computePlotState()`, create fresh `scaleBand` instances for fx/fy with correct range (`[0, plotWidth]` / `[0, plotHeight]`) and padding matching FacetGrid's layout - Fresh band scales are needed because d3 band scales created with `range([])` have corrupted internal ordinal mappings that can't be recovered by later setting the range Closes #498 ## Test plan - [x] 8 new test cases covering range validity, position correctness, bandwidth, DOM position matching, custom padding, and no-faceting edge case - [x] Existing facet tests pass (`facet-padding.test.ts`, `dot-faceted.test.svelte.ts`) - [x] Full test suite passes (685 tests) - [x] `svelte-check` passes (0 errors) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Gregor Aisch <gka@users.noreply.github.com>
1 parent 04dc695 commit e59c76a

File tree

5 files changed

+238
-2
lines changed

5 files changed

+238
-2
lines changed

‎src/lib/core/Plot.svelte‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import { setContext } from 'svelte';
1212
import { SvelteMap } from 'svelte/reactivity';
1313
import { writable } from 'svelte/store';
14+
import { scaleBand } from 'd3-scale';
1415
1516
import type {
1617
PlotOptions,
@@ -28,7 +29,7 @@
2829
import FacetGrid from './FacetGrid.svelte';
2930
3031
import mergeDeep from '../helpers/mergeDeep.js';
31-
import { computeScales, projectXY } from '../helpers/scales.js';
32+
import { computeScales, normalizeScaleFn, projectXY } from '../helpers/scales.js';
3233
import { CHANNEL_SCALE, SCALES } from '../constants.js';
3334
import { getPlotDefaults, setPlotDefaults } from 'svelteplot/hooks/plotDefaults.js';
3435
import { maybeNumber } from 'svelteplot/helpers/index.js';
@@ -285,6 +286,33 @@
285286
marks,
286287
DEFAULTS
287288
);
289+
// Fix fx/fy scale ranges: computeScales creates them with empty ranges
290+
// because getScaleRange has no case for fx/fy. We set the correct range
291+
// here using overall plotWidth/plotHeight, matching FacetGrid's layout.
292+
if (scales.fx.domain.length > 0) {
293+
const fxOpts = plotOptions.fx;
294+
const fxPaddingInner = fxOpts?.paddingInner ?? fxOpts?.padding ?? 0.1;
295+
const fxFn = scaleBand()
296+
.domain(scales.fx.domain as string[])
297+
.paddingOuter(0)
298+
.paddingInner(scales.fx.domain.length > 1 ? fxPaddingInner : 0)
299+
.rangeRound([0, plotWidth]) as any;
300+
fxFn.ticks = () => scales.fx.domain;
301+
scales.fx.fn = normalizeScaleFn(fxFn);
302+
scales.fx.range = fxFn.range();
303+
}
304+
if (scales.fy.domain.length > 0) {
305+
const fyOpts = plotOptions.fy;
306+
const fyPaddingInner = fyOpts?.paddingInner ?? fyOpts?.padding ?? 0.1;
307+
const fyFn = scaleBand()
308+
.domain(scales.fy.domain as string[])
309+
.paddingOuter(0)
310+
.paddingInner(scales.fy.domain.length > 1 ? fyPaddingInner : 0)
311+
.rangeRound([0, plotHeight]) as any;
312+
fyFn.ticks = () => scales.fy.domain;
313+
scales.fy.fn = normalizeScaleFn(fyFn);
314+
scales.fy.range = fyFn.range();
315+
}
288316
const colorSymbolRedundant =
289317
scales.color.uniqueScaleProps?.size === 1 &&
290318
scales.symbol.uniqueScaleProps?.size === 1 &&

‎src/lib/helpers/scales.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import { createProjection } from './projection.js';
3434
import { maybeInterval } from './autoTicks.js';
3535
import { IS_SORTED } from 'svelteplot/transforms/sort.js';
3636

37-
function normalizeScaleFn(fn: any): PlotScaleFunction {
37+
export function normalizeScaleFn(fn: any): PlotScaleFunction {
3838
const out = fn as PlotScaleFunction;
3939
out.range ||= () => [];
4040
out.invert ||= (value: number) => value;

‎src/tests/facetScales.test.svelte‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<script lang="ts">
2+
import { Plot, Dot } from '$lib/index.js';
3+
import FacetScalesChild from './facetScalesChild.test.svelte';
4+
let { plotArgs = {}, dotArgs = {} }: { plotArgs?: any; dotArgs?: any } = $props();
5+
</script>
6+
7+
<Plot width={200} height={200} margin={0} axes={false} {...plotArgs}>
8+
<Dot {...dotArgs} />
9+
<FacetScalesChild />
10+
</Plot>

‎src/tests/facetScales.test.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { render } from '@testing-library/svelte';
3+
import FacetScalesTest from './facetScales.test.svelte';
4+
import { getTranslate } from './utils';
5+
6+
const data = [
7+
{ x: 0, y: 0, gx: 'A', gy: 'P' },
8+
{ x: 1, y: 1, gx: 'B', gy: 'Q' }
9+
];
10+
11+
function getState(container: HTMLElement) {
12+
const el = container.querySelector('[data-testid="facet-scale-state"]') as HTMLElement;
13+
return {
14+
fxRange: JSON.parse(el.dataset.fxRange!),
15+
fxBandwidth: Number(el.dataset.fxBandwidth),
16+
fxPositions: JSON.parse(el.dataset.fxPositions!),
17+
fyRange: JSON.parse(el.dataset.fyRange!),
18+
fyBandwidth: Number(el.dataset.fyBandwidth),
19+
fyPositions: JSON.parse(el.dataset.fyPositions!),
20+
plotWidth: Number(el.dataset.plotWidth),
21+
plotHeight: Number(el.dataset.plotHeight)
22+
};
23+
}
24+
25+
describe('usePlot fx/fy scales', () => {
26+
it('fx scale has valid range spanning plotWidth', () => {
27+
const { container } = render(FacetScalesTest, {
28+
props: {
29+
plotArgs: {
30+
x: { domain: [0, 1] },
31+
y: { domain: [0, 1] }
32+
},
33+
dotArgs: { data, x: 'x', y: 'y', fx: 'gx' }
34+
}
35+
});
36+
37+
const state = getState(container);
38+
expect(state.fxRange).toHaveLength(2);
39+
expect(state.fxRange[0]).toBe(0);
40+
expect(state.fxRange[1]).toBe(state.plotWidth);
41+
});
42+
43+
it('fx positions are finite and distinct', () => {
44+
const { container } = render(FacetScalesTest, {
45+
props: {
46+
plotArgs: {
47+
x: { domain: [0, 1] },
48+
y: { domain: [0, 1] }
49+
},
50+
dotArgs: { data, x: 'x', y: 'y', fx: 'gx' }
51+
}
52+
});
53+
54+
const state = getState(container);
55+
expect(state.fxPositions).toHaveLength(2);
56+
expect(Number.isFinite(state.fxPositions[0])).toBe(true);
57+
expect(Number.isFinite(state.fxPositions[1])).toBe(true);
58+
expect(state.fxPositions[0]).not.toBe(state.fxPositions[1]);
59+
});
60+
61+
it('fx bandwidth is positive', () => {
62+
const { container } = render(FacetScalesTest, {
63+
props: {
64+
plotArgs: {
65+
x: { domain: [0, 1] },
66+
y: { domain: [0, 1] }
67+
},
68+
dotArgs: { data, x: 'x', y: 'y', fx: 'gx' }
69+
}
70+
});
71+
72+
const state = getState(container);
73+
expect(state.fxBandwidth).toBeGreaterThan(0);
74+
});
75+
76+
it('fx scale positions match facet DOM positions', () => {
77+
const { container } = render(FacetScalesTest, {
78+
props: {
79+
plotArgs: {
80+
x: { domain: [0, 1] },
81+
y: { domain: [0, 1] }
82+
},
83+
dotArgs: { data, x: 'x', y: 'y', fx: 'gx' }
84+
}
85+
});
86+
87+
const state = getState(container);
88+
const facets = container.querySelectorAll('g.facet') as NodeListOf<SVGGElement>;
89+
expect(facets.length).toBe(2);
90+
91+
const domX = Array.from(facets).map((f) => getTranslate(f)[0]);
92+
expect(state.fxPositions[0]).toBeCloseTo(domX[0], 0);
93+
expect(state.fxPositions[1]).toBeCloseTo(domX[1], 0);
94+
});
95+
96+
it('fy scale has valid range spanning plotHeight', () => {
97+
const { container } = render(FacetScalesTest, {
98+
props: {
99+
plotArgs: {
100+
x: { domain: [0, 1] },
101+
y: { domain: [0, 1] }
102+
},
103+
dotArgs: { data, x: 'x', y: 'y', fy: 'gy' }
104+
}
105+
});
106+
107+
const state = getState(container);
108+
expect(state.fyRange).toHaveLength(2);
109+
expect(state.fyRange[0]).toBe(0);
110+
expect(state.fyRange[1]).toBe(state.plotHeight);
111+
});
112+
113+
it('fy scale positions match facet DOM positions', () => {
114+
const { container } = render(FacetScalesTest, {
115+
props: {
116+
plotArgs: {
117+
x: { domain: [0, 1] },
118+
y: { domain: [0, 1] }
119+
},
120+
dotArgs: { data, x: 'x', y: 'y', fy: 'gy' }
121+
}
122+
});
123+
124+
const state = getState(container);
125+
const facets = container.querySelectorAll('g.facet') as NodeListOf<SVGGElement>;
126+
expect(facets.length).toBe(2);
127+
128+
const domY = Array.from(facets).map((f) => getTranslate(f)[1]);
129+
expect(state.fyPositions[0]).toBeCloseTo(domY[0], 0);
130+
expect(state.fyPositions[1]).toBeCloseTo(domY[1], 0);
131+
});
132+
133+
it('custom padding is respected', () => {
134+
const { container: c1 } = render(FacetScalesTest, {
135+
props: {
136+
plotArgs: {
137+
x: { domain: [0, 1] },
138+
y: { domain: [0, 1] }
139+
},
140+
dotArgs: { data, x: 'x', y: 'y', fx: 'gx' }
141+
}
142+
});
143+
const defaultBw = getState(c1).fxBandwidth;
144+
145+
const { container: c2 } = render(FacetScalesTest, {
146+
props: {
147+
plotArgs: {
148+
x: { domain: [0, 1] },
149+
y: { domain: [0, 1] },
150+
fx: { padding: 0.3 }
151+
},
152+
dotArgs: { data, x: 'x', y: 'y', fx: 'gx' }
153+
}
154+
});
155+
const customBw = getState(c2).fxBandwidth;
156+
157+
expect(customBw).toBeGreaterThan(0);
158+
expect(customBw).not.toBeCloseTo(defaultBw, 1);
159+
});
160+
161+
it('no faceting gives empty domain and no positions', () => {
162+
const { container } = render(FacetScalesTest, {
163+
props: {
164+
plotArgs: {
165+
x: { domain: [0, 1] },
166+
y: { domain: [0, 1] }
167+
},
168+
dotArgs: { data, x: 'x', y: 'y' }
169+
}
170+
});
171+
172+
const state = getState(container);
173+
// domain is empty when no faceting is used
174+
expect(state.fxPositions).toHaveLength(0);
175+
expect(state.fyPositions).toHaveLength(0);
176+
});
177+
});

‎src/tests/facetScalesChild.test.svelte‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<script lang="ts">
2+
import { usePlot } from '$lib/index.js';
3+
const plot = usePlot();
4+
// Extract fn to local to avoid Svelte proxy issues with d3 scale calls
5+
const fxFn = $derived(plot.scales.fx.fn);
6+
const fyFn = $derived(plot.scales.fy.fn);
7+
const fxPos = $derived(plot.scales.fx.domain.map((d) => fxFn(d)));
8+
const fyPos = $derived(plot.scales.fy.domain.map((d) => fyFn(d)));
9+
</script>
10+
11+
<div
12+
data-testid="facet-scale-state"
13+
data-fx-range={JSON.stringify(plot.scales.fx.range)}
14+
data-fx-bandwidth={plot.scales.fx.fn.bandwidth?.() ?? 0}
15+
data-fx-positions={JSON.stringify(fxPos)}
16+
data-fy-range={JSON.stringify(plot.scales.fy.range)}
17+
data-fy-bandwidth={plot.scales.fy.fn.bandwidth?.() ?? 0}
18+
data-fy-positions={JSON.stringify(fyPos)}
19+
data-plot-width={plot.plotWidth}
20+
data-plot-height={plot.plotHeight}>
21+
</div>

0 commit comments

Comments
 (0)