Skip to content

fix(visual-builder): grey structural actions while a collaborator is in the container #679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sairaj-cs merged 3 commits into from
Nov 6, 2026
Merged

fix(visual-builder): grey structural actions while a collaborator is in the container #679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c3605b
fix(visual-builder): grey structural actions while a collaborator is …
sairaj-cs Nov 3, 2026
f3dc780
fix(visual-builder): unmount add-instance buttons when removing them
sairaj-cs Nov 3, 2026
c69964c
fix(visual-builder): check a field's own lock by its index-form path
sairaj-cs Nov 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 36 additions & 10 deletions src/visualBuilder/components/FieldToolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ import { EntryPermissions } from "../utils/getEntryPermissions";
import { FieldLocationAppList } from "./FieldLocationAppList";
import { FieldLocationIcon } from "./FieldLocationIcon";
import { WorkflowStageDetails } from "../utils/getWorkflowStageDetails";
import { getEntryEditRestrictionForField } from "../utils/fieldLockIndicator";
import {
getEntryEditRestrictionForField,
getPeerLockForContainer,
peerLockMessage,
} from "../utils/fieldLockIndicator";
import { subscribeEntryFieldLockInfo } from "../utils/fieldLockStore";
import { ResolvedVariantPermissions } from "../utils/getResolvedVariantPermissions";
import { isCustomFieldMultipleInstance as checkIsCustomFieldMultipleInstance } from "../utils/isCustomFieldMultipleInstance";
Expand Down Expand Up @@ -201,12 +205,21 @@ function FieldToolbarComponent(
const [restrictionReason, setRestrictionReason] = useState(() =>
getEntryEditRestrictionForField(fieldMetadata)
);
const [containerPeerLock, setContainerPeerLock] = useState(() =>
getPeerLockForContainer(fieldMetadata)
);
useEffect(() => {
const update = () =>
const update = () => {
setRestrictionReason(getEntryEditRestrictionForField(fieldMetadata));
setContainerPeerLock(getPeerLockForContainer(fieldMetadata));
};
update();
return subscribeEntryFieldLockInfo(update);
}, [fieldMetadata]);
// Add / move / delete change the whole container, so a collaborator anywhere in it blocks them.
const containerLockReason = containerPeerLock
? peerLockMessage(containerPeerLock)
: null;

let disableFieldActions = Boolean(restrictionReason);
let isCustomFieldMultipleInstance = false;
Expand Down Expand Up @@ -560,9 +573,10 @@ function FieldToolbarComponent(
multipleFieldToolbarButtonClasses
}
data-tooltip={
direction === "vertical"
containerLockReason ??
(direction === "vertical"
? "Move up"
: "Move left"
: "Move left")
}
onClick={(e) => {
e.preventDefault();
Expand All @@ -573,7 +587,9 @@ function FieldToolbarComponent(
);
}}
disabled={
disableFieldActions || disableMoveLeft
disableFieldActions ||
Boolean(containerLockReason) ||
disableMoveLeft
}
>
<MoveLeftIcon
Expand All @@ -586,6 +602,7 @@ function FieldToolbarComponent(
})}
disabled={
disableFieldActions ||
Boolean(containerLockReason) ||
disableMoveLeft
}
/>
Expand All @@ -597,9 +614,10 @@ function FieldToolbarComponent(
multipleFieldToolbarButtonClasses
}
data-tooltip={
direction === "vertical"
containerLockReason ??
(direction === "vertical"
? "Move down"
: "Move right"
: "Move right")
}
onClick={(e) => {
e.preventDefault();
Expand All @@ -610,7 +628,9 @@ function FieldToolbarComponent(
);
}}
disabled={
disableFieldActions || disableMoveRight
disableFieldActions ||
Boolean(containerLockReason) ||
disableMoveRight
}
>
<MoveRightIcon
Expand All @@ -623,6 +643,7 @@ function FieldToolbarComponent(
})}
disabled={
disableFieldActions ||
Boolean(containerLockReason) ||
disableMoveRight
}
/>
Expand All @@ -637,13 +658,18 @@ function FieldToolbarComponent(
className={
multipleFieldToolbarButtonClasses
}
data-tooltip={"Delete"}
data-tooltip={
containerLockReason ?? "Delete"
}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
handleDeleteInstance(fieldMetadata);
}}
disabled={disableFieldActions}
disabled={
disableFieldActions ||
Boolean(containerLockReason)
}
>
<DeleteIcon />
</button>
Expand Down
66 changes: 66 additions & 0 deletions src/visualBuilder/components/__test__/addInstanceButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import { singleLineFieldSchema } from "../../../__test__/data/fields";
import AddInstanceButtonComponent from "../addInstanceButton";
import visualBuilderPostMessageActual from "../../utils/visualBuilderPostMessage";
import { getDiscussionIdByFieldMetaData } from "../../utils/getDiscussionIdByFieldMetaData";
import {
clearAllEntryFieldLockInfo,
setEntryFieldLockInfo,
} from "../../utils/fieldLockStore";

const visualBuilderPostMessage = vi.mocked(visualBuilderPostMessageActual);

Expand Down Expand Up @@ -147,4 +151,66 @@ describe("AddInstanceButtonComponent", () => {
expect(onFieldLockRefused).toHaveBeenCalled();
expect(onClickCallback).not.toHaveBeenCalled();
});
describe("a collaborator inside the container", () => {
const fieldMetadata = {
entry_uid: "e1",
locale: "en-us",
fieldPathWithIndex: "group",
};
const lockIn = (path: string) =>
setEntryFieldLockInfo(
{ entryUid: "e1", locale: "en-us" },
{
[path]: {
user: { uid: "u2", name: "Watcher" },
ttl: "2030-01-01",
isLocked: true,
isOwn: false,
},
}
);
const renderButton = async () => {
await act(() => {
render(
<AddInstanceButtonComponent
value={[]}
fieldSchema={singleLineFieldSchema}
// @ts-expect-error partial fieldMetadata
fieldMetadata={fieldMetadata}
index={0}
onClick={vi.fn()}
onFieldLockRefused={vi.fn()}
// @ts-expect-error mocking signal
loading={{ value: false }}
/>
);
});
return getByTestId(
document.body,
"visual-builder-add-instance-button"
);
};

afterEach(() => clearAllEntryFieldLockInfo());

test("is disabled with the holder in the tooltip", async () => {
lockIn("group.1.title");
const button = await renderButton();
expect(button).toBeDisabled();
expect(button).toHaveAttribute(
"data-tooltip",
"This field is locked by Watcher"
);
});

test("re-enables when the lock goes", async () => {
lockIn("group.1");
const button = await renderButton();
await act(() =>
setEntryFieldLockInfo({ entryUid: "e1", locale: "en-us" }, {})
);
expect(button).not.toBeDisabled();
expect(button).toHaveAttribute("data-tooltip", "Add section");
});
});
});
56 changes: 56 additions & 0 deletions src/visualBuilder/components/__test__/fieldToolbar.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { VisualBuilderPostMessageEvents } from "../../utils/types/postMessage.ty
import {
clearAllEntryFieldLockInfo,
setEntryEditRestriction,
setEntryFieldLockInfo,
} from "../../utils/fieldLockStore";
import { VisualBuilder } from "../..";

Expand Down Expand Up @@ -817,6 +818,61 @@ describe("FieldToolbarComponent", () => {
await waitFor(() => expect(deleteButton()).toBeDisabled());
});

describe("a collaborator inside the container", () => {
const lockIn = (path: string) =>
setEntryFieldLockInfo(scope, {
[path]: {
user: { uid: "u2", name: "Watcher" },
ttl: "2030-01-01",
isLocked: true,
isOwn: false,
},
});
const button = (container: Element, name: string) =>
container.querySelector(
`[data-testid="visual-builder__focused-toolbar__multiple-field-toolbar__${name}-button"]`
);

test("greys move and delete with the holder in the tooltip", async () => {
lockIn("group.link.1");
const deleteButton = await renderToolbar();

await waitFor(() => expect(deleteButton()).toBeDisabled());
expect(deleteButton()).toHaveAttribute(
"data-tooltip",
"This field is locked by Watcher"
);
const toolbar = deleteButton()!.parentElement!;
expect(button(toolbar, "move-left")).toBeDisabled();
expect(button(toolbar, "move-right")).toBeDisabled();
});

test("re-enables them when the lock is released", async () => {
lockIn("group.link.1");
const deleteButton = await renderToolbar();
await waitFor(() => expect(deleteButton()).toBeDisabled());

act(() => setEntryFieldLockInfo(scope, {}));

await waitFor(() =>
expect(deleteButton()).not.toBeDisabled()
);
expect(deleteButton()).toHaveAttribute(
"data-tooltip",
"Delete"
);
});

test("ignores a lock in another field", async () => {
lockIn("group.title");
const deleteButton = await renderToolbar();

await waitFor(() =>
expect(deleteButton()).not.toBeDisabled()
);
});
});

test("disables the field actions when the restriction arrives after the toolbar opened", async () => {
const deleteButton = await renderToolbar();
await waitFor(() => expect(deleteButton()).not.toBeDisabled());
Expand Down
25 changes: 22 additions & 3 deletions src/visualBuilder/components/addInstanceButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from "preact/compat";
import React, { useEffect, useState } from "preact/compat";
import classNames from "classnames";
import { visualBuilderStyles } from "../visualBuilder.style";
import { PlusIcon } from "./icons";
Expand All @@ -8,6 +8,11 @@ import visualBuilderPostMessage from "../utils/visualBuilderPostMessage";
import { VisualBuilderPostMessageEvents } from "../utils/types/postMessage.types";
import { Signal } from "@preact/signals";
import { isFieldLockRefused } from "../utils/fieldLockRefused";
import {
getPeerLockForContainer,
peerLockMessage,
} from "../utils/fieldLockIndicator";
import { subscribeEntryFieldLockInfo } from "../utils/fieldLockStore";

interface AddInstanceButtonProps {
value: any;
Expand All @@ -28,6 +33,15 @@ function AddInstanceButtonComponent(
const fieldMetadata = props.fieldMetadata;
const index = props.index;
const loading = props.loading;
const [containerPeerLock, setContainerPeerLock] = useState(() =>
getPeerLockForContainer(fieldMetadata)
);
useEffect(() => {
const update = () =>
setContainerPeerLock(getPeerLockForContainer(fieldMetadata));
update();
return subscribeEntryFieldLockInfo(update);
}, [fieldMetadata]);

const onClick = async (event: MouseEvent) => {
loading.value = true;
Expand Down Expand Up @@ -71,12 +85,17 @@ function AddInstanceButtonComponent(
const isMaxInstances = maxInstances
? props.value.length >= maxInstances
: false;
const disabled = loading.value || isMaxInstances;
const disabled =
loading.value || isMaxInstances || Boolean(containerPeerLock);

return (
<button
className={buttonClassName}
data-tooltip={"Add section"}
data-tooltip={
containerPeerLock
? peerLockMessage(containerPeerLock)
: "Add section"
}
data-testid="visual-builder-add-instance-button"
disabled={disabled}
title={
Expand Down
6 changes: 2 additions & 4 deletions src/visualBuilder/components/fieldLabelWrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { RESULT_TYPES } from "../utils/constants";
import {
getEntryEditRestrictionForField,
getPeerLockForField,
lockAvatarInfo,
peerLockMessage,
} from "../utils/fieldLockIndicator";
import { subscribeEntryFieldLockInfo } from "../utils/fieldLockStore";

Expand Down Expand Up @@ -250,9 +250,7 @@ function FieldLabelWrapperComponent(
if (!base.fieldDisabled && restrictionReason) {
effectiveReason = restrictionReason;
} else if (!base.fieldDisabled && peerLock) {
effectiveReason = `This field is locked by ${
lockAvatarInfo(peerLock).name || "another user"
}`;
effectiveReason = peerLockMessage(peerLock);
}
const usePlainDataTooltip =
effectiveReason &&
Expand Down
33 changes: 32 additions & 1 deletion src/visualBuilder/generators/__test__/generateAddInstanceButtons.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React from "preact/compat";
import React, { useEffect } from "preact/compat";
import { singleLineFieldSchema } from "../../../__test__/data/fields";
import {
getAddInstanceButtons,
generateAddInstanceButton,
removeAddInstanceButton,
} from "../generateAddInstanceButtons";
import AddInstanceButtonComponentActual from "../../components/addInstanceButton";

Expand Down Expand Up @@ -41,6 +42,36 @@ describe("generateAddInstanceButton", () => {
expect(button).toBeInstanceOf(HTMLButtonElement);
});

test("removeAddInstanceButton unmounts the component so its effects clean up", async () => {
const mounted = vi.fn();
const cleanup = vi.fn();
AddInstanceButtonComponent.mockImplementationOnce(() => {
useEffect(() => {
mounted();
return cleanup;
}, []);
return <button>Add</button>;
});
const button = generateAddInstanceButton({
fieldSchema: singleLineFieldSchema,
value: "",
// @ts-expect-error mock field metadata
fieldMetadata: { hello: "world" },
onClick: vi.fn(),
onFieldLockRefused: vi.fn(),
// @ts-expect-error mocking preact signal
loading: { value: false },
index: 0,
});
document.body.appendChild(button);
await vi.waitFor(() => expect(mounted).toHaveBeenCalled());

removeAddInstanceButton(button);

expect(cleanup).toHaveBeenCalledTimes(1);
expect(button.isConnected).toBe(false);
});

test("should call the AddInstanceButtonComponent with the correct props", () => {
generateAddInstanceButton({
fieldSchema: singleLineFieldSchema,
Expand Down
Loading
Loading