Skip to content

Commit 81f72df

Browse files
committed
fix(vanilla): re-measure canvas pointer rects on pointerenter
The rect cache added in 529ebb8 invalidated on scroll and resize, but an element can move without either firing: content above the chart expanding, an accordion opening, a web font loading and reflowing the page. Any of those left stale boxes and offset hit-testing until the next scroll. Invalidating on pointerenter starts every hover session with a fresh measurement, bounding staleness to mid-hover layout shifts. The new test shifts the chart with no scroll event and asserts the next hover session hits against the moved box. Also flattens an empty-stops gradient to 'transparent' instead of 'none' in scatter-canvas/state.ts. 'none' is not a canvas color: assigning it to fillStyle is silently ignored, so the previous fill color would keep painting. Defensive only -- the compiler never emits an empty stop list.
1 parent f5536d3 commit 81f72df

4 files changed

Lines changed: 31 additions & 7 deletions

File tree

packages/vanilla/src/scatter-canvas/__tests__/interactions-rect-cache.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,23 @@ describe('pointer rect caching', () => {
119119
expect(canvasRectCalls).toBeGreaterThan(1);
120120
});
121121

122+
it('re-measures on pointerenter, so a scroll-less layout shift cannot strand the cache', () => {
123+
move(layer.canvas, 100, 100);
124+
expect(layer.state.hoverIndex).toBe(0);
125+
expect(canvasRectCalls).toBe(1);
126+
127+
// The chart moves 50px without any scroll or resize event (content above
128+
// it expanded). The pointer re-entering the canvas must trigger a fresh
129+
// measurement so the next move hit-tests against the new box.
130+
layer.state.hoverIndex = -1;
131+
scrollY = 50;
132+
layer.canvas.dispatchEvent(new Event('pointerenter'));
133+
134+
move(layer.canvas, 100, 50);
135+
expect(layer.state.hoverIndex).toBe(0);
136+
expect(canvasRectCalls).toBe(2);
137+
});
138+
122139
it('stops listening for scroll once cleaned up', () => {
123140
move(layer.canvas, 100, 100);
124141
const before = canvasRectCalls;

packages/vanilla/src/scatter-canvas/__tests__/state.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ describe('flattenFill', () => {
3636
expect(flattenFill(gradient)).toBe('#111111');
3737
});
3838

39-
it('falls back to none for an empty gradient', () => {
40-
expect(flattenFill({ gradient: 'linear', stops: [] } as GradientDef)).toBe('none');
39+
it('falls back to transparent (a valid canvas color) for an empty gradient', () => {
40+
expect(flattenFill({ gradient: 'linear', stops: [] } as GradientDef)).toBe('transparent');
4141
});
4242
});
4343

packages/vanilla/src/scatter-canvas/interactions.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,13 @@ export function wireCanvasInteractions({
4747
* `getBoundingClientRect()` forces layout, and reading two of them on every
4848
* `pointermove` is exactly the per-event cost this layer exists to avoid.
4949
* Neither box moves on its own: a container resize tears the layer down and
50-
* `render()` rebuilds it, so within one layer's life the only thing that
51-
* shifts these is the viewport scrolling underneath them. Invalidate on
52-
* scroll (capture phase, so ancestor scroll containers count) and on resize,
53-
* then re-measure lazily on the next event that needs a box.
50+
* `render()` rebuilds it, so within one layer's life the boxes shift only
51+
* when the page moves underneath them. Invalidate on scroll (capture phase,
52+
* so ancestor scroll containers count), on resize, and on `pointerenter` —
53+
* a layout shift with no scroll and no resize (content above the chart
54+
* expanding, a font loading) still moves the boxes, and starting each hover
55+
* session with a fresh measurement bounds that staleness to mid-hover
56+
* shifts. Re-measure lazily on the next event that needs a box.
5457
*/
5558
let svgRect: DOMRect | null = null;
5659
let canvasRect: DOMRect | null = null;
@@ -143,6 +146,7 @@ export function wireCanvasInteractions({
143146
canvas.addEventListener('pointermove', handlePointerMove);
144147
canvas.addEventListener('pointerdown', handlePointerDown);
145148
canvas.addEventListener('pointerleave', handlePointerLeave);
149+
canvas.addEventListener('pointerenter', invalidateRects);
146150
// Capture phase: a scroll in any ancestor moves our boxes, and scroll events
147151
// from those do not bubble.
148152
window.addEventListener('scroll', invalidateRects, true);
@@ -152,6 +156,7 @@ export function wireCanvasInteractions({
152156
canvas.removeEventListener('pointermove', handlePointerMove);
153157
canvas.removeEventListener('pointerdown', handlePointerDown);
154158
canvas.removeEventListener('pointerleave', handlePointerLeave);
159+
canvas.removeEventListener('pointerenter', invalidateRects);
155160
window.removeEventListener('scroll', invalidateRects, true);
156161
window.removeEventListener('resize', invalidateRects);
157162
};

packages/vanilla/src/scatter-canvas/state.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ import type { CanvasGridline, CanvasRect, ScatterCanvasState, ScatterPointsSoA }
2121
*/
2222
export function flattenFill(fill: string | GradientDef): string {
2323
if (typeof fill === 'string') return fill;
24-
return fill.stops[0]?.color ?? 'none';
24+
// 'transparent', not 'none': canvas silently ignores invalid fillStyle
25+
// assignments, so 'none' would leave the previous fill color in effect.
26+
return fill.stops[0]?.color ?? 'transparent';
2527
}
2628

2729
/** Collect gridlines from one axis into canvas-space records. */

0 commit comments

Comments
 (0)