From 5c44dd545c19414dd1a9f0ddb1a7b503a06a530c Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Tue, 14 Jul 2026 16:04:50 -0700 Subject: [PATCH] fix(site-builder): bounce stays visible + springier; image/video crop fills (cover) + resize shrinks footprint Co-Authored-By: Claude Opus 4.8 (1M context) --- .../right/styles/ImageStylePanel.test.tsx | 133 ++++++++++++++++++ .../panels/right/styles/ImageStylePanel.tsx | 31 +++- .../panels/right/styles/MediaStylePanel.tsx | 19 ++- .../styles/MediaStylePanel.video.test.tsx | 52 +++++++ craft/src/utils/html-export.test.ts | 52 +++++++ craft/src/utils/html-export.ts | 4 +- 6 files changed, 282 insertions(+), 9 deletions(-) create mode 100644 craft/src/panels/right/styles/ImageStylePanel.test.tsx diff --git a/craft/src/panels/right/styles/ImageStylePanel.test.tsx b/craft/src/panels/right/styles/ImageStylePanel.test.tsx new file mode 100644 index 0000000..80311ac --- /dev/null +++ b/craft/src/panels/right/styles/ImageStylePanel.test.tsx @@ -0,0 +1,133 @@ +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'; + +/* Same DOM-harness pattern as MediaStylePanel.video.test.tsx -- mock + @craftjs/core's useEditor so setProp calls can be observed without + mounting a real tree, and mock utils/assets so AssetPicker + doesn't hit the network. */ +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 { ImageStylePanel } from './ImageStylePanel'; + +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 qAll(testId: string): T[] { + return Array.from(container.querySelectorAll(`[data-testid="${testId}"]`)); +} + +/** Click the preset button with this exact label inside a given data-testid + * root (AspectRatioControl / PresetButtonGrid render plain buttons keyed by + * label, no per-button testid). */ +function clickPresetByLabel(root: Element | null, label: string) { + const btn = Array.from(root?.querySelectorAll('button') ?? []).find((b) => b.textContent === label); + expect(btn).toBeTruthy(); + act(() => { btn!.dispatchEvent(new MouseEvent('click', { bubbles: true })); }); +} + +beforeEach(() => { + setPropSpy.mockClear(); +}); + +afterEach(() => { + if (container) unmount(); +}); + +describe('ImageStylePanel crop-fills-by-default (fix-anim-image B)', () => { + test('applying a non-empty aspect ratio with objectFit unset also sets objectFit to cover', () => { + lastProps = { src: '/uploads/photo.jpg', alt: '', style: {} }; + render(); + + clickPresetByLabel(q('aspect-ratio-control'), '1:1'); + + expect(lastProps.style.aspectRatio).toBe('1 / 1'); + expect(lastProps.style.objectFit).toBe('cover'); + }); + + test('applying a ratio when objectFit is already "contain" leaves it as contain (no forced override)', () => { + lastProps = { src: '/uploads/photo.jpg', alt: '', style: { objectFit: 'contain' } }; + render(); + + clickPresetByLabel(q('aspect-ratio-control'), '16:9'); + + expect(lastProps.style.aspectRatio).toBe('16 / 9'); + expect(lastProps.style.objectFit).toBe('contain'); + }); + + test('clearing the ratio (Original) does not force-clear objectFit', () => { + lastProps = { src: '/uploads/photo.jpg', alt: '', style: { aspectRatio: '1 / 1', objectFit: 'cover' } }; + render(); + + clickPresetByLabel(q('aspect-ratio-control'), 'Original'); + + expect(lastProps.style.aspectRatio).toBe(''); + expect(lastProps.style.objectFit).toBe('cover'); + }); + + test('the Object Fit control (Cover/Contain/Fill/None) remains present and usable', () => { + lastProps = { src: '/uploads/photo.jpg', alt: '', style: { aspectRatio: '1 / 1', objectFit: 'cover' } }; + render(); + + // PresetButtonGrid buttons have no dedicated per-button testid; find by label text. + const containBtn = Array.from(container.querySelectorAll('button')).find((b) => b.textContent === 'Contain'); + expect(containBtn).toBeTruthy(); + act(() => { containBtn!.dispatchEvent(new MouseEvent('click', { bubbles: true })); }); + expect(lastProps.style.objectFit).toBe('contain'); + }); +}); + +describe('ImageStylePanel Height control gated on aspect-ratio (fix-anim-image C)', () => { + test('no aspect-ratio set: both Width and Height SizeControls render', () => { + lastProps = { src: '/uploads/photo.jpg', alt: '', style: {} }; + render(); + expect(qAll('size-control').length).toBe(2); + }); + + test('aspect-ratio set: only the Width SizeControl renders (Height is hidden)', () => { + lastProps = { src: '/uploads/photo.jpg', alt: '', style: { aspectRatio: '1 / 1', objectFit: 'cover' } }; + render(); + expect(qAll('size-control').length).toBe(1); + }); + + test('applying an aspect ratio clears a stale height so it cannot linger and conflict', () => { + lastProps = { src: '/uploads/photo.jpg', alt: '', style: { height: '50%' } }; + render(); + + clickPresetByLabel(q('aspect-ratio-control'), '9:16'); + + expect(lastProps.style.aspectRatio).toBe('9 / 16'); + expect(lastProps.style.height).toBe(''); + }); +}); diff --git a/craft/src/panels/right/styles/ImageStylePanel.tsx b/craft/src/panels/right/styles/ImageStylePanel.tsx index e43121f..2d55e3c 100644 --- a/craft/src/panels/right/styles/ImageStylePanel.tsx +++ b/craft/src/panels/right/styles/ImageStylePanel.tsx @@ -25,6 +25,19 @@ export const ImageStylePanel: React.FC = ({ selectedId, nodePro const { setProp, setPropStyle } = useNodeProp(selectedId); + // Applying an aspect-ratio crop should FILL the frame by default (object-fit: + // cover) rather than leave letterboxed empty bands, and width + ratio + cover + // fully determine the box -- so a stale `height` can't linger and conflict + // (kills the %-height no-op that made resize look like it wasn't working). + // Clearing the ratio (back to 'Original') leaves objectFit as the user left it. + const applyAspectRatio = (v: string) => { + setPropStyle('aspectRatio', v); + if (v) { + if (!style.objectFit) setPropStyle('objectFit', 'cover'); + setPropStyle('height', ''); + } + }; + return ( <> {/* Image source */} @@ -54,18 +67,24 @@ export const ImageStylePanel: React.FC = ({ selectedId, nodePro value={(style.maxWidth as string) || ''} onChange={(v) => setPropStyle('maxWidth', v)} /> - setPropStyle('height', v)} - /> + {/* Height is only meaningful when there's no aspect-ratio crop -- once a + ratio is set, Width + ratio + cover fully determine the box, so a + separate Height control would only conflict/mislead (see + applyAspectRatio, which clears any stale height at that moment). */} + {!style.aspectRatio && ( + setPropStyle('height', v)} + /> + )} {/* Crop & Framing -- aspect-ratio + object-fit + object-position on the itself is a CSS framing crop (no server-side image processing needed). */} setPropStyle('aspectRatio', v)} + onChange={applyAspectRatio} />
Object Fit diff --git a/craft/src/panels/right/styles/MediaStylePanel.tsx b/craft/src/panels/right/styles/MediaStylePanel.tsx index fd9a431..1f71c21 100644 --- a/craft/src/panels/right/styles/MediaStylePanel.tsx +++ b/craft/src/panels/right/styles/MediaStylePanel.tsx @@ -37,6 +37,18 @@ export const MediaStylePanel: React.FC = ({ selectedId, nodePro const style = nodeProps.style || {}; + // Same crop-fills-by-default + no stale-height-conflict treatment as + // ImageStylePanel (see there for the full rationale): applying a ratio + // defaults objectFit to 'cover' when unset, and clears height so Width + + // ratio + cover is the single source of truth for the box. + const applyAspectRatio = (v: string) => { + setPropStyle('aspectRatio', v); + if (v) { + if (!style.objectFit) setPropStyle('objectFit', 'cover'); + setPropStyle('height', ''); + } + }; + return ( <> {/* Video source -- upload/browse/paste-URL (paste-URL still handles @@ -63,9 +75,14 @@ export const MediaStylePanel: React.FC = ({ selectedId, nodePro value={(style.width as string) || ''} onChange={(v) => setPropStyle('width', v)} /> + {/* NOTE: unlike ImageStylePanel, there is no separate Height control + here to gate on aspect-ratio -- VideoBlock's