From 25507cb57a5dba9a81a43eec360cd0348e9dcb65 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 12 Jul 2026 17:32:47 -0700 Subject: [PATCH] Fix INT: ContentSlider Slides editor wrote wrong prop key (image vs imageSrc) MediaStylePanel's Slides array editor guarded on item.image !== undefined and wrote `image` via AssetPicker's onChange, but ContentSlider (render + toHtml) reads slide.imageSrc. Default slides (imageSrc:'', no `image` key) never showed an image picker at all, and any `image` value written was a silent no-op on render/export. Editor now guards/reads/writes `imageSrc` throughout, and the "add slide" emptyItem matches defaultSlides' exact shape (type/imageSrc/heading/text/bgColor). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../sections/ContentSlider.toHtml.test.ts | 19 +++ .../styles/MediaStylePanel.slides.test.tsx | 116 ++++++++++++++++++ .../panels/right/styles/MediaStylePanel.tsx | 8 +- 3 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 craft/src/panels/right/styles/MediaStylePanel.slides.test.tsx diff --git a/craft/src/components/sections/ContentSlider.toHtml.test.ts b/craft/src/components/sections/ContentSlider.toHtml.test.ts index c66491b..66d8060 100644 --- a/craft/src/components/sections/ContentSlider.toHtml.test.ts +++ b/craft/src/components/sections/ContentSlider.toHtml.test.ts @@ -91,3 +91,22 @@ describe('ContentSlider.toHtml autoplay silences aria-live and is pausable (F-ex expect(html).not.toMatch(/visibilitychange/); }); }); + +describe('ContentSlider.toHtml renders slide.imageSrc as a background-image (INT)', () => { + test('a slide with imageSrc set exports a background-image referencing it', () => { + const slidesWithImage = [ + { type: 'image' as const, imageSrc: 'https://example.com/photo.jpg', heading: 'One' }, + ]; + const { html } = toHtml({ slides: slidesWithImage }, ''); + expect(html).toContain("background-image:url('https://example.com/photo.jpg')"); + }); + + test('a slide with no imageSrc falls back to bgColor (no broken/empty background-image url)', () => { + const slidesNoImage = [ + { type: 'image' as const, imageSrc: '', heading: 'One', bgColor: '#123456' }, + ]; + const { html } = toHtml({ slides: slidesNoImage }, ''); + expect(html).not.toContain('background-image:url('); + expect(html).toContain('background-color:#123456'); + }); +}); diff --git a/craft/src/panels/right/styles/MediaStylePanel.slides.test.tsx b/craft/src/panels/right/styles/MediaStylePanel.slides.test.tsx new file mode 100644 index 0000000..97a304c --- /dev/null +++ b/craft/src/panels/right/styles/MediaStylePanel.slides.test.tsx @@ -0,0 +1,116 @@ +import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; + +/* MediaStylePanel (via useNodeProp/ArrayPropEditor in ./shared) only needs + useEditor from @craftjs/core. Mock it following the DOM-harness pattern + used in Footer.editguard.test.tsx / AssetPicker.test.tsx (no + @testing-library/react in this repo) so setProp calls can be observed + without mounting a real tree. */ +const setPropSpy = vi.fn((_id: string, updater: (p: any) => void) => { + updater(lastProps); +}); +let lastProps: any; + +vi.mock('@craftjs/core', () => ({ + useEditor: () => ({ actions: { setProp: setPropSpy } }), +})); + +vi.mock('../../../utils/assets', () => ({ + uploadAsset: vi.fn(), + listAssets: vi.fn(), +})); + +import { MediaStylePanel } from './MediaStylePanel'; + +let container: HTMLDivElement; +let root: Root; + +function render(ui: React.ReactElement) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(ui); + }); +} + +function unmount() { + act(() => { root.unmount(); }); + container.remove(); +} + +function q(testId: string): T | null { + return container.querySelector(`[data-testid="${testId}"]`); +} + +function setValue(input: HTMLInputElement, value: string) { + const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, 'value')!.set!; + setter.call(input, value); + input.dispatchEvent(new Event('input', { bubbles: true })); +} + +beforeEach(() => { + setPropSpy.mockClear(); +}); + +afterEach(() => { + if (container) unmount(); +}); + +/** + * INT: ContentSlider (src/components/sections/ContentSlider.tsx) reads + * `slide.imageSrc` in both render and toHtml, but the Slides array editor + * here guarded on `item.image !== undefined` and wrote `image` via + * AssetPicker's onChange -- a key that ContentSlider never reads. Default + * slides (`imageSrc:''`, no `image` key) therefore showed NO image picker at + * all (the guard was never true), and even a manually-added `image` value + * was a silent no-op on export/render. + */ +describe('MediaStylePanel Slides editor uses imageSrc (INT)', () => { + test('a slide shaped like defaultSlides (imageSrc present, no `image` key) renders the image AssetPicker', () => { + lastProps = { slides: [{ type: 'image', imageSrc: '', heading: 'First Slide', text: '', bgColor: '' }] }; + render(); + + // Before the fix, the guard `item.image !== undefined` was false for + // this default-shaped slide, so no AssetPicker rendered at all. + expect(q('asset-picker-compact')).not.toBeNull(); + }); + + test('editing the slide image writes imageSrc (not image) via setProp', () => { + lastProps = { slides: [{ type: 'image', imageSrc: '', heading: 'First Slide', text: '', bgColor: '' }] }; + render(); + + const urlInput = q('asset-picker-url-input')!; + expect(urlInput).not.toBeNull(); + setValue(urlInput, 'https://example.com/slide.jpg'); + act(() => { + urlInput.dispatchEvent(new FocusEvent('focusout', { bubbles: true })); + }); + + expect(setPropSpy).toHaveBeenCalled(); + const updatedSlide = lastProps.slides[0]; + expect(updatedSlide.imageSrc).toBe('https://example.com/slide.jpg'); + expect(updatedSlide.image).toBeUndefined(); + }); + + test('adding a new slide (emptyItem) includes imageSrc, matching defaultSlides shape', () => { + lastProps = { slides: [] }; + render(); + + // The "+ Add Item" button for the Slides ArrayPropEditor. + const addButtons = Array.from(container.querySelectorAll('button')).filter( + (b) => b.textContent?.includes('Add Item'), + ); + expect(addButtons.length).toBeGreaterThan(0); + act(() => { + addButtons[0].dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + expect(setPropSpy).toHaveBeenCalled(); + expect(lastProps.slides).toHaveLength(1); + expect(lastProps.slides[0]).toHaveProperty('imageSrc'); + expect(lastProps.slides[0].image).toBeUndefined(); + }); +}); diff --git a/craft/src/panels/right/styles/MediaStylePanel.tsx b/craft/src/panels/right/styles/MediaStylePanel.tsx index 6100e9b..8219dbb 100644 --- a/craft/src/panels/right/styles/MediaStylePanel.tsx +++ b/craft/src/panels/right/styles/MediaStylePanel.tsx @@ -145,14 +145,14 @@ export const MediaStylePanel: React.FC = ({ selectedId, nodePro }); }} placeholder="Text" rows={2} style={{ ...smallInputStyle, resize: 'vertical' }} /> )} - {item.image !== undefined && ( + {item.imageSrc !== undefined && ( { actions.setProp(selectedId, (props: any) => { const updated = [...(props.slides || [])]; - updated[index] = { ...updated[index], image: url }; + updated[index] = { ...updated[index], imageSrc: url }; props.slides = updated; }); }} @@ -160,7 +160,7 @@ export const MediaStylePanel: React.FC = ({ selectedId, nodePro )} )} - emptyItem={{ heading: 'New Slide', text: '', image: '' }} + emptyItem={{ type: 'image', imageSrc: '', heading: 'New Slide', text: '', bgColor: '' }} /> )}