diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..df2b8527 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,45 @@ +# Repository Guidelines + +## Project Structure & Module Organization +Core CLI code lives in `src/` (commands, API client, services, auth, server, TDD, and shared utils). +UI for local/static reports lives in `src/reporter/` (Vite + React). +Tests live in `tests/` with suites grouped by domain (`tests/commands`, `tests/server`, `tests/services`, etc.). +Type definition tests live in `test-d/`. +Framework integrations and SDK clients live in `clients/` (`storybook`, `static-site`, `vitest`, `ember`, `ruby`, `swift`). +Reference docs are in `docs/`, examples in `examples/`, and built artifacts output to `dist/`. + +## Build, Test, and Development Commands +- `npm run build`: clean and compile CLI + reporter bundles into `dist/`. +- `npm test`: run Node test runner suites with coverage enabled. +- `npm run test:watch`: watch mode for fast local iteration. +- `npm run test:reporter`: run Playwright reporter workflow tests. +- `npm run test:types`: validate published type definitions with `tsd`. +- `npm run lint` / `npm run format:check`: enforce Biome lint/format rules. +- `npm run fix`: run formatter + safe lint fixes. +- `npm run cli -- `: run local CLI entrypoint (example: `npm run cli -- status`). + +## Coding Style & Naming Conventions +Use ESM JavaScript with top-level imports. Prefer functional modules and explicit inputs/outputs over class-heavy designs. +Project convention: prefer `let` over `const`. +Biome is the source of truth: 2-space indent, single quotes, semicolons, trailing commas (`es5`), 80-char line width. +File names are lowercase with hyphens where needed (example: `config-service.js`); tests use `*.test.js`. + +## Testing Guidelines +Primary frameworks: Node’s built-in test runner (`node --test`), Playwright (reporter E2E), and `tsd` (types). +Write tests around user outcomes and observable behavior; avoid mocking internal modules. +Mock only external boundaries (network/time/randomness). +No arbitrary sleeps; wait on concrete state/events. +There is no strict coverage percentage gate today, but changed behavior should include focused tests. + +## Commit & Pull Request Guidelines +Follow existing history style: gitmoji-prefixed, action-oriented commit subjects (examples: `✨`, `🐛`, `🔧`, `🔖`, `⚡️`). +Keep subjects concise; include issue/PR refs when relevant (example: `(#217)`). +PRs should include: +- Why the change is needed. +- A clear summary of all meaningful diff areas. +- A test plan with exact commands run. +- Screenshots or terminal output for UI/reporting changes when helpful. + +## Security & Configuration Tips +Use Node.js `>=22`. Keep secrets (for example `VIZZLY_TOKEN`) out of git. +For local development, isolate CLI state with `VIZZLY_HOME` (for example `~/.vizzly.dev`) to avoid mutating real user config. diff --git a/src/reporter/src/components/app-router.jsx b/src/reporter/src/components/app-router.jsx index e17358a8..5549a8df 100644 --- a/src/reporter/src/components/app-router.jsx +++ b/src/reporter/src/components/app-router.jsx @@ -42,7 +42,19 @@ function ErrorState({ error, onRetry }) { export default function AppRouter() { const [location, setLocation] = useLocation(); - const { data: reportData, isLoading, error, refetch } = useReportData(); + + // Settings and Builds can load independently without report-data polling/fetching + const isManagementRoute = location === '/settings' || location === '/builds'; + + const { + data: reportData, + isLoading, + error, + refetch, + } = useReportData({ + enabled: !isManagementRoute, + polling: !isManagementRoute, + }); // Check if we're on a comparison detail route (fullscreen) const isComparisonRoute = location.startsWith('/comparison/'); @@ -64,9 +76,6 @@ export default function AppRouter() { else setLocation('/'); }; - // Settings, Projects, and Builds don't need screenshot data - always allow access - const isManagementRoute = location === '/settings' || location === '/builds'; - // Loading state (but not for management routes) if (isLoading && !reportData && !isManagementRoute) { return ( diff --git a/src/reporter/src/components/comparison/fullscreen-viewer.jsx b/src/reporter/src/components/comparison/fullscreen-viewer.jsx index b2931963..9f09cb5d 100644 --- a/src/reporter/src/components/comparison/fullscreen-viewer.jsx +++ b/src/reporter/src/components/comparison/fullscreen-viewer.jsx @@ -101,7 +101,7 @@ function FullscreenViewerInner({ let [showQueue, setShowQueue] = useState(true); let [showInspector, setShowInspector] = useState(false); let [queueFilter, setQueueFilter] = useState('needs-review'); - let [_showBaseline, setShowBaseline] = useState(true); + let [showBaseline, setShowBaseline] = useState(true); let [showRegions, setShowRegions] = useState(false); let { zoom, setZoom } = useZoom('fit'); @@ -369,6 +369,10 @@ function FullscreenViewerInner({ // Scroll active queue item into view useEffect(() => { + if (!showQueue) return; + if (currentFilteredIndex < 0) return; + if (currentFilteredIndex >= filteredQueueItems.length) return; + let item = activeQueueItemRef.current; if (!item) return; @@ -389,7 +393,7 @@ function FullscreenViewerInner({ behavior: 'smooth', }); } - }, []); + }, [currentFilteredIndex, filteredQueueItems.length, showQueue]); if (!comparison) { return ( @@ -680,6 +684,8 @@ function FullscreenViewerInner({ viewMode={viewMode === VIEW_MODES.ONION ? 'onion-skin' : viewMode} showDiffOverlay={showDiffOverlay} onDiffToggle={() => setShowDiffOverlay(prev => !prev)} + showBaseline={showBaseline} + onToggleBaseline={() => setShowBaseline(prev => !prev)} onionSkinPosition={onionSkinPosition} onOnionSkinChange={setOnionSkinPosition} zoom={zoom} diff --git a/src/reporter/src/components/comparison/screenshot-display.jsx b/src/reporter/src/components/comparison/screenshot-display.jsx index f26e1a9f..f134291e 100644 --- a/src/reporter/src/components/comparison/screenshot-display.jsx +++ b/src/reporter/src/components/comparison/screenshot-display.jsx @@ -3,7 +3,7 @@ import { HotSpotOverlay, OnionSkinMode, OverlayMode, - ToggleView, + ToggleMode, } from '@vizzly-testing/observatory'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; @@ -16,6 +16,8 @@ export function ScreenshotDisplay({ comparison, viewMode = 'overlay', showDiffOverlay = true, + showBaseline = true, + onToggleBaseline, onionSkinPosition = 50, onOnionSkinChange, onDiffToggle, @@ -29,6 +31,10 @@ export function ScreenshotDisplay({ }) { const [imageErrors, setImageErrors] = useState(new Set()); const [imageLoadStates, setImageLoadStates] = useState(new Map()); + const [baselineImageLoaded, setBaselineImageLoaded] = useState(false); + const [currentImageLoaded, setCurrentImageLoaded] = useState(false); + const [baselineImageError, setBaselineImageError] = useState(false); + const [currentImageError, setCurrentImageError] = useState(false); const [fitScale, setFitScale] = useState(1); const [naturalImageSize, setNaturalImageSize] = useState({ width: 0, @@ -135,6 +141,22 @@ export function ScreenshotDisplay({ }; }, [comparison]); + // Reset controlled toggle state when navigating between screenshots + useEffect(() => { + if (!screenshot?.id) { + setBaselineImageLoaded(false); + setCurrentImageLoaded(false); + setBaselineImageError(false); + setCurrentImageError(false); + return; + } + + setBaselineImageLoaded(false); + setCurrentImageLoaded(false); + setBaselineImageError(false); + setCurrentImageError(false); + }, [screenshot?.id]); + // Render new screenshot - just show current image if ( !comparison || @@ -276,13 +298,23 @@ export function ScreenshotDisplay({ > {/* Render appropriate comparison mode */} {viewMode === 'toggle' && imageUrls.baseline ? ( - ) : viewMode === 'onion-skin' && imageUrls.baseline ? ( c.status === 'passed').length, failed: comparisons.filter(c => c.status === 'failed').length, - new: comparisons.filter(c => c.status === 'new').length, + new: comparisons.filter(c => isNewComparisonStatus(c.status)).length, error: comparisons.filter(c => c.status === 'error').length, }; // Group comparisons by status let failed = comparisons.filter(c => c.status === 'failed'); - let newItems = comparisons.filter(c => c.status === 'new'); + let newItems = comparisons.filter(c => isNewComparisonStatus(c.status)); let passed = comparisons.filter(c => c.status === 'passed'); let errors = comparisons.filter(c => c.status === 'error'); diff --git a/src/reporter/src/components/ui/smart-image.jsx b/src/reporter/src/components/ui/smart-image.jsx index c71252bf..ce3b75b2 100644 --- a/src/reporter/src/components/ui/smart-image.jsx +++ b/src/reporter/src/components/ui/smart-image.jsx @@ -1,12 +1,19 @@ import { - ArrowPathIcon, ExclamationTriangleIcon, PhotoIcon, } from '@heroicons/react/24/outline'; -import useImageLoader from '../../hooks/use-image-loader.js'; +import { useEffect, useState } from 'react'; export default function SmartImage({ src, alt, className, style, onClick }) { - const status = useImageLoader(src); + let [status, setStatus] = useState(src ? 'loading' : 'missing'); + + useEffect(() => { + if (!src) { + setStatus('missing'); + return; + } + setStatus('loading'); + }, [src]); if (status === 'missing') { return ( @@ -22,20 +29,6 @@ export default function SmartImage({ src, alt, className, style, onClick }) { ); } - if (status === 'loading') { - return ( -
-
- -
Loading {alt.toLowerCase()}...
-
-
- ); - } - if (status === 'error') { return (
setStatus('loaded')} + onError={() => setStatus('error')} onClick={onClick} onKeyDown={handleKeyDown} role={onClick ? 'button' : undefined} diff --git a/src/reporter/src/components/views/comparisons-view.jsx b/src/reporter/src/components/views/comparisons-view.jsx index 6180fc88..61430688 100644 --- a/src/reporter/src/components/views/comparisons-view.jsx +++ b/src/reporter/src/components/views/comparisons-view.jsx @@ -14,6 +14,10 @@ import { useReportData, } from '../../hooks/queries/use-tdd-queries.js'; import useComparisonFilters from '../../hooks/use-comparison-filters.js'; +import { + isNewComparisonStatus, + needsReviewComparisonStatus, +} from '../../utils/status-utils.js'; import ScreenshotList from '../comparison/screenshot-list.jsx'; import DashboardFilters from '../dashboard/dashboard-filters.jsx'; import { Button, Card, CardBody, EmptyState } from '../design-system/index.js'; @@ -213,15 +217,16 @@ export default function ComparisonsView() { selectedViewport !== 'all'; // Check if there are changes to accept (failed or new comparisons) - let hasChangesToAccept = reportData?.comparisons?.some( - c => c.status === 'failed' || c.status === 'new' + let hasChangesToAccept = reportData?.comparisons?.some(c => + needsReviewComparisonStatus(c.status) ); // Count failed and new comparisons for the button label let failedCount = reportData?.comparisons?.filter(c => c.status === 'failed').length || 0; let newCount = - reportData?.comparisons?.filter(c => c.status === 'new').length || 0; + reportData?.comparisons?.filter(c => isNewComparisonStatus(c.status)) + .length || 0; let _totalToAccept = failedCount + newCount; if (hasNoComparisons) { diff --git a/src/reporter/src/components/views/stats-view.jsx b/src/reporter/src/components/views/stats-view.jsx index d46fde5d..54fd1cab 100644 --- a/src/reporter/src/components/views/stats-view.jsx +++ b/src/reporter/src/components/views/stats-view.jsx @@ -12,6 +12,10 @@ import { useReportData, useResetBaselines, } from '../../hooks/queries/use-tdd-queries.js'; +import { + isNewComparisonStatus, + needsReviewComparisonStatus, +} from '../../utils/status-utils.js'; import { Badge, Button, @@ -65,12 +69,13 @@ export default function StatsView() { const total = comparisons?.length || 0; const passed = comparisons?.filter(c => c.status === 'passed').length || 0; const failed = comparisons?.filter(c => c.status === 'failed').length || 0; - const newCount = comparisons?.filter(c => c.status === 'new').length || 0; + const newCount = + comparisons?.filter(c => isNewComparisonStatus(c.status)).length || 0; const passRate = total > 0 ? Math.round((passed / total) * 100) : 0; // Check if there are any changes to accept - const hasChanges = comparisons?.some( - c => c.status === 'failed' || c.status === 'new' + const hasChanges = comparisons?.some(c => + needsReviewComparisonStatus(c.status) ); const handleAcceptAll = useCallback(async () => { diff --git a/src/reporter/src/hooks/queries/use-tdd-queries.js b/src/reporter/src/hooks/queries/use-tdd-queries.js index 33d20343..fcf1f4d9 100644 --- a/src/reporter/src/hooks/queries/use-tdd-queries.js +++ b/src/reporter/src/hooks/queries/use-tdd-queries.js @@ -14,6 +14,8 @@ export function useComparison(id, options = {}) { } export function useReportData(options = {}) { + let { polling = true, ...queryOptions } = options; + // Read SSE state from the singleton provider let { state: sseState } = useSSEState(); @@ -25,8 +27,8 @@ export function useReportData(options = {}) { queryKey: queryKeys.reportData(), queryFn: tdd.getReportData, // Only poll as fallback when SSE is not connected - refetchInterval: options.polling !== false && !sseConnected ? 2000 : false, - ...options, + refetchInterval: polling !== false && !sseConnected ? 2000 : false, + ...queryOptions, }); } diff --git a/src/reporter/src/hooks/use-comparison-filters.js b/src/reporter/src/hooks/use-comparison-filters.js index a6b333f5..49936bba 100644 --- a/src/reporter/src/hooks/use-comparison-filters.js +++ b/src/reporter/src/hooks/use-comparison-filters.js @@ -4,6 +4,7 @@ import { sortComparisons, } from '../utils/comparison-helpers.js'; import { FILTER_TYPES, SORT_TYPES } from '../utils/constants.js'; +import { isNewComparisonStatus } from '../utils/status-utils.js'; // Read URL params const getInitialState = () => { @@ -130,7 +131,7 @@ export default function useComparisonFilters(comparisons = []) { all: comparisons.length, failed: comparisons.filter(c => c.status === 'failed').length, passed: comparisons.filter(c => c.status === 'passed').length, - new: comparisons.filter(c => c.status === 'new').length, + new: comparisons.filter(c => isNewComparisonStatus(c.status)).length, rejected: comparisons.filter(c => c.status === 'rejected').length, }, }; diff --git a/src/reporter/src/hooks/use-image-loader.js b/src/reporter/src/hooks/use-image-loader.js deleted file mode 100644 index bb199600..00000000 --- a/src/reporter/src/hooks/use-image-loader.js +++ /dev/null @@ -1,25 +0,0 @@ -import { useEffect, useState } from 'react'; - -export default function useImageLoader(src) { - const [status, setStatus] = useState('loading'); - - useEffect(() => { - if (!src) { - setStatus('missing'); - return; - } - - setStatus('loading'); - const img = new Image(); - img.onload = () => setStatus('loaded'); - img.onerror = () => setStatus('error'); - img.src = src; - - return () => { - img.onload = null; - img.onerror = null; - }; - }, [src]); - - return status; -} diff --git a/src/reporter/src/utils/status-utils.js b/src/reporter/src/utils/status-utils.js new file mode 100644 index 00000000..e47de521 --- /dev/null +++ b/src/reporter/src/utils/status-utils.js @@ -0,0 +1,7 @@ +export function isNewComparisonStatus(status) { + return status === 'new' || status === 'baseline-created'; +} + +export function needsReviewComparisonStatus(status) { + return status === 'failed' || isNewComparisonStatus(status); +} diff --git a/tests/reporter/specs/review-workflow.spec.js b/tests/reporter/specs/review-workflow.spec.js index d103bdd4..1319be52 100644 --- a/tests/reporter/specs/review-workflow.spec.js +++ b/tests/reporter/specs/review-workflow.spec.js @@ -4,6 +4,7 @@ import { fileURLToPath } from 'node:url'; import { expect, test } from '@playwright/test'; import { vizzlyScreenshot } from '../../../dist/client/index.js'; import { createReporterTestServer } from '../test-helper.js'; +import { screenshotFullscreenViewer } from './viewer-test-utils.js'; let __filename = fileURLToPath(import.meta.url); let __dirname = dirname(__filename); @@ -71,7 +72,7 @@ test.describe('Review Workflow', () => { // 📸 Fullscreen viewer await vizzlyScreenshot( 'fullscreen-viewer', - await page.screenshot({ fullPage: true }), + await screenshotFullscreenViewer(page), { browser: browserName, viewport: page.viewportSize() } ); }); diff --git a/tests/reporter/specs/status-handling.spec.js b/tests/reporter/specs/status-handling.spec.js new file mode 100644 index 00000000..91b34274 --- /dev/null +++ b/tests/reporter/specs/status-handling.spec.js @@ -0,0 +1,81 @@ +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { expect, test } from '@playwright/test'; +import { createReporterTestServer } from '../test-helper.js'; + +let __filename = fileURLToPath(import.meta.url); +let __dirname = dirname(__filename); +let fixturesDir = join(__dirname, '..', 'fixtures'); + +test.describe('Status Handling', () => { + let server; + let fixtureData; + let port = 3473; + + test.beforeAll(() => { + let mixedState = JSON.parse( + readFileSync(join(fixturesDir, 'mixed-state.json'), 'utf8') + ); + + fixtureData = { + ...mixedState, + comparisons: [ + ...mixedState.comparisons, + { + id: 'alerts-widget-chrome-1440x900', + name: 'alerts-widget-chrome-1440x900', + originalName: 'alerts-widget', + status: 'baseline-created', + baseline: null, + current: '/images/screenshots/homepage-desktop.png', + diff: null, + diffPercentage: null, + threshold: 2.0, + properties: { + browser: 'chrome', + viewport: { + width: 1440, + height: 900, + }, + }, + timestamp: 1700000006000, + }, + ], + }; + }); + + test.beforeEach(async () => { + server = createReporterTestServer(fixtureData, port); + await server.start(); + }); + + test.afterEach(async () => { + if (server) { + await server.stop(); + } + }); + + test('baseline-created is treated as a new comparison in filters and actions', async ({ + page, + }) => { + await page.goto(`http://localhost:${port}/`); + + // Existing fixture has one "new" + one "baseline-created" item. + await expect(page.getByRole('button', { name: /New.*2/i })).toBeVisible(); + + await page.getByRole('button', { name: /New.*2/i }).click(); + + await expect( + page.getByRole('heading', { name: /dashboard-widget/i }) + ).toBeVisible(); + await expect( + page.getByRole('heading', { name: /alerts-widget/i }) + ).toBeVisible(); + + // Failed (2) + new-like (2) = Accept All (4) + await expect( + page.getByRole('button', { name: /Accept All \(4\)/ }) + ).toBeVisible(); + }); +}); diff --git a/tests/reporter/specs/viewer-modes.spec.js b/tests/reporter/specs/viewer-modes.spec.js index 499bb86f..cb13c20b 100644 --- a/tests/reporter/specs/viewer-modes.spec.js +++ b/tests/reporter/specs/viewer-modes.spec.js @@ -4,6 +4,7 @@ import { fileURLToPath } from 'node:url'; import { expect, test } from '@playwright/test'; import { vizzlyScreenshot } from '../../../dist/client/index.js'; import { createReporterTestServer } from '../test-helper.js'; +import { screenshotFullscreenViewer } from './viewer-test-utils.js'; let __filename = fileURLToPath(import.meta.url); let __dirname = dirname(__filename); @@ -65,7 +66,7 @@ test.describe('Viewer Modes', () => { // 📸 Overlay mode await vizzlyScreenshot( 'viewer-overlay-mode', - await page.screenshot({ fullPage: true }), + await screenshotFullscreenViewer(page), { browser: browserName, viewport: page.viewportSize() } ); @@ -82,7 +83,7 @@ test.describe('Viewer Modes', () => { // 📸 Toggle mode await vizzlyScreenshot( 'viewer-toggle-mode', - await page.screenshot({ fullPage: true }), + await screenshotFullscreenViewer(page), { browser: browserName, viewport: page.viewportSize() } ); @@ -99,7 +100,7 @@ test.describe('Viewer Modes', () => { // 📸 Slide mode await vizzlyScreenshot( 'viewer-slide-mode', - await page.screenshot({ fullPage: true }), + await screenshotFullscreenViewer(page), { browser: browserName, viewport: page.viewportSize() } ); @@ -158,7 +159,7 @@ test.describe('Viewer Modes', () => { // 📸 Zoomed in view await vizzlyScreenshot( 'viewer-zoomed-100', - await page.screenshot({ fullPage: true }), + await screenshotFullscreenViewer(page), { browser: browserName, viewport: page.viewportSize() } ); @@ -176,4 +177,33 @@ test.describe('Viewer Modes', () => { await page.getByRole('button', { name: '1:1' }).click(); await expect(page.getByRole('button', { name: /100%/ })).toBeVisible(); }); + + test('review shortcut "d" toggles baseline/current in toggle mode', async ({ + page, + }) => { + // Skip on mobile - view mode buttons are hidden on small screens + let viewport = page.viewportSize(); + test.skip(viewport.width < 640, 'View mode buttons hidden on mobile'); + + await page.goto(`http://localhost:${port}/`); + + // Open a failed comparison and switch to Toggle mode + await page.getByRole('heading', { name: /pricing-page/i }).click(); + await expect(page.getByTestId('fullscreen-viewer')).toBeVisible(); + await page.getByRole('radio', { name: /toggle/i }).click(); + + // Enter review mode so keyboard shortcuts become active + await page.keyboard.press('Space'); + await expect(page.getByText('Review Mode')).toBeVisible(); + + await expect(page.getByText('Showing Baseline')).toBeVisible(); + + // D shortcut should flip to current image in Toggle mode + await page.keyboard.press('d'); + await expect(page.getByText('Showing Current')).toBeVisible(); + + // Pressing again should flip back + await page.keyboard.press('d'); + await expect(page.getByText('Showing Baseline')).toBeVisible(); + }); }); diff --git a/tests/reporter/specs/viewer-test-utils.js b/tests/reporter/specs/viewer-test-utils.js new file mode 100644 index 00000000..30272402 --- /dev/null +++ b/tests/reporter/specs/viewer-test-utils.js @@ -0,0 +1,67 @@ +import { expect } from '@playwright/test'; + +/** + * Wait until the fullscreen viewer has rendered stable pixels for snapshotting. + */ +export async function waitForFullscreenViewerReady(page) { + let viewer = page.getByTestId('fullscreen-viewer'); + await expect(viewer).toBeVisible(); + + await page.evaluate(async () => { + if (document.fonts && document.fonts.status !== 'loaded') { + await document.fonts.ready; + } + + let root = document.querySelector('[data-testid="fullscreen-viewer"]'); + if (!root) return; + + let visibleImages = Array.from(root.querySelectorAll('img')).filter(img => { + let rect = img.getBoundingClientRect(); + if (rect.width <= 0 || rect.height <= 0) return false; + + let style = window.getComputedStyle(img); + if (style.display === 'none' || style.visibility === 'hidden') { + return false; + } + + return true; + }); + + await Promise.all( + visibleImages.map(async img => { + if (img.complete && img.naturalWidth > 0) { + return; + } + + if (typeof img.decode === 'function') { + try { + await img.decode(); + return; + } catch { + // Fall through to load/error listeners when decode() rejects. + } + } + + await new Promise(resolve => { + let finish = () => { + img.removeEventListener('load', finish); + img.removeEventListener('error', finish); + resolve(); + }; + + img.addEventListener('load', finish, { once: true }); + img.addEventListener('error', finish, { once: true }); + }); + }) + ); + + await new Promise(resolve => { + requestAnimationFrame(() => requestAnimationFrame(resolve)); + }); + }); +} + +export async function screenshotFullscreenViewer(page) { + await waitForFullscreenViewerReady(page); + return await page.getByTestId('fullscreen-viewer').screenshot(); +} diff --git a/tests/reporter/test-helper.js b/tests/reporter/test-helper.js index 6ae0f532..e4e73562 100644 --- a/tests/reporter/test-helper.js +++ b/tests/reporter/test-helper.js @@ -74,6 +74,22 @@ export function createReporterTestServer( +