fix(studio): an undo or redo counts as a preview change from its key, not from its reload (#6683)

* fix(studio): an undo or redo counts as a preview change from its key, not from its reload

* refactor(studio): count a history step from its key with the page flag on noteIdle

packages/studio/src/hooks/useEditHistoryActions.test.tsx

@@ -4,6 +4,7 @@ import { act, createElement } from "react";
 import { createRoot, type Root } from "react-dom/client";
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { STUDIO_MOTION_PATH } from "../components/editor/studioMotion";
+import { isPreviewChanging } from "../player/previewReloading";
 import { useEditHistoryActions, type EditHistoryHandle } from "./useEditHistoryActions";
 import {
   beginStudioPendingEdit,
@@ -73,6 +74,30 @@ const PREDICTED = { id: "e1", files: { "index.html": { previous: "B", restored:
 const SERVER_FILES = { "index.html": { previous: "B", restored: "A2" } };
 
 describe("useEditHistoryActions", () => {
+  it("counts an undo as a preview change from its key until its preview update is applied", async () => {
+    const files = { "index.html": { previous: "B", restored: "A" } };
+    const stepped = { ok: true, label: "Undid: Move", paths: ["index.html"], files };
+    const { deps, actions } = mount(stepped);
+    let answer: () => void = () => {};
+    deps.editHistory.undo.mockImplementationOnce(
+      () => new Promise((resolve) => (answer = () => resolve(stepped))),
+    );
+    let applied: () => void = () => {};
+    deps.syncHistoryPreviewAfterApply.mockImplementationOnce(
+      () => new Promise<undefined>((resolve) => (applied = () => resolve(undefined))),
+    );
+    const undone = actions.undo();
+    expect(isPreviewChanging(), "from the key, before the server answers").toBe(true);
+    await vi.waitFor(() => expect(deps.editHistory.undo).toHaveBeenCalled());
+    expect(isPreviewChanging(), "while the server has the step").toBe(true);
+    answer();
+    await vi.waitFor(() => expect(deps.syncHistoryPreviewAfterApply).toHaveBeenCalled());
+    expect(isPreviewChanging(), "until the preview update is applied").toBe(true);
+    applied();
+    await act(() => undone);
+    expect(isPreviewChanging()).toBe(false);
+  });
+
   it("takes back a canvas press still waiting to run, with no history step", async () => {
     const { deps, actions } = mount({ ok: true, label: "Undid: Move" });
     const cancel = vi.fn(() => (setStudioWaitingPressCancel(null), true));

packages/studio/src/hooks/useEditHistoryActions.ts

@@ -3,6 +3,7 @@ import { useCallback, useMemo } from "react";
 import { STUDIO_MOTION_PATH } from "../components/editor/studioMotion";
 import { serializeStudioFileMutations } from "../utils/studioFileMutationCoordinator";
 import type { RestoreFiles } from "../utils/gsapUndoRestore";
+import { whileScriptWrites } from "../player/previewReloading";
 import {
   beginStudioShownRestore,
   cancelNewestStudioWaitingPress,
@@ -152,8 +153,8 @@ export function useEditHistoryActions({
     ],
   );
 
-  const undo = useCallback(() => apply("undo"), [apply]);
-  const redo = useCallback(() => apply("redo"), [apply]);
+  const undo = useCallback(() => whileScriptWrites(() => apply("undo")), [apply]);
+  const redo = useCallback(() => whileScriptWrites(() => apply("redo")), [apply]);
   return useMemo(() => ({ undo, redo }), [undo, redo]);
 }
 

packages/studio/src/player/previewReloading.test.ts

@@ -1,3 +1,5 @@
+// @vitest-environment happy-dom
+
 import { afterEach, describe, expect, it } from "vitest";
 import {
   giveUpOnPreviewChange,
@@ -45,4 +47,19 @@ describe("the preview is changing", () => {
     requestPreviewReload();
     expect(isPreviewChanging(), "the next change counts again").toBe(true);
   });
+
+  it("is mirrored on the page for a tool that watches the preview from outside", async () => {
+    const shown = () => document.documentElement.hasAttribute("data-preview-changing");
+    expect(shown()).toBe(false);
+    requestPreviewReload();
+    expect(shown()).toBe(true);
+    previewReloadBegun();
+    expect(shown()).toBe(false);
+    let land: () => void = () => {};
+    const write = whileScriptWrites(() => new Promise<void>((resolve) => (land = resolve)));
+    expect(shown()).toBe(true);
+    land();
+    await write;
+    expect(shown()).toBe(false);
+  });
 });

packages/studio/src/player/previewReloading.ts

@@ -9,10 +9,15 @@ let givenUp = false;
 const changing = () => reloading || reloadRequested || scriptWrites > 0;
 const noteIdle = () => {
   if (!changing()) givenUp = false;
+  globalThis.document?.documentElement.toggleAttribute(
+    "data-preview-changing",
+    isPreviewChanging(),
+  );
 };
 
 export function giveUpOnPreviewChange(): void {
   givenUp = changing();
+  noteIdle();
 }
 
 export function setPreviewReloading(next: boolean): void {
@@ -24,6 +29,7 @@ export function setPreviewReloading(next: boolean): void {
 /** Studio asked for a reload that has not begun yet; `previewReloadBegun` closes that gap. */
 export function requestPreviewReload(): void {
   reloadRequested = true;
+  noteIdle();
 }
 
 export function previewReloadBegun(): void {
@@ -31,9 +37,10 @@ export function previewReloadBegun(): void {
   noteIdle();
 }
 
-/** A script write or scene swap counts from its send until its preview update is applied, or it fails. */
+/** A script write, scene swap or history step counts from its send until its preview update is applied, or it fails. */
 export async function whileScriptWrites<T>(write: () => Promise<T>): Promise<T> {
   scriptWrites += 1;
+  noteIdle();
   try {
     return await write();
   } finally {

packages/studio/tests/e2e/edit-accuracy/case.mjs

@@ -321,8 +321,16 @@ async function hiddenTarget(page, selector = "#target") {
   return false;
 }
 
+// Studio's own word that the preview is about to change: a reload asked for, loading, or an undo or redo in flight.
+const previewChanging = (page) =>
+  Promise.resolve()
+    .then(() => page.evaluate(() => document.documentElement.hasAttribute("data-preview-changing")))
+    .catch(() => false);
+
 // Keyframed cases only: waiting out the swap lands a later undo in the preview's burst of requests.
-export const swapPending = (ctx) => Boolean(ctx.keys) && hiddenTarget(ctx.page, ctx.selector);
+export const swapPending = async (ctx) =>
+  Boolean(ctx.keys) &&
+  ((await previewChanging(ctx.page)) || (await hiddenTarget(ctx.page, ctx.selector)));
 
 // fallow-ignore-next-line complexity
 const netMoved = (start, now) =>

packages/studio/tests/e2e/edit-accuracy/settle.test.mjs

@@ -53,6 +53,13 @@ describe("settling on the shown preview", () => {
     expect(await swapPending({ page: page(frame("/studio", false, true)), keys })).toBe(false);
   });
 
+  it("calls a swap pending while Studio says the preview is changing, with no hidden preview yet", async () => {
+    const changing = (on) => ({ frames: () => [], evaluate: async () => on });
+    expect(await swapPending({ page: changing(true), keys })).toBe(true);
+    expect(await swapPending({ page: changing(false), keys })).toBe(false);
+    expect(await swapPending({ page: changing(true) }), "a case without keyframes").toBe(false);
+  });
+
   it("never waits on a swap in a case without keyframes, so its undo keeps main's timing", async () => {
     expect(await swapPending({ page: swapping })).toBe(false);
     expect(await swapPending({ page: swapping, keys: undefined, selector: "#other" })).toBe(false);