From c71a6b64a6ae8736eb667fabaccf1226871720aa Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 1 Nov 2026 15:23:37 -0800 Subject: [PATCH] Fix ImageBlock (any non-Container) directly inside ColumnLayout exporting an empty div Craft's only adopts an existing linkedNode whose type is Container; otherwise it creates a fresh empty Container and orphans ours. flattenTreeForCraft now wraps a non-Container column child in a pre-created Container. Co-Authored-By: Claude Opus 5.5 --- .../__fixtures__/hero-image-in-columns.json | 127 ++++++++++++++++++ .../src/state/PageContext.treeToState.test.ts | 4 +- .../image-in-columns.integration.test.tsx | 27 ++++ craft/src/utils/craft-tree.test.ts | 29 +++- craft/src/utils/craft-tree.ts | 23 ++++ 5 files changed, 205 insertions(+), 5 deletions(-) create mode 100644 craft/src/sitesmith/__fixtures__/hero-image-in-columns.json create mode 100644 craft/src/test-utils/integration/image-in-columns.integration.test.tsx diff --git a/craft/src/sitesmith/__fixtures__/hero-image-in-columns.json b/craft/src/sitesmith/__fixtures__/hero-image-in-columns.json new file mode 100644 index 0000000..d4e5279 --- /dev/null +++ b/craft/src/sitesmith/__fixtures__/hero-image-in-columns.json @@ -0,0 +1,127 @@ +{ + "type": { + "resolvedName": "Section" + }, + "props": { + "aiName": "Hero", + "node_id": "ai-hero-1", + "anchorId": "about", + "style": { + "backgroundColor": "color-mix(in srgb, var(--wsb-text) 5%, var(--wsb-bg))", + "paddingTop": "96px", + "paddingBottom": "96px", + "paddingLeft": "24px", + "paddingRight": "24px" + } + }, + "nodes": [ + { + "type": { + "resolvedName": "ColumnLayout" + }, + "props": { + "aiName": "Hero columns", + "node_id": "ai-hero-columns-1", + "columns": 2, + "split": "50-50", + "gap": "64px", + "style": { + "display": "flex", + "flexDirection": "row", + "alignItems": "center" + } + }, + "nodes": [ + { + "type": { + "resolvedName": "Container" + }, + "props": { + "aiName": "Hero text", + "node_id": "ai-hero-text-1", + "style": { + "padding": "0px" + } + }, + "nodes": [ + { + "type": { + "resolvedName": "Heading" + }, + "props": { + "aiName": "Hero heading", + "node_id": "ai-hero-heading-1", + "text": "Digital brands built on honesty", + "level": "h1", + "style": { + "fontFamily": "var(--wsb-heading-font)", + "fontSize": "clamp(2.25rem, 6vw, 4rem)", + "fontWeight": "800", + "lineHeight": "1.1", + "letterSpacing": "-0.02em", + "color": "var(--wsb-text)", + "marginBottom": "24px" + } + } + }, + { + "type": { + "resolvedName": "TextBlock" + }, + "props": { + "aiName": "Hero lead", + "node_id": "ai-hero-lead-1", + "text": "OLS Digital is the privately held parent company of online brands. We acquire and grow digital-first businesses, and each one keeps its own identity.", + "style": { + "fontFamily": "var(--wsb-body-font)", + "fontSize": "19px", + "lineHeight": "1.6", + "color": "var(--wsb-text)", + "maxWidth": "520px", + "marginBottom": "32px" + } + } + }, + { + "type": { + "resolvedName": "ButtonLink" + }, + "props": { + "aiName": "Meet our brands", + "node_id": "ai-hero-button-1", + "text": "Meet our brands", + "href": "#brands", + "hoverBg": "var(--wsb-text)", + "style": { + "backgroundColor": "var(--wsb-brand-1)", + "color": "var(--wsb-bg)", + "padding": "14px 28px", + "borderRadius": "var(--wsb-button-radius)", + "fontWeight": "600", + "display": "inline-block" + } + } + } + ] + }, + { + "type": { + "resolvedName": "ImageBlock" + }, + "props": { + "aiName": "Hero image", + "node_id": "ai-hero-image-1", + "src": "https://placehold.co/800x600/e5e7eb/1f2937?text=OLS+Digital+team+or+workspace", + "alt": "Placeholder for a photo of the OLS Digital team or workspace", + "style": { + "width": "100%", + "aspectRatio": "4 / 3", + "objectFit": "cover", + "borderRadius": "var(--wsb-radius)" + } + } + } + ] + } + ] +} \ No newline at end of file diff --git a/craft/src/state/PageContext.treeToState.test.ts b/craft/src/state/PageContext.treeToState.test.ts index df4ae58..b54cc1e 100644 --- a/craft/src/state/PageContext.treeToState.test.ts +++ b/craft/src/state/PageContext.treeToState.test.ts @@ -86,8 +86,8 @@ describe('treeToCraftState resolvedName guard', () => { type: { resolvedName: 'ColumnLayout' }, props: { node_id: 'cols-1' }, nodes: [ - { type: { resolvedName: 'Heading' }, props: { node_id: 'a' }, nodes: [] }, - { type: { resolvedName: 'Heading' }, props: { node_id: 'b' }, nodes: [] }, + { type: { resolvedName: 'Container' }, props: { node_id: 'a' }, nodes: [] }, + { type: { resolvedName: 'Container' }, props: { node_id: 'b' }, nodes: [] }, ], }; const state = treeToCraftState(tree); diff --git a/craft/src/test-utils/integration/image-in-columns.integration.test.tsx b/craft/src/test-utils/integration/image-in-columns.integration.test.tsx new file mode 100644 index 0000000..4746506 --- /dev/null +++ b/craft/src/test-utils/integration/image-in-columns.integration.test.tsx @@ -0,0 +1,27 @@ +import { describe, test, expect, afterEach } from 'vitest'; +import React from 'react'; +import hero from '../../sitesmith/__fixtures__/hero-image-in-columns.json'; +import { renderEditorHarness, EditorHarness } from '../editorHarness'; +import { useApplyAiResponse } from '../../utils/apply-ai-response'; +import { exportBodyHtml } from '../../utils/html-export'; + +const INITIAL = JSON.stringify({ + ROOT: { type: { resolvedName: 'Container' }, isCanvas: true, props: { style: {}, tag: 'div' }, displayName: 'Container', custom: {}, hidden: false, nodes: [], linkedNodes: {} }, +}); +let harness: EditorHarness | null = null; +afterEach(() => { harness?.unmount(); harness = null; }); + +describe('Bug D: ImageBlock directly inside ColumnLayout (real model tree, real Craft)', () => { + test('applied + serialized + exported keeps the ', async () => { + harness = renderEditorHarness({ initialState: INITIAL }); + const ref: { current: ReturnType | null } = { current: null }; + harness.mountChild(React.createElement(() => { ref.current = useApplyAiResponse(); return null; })); + await harness.act(async () => { + await ref.current!({ type: 'replace', scope: 'section', pages: [{ name: 'section', tree: hero as any }], message: 'x' } as any); + }); + const state = harness.query.serialize(); + const { html } = exportBodyHtml(state) as any; + expect(html).toContain(' { type: { resolvedName: 'ColumnLayout' }, props: { node_id: 'cols' }, nodes: [ - { type: { resolvedName: 'Heading' }, props: { node_id: 'a' }, nodes: [] }, - { type: { resolvedName: 'Heading' }, props: { node_id: 'b' }, nodes: [] }, + { type: { resolvedName: 'Container' }, props: { node_id: 'a' }, nodes: [] }, + { type: { resolvedName: 'Container' }, props: { node_id: 'b' }, nodes: [] }, ], }; const { nodes } = flattenTreeForCraft(tree); @@ -240,7 +240,7 @@ describe('flattenTreeForCraft', () => { type: { resolvedName: 'ColumnLayout' }, props: { node_id: 'cols' }, nodes: [ - { type: { resolvedName: 'Heading' }, props: { node_id: 'a' }, nodes: [] }, + { type: { resolvedName: 'Container' }, props: { node_id: 'a' }, nodes: [] }, ], }, ], @@ -291,3 +291,26 @@ describe('sanitizeAiTree (re-exported for treeToCraftState parity with buildNode expect(out!.nodes![0].props.node_id).toBe('h1'); }); }); + +describe('flattenTreeForCraft: non-Container ColumnLayout children (Bug D)', () => { + const n = (type: string, id: string, nodes: any[] = []) => ({ type: { resolvedName: type }, props: { node_id: id }, nodes }); + + test('a bare ImageBlock column is wrapped in a Container column; Container columns are untouched', () => { + const { nodes } = flattenTreeForCraft(n('ColumnLayout', 'cols', [n('Container', 'c0'), n('ImageBlock', 'img')]) as any); + expect(nodes.cols.linkedNodes).toEqual({ 'col-0': 'c0', 'col-1': 'img__col' }); + expect(nodes['img__col'].type.resolvedName).toBe('Container'); + expect(nodes['img__col'].isCanvas).toBe(true); + expect(nodes['img__col'].nodes).toEqual(['img']); + expect(nodes['img__col'].parent).toBe('cols'); + expect(nodes.img.parent).toBe('img__col'); + expect(nodes.img.isCanvas).toBe(false); + expect(nodes.c0.parent).toBe('cols'); + expect(nodes.c0.isCanvas).toBe(true); + }); + + test('every linked column is a Container (Craft only adopts Container-typed column nodes)', () => { + const { nodes } = flattenTreeForCraft(n('ColumnLayout', 'cols', [n('TextBlock', 't'), n('ImageBlock', 'i'), n('ColumnLayout', 'inner')]) as any); + for (const id of Object.values(nodes.cols.linkedNodes)) expect(nodes[id as string].type.resolvedName).toBe('Container'); + expect(nodes.cols.props.columns).toBe(3); + }); +}); diff --git a/craft/src/utils/craft-tree.ts b/craft/src/utils/craft-tree.ts index 19cb19a..cbb5b46 100644 --- a/craft/src/utils/craft-tree.ts +++ b/craft/src/utils/craft-tree.ts @@ -177,6 +177,29 @@ export function flattenTreeForCraft(tree: SerializedTreeNode): FlattenTreeResult if (typeName === 'ColumnLayout' && childIds.length > 0) { const linked: Record = {}; childIds.forEach((cid, i) => { + // Craft's only ADOPTS an existing + // linkedNode whose `data.type === Container`; any other type (e.g. a bare + // ImageBlock, TextBlock, nested ColumnLayout) makes it create a fresh empty + // Container and orphan ours -- the child silently vanishes (Bug D). So a + // non-Container column child is wrapped in a pre-created Container column. + if (nodes[cid] && nodes[cid].type?.resolvedName !== 'Container') { + const wrapId = `${cid}__col`; + nodes[wrapId] = { + type: { resolvedName: 'Container' }, + isCanvas: true, + props: { tag: 'div', style: { padding: '0px' } }, + displayName: 'Container', + custom: {}, + hidden: false, + parent: id, + nodes: [cid], + linkedNodes: {}, + }; + syntheticIds.add(wrapId); + nodes[cid].parent = wrapId; + linked[`col-${i}`] = wrapId; + return; + } linked[`col-${i}`] = cid; if (nodes[cid]) nodes[cid].isCanvas = true; // columns are canvases });