feat(plugin-page-builder): tell an author why a drop was refused

.changeset/drop-refusal-feedback.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 tells an author WHY a drop was refused instead of silently doing nothing. Drop planning returns a discriminated outcome — action, refused with a reason, unchanged, or unresolved — and the drag overlay shows the reason while the drag is still in the air.

e2e/tests/canvas/acceptance.spec.ts

@@ -41,6 +41,7 @@ import { mapFramePointToHost } from "./coordinate-mapping";
 import {
   CanvasCapabilityError,
   dragPointerTo,
+  dragSourceUntilTarget,
   dragToZoneEdge,
   dragUntilInsideZone,
   dragUntilTarget,
@@ -111,6 +112,24 @@ async function dragOntoZone(driver: CanvasDriver): Promise<number> {
   return active;
 }
 
+/**
+ * The two sources the invalid-target point needs, each carried onto a zone.
+ *
+ * A pair, because either drag alone is satisfied by a canvas that treats every target the same
+ * way. Both go through the shared transport so the gesture is identical and only the block differs.
+ */
+async function dragRestrictedOntoZone(driver: CanvasDriver): Promise<number> {
+  return dragSourceUntilTarget(
+    driver,
+    await driver.restrictedDragSourceCentre()
+  );
+}
+
+/** A drag an ordinary container will take. */
+async function dragAcceptedOntoZone(driver: CanvasDriver): Promise<number> {
+  return dragSourceUntilTarget(driver, await driver.acceptedDragSourceCentre());
+}
+
 /**
  * What a running drag looks like from outside, whichever engine is driving it.
  *
@@ -687,50 +706,39 @@ test.describe("a canvas any Nextly editor could ship", () => {
   test("shows an explicit state over an invalid target", async ({
     request,
   }) => {
-    note(
-      PLAN_POINT.invalidTargetVisible,
-      "B-7",
-      "this canvas shows nothing over an illegal target"
-    );
+    note(PLAN_POINT.invalidTargetVisible, "B-7");
     await driver.mountTree(await seedPage(request, NESTED_FIXTURE));
-    await dragFromPanel(driver);
-
-    // 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.readsInvalidTarget()).rejects.toThrow(
-      CanvasCapabilityError
-    );
 
-    // Marked only now. Everything above ran unprotected.
-    test.fail(true, "nothing is shown over an illegal target");
-
-    // NOT SEPARATING YET, and the reason is in the product rather than here.
-    // `canDrop` refuses a drop for four reasons, and the only one a panel drag
-    // can reach is `not-allowed-in-slot`, which needs a slot declaring
-    // `allowedBlocks`. Measured against the shipped registry: no block declares
-    // one, so every slot accepts every child and there is no illegal target for
-    // this drag to enter. The pointer therefore rests somewhere legal, and a
-    // canvas that answered `false` here forever would satisfy the assertion the
-    // day the reader starts working.
-    //
-    // Closing this needs a block whose slot restricts its children — a product
-    // decision, not a harness one. Until then the expected failure records a
-    // capability the canvas lacks and NOT a judgement about what it draws over
-    // an illegal target, because it is never over one.
+    // The driver's restricted source is a block restricting its own PARENT, which is what makes an
+    // illegal target reachable at all: no block in the shipped registry declares an `allowedBlocks`
+    // slot, so a slot-side refusal has nothing to refuse and a container-side one has everything.
+    const refused = await dragRestrictedOntoZone(driver);
+    expect(
+      refused,
+      "the refused drag must reach a drop zone before the canvas is read"
+    ).toBeGreaterThanOrEqual(0);
     const explicit = await chrome.readsInvalidTarget();
     await driver.cancel();
 
     // Showing nothing is not a state. The author cannot tell "you may not drop
     // here" from "the drag broke", and both read as an unresponsive editor.
     expect(explicit, "an invalid target must be shown, not implied").toBe(true);
+
+    // The other half, and without it the point is satisfied by a canvas that draws a refusal over
+    // EVERY target. That canvas tells the author nothing — a signal present everywhere carries no
+    // information — and it passes every assertion above.
+    await driver.mountTree(await seedPage(request, NESTED_FIXTURE));
+    const accepted = await dragAcceptedOntoZone(driver);
+    expect(
+      accepted,
+      "the permitted drag must reach a drop zone before the canvas is read"
+    ).toBeGreaterThanOrEqual(0);
+    const overLegal = await chrome.readsInvalidTarget();
+    await driver.cancel();
+    expect(
+      overLegal,
+      "a target that accepts the block must not be shown as invalid"
+    ).toBe(false);
   });
 
   test("autoscrolls toward an edge and stops at the bounds", async ({

e2e/tests/canvas/driver.ts

@@ -69,6 +69,25 @@ export interface CanvasDriver {
    */
   dragSourceCentre(): Promise<Point>;
 
+  /**
+   * Two sources chosen by whether an ordinary container will TAKE them, for the
+   * one acceptance point that needs a refused drag and a permitted one.
+   *
+   * A pair rather than a single "restricted" reader, because either half alone
+   * is satisfied by a canvas that behaves identically over both. Showing a
+   * refusal everywhere and showing it nowhere are different defects and each
+   * passes the other's test; only running the same drag with the two sources
+   * separates them.
+   *
+   * On the driver for the same reason {@link dragSourceCentre} is: WHICH block a
+   * canvas refuses is its own structural rule, and a suite that picked one by
+   * name could not be retargeted by swapping the driver.
+   */
+  restrictedDragSourceCentre(): Promise<Point>;
+
+  /** The permitted half of the pair above. */
+  acceptedDragSourceCentre(): Promise<Point>;
+
   /** A point over the canvas, near its top, in host coordinates. */
   canvasCentre(): Promise<Point>;
 
@@ -1264,6 +1283,26 @@ export async function jitterAcrossEdge(
  * 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.
  */
+/**
+ * Start a panel drag at `source` and carry it until a drop zone is active.
+ *
+ * The whole sequence in one place because it is three steps that only work together, and a
+ * per-suite copy gets the middle one wrong invisibly: computing the delta from the SOURCE point
+ * rather than from where the pointer actually is overshoots by whatever the driver moved to cross
+ * the activation threshold, and the drag still runs and still ends somewhere plausible.
+ *
+ * Returns the zone's ordinal, or -1 when the descent found none. The CALLER asserts on that:
+ * reaching no zone means different things to a test about refusals and one about geometry.
+ */
+export async function dragSourceUntilTarget(
+  driver: CanvasDriver,
+  source: Point
+): Promise<number> {
+  await driver.startDragAt(source);
+  await dragPointerTo(driver, await driver.canvasCentre());
+  return dragUntilTarget(driver);
+}
+
 export async function dragPointerTo(
   driver: CanvasDriver,
   target: Point,

e2e/tests/canvas/invalid-drop-feedback.test.ts

@@ -0,0 +1,139 @@
+/**
+ * A refused drop has to REACH the author, and this is the only check that follows the reason the
+ * whole way: from the drop rules, through the drag handlers, into something on screen.
+ *
+ * The unit suites cover each end of that path and neither covers the middle. `canDrop` returning
+ * `wrong-parent` and `dropRefusalMessage` turning it into a sentence both stay green when the
+ * handler stops calling `setRefusal`, or when the status element stops rendering it — and the
+ * symptom of either is an editor that shows nothing, which is exactly the state this feature
+ * replaced.
+ *
+ * ## The separating property
+ *
+ * "The tree does not change" is NOT it. A canvas that draws nothing satisfies that perfectly, and
+ * that canvas is the one being replaced. What separates a working refusal from a silent one is the
+ * REASON arriving where the author reads it, so that is what is asserted — against a control drag
+ * that must reach a target and say nothing.
+ *
+ * ## Why no special fixture
+ *
+ * The driver's restricted source is a block that restricts its own PARENT, so an ordinary
+ * container refuses it by the shipped registry's own rule. A fixture manufacturing a slot with an
+ * allowlist would be testing a document nobody authors.
+ */
+import { expect, test, type Page } from "@playwright/test";
+
+import { dragSourceUntilTarget } from "./driver";
+import type { CanvasFixture } from "./driver";
+import { FLAT_LIST_FIXTURE, seedPage } from "./fixtures";
+import { createPocDriver } from "./poc-driver";
+
+test.describe.configure({ timeout: 240_000 });
+// Below roughly 1280px the editor drops the canvas preview entirely, and `mountTree` then times
+// out against a canvas that is working.
+test.use({ viewport: { width: 2560, height: 1400 } });
+
+/** The element dnd-kit positions under the cursor; the chip and the refusal are inside it. */
+const DRAG_OVERLAY = ".nx-pb-drag-overlay";
+
+/**
+ * The sentence `wrong-parent` produces, restated rather than imported.
+ *
+ * The plugin does not export `dropRefusalMessage` from any entry, and widening a package's public
+ * surface to let a test read one string is the wrong trade. Restating it errs in the loud
+ * direction: a copy change fails here and is confirmed by whoever made it, whereas the exhaustive
+ * mapping over every reason is the unit suite's job and stays there.
+ */
+const WRONG_PARENT_TEXT = "This block can only go inside certain containers.";
+
+/**
+ * Drag one of the driver's two named sources onto a real drop zone, refusing to return without
+ * one.
+ *
+ * The canvas centre is over dead space as often as not, so reading the overlay straight after
+ * arriving would measure a drag with no target — and "no refusal shown" is then true for a reason
+ * that has nothing to do with the feature.
+ *
+ * WHICH block each source is belongs to the driver, not here: it is the canvas's own structural
+ * rule that decides what a container refuses, and naming a block by label in this file would make
+ * the test unretargetable and duplicate a lookup that already exists. The transport is shared for
+ * the same reason — the two drags must differ only in the block, so any difference the assertions
+ * see is the rule rather than the gesture.
+ */
+async function dragOntoZone(
+  page: Page,
+  fixture: CanvasFixture,
+  pick: "restricted" | "accepted"
+): Promise<void> {
+  const driver = createPocDriver(page);
+  await driver.mountTree(fixture);
+  const source =
+    pick === "restricted"
+      ? await driver.restrictedDragSourceCentre()
+      : await driver.acceptedDragSourceCentre();
+  const active = await dragSourceUntilTarget(driver, source);
+  expect(
+    active,
+    `the ${pick} drag must reach a drop zone before the overlay is read`
+  ).toBeGreaterThanOrEqual(0);
+}
+
+test("tells the author which rule refused the drop", async ({
+  page,
+  request,
+}) => {
+  await dragOntoZone(
+    page,
+    await seedPage(request, FLAT_LIST_FIXTURE),
+    "restricted"
+  );
+
+  const overlay = page.locator(DRAG_OVERLAY);
+  await expect(
+    overlay,
+    "the drag overlay must be on screen for its contents to be read"
+  ).toHaveCount(1);
+  await expect(
+    overlay.locator("[data-refused]"),
+    "the overlay must mark itself refused while a rule is stopping the drop"
+  ).toHaveCount(1);
+  // The sentence, in the live region. Both halves matter and they break independently: the text is
+  // what a sighted author reads, and `role="status"` is what carries it to one who is not looking
+  // at the cursor.
+  //
+  // EXACTLY the sentence. The refusal mark beside it is an SVG, so it contributes no text of its
+  // own, and equality then rejects a region that has accumulated a second reason rather than
+  // replaced the first.
+  await expect(
+    overlay.locator('[role="status"]'),
+    "the refusal must name the rule where the author reads it"
+  ).toHaveText(WRONG_PARENT_TEXT);
+});
+
+test("says nothing over a target that accepts the block", async ({
+  page,
+  request,
+}) => {
+  await dragOntoZone(
+    page,
+    await seedPage(request, FLAT_LIST_FIXTURE),
+    "accepted"
+  );
+
+  // The population first. Silence is the expected result here, and silence is also what a drag
+  // that never started, an overlay that never mounted and a broken selector all produce — so the
+  // overlay being present is what makes the emptiness below evidence rather than absence.
+  const overlay = page.locator(DRAG_OVERLAY);
+  await expect(
+    overlay,
+    "the drag overlay must be on screen for its contents to be read"
+  ).toHaveCount(1);
+  await expect(
+    overlay.locator("[data-refused]"),
+    "the target accepts this block, so nothing may be marked refused"
+  ).toHaveCount(0);
+  await expect(
+    overlay.locator('[role="status"]'),
+    "the live region must stay empty rather than carry a stale reason"
+  ).toHaveText("");
+});