diff --git a/.changeset/drag-click-swallowed.md b/.changeset/drag-click-swallowed.md new file mode 100644 index 000000000..b0e8a430b --- /dev/null +++ b/.changeset/drag-click-swallowed.md @@ -0,0 +1,5 @@ +--- +'layerchart': patch +--- + +fix(TransformContext): Don't click what a pan, rotate, or pinch is released over diff --git a/.changeset/isometric-drag-echo.md b/.changeset/isometric-drag-echo.md new file mode 100644 index 000000000..4af8f9b3f --- /dev/null +++ b/.changeset/isometric-drag-echo.md @@ -0,0 +1,5 @@ +--- +'layerchart': patch +--- + +fix(Chart): Don't ease an `isometric` view's angles while dragging diff --git a/.changeset/isometric-motion-reactive.md b/.changeset/isometric-motion-reactive.md new file mode 100644 index 000000000..6b3e3eb91 --- /dev/null +++ b/.changeset/isometric-motion-reactive.md @@ -0,0 +1,5 @@ +--- +'layerchart': patch +--- + +fix(Chart): Ease an `isometric` view added after the chart was created, starting it at its own angles diff --git a/packages/layerchart/src/lib/components/Chart/Chart.isometric.svelte.test.ts b/packages/layerchart/src/lib/components/Chart/Chart.isometric.svelte.test.ts index d968b8af1..41c2a0ffe 100644 --- a/packages/layerchart/src/lib/components/Chart/Chart.isometric.svelte.test.ts +++ b/packages/layerchart/src/lib/components/Chart/Chart.isometric.svelte.test.ts @@ -467,6 +467,78 @@ describe('Chart isometric `motion`', () => { await vi.waitFor(() => expect(ctx.transform.rotation).toEqual({ x: 0, y: 0 })); expect(ctx.transform.translate).toEqual({ x: 30, y: 10 }); }); + + it.each([ + ['as it is', (angle: number) => angle], + ['rounded', Math.round], + ])( + "follows a drag's rotation handed back %s as the view's angles, wrapped to ±180°", + async (_, echo) => { + const { ctx, turn } = await renderEased({ mode: 'canvas', drag: 'rotate' }); + const el = ctx.containerRef!.querySelector('.lc-transform-context')!; + const box = el.getBoundingClientRect(); + const at = (x: number) => ({ + bubbles: true, + pointerId: 1, + clientX: box.x + x, + clientY: box.y, + }); + const wrap = (angle: number) => ((((angle + 180) % 360) + 360) % 360) - 180; + /** The same turn, either way round */ + const same = (a: number, b: number) => expect(Math.abs(wrap(a - b))).toBeLessThanOrEqual(0.5); + + // Dragging right turns from -45° towards -180°, and on past it to 170° + el.dispatchEvent(new PointerEvent('pointerdown', at(0))); + for (const x of [100, 200, 270, 290]) { + el.dispatchEvent(new PointerEvent('pointermove', at(x))); + const turned = -45 - x / 2; + same(ctx.transform.rotation!.x, turned); + + // As `onTransform` would, for a slider that runs -180° to 180° + const rotate = echo(wrap(ctx.transform.rotation!.x)); + const tilt = echo(ctx.transform.rotation!.y); + await turn({ rotate, tilt }); + // Not eased back behind the pointer + expect(ctx.isometricAngles).toEqual({ rotate, tilt }); + same(ctx.transform.rotation!.x, turned); + } + el.dispatchEvent(new PointerEvent('pointerup', at(290))); + } + ); + + it('eases a transformed view given after the chart was made, from its own angles', async () => { + let ctx: ChartState = null!; + const motion = { type: 'tween' as const, duration: 300 }; + const chartProps = (view?: ReturnType) => ({ + data, + x: 'x', + y: 'y', + xDomain: [0, 10], + yDomain: [0, 10], + width: 400, + height: 300, + view, + transform: { mode: 'canvas' }, + }); + const screen = render(TooltipTestHarness, { + chartProps: chartProps(), + oncontext: (c: any) => (ctx = c), + }); + await vi.waitFor(() => expect(ctx?.transformState).toBeTruthy()); + expect(ctx.transform.rotation).toBeNull(); + + // Straight to the view's angles, not eased from the defaults + await screen.rerender({ chartProps: chartProps(isometric({ rotate: 0, tilt: 0, motion })) }); + expect(ctx.transform.rotation).toEqual({ x: 0, y: 0 }); + + await screen.rerender({ chartProps: chartProps(isometric({ rotate: 0, tilt: 60, motion })) }); + await vi.waitFor(() => { + const { y } = ctx.transform.rotation!; + expect(y).toBeGreaterThan(0); + expect(y).toBeLessThan(60); + }); + await vi.waitFor(() => expect(ctx.transform.rotation).toEqual({ x: 0, y: 60 })); + }); }); describe('Chart isometric brush', () => { diff --git a/packages/layerchart/src/lib/components/TransformContext.svelte b/packages/layerchart/src/lib/components/TransformContext.svelte index 5cbf4d48a..b1eb50e70 100644 --- a/packages/layerchart/src/lib/components/TransformContext.svelte +++ b/packages/layerchart/src/lib/components/TransformContext.svelte @@ -236,10 +236,7 @@ function onClick(e: MouseEvent & { currentTarget: HTMLElement }) { onclickcapture?.(e); - if (transformState.dragging) { - // Do not propagate click event to children if drag/moved. Registered in capture phase (top-down) - e.stopPropagation(); - } + transformState.onClickCapture(e); } function onDoubleClick(e: MouseEvent & { currentTarget: HTMLElement }) { diff --git a/packages/layerchart/src/lib/components/TransformContext.svelte.test.ts b/packages/layerchart/src/lib/components/TransformContext.svelte.test.ts index 6ab0284c0..54219a650 100644 --- a/packages/layerchart/src/lib/components/TransformContext.svelte.test.ts +++ b/packages/layerchart/src/lib/components/TransformContext.svelte.test.ts @@ -3,6 +3,7 @@ import { render } from 'vitest-browser-svelte'; import { tick } from 'svelte'; import TransformTestHarness from '$lib/tests/TransformTestHarness.svelte'; +import { isometric } from '$lib/views/isometric.js'; import { geoMercator, geoOrthographic } from 'd3-geo'; describe('TransformContext', () => { @@ -467,4 +468,161 @@ describe('TransformContext', () => { expect(transform.scale).toBe(1); }); }); + + describe('click after a drag', () => { + /** Render the harness with a clickable child inside the transform context */ + async function setup(chartProps: Record) { + let chartContext: any; + + render(TransformTestHarness, { + chartProps: { height: 300, ...chartProps }, + oncontext: (ctx: any) => { + chartContext = ctx; + }, + }); + + await vi.waitFor(() => expect(chartContext).toBeDefined()); + + // TransformContext is lazy-loaded, so wait for it to render + const element = await vi.waitFor(() => { + const el = document.querySelector('.lc-transform-context'); + expect(el).not.toBeNull(); + return el!; + }); + + // Stands in for a mark (ex. a treemap node that zooms when clicked) + const mark = document.createElement('div'); + element.append(mark); + const onclick = vi.fn(); + mark.addEventListener('click', onclick); + + const rect = element.getBoundingClientRect(); + function dispatch(type: string, x: number, y: number, pointerId = 1) { + mark.dispatchEvent( + new PointerEvent(type, { + pointerId, + pointerType: 'mouse', + clientX: rect.left + x, + clientY: rect.top + y, + bubbles: true, + cancelable: true, + }) + ); + } + + /** The `click` the browser fires after `pointerup` (`detail: 0` is a keyboard click) */ + function click(x: number, y: number, detail = 1) { + mark.dispatchEvent( + new MouseEvent('click', { + clientX: rect.left + x, + clientY: rect.top + y, + detail, + bubbles: true, + cancelable: true, + }) + ); + } + + /** Press, move, and release, followed by the browser's `click` */ + function drag(from: [number, number], to: [number, number]) { + dispatch('pointerdown', ...from); + dispatch('pointermove', ...to); + dispatch('pointerup', ...to); + click(...to); + } + + return { + get transform() { + return chartContext.transform; + }, + dispatch, + click, + drag, + onclick, + }; + } + + it('should not click what a pan is released over', async () => { + const { transform, drag, onclick } = await setup({ transform: { mode: 'canvas' } }); + + drag([100, 100], [150, 120]); + + expect(transform.translate).toEqual({ x: 50, y: 20 }); + expect(onclick).not.toHaveBeenCalled(); + }); + + it('should not click what a rotate is released over', async () => { + const { transform, drag, onclick } = await setup({ + view: isometric, + transform: { mode: 'canvas', drag: 'rotate' }, + }); + await vi.waitFor(() => expect(transform.rotation).not.toBeNull()); + const before = { ...transform.rotation }; + + drag([100, 100], [150, 100]); + + expect(transform.rotation.x).not.toBe(before.x); + expect(onclick).not.toHaveBeenCalled(); + }); + + it('should click when the pointer stays within `clickDistance`', async () => { + const { drag, onclick } = await setup({ transform: { mode: 'canvas' } }); + + drag([100, 100], [101, 101]); + + expect(onclick).toHaveBeenCalledOnce(); + }); + + it('should click on the next press after a drag that ended without a click', async () => { + const { dispatch, click, onclick } = await setup({ transform: { mode: 'canvas' } }); + + // Released without the browser firing `click` (ex. over another element) + dispatch('pointerdown', 100, 100); + dispatch('pointermove', 150, 100); + dispatch('pointerup', 150, 100); + + dispatch('pointerdown', 150, 100); + dispatch('pointerup', 150, 100); + click(150, 100); + + expect(onclick).toHaveBeenCalledOnce(); + }); + + it('should only swallow the one click', async () => { + const { drag, click, onclick } = await setup({ transform: { mode: 'canvas' } }); + + drag([100, 100], [150, 100]); + expect(onclick).not.toHaveBeenCalled(); + + click(150, 100); + expect(onclick).toHaveBeenCalledOnce(); + }); + + it('should not swallow a keyboard click', async () => { + const { dispatch, click, onclick } = await setup({ transform: { mode: 'canvas' } }); + + dispatch('pointerdown', 100, 100); + dispatch('pointermove', 150, 100); + dispatch('pointerup', 150, 100); + click(150, 100, 0); + + expect(onclick).toHaveBeenCalledOnce(); + }); + + it('should not click what a pinch is released over', async () => { + const { transform, dispatch, click, onclick } = await setup({ + transform: { mode: 'canvas' }, + }); + + dispatch('pointerdown', 100, 100, 1); + dispatch('pointerdown', 200, 100, 2); + dispatch('pointermove', 300, 100, 2); + dispatch('pointerup', 300, 100, 2); + dispatch('pointerup', 100, 100, 1); + click(100, 100); + + expect(transform.scale).toBe(2); + expect(onclick).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/layerchart/src/lib/states/chart.svelte.ts b/packages/layerchart/src/lib/states/chart.svelte.ts index 57ea93d89..0cdfeed7d 100644 --- a/packages/layerchart/src/lib/states/chart.svelte.ts +++ b/packages/layerchart/src/lib/states/chart.svelte.ts @@ -39,7 +39,7 @@ import type { TooltipState } from './tooltip.svelte.js'; import type { BrushDomainType, BrushState } from './brush.svelte.js'; import { SeriesState, type SeriesLayout, type StackLayout } from './series.svelte.js'; import type { SeriesData } from '$lib/components/charts/types.js'; -import { createControlledMotion, createMotion, parseMotionProp } from '$lib/utils/motion.svelte.js'; +import { createControlledMotion, parseMotionProp } from '$lib/utils/motion.svelte.js'; const defaultPadding = { top: 0, right: 0, bottom: 0, left: 0 }; @@ -79,6 +79,9 @@ export interface MarkInfo { export type NodeKind = 'group' | 'mark' | 'composite-mark'; +/** An `isometric` view's turn and tip */ +type IsometricAngles = { rotate: number; tilt: number }; + export interface ComponentNode { id: symbol; kind: NodeKind; @@ -417,12 +420,28 @@ export class ChartState< // Read once — identity must stay stable for the life of the chart this.id = props.id ?? Symbol('Chart'); - // The view's `motion` is read once, like other `motion` props - this.#angles = createMotion( - this.#propAngles, - () => this.#propAngles, - this.#viewOptions?.motion - ); + let shown = false; + $effect.pre(() => { + const options = this.#viewOptions; + const angles = this.#propAngles; + untrack(() => { + // A view that appears starts at its own angles, having none to ease from, and a drag owns + // the view while it lasts (ex. its rotation handed back from `onTransform`) + const instant = !shown || !!this.transformState?.dragging; + // Without a view the angles hold, for one that comes back + shown = !!options; + if (!options) return; + if (!this.#angles && options.motion) { + this.#angles = createControlledMotion(angles, options.motion); + } + const motion = this.#angles; + if (!motion) return; + motion.set( + angles, + instant ? (motion.type === 'tween' ? { duration: 0 } : { instant: true }) : undefined + ); + }); + }); // Create GeoState instance — pass a dimensions getter so projection // is available during SSR (where $effect doesn't run) @@ -1149,16 +1168,20 @@ export class ChartState< ); /** The `isometric` options' `rotate` / `tilt`, with their defaults */ - #propAngles = $derived.by(() => { + #propAngles = $derived.by((): IsometricAngles => { const options = this.#viewOptions ?? {}; return { rotate: options.rotate ?? -45, tilt: options.tilt ?? ISOMETRIC_TILT }; }); - #angles: { readonly current: { rotate: number; tilt: number } }; + /** + * The angles eased by the view's `motion`, read once from the first view that has one (a chart + * may start without a view). `null` until then, following the angles as given. + */ + #angles = $state.raw> | null>(null); /** The `isometric` angles, eased by their `motion`; a transform rotates the view from these */ - get isometricAngles() { - return this.#angles.current; + get isometricAngles(): IsometricAngles { + return this.#angles?.current ?? this.#propAngles; } /** diff --git a/packages/layerchart/src/lib/states/chart.view.svelte.test.ts b/packages/layerchart/src/lib/states/chart.view.svelte.test.ts index 7eab36f18..ccaf71f04 100644 --- a/packages/layerchart/src/lib/states/chart.view.svelte.test.ts +++ b/packages/layerchart/src/lib/states/chart.view.svelte.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { flushSync } from 'svelte'; import { ChartState } from './chart.svelte.js'; @@ -45,4 +45,71 @@ describe('ChartState `view`', () => { expect(optedOut.state.view).toBeNull(); optedOut.cleanup(); }); + + describe('`motion`', () => { + const motion = { type: 'tween' as const, duration: 100 }; + + /** Change the props, then the angles as they are just after (before any easing) */ + function next(state: ChartState, change: () => void) { + change(); + flushSync(); + return { ...state.isometricAngles }; + } + + it('eases to new angles', async () => { + const props = $state>>({ + view: isometric({ rotate: 30, tilt: 60, motion }), + }); + const { state, cleanup } = chartState(props); + expect(state.isometricAngles).toEqual({ rotate: 30, tilt: 60 }); + + const angles = next(state, () => (props.view = isometric({ rotate: 90, tilt: 60, motion }))); + expect(angles.rotate).toBeLessThan(90); + await vi.waitFor(() => expect(state.isometricAngles).toEqual({ rotate: 90, tilt: 60 })); + cleanup(); + }); + + it('eases a view given after the chart was made, starting at its own angles', async () => { + const props = $state>>({ view: null }); + const { state, cleanup } = chartState(props); + + // Not from the default angles + const start = next(state, () => (props.view = isometric({ rotate: 30, tilt: 60, motion }))); + expect(start).toEqual({ rotate: 30, tilt: 60 }); + + const angles = next(state, () => (props.view = isometric({ rotate: 90, tilt: 60, motion }))); + expect(angles.rotate).toBeLessThan(90); + await vi.waitFor(() => expect(state.isometricAngles).toEqual({ rotate: 90, tilt: 60 })); + cleanup(); + }); + + it('reads `motion` once, from the first view that has one', async () => { + const props = $state>>({ + view: isometric({ rotate: 30, tilt: 60 }), + }); + const { state, cleanup } = chartState(props); + + // Starting where it's given, like a mark's `motion` + const start = next(state, () => (props.view = isometric({ rotate: 90, tilt: 60, motion }))); + expect(start).toEqual({ rotate: 90, tilt: 60 }); + + // Then easing, even once the view no longer gives it + const angles = next(state, () => (props.view = isometric({ rotate: 0, tilt: 60 }))); + expect(angles.rotate).toBeGreaterThan(0); + await vi.waitFor(() => expect(state.isometricAngles).toEqual({ rotate: 0, tilt: 60 })); + cleanup(); + }); + + it('starts a view that comes back at its own angles', async () => { + const props = $state>>({ + view: isometric({ rotate: 30, tilt: 60, motion }), + }); + const { state, cleanup } = chartState(props); + + next(state, () => (props.view = null)); + const angles = next(state, () => (props.view = isometric({ rotate: 90, tilt: 30, motion }))); + expect(angles).toEqual({ rotate: 90, tilt: 30 }); + cleanup(); + }); + }); }); diff --git a/packages/layerchart/src/lib/states/transform.svelte.ts b/packages/layerchart/src/lib/states/transform.svelte.ts index cbc589eb4..7090a7838 100644 --- a/packages/layerchart/src/lib/states/transform.svelte.ts +++ b/packages/layerchart/src/lib/states/transform.svelte.ts @@ -180,6 +180,11 @@ export class TransformState { startRotation = { x: 0, y: 0 }; /** What the drag in progress moves, settled as it starts */ private _dragging: TransformDrag = 'translate'; + /** + * The gesture just released was a drag (or pinch), so the `click` the browser fires after + * `pointerup` is swallowed rather than reaching what's under the pointer (see `onClickCapture`) + */ + private _suppressClick = false; // Velocity tracking for inertia private _pointerSamples: { x: number; y: number; t: number }[] = []; @@ -670,6 +675,7 @@ export class TransformState { this.pointerDown = true; this.dragging = false; + this._suppressClick = false; this.startPoint = localPoint(e); this.startTranslate = this._translate.current; this.startRotation = this._rotation.current; @@ -774,6 +780,7 @@ export class TransformState { // Pinch ended without inertia (velocity samples are not tracked while pinching) this.pointerDown = false; this.dragging = false; + this._suppressClick = true; this.ondragend?.(); return; } @@ -781,6 +788,7 @@ export class TransformState { const wasDragging = this.dragging; this.pointerDown = false; this.dragging = false; + if (wasDragging) this._suppressClick = true; // Apply inertia if enabled and was dragging with enough velocity if (this.inertia.enabled && wasDragging && this._pointerSamples.length >= 2) { @@ -894,6 +902,19 @@ export class TransformState { this.ondragend?.(); } + /** + * Keep the `click` that ends a drag from reaching what's under the pointer. Registered in the + * capture phase (top-down). The browser fires `click` after `pointerup`, which already ended the + * drag, so the drag is remembered until the next click or pointer down. A keyboard `click` + * (`detail: 0`) never ends a drag, so it always goes through. + */ + onClickCapture(e: MouseEvent) { + if ((this.dragging || this._suppressClick) && e.detail !== 0) { + e.stopPropagation(); + } + this._suppressClick = false; + } + onDoubleClick(e: MouseEvent & { currentTarget: HTMLElement }) { if (this.mode === 'none') return; const point = localPoint(e);