fix(site-builder): bounce stays visible + springier; image/video crop fills (cover) + resize shrinks footprint

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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 <Editor> 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<T extends Element = Element>(testId: string): T | null {
+  return container.querySelector(`[data-testid="${testId}"]`);
+}
+
+function qAll<T extends Element = Element>(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(<ImageStylePanel selectedId="img-1" nodeProps={lastProps} />);
+
+    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(<ImageStylePanel selectedId="img-1" nodeProps={lastProps} />);
+
+    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(<ImageStylePanel selectedId="img-1" nodeProps={lastProps} />);
+
+    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(<ImageStylePanel selectedId="img-1" nodeProps={lastProps} />);
+
+    // 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(<ImageStylePanel selectedId="img-1" nodeProps={lastProps} />);
+    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(<ImageStylePanel selectedId="img-1" nodeProps={lastProps} />);
+    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(<ImageStylePanel selectedId="img-1" nodeProps={lastProps} />);
+
+    clickPresetByLabel(q('aspect-ratio-control'), '9:16');
+
+    expect(lastProps.style.aspectRatio).toBe('9 / 16');
+    expect(lastProps.style.height).toBe('');
+  });
+});

craft/src/panels/right/styles/ImageStylePanel.tsx

@@ -25,6 +25,19 @@ export const ImageStylePanel: React.FC<StylePanelProps> = ({ 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<StylePanelProps> = ({ selectedId, nodePro
         value={(style.maxWidth as string) || ''}
         onChange={(v) => setPropStyle('maxWidth', v)}
       />
-      <SizeControl
-        label="Height"
-        value={(style.height as string) || ''}
-        onChange={(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 && (
+        <SizeControl
+          label="Height"
+          value={(style.height as string) || ''}
+          onChange={(v) => setPropStyle('height', v)}
+        />
+      )}
 
       {/* Crop & Framing -- aspect-ratio + object-fit + object-position on the
           <img> itself is a CSS framing crop (no server-side image processing
           needed). */}
       <AspectRatioControl
         value={(style.aspectRatio as string) || ''}
-        onChange={(v) => setPropStyle('aspectRatio', v)}
+        onChange={applyAspectRatio}
       />
       <div className="guided-section">
         <SectionLabel>Object Fit</SectionLabel>

craft/src/panels/right/styles/MediaStylePanel.tsx

@@ -37,6 +37,18 @@ export const MediaStylePanel: React.FC<StylePanelProps> = ({ 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<StylePanelProps> = ({ 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 <video>/iframe size
+              themselves from `width` + `aspectRatio` directly (see
+              VideoBlock.tsx), not from an outer-wrapper height, so adding one
+              would reintroduce the exact empty-space bug this fix targets. */}
           <AspectRatioControl
             value={(style.aspectRatio as string) || ''}
-            onChange={(v) => setPropStyle('aspectRatio', v)}
+            onChange={applyAspectRatio}
           />
         </>
       )}

craft/src/panels/right/styles/MediaStylePanel.video.test.tsx

@@ -121,6 +121,58 @@ describe('MediaStylePanel Video size controls (Width + Aspect Ratio) are gated o
     expect(q('size-control')).toBeNull();
     expect(q('aspect-ratio-control')).toBeNull();
   });
+
+  test('a video-shaped selection only renders one SizeControl (Width) -- no separate Height control', () => {
+    // See MediaStylePanel.tsx's note: VideoBlock sizes its <video>/iframe from
+    // width + aspectRatio directly, not from an outer-wrapper height, so a
+    // Height control would reintroduce the empty-space bug this fix targets.
+    lastProps = { videoUrl: 'https://example.com/clip.mp4', poster: '', style: {} };
+    render(<MediaStylePanel selectedId="vid-1" nodeProps={lastProps} />);
+    expect(qAll('size-control').length).toBe(1);
+  });
+});
+
+/* FIX (fix-anim-image contract, B+C): applying a crop aspect-ratio to a video
+   should fill by default (objectFit defaults to 'cover' when unset) and clear
+   any stale height, matching ImageStylePanel's treatment -- for prop-schema
+   consistency even though VideoBlock's file-type <video> already hardcodes
+   object-fit: cover today. */
+describe('MediaStylePanel Video AspectRatioControl applies crop-fills-by-default treatment (fix-anim-image B+C)', () => {
+  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 })); });
+  }
+
+  test('applying a non-empty ratio with objectFit unset also sets objectFit to cover', () => {
+    lastProps = { videoUrl: 'https://example.com/clip.mp4', poster: '', style: {} };
+    render(<MediaStylePanel selectedId="vid-1" nodeProps={lastProps} />);
+
+    clickPresetByLabel(q('aspect-ratio-control'), '1:1');
+
+    expect(lastProps.style.aspectRatio).toBe('1 / 1');
+    expect(lastProps.style.objectFit).toBe('cover');
+  });
+
+  test('applying a ratio clears a stale height', () => {
+    lastProps = { videoUrl: 'https://example.com/clip.mp4', poster: '', style: { height: '50%' } };
+    render(<MediaStylePanel selectedId="vid-1" nodeProps={lastProps} />);
+
+    clickPresetByLabel(q('aspect-ratio-control'), '16:9');
+
+    expect(lastProps.style.aspectRatio).toBe('16 / 9');
+    expect(lastProps.style.height).toBe('');
+  });
+
+  test('clearing the ratio (Original) does not force-clear objectFit', () => {
+    lastProps = { videoUrl: 'https://example.com/clip.mp4', poster: '', style: { aspectRatio: '1 / 1', objectFit: 'cover' } };
+    render(<MediaStylePanel selectedId="vid-1" nodeProps={lastProps} />);
+
+    clickPresetByLabel(q('aspect-ratio-control'), 'Original');
+
+    expect(lastProps.style.aspectRatio).toBe('');
+    expect(lastProps.style.objectFit).toBe('cover');
+  });
 });
 
 function openCollapsibleByTitle(title: string) {