fix(studio): a deselect made while an edit saves stays deselected (#5907)

* fix(studio): a deselect made while an edit saves stays deselected

A save resyncs the selection it started with once the write returns. It now
does so only while that element is still the one selected, so a deselect or
a different pick made during the save is no longer undone. The preview-reload
sync now routes through the same resync instead of a copy of it.

* refactor(studio): drop two comments the names already say
This commit is contained in:
Miguel Ángel
2026-10-19 20:51:57 +00:00
committed by GitHub
parent 501b2d29c2
commit 4ff4284442
8 changed files with 93 additions and 96 deletions
@@ -2,7 +2,7 @@
import React, { act } from "react";
import { createRoot } from "react-dom/client";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { DomEditSelection } from "../components/editor/domEditing";
import { findElementForSelection, type DomEditSelection } from "../components/editor/domEditing";
import { resolveElementForOverlay } from "../components/editor/domEditOverlayGeometry";
import {
makePreview,
@@ -44,14 +44,13 @@ function mountHost(followPromotions: boolean) {
const live = useLivePreviewIframe(host);
useDomEditPreviewSync({
previewIframe: followPromotions ? live : host,
activeCompPath: null,
captionEditMode: false,
domEditSelectionRef: selectionRef,
domEditGroupSelectionsRef: { current: [] },
domEditSelection: selectionRef.current,
refreshDomEditGroupSelectionsFromPreview: async () => {},
applyDomSelection: (selection) => void (selectionRef.current = selection),
buildDomSelectionFromTarget: async (element) => selectionOf(element),
refreshDomEditSelectionFromPreview: async (selection, iframe) => {
const element = findElementForSelection(iframe.contentDocument!, selection, null);
selectionRef.current = element ? selectionOf(element) : null;
},
refreshPreviewDocumentVersion: () => {},
syncPreviewHotkeys: () => {},
applyStudioManualEditsToPreviewRef: { current: async () => {} },
@@ -25,14 +25,10 @@ describe("useDomEditPreviewSync", () => {
function Harness() {
useDomEditPreviewSync({
previewIframe: iframe,
activeCompPath: null,
captionEditMode: false,
domEditSelectionRef: { current: null },
domEditGroupSelectionsRef: { current: [] },
domEditSelection: null,
refreshDomEditGroupSelectionsFromPreview: async () => {},
applyDomSelection: () => {},
buildDomSelectionFromTarget: async () => null,
refreshDomEditSelectionFromPreview: async () => {},
refreshPreviewDocumentVersion: refreshVersion,
syncPreviewHotkeys: () => {},
applyStudioManualEditsToPreviewRef: { current: applyManualEdits },
@@ -4,31 +4,22 @@
* Extracted from useDomEditSession to keep file sizes under the 600-line limit.
*/
import { useEffect, useRef } from "react";
import { findElementForSelection, type DomEditSelection } from "../components/editor/domEditing";
import type { DomEditSelection } from "../components/editor/domEditing";
import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits";
import { useDockLayoutStore } from "../components/dock/dockLayoutStore";
import type { PatchTarget } from "../utils/sourcePatcher";
import { logSelect } from "../utils/selectDebug";
import { onPreviewContentReplaced } from "../player/sceneSwap";
import type { ResolveDomSelectionOptions } from "./useDomSelectionTypes";
interface UseDomEditPreviewSyncParams {
previewIframe: HTMLIFrameElement | null;
activeCompPath: string | null;
captionEditMode: boolean;
domEditSelectionRef: React.MutableRefObject<DomEditSelection | null>;
domEditGroupSelectionsRef: React.MutableRefObject<DomEditSelection[]>;
domEditSelection: DomEditSelection | null;
/** Re-resolves a whole multi-selection against the current preview document. */
refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise<void>;
applyDomSelection: (
selection: DomEditSelection | null,
options?: { revealPanel?: boolean; preserveGroup?: boolean },
) => void;
buildDomSelectionFromTarget: (
element: HTMLElement,
options?: Pick<ResolveDomSelectionOptions, "previous">,
) => Promise<DomEditSelection | null>;
/** Re-resolves the selection against the given preview, unless it was deselected meanwhile. */
refreshDomEditSelectionFromPreview: (
selection: DomEditSelection,
iframe: HTMLIFrameElement,
) => Promise<void>;
refreshPreviewDocumentVersion: () => void;
syncPreviewHotkeys: (iframe: HTMLIFrameElement | null) => void;
applyStudioManualEditsToPreviewRef: React.MutableRefObject<
@@ -40,14 +31,10 @@ interface UseDomEditPreviewSyncParams {
export function useDomEditPreviewSync({
previewIframe,
activeCompPath,
captionEditMode,
domEditSelectionRef,
domEditGroupSelectionsRef,
domEditSelection,
applyDomSelection,
refreshDomEditGroupSelectionsFromPreview,
buildDomSelectionFromTarget,
refreshDomEditSelectionFromPreview,
refreshPreviewDocumentVersion,
syncPreviewHotkeys,
applyStudioManualEditsToPreviewRef,
@@ -73,39 +60,7 @@ export function useDomEditPreviewSync({
if (!doc) return;
reapplyPositionEditsAfterSeek(doc);
const nextElement = findElementForSelection(doc, currentSelection, activeCompPath);
if (!nextElement) {
// The selected element no longer resolves in the (re-synced) document
// — comp/hot reload, activeCompPath swap, or post-save replacement.
// Clear so overlay geometry isn't computed on a stale, detached node.
// (Drag-release-in-gray-zone is handled separately by
// suppressNextBoxClickRef; the dragged element still resolves here.)
//
// One lost member is not the whole selection though. A multi-select that
// loses its primary here used to be wiped entirely, so moving a group and
// having any one of its elements fail to re-resolve deselected all of
// them. Re-resolve the group instead and keep whoever survived; it only
// clears when nobody did.
const group = domEditGroupSelectionsRef.current;
logSelect("preview-sync-lost", {
target: currentSelection.selector ?? currentSelection.id ?? null,
group: group.length,
});
if (group.length > 1) {
await refreshDomEditGroupSelectionsFromPreview(group);
return;
}
applyDomSelection(null, { revealPanel: false });
return;
}
const nextSelection = await buildDomSelectionFromTarget(nextElement, {
previous: currentSelection,
});
if (nextSelection) {
applyDomSelection(nextSelection, { revealPanel: false, preserveGroup: true });
}
await refreshDomEditSelectionFromPreview(currentSelection, previewIframe);
};
syncPreviewHotkeys(previewIframe);
@@ -122,14 +77,10 @@ export function useDomEditPreviewSync({
return onPreviewContentReplaced(previewIframe, handleLoad);
}, [
activeCompPath,
applyDomSelection,
buildDomSelectionFromTarget,
captionEditMode,
domEditGroupSelectionsRef,
domEditSelectionRef,
previewIframe,
refreshDomEditGroupSelectionsFromPreview,
refreshDomEditSelectionFromPreview,
refreshPreviewDocumentVersion,
syncPreviewHotkeys,
applyStudioManualEditsToPreviewRef,
@@ -122,7 +122,6 @@ export function useDomEditSession({
buildDomSelectionForTimelineElement,
handleTimelineElementSelect,
refreshDomEditSelectionFromPreview,
refreshDomEditGroupSelectionsFromPreview,
applyMarqueeSelection,
} = useDomSelection({
projectId,
@@ -394,8 +393,7 @@ export function useDomEditSession({
activeCompPath,
domEditSelection,
domEditSelectionRef,
domEditGroupSelectionsRef,
refreshDomEditGroupSelectionsFromPreview,
refreshDomEditSelectionFromPreview,
previewIframeRef,
previewIframe,
captionEditMode: previewCaptionEditMode,
@@ -406,8 +404,6 @@ export function useDomEditSession({
refreshPreviewDocumentVersion,
syncPreviewHotkeys,
applyStudioManualEditsToPreviewRef,
applyDomSelection,
buildDomSelectionFromTarget,
openSourceForSelection,
...gsapScriptEdits,
handleDomManualEditsReset,
+6 -16
View File
@@ -23,8 +23,10 @@ export interface UseDomEditWiringParams {
activeCompPath: string | null;
domEditSelection: DomEditSelection | null;
domEditSelectionRef: React.MutableRefObject<DomEditSelection | null>;
domEditGroupSelectionsRef: React.MutableRefObject<DomEditSelection[]>;
refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise<void>;
refreshDomEditSelectionFromPreview: (
selection: DomEditSelection,
iframe: HTMLIFrameElement,
) => Promise<void>;
previewIframeRef: React.RefObject<HTMLIFrameElement | null>;
previewIframe: HTMLIFrameElement | null;
captionEditMode: boolean;
@@ -37,11 +39,6 @@ export interface UseDomEditWiringParams {
applyStudioManualEditsToPreviewRef: React.MutableRefObject<
(iframe: HTMLIFrameElement) => Promise<void>
>;
applyDomSelection: (
selection: DomEditSelection | null,
options?: { revealPanel?: boolean; preserveGroup?: boolean },
) => void;
buildDomSelectionFromTarget: (element: HTMLElement) => Promise<DomEditSelection | null>;
openSourceForSelection?: (sourceFile: string, target: PatchTarget) => void;
// GSAP script commit ops (from useGsapScriptCommits)
updateGsapProperty: (
@@ -117,8 +114,7 @@ export function useDomEditWiring(params: UseDomEditWiringParams) {
activeCompPath,
domEditSelection,
domEditSelectionRef,
domEditGroupSelectionsRef,
refreshDomEditGroupSelectionsFromPreview,
refreshDomEditSelectionFromPreview,
previewIframeRef,
previewIframe,
captionEditMode,
@@ -129,8 +125,6 @@ export function useDomEditWiring(params: UseDomEditWiringParams) {
refreshPreviewDocumentVersion,
syncPreviewHotkeys,
applyStudioManualEditsToPreviewRef,
applyDomSelection,
buildDomSelectionFromTarget,
openSourceForSelection,
updateGsapProperty,
updateGsapMeta,
@@ -251,14 +245,10 @@ export function useDomEditWiring(params: UseDomEditWiringParams) {
useDomEditPreviewSync({
previewIframe,
activeCompPath,
captionEditMode,
domEditSelectionRef,
domEditGroupSelectionsRef,
domEditSelection,
applyDomSelection,
refreshDomEditGroupSelectionsFromPreview,
buildDomSelectionFromTarget,
refreshDomEditSelectionFromPreview,
refreshPreviewDocumentVersion,
syncPreviewHotkeys,
applyStudioManualEditsToPreviewRef,
@@ -326,6 +326,68 @@ describe("useDomSelection", () => {
harness.cleanup();
});
describe("a save's resync after the user moved on", () => {
function selectHeadlineInPreview() {
const doc = document.implementation.createHTMLDocument("");
const element = doc.body.appendChild(
Object.assign(doc.createElement("div"), { id: "headline" }),
);
const headline = makeSelection("Headline", element);
const harness = renderHarness(
{ activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 },
{ previewDoc: doc },
);
act(() => harness.current().applyDomSelection(headline));
return { headline, harness };
}
it("keeps a deselect made while the save was in flight", async () => {
const { headline, harness } = selectHeadlineInPreview();
// Escape after a crop's release, before its save returns.
act(() => harness.current().applyDomSelection(null, { revealPanel: false }));
await act(() => harness.current().refreshDomEditSelectionFromPreview(headline));
expect(harness.current().domEditSelection).toBeNull();
harness.cleanup();
});
it.each([
["still in", false],
["gone from", true],
])(
"keeps a different pick made while the save was in flight, the saved element %s the preview",
async (_, removed) => {
const { headline, harness } = selectHeadlineInPreview();
const card = makeSelection(
"Card",
Object.assign(document.createElement("div"), { id: "card" }),
);
act(() => harness.current().applyDomSelection(card));
if (removed) headline.element.remove();
await act(() => harness.current().refreshDomEditSelectionFromPreview(headline));
expect(harness.current().domEditSelection).toBe(card);
harness.cleanup();
},
);
it("keeps a deselect made while the resync was rebuilding the selection", async () => {
const { headline, harness } = selectHeadlineInPreview();
let resync!: Promise<void>;
act(() => {
resync = harness.current().refreshDomEditSelectionFromPreview(headline);
});
act(() => harness.current().applyDomSelection(null, { revealPanel: false }));
await act(() => resync);
expect(harness.current().domEditSelection).toBeNull();
harness.cleanup();
});
});
it("clears a committed selection when the active composition path changes", () => {
const { selection, harness } = setupSelectedHarness();
expect(harness.current().domEditSelection).toBe(selection);
+5 -4
View File
@@ -356,8 +356,10 @@ export function useDomSelection({
const refreshDomEditSelectionFromPreview = useCallback(
// fallow-ignore-next-line complexity
async (selection: DomEditSelection) => {
const iframe = previewIframeRef.current;
async (selection: DomEditSelection, iframe = previewIframeRef.current) => {
const stillSelected = () =>
domEditSelectionsTargetSame(domEditSelectionRef.current, selection);
if (!stillSelected()) return;
let doc: Document | null = null;
try {
doc = iframe?.contentDocument ?? null;
@@ -384,7 +386,7 @@ export function useDomSelection({
}
const nextSelection = await buildDomSelectionFromTarget(element, { previous: selection });
if (nextSelection) {
if (nextSelection && stillSelected()) {
applyDomSelection(nextSelection, {
revealPanel: false,
preserveGroup: true,
@@ -547,7 +549,6 @@ export function useDomSelection({
buildDomSelectionForTimelineElement,
handleTimelineElementSelect,
refreshDomEditSelectionFromPreview,
refreshDomEditGroupSelectionsFromPreview,
applyMarqueeSelection,
};
}
@@ -89,7 +89,9 @@ export interface UseDomSelectionReturn {
element: TimelineElement,
) => Promise<DomEditSelection | null>;
handleTimelineElementSelect: (element: TimelineElement | null) => Promise<void>;
refreshDomEditSelectionFromPreview: (selection: DomEditSelection) => Promise<void>;
refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise<void>;
refreshDomEditSelectionFromPreview: (
selection: DomEditSelection,
iframe?: HTMLIFrameElement | null,
) => Promise<void>;
applyMarqueeSelection: (selections: DomEditSelection[], additive: boolean) => DomSelectionResult;
}