Skip to content

Commit

Permalink
fix: stop duplicating required children on reparent (#4537)
Browse files Browse the repository at this point in the history
Turns our delete does not clear instances when detachable check fails.
Good news: new child constraints were added only to tabs. Bad news: I
think deleting pages kept instances in the build, oops.

Fixed reparent and added checks for delete only where necessary.

How to test

- insert tabs content
- delete second trigger and content
- reposition trigger
- it is duplicated
  • Loading branch information
TrySound authored Dec 8, 2024
1 parent fb2c6b5 commit dad162c
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 30 deletions.
8 changes: 8 additions & 0 deletions apps/builder/app/builder/features/ai/apply-operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { isBaseBreakpoint } from "~/shared/breakpoints";
import {
deleteInstanceMutable,
insertTemplateData,
isInstanceDetachable,
updateWebstudioData,
} from "~/shared/instance-utils";
import {
Expand Down Expand Up @@ -101,7 +102,14 @@ const deleteInstanceByOp = (
) => {
const instanceSelector = computeSelectorForInstanceId(operation.wsId);
if (instanceSelector) {
// @todo tell user they can't delete root
if (instanceSelector.length === 1) {
return;
}
updateWebstudioData((data) => {
if (isInstanceDetachable(data.instances, instanceSelector) === false) {
return;
}
deleteInstanceMutable(data, instanceSelector);
});
}
Expand Down
11 changes: 9 additions & 2 deletions apps/builder/app/builder/shared/commands.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { blockTemplateComponent } from "@webstudio-is/react-sdk";
import { toast } from "@webstudio-is/design-system";
import { createCommandsEmitter, type Command } from "~/shared/commands-emitter";
import {
$editingItemSelector,
Expand All @@ -19,6 +21,7 @@ import {
extractWebstudioFragment,
insertWebstudioFragmentCopy,
updateWebstudioData,
isInstanceDetachable,
} from "~/shared/instance-utils";
import type { InstanceSelector } from "~/shared/tree-utils";
import { serverSyncStore } from "~/shared/sync";
Expand All @@ -32,7 +35,6 @@ import { selectInstance } from "~/shared/awareness";
import { openCommandPanel } from "../features/command-panel";
import { builderApi } from "~/shared/builder-api";
import { findBlockSelector } from "../features/workspace/canvas-tools/outline/block-instance-outline";
import { blockTemplateComponent } from "@webstudio-is/react-sdk";

const makeBreakpointCommand = <CommandName extends string>(
name: CommandName,
Expand Down Expand Up @@ -65,8 +67,13 @@ const deleteSelectedInstance = () => {
if (selectedInstanceSelector.length === 1) {
return;
}

const instances = $instances.get();
if (isInstanceDetachable(instances, selectedInstanceSelector) === false) {
toast.error(
"This instance can not be moved outside of its parent component."
);
return false;
}

if ($isContentMode.get()) {
// In content mode we are allowing to delete childen of the editable block
Expand Down
49 changes: 31 additions & 18 deletions apps/builder/app/shared/instance-utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { $, ws, renderJsx, ExpressionValue } from "@webstudio-is/sdk/testing";
import { parseCss } from "@webstudio-is/css-data";
import { coreMetas } from "@webstudio-is/react-sdk";
import * as defaultMetas from "@webstudio-is/sdk-components-react/metas";
import * as radixMetas from "@webstudio-is/sdk-components-react-radix/metas";
import type {
Asset,
Breakpoint,
Expand Down Expand Up @@ -844,6 +845,36 @@ describe("reparent instance", () => {
])
);
});

test("reparent required child", () => {
$instances.set(
renderJsx(
<$.Body ws:id="body">
<$.Tooltip ws:id="tooltip">
<$.TooltipTrigger ws:id="trigger"></$.TooltipTrigger>
<$.TooltipContent ws:id="content"></$.TooltipContent>
</$.Tooltip>
</$.Body>
).instances
);
$registeredComponentMetas.set(
new Map(Object.entries({ ...defaultMetas, ...radixMetas }))
);
reparentInstance(["trigger", "tooltip", "body"], {
parentSelector: ["tooltip", "body"],
position: "end",
});
expect($instances.get()).toEqual(
renderJsx(
<$.Body ws:id="body">
<$.Tooltip ws:id="tooltip">
<$.TooltipContent ws:id="content"></$.TooltipContent>
<$.TooltipTrigger ws:id={expect.any(String)}></$.TooltipTrigger>
</$.Tooltip>
</$.Body>
).instances
);
});
});

const getWebstudioDataStub = (
Expand Down Expand Up @@ -888,24 +919,6 @@ describe("delete instance", () => {
);
});

test("prevent deleting root instance", () => {
// body
// box1
const instances = new Map([
createInstancePair("body", "Body", [{ type: "id", value: "box1" }]),
createInstancePair("box1", "Box", []),
]);
$registeredComponentMetas.set(createFakeComponentMetas({}));
const data = getWebstudioDataStub({ instances });
deleteInstanceMutable(data, ["body"]);
expect(data.instances).toEqual(
new Map([
createInstancePair("body", "Body", [{ type: "id", value: "box1" }]),
createInstancePair("box1", "Box", []),
])
);
});

test("delete instance from collection item", () => {
// body
// list
Expand Down
10 changes: 0 additions & 10 deletions apps/builder/app/shared/instance-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -496,16 +496,6 @@ export const deleteInstanceMutable = (
data: WebstudioData,
instanceSelector: InstanceSelector
) => {
// @todo tell user they can't delete root
if (instanceSelector.length === 1) {
return false;
}
if (isInstanceDetachable(data.instances, instanceSelector) === false) {
toast.error(
"This instance can not be moved outside of its parent component."
);
return false;
}
const {
instances,
props,
Expand Down

0 comments on commit dad162c

Please sign in to comment.