Skip to content

Commit

Permalink
fix: navigator dnd bugs (#4546)
Browse files Browse the repository at this point in the history
- fixed dropping into non-container instances (like image)
- fixed dropping instances from another block
  • Loading branch information
TrySound authored Dec 10, 2024
1 parent 1d2138a commit 8238438
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 15 deletions.
27 changes: 21 additions & 6 deletions apps/builder/app/builder/features/navigator/navigator-tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import {
getInstanceKey,
selectInstance,
} from "~/shared/awareness";
import { isTreeMatching } from "~/shared/matcher";
import { findClosestContainer, isTreeMatching } from "~/shared/matcher";

type TreeItemAncestor =
| undefined
Expand Down Expand Up @@ -465,20 +465,35 @@ const canDrop = (
) => {
const dropSelector = dropTarget.itemSelector;
const instances = $instances.get();

// in content mode allow drop only inside of block
const metas = $registeredComponentMetas.get();
// in content mode allow drop only within same block
if ($isContentMode.get()) {
const parentInstance = instances.get(dropSelector[0]);
if (parentInstance?.component !== blockComponent) {
return false;
}
// parent of dragging item should be the same as drop target
if (dropSelector[0] !== dragSelector[1]) {
return false;
}
}
// prevent dropping into non-container instances
const closestContainerIndex = findClosestContainer({
metas,
instances,
instanceSelector: dropSelector,
});
if (closestContainerIndex !== 0) {
return false;
}

return isTreeMatching({
instances,
metas: $registeredComponentMetas.get(),
metas,
// make sure dragging tree can be put inside of drop instance
instanceSelector: [dragSelector[0], ...dropSelector],
instanceSelector: [
dragSelector[0],
...dropSelector.slice(closestContainerIndex),
],
});
};

Expand Down
4 changes: 2 additions & 2 deletions apps/builder/app/shared/instance-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import { setDifference, setUnion } from "./shim";
import { breakCyclesMutable, findCycles } from "@webstudio-is/project-build";
import { $awareness, $selectedPage, selectInstance } from "./awareness";
import {
findClosestContainer,
findClosestNonTextualContainer,
findClosestInstanceMatchingFragment,
isTreeMatching,
} from "./matcher";
Expand Down Expand Up @@ -1436,7 +1436,7 @@ export const findClosestInsertable = (
}
const metas = $registeredComponentMetas.get();
const instances = $instances.get();
const closestContainerIndex = findClosestContainer({
const closestContainerIndex = findClosestNonTextualContainer({
metas,
instances,
instanceSelector,
Expand Down
71 changes: 67 additions & 4 deletions apps/builder/app/shared/matcher.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import * as baseMetas from "@webstudio-is/sdk-components-react/metas";
import type { WsComponentMeta } from "@webstudio-is/react-sdk";
import type { Matcher, WebstudioFragment } from "@webstudio-is/sdk";
import {
findClosestContainer,
findClosestNonTextualContainer,
findClosestInstanceMatchingFragment,
isInstanceMatching,
isTreeMatching,
findClosestContainer,
} from "./matcher";

const metas = new Map(Object.entries({ ...coreMetas, ...baseMetas }));
Expand Down Expand Up @@ -885,7 +886,7 @@ describe("find closest container", () => {
).toEqual(1);
});

test("skips containers with text", () => {
test("allow containers with text", () => {
expect(
findClosestContainer({
...renderJsx(
Expand All @@ -898,12 +899,74 @@ describe("find closest container", () => {
metas,
instanceSelector: ["box-with-text", "box", "body"],
})
).toEqual(0);
});

test("allow containers with expression", () => {
expect(
findClosestContainer({
...renderJsx(
<$.Body ws:id="body">
<$.Box ws:id="box">
<$.Box ws:id="box-with-expr">
{new ExpressionValue("1 + 1")}
</$.Box>
</$.Box>
</$.Body>
),
metas,
instanceSelector: ["box-with-expr", "box", "body"],
})
).toEqual(0);
});

test("allow root with text", () => {
expect(
findClosestContainer({
...renderJsx(<$.Body ws:id="body">text</$.Body>),
metas,
instanceSelector: ["body"],
})
).toEqual(0);
});
});

describe("find closest non textual container", () => {
test("skips non-container instances", () => {
expect(
findClosestNonTextualContainer({
...renderJsx(
<$.Body ws:id="body">
<$.Box ws:id="box">
<$.Image ws:id="image" />
</$.Box>
</$.Body>
),
metas,
instanceSelector: ["image", "box", "body"],
})
).toEqual(1);
});

test("skips containers with text", () => {
expect(
findClosestNonTextualContainer({
...renderJsx(
<$.Body ws:id="body">
<$.Box ws:id="box">
<$.Box ws:id="box-with-text">text</$.Box>
</$.Box>
</$.Body>
),
metas,
instanceSelector: ["box-with-text", "box", "body"],
})
).toEqual(1);
});

test("skips containers with expression", () => {
expect(
findClosestContainer({
findClosestNonTextualContainer({
...renderJsx(
<$.Body ws:id="body">
<$.Box ws:id="box">
Expand All @@ -921,7 +984,7 @@ describe("find closest container", () => {

test("allow root with text", () => {
expect(
findClosestContainer({
findClosestNonTextualContainer({
...renderJsx(<$.Body ws:id="body">text</$.Body>),
metas,
instanceSelector: ["body"],
Expand Down
29 changes: 27 additions & 2 deletions apps/builder/app/shared/matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,31 @@ export const findClosestContainer = ({
continue;
}
const meta = metas.get(instance.component);
if (meta === undefined) {
if (meta?.type === "container") {
return index;
}
}
return -1;
};

export const findClosestNonTextualContainer = ({
metas,
instances,
instanceSelector,
}: {
metas: Map<string, WsComponentMeta>;
instances: Instances;
instanceSelector: InstanceSelector;
}) => {
// page root with text can be used as container
if (instanceSelector.length === 1) {
return 0;
}
for (let index = 0; index < instanceSelector.length; index += 1) {
const instanceId = instanceSelector[index];
const instance = instances.get(instanceId);
// collection item can be undefined
if (instance === undefined) {
continue;
}
let hasText = false;
Expand All @@ -313,7 +337,8 @@ export const findClosestContainer = ({
if (hasText) {
continue;
}
if (meta.type === "container") {
const meta = metas.get(instance.component);
if (meta?.type === "container") {
return index;
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-sdk/src/core-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const blockMeta: WsComponentMeta = {
icon: EditIcon,
constraints: {
relation: "ancestor",
component: { $neq: collectionComponent },
component: { $nin: [collectionComponent, blockComponent] },
},
stylable: false,
template: [
Expand Down

0 comments on commit 8238438

Please sign in to comment.