From 4b18d683898873b26635aa1573346c9719c8f2e2 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Wed, 7 Oct 2026 13:17:13 -0700 Subject: [PATCH] fix(history): restore each page's menu settings (nav) snapshotToPages mapped only id/name/slug/craftState/seo and planPageRestore built/replaced pages without nav, so 'Restore site' and 'Restore one page' dropped every page's nav (show/parentId/label). The NavSync effect then rewrote the follow-pages header: hidden pages reappeared, dropdown nesting flattened, custom labels reverted. SnapshotPage now carries `nav?: PageNav`, accepted only when it is a plain non-null, non-array object (shallow-copied), else omitted. planPageRestore restores it in id/slug replace mode (like seo) and on append. HistoryModal already passes these rows straight to setPagesCraftState, which keeps nav, so no wiring change was needed. Co-Authored-By: Claude Opus 5.5 --- .../src/panels/history/HistoryModal.test.tsx | 29 +++++++++ craft/src/utils/history-restore.test.ts | 61 +++++++++++++++++++ craft/src/utils/history-restore.ts | 35 +++++++---- 3 files changed, 114 insertions(+), 11 deletions(-) diff --git a/craft/src/panels/history/HistoryModal.test.tsx b/craft/src/panels/history/HistoryModal.test.tsx index 3fc7fff..dbcecba 100644 --- a/craft/src/panels/history/HistoryModal.test.tsx +++ b/craft/src/panels/history/HistoryModal.test.tsx @@ -147,6 +147,35 @@ describe('HistoryModal', () => { expect(live.design.design.primaryColor).not.toBe('#abcdef'); }); + test('Restore site and Restore one page carry each page\'s menu settings (nav) -- finding 3', async () => { + const nav = { show: false, parentId: 'home', label: 'Who we are' }; + const navSnap = { + ...snapshot, + pages_craft_state: [ + snapshot.pages_craft_state[0], + { ...snapshot.pages_craft_state[1], nav }, + { id: 'p3', name: 'Team', slug: 'team', craftState: state('Snapshot team'), nav: 'bogus' }, + ], + }; + mockApi({ snap: navSnap }); + mount(cfg, {}} />); + await flush(); + await click(btn('Restore site…', row(0))); + await click(btn('Load this version')); + expect(live.pages.pages.map((p) => p.nav)).toEqual([undefined, nav, undefined]); + + // Restore one page: current About has different menu settings. + await h.act(async () => { + live.pages.setPagesCraftState([ + { id: 'home', name: 'Home', slug: 'index', craftState: state('Current home') }, + { id: 'p2', name: 'About', slug: 'about', craftState: state('Current about'), nav: { show: true, label: 'Now' } }, + ]); + }); + await click(btn('Restore one page…', row(0))); + await click(btn('About', document.body.querySelector('.wsb-history-pages')!)); + expect(live.pages.pages[1].nav).toEqual(nav); + }); + test('Restore one page on the active page puts it on the canvas', async () => { mockApi(); mount(cfg, {}} />); diff --git a/craft/src/utils/history-restore.test.ts b/craft/src/utils/history-restore.test.ts index f6a9d59..44f9261 100644 --- a/craft/src/utils/history-restore.test.ts +++ b/craft/src/utils/history-restore.test.ts @@ -126,3 +126,64 @@ describe('buildSnapshotPreviewHtml', () => { expect(html).toContain('Page A'); }); }); + +/** + * Finding 3: snapshots store each page's menu settings in `nav` + * (show / parentId / label). Restore used to drop them, so hidden pages + * reappeared, dropdown nesting flattened and custom labels reverted. + */ +describe('page nav survives restore (finding 3)', () => { + const nav = { show: false, parentId: 'home', label: 'Who we are' }; + const withNav = (navValue: unknown) => ({ + pages_craft_state: [ + { id: 'home', name: 'Home', slug: 'index', craftState: state('h') }, + { id: 'p2', name: 'About', slug: 'about', craftState: state('a'), nav: navValue }, + ], + }); + + test('snapshotToPages carries a plain-object nav', () => { + const pages = snapshotToPages(withNav(nav)); + expect(pages[1].nav).toEqual(nav); + expect(pages[0]).not.toHaveProperty('nav'); + }); + + test.each([['a string', 'x'], ['an array', [{ show: false }]], ['null', null], ['a number', 3], ['a boolean', true]])( + 'snapshotToPages drops nav when it is %s', + (_label, bad) => { + const pages = snapshotToPages(withNav(bad)); + expect(pages[1]).not.toHaveProperty('nav'); + }, + ); + + const current: PageData[] = [ + { id: 'home', name: 'Home', slug: 'index', craftState: 'cur-home' }, + { id: 'p2', name: 'About now', slug: 'about', craftState: 'cur-about', nav: { show: true, label: 'Current label' } }, + ]; + + test('planPageRestore replace-by-id restores the snapshot nav', () => { + const [snapPage] = snapshotToPages(withNav(nav)).slice(1); + const plan = planPageRestore(current, snapPage); + expect(plan.mode).toBe('id'); + expect(plan.pages[1].nav).toEqual(nav); + expect(plan.pages[1].name).toBe('About now'); + }); + + test('planPageRestore replace-by-slug restores the snapshot nav', () => { + const plan = planPageRestore(current, { id: 'old', name: 'About', slug: 'about', craftState: 'snap', nav }); + expect(plan.mode).toBe('slug'); + expect(plan.pages[1].nav).toEqual(nav); + }); + + test('planPageRestore append carries the snapshot nav onto the new page', () => { + const plan = planPageRestore(current, { id: 'gone', name: 'Deleted', slug: 'deleted', craftState: 'snap', nav }); + expect(plan.mode).toBe('append'); + expect(plan.pages[2].nav).toEqual(nav); + }); + + test('restored nav is a copy, not the snapshot object', () => { + const snapNav = { show: false, label: 'L' }; + const [, p] = snapshotToPages(withNav(snapNav)); + expect(p.nav).toEqual(snapNav); + expect(p.nav).not.toBe(snapNav); + }); +}); diff --git a/craft/src/utils/history-restore.ts b/craft/src/utils/history-restore.ts index 7ac8509..ae4b889 100644 --- a/craft/src/utils/history-restore.ts +++ b/craft/src/utils/history-restore.ts @@ -1,4 +1,4 @@ -import type { PageData, PageSeo } from '../types'; +import type { PageData, PageNav, PageSeo } from '../types'; import { DEFAULT_SITE_DESIGN, type SiteDesign } from '../state/SiteDesignContext'; import { exportBodyHtml, exportToHtml, buildAnimationScript } from './html-export'; @@ -15,6 +15,8 @@ export interface SnapshotPage { slug: string; craftState: string | null; seo?: PageSeo; + /** Menu settings (#53: show / parentId / label); absent when the snapshot has none. */ + nav?: PageNav; } export interface Snapshot { @@ -33,19 +35,29 @@ const asState = (v: unknown): string | null => { return null; }; +/** A snapshot page's `nav`, only when it is a plain non-null, non-array object + * (shallow-copied so the restored page never aliases the snapshot); else undefined. */ +const asNav = (v: unknown): PageNav | undefined => + v && typeof v === 'object' && !Array.isArray(v) ? { ...(v as PageNav) } : undefined; + /** The snapshot's pages as editor PageData-shaped rows (craftState always a string or null). */ export function snapshotToPages(snapshot: Snapshot): SnapshotPage[] { const raw = snapshot?.pages_craft_state; if (!Array.isArray(raw)) return []; return raw .filter((p) => p && typeof p === 'object') - .map((p: any, i) => ({ - id: typeof p.id === 'string' && p.id ? p.id : `restored_${i}`, - name: typeof p.name === 'string' ? p.name : `Page ${i + 1}`, - slug: typeof p.slug === 'string' ? p.slug : `page-${i + 1}`, - craftState: asState(p.craftState), - seo: p.seo && typeof p.seo === 'object' ? (p.seo as PageSeo) : undefined, - })); + .map((p: any, i) => { + const row: SnapshotPage = { + id: typeof p.id === 'string' && p.id ? p.id : `restored_${i}`, + name: typeof p.name === 'string' ? p.name : `Page ${i + 1}`, + slug: typeof p.slug === 'string' ? p.slug : `page-${i + 1}`, + craftState: asState(p.craftState), + seo: p.seo && typeof p.seo === 'object' ? (p.seo as PageSeo) : undefined, + }; + const nav = asNav(p.nav); + if (nav) row.nav = nav; + return row; + }); } export function snapshotExtras(snapshot: Snapshot): Record { @@ -95,7 +107,8 @@ export interface PageRestorePlan { /** * Replace one page with its snapshot version: match by id, else slug, else * append as a new page. A matched page keeps its identity (id, name, slug) so - * links and the landing-page rule stay put; only content and SEO come back. + * links and the landing-page rule stay put; content, SEO and menu settings + * (`nav`) come back. */ export function planPageRestore(currentPages: PageData[], snapshotPage: SnapshotPage): PageRestorePlan { let idx = currentPages.findIndex((p) => p.id === snapshotPage.id); @@ -105,10 +118,10 @@ export function planPageRestore(currentPages: PageData[], snapshotPage: Snapshot mode = 'slug'; } if (idx === -1) { - const added: PageData = { id: snapshotPage.id, name: snapshotPage.name, slug: snapshotPage.slug, craftState: snapshotPage.craftState, seo: snapshotPage.seo }; + const added: PageData = { id: snapshotPage.id, name: snapshotPage.name, slug: snapshotPage.slug, craftState: snapshotPage.craftState, seo: snapshotPage.seo, nav: snapshotPage.nav }; return { pages: [...currentPages, added], targetId: added.id, mode: 'append' }; } - const pages = currentPages.map((p, i) => (i === idx ? { ...p, craftState: snapshotPage.craftState, seo: snapshotPage.seo } : p)); + const pages = currentPages.map((p, i) => (i === idx ? { ...p, craftState: snapshotPage.craftState, seo: snapshotPage.seo, nav: snapshotPage.nav } : p)); return { pages, targetId: currentPages[idx].id, mode }; }