Skip to content

Commit fc70898

Browse files
authored
fix: Preserve whitespace between sibling elements (#5718)
Treat whitespace-only text nodes between element siblings as part of the previous element instead of creating a separate text node. generateFragmentFromHtml now detects when a text node consisting only of space appears between two elements and appends that space to the previous instance's last text child (if present), avoiding creation of a standalone text child. Added a regression test in plugin-html.test.tsx to verify pasted HTML like `<span>✓</span> <span>text</span>` preserves the space and results in two span instances (no separate text node), ensuring text-content control remains correct in the settings panel.
1 parent 13a7c66 commit fc70898

4 files changed

Lines changed: 93 additions & 4 deletions

File tree

apps/builder/app/shared/copy-paste/plugin-html.test.tsx

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,59 @@ test("ignore html without any tags", async () => {
6363
expect(await html.onPaste?.(`It works`)).toEqual(false);
6464
expect($instances.get()).toEqual(data.instances);
6565
});
66+
67+
test("skip whitespace-only text nodes between element siblings", async () => {
68+
const data = renderData(
69+
<ws.element ws:tag="body" ws:id="bodyId">
70+
<ws.element ws:tag="div" ws:id="divId"></ws.element>
71+
</ws.element>
72+
);
73+
$project.set({ id: "" } as Project);
74+
$instances.set(data.instances);
75+
$pages.set(
76+
createDefaultPages({ rootInstanceId: "bodyId", homePageId: "pageId" })
77+
);
78+
$awareness.set({ pageId: "pageId", instanceSelector: ["divId", "bodyId"] });
79+
80+
// Regression test: whitespace between elements should not create separate text nodes
81+
// but the space should be preserved as part of one of the adjacent span instances
82+
// This ensures the second span gets text-content control in settings panel
83+
expect(
84+
await html.onPaste?.(`<div><span>✓</span> <span>text</span></div>`)
85+
).toEqual(true);
86+
87+
const instances = Array.from($instances.get().values()).filter(
88+
(i) => i.id !== "bodyId" && i.id !== "divId"
89+
);
90+
91+
const divs = instances.filter((i) => i.tag === "div");
92+
expect(divs.length).toBeGreaterThan(0);
93+
94+
const pastedDiv = divs[0];
95+
expect(pastedDiv?.children).toHaveLength(2);
96+
97+
// Both children should be element ids (the two spans), not a text node for the space
98+
expect(pastedDiv?.children[0].type).toBe("id");
99+
expect(pastedDiv?.children[1].type).toBe("id");
100+
expect(pastedDiv?.children.some((c) => c.type === "text")).toBe(false);
101+
102+
// Verify the space is preserved in one of the spans' text content
103+
const child0 = pastedDiv?.children[0];
104+
const child1 = pastedDiv?.children[1];
105+
const span1Id = child0?.type === "id" ? child0.value : undefined;
106+
const span2Id = child1?.type === "id" ? child1.value : undefined;
107+
108+
const span1 = span1Id ? $instances.get().get(span1Id) : undefined;
109+
const span2 = span2Id ? $instances.get().get(span2Id) : undefined;
110+
111+
const span1Text =
112+
span1?.children[0]?.type === "text" ? span1.children[0].value : "";
113+
const span2Text =
114+
span2?.children[0]?.type === "text" ? span2.children[0].value : "";
115+
116+
// Space should be preserved as part of one of the spans, not lost
117+
const combinedText = span1Text + span2Text;
118+
expect(combinedText).toContain("✓");
119+
expect(combinedText).toContain("text");
120+
expect(combinedText).toContain(" ");
121+
});

apps/builder/app/shared/copy-paste/plugin-markdown.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ test("preserve spaces between strong and em", () => {
196196
renderTemplate(
197197
<>
198198
<ws.element ws:tag="p">
199-
<ws.element ws:tag="strong">One</ws.element>{" "}
199+
<ws.element ws:tag="strong">{"One "}</ws.element>
200200
<ws.element ws:tag="em">two</ws.element>
201201
{" text"}
202202
</ws.element>

apps/builder/app/shared/html.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ test("collapse any spacing characters inside rich text", () => {
219219
).toEqual(
220220
renderTemplate(
221221
<ws.element ws:tag="div">
222-
<ws.element ws:tag="i">line</ws.element>{" "}
222+
<ws.element ws:tag="i">{"line "}</ws.element>
223223
<ws.element ws:tag="b">another line</ws.element> text
224224
</ws.element>
225225
)

apps/builder/app/shared/html.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -887,13 +887,16 @@ export const generateFragmentFromHtml = (
887887
}
888888
}
889889
}
890+
let spaceAttachedToPrev = false;
890891
for (let index = 0; index < node.childNodes.length; index += 1) {
891892
const childNode = node.childNodes[index];
892893
if (defaultTreeAdapter.isElementNode(childNode)) {
893894
const lastChild = instance.children.at(-1);
894895
const nextPreserveLeadingSpace =
896+
!spaceAttachedToPrev &&
895897
instance.children.length > 0 &&
896898
!(lastChild?.type === "text" && lastChild.value.endsWith(" "));
899+
spaceAttachedToPrev = false;
897900
const child = convertElementToInstance(childNode, {
898901
preserveLeadingSpace: nextPreserveLeadingSpace,
899902
});
@@ -902,12 +905,42 @@ export const generateFragmentFromHtml = (
902905
}
903906
}
904907
if (defaultTreeAdapter.isTextNode(childNode)) {
905-
// trim spaces around rich text
906-
// do not for code
908+
// trim spaces around rich text, do not for code
907909
if (spaceRegex.test(childNode.value) && node.tagName !== "code") {
910+
// Skip whitespace at start or end of parent
908911
if (index === 0 || index === node.childNodes.length - 1) {
909912
continue;
910913
}
914+
const prevChild = node.childNodes[index - 1];
915+
const nextChild = node.childNodes[index + 1];
916+
const prevIsElement = defaultTreeAdapter.isElementNode(prevChild);
917+
const nextIsElement = defaultTreeAdapter.isElementNode(nextChild);
918+
919+
// In rich-text contexts, attach whitespace between two sibling elements
920+
// to the previous element instead of creating a separate text node.
921+
// This avoids a standalone space child that would prevent the next element
922+
// from being recognized as the last child for text-content control.
923+
if (
924+
prevIsElement &&
925+
nextIsElement &&
926+
!hasNonRichTextContent &&
927+
instance.children.length > 0
928+
) {
929+
const lastChild = instance.children.at(-1);
930+
if (lastChild?.type === "id") {
931+
const prevInstanceId = lastChild.value;
932+
const prevInstance = instances.get(prevInstanceId);
933+
if (prevInstance && prevInstance.children.length > 0) {
934+
const prevLastChild = prevInstance.children.at(-1);
935+
if (prevLastChild?.type === "text") {
936+
prevLastChild.value += " ";
937+
spaceAttachedToPrev = true;
938+
continue;
939+
}
940+
}
941+
}
942+
continue;
943+
}
911944
}
912945
let child: Instance["children"][number] = {
913946
type: "text",

0 commit comments

Comments
 (0)