diff --git a/.changeset/canvas-drag-hysteresis.md b/.changeset/canvas-drag-hysteresis.md new file mode 100644 index 0000000000..41f2f9b818 --- /dev/null +++ b/.changeset/canvas-drag-hysteresis.md @@ -0,0 +1,28 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": patch +"@nextlyhq/ui": patch +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +"@nextlyhq/builder": patch +"@nextlyhq/module-specifiers": patch +--- + +The page builder canvas now resists switching drop targets until the pointer has moved a clear margin past a zone boundary. Resting the pointer near a boundary previously flipped the target on every pixel of jitter, so the insertion line strobed between two zones and a drop landed wherever the pointer happened to be sampled. diff --git a/packages/plugin-page-builder/package.json b/packages/plugin-page-builder/package.json index 1f83a638bd..766690d67e 100644 --- a/packages/plugin-page-builder/package.json +++ b/packages/plugin-page-builder/package.json @@ -53,6 +53,8 @@ "test:integration": "vitest run --config vitest.integration.config.ts" }, "dependencies": { + "@dnd-kit/abstract": "0.5.0", + "@dnd-kit/collision": "0.5.0", "@dnd-kit/dom": "0.5.0", "@dnd-kit/react": "0.5.0", "@nextlyhq/blocks-engine": "workspace:*", diff --git a/packages/plugin-page-builder/src/admin/canvas/CanvasNode.tsx b/packages/plugin-page-builder/src/admin/canvas/CanvasNode.tsx index 49b575a9f5..7ce871feb2 100644 --- a/packages/plugin-page-builder/src/admin/canvas/CanvasNode.tsx +++ b/packages/plugin-page-builder/src/admin/canvas/CanvasNode.tsx @@ -16,6 +16,7 @@ * * The root container renders via `CanvasNode`; descendants render via `DraggableNode`. */ +import { defaultCollisionDetection } from "@dnd-kit/collision"; import { useDraggable, useDroppable } from "@dnd-kit/react"; import { cloneElement, @@ -34,9 +35,14 @@ import { useEditor } from "../store/EditorProvider"; import { QueryLoopSamplePreview } from "./CanvasQueryLoop"; import { DropZone } from "./DropZone"; +import { withTargetHysteresis } from "./hysteresis"; const BLOCK_TYPE = "nx-block"; +// Shared by both of this node's drop targets, and built once for the reason the +// zone module gives: a per-render identity would be reassigned mid-drag. +const STICKY_COLLISION = withTargetHysteresis(defaultCollisionDetection); + /** Visual stand-in shown on the canvas for a block whose render() is empty (e.g. an * Image with no source), so it stays visible and selectable at author time. */ const placeholderStyle = { @@ -222,6 +228,7 @@ function DraggableNode({ accept: BLOCK_TYPE, disabled: dropBeforeIndex == null, data: { kind: "dropzone", parentId, slot, index: dropBeforeIndex ?? 0 }, + collisionDetector: STICKY_COLLISION, }); // Grid itself: "append" target for its own default slot. @@ -238,6 +245,7 @@ function DraggableNode({ slot: "default", index: appendIndex, }, + collisionDetector: STICKY_COLLISION, }); const className = classFor( diff --git a/packages/plugin-page-builder/src/admin/canvas/DropZone.tsx b/packages/plugin-page-builder/src/admin/canvas/DropZone.tsx index 106d9499f6..4be3dcc3a6 100644 --- a/packages/plugin-page-builder/src/admin/canvas/DropZone.tsx +++ b/packages/plugin-page-builder/src/admin/canvas/DropZone.tsx @@ -7,11 +7,18 @@ * placeholder. Zones only claim space while a drag is in progress, so the canvas stays * clean at rest. */ +import { defaultCollisionDetection } from "@dnd-kit/collision"; import { useDragDropMonitor, useDroppable } from "@dnd-kit/react"; import { useState, type ReactNode } from "react"; +import { withTargetHysteresis } from "./hysteresis"; + const BLOCK_TYPE = "nx-block"; +// Built once. A detector identity that changed per render would be reassigned on +// the droppable during a drag, which is churn for a value that never varies. +const STICKY_COLLISION = withTargetHysteresis(defaultCollisionDetection); + export function DropZone({ parentId, slot, @@ -38,6 +45,7 @@ export function DropZone({ type: BLOCK_TYPE, accept: BLOCK_TYPE, data: { kind: "dropzone", parentId, slot, index }, + collisionDetector: STICKY_COLLISION, }); if (empty) { diff --git a/packages/plugin-page-builder/src/admin/canvas/hysteresis.test.ts b/packages/plugin-page-builder/src/admin/canvas/hysteresis.test.ts new file mode 100644 index 0000000000..9293c68a8b --- /dev/null +++ b/packages/plugin-page-builder/src/admin/canvas/hysteresis.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it } from "vitest"; + +import { + TARGET_SWITCH_BAND_CENTRE_DELTA_PX, + bandedValue, + centreDistance, +} from "./hysteresis"; + +/** Two equal zones stacked vertically, centres `ZONE` apart, boundary midway. */ +const ZONE = 40; + +/** + * Which zone owns the pointer at `x` pixels past the boundary, decided the way + * the library decides it: higher value wins. + * + * The winner is DERIVED from `bandedValue` rather than from a second copy of + * the switching rule. A helper that re-expressed the rule as + * `dIncumbent - dChallenger >= band` would agree with the implementation on the + * day it was written and drift afterwards, and the drift would be invisible + * because both halves would look correct. + */ +function ownerAt(x: number, band: number): "incumbent" | "challenger" { + const pointer = { x: 0, y: x }; + const incumbent = centreDistance({ x: 0, y: -ZONE / 2 }, pointer); + const challenger = centreDistance({ x: 0, y: ZONE / 2 }, pointer); + const incumbentValue = bandedValue(1 / incumbent, incumbent, band); + const challengerValue = 1 / challenger; + return challengerValue > incumbentValue ? "challenger" : "incumbent"; +} + +/** The pointer offset at which ownership actually changes, to 0.01px. */ +function measuredSwitchPx(band: number): number { + for (let step = 0; step <= ZONE * 100; step += 1) { + const x = step / 100; + if (ownerAt(x, band) === "challenger") return x; + } + return Number.POSITIVE_INFINITY; +} + +describe("target-switch hysteresis", () => { + it("holds the incumbent through jitter that crosses the boundary", () => { + // The defect: with the pointer resting near a boundary, ownership flipped on + // every 2px crossing and the insertion line strobed between two targets. + for (const x of [2, -2, 2, -2, 2, -2]) { + expect(ownerAt(x, TARGET_SWITCH_BAND_CENTRE_DELTA_PX)).toBe("incumbent"); + } + }); + + it("gives up the incumbent once the pointer commits", () => { + // The positive control. Without it every assertion here is satisfied by a + // rule that never switches at all, which is not hysteresis but paralysis. + expect(ownerAt(ZONE / 2, TARGET_SWITCH_BAND_CENTRE_DELTA_PX)).toBe( + "challenger" + ); + }); + + it("resists for 8-12px of POINTER movement, measured not converted", () => { + // The requirement is in pixels of pointer movement; the constant is in + // difference-of-centre-distances, and the two differ by a factor of two + // because the pointer recedes from one centre while approaching the other. + // + // So the width is measured from the implementation's own decisions rather + // than computed from the constant. Asserting `band / 2` here would restate + // the conversion the implementation uses, and the pair would be wrong + // together: a constant of 10 reads as "10px" and delivers 5px, which is + // under the requirement's floor while appearing to sit inside its range. + const achieved = measuredSwitchPx(TARGET_SWITCH_BAND_CENTRE_DELTA_PX); + + expect(achieved).toBeGreaterThanOrEqual(8); + expect(achieved).toBeLessThanOrEqual(12); + }); + + it("is monotonic in the band: a wider band resists further", () => { + // Guards the direction of the scaling. A reciprocal applied the wrong way + // round still produces a switch point, and still passes a single-value + // range check if the number happens to land inside it. + const narrow = measuredSwitchPx(8); + const wide = measuredSwitchPx(32); + + expect(wide).toBeGreaterThan(narrow); + }); + + it("switches immediately when there is no band", () => { + // The zero-band control establishes that the resistance above comes from the + // band and not from the geometry of the fixture: at the boundary the two + // distances are equal, so any positive offset must hand over at once. + expect(measuredSwitchPx(0)).toBeLessThanOrEqual(0.01); + }); +}); + +describe("bandedValue at the edges", () => { + it("leaves an unbeatable score unbeatable", () => { + // Pointer containment reports exactly Infinity when the pointer sits on a + // centre, with no guard of its own. Scaling it would return a finite number, + // so the incumbent would LOSE at the one position where it is most clearly + // the right target — the band inverting exactly where it should be + // strongest, with nothing in the symptom pointing at the clamp. + expect(bandedValue(Number.POSITIVE_INFINITY, 0, 20)).toBe( + Number.POSITIVE_INFINITY + ); + }); + + it("never returns a negative or NaN value inside the band", () => { + // A negative divisor inverts the ranking, and NaN makes the comparator's + // ordering undefined. Both are silent. + for (const distance of [0.5, 5, 19, 19.999, 20]) { + const value = bandedValue(1 / distance, distance, 20); + expect(Number.isNaN(value)).toBe(false); + expect(value).toBeGreaterThan(0); + } + }); + + it("weakens the incumbent by distance, not by a constant", () => { + // The property that separates this from adding a bonus: the same band moves + // the value by different amounts at different distances, because the value + // is an inverse. Equal deltas would mean the band buys a pixel width that + // varies with how far the pointer already is. + const near = bandedValue(1 / 30, 30, 20) - 1 / 30; + const far = bandedValue(1 / 100, 100, 20) - 1 / 100; + + expect(near).toBeGreaterThan(far); + }); + + it("returns the base value when the band is absent or nonsensical", () => { + expect(bandedValue(0.25, 40, 0)).toBe(0.25); + expect(bandedValue(0.25, 40, -5)).toBe(0.25); + expect(bandedValue(0.25, Number.NaN, 20)).toBe(0.25); + }); +}); diff --git a/packages/plugin-page-builder/src/admin/canvas/hysteresis.ts b/packages/plugin-page-builder/src/admin/canvas/hysteresis.ts new file mode 100644 index 0000000000..b337aa5e7d --- /dev/null +++ b/packages/plugin-page-builder/src/admin/canvas/hysteresis.ts @@ -0,0 +1,127 @@ +/** + * Target-switch hysteresis for the canvas. + * + * With the pointer resting near a boundary between two drop zones, ownership + * flipped on every pixel of jitter, so the insertion line strobed between two + * targets and a drop landed wherever the pointer happened to be sampled. The + * incumbent target now has to be beaten by a margin rather than by any amount + * at all. + * + * A margin rather than a dwell. Both satisfy the requirement; a dwell charges + * latency on every deliberate move and puts wall-clock dependence into the + * canvas, while a margin costs nothing while the pointer moves decisively and + * resists only near a boundary. It is also deterministic, which is what lets a + * test assert the width instead of waiting for it to settle. + */ +import type { CollisionDetector } from "@dnd-kit/abstract"; + +/** + * How far a challenger must beat the incumbent by, as a difference of distances + * to the two candidates' centres. + * + * NOT pixels of pointer movement, and the distinction is a factor of two. The + * comparison this feeds reduces to `dInc - dNew < band`, so the quantity bounded + * is the DIFFERENCE between the two distances. For two equal zones whose centres + * the pointer travels between, moving `x` past the boundary lengthens one + * distance by `x` and shortens the other by `x`, so the difference grows at + * twice the pointer's rate and this constant buys half its value in pointer + * movement. + * + * That relation holds exactly only for equal zones on the line joining their + * centres. It is therefore not used to derive the requirement: the spec asks for + * 8-12px of pointer movement, and `hysteresis.test.ts` MEASURES what this + * constant achieves rather than converting it, so the conversion is under test + * instead of being assumed identically here and there. + */ +export const TARGET_SWITCH_BAND_CENTRE_DELTA_PX = 20; + +/** + * The incumbent's collision value, weakened by the band. + * + * Both default detectors report a value inversely proportional to the distance + * from the droppable's centre to the pointer — `1 / d` for pointer containment, + * `intersectionRatio / d` for shape overlap. Scaling by `d / (d - band)` + * therefore applies the band in DISTANCE space for either of them, without this + * code needing to know which one produced the number or to recompute it. The + * band is expressed once, in pixels, and the library's own value carries + * whatever else it encodes. + * + * Adding a constant instead would not work: an inverse is non-linear, so a fixed + * bonus buys a pixel width that depends on how far the pointer already is. + */ +export function bandedValue( + baseValue: number, + distance: number, + band: number +): number { + // An unbeatable score stays unbeatable. Pointer containment reports exactly + // `Infinity` when the pointer sits on a centre, with no guard of its own, and + // scaling that would produce a finite number — leaving the incumbent WEAKER at + // the one position where it is most clearly the right target, and inverting + // the behaviour this function exists to add. + if (!Number.isFinite(baseValue)) return baseValue; + if (!Number.isFinite(distance) || distance < 0) return baseValue; + if (!(band > 0)) return baseValue; + + const effective = distance - band; + // Nearer its own centre than the band is wide. Clamped rather than left to go + // negative or infinite: a negative divisor inverts the ranking, and an + // infinite value ties with another infinite one to produce NaN in the + // comparator, whose ordering is then undefined. + if (effective <= Number.EPSILON) { + return baseValue * (distance / Number.EPSILON); + } + return baseValue * (distance / effective); +} + +/** Straight-line distance, the same measure the collision values are built on. */ +export function centreDistance( + centre: { x: number; y: number }, + pointer: { x: number; y: number } +): number { + return Math.hypot(centre.x - pointer.x, centre.y - pointer.y); +} + +/** + * A collision detector that makes the current target harder to displace. + * + * Wraps a base detector rather than replacing it, so the ranking stays whatever + * the library computed and this only weakens ONE candidate: the one already + * holding the drag. Everything else is passed through untouched, which keeps the + * band the single difference between this and the stock behaviour. + * + * The scope is narrower than it may look, and deliberately so. + * `sortCollisions` orders by priority, then by collision TYPE, and only then by + * value — so weakening a value damps a switch between candidates of the same + * priority and type, and cannot damp a move from shape overlap to pointer + * containment. That is the correct scope: entering a zone outright should take + * effect at once, while drifting between two comparable neighbours should not. + */ +export function withTargetHysteresis( + base: CollisionDetector, + band: number = TARGET_SWITCH_BAND_CENTRE_DELTA_PX +): CollisionDetector { + return input => { + const collision = base(input); + if (!collision) return null; + + const { droppable, dragOperation } = input; + // Only the incumbent is weakened. Reading the current target from the drag + // operation rather than tracking it here means there is no state to seed, + // invalidate, or clear when a drag ends. + if (dragOperation.target?.id !== droppable.id) return collision; + + const centre = droppable.shape?.center; + const pointer = dragOperation.position?.current; + if (!centre || !pointer) return collision; + + return { + ...collision, + value: bandedValue( + collision.value, + centreDistance(centre, pointer), + band + ), + }; + }; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index da1977cde1..371866f574 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1310,6 +1310,12 @@ importers: packages/plugin-page-builder: dependencies: + '@dnd-kit/abstract': + specifier: 0.5.0 + version: 0.5.0 + '@dnd-kit/collision': + specifier: 0.5.0 + version: 0.5.0 '@dnd-kit/dom': specifier: 0.5.0 version: 0.5.0