Skip to content

[3260 by Claude Opus 5.5] The current "Visual Evidence" component fails basic diagnostic ergonomics because it treats functional test artifacts like a Dribbble marketing showcase. #3261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
integry merged 2 commits into from
Oct 22, 2026
Merged

[3260 by Claude Opus 5.5] The current "Visual Evidence" component fails basic diagnostic ergonomics because it treats functional test artifacts like a Dribbble marketing showcase. #3261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
270676e
I've rebuilt the Visual Evidence section in `propr-ui/src/components/…
Oct 19, 2026
51e67a4
feat(ai): I've fixed F1. A capture titled "Checkout assertion passed"…
Oct 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 32 additions & 15 deletions propr-ui/e2e/task-details-visual-previews.pw.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,36 +58,53 @@ 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}`);
const section = page.getByRole('region', { name: /Visual evidence/ });
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}`);
});
}

Expand Down
71 changes: 55 additions & 16 deletions propr-ui/src/components/TaskDetails/TaskVisualPreviews.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,53 +5,92 @@ 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(<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 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();
expect(screen.queryByText('Untrusted')).toBeNull();
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(<TaskVisualPreviews previews={[{ type: 'image', title: 'Tall mobile page', url: asset('tall') }]} />);
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(<TaskVisualPreviews previews={[{ type: 'image', title: 'Dialog', url: asset('dialog') }]} />);
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(<TaskVisualPreviews previews={[
{ type: 'image', title: 'Chart at desktop 1440px', url: asset('a') },
{ type: 'image', title: 'Assertion failed at 390px', url: asset('b') },
{ type: 'image', title: 'Checkout assertion passed', url: asset('c') },
]} />);
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(<TaskVisualPreviews previews={[
{ type: 'image', title: 'Before the fix', url: asset('before') },
{ type: 'image', title: 'After the fix on mobile 390px', url: asset('after') },
{ 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' }));
expect(screen.getByAltText('After the fix on mobile 390px')).toBeInTheDocument();
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', () => {
Expand All @@ -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();
Expand All @@ -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', () => {
Expand Down
Loading
Loading