Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 12 additions & 3 deletions packages/studio/src/hooks/useDomEditCommits.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ function renderDomEditCommits(
applyDomSelection: vi.fn(),
clearDomSelection: vi.fn(),
refreshDomEditSelectionFromPreview: vi.fn(),
buildDomSelectionFromTarget: vi.fn(async () => null),
buildDomSelectionFromTarget: vi.fn(async (target: HTMLElement) =>
target === selection.element ? selection : null,
),
onTrySdkPersist: options.onTrySdkPersist,
readOnlyPreview: false,
});
Expand Down Expand Up @@ -893,7 +895,7 @@ describe("useDomEditCommits rich-text persist handling", () => {
}
});

it("does not retarget a commit onto a replacement preview node", async () => {
it("does not retarget a commit onto a replacement preview node, and says the text was not saved", async () => {
const fetchMock = stubPatchFetch({ ok: true, changed: true, matched: true });
const html = '<span style="color: blue">After</span>';
const { iframe, element } = createPreviewElement(`<div data-hf-id="hf-card">${html}</div>`);
Expand All @@ -902,6 +904,7 @@ describe("useDomEditCommits rich-text persist handling", () => {
replacement.dataset.hfId = "hf-card";
replacement.innerHTML = "Reloaded elsewhere";
element.replaceWith(replacement);
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});

try {
await act(async () => {
Expand All @@ -914,8 +917,14 @@ describe("useDomEditCommits rich-text persist handling", () => {

expect(replacement.innerHTML).toBe("Reloaded elsewhere");
expect(fetchMock).not.toHaveBeenCalled();
expect(rendered.showToast).not.toHaveBeenCalled();
expect(rendered.showToast).toHaveBeenCalledWith(
expect.stringMatching(
/Couldn't save the text edit: the text's element is gone from the preview/,
),
"error",
);
} finally {
errorSpy.mockRestore();
rendered.cleanup();
}
});
Expand Down
3 changes: 2 additions & 1 deletion packages/studio/src/hooks/useDomEditCommits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { StudioSaveHttpError, trackStudioSaveFailure } from "../utils/studioSave
import type { DomEditSelection } from "../components/editor/domEditing";
import { fontFamilyFromAssetPath, type ImportedFontAsset } from "../components/editor/fontAssets";
import type { CommitDomEditPatchBatches } from "./domEditCommitTypes";
import type { ResolveDomSelectionOptions } from "./useDomSelectionTypes";
import type { PatchOperation } from "../utils/sourcePatcher";
import { DomEditPersistUnsafeValueError } from "./domEditPersistFailure";
import { useDomEditPersist, type RecordEditInput } from "./useDomEditPersist";
Expand Down Expand Up @@ -46,7 +47,7 @@ export interface UseDomEditCommitsParams {
refreshDomEditSelectionFromPreview: (selection: DomEditSelection) => void;
buildDomSelectionFromTarget: (
target: HTMLElement,
options?: { preferClipAncestor?: boolean },
options?: ResolveDomSelectionOptions,
) => Promise<DomEditSelection | null>;
/** Resync the in-memory SDK session after a SERVER-side write (NOT the SDK
* path, whose session is already current) so a later SDK edit doesn't
Expand Down
67 changes: 62 additions & 5 deletions packages/studio/src/hooks/useDomEditTextCommits.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,20 @@ afterEach(() => {
});

describe("useDomEditTextCommits", () => {
function richTextProbe() {
const { iframe, element } = previewElement('<h1 id="t">Old</h1>', "t");
function richTextProbe(
html = '<h1 id="t">Old</h1>',
overrides: (doc: Document) => Partial<UseDomEditTextCommitsParams> = () => ({}),
) {
const { iframe, element } = previewElement(html, "t");
const persist = vi.fn().mockResolvedValue(undefined);
const showToast = vi.fn();
const base = commitParams({
previewIframeRef: { current: iframe },
domEditSelection: selectionFor(element),
buildDomSelectionFromTarget: vi.fn(async (target: HTMLElement) => selectionFor(target)),
persistDomEditOperations: persist,
showToast,
...overrides(element.ownerDocument),
});
const captured: { hook: ReturnType<typeof useDomEditTextCommits> | null } = { hook: null };
function Probe({ readOnlyPreview }: { readOnlyPreview: boolean }) {
Expand All @@ -143,13 +150,25 @@ describe("useDomEditTextCommits", () => {
const save = captured.hook!.handleDomRichTextCommit;
const commit = { element, html: "New", previousHtml: "Old" };
element.innerHTML = "New";
return { root, Probe, persist, element, save: () => act(async () => save(commit)) };
return {
root,
Probe,
persist,
showToast,
element,
save: () => act(async () => save(commit)),
};
}

it("saves in-place text while the preview is editable", async () => {
const { persist, save } = richTextProbe();
it("saves in-place text while the preview is editable, and refreshes the selection it edited", async () => {
const applyDomSelection = vi.fn();
const { persist, element, save } = richTextProbe(undefined, () => ({ applyDomSelection }));
await save();
expect(persist).toHaveBeenCalledTimes(1);
expect(applyDomSelection).toHaveBeenCalledWith(
expect.objectContaining({ element }),
expect.objectContaining({ preserveGroup: true }),
);
});

it("refuses in-place text once the preview turns read-only, through an earlier handler, and puts the old text back", async () => {
Expand All @@ -160,6 +179,44 @@ describe("useDomEditTextCommits", () => {
expect(element.innerHTML).toBe("Old");
});

it("saves the edited element, and leaves the selection alone, when the selection is another element or none", async () => {
for (const selected of ["card", null]) {
const applyDomSelection = vi.fn();
const { persist, element, save } = richTextProbe(
'<div id="card"><p id="t">Old</p></div>',
(doc) => ({
domEditSelection: selected ? selectionFor(doc.getElementById(selected)!) : null,
applyDomSelection,
buildDomSelectionFromTarget: vi.fn(async (target: HTMLElement) => ({
...selectionFor(target),
label: "Resolved from the edited element",
})),
}),
);
await save();
expect(persist).toHaveBeenCalledTimes(1);
expect(persist.mock.calls[0]![0]).toMatchObject({
element,
label: "Resolved from the edited element",
});
expect(applyDomSelection).not.toHaveBeenCalled();
cleanup?.();
cleanup = null;
}
});

it("says so and puts the old text back when the edited text cannot be saved", async () => {
const error = vi.spyOn(console, "error").mockImplementation(() => {});
const { persist, showToast, element, save } = richTextProbe(undefined, () => ({
buildDomSelectionFromTarget: vi.fn(async () => null),
}));
await save();
expect(persist).not.toHaveBeenCalled();
expect(showToast).toHaveBeenCalledWith(expect.stringContaining("Couldn't save"), "error");
expect(error).toHaveBeenCalled();
expect(element.innerHTML).toBe("Old");
});

it("keeps concurrent text commit ownership isolated by target", async () => {
const { iframe, element: firstElement } = previewElement(
"<div id='first'>First</div><div id='second'>Second</div>",
Expand Down
70 changes: 35 additions & 35 deletions packages/studio/src/hooks/useDomEditTextCommits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
import { commitDomStyles } from "./domStyleCommit";
import { useDomEditAttributeCommits } from "./useDomEditAttributeCommits";
import type { InlineTextEditCommit } from "./useInlineTextEdit";
import type { ResolveDomSelectionOptions } from "./useDomSelectionTypes";

// ── Types ──

Expand All @@ -44,7 +45,7 @@ export interface UseDomEditTextCommitsParams {
refreshDomEditSelectionFromPreview: (selection: DomEditSelection) => void;
buildDomSelectionFromTarget: (
target: HTMLElement,
options?: { preferClipAncestor?: boolean },
options?: ResolveDomSelectionOptions,
) => Promise<DomEditSelection | null>;
persistDomEditOperations: PersistDomEditOperations;
resolveImportedFontAsset: (fontFamilyValue: string) => ImportedFontAsset | null;
Expand All @@ -56,15 +57,6 @@ function canCommitInlineTextSelection(selection: DomEditSelection, element: HTML
return canEditElementTextInline(element);
}

function ownsCurrentPreviewElement(
selection: DomEditSelection,
element: HTMLElement,
document: Document | null | undefined,
): document is Document {
if (!document || !element.isConnected) return false;
return element === selection.element && element.ownerDocument === document;
}

async function resyncDomTextSelectionFromPreview(
doc: Document | null | undefined,
selection: DomEditSelection,
Expand Down Expand Up @@ -96,6 +88,8 @@ export function useDomEditTextCommits({
}: UseDomEditTextCommitsParams) {
const latestReadOnlyPreviewRef = useRef(readOnlyPreview);
latestReadOnlyPreviewRef.current = readOnlyPreview;
const latestSelectionRef = useRef(domEditSelection);
latestSelectionRef.current = domEditSelection;
const domTextCommitVersionRef = useRef(new Map<string, symbol>());
const domStyleCommitVersionRef = useRef(new Map<string, symbol>());

Expand Down Expand Up @@ -238,29 +232,36 @@ export function useDomEditTextCommits({
*/
const handleDomRichTextCommit = useCallback(
async ({ element, html, previousHtml }: InlineTextEditCommit) => {
if (!domEditSelection) return;
if (latestReadOnlyPreviewRef.current) {
const putBack = () => {
if (element.isConnected && element.innerHTML === html) element.innerHTML = previousHtml;
return;
};
if (latestReadOnlyPreviewRef.current) return putBack();
const refuse = (reason: string) => {
console.error("[Studio] text edit not saved:", reason, element);
showToast(`Couldn't save the text edit: ${reason}`, "error");
putBack();
};
// The edited node, not the current selection: a press can open an edit on a child of what
// is selected, and a host can clear the selection before the edit closes.
const doc = previewIframeRef.current?.contentDocument;
if (!doc || !element.isConnected || element.ownerDocument !== doc) {
return refuse("the text's element is gone from the preview");
}
const selection = await buildDomSelectionFromTarget(element, {
exactTarget: true,
skipSourceProbe: true,
});
if (selection?.element !== element) {
return refuse("this text was not found in the composition's source");
}
// The same gate that let the edit open, not the design panel's field rule: an element
// whose text holds a line break has no fields, and editing in place rewrites its markup.
if (!canCommitInlineTextSelection(selection, element)) {
return refuse("this text can't be edited in place");
}
// The same gate that let the edit open, not the design panel's.
//
// The panel's rule is about its text fields, and it has none for an
// element whose text contains a line break: a `<span>` holding `<br>`s
// is not a leaf, so nothing inside is a field and the element reports no
// editable text at all. Editing in place does not use fields — it
// rewrites the element's own markup — so refusing on that rule refused
// elements the caret had just been opened in, and every colour the user
// chose was dropped on the way out with nothing said about it.
if (!canCommitInlineTextSelection(domEditSelection, element)) return;
const iframe = previewIframeRef.current;
const doc = iframe?.contentDocument;
// A preview reload replaces the document. Never resolve this commit onto
// the replacement node: it did not own the edit or its rollback snapshot.
if (!ownsCurrentPreviewElement(domEditSelection, element, doc)) return;
const isLatestTextCommit = bumpDomEditCommitMapVersion(
domTextCommitVersionRef.current,
getDomEditTargetKey(domEditSelection),
getDomEditTargetKey(selection),
);
const operations = [buildDomEditRichTextPatchOperation(html)];
let appliedHtml = "";
Expand All @@ -274,7 +275,7 @@ export function useDomEditTextCommits({
appliedHtml = element.innerHTML;
},
persist: async () => {
await persistDomEditOperations(domEditSelection, operations, {
await persistDomEditOperations(selection, operations, {
label: "Edit text",
skipRefresh: true,
shouldSave: isLatestTextCommit,
Expand All @@ -288,13 +289,13 @@ export function useDomEditTextCommits({
element.innerHTML = previousHtml;
}
},
onError: (error) =>
reportDomEditPersistFailure(domEditSelection, operations, error, showToast),
shouldResync: isLatestTextCommit,
onError: (error) => reportDomEditPersistFailure(selection, operations, error, showToast),
// Re-select only what is still selected: the selection may have moved on, or a host cleared it.
shouldResync: () => isLatestTextCommit() && latestSelectionRef.current?.element === element,
resync: () =>
resyncDomTextSelectionFromPreview(
doc,
domEditSelection,
selection,
activeCompPath,
buildDomSelectionFromTarget,
applyDomSelection,
Expand All @@ -306,7 +307,6 @@ export function useDomEditTextCommits({
activeCompPath,
applyDomSelection,
buildDomSelectionFromTarget,
domEditSelection,
persistDomEditOperations,
previewIframeRef,
showToast,
Expand Down
Loading