Skip to content

Commit bd77a25

Browse files
authored
refactor: detect empty tags and components (#5132)
Here replaced "embed" and "control" components with content model. Many components now detect no content based on html content model like with img or input tags. In other cases component meta can explicitly specify empty array to not accept anything. ``` meta = { contentModel: { category: 'instance', children: [] } } ```
1 parent 5287e96 commit bd77a25

28 files changed

+287
-231
lines changed

apps/builder/app/builder/features/ai/apply-operations.ts

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ import { serverSyncStore } from "~/shared/sync";
66
import { isBaseBreakpoint } from "~/shared/breakpoints";
77
import {
88
deleteInstanceMutable,
9+
findClosestInsertable,
910
insertWebstudioFragmentAt,
1011
updateWebstudioData,
1112
type Insertable,
1213
} from "~/shared/instance-utils";
1314
import {
1415
$breakpoints,
1516
$instances,
17+
$props,
1618
$registeredComponentMetas,
1719
$selectedInstanceSelector,
1820
$styleSourceSelections,
@@ -22,6 +24,7 @@ import {
2224
import type { InstanceSelector } from "~/shared/tree-utils";
2325
import { $selectedInstance, getInstancePath } from "~/shared/awareness";
2426
import { isInstanceDetachable } from "~/shared/matcher";
27+
import { isRichTextTree } from "~/shared/content-model";
2528

2629
export const applyOperations = (operations: operations.WsOperations) => {
2730
for (const operation of operations) {
@@ -47,7 +50,7 @@ const insertTemplateByOp = (
4750
operation: operations.generateInsertTemplateWsOperation
4851
) => {
4952
const metas = $registeredComponentMetas.get();
50-
const templateData = generateDataFromEmbedTemplate(operation.template, metas);
53+
const fragment = generateDataFromEmbedTemplate(operation.template, metas);
5154

5255
// @todo Find a way to avoid the workaround below, peharps improving the prompt.
5356
// Occasionally the LLM picks a component name or the entire data-ws-id attribute as the insertion point.
@@ -63,26 +66,18 @@ const insertTemplateByOp = (
6366
}
6467
}
6568

66-
const rootInstanceIds = templateData.children
69+
const rootInstanceIds = fragment.children
6770
.filter((child) => child.type === "id")
6871
.map((child) => child.value);
6972

7073
const instanceSelector = computeSelectorForInstanceId(operation.addTo);
7174
if (instanceSelector) {
72-
const currentInstance = $instances.get().get(instanceSelector[0]);
73-
// Only container components are allowed to have child elements.
74-
if (
75-
currentInstance &&
76-
metas.get(currentInstance.component)?.type !== "container"
77-
) {
78-
return;
79-
}
80-
81-
const dropTarget: Insertable = {
75+
let insertable: Insertable = {
8276
parentSelector: instanceSelector,
8377
position: operation.addAtIndex + 1,
8478
};
85-
insertWebstudioFragmentAt(templateData, dropTarget);
79+
insertable = findClosestInsertable(fragment, insertable) ?? insertable;
80+
insertWebstudioFragmentAt(fragment, insertable);
8681
return rootInstanceIds;
8782
}
8883
};
@@ -214,39 +209,47 @@ const computeSelectorForInstanceId = (instanceId: Instance["id"]) => {
214209
};
215210

216211
export const patchTextInstance = (textInstance: copywriter.TextInstance) => {
217-
serverSyncStore.createTransaction([$instances], (instances) => {
218-
const currentInstance = instances.get(textInstance.instanceId);
219-
220-
if (currentInstance === undefined) {
221-
return;
222-
}
223-
224-
const meta = $registeredComponentMetas.get().get(currentInstance.component);
212+
serverSyncStore.createTransaction(
213+
[$instances, $props],
214+
(instances, props) => {
215+
const currentInstance = instances.get(textInstance.instanceId);
225216

226-
// Only container components are allowed to have child elements.
227-
if (meta?.type !== "container") {
228-
return;
229-
}
217+
if (currentInstance === undefined) {
218+
return;
219+
}
230220

231-
if (currentInstance.children.length === 0) {
232-
currentInstance.children = [{ type: "text", value: textInstance.text }];
233-
return;
234-
}
221+
const canBeEdited = isRichTextTree({
222+
instanceId: textInstance.instanceId,
223+
instances,
224+
props,
225+
metas: $registeredComponentMetas.get(),
226+
});
227+
if (!canBeEdited) {
228+
return;
229+
}
235230

236-
// Instances can have a number of text child nodes without interleaving components.
237-
// When this is the case we treat the child nodes as a single text node,
238-
// otherwise the AI would generate children.length chunks of separate text.
239-
// We can identify this case of "joint" text instances when the index is -1.
240-
const replaceAll = textInstance.index === -1;
241-
if (replaceAll) {
242-
if (currentInstance.children.every((child) => child.type === "text")) {
231+
if (currentInstance.children.length === 0) {
243232
currentInstance.children = [{ type: "text", value: textInstance.text }];
233+
return;
244234
}
245-
return;
246-
}
247235

248-
if (currentInstance.children[textInstance.index].type === "text") {
249-
currentInstance.children[textInstance.index].value = textInstance.text;
236+
// Instances can have a number of text child nodes without interleaving components.
237+
// When this is the case we treat the child nodes as a single text node,
238+
// otherwise the AI would generate children.length chunks of separate text.
239+
// We can identify this case of "joint" text instances when the index is -1.
240+
const replaceAll = textInstance.index === -1;
241+
if (replaceAll) {
242+
if (currentInstance.children.every((child) => child.type === "text")) {
243+
currentInstance.children = [
244+
{ type: "text", value: textInstance.text },
245+
];
246+
}
247+
return;
248+
}
249+
250+
if (currentInstance.children[textInstance.index].type === "text") {
251+
currentInstance.children[textInstance.index].value = textInstance.text;
252+
}
250253
}
251-
});
254+
);
252255
};

apps/builder/app/builder/features/navigator/navigator-tree.tsx

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,9 @@ import {
6666
getInstanceKey,
6767
selectInstance,
6868
} from "~/shared/awareness";
69-
import { findClosestContainer, isTreeMatching } from "~/shared/matcher";
69+
import { isTreeMatching } from "~/shared/matcher";
7070
import {
71+
findClosestContainer,
7172
isRichTextContent,
7273
isTreeSatisfyingContentModel,
7374
} from "~/shared/content-model";
@@ -550,19 +551,17 @@ const canDrop = (
550551
}
551552
}
552553
// prevent dropping into non-container instances
553-
const closestContainerIndex = findClosestContainer({
554+
const containerSelector = findClosestContainer({
554555
metas,
556+
props,
555557
instances,
556558
instanceSelector: dropSelector,
557559
});
558-
if (closestContainerIndex !== 0) {
560+
if (dropSelector.length !== containerSelector.length) {
559561
return false;
560562
}
561563
// make sure dragging tree can be put inside of drop instance
562-
const containerInstanceSelector = [
563-
dragSelector[0],
564-
...dropSelector.slice(closestContainerIndex),
565-
];
564+
const containerInstanceSelector = [dragSelector[0], ...dropSelector];
566565
let matches = isTreeMatching({
567566
instances,
568567
metas,

apps/builder/app/builder/features/settings-panel/props-section/use-props-logic.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
11
import { nanoid } from "nanoid";
2+
import { computed } from "nanostores";
23
import { useStore } from "@nanostores/react";
34
import type { PropMeta, Instance, Prop } from "@webstudio-is/sdk";
4-
import { collectionComponent, descendantComponent } from "@webstudio-is/sdk";
5+
import { descendantComponent } from "@webstudio-is/sdk";
56
import { showAttribute, textContentAttribute } from "@webstudio-is/react-sdk";
6-
import { showAttributeMeta, type PropValue } from "../shared";
77
import {
8+
$instances,
89
$isContentMode,
10+
$props,
911
$registeredComponentMetas,
1012
$registeredComponentPropsMetas,
1113
} from "~/shared/nano-states";
14+
import { isRichText } from "~/shared/content-model";
15+
import { $selectedInstancePath } from "~/shared/awareness";
16+
import { showAttributeMeta, type PropValue } from "../shared";
1217

1318
type PropOrName = { prop?: Prop; propName: string };
19+
1420
export type PropAndMeta = {
1521
prop?: Prop;
1622
propName: string;
@@ -132,6 +138,22 @@ const getAndDelete = <Value>(map: Map<string, Value>, key: string) => {
132138
return value;
133139
};
134140

141+
const $canHaveTextContent = computed(
142+
[$instances, $props, $registeredComponentMetas, $selectedInstancePath],
143+
(instances, props, metas, instancePath) => {
144+
if (instancePath === undefined) {
145+
return false;
146+
}
147+
const [{ instanceSelector }] = instancePath;
148+
return isRichText({
149+
instances,
150+
props,
151+
metas,
152+
instanceSelector,
153+
});
154+
}
155+
);
156+
135157
/** usePropsLogic expects that key={instanceId} is used on the ancestor component */
136158
export const usePropsLogic = ({
137159
instance,
@@ -160,9 +182,6 @@ export const usePropsLogic = ({
160182
return propsWhiteList.includes(propName);
161183
};
162184

163-
const instanceMeta = useStore($registeredComponentMetas).get(
164-
instance.component
165-
);
166185
const meta = useStore($registeredComponentPropsMetas).get(
167186
instance.component
168187
) ?? {
@@ -192,9 +211,7 @@ export const usePropsLogic = ({
192211
});
193212
}
194213

195-
const canHaveTextContent =
196-
instanceMeta?.type === "container" &&
197-
instance.component !== collectionComponent;
214+
const canHaveTextContent = useStore($canHaveTextContent);
198215

199216
const hasNoChildren = instance.children.length === 0;
200217
const hasOnlyTextChild =

apps/builder/app/canvas/features/text-editor/text-editor.stories.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,8 @@ export const CursorPositioningUpDown: StoryFn<typeof TextEditor> = () => {
244244

245245
$registeredComponentMetas.set(
246246
new Map([
247-
["Box", { type: "container", icon: "icon" }],
248-
["Bold", { type: "container", icon: "icon" }],
247+
["Box", { icon: "icon" }],
248+
["Bold", { icon: "icon" }],
249249
])
250250
);
251251

apps/builder/app/canvas/shared/use-drag-drop.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ import {
3030
areInstanceSelectorsEqual,
3131
} from "~/shared/tree-utils";
3232
import {
33-
findClosestContainer,
3433
findClosestInstanceMatchingFragment,
3534
isTreeMatching,
3635
} from "~/shared/matcher";
3736
import {
37+
findClosestContainer,
3838
findClosestRichText,
3939
isTreeSatisfyingContentModel,
4040
} from "~/shared/content-model";
@@ -74,15 +74,13 @@ const findClosestDroppableInstanceSelector = (
7474
const metas = $registeredComponentMetas.get();
7575

7676
// prevent dropping anything into non containers like image
77-
let droppableIndex = findClosestContainer({
77+
instanceSelector = findClosestContainer({
7878
metas,
79+
props,
7980
instances,
8081
instanceSelector,
8182
});
82-
if (droppableIndex === -1) {
83-
return;
84-
}
85-
instanceSelector = instanceSelector.slice(droppableIndex);
83+
let droppableIndex = -1;
8684
if (dragPayload?.type === "insert") {
8785
const fragment = getComponentTemplateData(dragPayload.dragComponent);
8886
if (fragment) {

apps/builder/app/shared/content-model.test.tsx

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { coreMetas } from "@webstudio-is/sdk";
33
import * as baseComponentMetas from "@webstudio-is/sdk-components-react/metas";
44
import { $, expression, renderData, ws } from "@webstudio-is/template";
55
import {
6+
findClosestContainer,
67
findClosestNonTextualContainer,
78
findClosestRichText,
89
isRichTextTree,
@@ -835,8 +836,68 @@ describe("rich text tree", () => {
835836
});
836837
});
837838

838-
describe("closest non textual container", () => {
839+
describe("closest container", () => {
839840
test("skips non-container instances", () => {
841+
expect(
842+
findClosestContainer({
843+
...renderData(
844+
<$.Body ws:id="bodyId">
845+
<$.Box ws:id="boxId">
846+
<$.Image ws:id="imageId" />
847+
</$.Box>
848+
</$.Body>
849+
),
850+
metas: defaultMetas,
851+
instanceSelector: ["imageId", "boxId", "bodyId"],
852+
})
853+
).toEqual(["boxId", "bodyId"]);
854+
});
855+
856+
test("allow containers with text", () => {
857+
expect(
858+
findClosestContainer({
859+
...renderData(
860+
<$.Body ws:id="bodyId">
861+
<$.Box ws:id="boxId">
862+
<$.Box ws:id="box-with-text">text</$.Box>
863+
</$.Box>
864+
</$.Body>
865+
),
866+
metas: defaultMetas,
867+
instanceSelector: ["box-with-text", "boxId", "bodyId"],
868+
})
869+
).toEqual(["box-with-text", "boxId", "bodyId"]);
870+
});
871+
872+
test("allow containers with expression", () => {
873+
expect(
874+
findClosestContainer({
875+
...renderData(
876+
<$.Body ws:id="bodyId">
877+
<$.Box ws:id="boxId">
878+
<$.Box ws:id="box-with-expr">{expression`1 + 1`}</$.Box>
879+
</$.Box>
880+
</$.Body>
881+
),
882+
metas: defaultMetas,
883+
instanceSelector: ["box-with-expr", "boxId", "bodyId"],
884+
})
885+
).toEqual(["box-with-expr", "boxId", "bodyId"]);
886+
});
887+
888+
test("allow root with text", () => {
889+
expect(
890+
findClosestContainer({
891+
...renderData(<$.Body ws:id="bodyId">text</$.Body>),
892+
metas: defaultMetas,
893+
instanceSelector: ["bodyId"],
894+
})
895+
).toEqual(["bodyId"]);
896+
});
897+
});
898+
899+
describe("closest non textual container", () => {
900+
test("skips image tag", () => {
840901
expect(
841902
findClosestNonTextualContainer({
842903
...renderData(
@@ -852,6 +913,22 @@ describe("closest non textual container", () => {
852913
).toEqual(["boxId", "bodyId"]);
853914
});
854915

916+
test("skips CodeText component", () => {
917+
expect(
918+
findClosestNonTextualContainer({
919+
...renderData(
920+
<$.Body ws:id="bodyId">
921+
<$.Box ws:id="boxId">
922+
<$.CodeText ws:id="codeId" />
923+
</$.Box>
924+
</$.Body>
925+
),
926+
metas: defaultMetas,
927+
instanceSelector: ["codeId", "boxId", "bodyId"],
928+
})
929+
).toEqual(["boxId", "bodyId"]);
930+
});
931+
855932
test("skips containers with text", () => {
856933
expect(
857934
findClosestNonTextualContainer({

0 commit comments

Comments
 (0)