Skip to content

Commit 823fb5f

Browse files
TrySoundistarkov
authored andcommitted
fix: prevent inserting into instance with placeholder (#4901)
Closes #4897 Added "placeholder" field to component meta to signal the instance accepts should not be insertion target. This reverts us to previous behavior even with empty instances. Also slightly refactored placeholder rendering to use attr() instead of extra css variable.
1 parent 6e68cfa commit 823fb5f

File tree

13 files changed

+72
-66
lines changed

13 files changed

+72
-66
lines changed

apps/builder/app/builder/shared/commands.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import {
3636
import { $selectedInstancePath, selectInstance } from "~/shared/awareness";
3737
import { openCommandPanel } from "../features/command-panel";
3838
import { builderApi } from "~/shared/builder-api";
39-
4039
import {
4140
findClosestNonTextualContainer,
4241
isInstanceDetachable,

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ import {
9797
insertListItemAt,
9898
insertTemplateAt,
9999
} from "~/builder/features/workspace/canvas-tools/outline/block-utils";
100-
import { editablePlaceholderComponents } from "~/canvas/shared/styles";
101100

102101
const BindInstanceToNodePlugin = ({
103102
refs,
@@ -1580,6 +1579,7 @@ export const TextEditor = ({
15801579
const handleNext = useEffectEvent(
15811580
(state: EditorState, args: HandleNextParams) => {
15821581
const rootInstanceId = $selectedPage.get()?.rootInstanceId;
1582+
const metas = $registeredComponentMetas.get();
15831583

15841584
if (rootInstanceId === undefined) {
15851585
return;
@@ -1589,7 +1589,7 @@ export const TextEditor = ({
15891589
findAllEditableInstanceSelector(
15901590
[rootInstanceId],
15911591
instances,
1592-
$registeredComponentMetas.get(),
1592+
metas,
15931593
editableInstanceSelectors
15941594
);
15951595

@@ -1638,14 +1638,12 @@ export const TextEditor = ({
16381638
if (instance === undefined) {
16391639
continue;
16401640
}
1641-
1642-
// Components with pseudo-elements (e.g., ::marker) that prevent content from collapsing
1643-
const componentsWithPseudoElementChildren =
1644-
editablePlaceholderComponents;
1641+
const meta = metas.get(instance.component);
16451642

16461643
// opinionated: Non-collapsed elements without children can act as spacers (they have size for some reason).
16471644
if (
1648-
!componentsWithPseudoElementChildren.includes(instance.component) &&
1645+
// Components with pseudo-elements (e.g., ::marker) that prevent content from collapsing
1646+
meta?.placeholder === undefined &&
16491647
instance?.children.length === 0
16501648
) {
16511649
const elt = getElementByInstanceSelector(nextSelector);

apps/builder/app/canvas/features/webstudio-component/webstudio-component.tsx

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ import {
3636
getIndexesWithinAncestors,
3737
type AnyComponent,
3838
textContentAttribute,
39-
editingPlaceholderVariable,
40-
editablePlaceholderVariable,
4139
} from "@webstudio-is/react-sdk";
4240
import { rawTheme } from "@webstudio-is/design-system";
4341
import {
@@ -68,8 +66,10 @@ import {
6866
} from "~/canvas/elements";
6967
import { Block } from "../build-mode/block";
7068
import { BlockTemplate } from "../build-mode/block-template";
71-
import { getInstanceLabel } from "~/shared/instance-utils";
72-
import { editablePlaceholderComponents } from "~/canvas/shared/styles";
69+
import {
70+
editablePlaceholderAttribute,
71+
editingPlaceholderVariable,
72+
} from "~/canvas/shared/styles";
7373

7474
const ContentEditable = ({
7575
placeholder,
@@ -376,19 +376,14 @@ const getEditableComponentPlaceholder = (
376376
metas: Map<string, WsComponentMeta>,
377377
mode: "editing" | "editable"
378378
) => {
379-
if (!editablePlaceholderComponents.includes(instance.component)) {
379+
const meta = metas.get(instance.component);
380+
if (meta?.placeholder === undefined) {
380381
return;
381382
}
382383

383384
const isContentBlockChild =
384385
undefined !== findBlockSelector(instanceSelector, instances);
385386

386-
const meta = metas.get(instance.component);
387-
388-
const label = meta
389-
? getInstanceLabel(instance, meta)
390-
: (instance.label ?? instance.component);
391-
392387
const isParagraph = instance.component === "Paragraph";
393388

394389
if (isParagraph && isContentBlockChild) {
@@ -398,7 +393,7 @@ const getEditableComponentPlaceholder = (
398393
undefined;
399394
}
400395

401-
return label;
396+
return meta.placeholder;
402397
};
403398

404399
export const WebstudioComponentCanvas = forwardRef<
@@ -498,14 +493,6 @@ export const WebstudioComponentCanvas = forwardRef<
498493
Component = BlockTemplate;
499494
}
500495

501-
const placeholder = getEditableComponentPlaceholder(
502-
instance,
503-
instanceSelector,
504-
instances,
505-
metas,
506-
"editable"
507-
);
508-
509496
const mergedProps = mergeProps(restProps, instanceProps, "delete");
510497

511498
const props: {
@@ -514,20 +501,19 @@ export const WebstudioComponentCanvas = forwardRef<
514501
[selectorIdAttribute]: string;
515502
} & Record<string, unknown> = {
516503
...mergedProps,
517-
...(placeholder !== undefined
518-
? {
519-
style: {
520-
...mergedProps.style,
521-
[editablePlaceholderVariable]: `'${placeholder.replaceAll("'", "\\'")}'`,
522-
},
523-
}
524-
: null),
525504
// current props should override bypassed from parent
526505
// important for data-ws-* props
527506
tabIndex: 0,
528507
[selectorIdAttribute]: instanceSelector.join(","),
529508
[componentAttribute]: instance.component,
530509
[idAttribute]: instance.id,
510+
[editablePlaceholderAttribute]: getEditableComponentPlaceholder(
511+
instance,
512+
instanceSelector,
513+
instances,
514+
metas,
515+
"editable"
516+
),
531517
};
532518

533519
// React ignores defaultValue changes after first render.

apps/builder/app/canvas/shared/styles.ts

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,7 @@ import {
1212
createImageValueTransformer,
1313
addFontRules,
1414
} from "@webstudio-is/sdk";
15-
import {
16-
collapsedAttribute,
17-
idAttribute,
18-
editingPlaceholderVariable,
19-
editablePlaceholderVariable,
20-
componentAttribute,
21-
} from "@webstudio-is/react-sdk";
15+
import { collapsedAttribute, idAttribute } from "@webstudio-is/react-sdk";
2216
import {
2317
StyleValue,
2418
type TransformValue,
@@ -68,31 +62,22 @@ export const mountStyles = () => {
6862
helpersSheet.render();
6963
};
7064

71-
/**
72-
* Opinionated list of non collapsible components in the builder
73-
*/
74-
export const editablePlaceholderComponents = [
75-
"Paragraph",
76-
"Heading",
77-
"ListItem",
78-
"Blockquote",
79-
"Link",
80-
];
81-
82-
const editablePlaceholderSelector = editablePlaceholderComponents
83-
.map((component) => `[${componentAttribute}= "${component}"]`)
84-
.join(", ");
65+
export const editablePlaceholderAttribute = "data-ws-editable-placeholder";
66+
// @todo replace with modern typed attr() when supported in all browsers
67+
// see the second edge case
68+
// https://developer.mozilla.org/en-US/docs/Web/CSS/attr#backwards_compatibility
69+
export const editingPlaceholderVariable = "--ws-editing-placeholder";
8570

8671
const helperStylesShared = [
8772
// Display a placeholder text for elements that are editable but currently empty
88-
`:is(${editablePlaceholderSelector}):empty::before {
89-
content: var(${editablePlaceholderVariable}, '\\200B');
73+
`:is([${editablePlaceholderAttribute}]):empty::before {
74+
content: attr(${editablePlaceholderAttribute});
9075
opacity: 0.3;
9176
}
9277
`,
9378

9479
// Display a placeholder text for elements that are editing but empty (Lexical adds p>br children)
95-
`:is(${editablePlaceholderSelector})[contenteditable] > p:only-child:has(br:only-child) {
80+
`:is([${editablePlaceholderAttribute}])[contenteditable] > p:only-child:has(br:only-child) {
9681
position: relative;
9782
display: block;
9883
&:after {

apps/builder/app/shared/instance-utils.test.tsx

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,6 +1388,36 @@ describe("find closest insertable", () => {
13881388
});
13891389
});
13901390

1391+
test("finds closest container without textual placeholder", () => {
1392+
const { instances } = renderData(
1393+
<$.Body ws:id="bodyId">
1394+
<$.Paragraph ws:id="paragraphId"></$.Paragraph>
1395+
</$.Body>
1396+
);
1397+
$instances.set(instances);
1398+
selectInstance(["paragraphId", "bodyId"]);
1399+
expect(findClosestInsertable(newBoxFragment)).toEqual({
1400+
parentSelector: ["bodyId"],
1401+
position: 1,
1402+
});
1403+
});
1404+
1405+
test("finds closest container even with when parent has placeholder", () => {
1406+
const { instances } = renderData(
1407+
<$.Body ws:id="bodyId">
1408+
<$.Paragraph ws:id="paragraphId">
1409+
<$.Box ws:id="spanId" tag="span"></$.Box>
1410+
</$.Paragraph>
1411+
</$.Body>
1412+
);
1413+
$instances.set(instances);
1414+
selectInstance(["boxId", "paragraphId", "bodyId"]);
1415+
expect(findClosestInsertable(newBoxFragment)).toEqual({
1416+
parentSelector: ["paragraphId", "bodyId"],
1417+
position: 0,
1418+
});
1419+
});
1420+
13911421
test("forbids inserting into :root", () => {
13921422
const { instances } = renderData(<$.Body ws:id="bodyId"></$.Body>);
13931423
$instances.set(instances);

apps/builder/app/shared/matcher.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,10 @@ export const findClosestNonTextualContainer = ({
376376
if (instance === undefined) {
377377
continue;
378378
}
379-
let hasText = false;
379+
const meta = metas.get(instance.component);
380+
// placeholder exists only inside of empty instances
381+
let hasText =
382+
meta?.placeholder !== undefined && instance.children.length === 0;
380383
for (const child of instance.children) {
381384
if (child.type === "text" || child.type === "expression") {
382385
hasText = true;
@@ -392,7 +395,6 @@ export const findClosestNonTextualContainer = ({
392395
if (hasText) {
393396
continue;
394397
}
395-
const meta = metas.get(instance.component);
396398
if (meta?.type === "container") {
397399
return index;
398400
}

packages/react-sdk/src/props.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,6 @@ export const showAttribute = "data-ws-show" as const;
129129
export const indexAttribute = "data-ws-index" as const;
130130
export const collapsedAttribute = "data-ws-collapsed" as const;
131131
export const textContentAttribute = "data-ws-text-content" as const;
132-
export const editablePlaceholderVariable =
133-
"--data-ws-editable-placeholder" as const;
134-
export const editingPlaceholderVariable =
135-
"--data-ws-editing-placeholder" as const;
136132

137133
/**
138134
* Copyright (c) Meta Platforms, Inc. and affiliates.

packages/sdk-components-react/src/blockquote.ws.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ const presetStyle = {
6161

6262
export const meta: WsComponentMeta = {
6363
type: "container",
64+
placeholder: "Blockquote",
6465
icon: BlockquoteIcon,
6566
states: defaultStates,
6667
presetStyle,

packages/sdk-components-react/src/heading.ws.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ const presetStyle = {
2323

2424
export const meta: WsComponentMeta = {
2525
type: "container",
26+
placeholder: "Heading",
2627
icon: HeadingIcon,
2728
constraints: {
2829
relation: "ancestor",

packages/sdk-components-react/src/link.ws.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const presetStyle = {
2121

2222
export const meta: WsComponentMeta = {
2323
type: "container",
24+
placeholder: "Link",
2425
icon: LinkIcon,
2526
constraints: {
2627
relation: "ancestor",

0 commit comments

Comments
 (0)