From 710314c15e11bb1f49cd83117f42b3cf2b2fc62e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 28 Oct 2026 17:51:22 +0000 Subject: [PATCH] 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 --- .../src/hooks/useEditHistoryActions.test.tsx | 25 +++++++++++++++++++ .../studio/src/hooks/useEditHistoryActions.ts | 5 ++-- .../src/player/previewReloading.test.ts | 17 +++++++++++++ .../studio/src/player/previewReloading.ts | 9 ++++++- .../studio/tests/e2e/edit-accuracy/case.mjs | 10 +++++++- .../tests/e2e/edit-accuracy/settle.test.mjs | 7 ++++++ 6 files changed, 69 insertions(+), 4 deletions(-) diff --git a/packages/studio/src/hooks/useEditHistoryActions.test.tsx b/packages/studio/src/hooks/useEditHistoryActions.test.tsx index 019a0d8f09..518bef8546 100644 --- a/packages/studio/src/hooks/useEditHistoryActions.test.tsx +++ b/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((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)); diff --git a/packages/studio/src/hooks/useEditHistoryActions.ts b/packages/studio/src/hooks/useEditHistoryActions.ts index 73cb4c7327..5c34ae3f57 100644 --- a/packages/studio/src/hooks/useEditHistoryActions.ts +++ b/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]); } diff --git a/packages/studio/src/player/previewReloading.test.ts b/packages/studio/src/player/previewReloading.test.ts index 7cbe5e05b2..1e3c0760bc 100644 --- a/packages/studio/src/player/previewReloading.test.ts +++ b/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((resolve) => (land = resolve))); + expect(shown()).toBe(true); + land(); + await write; + expect(shown()).toBe(false); + }); }); diff --git a/packages/studio/src/player/previewReloading.ts b/packages/studio/src/player/previewReloading.ts index 3577cebd84..408068bcd8 100644 --- a/packages/studio/src/player/previewReloading.ts +++ b/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(write: () => Promise): Promise { scriptWrites += 1; + noteIdle(); try { return await write(); } finally { diff --git a/packages/studio/tests/e2e/edit-accuracy/case.mjs b/packages/studio/tests/e2e/edit-accuracy/case.mjs index e0a5f67b8a..1b2870b8ec 100644 --- a/packages/studio/tests/e2e/edit-accuracy/case.mjs +++ b/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) => diff --git a/packages/studio/tests/e2e/edit-accuracy/settle.test.mjs b/packages/studio/tests/e2e/edit-accuracy/settle.test.mjs index ee2fa64794..930ba4310f 100644 --- a/packages/studio/tests/e2e/edit-accuracy/settle.test.mjs +++ b/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);