[3339 by GPT-6.1 Sol] Visual evidence Fit frame ignores the image's aspect ratio

propr-ui/e2e/task-details-visual-previews.pw.ts

@@ -11,7 +11,7 @@ const previewMedia = [
   { type: 'image', title: 'Goal workspace', description: 'The goal workspace with published evidence.', url: privateAsset('goals') },
 ];
 
-async function fixture(page: Page, title = 'Render visual previews full width') {
+async function fixture(page: Page, title = 'Render visual previews full width', captureSize = { width: 1440, height: 900 }) {
   let image: Buffer | undefined;
   let showMedia = false;
   await page.routeWebSocket('**/socket.io/**', socket => socket.close());
@@ -42,7 +42,7 @@ async function fixture(page: Page, title = 'Render visual previews full width')
     return route.fulfill(path in responses ? { json: responses[path] } : { status: 503, json: { error: 'Unavailable in visual preview fixture' } });
   });
   // Publish a real rendered application screen as the image fixture rather than invented artwork.
-  await page.setViewportSize({ width: 1440, height: 900 });
+  await page.setViewportSize(captureSize);
   await page.goto(`/tasks/${taskId}`);
   await expect(page.getByTestId('task-details')).toBeVisible();
   image = await page.screenshot();
@@ -58,7 +58,7 @@ async function capture(page: Page, name: string) {
 const noHorizontalOverflow = (page: Page) => page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth);
 
 for (const width of [320, 390, 1024, 1440, 2560]) {
-  test(`stacks the context strip over a full-width imaging stage at ${width}px`, async ({ page }) => {
+  test(`fits the imaging stage to the capture at ${width}px`, async ({ page }) => {
     await fixture(page);
     await page.setViewportSize({ width, height: 900 });
     await page.goto(`/tasks/${taskId}`);
@@ -73,13 +73,16 @@ for (const width of [320, 390, 1024, 1440, 2560]) {
       const style = getComputedStyle(node);
       return node.clientWidth - parseFloat(style.paddingLeft) - parseFloat(style.paddingRight);
     });
-    // The stage spans the section and sits below the context strip, not beside it.
-    expect(Math.abs(stageBox.width - sectionContentWidth)).toBeLessThanOrEqual(2);
+    // The image frame stays within the section and sits below the context strip.
+    expect(stageBox.width).toBeLessThanOrEqual(sectionContentWidth);
     const contextBox = (await section.getByTestId('visual-evidence-context').boundingBox())!;
     expect(contextBox.y + contextBox.height).toBeLessThanOrEqual(stageBox.y + 1);
-    expect(Math.abs(stageBox.height - (width >= 640 ? 440 : 320))).toBeLessThanOrEqual(2);
+    expect(stageBox.height).toBeLessThanOrEqual(width >= 640 ? 440 : 320);
     // Fit shows the whole capture: nothing is clipped by the stage.
     const imageBox = (await image.boundingBox())!;
+    expect(Math.abs(stageBox.width - imageBox.width)).toBeLessThanOrEqual(1);
+    expect(Math.abs(stageBox.height - imageBox.height)).toBeLessThanOrEqual(1);
+    expect(Math.abs(imageBox.width / imageBox.height - 1440 / 900)).toBeLessThan(0.01);
     expect(imageBox.x).toBeGreaterThanOrEqual(stageBox.x);
     expect(imageBox.y).toBeGreaterThanOrEqual(stageBox.y);
     expect(imageBox.x + imageBox.width).toBeLessThanOrEqual(stageBox.x + stageBox.width + 0.5);
@@ -94,20 +97,99 @@ for (const width of [320, 390, 1024, 1440, 2560]) {
     await expect(stage).toHaveAttribute('data-zoom', 'actual');
     expect(Math.round((await image.boundingBox())!.width)).toBe(1440 + 2);
     expect(await stage.evaluate(node => node.scrollWidth > node.clientWidth || node.scrollHeight > node.clientHeight)).toBe(true);
-    expect(Math.abs((await stage.boundingBox())!.height - stageBox.height)).toBeLessThanOrEqual(1);
+    expect(Math.abs((await stage.boundingBox())!.height - (width >= 640 ? 440 : 320))).toBeLessThanOrEqual(1);
     if (width === 1440) await capture(page, `task-visual-previews-actual-size-${width}`);
     await section.getByRole('button', { name: 'Fit' }).click();
 
     const switcher = section.getByRole('group', { name: 'Captures' });
     await switcher.getByRole('button', { name: 'Checkout walkthrough' }).click();
     await expect(section.locator('video[controls]')).toHaveCount(1);
-    expect(Math.abs((await stage.boundingBox())!.height - stageBox.height)).toBeLessThanOrEqual(1);
+    expect(Math.abs((await stage.boundingBox())!.height - (width >= 640 ? 440 : 320))).toBeLessThanOrEqual(1);
     await switcher.getByRole('button', { name: 'Desktop' }).click();
     await expect(image).toBeVisible();
     expect(await noHorizontalOverflow(page)).toBe(true);
   });
 }
 
+for (const captureSize of [{ width: 2560, height: 600 }, { width: 390, height: 1800 }]) {
+  test(`fits a ${captureSize.width}×${captureSize.height} capture in a narrow pane`, async ({ page }) => {
+    await fixture(page, 'Fit visual evidence to the capture', captureSize);
+    await page.setViewportSize({ width: 406, height: 900 });
+    await page.goto(`/tasks/${taskId}`);
+    const section = page.getByRole('region', { name: /Visual evidence/ });
+    const image = section.getByAltText('Task queue at desktop width');
+    await expect(section.getByTestId('visual-evidence-dimensions')).toHaveText(`${captureSize.width}×${captureSize.height}`);
+    const frame = (await section.getByTestId('visual-evidence-canvas').boundingBox())!;
+    const imageBox = (await image.boundingBox())!;
+    expect(Math.abs(frame.width - imageBox.width)).toBeLessThanOrEqual(1);
+    expect(Math.abs(frame.height - imageBox.height)).toBeLessThanOrEqual(1);
+    expect(Math.abs(frame.width / frame.height - captureSize.width / captureSize.height)).toBeLessThan(0.01);
+    expect(frame.height).toBeLessThanOrEqual(320);
+    expect(frame.width).toBeLessThanOrEqual(372);
+    expect(await noHorizontalOverflow(page)).toBe(true);
+
+    // An opaque image fills the Fit trigger, so an inset shadow would be hidden beneath it.
+    await section.getByRole('button', { name: 'Lightbox', exact: true }).focus();
+    await page.keyboard.press('Tab');
+    const trigger = section.getByRole('button', { name: 'Open full-size preview: Task queue at desktop width' });
+    await expect(trigger).toBeFocused();
+    expect(await trigger.evaluate(node => node.matches(':focus-visible'))).toBe(true);
+    const shadow = await trigger.evaluate(node => getComputedStyle(node).boxShadow);
+    expect(shadow).toContain('rgb(56, 189, 248) 0px 0px 0px 2px');
+    expect(shadow).not.toContain('inset');
+    await expect(section.getByTestId('visual-evidence-canvas')).toHaveCSS('overflow', 'visible');
+    if (process.env.PROPR_CAPTURE_PREVIEWS) {
+      await mkdir('../.propr/previews', { recursive: true });
+      await section.screenshot({ animations: 'disabled', path: `../.propr/previews/task-visual-previews-fit-focus-${captureSize.width}x${captureSize.height}.png` });
+    }
+  });
+}
+
+for (const width of [390, 1440]) {
+  for (const failure of ['403', '404', 'network', 'decode'] as const) {
+    test(`keeps pending and ${failure} image placeholders visible in Fit at ${width}px`, async ({ page }) => {
+      await fixture(page);
+      await page.setViewportSize({ width, height: 900 });
+      let release!: () => void;
+      const pending = new Promise<void>(resolve => { release = resolve; });
+      await page.route('**/api/preview-media/pulls/acme/web/42/queue', async route => {
+        await pending;
+        if (failure === 'network') return route.abort();
+        if (failure === 'decode') return route.fulfill({ contentType: 'image/png', body: 'Invalid image data' });
+        return route.fulfill({ status: Number(failure), body: 'Image unavailable' });
+      });
+      await page.goto(`/tasks/${taskId}`);
+      const section = page.getByRole('region', { name: /Visual evidence/ });
+      const stage = section.getByTestId('visual-evidence-canvas');
+      const loading = section.getByRole('status', { name: 'Task queue at desktop width — image loading' });
+      await expect(loading).toBeVisible();
+      await expect(stage).toHaveAttribute('data-zoom', 'fit');
+      for (const element of [stage, loading]) {
+        const box = (await element.boundingBox())!;
+        expect(box.height).toBeGreaterThanOrEqual(64);
+        expect(box.width).toBeGreaterThanOrEqual(128);
+      }
+      if (failure === '403' && process.env.PROPR_CAPTURE_PREVIEWS) {
+        await mkdir('../.propr/previews', { recursive: true });
+        await section.screenshot({ animations: 'disabled', path: `../.propr/previews/task-visual-previews-loading-${width}.png` });
+      }
+
+      release();
+      const unavailable = section.getByRole('img', { name: 'Task queue at desktop width — image unavailable' });
+      await expect(unavailable).toBeVisible();
+      for (const element of [stage, unavailable]) {
+        const box = (await element.boundingBox())!;
+        expect(box.height).toBeGreaterThanOrEqual(64);
+        expect(box.width).toBeGreaterThanOrEqual(128);
+      }
+      expect(await noHorizontalOverflow(page)).toBe(true);
+      if (failure === '403' && process.env.PROPR_CAPTURE_PREVIEWS) {
+        await section.screenshot({ animations: 'disabled', path: `../.propr/previews/task-visual-previews-unavailable-${width}.png` });
+      }
+    });
+  }
+}
+
 test('truncates a long title in the collapsed mobile header before the overflow button', async ({ page }) => {
   const longTitle = 'Render visual previews full width across every task details pane and the lightbox';
   expect(longTitle.length).toBeGreaterThanOrEqual(80);

propr-ui/src/components/TaskDetails/TaskVisualPreviews.test.tsx

@@ -5,20 +5,22 @@ import TaskVisualPreviews from './TaskVisualPreviews';
 const asset = (id: string) => `https://github.com/user-attachments/assets/${id}`;
 
 describe('TaskVisualPreviews', () => {
-  it('stacks the context strip over a full-width stage that fits the whole capture', () => {
+  it('stacks the context strip over an intrinsically sized frame that fits the whole capture', () => {
     const { container } = render(<TaskVisualPreviews previews={[
       { type: 'image', title: 'Task list at Desktop 1920px', description: 'Row keeps its full title', url: asset('image') },
       { type: 'video', title: 'Checkout flow', url: asset('video') },
       { type: 'image', title: 'Untrusted', url: 'https://evil.test/image.png' },
     ]} />);
     expect(screen.getByRole('heading', { name: 'Visual evidence (2 captures)' })).toBeInTheDocument();
     const stage = screen.getByTestId('visual-evidence-canvas');
-    expect(stage).toHaveClass('w-full', 'sm:h-[440px]', 'items-center', 'justify-center');
+    expect(stage).toHaveClass('w-fit', 'max-w-full', 'justify-center');
+    expect(stage).not.toHaveClass('h-80', 'sm:h-[440px]', 'p-2');
     expect(stage).toHaveAttribute('data-zoom', 'fit');
     const image = screen.getByAltText('Task list at Desktop 1920px');
     expect(image).toHaveAttribute('src', asset('image'));
     // Fit never clips: the whole capture is scaled into the stage.
-    expect(image).toHaveClass('max-h-full', 'max-w-full', 'object-contain');
+    expect(image).toHaveClass('h-auto', 'w-auto', 'max-h-80', 'sm:max-h-[440px]', 'max-w-full', 'object-contain');
+    expect(image).not.toHaveClass('min-h-16', 'min-w-32', 'bg-white');
     expect(image).not.toHaveClass('object-cover');
     const context = screen.getByTestId('visual-evidence-context');
     expect(within(context).getByText('Row keeps its full title')).toBeInTheDocument();
@@ -42,7 +44,7 @@ describe('TaskVisualPreviews', () => {
     expect(within(zoom).getByRole('button', { name: '100%' })).toHaveAttribute('aria-pressed', 'true');
     const stage = screen.getByTestId('visual-evidence-canvas');
     expect(stage).toHaveAttribute('data-zoom', 'actual');
-    expect(stage).toHaveClass('overflow-auto');
+    expect(stage).toHaveClass('overflow-auto', 'h-80', 'sm:h-[440px]', 'w-full');
     expect(screen.getByAltText('Tall mobile page')).toHaveClass('max-w-none', 'h-auto', 'w-auto');
   });
 

propr-ui/src/components/TaskDetails/TaskVisualPreviews.tsx

@@ -135,7 +135,7 @@ function ContextStrip({ preview, dimensions }: { preview: PublishedVisualPreview
 
 /**
  * Visual evidence published by this specific task run, stacked for inspection: a header with the
- * capture switcher and Fit/100% controls, a one-line context strip, and a full-width imaging stage
+ * capture switcher and Fit/100% controls, a one-line context strip, and an imaging stage
  * that shows the whole capture (Fit) or its pixels 1:1 in a scrollable pane (100%).
  * Renders nothing when the run published none.
  */
@@ -193,18 +193,20 @@ export default function TaskVisualPreviews({ previews, className = 'border-b bor
 
     <figure className="m-0 mt-2 min-w-0 border border-slate-200 bg-white">
       <ContextStrip preview={selected} dimensions={dimensions} />
-      {/* The imaging stage: full width, one fixed height so switching captures never moves the trace below. */}
+      {/* Fit follows the image's intrinsic ratio; only actual-size images and videos use a fixed stage. */}
       <div ref={stageRef} data-testid="visual-evidence-canvas" data-zoom={actual ? 'actual' : 'fit'} style={checkerboard}
-        className={`h-80 w-full sm:h-[440px] ${actual ? 'overflow-auto p-3' : 'flex items-center justify-center p-2'}`}>
+        className={isImage && !actual ? 'mx-auto flex w-fit max-w-full justify-center'
+          : `h-80 w-full sm:h-[440px] ${actual ? 'overflow-auto p-3' : 'flex items-center justify-center p-2'}`}>
         {!isImage
           ? <PreviewVideo key={selected.url} preview={selected} className="max-h-full max-w-full bg-slate-900" />
           // One tree for both modes, so toggling zoom restyles the loaded image instead of fetching it again.
           : <button type="button" aria-haspopup="dialog" aria-label={`Open full-size preview: ${selected.title}`}
             onClick={event => openLightbox(event.currentTarget)}
-            className={`cursor-zoom-in focus:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-sky-400 ${actual ? 'block w-max' : 'flex h-full w-full items-center justify-center'}`}>
+            className={`cursor-zoom-in focus:outline-none focus-visible:ring-2 focus-visible:ring-sky-400 ${actual ? 'block w-max focus-visible:ring-inset' : 'flex min-w-0 max-w-full items-center justify-center [&>span]:min-h-16 [&>span]:min-w-32'}`}>
+            {/* Placeholder spans need their own size before an image has intrinsic dimensions. */}
             <PreviewImage key={selected.url} preview={selected} className={actual
               ? 'block h-auto w-auto max-w-none border border-slate-200/80 bg-white'
-              : 'min-h-16 min-w-32 max-h-full max-w-full border border-slate-200/80 bg-white object-contain shadow-sm'} />
+              : 'block h-auto w-auto max-h-80 max-w-full object-contain sm:max-h-[440px]'} />
           </button>}
       </div>
     </figure>