fix(site-builder): Undo can no longer cross the load or a page switch

Craft records actions.deserialize as an undoable step and nothing cleared
history, so Undo right after load returned the canvas to the blank pre-load
Frame (autosave would then save it), and Undo after a page switch put the
previous page's content into the current slot.

- clear history after the load-time deserialize and after every page/
  header/footer switch (in-place AI replacement of the same slot keeps it)
- continuousSetProp opens a burst only when its first call actually
  changed something, so it never merges into an entry it did not create
- the empty-save banner's Undo is disabled when there is nothing to undo

Real-editor tests: load, page/header switch, no-op-first burst, banner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
shadowdaoandClaude Opus 5.5 committed 2026-10-07 10:32:15 -07:00
1 parent eb134a4743
commit 572e736f1f
14 files changed
+310 -17

No files matched your search

+6 -2
View File
@@ -10,14 +10,16 @@ import { act } from 'react-dom/test-utils';
* `@craftjs/core`'s useEditor and sibling chrome are stubbed.
*/
const undoMock = vi.fn();
// The banner's Undo is disabled when Craft reports nothing to undo; tests that click it set this.
const mockCanUndo = { value: false };
const deserializeMock = vi.fn();
vi.mock('@craftjs/core', () => ({
useEditor: (collector?: (state: unknown, query: unknown) => Record<string, unknown>) => {
const query = {
serialize: () => '{}',
history: { canUndo: () => false, canRedo: () => false },
history: { canUndo: () => mockCanUndo.value, canRedo: () => false },
};
const actions = { history: { undo: undoMock, redo: vi.fn() }, deserialize: deserializeMock };
const actions = { history: { undo: undoMock, redo: vi.fn(), clear: vi.fn() }, deserialize: deserializeMock };
const collected = collector ? collector({}, query) : {};
return { actions, query, ...collected };
},
@@ -82,6 +84,7 @@ function mount() {
}
beforeEach(() => {
mockCanUndo.value = false;
vi.useFakeTimers();
fetchMock = vi.fn();
reloadMock.mockReset();
@@ -240,6 +243,7 @@ describe('revision and overwrite edge cases', () => {
});
test('Keep mine -> empty refusal -> Undo disarms force_overwrite: the next autosave is conflict-checked', async () => {
mockCanUndo.value = true;
loadedThen('r1', [
conflict409,
jsonResp({ success: false, error: 'empty_overwrite_refused', message: 'e' }, 409),
@@ -10,14 +10,16 @@ import { act } from 'react-dom/test-utils';
* timers; only `@craftjs/core`'s useEditor and sibling chrome are stubbed.
*/
const undoMock = vi.fn();
// The banner's Undo is disabled when Craft reports nothing to undo; tests that click it set this.
const mockCanUndo = { value: false };
const deserializeMock = vi.fn();
vi.mock('@craftjs/core', () => ({
useEditor: (collector?: (state: unknown, query: unknown) => Record<string, unknown>) => {
const query = {
serialize: () => '{}',
history: { canUndo: () => false, canRedo: () => false },
history: { canUndo: () => mockCanUndo.value, canRedo: () => false },
};
const actions = { history: { undo: undoMock, redo: vi.fn() }, deserialize: deserializeMock };
const actions = { history: { undo: undoMock, redo: vi.fn(), clear: vi.fn() }, deserialize: deserializeMock };
const collected = collector ? collector({}, query) : {};
return { actions, query, ...collected };
},
@@ -81,6 +83,7 @@ function mount() {
}
beforeEach(() => {
mockCanUndo.value = false;
vi.useFakeTimers();
fetchMock = vi.fn();
vi.stubGlobal('fetch', fetchMock);
@@ -245,6 +248,7 @@ describe('empty-overwrite guard', () => {
});
test('Undo on the banner runs Craft history undo and dismisses it', async () => {
mockCanUndo.value = true;
loadedThen([jsonResp({ success: false, error: 'empty_overwrite_refused', message: 'r' }, 409)]);
mount();
await flush();
+1 -1
View File
@@ -17,7 +17,7 @@ vi.mock('@craftjs/core', () => ({
serialize: () => '{}',
history: { canUndo: () => false, canRedo: () => false },
};
const actions = { history: { undo: vi.fn(), redo: vi.fn() }, deserialize: vi.fn() };
const actions = { history: { undo: vi.fn(), redo: vi.fn(), clear: vi.fn() }, deserialize: vi.fn() };
const collected = collector ? collector({}, query) : {};
return { actions, query, ...collected };
},
+1 -1
View File
@@ -21,7 +21,7 @@ const deserializeMock = vi.fn();
vi.mock('@craftjs/core', () => ({
useEditor: () => ({
query: { serialize: () => '{}' },
actions: { deserialize: deserializeMock },
actions: { deserialize: deserializeMock, history: { clear: () => {} } },
}),
}));
@@ -31,7 +31,7 @@ const deserializeMock = vi.fn();
vi.mock('@craftjs/core', () => ({
useEditor: () => ({
query: { serialize: () => '{}' },
actions: { deserialize: deserializeMock },
actions: { deserialize: deserializeMock, history: { clear: () => {} } },
}),
}));
+4
View File
@@ -514,6 +514,10 @@ export function useWhpApi() {
);
}
actions.deserialize(state);
// Craft records deserialize as an undoable step; left in, Undo
// right after load returns the canvas to the pre-load blank Frame
// (and autosave would then save that). History starts here.
actions.history.clear();
} catch (e) {
// Never fall back to a blank canvas: autosave would then replace
// the customer's real (if unreadable) page with it. Fail the load.
+14 -2
View File
@@ -344,8 +344,20 @@ export function continuousSetProp(actions: any, nodeId: string, controlKey: stri
const continuing = openBurst?.key === key;
if (!continuing) closeContinuousBurst();
else clearTimeout(openBurst!.timer);
(continuing ? actions.history.merge() : actions).setProp(nodeId, updater);
openBurst = { key, timer: setTimeout(closeContinuousBurst, CONTINUOUS_EDIT_IDLE_MS) };
// Craft records no history entry for a change that changes nothing. A burst
// must only be opened by a call that recorded its OWN entry: otherwise the
// next change would merge() into whatever entry came before (another edit,
// or the page load -- and Undo would then blank the canvas).
let changed = false;
const tracked = (p: any) => {
const before = JSON.stringify(p);
updater(p);
changed = JSON.stringify(p) !== before;
};
(continuing ? actions.history.merge() : actions).setProp(nodeId, tracked);
if (continuing || changed) {
openBurst = { key, timer: setTimeout(closeContinuousBurst, CONTINUOUS_EDIT_IDLE_MS) };
}
}
/** Plain setProp for a discrete edit: always its own undo step. */
+4 -2
View File
@@ -3,15 +3,17 @@ import React from 'react';
export interface EmptySaveBannerProps {
onSaveAnyway: () => void;
onUndo: () => void;
/** Nothing to undo (e.g. the empty page came straight from load): Undo is disabled, not a dead button. */
undoDisabled?: boolean;
}
/** Persistent banner shown when the server refused a save that would replace
* the saved site with an empty page. Autosave stays paused while it's up. */
export const EmptySaveBanner: React.FC<EmptySaveBannerProps> = ({ onSaveAnyway, onUndo }) => (
export const EmptySaveBanner: React.FC<EmptySaveBannerProps> = ({ onSaveAnyway, onUndo, undoDisabled }) => (
<div className="empty-save-banner" role="alert" data-testid="empty-save-banner">
<i className="fa fa-exclamation-triangle" aria-hidden="true" />
<span className="empty-save-banner-text">This would replace your saved site with an empty page.</span>
<button type="button" onClick={onSaveAnyway} data-testid="empty-save-anyway">Save anyway</button>
<button type="button" onClick={onUndo} data-testid="empty-save-undo">Undo</button>
<button type="button" onClick={onUndo} disabled={undoDisabled} data-testid="empty-save-undo">Undo</button>
</div>
);
+1
View File
@@ -187,6 +187,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
const emptyBanner = emptyRefusal ? (
<EmptySaveBanner
onSaveAnyway={() => { void handleSave({ forceEmpty: true }); }}
undoDisabled={!canUndo}
onUndo={() => {
actions.history.undo();
setEmptyRefusal(null);
@@ -28,7 +28,7 @@ const deserializeMock = vi.fn();
vi.mock('@craftjs/core', () => ({
useEditor: () => ({
query: { serialize: () => serializeReturn },
actions: { deserialize: deserializeMock },
actions: { deserialize: deserializeMock, history: { clear: () => {} } },
}),
}));
@@ -17,7 +17,7 @@ const deserializeMock = vi.fn();
vi.mock('@craftjs/core', () => ({
useEditor: () => ({
query: { serialize: () => '{}' },
actions: { deserialize: deserializeMock },
actions: { deserialize: deserializeMock, history: { clear: () => {} } },
}),
}));
+11 -4
View File
@@ -385,7 +385,12 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
* can't be selected or deleted) -- that report is still unreproduced;
* see `orphan-repair.ts` for detail. */
const loadState = useCallback(
(craftState: string | null, fallback: string) => {
(craftState: string | null, fallback: string, opts?: { keepHistory?: boolean }) => {
// A switch puts a different page/zone on the canvas: its history must
// start fresh, or Undo would deserialize the previous page's content
// into this slot. Only an in-place replacement of the SAME slot (the
// AI rewriting the current page) keeps history, so it stays undoable.
const clearHistory = !opts?.keepHistory;
setTimeout(() => {
const source = craftState || fallback;
const { state, repaired } = repairOrphanNodes(source);
@@ -401,6 +406,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
}
try {
actions.deserialize(state);
if (clearHistory) actions.history.clear();
} catch (e) {
console.error('Failed to deserialize state:', e);
try {
@@ -410,6 +416,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
// never untrusted/stored data, since this is the last line of
// defense before giving up silently below.
actions.deserialize(fallback);
if (clearHistory) actions.history.clear();
} catch (_e2) {
// give up
}
@@ -717,7 +724,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
prev.map((p) => (p.id === currentId ? { ...p, name: page.name, craftState } : p)),
);
}
loadState(craftState, EMPTY_CANVAS);
loadState(craftState, EMPTY_CANVAS, { keepHistory: true });
}, [loadState]);
/**
@@ -729,7 +736,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
setHeaderPage((prev) => ({ ...prev, craftState }));
// If the canvas is currently showing the header, refresh it live
if (activePageIdRef.current === HEADER_ID) {
loadState(craftState, EMPTY_HEADER);
loadState(craftState, EMPTY_HEADER, { keepHistory: true });
}
}, [loadState]);
@@ -742,7 +749,7 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
setFooterPage((prev) => ({ ...prev, craftState }));
// If the canvas is currently showing the footer, refresh it live
if (activePageIdRef.current === FOOTER_ID) {
loadState(craftState, EMPTY_FOOTER);
loadState(craftState, EMPTY_FOOTER, { keepHistory: true });
}
}, [loadState]);
@@ -0,0 +1,175 @@
import { describe, test, expect, afterEach, beforeEach, vi } from 'vitest';
import React from 'react';
import { renderEditorHarness, EditorHarness } from '../editorHarness';
import { EditorConfigProvider } from '../../state/EditorConfigContext';
import { SiteDesignProvider } from '../../state/SiteDesignContext';
import { PageProvider, usePages } from '../../state/PageContext';
import { useWhpApi } from '../../hooks/useWhpApi';
import { useNodeProp, closeContinuousBurst } from '../../panels/right/styles/shared';
import { WhpConfig } from '../../types';
/**
* The undo history must start at the state the customer sees, never before it.
* Against a REAL Craft.js editor: the load-time deserialize and every page /
* header / footer switch reset history, and a continuous-edit burst never
* merges into an entry it did not create (that is how Undo could fold back
* into the load and blank the page).
*/
const node = (id: string, text: string) => ({
type: { resolvedName: 'Heading' }, isCanvas: false,
props: { text, level: 'h2', style: { color: '#000000' } },
displayName: 'Heading', custom: {}, hidden: false, parent: 'ROOT', nodes: [], linkedNodes: {},
});
const pageState = (id: string, text: string) => JSON.stringify({
ROOT: {
type: { resolvedName: 'Container' }, isCanvas: true, props: { style: {}, tag: 'div' },
displayName: 'Container', custom: {}, hidden: false, nodes: [id], linkedNodes: {},
},
[id]: node(id, text),
});
const whpConfig: WhpConfig = {
user: 'u', apiUrl: '/api', csrfToken: 't', siteId: 1,
siteDomain: 'example.com', siteName: 'Example', backUrl: '/', isRoot: false,
};
const project = {
success: true,
rev: 'r1',
project: {
pages_craft_state: [
{ id: 'home', name: 'Home', slug: 'index', craftState: pageState('h-1', 'Home text') },
{ id: 'about', name: 'About', slug: 'about', craftState: pageState('a-1', 'About text') },
],
},
};
let harness: EditorHarness | null = null;
let api: { load: () => Promise<unknown>; switchPage: (id: string) => void; activePageId: string };
const Probe: React.FC = () => {
const { load } = useWhpApi();
const { switchPage, activePageId } = usePages();
api = { load, switchPage, activePageId };
return null;
};
function mountWhp(extra?: React.ReactNode) {
harness = renderEditorHarness();
harness.mountChild(
<EditorConfigProvider config={whpConfig}>
<SiteDesignProvider>
<PageProvider>
<Probe />
{extra}
</PageProvider>
</SiteDesignProvider>
</EditorConfigProvider>,
);
}
const rootNodes = () => harness!.query.node('ROOT').get().data.nodes as string[];
const textOf = (id: string) => harness!.query.node(id).get().data.props.text;
const undo = () => harness!.act(() => { harness!.actions.history.undo(); });
const canUndo = () => harness!.query.history.canUndo() as boolean;
async function load() {
await harness!.act(async () => { await api.load(); });
}
beforeEach(() => {
vi.stubGlobal('fetch', vi.fn(async () => ({ ok: true, status: 200, json: async () => project })));
vi.spyOn(console, 'error').mockImplementation(() => {});
});
afterEach(() => {
closeContinuousBurst();
harness?.unmount();
harness = null;
vi.useRealTimers();
vi.unstubAllGlobals();
vi.restoreAllMocks();
});
describe('history baseline after load', () => {
test('a loaded populated project has nothing to undo, and Undo does not blank it', async () => {
mountWhp();
await load();
expect(rootNodes()).toEqual(['h-1']);
expect(canUndo()).toBe(false);
undo();
expect(rootNodes()).toEqual(['h-1']);
expect(textOf('h-1')).toBe('Home text');
});
});
describe('history baseline after a page switch', () => {
test('Undo right after switching pages does not bring the previous page back', async () => {
vi.useFakeTimers();
mountWhp();
await load();
// An undoable edit on Home, then switch to About.
harness!.act(() => { harness!.actions.setProp('h-1', (p: any) => { p.text = 'Edited home'; }); });
expect(canUndo()).toBe(true);
harness!.act(() => { api.switchPage('about'); });
harness!.act(() => { vi.advanceTimersByTime(0); });
expect(rootNodes()).toEqual(['a-1']);
expect(canUndo()).toBe(false);
undo();
undo();
expect(rootNodes()).toEqual(['a-1']);
expect(textOf('a-1')).toBe('About text');
});
test('switching to the header and back also starts a fresh history', async () => {
vi.useFakeTimers();
mountWhp();
await load();
harness!.act(() => { harness!.actions.setProp('h-1', (p: any) => { p.text = 'Edited home'; }); });
harness!.act(() => { api.switchPage('__header__'); });
harness!.act(() => { vi.advanceTimersByTime(0); });
expect(canUndo()).toBe(false);
harness!.act(() => { api.switchPage('home'); });
harness!.act(() => { vi.advanceTimersByTime(0); });
expect(canUndo()).toBe(false);
expect(textOf('h-1')).toBe('Edited home');
});
});
/** A continuous control (colour / text) wired through the real useNodeProp. */
const BurstProbe: React.FC = () => {
const { setPropContinuous, setPropStyle } = useNodeProp('h-1');
(window as any).__burst = { setPropContinuous, setPropStyle };
return null;
};
const burst = () => (window as any).__burst as {
setPropContinuous: (k: string, v: string) => void;
setPropStyle: (k: string, v: string) => void;
};
describe('a burst that opens with a no-op change', () => {
test('no-op first change, then a real change: Undo restores only the real change and never blanks the canvas', () => {
// Deliberately keep the pre-edit deserialize entry in history (as any
// earlier entry would be): the burst must not merge into it.
harness = renderEditorHarness({ initialState: pageState('h-1', 'Home text') });
harness.mountChild(<BurstProbe />);
expect(canUndo()).toBe(true); // the foreign (load-like) entry
harness.act(() => { burst().setPropContinuous('text', 'Home text'); }); // no-op plain prop
harness.act(() => { burst().setPropContinuous('text', 'Home text!'); }); // real
expect(textOf('h-1')).toBe('Home text!');
undo();
expect(rootNodes()).toEqual(['h-1']);
expect(textOf('h-1')).toBe('Home text');
});
test('no-op first change after a discrete edit: Undo reverts the text but keeps the discrete edit', () => {
harness = renderEditorHarness({ initialState: pageState('h-1', 'Home text') });
harness.mountChild(<BurstProbe />);
harness.act(() => { harness!.actions.history.clear(); });
harness.act(() => { burst().setPropStyle('fontWeight', '700'); });
harness.act(() => { burst().setPropContinuous('text', 'Home text'); }); // no-op
harness.act(() => { burst().setPropContinuous('text', 'Home text!'); });
undo();
expect(textOf('h-1')).toBe('Home text');
expect(harness.query.node('h-1').get().data.props.style.fontWeight).toBe('700');
});
});
@@ -0,0 +1,84 @@
import { describe, test, expect, afterEach, beforeEach, vi } from 'vitest';
import React from 'react';
import { renderEditorHarness, EditorHarness } from '../editorHarness';
vi.mock('../../panels/topbar/TemplateModal', () => ({ TemplateModal: () => null }));
vi.mock('../../panels/topbar/HeadCodeModal', () => ({ HeadCodeModal: () => null }));
vi.mock('../../panels/sitesmith/SitesmithButton', () => ({ SitesmithButton: () => null }));
import { EditorConfigProvider } from '../../state/EditorConfigContext';
import { SaveGuardProvider } from '../../state/SaveGuardContext';
import { SiteDesignProvider } from '../../state/SiteDesignContext';
import { PageProvider } from '../../state/PageContext';
import { LoadGate } from '../../editor/LoadGate';
import { TopBar } from '../../panels/topbar/TopBar';
import { WhpConfig } from '../../types';
/**
* The empty-save banner's Undo, driven through the real LoadGate + TopBar on
* a REAL Craft.js editor: right after load there is nothing to undo, so the
* button must be disabled rather than silently doing nothing (or, before the
* history reset, blanking the page).
*/
const pageState = JSON.stringify({
ROOT: {
type: { resolvedName: 'Container' }, isCanvas: true, props: { style: {}, tag: 'div' },
displayName: 'Container', custom: {}, hidden: false, nodes: ['h-1'], linkedNodes: {},
},
'h-1': {
type: { resolvedName: 'Heading' }, isCanvas: false,
props: { text: 'Home text', level: 'h2', style: {} },
displayName: 'Heading', custom: {}, hidden: false, parent: 'ROOT', nodes: [], linkedNodes: {},
},
});
const whpConfig: WhpConfig = {
user: 'u', apiUrl: '/api', csrfToken: 't', siteId: 1,
siteDomain: 'example.com', siteName: 'Example', backUrl: '/', isRoot: false,
};
const jsonResp = (body: unknown, status = 200) => ({ ok: status >= 200 && status < 300, status, json: async () => body });
let harness: EditorHarness | null = null;
const q = (sel: string) => harness!.container.querySelector(sel) as HTMLButtonElement | null;
const flush = () => harness!.act(async () => { for (let i = 0; i < 10; i++) await Promise.resolve(); });
beforeEach(() => {
vi.spyOn(console, 'error').mockImplementation(() => {});
vi.stubGlobal('fetch', vi.fn(async (url: string) => String(url).includes('action=load')
? jsonResp({ success: true, rev: 'r1', project: { pages_craft_state: [{ id: 'home', name: 'Home', slug: 'index', craftState: pageState }] } })
: jsonResp({ success: false, error: 'empty_overwrite_refused', message: 'e' }, 409)));
});
afterEach(() => {
harness?.unmount();
harness = null;
vi.unstubAllGlobals();
vi.restoreAllMocks();
});
describe('empty-save banner Undo', () => {
test('is disabled when there is nothing to undo, and enabled once there is', async () => {
harness = renderEditorHarness();
harness.mountChild(
<EditorConfigProvider config={whpConfig}>
<SaveGuardProvider>
<SiteDesignProvider>
<PageProvider>
<LoadGate />
<TopBar device="desktop" onDeviceChange={() => {}} showGuides onToggleGuides={() => {}} />
</PageProvider>
</SiteDesignProvider>
</SaveGuardProvider>
</EditorConfigProvider>,
);
await flush();
await harness.act(async () => { q('button[aria-label="Save Draft"]')!.click(); });
await flush();
expect(q('[data-testid="empty-save-banner"]')).not.toBeNull();
expect(q('[data-testid="empty-save-undo"]')!.disabled).toBe(true);
harness.act(() => { q('[data-testid="empty-save-undo"]')!.click(); });
expect(harness.query.node('ROOT').get().data.nodes).toEqual(['h-1']);
harness.act(() => { harness!.actions.setProp('h-1', (p: any) => { p.text = 'x'; }); });
expect(q('[data-testid="empty-save-undo"]')!.disabled).toBe(false);
});
});