From e6e0d97cc560ebca57a28f5c0872e706679f568d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Fri, 3 Jul 2026 18:22:22 -0700 Subject: [PATCH] fix(studio): surface persist failures with toast and guarded revert (#1910) * test(studio): add design-panel QA fixture and triage matrix Fixture project covering all panel-editable element archetypes, plus the QA findings matrix from the design-panel bug campaign. * fix(studio): make canvas selection hit intended elements - honor author pointer-events:none in hit-testing (was selecting invisible overlays) - pause playback before mousedown sampling; fall back to hover selection on null resolve - invalidate committed selection when the active composition changes - double-click keeps selection and defers to multi-candidate click cycling * fix(studio): close remaining selection-layer review findings - hoverSelection fallback now wired at all 3 mousedown call sites (box-click, blocked-drag, plain overlay click) instead of just the overlay path - pointer-events override detection reads computed style, not inline style, so a CSS-class opt-in (not just inline style=) on a descendant is honored - defensively remove the pointer-events override before the group-fallback check too, closing a theoretical gap in the no-elementsFromPoint branch - a click that resolves to nothing (dead-zone / deselect) no longer leaves playback paused if it was already playing * fix(studio-server): child-scoped patch operations with batch abort - PatchOperation gains optional childSelector/childIndex resolved under the matched parent - pre-pass resolves every op target; any miss aborts the batch with matched:false, no partial write - style-decl parsing extracted to sourceStyleMutation to stay under the file-size cap - new ./source-mutation subpath export (mirrors ./finite-mutation) * fix(studio): per-child patch op builders and persist-seam harness - buildTextFieldChildLocator indexes over the parent's full same-tag child list - buildTextFieldChildOperations emits per-field ops for same-shape multi-field edits - SDK cutover declines child-scoped batches (hfId mapping would hit the parent) - persist-seam integration harness drives real client ops through patchElementInHtml * fix(studio): fail closed on unresolved text-field child index buildTextFieldChildLocator guessed a synthetic field's position by counting same-tag "child" fields elsewhere in the array whenever sourceChildIndex was absent. That heuristic is unreachable today (the count-mismatch guard in buildTextFieldChildOperations already refuses add/remove edits before it's reached) but would silently locate the wrong element for a future caller that wires up synthetic-field support without also computing a real sourceChildIndex. Return null instead so the caller falls back to the unsupported-structure path. * fix(studio): surface persist failures with toast and guarded revert - matched:false and persist errors toast, warn structurally, and revert the optimistic write - reverts guarded by a per-property version counter so stale failures never stomp newer edits - structural text-field edits refuse persist instead of writing escaped markup - multi-field child edits persist via per-child ops; shared commit runner extracted * fix(studio): revert data-attribute and html-attribute commits on persist failure commitDataAttribute and handleDomHtmlAttributeCommit toasted on failure but never reverted the optimistic attribute write, leaving the preview showing an edit that never reached disk (the exact bug this PR closes for style commits). Extracted into useDomEditAttributeCommits.ts (useDomEditTextCommits.ts was at the file-size cap) and routed through runDomEditCommit with a per-target+ attribute version guard, mirroring handleDomStyleCommit. * fix(studio): close coupled persist-hook review findings Three findings from R2 review that must land together: a patch-rejection toast doubled up with the generic persist-failure toast (StudioSaveHttpError had no alreadyToasted marker), a failed prepareContent write (e.g. font-face injection) reverted and re-toasted a change the server had already persisted, and text-commit shouldRevert only rolled back on one narrow error type instead of any persist failure. --- .../studio/src/hooks/domEditCommitRunner.ts | 46 ++ .../src/hooks/domEditPersistFailure.test.ts | 123 +++++ .../studio/src/hooks/domEditPersistFailure.ts | 89 ++++ .../src/hooks/useDomEditAttributeCommits.ts | 227 +++++++++ .../studio/src/hooks/useDomEditCommits.ts | 38 +- .../studio/src/hooks/useDomEditTextCommits.ts | 469 +++++++++--------- .../studio/src/utils/studioSaveDiagnostics.ts | 7 +- 7 files changed, 769 insertions(+), 230 deletions(-) create mode 100644 packages/studio/src/hooks/domEditCommitRunner.ts create mode 100644 packages/studio/src/hooks/domEditPersistFailure.test.ts create mode 100644 packages/studio/src/hooks/domEditPersistFailure.ts create mode 100644 packages/studio/src/hooks/useDomEditAttributeCommits.ts diff --git a/packages/studio/src/hooks/domEditCommitRunner.ts b/packages/studio/src/hooks/domEditCommitRunner.ts new file mode 100644 index 000000000..9e11e6e3c --- /dev/null +++ b/packages/studio/src/hooks/domEditCommitRunner.ts @@ -0,0 +1,46 @@ +interface DomEditCommitRunnerConfig { + capture: () => void; + apply: () => void; + persist: () => Promise; + shouldRevert: (error: unknown) => boolean; + revert: () => void; + onError: (error: unknown) => void; + shouldResync: () => boolean; + resync: () => void | Promise; +} + +interface CommitVersionRef { + current: number; +} + +export function bumpDomEditCommitVersion(versionRef: CommitVersionRef): () => boolean { + const commitVersion = versionRef.current + 1; + versionRef.current = commitVersion; + return () => versionRef.current === commitVersion; +} + +export function bumpDomEditCommitMapVersion( + versionMap: Map, + versionKey: TKey, +): () => boolean { + const commitVersion = (versionMap.get(versionKey) ?? 0) + 1; + versionMap.set(versionKey, commitVersion); + return () => versionMap.get(versionKey) === commitVersion; +} + +export async function runDomEditCommit(config: DomEditCommitRunnerConfig): Promise { + config.capture(); + config.apply(); + + try { + await config.persist(); + } catch (error) { + if (config.shouldRevert(error)) { + config.revert(); + } + config.onError(error); + } + + if (!config.shouldResync()) return; + await config.resync(); +} diff --git a/packages/studio/src/hooks/domEditPersistFailure.test.ts b/packages/studio/src/hooks/domEditPersistFailure.test.ts new file mode 100644 index 000000000..6b2c2ae17 --- /dev/null +++ b/packages/studio/src/hooks/domEditPersistFailure.test.ts @@ -0,0 +1,123 @@ +import { describe, expect, it, vi } from "vitest"; +import type { PatchOperation } from "../utils/sourcePatcher"; +import { StudioSaveHttpError } from "../utils/studioSaveDiagnostics"; +import { + DomEditPersistUnsafeValueError, + reportDomEditPersistFailure, + warnDomEditPersistNoOp, +} from "./domEditPersistFailure"; + +const selection = { + label: "Hero title", + hfId: "hf-hero", + id: "hero", + selector: ".hero", + selectorIndex: 0, + sourceFile: "index.html", +}; + +const operations: PatchOperation[] = [{ type: "inline-style", property: "color", value: "red" }]; + +describe("reportDomEditPersistFailure", () => { + it("toasts with the selected label and underlying error detail", () => { + const showToast = vi.fn<(message: string, tone?: "error" | "info") => void>(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + reportDomEditPersistFailure(selection, operations, new Error("network down"), showToast); + + expect(showToast).toHaveBeenCalledWith('Couldn\'t save "Hero title": network down', "error"); + expect(warnSpy).toHaveBeenCalledWith( + "[Studio] DOM edit persist failed", + expect.objectContaining({ + target: { + hfId: "hf-hero", + id: "hero", + selector: ".hero", + selectorIndex: 0, + sourceFile: "index.html", + }, + operations: "inline-style:color", + error: "network down", + }), + ); + + warnSpy.mockRestore(); + }); + + it("toasts StudioSaveHttpError and unmarked unsafe errors", () => { + const showToast = vi.fn<(message: string, tone?: "error" | "info") => void>(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + reportDomEditPersistFailure( + selection, + operations, + new StudioSaveHttpError("Failed to patch index.html (500)", 500), + showToast, + ); + reportDomEditPersistFailure( + selection, + operations, + new DomEditPersistUnsafeValueError("DOM patch contains unsafe values: style.width"), + showToast, + ); + + expect(showToast).toHaveBeenCalledTimes(2); + expect(showToast).toHaveBeenCalledWith( + expect.stringContaining("Failed to patch index.html"), + "error", + ); + expect(showToast).toHaveBeenCalledWith( + expect.stringContaining("DOM patch contains unsafe values: style.width"), + "error", + ); + expect(warnSpy).toHaveBeenCalledTimes(2); + + warnSpy.mockRestore(); + }); + + it("does not toast errors explicitly marked as already toasted", () => { + const showToast = vi.fn<(message: string, tone?: "error" | "info") => void>(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const error = new DomEditPersistUnsafeValueError( + "DOM patch contains unsafe values: style.width", + ); + Object.defineProperty(error, "alreadyToasted", { value: true }); + + reportDomEditPersistFailure(selection, operations, error, showToast); + + expect(showToast).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + "[Studio] DOM edit persist failed", + expect.objectContaining({ + target: expect.objectContaining({ hfId: "hf-hero", sourceFile: "index.html" }), + }), + ); + + warnSpy.mockRestore(); + }); +}); + +describe("warnDomEditPersistNoOp", () => { + it("logs a structured breadcrumb without requiring a toast callback", () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + warnDomEditPersistNoOp(selection, operations); + + expect(warnSpy).toHaveBeenCalledWith( + "[Studio] DOM edit persist no-op", + expect.objectContaining({ + target: { + hfId: "hf-hero", + id: "hero", + selector: ".hero", + selectorIndex: 0, + sourceFile: "index.html", + }, + operations: "inline-style:color", + }), + ); + + warnSpy.mockRestore(); + }); +}); diff --git a/packages/studio/src/hooks/domEditPersistFailure.ts b/packages/studio/src/hooks/domEditPersistFailure.ts new file mode 100644 index 000000000..1e7bd7a40 --- /dev/null +++ b/packages/studio/src/hooks/domEditPersistFailure.ts @@ -0,0 +1,89 @@ +import type { DomEditSelection } from "../components/editor/domEditing"; +import { StudioSaveHttpError } from "../utils/studioSaveDiagnostics"; +import type { PatchOperation } from "../utils/sourcePatcher"; + +export class DomEditPersistUnresolvableError extends Error { + constructor(targetPath: string) { + super(`Couldn't find this element in the source file (${targetPath})`); + this.name = "DomEditPersistUnresolvableError"; + } +} + +export class DomEditPersistUnsafeValueError extends Error { + readonly alreadyToasted: boolean; + + constructor(message: string, options: { alreadyToasted?: boolean } = {}) { + super(message); + this.name = "DomEditPersistUnsafeValueError"; + this.alreadyToasted = options.alreadyToasted ?? false; + } +} + +export class DomEditPersistUnsupportedTextStructureError extends Error { + constructor() { + super("Couldn't save this text structure change"); + this.name = "DomEditPersistUnsupportedTextStructureError"; + } +} + +export type DomEditPersistFailureSelection = Pick< + DomEditSelection, + "label" | "hfId" | "id" | "selector" | "selectorIndex" | "sourceFile" +>; + +function summarizeOperations(operations: PatchOperation[]): string { + return operations.map((op) => `${op.type}:${op.property}`).join(", "); +} + +function getTargetTuple(selection: DomEditPersistFailureSelection) { + return { + hfId: selection.hfId, + id: selection.id, + selector: selection.selector, + selectorIndex: selection.selectorIndex, + sourceFile: selection.sourceFile, + }; +} + +function getErrorDetail(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +function getSelectionLabel(selection: DomEditPersistFailureSelection): string { + return selection.label || selection.selector || selection.id || "this element"; +} + +export function reportDomEditPersistFailure( + selection: DomEditPersistFailureSelection, + operations: PatchOperation[], + error: unknown, + showToast: (message: string, tone?: "error" | "info") => void, +): void { + const detail = getErrorDetail(error); + console.warn("[Studio] DOM edit persist failed", { + target: getTargetTuple(selection), + operations: summarizeOperations(operations), + error: detail, + }); + + const wasAlreadyToasted = + (error instanceof DomEditPersistUnsafeValueError || error instanceof StudioSaveHttpError) && + error.alreadyToasted; + if (wasAlreadyToasted) { + return; + } + + showToast(`Couldn't save "${getSelectionLabel(selection)}": ${detail}`, "error"); +} + +export function warnDomEditPersistNoOp( + selection: DomEditPersistFailureSelection, + operations: PatchOperation[], +): void { + console.warn("[Studio] DOM edit persist no-op", { + target: getTargetTuple(selection), + operations: summarizeOperations(operations), + detail: + "Server matched the target but reported no change even though the client believed the value changed.", + }); +} diff --git a/packages/studio/src/hooks/useDomEditAttributeCommits.ts b/packages/studio/src/hooks/useDomEditAttributeCommits.ts new file mode 100644 index 000000000..34d7a1599 --- /dev/null +++ b/packages/studio/src/hooks/useDomEditAttributeCommits.ts @@ -0,0 +1,227 @@ +import { useCallback, useRef } from "react"; +import type { PatchOperation } from "../utils/sourcePatcher"; +import { + findElementForSelection, + getDomEditTargetKey, + type DomEditSelection, +} from "../components/editor/domEditing"; +import type { PersistDomEditOperations } from "./domEditCommitTypes"; +import { reportDomEditPersistFailure } from "./domEditPersistFailure"; +import { bumpDomEditCommitMapVersion, runDomEditCommit } from "./domEditCommitRunner"; + +// ── Types ── + +export interface UseDomEditAttributeCommitsParams { + activeCompPath: string | null; + previewIframeRef: React.MutableRefObject; + showToast: (message: string, tone?: "error" | "info") => void; + domEditSelection: DomEditSelection | null; + refreshDomEditSelectionFromPreview: (selection: DomEditSelection) => void; + persistDomEditOperations: PersistDomEditOperations; +} + +interface DataAttributeCommitOptions { + label: string; + coalescePrefix: string; + skipRefresh: boolean; + refreshAfter?: boolean; +} + +function resolveFullAttrName(attr: string, prefixData: boolean | undefined): string { + return prefixData && !attr.startsWith("data-") ? `data-${attr}` : attr; +} + +function setOrRemovePreviewAttribute( + el: HTMLElement, + fullAttr: string, + value: string | null, +): void { + if (value === null) { + el.removeAttribute(fullAttr); + } else { + el.setAttribute(fullAttr, value); + } +} + +function findPreviewAttributeElement( + doc: Document | null | undefined, + selection: DomEditSelection, + activeCompPath: string | null, +): HTMLElement | null { + if (!doc) return null; + return findElementForSelection(doc, selection, activeCompPath); +} + +interface CapturedAttributeElement { + element: HTMLElement; + previousValue: string | null; +} + +function captureAttributeElement( + doc: Document | null | undefined, + selection: DomEditSelection, + activeCompPath: string | null, + fullAttr: string, +): CapturedAttributeElement | null { + const el = findPreviewAttributeElement(doc, selection, activeCompPath); + if (!el) return null; + return { element: el, previousValue: el.getAttribute(fullAttr) }; +} + +// ── Hook ── + +// data-* attribute commits and raw HTML-attribute commits (e.g. muted, loop): +// both revert the optimistic write on persist failure, version-guarded per +// target+attribute so a stale failure can't stomp a newer successful commit. +export function useDomEditAttributeCommits({ + activeCompPath, + previewIframeRef, + showToast, + domEditSelection, + refreshDomEditSelectionFromPreview, + persistDomEditOperations, +}: UseDomEditAttributeCommitsParams) { + const domAttributeCommitVersionRef = useRef(new Map()); + + const commitDataAttribute = useCallback( + async (attr: string, value: string | null, options: DataAttributeCommitOptions) => { + if (!domEditSelection) return; + const iframe = previewIframeRef.current; + const fullAttr = resolveFullAttrName(attr, true); + const commitKey = `${options.coalescePrefix}:${attr}:${getDomEditTargetKey(domEditSelection)}`; + const isLatestCommit = bumpDomEditCommitMapVersion( + domAttributeCommitVersionRef.current, + commitKey, + ); + const op: PatchOperation = { type: "attribute", property: attr, value }; + let editedElement: HTMLElement | null = null; + let previousValue: string | null = null; + + await runDomEditCommit({ + capture: () => { + const captured = captureAttributeElement( + iframe?.contentDocument, + domEditSelection, + activeCompPath, + fullAttr, + ); + if (!captured) return; + editedElement = captured.element; + previousValue = captured.previousValue; + }, + apply: () => { + if (!editedElement) return; + const nextValue = value === null || value === "" ? null : value; + setOrRemovePreviewAttribute(editedElement, fullAttr, nextValue); + }, + persist: () => + persistDomEditOperations(domEditSelection, [op], { + label: options.label, + coalesceKey: commitKey, + skipRefresh: options.skipRefresh, + }), + shouldRevert: () => isLatestCommit(), + revert: () => { + if (!editedElement) return; + setOrRemovePreviewAttribute(editedElement, fullAttr, previousValue); + }, + onError: (error) => reportDomEditPersistFailure(domEditSelection, [op], error, showToast), + shouldResync: () => isLatestCommit() && !!options.refreshAfter, + resync: () => refreshDomEditSelectionFromPreview(domEditSelection), + }); + }, + [ + activeCompPath, + domEditSelection, + persistDomEditOperations, + refreshDomEditSelectionFromPreview, + showToast, + previewIframeRef, + ], + ); + + const handleDomAttributeCommit = useCallback( + async (attr: string, value: string) => { + await commitDataAttribute(attr, value, { + label: `Edit ${attr.replace(/-/g, " ")}`, + coalescePrefix: "attr", + skipRefresh: false, + refreshAfter: true, + }); + }, + [commitDataAttribute], + ); + + const handleDomAttributeLiveCommit = useCallback( + async (attr: string, value: string | null) => { + await commitDataAttribute(attr, value, { + label: `Edit ${attr.replace(/^(data-)?/, "").replace(/-/g, " ")}`, + coalescePrefix: "attr-live", + skipRefresh: true, + }); + }, + [commitDataAttribute], + ); + + const handleDomHtmlAttributeCommit = useCallback( + async (attr: string, value: string | null) => { + if (!domEditSelection) return; + const iframe = previewIframeRef.current; + const commitKey = `html-attr:${attr}:${getDomEditTargetKey(domEditSelection)}`; + const isLatestCommit = bumpDomEditCommitMapVersion( + domAttributeCommitVersionRef.current, + commitKey, + ); + const op: PatchOperation = { type: "html-attribute", property: attr, value }; + let editedElement: HTMLElement | null = null; + let previousValue: string | null = null; + + await runDomEditCommit({ + capture: () => { + const captured = captureAttributeElement( + iframe?.contentDocument, + domEditSelection, + activeCompPath, + attr, + ); + if (!captured) return; + editedElement = captured.element; + previousValue = captured.previousValue; + }, + apply: () => { + if (!editedElement) return; + const nextValue = value === null || value === "false" ? null : value; + setOrRemovePreviewAttribute(editedElement, attr, nextValue); + }, + persist: () => + persistDomEditOperations(domEditSelection, [op], { + label: `Edit ${attr}`, + coalesceKey: commitKey, + skipRefresh: false, + }), + shouldRevert: () => isLatestCommit(), + revert: () => { + if (!editedElement) return; + setOrRemovePreviewAttribute(editedElement, attr, previousValue); + }, + onError: (error) => reportDomEditPersistFailure(domEditSelection, [op], error, showToast), + shouldResync: () => isLatestCommit(), + resync: () => refreshDomEditSelectionFromPreview(domEditSelection), + }); + }, + [ + activeCompPath, + domEditSelection, + persistDomEditOperations, + refreshDomEditSelectionFromPreview, + showToast, + previewIframeRef, + ], + ); + + return { + handleDomAttributeCommit, + handleDomAttributeLiveCommit, + handleDomHtmlAttributeCommit, + }; +} diff --git a/packages/studio/src/hooks/useDomEditCommits.ts b/packages/studio/src/hooks/useDomEditCommits.ts index 38ab0c0b9..3ae522f83 100644 --- a/packages/studio/src/hooks/useDomEditCommits.ts +++ b/packages/studio/src/hooks/useDomEditCommits.ts @@ -10,6 +10,11 @@ import { fontFamilyFromAssetPath, type ImportedFontAsset } from "../components/e import type { EditHistoryKind } from "../utils/editHistory"; import type { PersistDomEditOperations } from "./domEditCommitTypes"; import type { PatchOperation } from "../utils/sourcePatcher"; +import { + DomEditPersistUnsafeValueError, + DomEditPersistUnresolvableError, + warnDomEditPersistNoOp, +} from "./domEditPersistFailure"; import { useDomEditPositionPatchCommit } from "./useDomEditPositionPatchCommit"; import { useDomEditTextCommits } from "./useDomEditTextCommits"; import { useDomGeometryCommits } from "./useDomGeometryCommits"; @@ -22,6 +27,10 @@ function formatUnsafeFieldList(fields: Array<{ path: string }>): string { return fields.map((field) => field.path).join(", "); } +function getErrorDetail(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + async function readErrorResponseBody( response: Response, ): Promise<{ error?: string; fields?: string[] } | null> { @@ -168,7 +177,9 @@ export function useDomEditCommits({ if (unsafeFields.length > 0) { const fields = formatUnsafeFieldList(unsafeFields); showToast("Couldn't save edit because it contains invalid layout values", "error"); - throw new Error(`DOM patch contains unsafe values: ${fields}`); + throw new DomEditPersistUnsafeValueError(`DOM patch contains unsafe values: ${fields}`, { + alreadyToasted: true, + }); } // Skip the SDK path when prepareContent is set (e.g. @font-face injection @@ -203,7 +214,9 @@ export function useDomEditCommits({ ); if (!patchResponse.ok) { showToast(formatPatchRejectionMessage(await readErrorResponseBody(patchResponse)), "error"); - throw await createStudioSaveHttpError(patchResponse, `Failed to patch ${targetPath}`); + throw await createStudioSaveHttpError(patchResponse, `Failed to patch ${targetPath}`, { + alreadyToasted: true, + }); } const patchData = (await patchResponse.json()) as { @@ -225,7 +238,9 @@ export function useDomEditCommits({ composition: activeCompPath ?? undefined, }); } + throw new DomEditPersistUnresolvableError(targetPath); } + warnDomEditPersistNoOp(selection, operations); return; } @@ -234,9 +249,21 @@ export function useDomEditCommits({ let finalContent = patchedContent; if (options?.prepareContent) { - finalContent = options.prepareContent(patchedContent, targetPath); - if (finalContent !== patchedContent) { - await writeProjectFile(targetPath, finalContent); + const preparedContent = options.prepareContent(patchedContent, targetPath); + if (preparedContent !== patchedContent) { + try { + await writeProjectFile(targetPath, preparedContent); + finalContent = preparedContent; + } catch (error) { + // The patch above already landed on disk — only the prepareContent + // embellishment (e.g. an injected @font-face) failed to write. Keep + // the already-persisted patchedContent instead of throwing, which + // would otherwise revert a change the server already committed. + showToast( + `Saved, but couldn't finish updating ${targetPath}: ${getErrorDetail(error)}`, + "error", + ); + } } } @@ -286,6 +313,7 @@ export function useDomEditCommits({ buildDomSelectionFromTarget, persistDomEditOperations, resolveImportedFontAsset, + showToast, }); // ── Position patch helper (shared by geometry + lifecycle hooks) ── diff --git a/packages/studio/src/hooks/useDomEditTextCommits.ts b/packages/studio/src/hooks/useDomEditTextCommits.ts index 1d3159f22..36061f1a1 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.ts +++ b/packages/studio/src/hooks/useDomEditTextCommits.ts @@ -23,12 +23,24 @@ import { } from "../components/editor/domEditing"; import type { ImportedFontAsset } from "../components/editor/fontAssets"; import type { PersistDomEditOperations } from "./domEditCommitTypes"; +import { buildTextFieldChildOperations } from "./domEditTextFieldCommitOps"; +import { + DomEditPersistUnsupportedTextStructureError, + reportDomEditPersistFailure, +} from "./domEditPersistFailure"; +import { + bumpDomEditCommitMapVersion, + bumpDomEditCommitVersion, + runDomEditCommit, +} from "./domEditCommitRunner"; +import { useDomEditAttributeCommits } from "./useDomEditAttributeCommits"; // ── Types ── export interface UseDomEditTextCommitsParams { activeCompPath: string | null; previewIframeRef: React.MutableRefObject; + showToast: (message: string, tone?: "error" | "info") => void; domEditSelection: DomEditSelection | null; applyDomSelection: ( selection: DomEditSelection | null, @@ -43,31 +55,78 @@ export interface UseDomEditTextCommitsParams { resolveImportedFontAsset: (fontFamilyValue: string) => ImportedFontAsset | null; } -function applyPreviewAttribute( +interface DomTextCommitPlan { + usesSerializedTextFields: boolean; + nextContent: string; + childOperations: PatchOperation[] | null; + operations: PatchOperation[]; +} + +function buildDomStyleCommitOperations( + property: string, + value: string, + isImageBackgroundCommit: boolean, +): PatchOperation[] { + const operations: PatchOperation[] = [ + buildDomEditStylePatchOperation(property, normalizeDomEditStyleValue(property, value)), + ]; + if (isImageBackgroundCommit) { + operations.push( + buildDomEditStylePatchOperation("background-position", "center"), + buildDomEditStylePatchOperation("background-repeat", "no-repeat"), + buildDomEditStylePatchOperation("background-size", "contain"), + ); + } + return operations; +} + +function buildNextDomTextFields( + textFields: DomEditTextField[], + value: string, + fieldKey?: string, +): DomEditTextField[] { + if (textFields.length === 0) return []; + return textFields.map((field) => (field.key === fieldKey ? { ...field, value } : field)); +} + +function planDomTextCommit( + originalTextFields: DomEditTextField[], + nextTextFields: DomEditTextField[], + plainTextContent: string, +): DomTextCommitPlan { + const usesSerializedTextFields = + nextTextFields.length > 1 || nextTextFields.some((field) => field.source === "child"); + const nextContent = usesSerializedTextFields + ? serializeDomEditTextFields(nextTextFields) + : plainTextContent; + const childOperations = usesSerializedTextFields + ? buildTextFieldChildOperations(originalTextFields, nextTextFields) + : null; + const operations = + childOperations ?? + (usesSerializedTextFields ? [] : [buildDomEditTextPatchOperation(nextContent)]); + + return { + usesSerializedTextFields, + nextContent, + childOperations, + operations, + }; +} + +async function resyncDomTextSelectionFromPreview( doc: Document | null | undefined, selection: DomEditSelection, activeCompPath: string | null, - attr: string, - value: string | null, - options: { prefixData?: boolean; removeFalse?: boolean } = {}, -): void { + buildDomSelectionFromTarget: UseDomEditTextCommitsParams["buildDomSelectionFromTarget"], + applyDomSelection: UseDomEditTextCommitsParams["applyDomSelection"], +): Promise { if (!doc) return; - const el = findElementForSelection(doc, selection, activeCompPath); - if (!el) return; - const fullAttr = options.prefixData && !attr.startsWith("data-") ? `data-${attr}` : attr; - if (value === null || value === "" || (options.removeFalse && value === "false")) { - el.removeAttribute(fullAttr); - } else { - el.setAttribute(fullAttr, value); - } -} - -interface DataAttributeCommitOptions { - label: string; - coalescePrefix: string; - skipRefresh: boolean; - warningMessage: string; - refreshAfter?: boolean; + const refreshed = findElementForSelection(doc, selection, activeCompPath); + if (!refreshed) return; + const nextSelection = await buildDomSelectionFromTarget(refreshed); + if (!nextSelection) return; + applyDomSelection(nextSelection, { revealPanel: false, preserveGroup: true }); } // ── Hook ── @@ -75,6 +134,7 @@ interface DataAttributeCommitOptions { export function useDomEditTextCommits({ activeCompPath, previewIframeRef, + showToast, domEditSelection, applyDomSelection, refreshDomEditSelectionFromPreview, @@ -83,51 +143,83 @@ export function useDomEditTextCommits({ resolveImportedFontAsset, }: UseDomEditTextCommitsParams) { const domTextCommitVersionRef = useRef(0); + const domStyleCommitVersionRef = useRef(new Map()); + + const { handleDomAttributeCommit, handleDomAttributeLiveCommit, handleDomHtmlAttributeCommit } = + useDomEditAttributeCommits({ + activeCompPath, + previewIframeRef, + showToast, + domEditSelection, + refreshDomEditSelectionFromPreview, + persistDomEditOperations, + }); const handleDomStyleCommit = useCallback( async (property: string, value: string) => { if (!domEditSelection) return; if (isManualGeometryStyleProperty(property)) return; if (!domEditSelection.capabilities.canEditStyles) return; + const styleCommitKey = `${getDomEditTargetKey(domEditSelection)}:${property}`; + const isLatestStyleCommit = bumpDomEditCommitMapVersion( + domStyleCommitVersionRef.current, + styleCommitKey, + ); const importedFont = property === "font-family" ? resolveImportedFontAsset(value) : null; const iframe = previewIframeRef.current; const doc = iframe?.contentDocument; - if (doc) { - const el = findElementForSelection(doc, domEditSelection, activeCompPath); - if (el) { - el.style.setProperty(property, normalizeDomEditStyleValue(property, value)); - if (property === "font-family") { + const normalizedValue = normalizeDomEditStyleValue(property, value); + const isImageBackgroundCommit = + property === "background-image" && isImageBackgroundValue(value); + let editedElement: HTMLElement | null = null; + let previousInlineValue: string | null = null; + const operations = buildDomStyleCommitOperations(property, value, isImageBackgroundCommit); + const skipRefresh = property !== "z-index"; + + await runDomEditCommit({ + capture: () => { + if (!doc) return; + const el = findElementForSelection(doc, domEditSelection, activeCompPath); + if (!el) return; + editedElement = el; + previousInlineValue = el.style.getPropertyValue(property); + }, + apply: () => { + if (!editedElement) return; + editedElement.style.setProperty(property, normalizedValue); + if (property === "font-family" && doc) { injectPreviewGoogleFont(doc, value); if (importedFont) injectPreviewImportedFont(doc, importedFont); } - if (property === "background-image" && isImageBackgroundValue(value)) { - el.style.setProperty("background-position", "center"); - el.style.setProperty("background-repeat", "no-repeat"); - el.style.setProperty("background-size", "contain"); + if (isImageBackgroundCommit) { + editedElement.style.setProperty("background-position", "center"); + editedElement.style.setProperty("background-repeat", "no-repeat"); + editedElement.style.setProperty("background-size", "contain"); } - } - } - const operations: PatchOperation[] = [ - buildDomEditStylePatchOperation(property, normalizeDomEditStyleValue(property, value)), - ]; - if (property === "background-image" && isImageBackgroundValue(value)) { - operations.push( - buildDomEditStylePatchOperation("background-position", "center"), - buildDomEditStylePatchOperation("background-repeat", "no-repeat"), - buildDomEditStylePatchOperation("background-size", "contain"), - ); - } - const skipRefresh = property !== "z-index"; - try { - await persistDomEditOperations(domEditSelection, operations, { - label: "Edit layer style", - skipRefresh, - prepareContent: importedFont - ? (html, sourceFile) => ensureImportedFontFace(html, importedFont, sourceFile) - : undefined, - }); - } catch {} - refreshDomEditSelectionFromPreview(domEditSelection); + }, + persist: () => + persistDomEditOperations(domEditSelection, operations, { + label: "Edit layer style", + skipRefresh, + prepareContent: importedFont + ? (html, sourceFile) => ensureImportedFontFace(html, importedFont, sourceFile) + : undefined, + }), + shouldRevert: () => isLatestStyleCommit(), + revert: () => { + if (!editedElement || previousInlineValue === null) return; + // ponytail: background-image side-effect styles are not reverted here. + if (previousInlineValue === "") { + editedElement.style.removeProperty(property); + } else { + editedElement.style.setProperty(property, previousInlineValue); + } + }, + onError: (error) => + reportDomEditPersistFailure(domEditSelection, operations, error, showToast), + shouldResync: isLatestStyleCommit, + resync: () => refreshDomEditSelectionFromPreview(domEditSelection), + }); }, [ activeCompPath, @@ -135,99 +227,7 @@ export function useDomEditTextCommits({ persistDomEditOperations, refreshDomEditSelectionFromPreview, resolveImportedFontAsset, - previewIframeRef, - ], - ); - - const commitDataAttribute = useCallback( - async (attr: string, value: string | null, options: DataAttributeCommitOptions) => { - if (!domEditSelection) return; - const iframe = previewIframeRef.current; - applyPreviewAttribute( - iframe?.contentDocument, - domEditSelection, - activeCompPath, - attr, - value, - { - prefixData: true, - }, - ); - const op: PatchOperation = { type: "attribute", property: attr, value }; - try { - await persistDomEditOperations(domEditSelection, [op], { - label: options.label, - coalesceKey: `${options.coalescePrefix}:${attr}:${getDomEditTargetKey(domEditSelection)}`, - skipRefresh: options.skipRefresh, - }); - } catch {} - if (options.refreshAfter) { - refreshDomEditSelectionFromPreview(domEditSelection); - } - }, - [ - activeCompPath, - domEditSelection, - persistDomEditOperations, - refreshDomEditSelectionFromPreview, - previewIframeRef, - ], - ); - - const handleDomAttributeCommit = useCallback( - async (attr: string, value: string) => { - await commitDataAttribute(attr, value, { - label: `Edit ${attr.replace(/-/g, " ")}`, - coalescePrefix: "attr", - skipRefresh: false, - warningMessage: "[Studio] Attribute persist failed:", - refreshAfter: true, - }); - }, - [commitDataAttribute], - ); - - const handleDomAttributeLiveCommit = useCallback( - async (attr: string, value: string | null) => { - await commitDataAttribute(attr, value, { - label: `Edit ${attr.replace(/^(data-)?/, "").replace(/-/g, " ")}`, - coalescePrefix: "attr-live", - skipRefresh: true, - warningMessage: "[Studio] Live attribute persist failed:", - }); - }, - [commitDataAttribute], - ); - - const handleDomHtmlAttributeCommit = useCallback( - async (attr: string, value: string | null) => { - if (!domEditSelection) return; - const iframe = previewIframeRef.current; - applyPreviewAttribute( - iframe?.contentDocument, - domEditSelection, - activeCompPath, - attr, - value, - { - removeFalse: true, - }, - ); - const op: PatchOperation = { type: "html-attribute", property: attr, value }; - try { - await persistDomEditOperations(domEditSelection, [op], { - label: `Edit ${attr}`, - coalesceKey: `html-attr:${attr}:${getDomEditTargetKey(domEditSelection)}`, - skipRefresh: false, - }); - } catch {} - refreshDomEditSelectionFromPreview(domEditSelection); - }, - [ - activeCompPath, - domEditSelection, - persistDomEditOperations, - refreshDomEditSelectionFromPreview, + showToast, previewIframeRef, ], ); @@ -236,53 +236,57 @@ export function useDomEditTextCommits({ async (value: string, fieldKey?: string) => { if (!domEditSelection) return; if (!isTextEditableSelection(domEditSelection)) return; - const commitVersion = domTextCommitVersionRef.current + 1; - domTextCommitVersionRef.current = commitVersion; - const nextTextFields = - domEditSelection.textFields.length > 0 - ? domEditSelection.textFields.map((field) => - field.key === fieldKey ? { ...field, value } : field, - ) - : []; - const nextContent = - nextTextFields.length > 1 || nextTextFields.some((field) => field.source === "child") - ? serializeDomEditTextFields(nextTextFields) - : value; + const isLatestTextCommit = bumpDomEditCommitVersion(domTextCommitVersionRef); + const nextTextFields = buildNextDomTextFields(domEditSelection.textFields, value, fieldKey); + const textCommit = planDomTextCommit(domEditSelection.textFields, nextTextFields, value); const iframe = previewIframeRef.current; const doc = iframe?.contentDocument; - if (doc) { - const el = findElementForSelection(doc, domEditSelection, activeCompPath); - if (el) { - if ( - nextTextFields.length > 1 || - nextTextFields.some((field) => field.source === "child") - ) { - el.innerHTML = nextContent; - } else { - el.textContent = value; - } - } - } - await persistDomEditOperations( - domEditSelection, - [buildDomEditTextPatchOperation(nextContent)], - { - label: "Edit text", - skipRefresh: true, - shouldSave: () => domTextCommitVersionRef.current === commitVersion, - }, - ); - if (domTextCommitVersionRef.current !== commitVersion) return; + let editedElement: HTMLElement | null = null; + let previousInnerHtml: string | null = null; - if (doc) { - const refreshed = findElementForSelection(doc, domEditSelection, activeCompPath); - if (refreshed) { - const nextSelection = await buildDomSelectionFromTarget(refreshed); - if (nextSelection) { - applyDomSelection(nextSelection, { revealPanel: false, preserveGroup: true }); + await runDomEditCommit({ + capture: () => { + if (!doc) return; + const el = findElementForSelection(doc, domEditSelection, activeCompPath); + if (!el) return; + editedElement = el; + previousInnerHtml = el.innerHTML; + }, + apply: () => { + if (!editedElement) return; + if (textCommit.usesSerializedTextFields) { + editedElement.innerHTML = textCommit.nextContent; + } else { + editedElement.textContent = value; } - } - } + }, + persist: async () => { + if (textCommit.usesSerializedTextFields && textCommit.childOperations === null) { + throw new DomEditPersistUnsupportedTextStructureError(); + } + await persistDomEditOperations(domEditSelection, textCommit.operations, { + label: "Edit text", + skipRefresh: true, + shouldSave: isLatestTextCommit, + }); + }, + shouldRevert: () => isLatestTextCommit(), + revert: () => { + if (!editedElement || previousInnerHtml === null) return; + editedElement.innerHTML = previousInnerHtml; + }, + onError: (error) => + reportDomEditPersistFailure(domEditSelection, textCommit.operations, error, showToast), + shouldResync: isLatestTextCommit, + resync: () => + resyncDomTextSelectionFromPreview( + doc, + domEditSelection, + activeCompPath, + buildDomSelectionFromTarget, + applyDomSelection, + ), + }); }, [ activeCompPath, @@ -291,6 +295,7 @@ export function useDomEditTextCommits({ domEditSelection, persistDomEditOperations, previewIframeRef, + showToast, ], ); @@ -300,45 +305,62 @@ export function useDomEditTextCommits({ nextTextFields: DomEditTextField[], options?: { importedFont?: ImportedFontAsset | null }, ) => { - const nextContent = - nextTextFields.length > 1 || nextTextFields.some((field) => field.source === "child") - ? serializeDomEditTextFields(nextTextFields) - : (nextTextFields[0]?.value ?? ""); - + const textCommit = planDomTextCommit( + selection.textFields, + nextTextFields, + nextTextFields[0]?.value ?? "", + ); const iframe = previewIframeRef.current; const doc = iframe?.contentDocument; - if (doc) { - const el = findElementForSelection(doc, selection, activeCompPath); - if (el) { - if ( - nextTextFields.length > 1 || - nextTextFields.some((field) => field.source === "child") - ) { - el.innerHTML = nextContent; - } else { - el.textContent = nextContent; - } - } - } - + let editedElement: HTMLElement | null = null; + let previousInnerHtml: string | null = null; const importedFont = options?.importedFont ?? null; - await persistDomEditOperations(selection, [buildDomEditTextPatchOperation(nextContent)], { - label: "Edit text", - skipRefresh: true, - prepareContent: importedFont - ? (html, sourceFile) => ensureImportedFontFace(html, importedFont, sourceFile) - : undefined, - }); - if (doc) { - const refreshed = findElementForSelection(doc, selection, activeCompPath); - if (refreshed) { - const nextSelection = await buildDomSelectionFromTarget(refreshed); - if (nextSelection) { - applyDomSelection(nextSelection, { revealPanel: false, preserveGroup: true }); + await runDomEditCommit({ + capture: () => { + if (!doc) return; + const el = findElementForSelection(doc, selection, activeCompPath); + if (!el) return; + editedElement = el; + previousInnerHtml = el.innerHTML; + }, + apply: () => { + if (!editedElement) return; + if (textCommit.usesSerializedTextFields) { + editedElement.innerHTML = textCommit.nextContent; + } else { + editedElement.textContent = textCommit.nextContent; } - } - } + }, + persist: async () => { + if (textCommit.usesSerializedTextFields && textCommit.childOperations === null) { + throw new DomEditPersistUnsupportedTextStructureError(); + } + await persistDomEditOperations(selection, textCommit.operations, { + label: "Edit text", + skipRefresh: true, + prepareContent: importedFont + ? (html, sourceFile) => ensureImportedFontFace(html, importedFont, sourceFile) + : undefined, + }); + }, + shouldRevert: () => true, + revert: () => { + if (!editedElement || previousInnerHtml === null) return; + editedElement.innerHTML = previousInnerHtml; + }, + onError: (error) => + reportDomEditPersistFailure(selection, textCommit.operations, error, showToast), + shouldResync: () => true, + resync: () => + resyncDomTextSelectionFromPreview( + doc, + selection, + activeCompPath, + buildDomSelectionFromTarget, + applyDomSelection, + ), + }); }, [ activeCompPath, @@ -346,6 +368,7 @@ export function useDomEditTextCommits({ buildDomSelectionFromTarget, persistDomEditOperations, previewIframeRef, + showToast, ], ); diff --git a/packages/studio/src/utils/studioSaveDiagnostics.ts b/packages/studio/src/utils/studioSaveDiagnostics.ts index 8f4f34367..9424aba81 100644 --- a/packages/studio/src/utils/studioSaveDiagnostics.ts +++ b/packages/studio/src/utils/studioSaveDiagnostics.ts @@ -18,11 +18,13 @@ export interface StudioSaveFailureInput { export class StudioSaveHttpError extends Error { readonly statusCode: number; + readonly alreadyToasted: boolean; - constructor(message: string, statusCode: number) { + constructor(message: string, statusCode: number, options: { alreadyToasted?: boolean } = {}) { super(message); this.name = "StudioSaveHttpError"; this.statusCode = statusCode; + this.alreadyToasted = options.alreadyToasted ?? false; } } @@ -130,6 +132,7 @@ export function trackStudioSaveFailure(input: StudioSaveFailureInput): void { export async function createStudioSaveHttpError( response: Response, fallbackMessage: string, + options: { alreadyToasted?: boolean } = {}, ): Promise { let body = ""; try { @@ -141,7 +144,7 @@ export async function createStudioSaveHttpError( const message = detail ? `${fallbackMessage} (${response.status}): ${detail}` : `${fallbackMessage} (${response.status})`; - return new StudioSaveHttpError(message, response.status); + return new StudioSaveHttpError(message, response.status, options); } export async function retryStudioSave(