diff --git a/propr-ui/e2e/task-details-visual-previews.pw.ts b/propr-ui/e2e/task-details-visual-previews.pw.ts index a878ae5c31..5386934b89 100644 --- a/propr-ui/e2e/task-details-visual-previews.pw.ts +++ b/propr-ui/e2e/task-details-visual-previews.pw.ts @@ -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(`frames one capture at a time in a fixed-height canvas at ${width}px`, async ({ page }) => { + test(`stacks the context strip over a full-width imaging stage at ${width}px`, async ({ page }) => { await fixture(page); await page.setViewportSize({ width, height: 900 }); await page.goto(`/tasks/${taskId}`); @@ -66,28 +66,45 @@ for (const width of [320, 390, 1024, 1440, 2560]) { await expect(section.getByRole('heading', { name: 'Visual evidence (3 captures)' })).toBeVisible(); const image = section.getByAltText('Task queue at desktop width'); await expect(image).toBeVisible(); - const canvas = section.getByTestId('visual-evidence-canvas'); - const canvasBox = (await canvas.boundingBox())!; - if (width >= 640) { - // Side by side: the framed capture takes three fifths, its description the rest, 224px tall. - expect(Math.abs(canvasBox.height - 224)).toBeLessThanOrEqual(1); - const imageBox = (await image.boundingBox())!; - expect(imageBox.width / canvasBox.width).toBeGreaterThan(0.55); - expect(imageBox.width / canvasBox.width).toBeLessThan(0.62); - } else { - expect(canvasBox.height).toBeLessThanOrEqual(420); - } + await expect.poll(() => image.evaluate(node => (node as HTMLImageElement).complete && (node as HTMLImageElement).naturalWidth)).toBeGreaterThan(0); + const stage = section.getByTestId('visual-evidence-canvas'); + const stageBox = (await stage.boundingBox())!; + const sectionContentWidth = await section.evaluate(node => { + 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); + 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); + // Fit shows the whole capture: nothing is clipped by the stage. + const imageBox = (await image.boundingBox())!; + expect(imageBox.x).toBeGreaterThanOrEqual(stageBox.x); + expect(imageBox.y).toBeGreaterThanOrEqual(stageBox.y); + expect(imageBox.x + imageBox.width).toBeLessThanOrEqual(stageBox.x + stageBox.width + 0.5); + expect(imageBox.y + imageBox.height).toBeLessThanOrEqual(stageBox.y + stageBox.height + 0.5); + await expect(section.getByTestId('visual-evidence-dimensions')).toHaveText(`1440×900`); await expect(section.getByText('Private PR evidence served through the authenticated application media path.')).toBeVisible(); await expect(section.locator('img, video')).toHaveCount(1); + if (width === 390 || width === 1440) await capture(page, `task-visual-previews-${width}`); + + // 100% renders the capture's own pixels and scrolls inside the stage. + await section.getByRole('button', { name: '100%' }).click(); + 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); + 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: 'Capture 2' }).click(); + await switcher.getByRole('button', { name: 'Checkout walkthrough' }).click(); await expect(section.locator('video[controls]')).toHaveCount(1); - expect(Math.abs((await canvas.boundingBox())!.height - canvasBox.height)).toBeLessThanOrEqual(1); + expect(Math.abs((await stage.boundingBox())!.height - stageBox.height)).toBeLessThanOrEqual(1); await switcher.getByRole('button', { name: 'Desktop' }).click(); await expect(image).toBeVisible(); expect(await noHorizontalOverflow(page)).toBe(true); - if (width === 390 || width === 1440) await capture(page, `task-visual-previews-${width}`); }); } diff --git a/propr-ui/src/components/TaskDetails/TaskVisualPreviews.test.tsx b/propr-ui/src/components/TaskDetails/TaskVisualPreviews.test.tsx index c0b43685ff..5c133d5860 100644 --- a/propr-ui/src/components/TaskDetails/TaskVisualPreviews.test.tsx +++ b/propr-ui/src/components/TaskDetails/TaskVisualPreviews.test.tsx @@ -5,26 +5,28 @@ import TaskVisualPreviews from './TaskVisualPreviews'; const asset = (id: string) => `https://github.com/user-attachments/assets/${id}`; describe('TaskVisualPreviews', () => { - it('frames one capture at a time beside its title, description and viewport', () => { + it('stacks the context strip over a full-width stage that fits the whole capture', () => { const { container } = render(); expect(screen.getByRole('heading', { name: 'Visual evidence (2 captures)' })).toBeInTheDocument(); - const canvas = screen.getByTestId('visual-evidence-canvas'); - expect(canvas).toHaveClass('sm:h-56', 'sm:flex-row'); + const stage = screen.getByTestId('visual-evidence-canvas'); + expect(stage).toHaveClass('w-full', 'sm:h-[440px]', 'items-center', 'justify-center'); + expect(stage).toHaveAttribute('data-zoom', 'fit'); const image = screen.getByAltText('Task list at Desktop 1920px'); expect(image).toHaveAttribute('src', asset('image')); - expect(image).toHaveClass('h-full', 'w-full', 'object-cover', 'object-left-top'); - expect(within(canvas).getByText('Row keeps its full title')).toBeInTheDocument(); - expect(within(canvas).getByText('Desktop 1920px')).toBeInTheDocument(); - // A light browser window frames the capture, sized like one, with no black bezel around it. - const frame = within(canvas).getByTestId('visual-evidence-window'); - expect(frame).toHaveClass('border-slate-200', 'bg-white'); - expect(within(frame).getByTestId('visual-evidence-window-size')).toHaveTextContent(/^1920px$/); - expect(canvas.innerHTML).not.toMatch(/bg-slate-950 p-|bg-slate-900 /); - expect(screen.getByText('Click to inspect full resolution')).toBeInTheDocument(); + // Fit never clips: the whole capture is scaled into the stage. + expect(image).toHaveClass('max-h-full', 'max-w-full', 'object-contain'); + expect(image).not.toHaveClass('object-cover'); + const context = screen.getByTestId('visual-evidence-context'); + expect(within(context).getByText('Row keeps its full title')).toBeInTheDocument(); + expect(within(context).getByText('Desktop 1920px')).toBeInTheDocument(); + expect(within(context).getByText('Screenshot')).toBeInTheDocument(); + // No faux browser window around the capture. + expect(screen.queryByTestId('visual-evidence-window')).toBeNull(); + expect(container.querySelectorAll('.rounded-full')).toHaveLength(0); expect(screen.getByRole('link', { name: 'Open original: Task list at Desktop 1920px' })).toHaveAttribute('href', asset('image')); // Only the selected capture is on screen: the video waits behind the switcher. expect(container.querySelector('video')).toBeNull(); @@ -32,6 +34,42 @@ describe('TaskVisualPreviews', () => { expect(container.querySelector('img[src="https://evil.test/image.png"]')).toBeNull(); }); + it('shows the capture at its real pixel size in a scrollable stage at 100%', () => { + render(); + const zoom = screen.getByRole('group', { name: 'Zoom' }); + expect(within(zoom).getByRole('button', { name: 'Fit' })).toHaveAttribute('aria-pressed', 'true'); + fireEvent.click(within(zoom).getByRole('button', { name: '100%' })); + 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(screen.getByAltText('Tall mobile page')).toHaveClass('max-w-none', 'h-auto', 'w-auto'); + }); + + it('reports the captured resolution once the image loads', () => { + render(); + const image = screen.getByAltText('Dialog'); + Object.defineProperty(image, 'naturalWidth', { value: 1440 }); + Object.defineProperty(image, 'naturalHeight', { value: 900 }); + fireEvent.load(image); + expect(screen.getByTestId('visual-evidence-dimensions')).toHaveTextContent('1440×900'); + }); + + it('numbers captures in the switcher, marks their device and flags failed ones', () => { + render(); + const switcher = screen.getByRole('group', { name: 'Captures' }); + expect(within(switcher).getAllByRole('button').map(button => button.textContent)).toEqual(['01Desktop 1440px', '02390px', '03Checkout assertion passed']); + expect(within(switcher).getByRole('button', { name: '390px' })).toHaveClass('border-red-300', 'text-red-700', 'bg-red-50'); + expect(within(switcher).getByRole('button', { name: 'Desktop 1440px' })).not.toHaveClass('border-red-300'); + const passed = within(switcher).getByRole('button', { name: 'Checkout assertion passed' }); + expect(passed).not.toHaveClass('border-red-300', 'text-red-700', 'bg-red-50'); + expect(passed).toHaveClass('border-transparent', 'text-slate-600'); + }); + it('switches captures in place and plays videos with their controls', () => { const { container } = render( { { type: 'video', title: 'Walkthrough', url: asset('video') }, ]} />); const switcher = screen.getByRole('group', { name: 'Captures' }); - expect(within(switcher).getAllByRole('button').map(button => button.textContent)).toEqual(['Before', 'After', 'Capture 3']); + expect(within(switcher).getAllByRole('button').map(button => button.textContent)).toEqual(['01Before', '02After', '03Walkthrough']); expect(within(switcher).getByRole('button', { name: 'Before' })).toHaveAttribute('aria-pressed', 'true'); fireEvent.click(within(switcher).getByRole('button', { name: 'After' })); @@ -47,11 +85,12 @@ describe('TaskVisualPreviews', () => { expect(screen.queryByAltText('Before the fix')).toBeNull(); expect(screen.getByText('Mobile 390px')).toBeInTheDocument(); - fireEvent.click(within(switcher).getByRole('button', { name: 'Capture 3' })); + fireEvent.click(within(switcher).getByRole('button', { name: 'Walkthrough' })); const video = container.querySelector('video'); expect(video).toHaveAttribute('src', asset('video')); expect(video).toHaveAttribute('controls'); expect(screen.queryByRole('button', { name: 'Lightbox' })).toBeNull(); + expect(screen.queryByRole('group', { name: 'Zoom' })).toBeNull(); }); it('opens images in an in-app lightbox, skips videos, and restores focus on close', () => { @@ -60,7 +99,7 @@ describe('TaskVisualPreviews', () => { { type: 'video', title: 'Walkthrough', url: asset('video') }, { type: 'image', title: 'Second', url: asset('second') }, ]} />); - fireEvent.click(screen.getByRole('button', { name: 'Capture 3' })); + fireEvent.click(screen.getByRole('button', { name: 'Second' })); const trigger = screen.getByRole('button', { name: 'Open full-size preview: Second' }); expect(trigger).toHaveAttribute('aria-haspopup', 'dialog'); trigger.focus(); @@ -76,7 +115,7 @@ describe('TaskVisualPreviews', () => { expect(screen.queryByRole('dialog')).toBeNull(); expect(trigger).toHaveFocus(); // The canvas follows the capture the lightbox was left on. - expect(screen.getByRole('button', { name: 'Capture 1' })).toHaveAttribute('aria-pressed', 'true'); + expect(screen.getByRole('button', { name: 'First' })).toHaveAttribute('aria-pressed', 'true'); }); it('opens the lightbox from the header button too', () => { diff --git a/propr-ui/src/components/TaskDetails/TaskVisualPreviews.tsx b/propr-ui/src/components/TaskDetails/TaskVisualPreviews.tsx index 4e0b1d5810..a66cf4cc19 100644 --- a/propr-ui/src/components/TaskDetails/TaskVisualPreviews.tsx +++ b/propr-ui/src/components/TaskDetails/TaskVisualPreviews.tsx @@ -1,15 +1,28 @@ -import { useEffect, useRef, useState } from 'react'; -import { ExternalLink, Maximize2, ZoomIn } from 'lucide-react'; +import { useEffect, useLayoutEffect, useRef, useState, type CSSProperties, type RefObject } from 'react'; +import { ArrowUpRight, Film, Image as ImageIcon, Maximize2, Monitor, Smartphone, Tablet } from 'lucide-react'; import { trustedApplicationPreviewMediaUrl, trustedPreviewMedia, type PublishedVisualPreview } from '@propr/shared'; import { PreviewImage, PreviewVideo } from '../PreviewMedia'; import PreviewLightbox from '../PreviewLightbox'; +type DeviceKind = 'desktop' | 'tablet' | 'mobile'; +type ZoomMode = 'fit' | 'actual'; + /** The width a capture was taken at, when its title or description says, e.g. `1920`. */ function captureWidth(preview: PublishedVisualPreview): string | null { const text = `${preview.title} ${preview.description ?? ''}`; return text.match(/\b(\d{3,4})\s*(?:px\b|[x×]\s*\d{3,4})/i)?.[1] ?? null; } +/** The device class a capture was taken on: named in its title or description, or implied by its width. */ +function captureDevice(preview: PublishedVisualPreview): DeviceKind | null { + const text = `${preview.title} ${preview.description ?? ''}`; + const named = text.match(/\b(desktop|tablet|mobile)\b/i)?.[1]?.toLowerCase() as DeviceKind | undefined; + if (named) return named; + const width = Number(captureWidth(preview)); + if (!width) return null; + return width < 640 ? 'mobile' : width < 1024 ? 'tablet' : 'desktop'; +} + /** The viewport a capture was taken at, when its title or description says, e.g. `Desktop 1920px`. */ function captureViewport(preview: PublishedVisualPreview): string | null { const text = `${preview.title} ${preview.description ?? ''}`; @@ -20,18 +33,110 @@ function captureViewport(preview: PublishedVisualPreview): string | null { return label ?? (width ? `${width}px` : null); } -/** A short name for a capture in the switcher: its viewport, a before/after side, or its position. */ -function captureLabel(preview: PublishedVisualPreview, index: number): string { +/** A short name for a capture in the switcher: a before/after side, its viewport, or its own title. */ +function captureLabel(preview: PublishedVisualPreview): string { const side = preview.title.match(/\b(before|after)\b/i)?.[1]; if (side) return side[0].toUpperCase() + side.slice(1).toLowerCase(); - return captureViewport(preview) ?? `Capture ${index + 1}`; + return captureViewport(preview) ?? preview.title; +} + +/** Captures whose title reports a failure are flagged in the switcher; naming an assertion alone is not a failure. */ +const isFailureCapture = (preview: PublishedVisualPreview) => /\bfail(?:s|ed|ing|ure)?\b/i.test(preview.title); + +function CaptureIcon({ preview }: { preview: PublishedVisualPreview }) { + const device = captureDevice(preview); + const Icon = preview.type === 'video' ? Film : device ? { desktop: Monitor, tablet: Tablet, mobile: Smartphone }[device] : ImageIcon; + return