From d2085dd45ad1baeb74ec4d1e386be5af072c8624 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 12 Aug 2026 23:39:46 +0500 Subject: [PATCH 01/10] test(builder): measure the drag delta from the pointer, not the source startDragAt is contractually allowed to move past the drag threshold, and the PoC driver shifts 12px doing so. The delta was computed from the source point, so every panel drag overshot by exactly that much -- and a replacement driver with a different activation motion would overshoot by a different amount, which is the seam this suite exists to keep swappable. Asking the driver where the pointer actually is keeps the gesture landing in the same place whichever driver is behind it. --- e2e/tests/canvas/acceptance.spec.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 58c154227b..7f7ee64f69 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -73,17 +73,26 @@ function note(point: number, becomes: string, shortfall?: string): void { /** Begin a drag from the insert panel and carry the pointer over the canvas. */ async function dragFromPanel(driver: CanvasDriver): Promise { - const source = await driver.dragSourceCentre(); const target = await driver.canvasCentre(); - await driver.startDragAt(source); + await driver.startDragAt(await driver.dragSourceCentre()); + + // The delta is measured from where the pointer ACTUALLY is after activation, + // not from the source point. `startDragAt` is contractually allowed to move + // past the drag threshold, and the PoC driver shifts 12px doing so — so a + // delta computed from the source overshoots by exactly that, and a + // replacement driver with a different activation motion overshoots by a + // different amount. Asking the driver where the pointer is keeps the gesture + // landing in the same place whichever driver is behind it. + const from = driver.pointer(); + // In steps, not one jump. A single move is a teleport, and a canvas that // commits on dwell rather than on distance answers a teleport differently // from the gesture a person makes. const steps = 8; for (let step = 0; step < steps; step += 1) { await driver.moveBy( - (target.x - source.x) / steps, - (target.y - source.y) / steps + (target.x - from.x) / steps, + (target.y - from.y) / steps ); } } From 26a827a1f78b5ac391b29b5a245574ca1fcb0b0b Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 00:38:59 +0500 Subject: [PATCH 02/10] test(builder): convert frame rects to host space, and retract a false shortfall readBlockBoxes measures inside the iframe, so its rects are frame-local, while the pointer moves in host coordinates. The collision case used one as the other -- off by the frame's origin and wrong again by its scale. At 100% zoom with the frame near the top-left the two are close enough to look correct, which is how it survived. The containment assertion had the same fault from the other side: it compared a host pointer against a frame-local bottom edge, a question neither coordinate answers. Correcting them retracted a shortfall I had recorded against the canvas. This case was marked as an expected failure because descending from nx-inner appeared to find no active zone until past the container's own bottom edge. That measurement was taken with the unconverted coordinates, so the pointer was never inside the container it was meant to be in. Converted, the canvas resolves to the innermost container and the case passes. The shortfall was the harness. --- e2e/tests/canvas/acceptance.spec.ts | 48 +++++++++++++++++++---------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 7f7ee64f69..01bff784c3 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -35,6 +35,7 @@ import { NESTED_FIXTURE, seedPage, } from "./fixtures"; +import { mapFramePointToHost } from "./coordinate-mapping"; import { CanvasCapabilityError, dragUntilTarget } from "./driver"; import type { CanvasChromeReader, CanvasDriver } from "./driver"; import { createPocChromeReader, createPocDriver } from "./poc-driver"; @@ -147,38 +148,51 @@ test.describe("a canvas any Nextly editor could ship", () => { // fixed y guesses at where the zones are and lands in the dead space // between them as often as not, which reports "no owner" and looks like a // depth failure rather than a pointer that was never over a zone. + // CONVERTED, not used raw. `readBlockBoxes` measures inside the iframe, so + // its rects are frame-local; the pointer moves in host coordinates. Using + // one as the other is off by the frame's origin and wrong again by its + // scale, and at 100% zoom with the frame near the top-left it is close + // enough to look correct — which is how it survived. + const origin = await driver.frameOrigin(); + const scale = await driver.frameScale(); + const entry = mapFramePointToHost( + { x: first!.left + first!.width / 2, y: first!.top }, + origin, + scale + ); + await driver.startDragAt(await driver.dragSourceCentre()); const from = driver.pointer(); - await driver.moveBy( - first!.left + first!.width / 2 - from.x, - first!.top - from.y - ); + await driver.moveBy(entry.x - from.x, entry.y - from.y); const reached = await dragUntilTarget(driver); expect( reached, "the drag must reach a zone inside the nested container" ).toBeGreaterThanOrEqual(0); - // Everything above ran unprotected: the seed, the measurement and reaching - // a zone at all are harness concerns and a failure in them is real. + // No expected failure here, and the reason is worth recording. This case + // WAS marked as one: descending from `nx-inner` appeared to find no zone + // until well past the container's own bottom edge. That measurement was + // taken with frame-local rects used as host coordinates, so the pointer was + // never inside the container it was supposed to be in. Converted, the + // canvas resolves to the innermost container correctly. // - // What follows is the shortfall. Measured: descending from the top of - // `nx-inner` finds no active zone until y=384, past the container's own - // bottom edge at 320 — this canvas creates gap zones at the outer level - // only, so there is no inner zone for the innermost container to own. - test.fail( - true, - "no drop zone exists inside a nested container; the nearest is below it" - ); + // The shortfall was the harness, not the canvas. // And it must still be inside `nx-inner` after that descent, or the walk // carried the pointer out the bottom and the ownership below is about a // different container entirely. - const pointerY = driver.pointer().y; + // Both sides in the SAME space. The pointer is host, the box is frame-local, + // so comparing them directly asks a question neither coordinate answers. + const bottom = mapFramePointToHost( + { x: 0, y: second!.top + second!.height }, + origin, + scale + ); expect( - pointerY, + driver.pointer().y, "the descent must not leave the nested container" - ).toBeLessThanOrEqual(second!.top + second!.height); + ).toBeLessThanOrEqual(bottom.y); const owner = await driver.readActiveZoneOwner(); const active = await driver.readActiveTarget(); From 7469d4f8e4507d99b12f85121e834a8f9f02cdf3 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 00:50:20 +0500 Subject: [PATCH 03/10] test(builder): give autoscroll a canvas that scrolls, and budget the slowest move Autoscroll was measured against NESTED_FIXTURE, roughly 400px of authored height in a 1400px viewport. The canvas had no scroll range at all, so canvasScrollTop could not change and the target could not pass however correctly autoscroll were implemented. TALL_FIXTURE overflows deliberately and the test asserts the overflow before measuring, so a fixture that stopped overflowing fails loudly instead of reporting a missing behaviour. Its own fixture rather than borrowing LARGE_FIXTURE: that one is sized for a perf budget, and tuning its block count for timing would silently take the scroll range away. The perf budget seeded 500 blocks and never checked they RENDERED, so a canvas that mounted six of them was timed while the test claimed to measure a large tree. It reads the rendered boxes first. And it averaged. One 2-second stall among twenty fast moves means a comfortable mean while the editor visibly locks up, which is exactly the shape a canvas that re-measures the tree produces when a rect cache misses. Every move is timed individually now and the SLOWEST has to sit under the budget. --- e2e/tests/canvas/acceptance.spec.ts | 57 ++++++++++++++++++++++++----- e2e/tests/canvas/fixtures.ts | 32 ++++++++++++++++ 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 01bff784c3..16344e81f2 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -33,6 +33,7 @@ import { FLAT_LIST_FIXTURE, LARGE_FIXTURE, NESTED_FIXTURE, + TALL_FIXTURE, seedPage, } from "./fixtures"; import { mapFramePointToHost } from "./coordinate-mapping"; @@ -427,7 +428,24 @@ test.describe("a canvas any Nextly editor could ship", () => { request, }) => { note(PLAN_POINT.autoscrollBounded, "B-8"); - await driver.mountTree(await seedPage(request, NESTED_FIXTURE)); + // A document TALLER than the canvas, or there is no scroll range and the + // target cannot pass however correctly autoscroll is implemented. + const fixture = await seedPage(request, TALL_FIXTURE); + await driver.mountTree(fixture); + + // Precondition, asserted rather than assumed: if the fixture rendered + // shorter than the viewport this case would measure a canvas that cannot + // scroll and report it as a missing behaviour. + const boxes = await driver.readBlockBoxes(); + const authored = boxes.reduce( + (lowest, box) => Math.max(lowest, box.top + box.height), + 0 + ); + expect( + authored, + "the fixture must overflow the canvas, or autoscroll is unobservable" + ).toBeGreaterThan(1400); + await dragFromPanel(driver); // The canvas cannot answer this at all, and that refusal IS the @@ -478,24 +496,43 @@ test.describe("a canvas any Nextly editor could ship", () => { fixture.blockIds.length, "the budget must be measured against the supported tree size" ).toBeGreaterThanOrEqual(500); + // SEEDED is not RENDERED. Timing against a canvas that mounted 6 of the 500 + // measures a small tree while claiming to measure a large one, and the + // budget then passes on exactly the implementation it exists to reject. + const rendered = await driver.readBlockBoxes(); + expect( + rendered.length, + "the tree must be on screen before timing moves against it" + ).toBeGreaterThanOrEqual(500); + await dragFromPanel(driver); - const started = Date.now(); - const moves = 20; - for (let move = 0; move < moves; move += 1) await driver.moveBy(0, 12); - const perMove = (Date.now() - started) / moves; + // Every move timed individually. A mean hides the shape that matters: one + // 2-second stall among twenty fast moves averages to a comfortable number + // while the editor visibly locks up, and a canvas that re-measures the tree + // does exactly that on the move where a rect cache misses. + const durations: number[] = []; + for (let move = 0; move < 20; move += 1) { + const started = Date.now(); + await driver.moveBy(0, 12); + durations.push(Date.now() - started); + } await driver.cancel(); + const slowest = Math.max(...durations); + const mean = durations.reduce((sum, ms) => sum + ms, 0) / durations.length; test.info().annotations.push({ type: "per-move-ms", - description: String(Math.round(perMove)), + description: `slowest=${String(slowest)} mean=${String(Math.round(mean))}`, }); // A budget, not a frame rate. Wall clock on a machine running several // matrices measures load as much as code, so this sits where only a - // re-measure-the-whole-tree-every-move regression can cross it. - expect(perMove, "a move must not re-measure the whole tree").toBeLessThan( - 120 - ); + // re-measure-the-whole-tree-every-move regression can cross it — but it is + // the SLOWEST move that has to sit under it, not the average. + expect( + slowest, + "no single move may re-measure the whole tree" + ).toBeLessThan(120); }); test("records exactly one undo entry for one drop", async ({ request }) => { diff --git a/e2e/tests/canvas/fixtures.ts b/e2e/tests/canvas/fixtures.ts index 9931053877..372025500b 100644 --- a/e2e/tests/canvas/fixtures.ts +++ b/e2e/tests/canvas/fixtures.ts @@ -119,6 +119,38 @@ export const NESTED_FIXTURE: SeedOptions = { ], }; +/** + * A document taller than the canvas viewport, so the canvas can actually + * scroll. + * + * Autoscroll is unobservable without this. The suite runs at 1400px tall and + * `NESTED_FIXTURE` renders roughly 400px of authored height, so the canvas has + * no scroll range at all — `canvasScrollTop()` cannot change, and the target + * could not pass even once autoscroll is implemented correctly. + * + * Its own fixture rather than borrowing `LARGE_FIXTURE`: that one is sized for + * a PERF budget, and a later tuning of the block count for timing reasons would + * silently take the scroll range away from this. Two questions, two fixtures. + */ +const TALL_BLOCK_HEIGHT = 200; +const TALL_COUNT = 30; +export const TALL_FIXTURE: SeedOptions = { + title: "spike tall", + slug: "spike-tall", + content: document( + Array.from({ length: TALL_COUNT }, (_unused, index) => + spacer(`nx-tall-${String(index)}`, `${String(TALL_BLOCK_HEIGHT)}px`) + ) + ), + blockIds: [ + "nx-spike-root", + ...Array.from( + { length: TALL_COUNT }, + (_unused, index) => `nx-tall-${String(index)}` + ), + ], +}; + /** 500 siblings: the tree size the perf budget is stated against. */ const LARGE_COUNT = 500; export const LARGE_FIXTURE: SeedOptions = { From 9ee09b743b4c948a0659210071956fb82869c1d0 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 02:52:03 +0500 Subject: [PATCH 04/10] test(builder): search for the collision state rather than aiming at it Two previous versions computed a coordinate inside nx-inner and moved there. Starting a drag expands every gap zone from zero height, so the layout that coordinate described no longer existed by the time the pointer arrived, and how far it shifted depended on load -- which is why it resolved to the inner container locally and to the root on CI. Capturing the rects after activation narrowed it and did not close it. Descending until the OWNER is the nested container asks the question directly instead of predicting where the answer will be. Three consecutive full runs hold. The performance budget is now differential. Absolute wall clock includes the Playwright round trip and runner scheduling, so a single GC pause read as a re-measured tree. Twenty moves are timed with no drag running as a control, twenty with one live, and the budget applies to the difference of the medians: the transport cost is common to both and cancels, and a median rather than a max stops one outlier deciding the result. The panel-drag delta correction moves into a shared dragPointerTo. Three suites carried their own copy of that arithmetic and each had the same fault, so fixing one left two. --- e2e/tests/canvas/acceptance.spec.ts | 200 ++++++++++++++-------------- e2e/tests/canvas/driver.ts | 29 ++++ e2e/tests/canvas/scenarios.spec.ts | 9 +- 3 files changed, 133 insertions(+), 105 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 16344e81f2..6786ba6e4f 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -37,7 +37,11 @@ import { seedPage, } from "./fixtures"; import { mapFramePointToHost } from "./coordinate-mapping"; -import { CanvasCapabilityError, dragUntilTarget } from "./driver"; +import { + CanvasCapabilityError, + dragPointerTo, + dragUntilTarget, +} from "./driver"; import type { CanvasChromeReader, CanvasDriver } from "./driver"; import { createPocChromeReader, createPocDriver } from "./poc-driver"; @@ -77,26 +81,7 @@ function note(point: number, becomes: string, shortfall?: string): void { async function dragFromPanel(driver: CanvasDriver): Promise { const target = await driver.canvasCentre(); await driver.startDragAt(await driver.dragSourceCentre()); - - // The delta is measured from where the pointer ACTUALLY is after activation, - // not from the source point. `startDragAt` is contractually allowed to move - // past the drag threshold, and the PoC driver shifts 12px doing so — so a - // delta computed from the source overshoots by exactly that, and a - // replacement driver with a different activation motion overshoots by a - // different amount. Asking the driver where the pointer is keeps the gesture - // landing in the same place whichever driver is behind it. - const from = driver.pointer(); - - // In steps, not one jump. A single move is a teleport, and a canvas that - // commits on dwell rather than on distance answers a teleport differently - // from the gesture a person makes. - const steps = 8; - for (let step = 0; step < steps; step += 1) { - await driver.moveBy( - (target.x - from.x) / steps, - (target.y - from.y) / steps - ); - } + await dragPointerTo(driver, target); } /** @@ -133,83 +118,52 @@ test.describe("a canvas any Nextly editor could ship", () => { note(PLAN_POINT.collisionByDepth, "B-6"); await driver.mountTree(await seedPage(request, NESTED_FIXTURE)); - // INSIDE the nested container, not merely somewhere on the canvas. The - // point of depth resolution is that two containers both contain the - // pointer and the innermost has to win, so a pointer that never entered - // `nx-inner` cannot separate that from ordinary nearest-zone handling. + // SEARCHED, not aimed. Two previous versions computed a coordinate inside + // `nx-inner` and moved there — and activation expands every gap zone from + // zero height, so the layout the coordinate described no longer existed by + // the time the pointer arrived. How far it shifted depended on load, which + // is why it resolved to the inner container locally and to the root on CI. + // + // Descending until the OWNER is the nested container asks the question + // directly instead of predicting where the answer will be. + await driver.startDragAt(await driver.dragSourceCentre()); + const boxes = await driver.readBlockBoxes(); - const first = boxes.find(box => box.id === "nx-inner-0"); - const second = boxes.find(box => box.id === "nx-inner-1"); - expect( - first && second, - "the nested children must be measurable, or the pointer cannot be aimed" - ).toBeTruthy(); - - // Enter `nx-inner` at its top, then descend until a zone activates. A - // fixed y guesses at where the zones are and lands in the dead space - // between them as often as not, which reports "no owner" and looks like a - // depth failure rather than a pointer that was never over a zone. - // CONVERTED, not used raw. `readBlockBoxes` measures inside the iframe, so - // its rects are frame-local; the pointer moves in host coordinates. Using - // one as the other is off by the frame's origin and wrong again by its - // scale, and at 100% zoom with the frame near the top-left it is close - // enough to look correct — which is how it survived. + const inner = boxes.find(box => box.id === "nx-inner"); + expect(inner, "the nested container must be measurable").toBeTruthy(); + const origin = await driver.frameOrigin(); const scale = await driver.frameScale(); - const entry = mapFramePointToHost( - { x: first!.left + first!.width / 2, y: first!.top }, + const top = mapFramePointToHost( + { x: inner!.left + inner!.width / 2, y: inner!.top }, origin, scale ); + await dragPointerTo(driver, top, 1); + + let owner: string | null = null; + let active = -1; + for (let step = 0; step < 60; step += 1) { + owner = await driver.readActiveZoneOwner(); + if (owner === "nx-inner") { + active = await driver.readActiveTarget(); + break; + } + await driver.moveBy(0, 6); + } - await driver.startDragAt(await driver.dragSourceCentre()); - const from = driver.pointer(); - await driver.moveBy(entry.x - from.x, entry.y - from.y); - const reached = await dragUntilTarget(driver); - expect( - reached, - "the drag must reach a zone inside the nested container" - ).toBeGreaterThanOrEqual(0); - - // No expected failure here, and the reason is worth recording. This case - // WAS marked as one: descending from `nx-inner` appeared to find no zone - // until well past the container's own bottom edge. That measurement was - // taken with frame-local rects used as host coordinates, so the pointer was - // never inside the container it was supposed to be in. Converted, the - // canvas resolves to the innermost container correctly. - // - // The shortfall was the harness, not the canvas. - - // And it must still be inside `nx-inner` after that descent, or the walk - // carried the pointer out the bottom and the ownership below is about a - // different container entirely. - // Both sides in the SAME space. The pointer is host, the box is frame-local, - // so comparing them directly asks a question neither coordinate answers. - const bottom = mapFramePointToHost( - { x: 0, y: second!.top + second!.height }, - origin, - scale - ); - expect( - driver.pointer().y, - "the descent must not leave the nested container" - ).toBeLessThanOrEqual(bottom.y); - - const owner = await driver.readActiveZoneOwner(); - const active = await driver.readActiveTarget(); - const nearest = await driver.nearestZoneToPointer(); + const nearest = active >= 0 ? await driver.nearestZoneToPointer() : -1; await driver.cancel(); - // The separating property, and the one the previous version never asked: - // the zone under the pointer must belong to the INNERMOST container. A - // canvas that always lets the outer container win passes an - // active-equals-nearest check and fails this. + // The separating property: a zone owned by the INNERMOST container under + // the pointer. A canvas that always lets the outer container win never + // produces this owner however far the descent goes. expect( owner, - "the innermost container under the pointer must own the drop zone" + "the innermost container under the pointer must own a drop zone" ).toBe("nx-inner"); - // Kept as well, because it catches a different fault: a stale rect or an - // unscaled transform selects a zone that is not the nearest at all. + // And it must be the nearest, which catches a different fault: a stale rect + // or an unscaled transform selects a zone that is not the nearest at all. expect(active, "and it must be the zone nearest the pointer").toBe(nearest); }); @@ -505,34 +459,78 @@ test.describe("a canvas any Nextly editor could ship", () => { "the tree must be on screen before timing moves against it" ).toBeGreaterThanOrEqual(500); + // A CONTROL first: the same pointer moves with no drag running. Every + // sample includes the Playwright protocol round trip, Node scheduling and + // whatever else a loaded runner is doing, and none of that is canvas work + // — so measuring the drag alone makes a single GC pause look exactly like + // a re-measured tree. + const idle: number[] = []; + for (let move = 0; move < 20; move += 1) { + const started = Date.now(); + await driver.moveBy(0, 4); + idle.push(Date.now() - started); + } + await dragFromPanel(driver); - // Every move timed individually. A mean hides the shape that matters: one - // 2-second stall among twenty fast moves averages to a comfortable number - // while the editor visibly locks up, and a canvas that re-measures the tree - // does exactly that on the move where a rect cache misses. - const durations: number[] = []; + const dragging: number[] = []; for (let move = 0; move < 20; move += 1) { const started = Date.now(); await driver.moveBy(0, 12); - durations.push(Date.now() - started); + dragging.push(Date.now() - started); } await driver.cancel(); - const slowest = Math.max(...durations); - const mean = durations.reduce((sum, ms) => sum + ms, 0) / durations.length; + // The DIFFERENCE of the medians. A median because one outlier is + // scheduling rather than code; a difference because the transport cost is + // common to both samples and cancels. What survives is the work the canvas + // does per move while a drag is live, which is what the budget is about. + const median = (samples: number[]): number => { + const sorted = [...samples].sort((a, b) => a - b); + return sorted[Math.floor(sorted.length / 2)] ?? 0; + }; + const canvasCost = median(dragging) - median(idle); + test.info().annotations.push({ type: "per-move-ms", - description: `slowest=${String(slowest)} mean=${String(Math.round(mean))}`, + description: `canvasCost=${String(canvasCost)} drag=${String(median(dragging))} idle=${String(median(idle))}`, }); - // A budget, not a frame rate. Wall clock on a machine running several - // matrices measures load as much as code, so this sits where only a - // re-measure-the-whole-tree-every-move regression can cross it — but it is - // the SLOWEST move that has to sit under it, not the average. + + // A budget, not a frame rate. It sits where only a + // re-measure-the-whole-tree-every-move regression can cross it: on 500 + // blocks that costs tens of milliseconds per move, while the transport it + // is measured against costs the same either way. expect( - slowest, - "no single move may re-measure the whole tree" + canvasCost, + "a move must not re-measure the whole tree" ).toBeLessThan(120); + await driver.mountTree(await seedPage(request, FLAT_LIST_FIXTURE)); + + // The canvas cannot answer this at all, and that refusal IS the + // shortfall. Asserted as the reader's OWN error type BEFORE the + // expectation is marked, so a broken selector, a missing iframe or a + // failed seed stays a real failure instead of becoming another + // expected one. It also fires the day the capability arrives: this + // line goes red first and forces the target below to be rewritten. + // + // Wrapped in an async thunk because these readers throw SYNCHRONOUSLY: + // `expect(reader())` never receives a promise, so `.rejects` cannot see + // the refusal and the raw error escapes the assertion entirely. + await expect(async () => chrome.undoDepth()).rejects.toThrow( + CanvasCapabilityError + ); + + // Marked only now. Everything above ran unprotected. + test.fail(true, "this canvas keeps no undo history to count"); + + const before = await chrome.undoDepth(); + await dragFromPanel(driver); + await driver.drop(); + const after = await chrome.undoDepth(); + + // Exactly one. A drop recorded as several makes undo feel broken: the + // author presses it once and the block half-moves. + expect(after - before, "one drop is one undoable edit").toBe(1); }); test("records exactly one undo entry for one drop", async ({ request }) => { diff --git a/e2e/tests/canvas/driver.ts b/e2e/tests/canvas/driver.ts index 772b1ecf69..5f0ad058df 100644 --- a/e2e/tests/canvas/driver.ts +++ b/e2e/tests/canvas/driver.ts @@ -290,3 +290,32 @@ export async function dragUntilTarget( } return -1; } + +/** + * Carries a panel drag to a point, measuring from where the pointer ACTUALLY is. + * + * `startDragAt` is contractually allowed to move past the drag activation + * threshold, and the PoC driver shifts 12px doing so. A delta computed from the + * SOURCE point therefore overshoots by exactly that, and a replacement driver + * with different activation motion overshoots by a different amount — which + * defeats the seam this suite exists to keep swappable. + * + * Shared rather than repeated: three suites carried their own copy of this + * arithmetic and each got the same thing wrong, so correcting one left two. + * + * In steps rather than one jump, because a single move is a teleport and a + * canvas that commits on dwell answers a teleport differently from a gesture. + */ +export async function dragPointerTo( + driver: CanvasDriver, + target: Point, + steps = 8 +): Promise { + const from = driver.pointer(); + for (let step = 0; step < steps; step += 1) { + await driver.moveBy( + (target.x - from.x) / steps, + (target.y - from.y) / steps + ); + } +} diff --git a/e2e/tests/canvas/scenarios.spec.ts b/e2e/tests/canvas/scenarios.spec.ts index a1170270e2..2279933bbe 100644 --- a/e2e/tests/canvas/scenarios.spec.ts +++ b/e2e/tests/canvas/scenarios.spec.ts @@ -11,7 +11,7 @@ */ import { expect, test } from "@playwright/test"; -import { dragUntilTarget } from "./driver"; +import { dragPointerTo, dragUntilTarget } from "./driver"; import type { ActiveTargetTransition, CanvasDriver } from "./driver"; import { EXTREME_RATIO_FIXTURE, FLAT_LIST_FIXTURE, seedPage } from "./fixtures"; import { createPocDriver } from "./poc-driver"; @@ -48,10 +48,11 @@ test.use({ viewport: { width: 2560, height: 1400 } }); /** Step the pointer down until a drop zone becomes active, and report where. */ /** Begin a drag from the insert panel and carry the pointer over the canvas. */ async function startPanelDrag(driver: CanvasDriver) { - const source = await driver.dragSourceCentre(); const target = await driver.canvasCentre(); - await driver.startDragAt(source); - await driver.moveBy(target.x - source.x, target.y - source.y); + await driver.startDragAt(await driver.dragSourceCentre()); + // Shared, because the delta must be measured from the post-activation + // pointer rather than from the source point. + await dragPointerTo(driver, target, 1); } /** From a1564d088f4d8d2844c99c8c0304852a863da5c4 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 07:23:43 +0500 Subject: [PATCH 05/10] test(builder): unsplice the undo probe, and sample the whole nested region Restoring the undo case by line range spliced its BODY into the performance test without its own wrapper, so a passing budget assertion then marked the performance case expected-to-fail and threw from the undo reader. The undo test now stands on its own and the performance test ends at its assertion. The depth loop exited on the first sample that agreed, which passes an implementation resolving correctly at one depth while the outer container wins everywhere else in the same region. It now samples every active zone while the pointer is inside the region and requires all of them to be owned by it. Comments describing the code's history are removed again. The rule is that a comment explains the code; where a correction came from belongs in the commit message and the task file. --- e2e/tests/canvas/acceptance.spec.ts | 80 +++++++++++++---------------- 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 6786ba6e4f..b6057010e8 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -118,14 +118,12 @@ test.describe("a canvas any Nextly editor could ship", () => { note(PLAN_POINT.collisionByDepth, "B-6"); await driver.mountTree(await seedPage(request, NESTED_FIXTURE)); - // SEARCHED, not aimed. Two previous versions computed a coordinate inside - // `nx-inner` and moved there — and activation expands every gap zone from - // zero height, so the layout the coordinate described no longer existed by - // the time the pointer arrived. How far it shifted depended on load, which - // is why it resolved to the inner container locally and to the root on CI. - // - // Descending until the OWNER is the nested container asks the question - // directly instead of predicting where the answer will be. + // Searched, not aimed. Activation expands every gap zone from zero height, + // so a coordinate computed before the drag starts describes a layout that + // no longer exists when the pointer arrives, and how far it shifts depends + // on how the canvas lays out under load. Descending until the OWNER is the + // nested container asks the question directly rather than predicting where + // the answer will be. await driver.startDragAt(await driver.dragSourceCentre()); const boxes = await driver.readBlockBoxes(); @@ -141,27 +139,46 @@ test.describe("a canvas any Nextly editor could ship", () => { ); await dragPointerTo(driver, top, 1); - let owner: string | null = null; + // EVERY sample taken while the pointer is inside the nested region, not the + // first one that agrees. Exiting on the first `nx-inner` would pass an + // implementation that resolves correctly at one depth and lets the outer + // container win everywhere else in the same region. + const bottom = mapFramePointToHost( + { x: 0, y: inner!.top + inner!.height }, + origin, + scale + ); + const owners: (string | null)[] = []; let active = -1; + let nearest = -1; for (let step = 0; step < 60; step += 1) { - owner = await driver.readActiveZoneOwner(); - if (owner === "nx-inner") { - active = await driver.readActiveTarget(); - break; + if (driver.pointer().y > bottom.y) break; + const owner = await driver.readActiveZoneOwner(); + if (owner !== null) { + owners.push(owner); + if (active < 0) { + active = await driver.readActiveTarget(); + nearest = await driver.nearestZoneToPointer(); + } } await driver.moveBy(0, 6); } - - const nearest = active >= 0 ? await driver.nearestZoneToPointer() : -1; await driver.cancel(); + expect( + owners.length, + "the descent must find at least one active zone inside the region" + ).toBeGreaterThan(0); + // The separating property: a zone owned by the INNERMOST container under // the pointer. A canvas that always lets the outer container win never // produces this owner however far the descent goes. + // Every one of them, so a canvas that resolves depth at one sample and not + // the next cannot pass. expect( - owner, - "the innermost container under the pointer must own a drop zone" - ).toBe("nx-inner"); + [...new Set(owners)], + "every zone inside the nested region must be owned by it" + ).toEqual(["nx-inner"]); // And it must be the nearest, which catches a different fault: a stale rect // or an unscaled transform selects a zone that is not the nearest at all. expect(active, "and it must be the zone nearest the pointer").toBe(nearest); @@ -504,33 +521,6 @@ test.describe("a canvas any Nextly editor could ship", () => { canvasCost, "a move must not re-measure the whole tree" ).toBeLessThan(120); - await driver.mountTree(await seedPage(request, FLAT_LIST_FIXTURE)); - - // The canvas cannot answer this at all, and that refusal IS the - // shortfall. Asserted as the reader's OWN error type BEFORE the - // expectation is marked, so a broken selector, a missing iframe or a - // failed seed stays a real failure instead of becoming another - // expected one. It also fires the day the capability arrives: this - // line goes red first and forces the target below to be rewritten. - // - // Wrapped in an async thunk because these readers throw SYNCHRONOUSLY: - // `expect(reader())` never receives a promise, so `.rejects` cannot see - // the refusal and the raw error escapes the assertion entirely. - await expect(async () => chrome.undoDepth()).rejects.toThrow( - CanvasCapabilityError - ); - - // Marked only now. Everything above ran unprotected. - test.fail(true, "this canvas keeps no undo history to count"); - - const before = await chrome.undoDepth(); - await dragFromPanel(driver); - await driver.drop(); - const after = await chrome.undoDepth(); - - // Exactly one. A drop recorded as several makes undo feel broken: the - // author presses it once and the block half-moves. - expect(after - before, "one drop is one undoable edit").toBe(1); }); test("records exactly one undo entry for one drop", async ({ request }) => { From 88402e5419d4d2616a78548f8af970f82e18310e Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 08:01:24 +0500 Subject: [PATCH 06/10] test(builder): resolve the canvas probes against the collision rule dnd-kit uses The depth descent stopped on an iteration limit rather than at the far edge of the nested region, so a canvas resolving depth correctly near the top and letting the outer container win lower down passed. The step count now follows from the mapped span and the exit is asserted from the pointer's position. Both edges of the nested region are excluded by one drop-zone height: the insertion gap at a boundary belongs to the outer container, so a sample landing there is a position both containers can claim. The zone assertion followed "the nearest zone wins", which is not a rule this canvas follows. `@dnd-kit/collision` resolves pointer intersection first and falls back to the dragged shape's overlap only when the pointer is inside no zone, so next to a boundary the nearest zone and the resolved zone legitimately differ. Measured: one sample of 28 diverged by one ordinal. It is now exact where a zone contains the pointer and bounded to one ordinal elsewhere, and the descent steps by half the shortest zone so it cannot step over a zone and leave the exact half with nothing to check. The perf control moved 4px per step against the drag's 12px and over a different element, so part of the difference it subtracts was distance and position rather than drag work. Both samples now differ only in whether a drag is live. Also: `dragPointerTo` rejects a step count below one instead of resolving without moving, the tall fixture derives its ids once for both the rendered nodes and the declared list, and three doc comments describing code that is no longer there are removed. --- e2e/tests/canvas/acceptance.spec.ts | 144 +++++++++++++++++++++++----- e2e/tests/canvas/driver.ts | 33 ++++++- e2e/tests/canvas/fixtures.ts | 21 ++-- e2e/tests/canvas/poc-driver.ts | 34 +++++++ e2e/tests/canvas/scenarios.spec.ts | 16 +--- 5 files changed, 199 insertions(+), 49 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index b6057010e8..46dbd2e1d9 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -132,39 +132,96 @@ test.describe("a canvas any Nextly editor could ship", () => { const origin = await driver.frameOrigin(); const scale = await driver.frameScale(); + + // A margin at each edge, in frame units. The insertion gap immediately + // before and after the nested container belongs to the OUTER one, so a + // pointer sitting on the boundary is a position both containers can + // legitimately claim. Depth priority is a statement about being INSIDE, + // and the same margin is what makes the canonical probe stable. + const EDGE_MARGIN_PX = 8; + const centreX = inner!.left + inner!.width / 2; const top = mapFramePointToHost( - { x: inner!.left + inner!.width / 2, y: inner!.top }, + { x: centreX, y: inner!.top + EDGE_MARGIN_PX }, origin, scale ); - await dragPointerTo(driver, top, 1); + const bottom = mapFramePointToHost( + { x: centreX, y: inner!.top + inner!.height - EDGE_MARGIN_PX }, + origin, + scale + ); + // The unambiguous span must exist before anything is asserted about it: a + // container shorter than two margins leaves nothing to traverse, and the + // descent below would then report a pass having sampled nothing inside. + expect( + bottom.y - top.y, + "the nested region must be tall enough to sample inside its edges" + ).toBeGreaterThan(0); + await dragPointerTo(driver, top); // EVERY sample taken while the pointer is inside the nested region, not the // first one that agrees. Exiting on the first `nx-inner` would pass an // implementation that resolves correctly at one depth and lets the outer // container win everywhere else in the same region. - const bottom = mapFramePointToHost( - { x: 0, y: inner!.top + inner!.height }, - origin, - scale + // + // The step is derived from the zone height, and it has to be: a step larger + // than a zone steps OVER it, so the pointer lands inside a drop zone only by + // coincidence and the exact assertion below is left with nothing to check. + // Measured at a fixed 6px step against 6px zones, 2 samples of 28 were + // inside any zone. Half the shortest zone cannot skip one. + // + // Read while the drag is live, because activation is what gives the zones a + // height at all — measured before it, every zone is 0 and the step derived + // from them would not advance. + const zoneHeights = (await driver.readZoneHeights()).filter( + height => height > 0 + ); + expect( + zoneHeights.length, + "the drag must have expanded the drop zones before they can be sampled" + ).toBeGreaterThan(0); + const STEP_PX = Math.max( + 1, + Math.floor((Math.min(...zoneHeights) * scale) / 2) ); + // A runaway guard, derived from the span so it cannot become the reason the + // loop stops. A fixed count silently bounds how far down a tall container + // the descent reaches, and every assertion below then describes a partial + // descent while reading as though it covered the region. What the descent + // completed is asserted separately, from the exit position. + const maxSteps = Math.ceil((bottom.y - top.y) / STEP_PX) + 2; const owners: (string | null)[] = []; - let active = -1; - let nearest = -1; - for (let step = 0; step < 60; step += 1) { - if (driver.pointer().y > bottom.y) break; + const zoneChoices: Array<{ + active: number; + nearest: number; + containing: number; + }> = []; + let step = 0; + for (; step < maxSteps && driver.pointer().y <= bottom.y; step += 1) { const owner = await driver.readActiveZoneOwner(); if (owner !== null) { owners.push(owner); - if (active < 0) { - active = await driver.readActiveTarget(); - nearest = await driver.nearestZoneToPointer(); - } + // At every sample, not just the first. A nearest-zone check taken once + // proves the mapping at one depth, which is the same weakness the owner + // check above exists to close. + zoneChoices.push({ + active: await driver.readActiveTarget(), + nearest: await driver.nearestZoneToPointer(), + containing: await driver.zoneContainingPointer(), + }); } - await driver.moveBy(0, 6); + await driver.moveBy(0, STEP_PX); } + const exitedBelow = driver.pointer().y > bottom.y; await driver.cancel(); + // The descent left the region because it crossed the far edge, not because + // it ran out of iterations. Without this the assertions below are true of + // however much of the region the loop happened to cover. + expect( + exitedBelow, + "the descent must cross the far edge of the nested region" + ).toBe(true); expect( owners.length, "the descent must find at least one active zone inside the region" @@ -179,9 +236,40 @@ test.describe("a canvas any Nextly editor could ship", () => { [...new Set(owners)], "every zone inside the nested region must be owned by it" ).toEqual(["nx-inner"]); - // And it must be the nearest, which catches a different fault: a stale rect - // or an unscaled transform selects a zone that is not the nearest at all. - expect(active, "and it must be the zone nearest the pointer").toBe(nearest); + // Which zone won, which catches a different fault: a stale rect or an + // unscaled transform resolves the pointer to a zone far from where it is. + // + // Two assertions rather than one, because the canvas answers by two rules. + // `@dnd-kit/collision` tries pointer intersection FIRST and falls back to + // the dragged shape's overlap only when the pointer is inside no zone, so + // "the nearest zone wins" is not a rule this canvas follows and asserting it + // outright fails on a legitimate boundary tie. Measured: one sample of 28 + // resolved to the zone one ordinal away from the nearest, with the pointer + // inside neither. + // + // Inside a zone, the answer is exact and has no tie to lose. + const contained = zoneChoices.filter(choice => choice.containing >= 0); + expect( + contained.length, + "the descent must sample the pointer inside a drop zone at least once" + ).toBeGreaterThan(0); + expect( + contained.filter(choice => choice.active !== choice.containing), + "a zone containing the pointer must be the zone that resolves" + ).toEqual([]); + + // Outside every zone, the fallback may legitimately choose either side of a + // boundary, so the bound is one ordinal — the resolution limit of the rule + // itself, not a pixel tolerance. It still separates what this guards: at the + // measured ~34px zone spacing a #1705 stale rect after a 200px scroll lands + // about six zones away, and a #1706 unscaled 0.75 transform drifts further + // with every step of the descent. + expect( + zoneChoices.filter( + choice => Math.abs(choice.active - choice.nearest) > 1 + ), + "the resolved zone must be the nearest to the pointer or its neighbour" + ).toEqual([]); }); test("never turns a click into a drag", async ({ request }) => { @@ -481,19 +569,31 @@ test.describe("a canvas any Nextly editor could ship", () => { // whatever else a loaded runner is doing, and none of that is canvas work // — so measuring the drag alone makes a single GC pause look exactly like // a re-measured tree. + // + // The two samples must differ in ONE thing: whether a drag is live. Move + // distance is not transport cost — hit-testing and zone selection scale + // with how far the pointer travels — so a control that steps a different + // distance, or over a different element, leaves part of the difference + // explained by something other than drag work. + const MOVE_COUNT = 20; + const MOVE_PX = 12; + const centre = await driver.canvasCentre(); + const resting = driver.pointer(); + await driver.moveBy(centre.x - resting.x, centre.y - resting.y); + const idle: number[] = []; - for (let move = 0; move < 20; move += 1) { + for (let move = 0; move < MOVE_COUNT; move += 1) { const started = Date.now(); - await driver.moveBy(0, 4); + await driver.moveBy(0, MOVE_PX); idle.push(Date.now() - started); } await dragFromPanel(driver); const dragging: number[] = []; - for (let move = 0; move < 20; move += 1) { + for (let move = 0; move < MOVE_COUNT; move += 1) { const started = Date.now(); - await driver.moveBy(0, 12); + await driver.moveBy(0, MOVE_PX); dragging.push(Date.now() - started); } await driver.cancel(); diff --git a/e2e/tests/canvas/driver.ts b/e2e/tests/canvas/driver.ts index 5f0ad058df..1135149ab4 100644 --- a/e2e/tests/canvas/driver.ts +++ b/e2e/tests/canvas/driver.ts @@ -178,6 +178,24 @@ export interface CanvasDriver { /** Every drop zone's height in canvas-local pixels, document order. */ readZoneHeights(): Promise; + /** + * Ordinal of the drop zone whose mapped rect CONTAINS the current pointer, or + * -1 when the pointer is inside none of them. + * + * The exact form of "the indicator is where the pointer is", and it needs no + * tolerance. A zone containing the pointer is the one dnd-kit's default + * detector resolves to, so a canvas that answers with any other zone has + * mapped the pointer wrongly — which is what both the stale-rect (#1705) and + * unscaled-transform (#1706) failures do. + * + * Containment rather than proximity, because proximity is not a rule this + * canvas follows. `@dnd-kit/collision` ranks a containing zone by pointer + * intersection FIRST and only falls back to the dragged shape's overlap when + * no zone contains the pointer, so the nearest zone by centre distance and + * the resolved zone legitimately differ next to a boundary. + */ + zoneContainingPointer(): Promise; + /** * Ordinal of the drop zone geometrically nearest the current pointer. * @@ -300,8 +318,9 @@ export async function dragUntilTarget( * with different activation motion overshoots by a different amount — which * defeats the seam this suite exists to keep swappable. * - * Shared rather than repeated: three suites carried their own copy of this - * arithmetic and each got the same thing wrong, so correcting one left two. + * Shared rather than repeated, so every suite measures the delta the same way. + * A per-suite copy of this arithmetic is invisible when it is wrong: the drag + * still runs and still ends somewhere plausible, and only the distance is off. * * In steps rather than one jump, because a single move is a teleport and a * canvas that commits on dwell answers a teleport differently from a gesture. @@ -311,6 +330,16 @@ export async function dragPointerTo( target: Point, steps = 8 ): Promise { + // A precondition, not a clamp. Zero or fewer steps runs no move at all, so + // the helper resolves with the pointer where it started and the caller's next + // assertion reports a canvas fault for a gesture that never happened. A + // fractional count leaves the pointer short of the target for the same + // reason, with nothing to distinguish it from a canvas that ignored the move. + if (!Number.isInteger(steps) || steps < 1) { + throw new Error( + `dragPointerTo needs a whole number of steps, at least 1; got ${String(steps)}` + ); + } const from = driver.pointer(); for (let step = 0; step < steps; step += 1) { await driver.moveBy( diff --git a/e2e/tests/canvas/fixtures.ts b/e2e/tests/canvas/fixtures.ts index 372025500b..859cec410b 100644 --- a/e2e/tests/canvas/fixtures.ts +++ b/e2e/tests/canvas/fixtures.ts @@ -134,21 +134,22 @@ export const NESTED_FIXTURE: SeedOptions = { */ const TALL_BLOCK_HEIGHT = 200; const TALL_COUNT = 30; +// One sequence, read by both the rendered nodes and the declared ids. Two +// generators of the same names agree the day they are written; a later change +// to the count or the naming applied to one leaves the fixture declaring a +// document it does not render, and `mountTree` then waits for ids that never +// appear. +const TALL_BLOCK_IDS = Array.from( + { length: TALL_COUNT }, + (_unused, index) => `nx-tall-${String(index)}` +); export const TALL_FIXTURE: SeedOptions = { title: "spike tall", slug: "spike-tall", content: document( - Array.from({ length: TALL_COUNT }, (_unused, index) => - spacer(`nx-tall-${String(index)}`, `${String(TALL_BLOCK_HEIGHT)}px`) - ) + TALL_BLOCK_IDS.map(id => spacer(id, `${String(TALL_BLOCK_HEIGHT)}px`)) ), - blockIds: [ - "nx-spike-root", - ...Array.from( - { length: TALL_COUNT }, - (_unused, index) => `nx-tall-${String(index)}` - ), - ], + blockIds: ["nx-spike-root", ...TALL_BLOCK_IDS], }; /** 500 siblings: the tree size the perf budget is stated against. */ diff --git a/e2e/tests/canvas/poc-driver.ts b/e2e/tests/canvas/poc-driver.ts index 983c532eae..16264b5c50 100644 --- a/e2e/tests/canvas/poc-driver.ts +++ b/e2e/tests/canvas/poc-driver.ts @@ -239,6 +239,40 @@ export function createPocDriver(page: Page): CanvasDriver { ); }, + async zoneContainingPointer() { + const rects = await canvasFrame().evaluate( + selector => + Array.from(document.querySelectorAll(selector)).map(el => { + const r = el.getBoundingClientRect(); + return { y: r.y, height: r.height }; + }), + DROP_ZONES + ); + if (rects.length === 0) return -1; + + const origin = await driver.frameOrigin(); + const scale = await frameScale(); + const pointerY = pointer.y; + + // The FIRST containing zone, not the nearest of several. Gap zones do not + // overlap, so at most one can contain a point; taking the first keeps the + // answer defined if a canvas ever registers overlapping ones rather than + // silently picking whichever compared smaller. + return rects.findIndex(rect => { + // Mapped by the shared helper rather than multiplied out here. Written + // inline this is two numbers scaled and added, which is exactly the + // shape no import scan can tell from ordinary arithmetic — so it is the + // one that drifts silently when the mapping is corrected. + const top = mapFramePointToHost({ x: 0, y: rect.y }, origin, scale).y; + const bottom = mapFramePointToHost( + { x: 0, y: rect.y + rect.height }, + origin, + scale + ).y; + return pointerY >= top && pointerY <= bottom; + }); + }, + async nearestZoneToPointer() { const rects = await canvasFrame().evaluate( selector => diff --git a/e2e/tests/canvas/scenarios.spec.ts b/e2e/tests/canvas/scenarios.spec.ts index 2279933bbe..44bc6571b7 100644 --- a/e2e/tests/canvas/scenarios.spec.ts +++ b/e2e/tests/canvas/scenarios.spec.ts @@ -34,25 +34,11 @@ test.describe.configure({ timeout: 180_000 }); */ test.use({ viewport: { width: 2560, height: 1400 } }); -/** - * How far the reported indicator may sit from the pointer before the mapping is - * considered wrong. - * - * Generous on purpose: collision detection may legitimately choose a zone a - * little away from the pointer. It is still far tighter than every failure it - * guards. A stale-rect bug after a 200px scroll misreports by ~200px, and an - * unscaled 0.75 transform misreports by ~25% of the travel, which exceeds this - * within the first 240px of a sweep. - */ - -/** Step the pointer down until a drop zone becomes active, and report where. */ /** Begin a drag from the insert panel and carry the pointer over the canvas. */ async function startPanelDrag(driver: CanvasDriver) { const target = await driver.canvasCentre(); await driver.startDragAt(await driver.dragSourceCentre()); - // Shared, because the delta must be measured from the post-activation - // pointer rather than from the source point. - await dragPointerTo(driver, target, 1); + await dragPointerTo(driver, target); } /** From 6e2445f09106755cc4ed603fdae0060d0f872c91 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 08:49:36 +0500 Subject: [PATCH 07/10] test(builder): measure the nested region live, and give the indicator probes a precondition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The depth descent computed the nested region once and then asserted against it for the whole traversal. Gap zones expand when the drag activates and the canvas reflows as they do, so the span moved underneath the descent: the pointer left the container while assertions written against the stale span still called it inside, and the outer container was recorded owning a zone that contained the pointer. Two runs in six. The region is now re-measured every sample, and the runaway guard is bounded by the whole document rather than by a span that grows. Ownership is asserted where the collision rule is unambiguous — where the pointer is inside a zone — because under the shape-overlap fallback the ancestor's insertion gap before the container is a legitimate candidate. What that does not establish is depth priority, and the comment says so: this canvas registers no collision priority at all, so the nested container wins by being the zone under the pointer rather than by being the deeper one. Both indicator probes started a panel drag and read an indicator without establishing that one exists. The reader raises only when it finds an indicator it cannot model and answers a count of zero when the canvas draws none, so the refusal being asserted was contingent on where the drag happened to stop. Measured, it resolved with a count of zero and the missing indicator read as a canvas that had gained the capability. `puts the indicator in the gap the pointer is over` no longer expects to fail. Its reason was not true — the driver maps the indicator's frame rect into host coordinates, so it is comparable with the pointer. What kept it red was the unestablished precondition paired with a bound tighter than the fault it names: a correct indicator 31px from the pointer was rejected by a 24px constant while the blocks it must not trail are ~92px apart. The bound is now the shortest block in the tree under the drag. --- e2e/tests/canvas/acceptance.spec.ts | 175 +++++++++++++++++++++------- 1 file changed, 131 insertions(+), 44 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 46dbd2e1d9..235349cd6a 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -157,7 +157,20 @@ test.describe("a canvas any Nextly editor could ship", () => { bottom.y - top.y, "the nested region must be tall enough to sample inside its edges" ).toBeGreaterThan(0); - await dragPointerTo(driver, top); + + // The descent starts ABOVE the region and enters it under its own steps. + // Carrying the pointer straight to the first position to be asserted on + // makes that reading the one taken immediately after a long jump, before + // the canvas has necessarily resolved the arrival — and the outer container, + // which owned the target throughout the approach, is then recorded as + // owning a position inside the nested one. Observed once in five runs, at + // the first sample and nowhere else. + const approach = mapFramePointToHost( + { x: centreX, y: inner!.top - EDGE_MARGIN_PX }, + origin, + scale + ); + await dragPointerTo(driver, approach); // EVERY sample taken while the pointer is inside the nested region, not the // first one that agrees. Exiting on the first `nx-inner` would pass an @@ -184,27 +197,63 @@ test.describe("a canvas any Nextly editor could ship", () => { 1, Math.floor((Math.min(...zoneHeights) * scale) / 2) ); - // A runaway guard, derived from the span so it cannot become the reason the - // loop stops. A fixed count silently bounds how far down a tall container - // the descent reaches, and every assertion below then describes a partial - // descent while reading as though it covered the region. What the descent - // completed is asserted separately, from the exit position. - const maxSteps = Math.ceil((bottom.y - top.y) / STEP_PX) + 2; - const owners: (string | null)[] = []; + // A runaway guard, and it is bounded by the WHOLE document rather than by + // the nested span: the span grows under the descent as gap zones expand, so + // a cap sized to the span measured beforehand runs out partway down and the + // descent stops for the one reason this guard exists to rule out. Nothing + // legitimate needs more steps than the document is tall. What the descent + // completed is asserted separately, from the exit condition. + const root = boxes[0]; + expect(root, "the document root must be measurable").toBeTruthy(); + const maxSteps = Math.ceil((root!.height * scale) / STEP_PX) + 4; + const owners: string[] = []; const zoneChoices: Array<{ + owner: string; active: number; nearest: number; containing: number; }> = []; let step = 0; - for (; step < maxSteps && driver.pointer().y <= bottom.y; step += 1) { - const owner = await driver.readActiveZoneOwner(); + let exitedBelow = false; + for (; step < maxSteps; step += 1) { + // The region is re-measured every sample, not computed once above. Gap + // zones expand when the drag activates and the canvas reflows as they do, + // so a span taken before the descent describes a layout that has since + // moved — and the pointer then sits outside the nested container while + // an assertion written against the stale span still calls it inside. + // Measured: two runs in six recorded the outer container owning a zone + // that contained the pointer, at positions the stale span called inside. + const live = (await driver.readBlockBoxes()).find( + box => box.id === "nx-inner" + ); + if (!live) break; + const liveTop = mapFramePointToHost( + { x: centreX, y: live.top + EDGE_MARGIN_PX }, + origin, + scale + ).y; + const liveBottom = mapFramePointToHost( + { x: centreX, y: live.top + live.height - EDGE_MARGIN_PX }, + origin, + scale + ).y; + if (driver.pointer().y > liveBottom) { + exitedBelow = true; + break; + } + + // Recorded by POSITION, never by loop index. The descent begins above the + // region, so nothing measured on the way in is attributed to a position + // inside it. + const inside = driver.pointer().y >= liveTop; + const owner = inside ? await driver.readActiveZoneOwner() : null; if (owner !== null) { owners.push(owner); - // At every sample, not just the first. A nearest-zone check taken once - // proves the mapping at one depth, which is the same weakness the owner - // check above exists to close. + // At every sample, not just the first. A check taken once proves the + // mapping at one depth, which is the same weakness the owner check + // exists to close. zoneChoices.push({ + owner, active: await driver.readActiveTarget(), nearest: await driver.nearestZoneToPointer(), containing: await driver.zoneContainingPointer(), @@ -212,7 +261,6 @@ test.describe("a canvas any Nextly editor could ship", () => { } await driver.moveBy(0, STEP_PX); } - const exitedBelow = driver.pointer().y > bottom.y; await driver.cancel(); // The descent left the region because it crossed the far edge, not because @@ -227,28 +275,40 @@ test.describe("a canvas any Nextly editor could ship", () => { "the descent must find at least one active zone inside the region" ).toBeGreaterThan(0); - // The separating property: a zone owned by the INNERMOST container under - // the pointer. A canvas that always lets the outer container win never - // produces this owner however far the descent goes. - // Every one of them, so a canvas that resolves depth at one sample and not - // the next cannot pass. + // Ownership is asserted where the collision rule is UNAMBIGUOUS, which is + // where the pointer is inside a zone: `@dnd-kit/collision` tries pointer + // intersection first and only falls back to the dragged shape's overlap when + // the pointer is inside none. Under that fallback the ancestor's insertion + // gap immediately before the nested container is a legitimate candidate, + // because the dragged shape overlaps it too — so an ancestor winning there + // is not a fault, and asserting over every sample makes the test fail on + // correct behaviour. Measured: over five runs the root owned the target on + // two of them, at the topmost samples of the region and nowhere else. + // + // What this does NOT establish is depth priority, and the distinction + // matters because the title claims it. This canvas registers no collision + // priority at all — the term appears nowhere in `plugin-page-builder`, while + // `@dnd-kit/collision` uses it throughout — so the nested container wins + // these samples by being the zone under the pointer, not by being the + // deeper one. A canvas with no notion of depth passes this, which is why + // the boundary case is recorded as an acceptance shortfall rather than + // silently widened away. + const contained = zoneChoices.filter(choice => choice.containing >= 0); expect( - [...new Set(owners)], - "every zone inside the nested region must be owned by it" + [...new Set(contained.map(choice => choice.owner))], + "a zone containing the pointer inside the nested region must be its own" ).toEqual(["nx-inner"]); + // Which zone won, which catches a different fault: a stale rect or an // unscaled transform resolves the pointer to a zone far from where it is. // // Two assertions rather than one, because the canvas answers by two rules. - // `@dnd-kit/collision` tries pointer intersection FIRST and falls back to - // the dragged shape's overlap only when the pointer is inside no zone, so - // "the nearest zone wins" is not a rule this canvas follows and asserting it + // "The nearest zone wins" is not one this canvas follows, so asserting it // outright fails on a legitimate boundary tie. Measured: one sample of 28 // resolved to the zone one ordinal away from the nearest, with the pointer // inside neither. // // Inside a zone, the answer is exact and has no tie to lose. - const contained = zoneChoices.filter(choice => choice.containing >= 0); expect( contained.length, "the descent must sample the pointer inside a drop zone at least once" @@ -260,10 +320,15 @@ test.describe("a canvas any Nextly editor could ship", () => { // Outside every zone, the fallback may legitimately choose either side of a // boundary, so the bound is one ordinal — the resolution limit of the rule - // itself, not a pixel tolerance. It still separates what this guards: at the - // measured ~34px zone spacing a #1705 stale rect after a 200px scroll lands - // about six zones away, and a #1706 unscaled 0.75 transform drifts further - // with every step of the descent. + // itself, not a pixel tolerance. + // + // It is the WEAKER of the two, deliberately, and the exact assertion above + // is what carries the guarantee. Zone spacing here is not uniform (measured + // 9, 10, 92, 92, 132 and 133 frame px between centres), so one ordinal is + // ten pixels of slack in the tight places and over a hundred in the loose + // ones. A bound stated in ordinals cannot be tight everywhere, which is + // exactly why the containing-zone case is asserted exactly rather than + // folded in here. expect( zoneChoices.filter( choice => Math.abs(choice.active - choice.nearest) > 1 @@ -385,7 +450,13 @@ test.describe("a canvas any Nextly editor could ship", () => { "this canvas draws its indicator inside the iframe with CSS" ); await driver.mountTree(await seedPage(request, FLAT_LIST_FIXTURE)); - await dragFromPanel(driver); + // Onto a zone, not merely over the canvas. The refusal asserted below is + // contingent on an indicator EXISTING: the reader raises only when it finds + // one it cannot model, and answers `{count: 0}` when the canvas is drawing + // none. Stopping wherever the panel drag lands leaves that to chance — + // measured, the reader resolved with a count of zero and the assertion read + // the missing indicator as a canvas that had gained the capability. + await dragOntoZone(driver); // The canvas cannot answer this at all, and that refusal IS the // shortfall. Asserted as the reader's OWN error type BEFORE the @@ -418,25 +489,41 @@ test.describe("a canvas any Nextly editor could ship", () => { test("puts the indicator in the gap the pointer is over", async ({ request, }) => { - note( - PLAN_POINT.indicatorLeadsIntoGap, - "B-7", - "the indicator is not a host element, so its rect is not comparable" - ); + note(PLAN_POINT.indicatorLeadsIntoGap, "B-7"); await driver.mountTree(await seedPage(request, FLAT_LIST_FIXTURE)); - await dragFromPanel(driver); + // Onto a zone, not merely over the canvas: with no zone active the canvas + // draws no indicator and the rect is null, so the assertions below would be + // recorded against an absent indicator rather than against the one they are + // about. + await dragOntoZone(driver); const rect = await driver.readIndicatorRect(); const pointer = driver.pointer(); + // The bound comes from the tree being dragged over, not from a constant. + // The fault this names is an indicator trailing the pointer by a whole + // block, so the shortest block is what "a whole block" means here; a fixed + // number encodes whatever the fixture's spacing happened to be the day it + // was written. Measured, the indicator sits 31px from the pointer while the + // blocks are ~92px apart, so the previous constant of 24 rejected an + // indicator that was in the correct gap. + const blocks = await driver.readBlockBoxes(); + const scale = await driver.frameScale(); + const shortestBlock = Math.min( + ...blocks.slice(1).map(box => box.height * scale) + ); await driver.cancel(); + expect( + shortestBlock, + "the fixture must have blocks to measure the bound against" + ).toBeGreaterThan(0); - // Marked HERE, not on the declaration. The declaration form makes - // EVERY error in the body expected, so a failed seed or a broken - // reader goes green exactly like the shortfall. - test.fail( - true, - "the indicator is not a host element, so its rect is not comparable" - ); + // No expected-failure marking. This property is MET, and the marking it + // used to carry named a reason that is not true: the driver maps the + // indicator's frame rect into host coordinates, so it is comparable with the + // pointer. What kept the test red was the pairing of an unestablished + // precondition with a bound tighter than the fault it describes — with no + // zone active the rect was null, and where a rect existed a correct + // indicator 31px away was rejected by a 24px constant. expect(rect, "a drag in progress must show an indicator").not.toBeNull(); // In the gap, not merely somewhere on screen. What this catches is an // indicator trailing the pointer by a whole block. @@ -444,7 +531,7 @@ test.describe("a canvas any Nextly editor could ship", () => { expect( Math.abs(centre - pointer.y), "the indicator must lead the pointer into the gap it names" - ).toBeLessThanOrEqual(24); + ).toBeLessThan(shortestBlock); }); test("shows an explicit state over an invalid target", async ({ From ea057c4f5f36a7c6b0c922330f59d8e78ffb0090 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 13:30:13 +0500 Subject: [PATCH 08/10] test(builder): describe the indicator bound without its history The comment recounted the marking the test used to carry, why it previously stayed red and the constant it replaced. Only the current rationale remains: why the bound comes from the tree under the drag, and why the rect is comparable with the pointer. --- e2e/tests/canvas/acceptance.spec.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 235349cd6a..7178b5118d 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -502,10 +502,8 @@ test.describe("a canvas any Nextly editor could ship", () => { // The bound comes from the tree being dragged over, not from a constant. // The fault this names is an indicator trailing the pointer by a whole // block, so the shortest block is what "a whole block" means here; a fixed - // number encodes whatever the fixture's spacing happened to be the day it - // was written. Measured, the indicator sits 31px from the pointer while the - // blocks are ~92px apart, so the previous constant of 24 rejected an - // indicator that was in the correct gap. + // number encodes whatever the fixture's spacing happens to be, which is a + // property of the fixture rather than of the requirement. const blocks = await driver.readBlockBoxes(); const scale = await driver.frameScale(); const shortestBlock = Math.min( @@ -517,13 +515,9 @@ test.describe("a canvas any Nextly editor could ship", () => { "the fixture must have blocks to measure the bound against" ).toBeGreaterThan(0); - // No expected-failure marking. This property is MET, and the marking it - // used to carry named a reason that is not true: the driver maps the - // indicator's frame rect into host coordinates, so it is comparable with the - // pointer. What kept the test red was the pairing of an unestablished - // precondition with a bound tighter than the fault it describes — with no - // zone active the rect was null, and where a rect existed a correct - // indicator 31px away was rejected by a 24px constant. + // No expected-failure marking: this canvas meets the property. The rect is + // comparable with the pointer because the driver maps it out of frame + // coordinates into host ones. expect(rect, "a drag in progress must show an indicator").not.toBeNull(); // In the gap, not merely somewhere on screen. What this catches is an // indicator trailing the pointer by a whole block. From 9a014efe3033973d74989ec7f53a8b10e6e875d0 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 13:35:30 +0500 Subject: [PATCH 09/10] test(builder): record the depth point as unseparated rather than covered The collision case passes, and what it measures is weaker than the property it is named for. Depth priority decides which container wins when both could claim the pointer; this canvas has no such rule, and dnd-kit's default detector ranks by pointer containment then by the dragged shape's overlap, neither of which takes depth as an input. The case passes because the nested container's own gap zones cover its interior, so the container under the pointer is also the owner of the zone under it. Separating the two needs a position where an ancestor's zone competes inside a descendant, and this fixture offers none: every ancestor zone lies outside the nested container's box. The annotation now records that as a shortfall, which keeps the plan point off the covered list without marking an expected failure that would report red for a canvas answering every position correctly. Also removes run tallies from four comments, and a mechanism one of them named that the live re-measurement showed was not the cause. --- e2e/tests/canvas/acceptance.spec.ts | 57 ++++++++++++++++------------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index 7178b5118d..d5ced53faf 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -115,7 +115,28 @@ test.describe("a canvas any Nextly editor could ship", () => { test("resolves a pointer collision to the innermost container", async ({ request, }) => { - note(PLAN_POINT.collisionByDepth, "B-6"); + // Recorded as falling short even though it passes, because what it measures + // is weaker than the property it is named for. Depth priority says the + // DEEPER container wins when both could claim the pointer; this canvas has + // no such rule — `plugin-page-builder` registers no collision priority + // anywhere, while `@dnd-kit/collision` uses the concept throughout — and its + // default detector ranks by pointer containment then by the dragged shape's + // overlap, neither of which takes depth as an input. + // + // It passes regardless because the nested container's own gap zones cover + // its interior, so the container under the pointer is also the owner of the + // zone under the pointer. Separating the two needs a position where an + // ancestor's zone competes INSIDE a descendant, and this fixture offers + // none: every ancestor zone lies outside the nested container's box. + // + // So the shortfall is the missing coverage rather than a missing behaviour, + // and it is recorded here rather than as an expected failure, which would + // report red for a canvas that answers every position correctly. + note( + PLAN_POINT.collisionByDepth, + "B-6", + "no position in this fixture makes two depths compete, so depth priority is unseparated" + ); await driver.mountTree(await seedPage(request, NESTED_FIXTURE)); // Searched, not aimed. Activation expands every gap zone from zero height, @@ -158,13 +179,10 @@ test.describe("a canvas any Nextly editor could ship", () => { "the nested region must be tall enough to sample inside its edges" ).toBeGreaterThan(0); - // The descent starts ABOVE the region and enters it under its own steps. - // Carrying the pointer straight to the first position to be asserted on - // makes that reading the one taken immediately after a long jump, before - // the canvas has necessarily resolved the arrival — and the outer container, - // which owned the target throughout the approach, is then recorded as - // owning a position inside the nested one. Observed once in five runs, at - // the first sample and nowhere else. + // The descent starts ABOVE the region and enters it under its own steps, so + // no assertion rests on the reading taken immediately after the long jump + // that carries the pointer here. Every recorded sample then follows one + // small step, which is the same treatment for all of them. const approach = mapFramePointToHost( { x: centreX, y: inner!.top - EDGE_MARGIN_PX }, origin, @@ -180,8 +198,7 @@ test.describe("a canvas any Nextly editor could ship", () => { // The step is derived from the zone height, and it has to be: a step larger // than a zone steps OVER it, so the pointer lands inside a drop zone only by // coincidence and the exact assertion below is left with nothing to check. - // Measured at a fixed 6px step against 6px zones, 2 samples of 28 were - // inside any zone. Half the shortest zone cannot skip one. + // Half the shortest zone cannot skip one. // // Read while the drag is live, because activation is what gives the zones a // height at all — measured before it, every zone is 0 and the step derived @@ -221,8 +238,6 @@ test.describe("a canvas any Nextly editor could ship", () => { // so a span taken before the descent describes a layout that has since // moved — and the pointer then sits outside the nested container while // an assertion written against the stale span still calls it inside. - // Measured: two runs in six recorded the outer container owning a zone - // that contained the pointer, at positions the stale span called inside. const live = (await driver.readBlockBoxes()).find( box => box.id === "nx-inner" ); @@ -282,17 +297,10 @@ test.describe("a canvas any Nextly editor could ship", () => { // gap immediately before the nested container is a legitimate candidate, // because the dragged shape overlaps it too — so an ancestor winning there // is not a fault, and asserting over every sample makes the test fail on - // correct behaviour. Measured: over five runs the root owned the target on - // two of them, at the topmost samples of the region and nowhere else. + // correct behaviour. // - // What this does NOT establish is depth priority, and the distinction - // matters because the title claims it. This canvas registers no collision - // priority at all — the term appears nowhere in `plugin-page-builder`, while - // `@dnd-kit/collision` uses it throughout — so the nested container wins - // these samples by being the zone under the pointer, not by being the - // deeper one. A canvas with no notion of depth passes this, which is why - // the boundary case is recorded as an acceptance shortfall rather than - // silently widened away. + // What this does NOT establish is depth priority; the annotation at the top + // of the test records that and why. const contained = zoneChoices.filter(choice => choice.containing >= 0); expect( [...new Set(contained.map(choice => choice.owner))], @@ -304,9 +312,8 @@ test.describe("a canvas any Nextly editor could ship", () => { // // Two assertions rather than one, because the canvas answers by two rules. // "The nearest zone wins" is not one this canvas follows, so asserting it - // outright fails on a legitimate boundary tie. Measured: one sample of 28 - // resolved to the zone one ordinal away from the nearest, with the pointer - // inside neither. + // outright fails on a legitimate boundary tie: a zone one ordinal from the + // nearest can win when the pointer is inside neither. // // Inside a zone, the answer is exact and has no tie to lose. expect( From 6e507fa50d3fe38c9c7969eabc8caf4938b57254 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 13 Aug 2026 14:18:27 +0500 Subject: [PATCH 10/10] test(builder): read both zone answers from one snapshot, and measure containment unconditionally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The containment and nearest-zone readers each queried and mapped the drop-zone geometry themselves. Two readings of one thing agree until a selector, an ordering, the frame origin or the mapping is corrected in one and not the other, and the depth probe then compares two different models of the canvas: either a failure naming the canvas for a fault in the harness, or a pass that is self-consistent and wrong. Both now derive from one mapped snapshot. Containment was recorded only where a target had already activated, which drops exactly the samples that matter. A canvas missing most of its zones while activating one leaves every miss out of the record, and the samples that survive all agree. It is measured at every sample inside the region now, and a zone that contains the pointer without activating a target is a failure rather than a row that leaves the set. The indicator bound came from `Math.min()` over the blocks below the root, which is `Infinity` when only the root is measured — a child selector that stopped matching, or a replacement driver reporting less. The precondition passed on it and the distance assertion then accepted an indicator anywhere on screen. The children must exist, and the derived bound must be a real distance. --- e2e/tests/canvas/acceptance.spec.ts | 54 ++++++++++----- e2e/tests/canvas/poc-driver.ts | 104 ++++++++++++++-------------- 2 files changed, 89 insertions(+), 69 deletions(-) diff --git a/e2e/tests/canvas/acceptance.spec.ts b/e2e/tests/canvas/acceptance.spec.ts index d5ced53faf..200d9bfba0 100644 --- a/e2e/tests/canvas/acceptance.spec.ts +++ b/e2e/tests/canvas/acceptance.spec.ts @@ -225,7 +225,7 @@ test.describe("a canvas any Nextly editor could ship", () => { const maxSteps = Math.ceil((root!.height * scale) / STEP_PX) + 4; const owners: string[] = []; const zoneChoices: Array<{ - owner: string; + owner: string | null; active: number; nearest: number; containing: number; @@ -260,18 +260,20 @@ test.describe("a canvas any Nextly editor could ship", () => { // Recorded by POSITION, never by loop index. The descent begins above the // region, so nothing measured on the way in is attributed to a position // inside it. - const inside = driver.pointer().y >= liveTop; - const owner = inside ? await driver.readActiveZoneOwner() : null; - if (owner !== null) { - owners.push(owner); - // At every sample, not just the first. A check taken once proves the - // mapping at one depth, which is the same weakness the owner check - // exists to close. + if (driver.pointer().y >= liveTop) { + // Containment is measured at EVERY sample inside the region, before + // anything is known about whether a target activated. Recording it only + // where one did would drop exactly the positions that matter: a canvas + // missing most of its zones and activating one leaves the misses + // invisible, and the samples that survive all agree. + const containing = await driver.zoneContainingPointer(); + const owner = await driver.readActiveZoneOwner(); + if (owner !== null) owners.push(owner); zoneChoices.push({ owner, active: await driver.readActiveTarget(), nearest: await driver.nearestZoneToPointer(), - containing: await driver.zoneContainingPointer(), + containing, }); } await driver.moveBy(0, STEP_PX); @@ -302,6 +304,15 @@ test.describe("a canvas any Nextly editor could ship", () => { // What this does NOT establish is depth priority; the annotation at the top // of the test records that and why. const contained = zoneChoices.filter(choice => choice.containing >= 0); + // A zone under the pointer must be the ACTIVE one. Asserted before the + // owner, because ownership of a zone nothing selected says nothing about + // what the canvas resolved: a sample where the pointer sits inside a zone + // and no target is active is a zone the canvas missed, and it has to fail + // rather than quietly leave the set. + expect( + contained.filter(choice => choice.owner === null), + "a zone containing the pointer must have activated a target" + ).toEqual([]); expect( [...new Set(contained.map(choice => choice.owner))], "a zone containing the pointer inside the nested region must be its own" @@ -337,9 +348,9 @@ test.describe("a canvas any Nextly editor could ship", () => { // exactly why the containing-zone case is asserted exactly rather than // folded in here. expect( - zoneChoices.filter( - choice => Math.abs(choice.active - choice.nearest) > 1 - ), + zoneChoices + .filter(choice => choice.active >= 0) + .filter(choice => Math.abs(choice.active - choice.nearest) > 1), "the resolved zone must be the nearest to the pointer or its neighbour" ).toEqual([]); }); @@ -513,14 +524,23 @@ test.describe("a canvas any Nextly editor could ship", () => { // property of the fixture rather than of the requirement. const blocks = await driver.readBlockBoxes(); const scale = await driver.frameScale(); - const shortestBlock = Math.min( - ...blocks.slice(1).map(box => box.height * scale) - ); + const childHeights = blocks.slice(1).map(box => box.height * scale); await driver.cancel(); + // The children have to EXIST before a bound is derived from them. With only + // the root measured — a child selector that stopped matching, a replacement + // driver reporting less — `Math.min()` of nothing is `Infinity`, a + // "greater than 0" precondition passes on it, and the distance assertion + // below accepts any indicator anywhere on screen while reporting this + // acceptance point green. expect( - shortestBlock, - "the fixture must have blocks to measure the bound against" + childHeights.length, + "the fixture must render blocks to measure the bound against" ).toBeGreaterThan(0); + const shortestBlock = Math.min(...childHeights); + expect( + Number.isFinite(shortestBlock) && shortestBlock > 0, + "the derived bound must be a real distance" + ).toBe(true); // No expected-failure marking: this canvas meets the property. The rect is // comparable with the pointer because the driver maps it out of frame diff --git a/e2e/tests/canvas/poc-driver.ts b/e2e/tests/canvas/poc-driver.ts index 16264b5c50..d51f519bc5 100644 --- a/e2e/tests/canvas/poc-driver.ts +++ b/e2e/tests/canvas/poc-driver.ts @@ -112,6 +112,51 @@ export function createPocDriver(page: Page): CanvasDriver { }); } + /** + * Every drop zone's vertical extent in HOST coordinates, document order. + * + * ONE snapshot, because the two questions asked of it — which zone contains + * the pointer, and which zone's centre is nearest — are two views of the same + * geometry. Read separately they agree until a selector, an ordering, the + * frame origin or the mapping is corrected in one and not the other, and the + * depth probe then compares two different models of the canvas: either a + * failure that names the canvas for a fault in the harness, or a pass that is + * self-consistent and wrong. + * + * The centre comes from the mapped edges rather than from mapping the frame's + * own centre. Under an affine map the two are equal, and deriving it here + * keeps a single mapped value as the source for both answers. + */ + async function mappedZoneSpans(): Promise< + { top: number; bottom: number; centre: number }[] + > { + const rects = await canvasFrame().evaluate( + selector => + Array.from(document.querySelectorAll(selector)).map(el => { + const r = el.getBoundingClientRect(); + return { y: r.y, height: r.height }; + }), + DROP_ZONES + ); + if (rects.length === 0) return []; + + const origin = await driver.frameOrigin(); + const scale = await frameScale(); + // Mapped by the shared helper rather than multiplied out here. Written + // inline this is two numbers scaled and added, which is exactly the shape + // no import scan can tell from ordinary arithmetic — so it is the one that + // drifts silently when the mapping is corrected. + return rects.map(rect => { + const top = mapFramePointToHost({ x: 0, y: rect.y }, origin, scale).y; + const bottom = mapFramePointToHost( + { x: 0, y: rect.y + rect.height }, + origin, + scale + ).y; + return { top, bottom, centre: (top + bottom) / 2 }; + }); + } + let pointer: Point = { x: 0, y: 0 }; const driver: CanvasDriver = { @@ -240,67 +285,22 @@ export function createPocDriver(page: Page): CanvasDriver { }, async zoneContainingPointer() { - const rects = await canvasFrame().evaluate( - selector => - Array.from(document.querySelectorAll(selector)).map(el => { - const r = el.getBoundingClientRect(); - return { y: r.y, height: r.height }; - }), - DROP_ZONES - ); - if (rects.length === 0) return -1; - - const origin = await driver.frameOrigin(); - const scale = await frameScale(); - const pointerY = pointer.y; - + const spans = await mappedZoneSpans(); // The FIRST containing zone, not the nearest of several. Gap zones do not // overlap, so at most one can contain a point; taking the first keeps the // answer defined if a canvas ever registers overlapping ones rather than // silently picking whichever compared smaller. - return rects.findIndex(rect => { - // Mapped by the shared helper rather than multiplied out here. Written - // inline this is two numbers scaled and added, which is exactly the - // shape no import scan can tell from ordinary arithmetic — so it is the - // one that drifts silently when the mapping is corrected. - const top = mapFramePointToHost({ x: 0, y: rect.y }, origin, scale).y; - const bottom = mapFramePointToHost( - { x: 0, y: rect.y + rect.height }, - origin, - scale - ).y; - return pointerY >= top && pointerY <= bottom; - }); + return spans.findIndex( + span => pointer.y >= span.top && pointer.y <= span.bottom + ); }, async nearestZoneToPointer() { - const rects = await canvasFrame().evaluate( - selector => - Array.from(document.querySelectorAll(selector)).map(el => { - const r = el.getBoundingClientRect(); - return { y: r.y, height: r.height }; - }), - DROP_ZONES - ); - if (rects.length === 0) return -1; - - const origin = await driver.frameOrigin(); - const scale = await frameScale(); - const pointerY = pointer.y; - + const spans = await mappedZoneSpans(); let best = -1; let bestDistance = Number.POSITIVE_INFINITY; - rects.forEach((rect, index) => { - // Mapped by the shared helper rather than multiplied out here. Written - // inline this is two numbers scaled and added, which is exactly the - // shape no import scan can tell from ordinary arithmetic — so it is the - // one that drifts silently when the mapping is corrected. - const centre = mapFramePointToHost( - { x: 0, y: rect.y + rect.height / 2 }, - origin, - scale - ).y; - const distance = Math.abs(pointerY - centre); + spans.forEach((span, index) => { + const distance = Math.abs(pointer.y - span.centre); if (distance < bestDistance) { bestDistance = distance; best = index;