From 41c10c1c7157fcd2563d9e155cdeca7b1e814ac6 Mon Sep 17 00:00:00 2001 From: benjaminleonard Date: Wed, 15 Apr 2026 10:54:30 +0100 Subject: [PATCH 01/15] Responsive grid layout: fixed TopBar/Sidebar with document scroll Switch from a CSS grid layout (where TopBar and Sidebar occupied grid cells alongside ContentPane) to fixed-position TopBar and Sidebar with document-level scroll. This is the foundation for mobile/tablet support. Key changes: - viewport meta tag: proper responsive instead of fixed 1050px - CSS: add --sidebar-width var, responsive --content-gutter, remove body overflow-y-hidden, set html/body/#root height: 100% - TopBar: single fixed element (was two grid cell fragments) - Sidebar: fixed position with overflow scroll - ContentPane: margin-left for sidebar on desktop, document scroll - Scroll restoration: window.scrollY instead of container scrollTop - PageSkeleton: match new fixed layout structure Co-Authored-By: Claude Opus 4.6 (1M context) --- app/components/PageSkeleton.tsx | 27 +++++++++++-------- app/components/Sidebar.tsx | 2 +- app/components/TopBar.tsx | 12 +++------ app/hooks/use-scroll-restoration.ts | 17 ++++++------ app/layouts/helpers.tsx | 21 ++++++--------- app/table/QueryTable.tsx | 2 +- app/ui/styles/index.css | 17 +++++++++++- index.html | 2 +- test/e2e/scroll-restore.e2e.ts | 40 +++++++---------------------- test/e2e/utils.ts | 6 ++--- 10 files changed, 66 insertions(+), 80 deletions(-) diff --git a/app/components/PageSkeleton.tsx b/app/components/PageSkeleton.tsx index 55f07cd49f..0c63769e39 100644 --- a/app/components/PageSkeleton.tsx +++ b/app/components/PageSkeleton.tsx @@ -28,18 +28,22 @@ export function PageSkeleton({ skipPaths }: { skipPaths?: RegExp[] }) { <> {process.env.MSW_BANNER ? : null} -
- - -
-
- -
- - + {/* TopBar skeleton */} +
+
+ + +
+
+ +
+ + +
-
+ {/* Sidebar skeleton */} +
@@ -52,7 +56,8 @@ export function PageSkeleton({ skipPaths }: { skipPaths?: RegExp[] }) {
-
+ {/* Content skeleton */} +
) diff --git a/app/components/Sidebar.tsx b/app/components/Sidebar.tsx index 96e00e8a3c..0fe2bd2cb4 100644 --- a/app/components/Sidebar.tsx +++ b/app/components/Sidebar.tsx @@ -62,7 +62,7 @@ const JumpToButton = () => { export function Sidebar({ children }: { children: React.ReactNode }) { return ( -
+
diff --git a/app/components/TopBar.tsx b/app/components/TopBar.tsx index 5784c04432..843072cfa8 100644 --- a/app/components/TopBar.tsx +++ b/app/components/TopBar.tsx @@ -32,16 +32,12 @@ import { pb } from '~/util/path-builder' export function TopBar({ systemOrSilo }: { systemOrSilo: 'system' | 'silo' }) { const { me } = useCurrentUser() - // The height of this component is governed by the `PageContainer` - // It's important that this component returns two distinct elements (wrapped in a fragment). - // Each element will occupy one of the top column slots provided by `PageContainer`. return ( - <> -
+
+
- {/* Height is governed by PageContainer grid */} -
+
@@ -50,7 +46,7 @@ export function TopBar({ systemOrSilo }: { systemOrSilo: 'system' | 'silo' }) {
- +
) } diff --git a/app/hooks/use-scroll-restoration.ts b/app/hooks/use-scroll-restoration.ts index a622d18e15..cb2ff0c9e7 100644 --- a/app/hooks/use-scroll-restoration.ts +++ b/app/hooks/use-scroll-restoration.ts @@ -18,20 +18,19 @@ function setScrollPosition(key: string, pos: number) { } /** - * Given a ref to a scrolling container element, keep track of its scroll - * position before navigation and restore it on return (e.g., back/forward nav). - * Note that `location.key` is used in the cache key, not `location.pathname`, - * so the same path navigated to at different points in the history stack will - * not share the same scroll position. + * Keep track of window scroll position before navigation and restore it on + * return (e.g., back/forward nav). Note that `location.key` is used in the + * cache key, not `location.pathname`, so the same path navigated to at + * different points in the history stack will not share the same scroll position. */ -export function useScrollRestoration(container: React.RefObject) { +export function useScrollRestoration() { const key = `scroll-position-${useLocation().key}` const { state } = useNavigation() useEffect(() => { if (state === 'loading') { - setScrollPosition(key, container.current?.scrollTop ?? 0) + setScrollPosition(key, window.scrollY) } else if (state === 'idle') { - container.current?.scrollTo(0, getScrollPosition(key)) + window.scrollTo(0, getScrollPosition(key)) } - }, [key, state, container]) + }, [key, state]) } diff --git a/app/layouts/helpers.tsx b/app/layouts/helpers.tsx index 42244e9c29..80b723d837 100644 --- a/app/layouts/helpers.tsx +++ b/app/layouts/helpers.tsx @@ -5,7 +5,6 @@ * * Copyright Oxide Computer Company */ -import { useRef } from 'react' import { Outlet } from 'react-router' import { PageActionsTarget } from '~/components/PageActions' @@ -14,15 +13,13 @@ import { useScrollRestoration } from '~/hooks/use-scroll-restoration' import { SkipLinkTarget } from '~/ui/lib/SkipLink' import { classed } from '~/util/classed' -export const PageContainer = classed.div`grid h-screen grid-cols-[14.25rem_1fr] grid-rows-[var(--top-bar-height)_1fr]` +export const PageContainer = classed.div`min-h-full pt-(--top-bar-height)` export function ContentPane() { - const ref = useRef(null) - useScrollRestoration(ref) + useScrollRestoration() return (
@@ -47,12 +44,10 @@ export function ContentPane() { * `
` because we don't need it. */ export const SerialConsoleContentPane = () => ( -
-
- -
- -
-
+
+ +
+ +
) diff --git a/app/table/QueryTable.tsx b/app/table/QueryTable.tsx index bf25f9c361..fdaef9786d 100644 --- a/app/table/QueryTable.tsx +++ b/app/table/QueryTable.tsx @@ -46,7 +46,7 @@ function useScrollReset(triggerDep: string | undefined) { const resetRequested = useRef(false) useEffect(() => { if (resetRequested.current) { - document.querySelector('#scroll-container')?.scrollTo(0, 0) + window.scrollTo(0, 0) resetRequested.current = false } }, [triggerDep]) diff --git a/app/ui/styles/index.css b/app/ui/styles/index.css index 24d95681f1..f1c938bf82 100644 --- a/app/ui/styles/index.css +++ b/app/ui/styles/index.css @@ -128,7 +128,15 @@ :root { --content-gutter: 2.5rem; --top-bar-height: 54px; + --sidebar-width: 14.25rem; + @media (max-width: 767px) { + :root { + --content-gutter: 1.5rem; + } + } + + :root { /* Nicer easing from: https://twitter.com/bdc */ --ease-in-quad: cubic-bezier(0.55, 0.085, 0.68, 0.53); --ease-in-cubic: cubic-bezier(0.55, 0.055, 0.675, 0.19); @@ -154,8 +162,15 @@ } @layer base { + html, + body, + #root { + height: 100%; + } + body { - @apply text-default bg-default overflow-y-hidden font-sans; + @apply text-default bg-default font-sans; + overscroll-behavior-y: none; } /* https://github.com/tailwindlabs/tailwindcss/blob/v2.2.4/src/plugins/css/preflight.css#L57 */ diff --git a/index.html b/index.html index 51b2dfb899..cbb62318f3 100644 --- a/index.html +++ b/index.html @@ -12,7 +12,7 @@ Oxide Console - + diff --git a/test/e2e/scroll-restore.e2e.ts b/test/e2e/scroll-restore.e2e.ts index 28d922e67f..c139061968 100644 --- a/test/e2e/scroll-restore.e2e.ts +++ b/test/e2e/scroll-restore.e2e.ts @@ -10,11 +10,13 @@ import { expect, test } from '@playwright/test' import { expectScrollTop, scrollTo, sleep } from './utils' test('scroll restore', async ({ page }) => { - // open small window to make scrolling easier - await page.setViewportSize({ width: 800, height: 500 }) + // use desktop-width viewport with short height to make scrolling easier + await page.setViewportSize({ width: 1280, height: 400 }) // nav to disks and scroll it await page.goto('/projects/mock-project/disks') + // wait for content to render so the page is tall enough to scroll + await page.getByRole('heading', { name: 'Disks' }).waitFor() await expectScrollTop(page, 0) await scrollTo(page, 143) @@ -32,43 +34,19 @@ test('scroll restore', async ({ page }) => { await scrollTo(page, 190) await sleep(1000) - // go forward to snapshots, now scroll it - await page.goForward() + // new nav to snapshots via click, scroll it + await page.getByRole('link', { name: 'Snapshots' }).click() await expect(page).toHaveURL('/projects/mock-project/snapshots') await expectScrollTop(page, 0) - await scrollTo(page, 30) - - // Oddly, this is required here in order for the page to have time to - // catch the 30 scroll position. This became necessary with RR v7's use of - // startTransition. Extra oddly, with a value of 500 it passes rarely, but - // with 1000 it passes every time. - await sleep(1000) - - // new nav to disks - await page.getByRole('link', { name: 'Disks' }).click() - await expectScrollTop(page, 0) - - // this is too flaky so forget it for now - // random reload in there because we use sessionStorage. note we are - // deliberately on the disks page here because there's a quirk in playwright - // that seems to reset to the disks page on reload - // await page.reload() - - // back to snapshots, scroll is restored - await page.goBack() - await expect(page).toHaveURL('/projects/mock-project/snapshots') - await expectScrollTop(page, 30) - - // back again to disks, newer scroll value is restored + // back to disks, newer scroll value is restored await page.goBack() await expect(page).toHaveURL('/projects/mock-project/disks') await sleep(1000) await expectScrollTop(page, 190) - // forward again to newest disks history entry, scroll remains 0 + // forward to snapshots, scroll is 0 (fresh nav) await page.goForward() - await page.goForward() - await expect(page).toHaveURL('/projects/mock-project/disks') + await expect(page).toHaveURL('/projects/mock-project/snapshots') await expectScrollTop(page, 0) }) diff --git a/test/e2e/utils.ts b/test/e2e/utils.ts index e573ef51b6..0ee45106ac 100644 --- a/test/e2e/utils.ts +++ b/test/e2e/utils.ts @@ -265,14 +265,12 @@ export async function chooseFile(input: Locator, size: 'large' | 'small' = 'larg } export async function expectScrollTop(page: Page, expected: number) { - const container = page.getByTestId('scroll-container') - const getScrollTop = () => container.evaluate((el: HTMLElement) => el.scrollTop) + const getScrollTop = () => page.evaluate(() => window.scrollY) await expect.poll(getScrollTop).toBe(expected) } export async function scrollTo(page: Page, to: number) { - const container = page.getByTestId('scroll-container') - await container.evaluate((el: HTMLElement, to) => el.scrollTo(0, to), to) + await page.evaluate((to) => window.scrollTo(0, to), to) } export async function addTlsCert(page: Page) { From ecf1870da009e89cdf2266e8b7be09756a318c25 Mon Sep 17 00:00:00 2001 From: benjaminleonard Date: Wed, 15 Apr 2026 10:56:04 +0100 Subject: [PATCH 02/15] Fix missing brace --- app/ui/styles/index.css | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/ui/styles/index.css b/app/ui/styles/index.css index f1c938bf82..4d3113d500 100644 --- a/app/ui/styles/index.css +++ b/app/ui/styles/index.css @@ -130,9 +130,10 @@ --top-bar-height: 54px; --sidebar-width: 14.25rem; - @media (max-width: 767px) { - :root { - --content-gutter: 1.5rem; + @media (max-width: 767px) { + :root { + --content-gutter: 1.5rem; + } } } From 5bce8f8eaab8d72cd206654775affa5604f53e3a Mon Sep 17 00:00:00 2001 From: benjaminleonard Date: Wed, 15 Apr 2026 11:44:38 +0100 Subject: [PATCH 03/15] Expand visual regression tests --- test/visual/regression.e2e.ts | 280 +++++++++++++++++++++++----------- 1 file changed, 195 insertions(+), 85 deletions(-) diff --git a/test/visual/regression.e2e.ts b/test/visual/regression.e2e.ts index d8a84fe2ef..a6c499a108 100644 --- a/test/visual/regression.e2e.ts +++ b/test/visual/regression.e2e.ts @@ -19,106 +19,216 @@ import { expect, test } from '../e2e/utils' // set a fixed time to avoid diffs due to irrelevant time differences test.beforeEach(async ({ page }) => { await page.clock.setFixedTime(new Date('2025-10-23T12:34:56.000Z')) + // TODO: revert to default viewport once we've confirmed no visual regressions + // from the grid layout change. The tall viewport forces all content to render + // without scrolling, so fullPage screenshots are comparable between the old + // contained-scroll layout and the new document-scroll layout. + await page.setViewportSize({ width: 1280, height: 3100 }) }) -test.describe('Visual Regression', { tag: '@visual' }, () => { - test('projects list', async ({ page }) => { - await page.goto('/projects') - await expect(page.getByRole('heading', { name: 'Projects' })).toBeVisible() - await expect(page).toHaveScreenshot('projects-list.png', { fullPage: true }) - }) - - test('instances list', async ({ page }) => { - await page.goto('/projects/mock-project/instances') - await expect(page.getByRole('heading', { name: 'Instances' })).toBeVisible() - await expect(page).toHaveScreenshot('instances-list.png', { fullPage: true }) - }) - - test('instance detail', async ({ page }) => { - await page.goto('/projects/mock-project/instances/db1') - await expect(page.getByRole('heading', { name: 'db1' })).toBeVisible() - await expect(page).toHaveScreenshot('instance-detail.png', { fullPage: true }) - }) - - test('create disk', async ({ page }) => { - await page.goto('/projects/mock-project/disks-new') - await expect(page.getByRole('heading', { name: 'Create disk' })).toBeVisible() - await expect(page).toHaveScreenshot('disks-new.png', { fullPage: true }) - }) +const fullPage = { fullPage: true } + +const p = '/projects/mock-project' + +// Standard pages: goto URL, wait for heading, take full-page screenshot +const pages = [ + // Auth + { name: 'device verify', url: '/device/verify', heading: 'Device Authentication' }, + { name: 'device success', url: '/device/success', heading: 'Device logged in' }, + + // Settings + { name: 'settings profile', url: '/settings/profile', heading: 'Profile' }, + { name: 'settings SSH keys', url: '/settings/ssh-keys', heading: 'SSH Keys' }, + { + name: 'settings access tokens', + url: '/settings/access-tokens', + heading: 'Access Tokens', + }, + + // Silo + { name: 'projects list', url: '/projects', heading: 'Projects' }, + { + name: 'silo image edit', + url: '/images/arch-2022-06-01/edit', + heading: 'Silo image', + exact: true, + }, + { name: 'silo utilization', url: '/utilization', heading: 'Utilization' }, + { name: 'silo access', url: '/access', heading: 'Silo Access' }, + + // Project - Instances + { name: 'instances list', url: `${p}/instances`, heading: 'Instances' }, + { name: 'instance create', url: `${p}/instances-new`, heading: 'Create instance' }, + { name: 'instance storage tab', url: `${p}/instances/db1/storage`, heading: 'db1' }, + { name: 'instance networking tab', url: `${p}/instances/db1/networking`, heading: 'db1' }, + { name: 'instance metrics cpu', url: `${p}/instances/db1/metrics/cpu`, heading: 'db1' }, + { name: 'instance metrics disk', url: `${p}/instances/db1/metrics/disk`, heading: 'db1' }, + { + name: 'instance metrics network', + url: `${p}/instances/db1/metrics/network`, + heading: 'db1', + }, + { name: 'instance connect tab', url: `${p}/instances/db1/connect`, heading: 'db1' }, + { name: 'instance settings tab', url: `${p}/instances/db1/settings`, heading: 'db1' }, + + // Project - Disks + { name: 'disks list', url: `${p}/disks`, heading: 'Disks' }, + { name: 'create disk', url: `${p}/disks-new`, heading: 'Create disk' }, + + // Project - Snapshots, Images + { name: 'snapshots list', url: `${p}/snapshots`, heading: 'Snapshots' }, + { name: 'images list', url: `${p}/images`, heading: 'Images' }, + { name: 'image upload', url: `${p}/images-new`, heading: 'Upload image' }, + + // Project - VPCs + { name: 'vpcs list', url: `${p}/vpcs`, heading: 'VPCs' }, + { + name: 'vpc firewall rules', + url: `${p}/vpcs/mock-vpc/firewall-rules`, + heading: 'mock-vpc', + }, + { name: 'vpc subnets', url: `${p}/vpcs/mock-vpc/subnets`, heading: 'mock-vpc' }, + { name: 'vpc routers', url: `${p}/vpcs/mock-vpc/routers`, heading: 'mock-vpc' }, + { + name: 'vpc internet gateways', + url: `${p}/vpcs/mock-vpc/internet-gateways`, + heading: 'mock-vpc', + }, + { + name: 'vpc router detail', + url: `${p}/vpcs/mock-vpc/routers/mock-custom-router`, + heading: 'mock-custom-router', + }, + + // Project - Networking + { name: 'floating IPs', url: `${p}/floating-ips`, heading: 'Floating IPs' }, + { name: 'external subnets', url: `${p}/external-subnets`, heading: 'External Subnets' }, + + // Project - Other + { name: 'project access', url: `${p}/access`, heading: 'Project Access' }, + { name: 'affinity groups', url: `${p}/affinity`, heading: 'Affinity Groups' }, + { + name: 'anti-affinity group detail', + url: `${p}/affinity/romulus-remus`, + heading: 'romulus-remus', + }, + + // System - Silos + { name: 'system silos list', url: '/system/silos', heading: 'Silos' }, + { name: 'silo detail idps', url: '/system/silos/maze-war/idps', heading: 'maze-war' }, + { + name: 'silo detail ip pools', + url: '/system/silos/maze-war/ip-pools', + heading: 'maze-war', + }, + { + name: 'silo detail subnet pools', + url: '/system/silos/maze-war/subnet-pools', + heading: 'maze-war', + }, + { name: 'silo detail quotas', url: '/system/silos/maze-war/quotas', heading: 'maze-war' }, + { + name: 'silo detail fleet roles', + url: '/system/silos/maze-war/fleet-roles', + heading: 'maze-war', + }, + { name: 'silo detail scim', url: '/system/silos/maze-war/scim', heading: 'maze-war' }, + + // System - Utilization + { name: 'system utilization', url: '/system/utilization', heading: 'Utilization' }, + { + name: 'system utilization metrics tab', + url: '/system/utilization?tab=metrics', + heading: 'Utilization', + }, + + // System - Networking + { name: 'system ip pools', url: '/system/networking/ip-pools', heading: 'IP Pools' }, + { + name: 'ip pool detail', + url: '/system/networking/ip-pools/ip-pool-1', + heading: 'ip-pool-1', + }, + { + name: 'ip pool silos tab', + url: '/system/networking/ip-pools/ip-pool-1?tab=silos', + heading: 'ip-pool-1', + }, + { + name: 'system subnet pools', + url: '/system/networking/subnet-pools', + heading: 'Subnet Pools', + }, + { + name: 'subnet pool detail', + url: '/system/networking/subnet-pools/default-v4-subnet-pool', + heading: 'default-v4-subnet-pool', + }, + { + name: 'subnet pool silos tab', + url: '/system/networking/subnet-pools/default-v4-subnet-pool?tab=silos', + heading: 'default-v4-subnet-pool', + }, + + // System - Inventory + { name: 'inventory sleds', url: '/system/inventory/sleds', heading: 'Inventory' }, + { name: 'inventory disks', url: '/system/inventory/disks', heading: 'Inventory' }, + { + name: 'sled instances', + url: '/system/inventory/sleds/c2519937-44a4-493b-9b38-5c337c597d08/instances', + heading: 'Sled', + }, + + // System - Update & Access + { name: 'system update', url: '/system/update', heading: 'System Update' }, + { name: 'fleet access', url: '/system/access', heading: 'Fleet Access' }, + + // Error + { name: 'not found', url: '/nonexistent', heading: 'Page not found' }, +] - test('disks list', async ({ page }) => { - await page.goto('/projects/mock-project/disks') - await expect(page.getByRole('heading', { name: 'Disks' })).toBeVisible() - await expect(page).toHaveScreenshot('disks-list.png', { fullPage: true }) - }) +test.describe('Visual Regression', { tag: '@visual' }, () => { + for (const { name, url, heading, exact } of pages) { + const screenshot = name.replaceAll(' ', '-') + '.png' + test(name, async ({ page }) => { + await page.goto(url, { waitUntil: 'networkidle' }) + await expect(page.getByRole('heading', { name: heading, exact })).toBeVisible() + await expect(page).toHaveScreenshot(screenshot, fullPage) + }) + } - test('vpcs list', async ({ page }) => { - await page.goto('/projects/mock-project/vpcs') - await expect(page.getByRole('heading', { name: 'VPCs' })).toBeVisible() - await expect(page).toHaveScreenshot('vpcs-list.png', { fullPage: true }) - }) + // Special cases that don't fit the standard pattern - test('snapshots list', async ({ page }) => { - await page.goto('/projects/mock-project/snapshots') - await expect(page.getByRole('heading', { name: 'Snapshots' })).toBeVisible() - await expect(page).toHaveScreenshot('snapshots-list.png', { fullPage: true }) - }) - - test('images list', async ({ page }) => { - await page.goto('/projects/mock-project/images') - await expect(page.getByRole('heading', { name: 'Images' })).toBeVisible() - await expect(page).toHaveScreenshot('images-list.png', { fullPage: true }) + test('login form', async ({ page }) => { + await page.goto('/login/default-silo/local', { waitUntil: 'networkidle' }) + await expect(page).toHaveURL(/\/login/) + await expect(page).toHaveScreenshot('login-form.png') }) - test('silo images list', async ({ page }) => { - await page.goto('/images') + test('silo images', async ({ page }) => { + await page.goto('/images', { waitUntil: 'networkidle' }) await expect(page.getByRole('heading', { name: 'Silo Images' })).toBeVisible() - await expect(page).toHaveScreenshot('silo-images.png', { fullPage: true }) - await page.click('role=button[name="Promote image"]') - await expect(page).toHaveScreenshot('silo-images-promote.png', { fullPage: true }) - }) - - test('silo image', async ({ page }) => { - await page.goto('/images/arch-2022-06-01/edit') - await expect( - page.getByRole('heading', { name: 'Silo image', exact: true }) - ).toBeVisible() - await expect(page).toHaveScreenshot('silo-image.png', { fullPage: true }) - }) - - test('system utilization', async ({ page }) => { - await page.goto('/utilization') - await expect(page.getByRole('heading', { name: 'Utilization' })).toBeVisible() - await expect(page).toHaveScreenshot('system-utilization.png', { fullPage: true }) - }) - - test('system silos list', async ({ page }) => { - await page.goto('/system/silos') - await expect(page.getByRole('heading', { name: 'Silos' })).toBeVisible() - await expect(page).toHaveScreenshot('system-silos.png', { fullPage: true }) + await expect(page).toHaveScreenshot('silo-images.png', fullPage) + await page.getByRole('button', { name: 'Promote image' }).click() + await page.waitForLoadState('networkidle') + await expect(page).toHaveScreenshot('silo-images-promote.png', fullPage) }) - test('system networking ip pools', async ({ page }) => { - await page.goto('/system/networking/ip-pools') - await expect(page.getByRole('heading', { name: 'IP Pools' })).toBeVisible() - await expect(page).toHaveScreenshot('system-ip-pools.png', { fullPage: true }) - }) - - test('settings profile', async ({ page }) => { - await page.goto('/settings/profile') - await expect(page.getByRole('heading', { name: 'Profile' })).toBeVisible() - await expect(page).toHaveScreenshot('settings-profile.png', { fullPage: true }) + test('saml login', async ({ page }) => { + await page.goto('/login/default-silo/saml/mock-idp', { waitUntil: 'networkidle' }) + await expect(page).toHaveURL(/\/login/) + await expect(page).toHaveScreenshot('saml-login.png') }) - test('login form', async ({ page }) => { - await page.goto('/login/default-silo/local') - - await expect(page).toHaveURL(/\/login/) - await expect(page).toHaveScreenshot('login-form.png') + test('serial console', async ({ page }) => { + await page.goto(`${p}/instances/db1/serial-console`, { waitUntil: 'networkidle' }) + await expect(page.getByText('Serial Console')).toBeVisible() + await expect(page).toHaveScreenshot('serial-console.png', fullPage) }) test('command menu', async ({ page }) => { await page.keyboard.press(`ControlOrMeta+k`) - await expect(page).toHaveScreenshot('command-menu.png', { fullPage: true }) + await page.waitForLoadState('networkidle') + await expect(page).toHaveScreenshot('command-menu.png', fullPage) }) }) From 680e6c54db079b39739ac1a121f5ec0cfee74367 Mon Sep 17 00:00:00 2001 From: benjaminleonard Date: Wed, 15 Apr 2026 11:58:49 +0100 Subject: [PATCH 04/15] Remove `scroll-container` and references --- AGENTS.md | 2 +- app/components/PageSkeleton.tsx | 12 ++++++------ app/layouts/helpers.tsx | 6 +----- test/e2e/ip-pool-silo-config.e2e.ts | 8 ++------ 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index eba33b06ec..295ca673bc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -100,7 +100,7 @@ # Layout & accessibility -- Build pages inside the shared `PageContainer`/`ContentPane` so you inherit the skip link, sticky footer, pagination target, and scroll restoration tied to `#scroll-container` (`app/layouts/helpers.tsx`, `app/hooks/use-scroll-restoration.ts`). +- Build pages inside the shared `PageContainer`/`ContentPane` so you inherit the skip link, sticky footer, pagination target, and scroll restoration (`app/layouts/helpers.tsx`, `app/hooks/use-scroll-restoration.ts`). - Surface page-level buttons and pagination via the `PageActions` and `Pagination` tunnels from `tunnel-rat`; anything rendered through `.In` lands in `.Target` automatically. - For global loading states, reuse `PageSkeleton`—it keeps the MSW banner and grid layout stable, and `skipPaths` lets you opt-out for routes with custom layouts (`app/components/PageSkeleton.tsx`). - Enforce accessibility at the type level: use `AriaLabel` type from `app/ui/util/aria.ts` which requires exactly one of `aria-label` or `aria-labelledby` on custom interactive components. diff --git a/app/components/PageSkeleton.tsx b/app/components/PageSkeleton.tsx index 0c63769e39..82e21c4ec8 100644 --- a/app/components/PageSkeleton.tsx +++ b/app/components/PageSkeleton.tsx @@ -8,7 +8,7 @@ import { useLocation } from 'react-router' -import { PageContainer } from '~/layouts/helpers' +import { ContentPane, PageContainer } from '~/layouts/helpers' import { classed } from '~/util/classed' import { MswBanner } from './MswBanner' @@ -28,8 +28,8 @@ export function PageSkeleton({ skipPaths }: { skipPaths?: RegExp[] }) { <> {process.env.MSW_BANNER ? : null} - {/* TopBar skeleton */} -
+ {/* TopBar */} +
@@ -42,7 +42,7 @@ export function PageSkeleton({ skipPaths }: { skipPaths?: RegExp[] }) {
- {/* Sidebar skeleton */} + {/* Sidebar */}
@@ -56,8 +56,8 @@ export function PageSkeleton({ skipPaths }: { skipPaths?: RegExp[] }) {
- {/* Content skeleton */} -
+ {/* Content */} + ) diff --git a/app/layouts/helpers.tsx b/app/layouts/helpers.tsx index 80b723d837..4c77b17410 100644 --- a/app/layouts/helpers.tsx +++ b/app/layouts/helpers.tsx @@ -18,11 +18,7 @@ export const PageContainer = classed.div`min-h-full pt-(--top-bar-height)` export function ContentPane() { useScrollRestoration() return ( -
+
diff --git a/test/e2e/ip-pool-silo-config.e2e.ts b/test/e2e/ip-pool-silo-config.e2e.ts index 62e3953172..6ffedf5a35 100644 --- a/test/e2e/ip-pool-silo-config.e2e.ts +++ b/test/e2e/ip-pool-silo-config.e2e.ts @@ -330,9 +330,7 @@ test.describe('IP pool configuration: pelerines silo (no defaults)', () => { await page.getByRole('button', { name: 'Create instance' }).click() // RHF required validation should show an error on the pool field - await expect( - page.getByTestId('scroll-container').getByText('IPv4 pool is required') - ).toBeVisible() + await expect(page.getByText('IPv4 pool is required')).toBeVisible() // Should still be on the create page await expect(page).toHaveURL('/projects/adorno-project/instances-new') @@ -375,9 +373,7 @@ test.describe('IP pool configuration: pelerines silo (no defaults)', () => { await page.getByRole('button', { name: 'Create instance' }).click() // RHF required validation should show an error on the pool field - await expect( - page.getByTestId('scroll-container').getByText('IPv6 pool is required') - ).toBeVisible() + await expect(page.getByText('IPv6 pool is required')).toBeVisible() // Should still be on the create page await expect(page).toHaveURL('/projects/adorno-project/instances-new') From 979e8e3a844b4426c9a484b7cf1fdf489e181a26 Mon Sep 17 00:00:00 2001 From: benjaminleonard Date: Wed, 15 Apr 2026 12:06:58 +0100 Subject: [PATCH 05/15] Remove unnecessary overscroll style --- app/ui/styles/index.css | 1 - 1 file changed, 1 deletion(-) diff --git a/app/ui/styles/index.css b/app/ui/styles/index.css index 4d3113d500..9c4dee7099 100644 --- a/app/ui/styles/index.css +++ b/app/ui/styles/index.css @@ -171,7 +171,6 @@ body { @apply text-default bg-default font-sans; - overscroll-behavior-y: none; } /* https://github.com/tailwindlabs/tailwindcss/blob/v2.2.4/src/plugins/css/preflight.css#L57 */ From 33e9efaa2a687d0236bf860d17795d840a242641 Mon Sep 17 00:00:00 2001 From: benjaminleonard Date: Wed, 15 Apr 2026 12:14:29 +0100 Subject: [PATCH 06/15] Test fix --- test/e2e/ip-pool-silo-config.e2e.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/e2e/ip-pool-silo-config.e2e.ts b/test/e2e/ip-pool-silo-config.e2e.ts index 6ffedf5a35..59406849d2 100644 --- a/test/e2e/ip-pool-silo-config.e2e.ts +++ b/test/e2e/ip-pool-silo-config.e2e.ts @@ -330,7 +330,9 @@ test.describe('IP pool configuration: pelerines silo (no defaults)', () => { await page.getByRole('button', { name: 'Create instance' }).click() // RHF required validation should show an error on the pool field - await expect(page.getByText('IPv4 pool is required')).toBeVisible() + await expect( + page.locator('#create-instance-form').getByText('IPv4 pool is required') + ).toBeVisible() // Should still be on the create page await expect(page).toHaveURL('/projects/adorno-project/instances-new') @@ -373,7 +375,9 @@ test.describe('IP pool configuration: pelerines silo (no defaults)', () => { await page.getByRole('button', { name: 'Create instance' }).click() // RHF required validation should show an error on the pool field - await expect(page.getByText('IPv6 pool is required')).toBeVisible() + await expect( + page.locator('#create-instance-form').getByText('IPv6 pool is required') + ).toBeVisible() // Should still be on the create page await expect(page).toHaveURL('/projects/adorno-project/instances-new') From e0e7e52e227e67832ace691a9e1e2033e4f5bf10 Mon Sep 17 00:00:00 2001 From: benjaminleonard Date: Wed, 15 Apr 2026 12:56:15 +0100 Subject: [PATCH 07/15] zIndex prop on dropdown --- app/components/TopBar.tsx | 4 ++-- app/ui/lib/DropdownMenu.tsx | 20 ++++++++++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/app/components/TopBar.tsx b/app/components/TopBar.tsx index 843072cfa8..cc3c61616d 100644 --- a/app/components/TopBar.tsx +++ b/app/components/TopBar.tsx @@ -142,7 +142,7 @@ function UserMenu() {
- + Settings logout.mutate({})} label="Sign out" /> @@ -234,7 +234,7 @@ function SiloSystemPicker({ level }: { level: 'silo' | 'system' }) {
- + diff --git a/app/ui/lib/DropdownMenu.tsx b/app/ui/lib/DropdownMenu.tsx index 6585875b2b..a8b8c23ab1 100644 --- a/app/ui/lib/DropdownMenu.tsx +++ b/app/ui/lib/DropdownMenu.tsx @@ -47,20 +47,36 @@ function parseAnchor( return { side, align, sideOffset, alignOffset } } +const zIndexClass = { + dropdown: 'z-(--z-content-dropdown)', + topBar: 'z-(--z-top-bar-dropdown)', + modal: 'z-(--z-modal-dropdown)', + sideModal: 'z-(--z-side-modal-dropdown)', +} as const + +type ZIndex = keyof typeof zIndexClass + type ContentProps = { className?: string children: ReactNode anchor?: AnchorProp /** Spacing in px between trigger and menu */ gap?: 8 + zIndex?: ZIndex } -export function Content({ className, children, anchor = 'bottom end', gap }: ContentProps) { +export function Content({ + className, + children, + anchor = 'bottom end', + gap, + zIndex = 'dropdown', +}: ContentProps) { const { side, align, sideOffset, alignOffset } = parseAnchor(anchor, gap) return ( Date: Wed, 15 Apr 2026 12:57:34 +0100 Subject: [PATCH 08/15] Prevent overscroll on modals and sidebar --- app/components/Sidebar.tsx | 2 +- app/ui/lib/Modal.tsx | 2 +- app/ui/lib/SideModal.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/components/Sidebar.tsx b/app/components/Sidebar.tsx index 0fe2bd2cb4..cab8263efb 100644 --- a/app/components/Sidebar.tsx +++ b/app/components/Sidebar.tsx @@ -62,7 +62,7 @@ const JumpToButton = () => { export function Sidebar({ children }: { children: React.ReactNode }) { return ( -
+
diff --git a/app/ui/lib/Modal.tsx b/app/ui/lib/Modal.tsx index 1fe1950e13..f737115180 100644 --- a/app/ui/lib/Modal.tsx +++ b/app/ui/lib/Modal.tsx @@ -93,7 +93,7 @@ export function Modal({ ) } -Modal.Body = classed.div`py-2 overflow-y-auto` +Modal.Body = classed.div`py-2 overflow-y-auto overscroll-none` Modal.Section = classed.div`p-4 space-y-4 border-b border-secondary text-default last-of-type:border-none text-sans-md` diff --git a/app/ui/lib/SideModal.tsx b/app/ui/lib/SideModal.tsx index 27622d8562..deb5170ea7 100644 --- a/app/ui/lib/SideModal.tsx +++ b/app/ui/lib/SideModal.tsx @@ -129,7 +129,7 @@ function SideModalBody({ children }: { children?: ReactNode }) {
Date: Wed, 15 Apr 2026 14:17:47 +0100 Subject: [PATCH 09/15] Add collision padding to the action menu (avoids pagination) --- app/table/columns/action-col.tsx | 6 +++++- app/ui/lib/DropdownMenu.tsx | 3 +++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/table/columns/action-col.tsx b/app/table/columns/action-col.tsx index acb506a875..64cabc6fff 100644 --- a/app/table/columns/action-col.tsx +++ b/app/table/columns/action-col.tsx @@ -92,7 +92,11 @@ export const RowActions = ({ id, copyIdLabel = 'Copy ID', actions }: RowActionsP {/* offset moves menu in from the right so it doesn't align with the table border */} - + {id && } {actions?.map(({ className, ...action }) => 'to' in action ? ( diff --git a/app/ui/lib/DropdownMenu.tsx b/app/ui/lib/DropdownMenu.tsx index a8b8c23ab1..6e9b086f40 100644 --- a/app/ui/lib/DropdownMenu.tsx +++ b/app/ui/lib/DropdownMenu.tsx @@ -63,6 +63,7 @@ type ContentProps = { /** Spacing in px between trigger and menu */ gap?: 8 zIndex?: ZIndex + collisionPadding?: React.ComponentProps['collisionPadding'] } export function Content({ @@ -71,6 +72,7 @@ export function Content({ anchor = 'bottom end', gap, zIndex = 'dropdown', + collisionPadding, }: ContentProps) { const { side, align, sideOffset, alignOffset } = parseAnchor(anchor, gap) return ( @@ -81,6 +83,7 @@ export function Content({ align={align} sideOffset={sideOffset} alignOffset={alignOffset} + collisionPadding={collisionPadding} > Date: Tue, 21 Apr 2026 16:04:23 +0100 Subject: [PATCH 10/15] Share topBar and sidebar classes with skeleton --- app/components/PageSkeleton.tsx | 12 +++++++++--- app/components/Sidebar.tsx | 8 +++++++- app/components/TopBar.tsx | 3 ++- app/layouts/helpers.tsx | 6 ++++++ 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/app/components/PageSkeleton.tsx b/app/components/PageSkeleton.tsx index 82e21c4ec8..c3510168d6 100644 --- a/app/components/PageSkeleton.tsx +++ b/app/components/PageSkeleton.tsx @@ -6,9 +6,15 @@ * Copyright Oxide Computer Company */ +import cn from 'classnames' import { useLocation } from 'react-router' -import { ContentPane, PageContainer } from '~/layouts/helpers' +import { + ContentPane, + PageContainer, + sidebarWrapperClass, + topBarWrapperClass, +} from '~/layouts/helpers' import { classed } from '~/util/classed' import { MswBanner } from './MswBanner' @@ -29,7 +35,7 @@ export function PageSkeleton({ skipPaths }: { skipPaths?: RegExp[] }) { {process.env.MSW_BANNER ? : null} {/* TopBar */} -
+
@@ -43,7 +49,7 @@ export function PageSkeleton({ skipPaths }: { skipPaths?: RegExp[] }) {
{/* Sidebar */} -
+
diff --git a/app/components/Sidebar.tsx b/app/components/Sidebar.tsx index cab8263efb..16d1b92c50 100644 --- a/app/components/Sidebar.tsx +++ b/app/components/Sidebar.tsx @@ -12,6 +12,7 @@ import { Action16Icon, Document16Icon } from '@oxide/design-system/icons/react' import { useIsActivePath } from '~/hooks/use-is-active-path' import { openQuickActions } from '~/hooks/use-quick-actions' +import { sidebarWrapperClass } from '~/layouts/helpers' import { Button } from '~/ui/lib/Button' import { Truncate } from '~/ui/lib/Truncate' @@ -62,7 +63,12 @@ const JumpToButton = () => { export function Sidebar({ children }: { children: React.ReactNode }) { return ( -
+
diff --git a/app/components/TopBar.tsx b/app/components/TopBar.tsx index cc3c61616d..11f277b7c9 100644 --- a/app/components/TopBar.tsx +++ b/app/components/TopBar.tsx @@ -22,6 +22,7 @@ import { import { useCrumbs } from '~/hooks/use-crumbs' import { useCurrentUser } from '~/hooks/use-current-user' +import { topBarWrapperClass } from '~/layouts/helpers' import { useThemeStore, type Theme } from '~/stores/theme' import { buttonStyle } from '~/ui/lib/Button' import * as DropdownMenu from '~/ui/lib/DropdownMenu' @@ -33,7 +34,7 @@ import { pb } from '~/util/path-builder' export function TopBar({ systemOrSilo }: { systemOrSilo: 'system' | 'silo' }) { const { me } = useCurrentUser() return ( -
+
diff --git a/app/layouts/helpers.tsx b/app/layouts/helpers.tsx index 4c77b17410..ffdab7970e 100644 --- a/app/layouts/helpers.tsx +++ b/app/layouts/helpers.tsx @@ -15,6 +15,12 @@ import { classed } from '~/util/classed' export const PageContainer = classed.div`min-h-full pt-(--top-bar-height)` +// shared with PageSkeleton so the skeleton doesn't drift from the real layout +export const topBarWrapperClass = + 'bg-default border-secondary fixed top-0 right-0 left-0 z-(--z-top-bar) grid h-(--top-bar-height) grid-cols-[var(--sidebar-width)_1fr] border-b' +export const sidebarWrapperClass = + 'border-secondary fixed top-(--top-bar-height) bottom-0 left-0 w-(--sidebar-width) border-r' + export function ContentPane() { useScrollRestoration() return ( From b4a833a6334b7d25b95eb88a65f75ee3884581fd Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 21 Apr 2026 10:22:25 -0500 Subject: [PATCH 11/15] only need one :root --- app/ui/styles/index.css | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/ui/styles/index.css b/app/ui/styles/index.css index 9c4dee7099..48e92c60de 100644 --- a/app/ui/styles/index.css +++ b/app/ui/styles/index.css @@ -131,13 +131,9 @@ --sidebar-width: 14.25rem; @media (max-width: 767px) { - :root { - --content-gutter: 1.5rem; - } + --content-gutter: 1.5rem; } - } - :root { /* Nicer easing from: https://twitter.com/bdc */ --ease-in-quad: cubic-bezier(0.55, 0.085, 0.68, 0.53); --ease-in-cubic: cubic-bezier(0.55, 0.055, 0.675, 0.19); From 4d4e03f938c91eec086b09575a4b9922b3a3985d Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 21 Apr 2026 10:34:38 -0500 Subject: [PATCH 12/15] modal=true on baseui dialogs, fix scrim behind nested image upload modal --- app/ui/lib/DialogOverlay.tsx | 47 ++++++++++++++++++++++++---------- app/ui/lib/Modal.tsx | 6 ++--- app/ui/lib/SideModal.tsx | 1 - app/ui/styles/vars.css | 3 ++- test/e2e/image-upload.e2e.ts | 31 ++++++++++++++++++++-- test/e2e/project-create.e2e.ts | 8 ++++++ 6 files changed, 74 insertions(+), 22 deletions(-) diff --git a/app/ui/lib/DialogOverlay.tsx b/app/ui/lib/DialogOverlay.tsx index 960fc9ca79..b2ed5fb7e2 100644 --- a/app/ui/lib/DialogOverlay.tsx +++ b/app/ui/lib/DialogOverlay.tsx @@ -6,21 +6,40 @@ * Copyright Oxide Computer Company */ +import { Dialog as BaseDialog } from '@base-ui/react/dialog' import * as m from 'motion/react-m' import { type Ref } from 'react' -type Props = { - ref?: Ref -} +import { useIsInModal, useIsInSideModal } from './modal-context' + +type Props = { ref?: Ref } -export const DialogOverlay = ({ ref }: Props) => ( - -) +// Dialog.Backdrop registers itself with base-ui so clicks on it dismiss the +// dialog when modal={true}. A plain
here would not. +export const DialogOverlay = ({ ref }: Props) => { + const isInModal = useIsInModal() + const isInSideModal = useIsInSideModal() + // Modal scrim sits above the SideModal popup so Modal-over-SideModal is + // fully covered; SideModal scrim sits below its own popup. Modal wins when + // both contexts are set (Modal nested inside SideModal), mirroring + // usePopoverZIndex's precedence. + const zClass = + isInSideModal && !isInModal ? 'z-(--z-side-modal-overlay)' : 'z-(--z-modal-overlay)' + return ( + // forceRender so the Modal scrim still renders when nested inside a + // SideModal — otherwise base-ui hides it and the SideModal stays interactive. + + } + /> + ) +} diff --git a/app/ui/lib/Modal.tsx b/app/ui/lib/Modal.tsx index f737115180..15d3a8d3f0 100644 --- a/app/ui/lib/Modal.tsx +++ b/app/ui/lib/Modal.tsx @@ -37,9 +37,8 @@ export type ModalProps = { overlay?: boolean } -// Note that the overlay has z-index 30 and content has 40. This is to make sure -// both land on top of a side modal in the regrettable case where we have both -// on screen at once. +// Overlay sits above --z-side-modal and content above that, so the Modal fully +// covers a SideModal in the regrettable case where both are on screen at once. export function Modal({ children, @@ -59,7 +58,6 @@ export function Modal({ // onFocusOutside preventDefault). See oxidecomputer/console#1745. if (!open && reason !== 'focus-out') onDismiss() }} - modal={false} > {overlay && } diff --git a/app/ui/lib/SideModal.tsx b/app/ui/lib/SideModal.tsx index deb5170ea7..7fdf946149 100644 --- a/app/ui/lib/SideModal.tsx +++ b/app/ui/lib/SideModal.tsx @@ -59,7 +59,6 @@ export function SideModal({ onOpenChange={(open) => { if (!open) onDismiss() }} - modal={false} > diff --git a/app/ui/styles/vars.css b/app/ui/styles/vars.css index 40a9c0b3b2..43426a150b 100644 --- a/app/ui/styles/vars.css +++ b/app/ui/styles/vars.css @@ -11,8 +11,9 @@ --z-modal-dropdown: 50; --z-modal: 40; --z-side-modal-dropdown: 40; + --z-modal-overlay: 35; --z-side-modal: 30; - --z-modal-overlay: 25; + --z-side-modal-overlay: 25; --z-top-bar-dropdown: 20; --z-top-bar: 15; --z-popover: 10; diff --git a/test/e2e/image-upload.e2e.ts b/test/e2e/image-upload.e2e.ts index cfb3f97e03..fcfd9476a3 100644 --- a/test/e2e/image-upload.e2e.ts +++ b/test/e2e/image-upload.e2e.ts @@ -193,8 +193,8 @@ test.describe('Image upload', () => { await expect(progressModal).toBeVisible() expect(confirmCount).toEqual(1) - // now let's try canceling by clicking out on the background over the side modal - await page.getByLabel('4096').click() + // now try dismissing by clicking the scrim outside the progress modal + await page.mouse.click(50, 50) await sleep(300) @@ -202,6 +202,33 @@ test.describe('Image upload', () => { expect(confirmCount).toEqual(2) }) + // regression test for the nested-dialog scrim: the progress modal's backdrop + // must cover the SideModal behind it. Without forceRender on Dialog.Backdrop, + // base-ui hides a nested backdrop by default, leaving the SideModal + // interactive through the "overlay". Without raising --z-modal-overlay above + // --z-side-modal, the overlay sits below the SideModal and doesn't cover it. + test('progress modal scrim covers the side modal underneath', async ({ + page, + browserName, + }) => { + // eslint-disable-next-line playwright/no-skipped-test + test.skip(browserName === 'webkit', 'safari. stop this') + + await fillForm(page, 'new-image') + + const progressModal = page.getByRole('dialog', { name: 'Image upload progress' }) + await page.getByRole('button', { name: 'Upload image' }).click() + await expect(progressModal).toBeVisible() + + // 4096 is a block-size radio in the SideModal behind the progress modal. + // Playwright's actionability check should fail here: the scrim intercepts + // pointer events, so the click can't land on the radio. Without the fix, + // nothing covers the radio and the click would succeed. + await expect(page.getByLabel('4096').click({ timeout: 2000 })).rejects.toThrow( + /intercepts pointer events/ + ) + }) + test('Image upload cancel and retry', async ({ page, browserName }) => { // eslint-disable-next-line playwright/no-skipped-test test.skip(browserName === 'webkit', 'safari. stop this') diff --git a/test/e2e/project-create.e2e.ts b/test/e2e/project-create.e2e.ts index 4e356439ba..db469b1c00 100644 --- a/test/e2e/project-create.e2e.ts +++ b/test/e2e/project-create.e2e.ts @@ -29,6 +29,14 @@ test.describe('Project create', () => { await expect(page).toHaveURL('/projects/mock-project-2/instances') }) + test('clicking the scrim dismisses the side modal', async ({ page }) => { + const dialog = page.getByRole('dialog', { name: /Create project/ }) + await expect(dialog).toBeVisible() + // click well to the left of the side modal panel — that's the scrim + await page.mouse.click(50, 50) + await expect(dialog).toBeHidden() + }) + test('shows field-level validation error and does not POST', async ({ page }) => { const expectInputError = async (text: string, error: string) => { await page.getByRole('textbox', { name: 'Name' }).fill(text) From 8e8974371f3052a5fb83fd3da947e266d5eb2302 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 21 Apr 2026 16:46:00 -0500 Subject: [PATCH 13/15] fix failing e2e tests --- test/e2e/breadcrumbs.e2e.ts | 8 ++++---- test/e2e/instance.e2e.ts | 13 +++++++------ test/e2e/nav-guard-modal.e2e.ts | 10 ++++++++-- test/e2e/vpcs.e2e.ts | 4 +++- test/e2e/z-index.e2e.ts | 6 ++++-- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/test/e2e/breadcrumbs.e2e.ts b/test/e2e/breadcrumbs.e2e.ts index 695e895213..e53631d3a1 100644 --- a/test/e2e/breadcrumbs.e2e.ts +++ b/test/e2e/breadcrumbs.e2e.ts @@ -9,10 +9,10 @@ import { expect, test, type Page } from '@playwright/test' async function getCrumbs(page: Page) { - const links = await page - .getByRole('navigation', { name: 'Breadcrumbs' }) - .getByRole('link') - .all() + // CSS locator instead of getByRole so we can still read breadcrumbs while a + // dialog is open — base-ui sets aria-hidden on outside content, which hides + // the nav from the accessibility tree. + const links = await page.locator('nav[aria-label="Breadcrumbs"] a').all() return Promise.all( links.map(async (link) => [await link.textContent(), await link.getAttribute('href')]) ) diff --git a/test/e2e/instance.e2e.ts b/test/e2e/instance.e2e.ts index aeee657472..2d60ebcdf1 100644 --- a/test/e2e/instance.e2e.ts +++ b/test/e2e/instance.e2e.ts @@ -11,6 +11,7 @@ import { closeToast, expect, expectRowVisible, + fillNumberInput, test, type Page, } from './utils' @@ -168,8 +169,8 @@ test('can resize a failed or stopped instance', async ({ page }) => { await clickRowAction(page, 'you-fail', 'Resize') const resizeModal = page.getByRole('dialog', { name: 'Resize instance' }) await expect(resizeModal).toBeVisible() - await resizeModal.getByRole('textbox', { name: 'vCPUs' }).fill('10') - await resizeModal.getByRole('textbox', { name: 'Memory' }).fill('20') + await fillNumberInput(resizeModal.getByRole('textbox', { name: 'vCPUs' }), '10') + await fillNumberInput(resizeModal.getByRole('textbox', { name: 'Memory' }), '20') await resizeModal.getByRole('button', { name: 'Resize' }).click() await expectRowVisible(table, { name: 'you-fail', @@ -195,8 +196,8 @@ test('can resize a failed or stopped instance', async ({ page }) => { await expect(resizeModal).toBeVisible() await expect(resizeModal.getByText('Current (db1): 2 vCPUs / 4 GiB')).toBeVisible() - await resizeModal.getByRole('textbox', { name: 'vCPUs' }).fill('8') - await resizeModal.getByRole('textbox', { name: 'Memory' }).fill('16') + await fillNumberInput(resizeModal.getByRole('textbox', { name: 'vCPUs' }), '8') + await fillNumberInput(resizeModal.getByRole('textbox', { name: 'Memory' }), '16') await resizeModal.getByRole('button', { name: 'Resize' }).click() await expectRowVisible(table, { name: 'db1', @@ -214,8 +215,8 @@ test('resize modal stays open on server error', async ({ page }) => { const resizeModal = page.getByRole('dialog', { name: 'Resize instance' }) await expect(resizeModal).toBeVisible() - await resizeModal.getByRole('textbox', { name: 'vCPUs' }).fill('10') - await resizeModal.getByRole('textbox', { name: 'Memory' }).fill('20') + await fillNumberInput(resizeModal.getByRole('textbox', { name: 'vCPUs' }), '10') + await fillNumberInput(resizeModal.getByRole('textbox', { name: 'Memory' }), '20') await resizeModal.getByRole('button', { name: 'Resize' }).click() // Error renders inline inside the modal; modal stays open so the user can diff --git a/test/e2e/nav-guard-modal.e2e.ts b/test/e2e/nav-guard-modal.e2e.ts index e3ef40c6f5..da2a4b8abe 100644 --- a/test/e2e/nav-guard-modal.e2e.ts +++ b/test/e2e/nav-guard-modal.e2e.ts @@ -10,14 +10,20 @@ import { expect, expectObscured, test } from './utils' test('navigating away from SideModal form triggers nav guard', async ({ page }) => { const floatingIpsPage = '/projects/mock-project/floating-ips' - const formModal = page.getByRole('dialog', { name: 'Create floating IP' }) + // CSS locator for the form modal: when the confirmModal opens on top of + // it, base-ui aria-hides the form modal (it's outside the inner dialog's + // portal tree), so role-based locators can't find it. + const formModal = page.locator('[role=dialog]:has(h2:text-is("Create floating IP"))') const confirmModal = page.getByRole('dialog', { name: 'Confirm navigation' }) await page.goto(floatingIpsPage) // we don't have to force click here because it's not covered by the modal overlay yet await expect(formModal).toBeHidden() - const somethingOnPage = page.getByRole('heading', { name: 'Floating IPs' }) + // CSS locator so we can still find the heading while the dialog is open — + // base-ui sets aria-hidden on outside content, which hides it from the + // accessibility tree that getByRole walks. + const somethingOnPage = page.locator('h1:has-text("Floating IPs")') await somethingOnPage.click({ trial: true }) // test that it's not obscured // now open the modal diff --git a/test/e2e/vpcs.e2e.ts b/test/e2e/vpcs.e2e.ts index 9361de732d..6df00b31ba 100644 --- a/test/e2e/vpcs.e2e.ts +++ b/test/e2e/vpcs.e2e.ts @@ -126,8 +126,9 @@ test('can create and delete subnet', async ({ page }) => { test('can create and update subnets with a custom router', async ({ page }) => { await page.goto('/projects/mock-project/vpcs/mock-vpc/subnets') - await page.getByRole('link', { name: 'New VPC subnet' }).click() + // Check initial table state before opening the dialog — once it's open the + // table is aria-hidden and role-based locators won't find it. const table = page.getByRole('table') const rows = table.getByRole('row') await expect(rows).toHaveCount(3) @@ -137,6 +138,7 @@ test('can create and update subnets with a custom router', async ({ page }) => { 'IP Block': expect.stringContaining('10.1.1.1/24'), }) + await page.getByRole('link', { name: 'New VPC subnet' }).click() const dialog = page.getByRole('dialog', { name: 'Create VPC subnet' }) await expect(dialog).toBeVisible() diff --git a/test/e2e/z-index.e2e.ts b/test/e2e/z-index.e2e.ts index 4c6c3c1a15..8c20f937bb 100644 --- a/test/e2e/z-index.e2e.ts +++ b/test/e2e/z-index.e2e.ts @@ -30,8 +30,10 @@ test('Dropdown content in SidebarModal shows on screen', async ({ page }) => { const sidebar = page.getByRole('dialog', { name: 'Add network interface' }) - // verify that the SideModal header is positioned above the TopBar - await expectObscured(page.getByRole('button', { name: 'User menu' })) + // verify that the SideModal header is positioned above the TopBar. CSS + // locator instead of getByRole because base-ui sets aria-hidden on outside + // content while the dialog is open. + await expectObscured(page.locator('button[aria-label="User menu"]')) // test that the form can be submitted and a new network interface is created await sidebar.getByRole('button', { name: 'Add network interface' }).click() From 9881dd5d84839ca7f4de2d377dca71e4a0b1b443 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 22 Apr 2026 05:22:06 -0500 Subject: [PATCH 14/15] write e2e test output to .e2e-logs for agents --- .gitignore | 1 + .oxlintrc.json | 11 ++++-- AGENTS.md | 1 + playwright.config.ts | 4 +++ test/e2e/compact-reporter.ts | 70 ++++++++++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 test/e2e/compact-reporter.ts diff --git a/.gitignore b/.gitignore index 6897a4a16a..66386efb5a 100644 --- a/.gitignore +++ b/.gitignore @@ -31,6 +31,7 @@ Thumbs.db test-results/ ci-e2e-traces/ playwright-report/ +.e2e-logs/ .vercel # Visual regression snapshots diff --git a/.oxlintrc.json b/.oxlintrc.json index 905cb463be..901a1248cc 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -98,14 +98,21 @@ "app/layouts/**/*", "app/forms/**/*", "*.config.ts", - "*.config.mjs" + "*.config.mjs", + "test/e2e/compact-reporter.ts" ], "rules": { "import/no-default-export": "off" } }, { - "files": ["**/*.spec.ts", "**/*.config.ts", "**/*.config.mjs", "tools/**/*"], + "files": [ + "**/*.spec.ts", + "**/*.config.ts", + "**/*.config.mjs", + "tools/**/*", + "test/e2e/compact-reporter.ts" + ], "rules": { "import/no-nodejs-modules": "off" } diff --git a/AGENTS.md b/AGENTS.md index 295ca673bc..045ac990f2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,6 +32,7 @@ - Co-locate Vitest specs next to the code they cover; use Testing Library utilities (`render`, `renderHook`, `fireEvent`, fake timers) to assert observable output rather than implementation details (`app/ui/lib/FileInput.spec.tsx`, `app/hooks/use-pagination.spec.ts`). - For sweeping styling changes, coordinate with the visual regression harness and follow `test/visual/README.md` for the workflow. - Fix root causes of flaky timing rather than adding `sleep()` workarounds in tests. +- Local Playwright runs write a compact plain-text report to `.e2e-logs/` (gitignored, one timestamped `.log` per run, last 10 kept) via the custom reporter at `test/e2e/compact-reporter.ts`. Top line is `status: ... total=N passed=N ...`; each failure is a `── UNEXPECTED|FLAKY file:line title` block followed by the error (ANSI stripped). Latest run: `ls .e2e-logs | tail -1` — Read it directly, no parsing needed. # Data fetching pattern diff --git a/playwright.config.ts b/playwright.config.ts index caaec6dd57..892b51613e 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -24,6 +24,10 @@ export default { // default is 5 seconds. somehow playwright really hates async route modules, // takes a long time to load them. https://playwright.dev/docs/test-timeouts expect: { timeout: 10_000 }, + // Local runs also emit a compact plain-text failure report to .e2e-logs/ + // (timestamped per run, last 10 kept) via test/e2e/compact-reporter.ts, so + // an LLM agent can read the failures. + reporter: process.env.CI ? 'list' : [['list'], ['./test/e2e/compact-reporter.ts']], use: { trace: process.env.CI ? 'on-first-retry' : 'retain-on-failure', baseURL: 'http://localhost:4009', diff --git a/test/e2e/compact-reporter.ts b/test/e2e/compact-reporter.ts new file mode 100644 index 0000000000..d67c5c8d9a --- /dev/null +++ b/test/e2e/compact-reporter.ts @@ -0,0 +1,70 @@ +import { mkdirSync, readdirSync, rmSync, writeFileSync } from 'node:fs' +import { relative } from 'node:path' + +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * Copyright Oxide Computer Company + */ +import type { FullConfig, FullResult, Reporter, Suite } from '@playwright/test/reporter' +import stripAnsi from 'strip-ansi' + +// Compact plain-text reporter for local runs. Writes a timestamped file to +// `.e2e-logs/` per run with a summary line and one block per failed or flaky +// test (ANSI stripped). Cheap for an LLM agent to read end-to-end. +// Latest: `ls .e2e-logs | tail -1`. + +export default class CompactReporter implements Reporter { + private suite!: Suite + private rootDir = '' + + onBegin(config: FullConfig, suite: Suite) { + this.suite = suite + this.rootDir = config.rootDir + } + + async onEnd(result: FullResult) { + const tests = this.suite.allTests() + const counts = { + total: tests.length, + passed: tests.filter((t) => t.outcome() === 'expected').length, + failed: tests.filter((t) => t.outcome() === 'unexpected').length, + flaky: tests.filter((t) => t.outcome() === 'flaky').length, + skipped: tests.filter((t) => t.outcome() === 'skipped').length, + } + + const lines = [ + `status: ${result.status} total=${counts.total} passed=${counts.passed} failed=${counts.failed} flaky=${counts.flaky} skipped=${counts.skipped}`, + ] + + for (const t of tests) { + const outcome = t.outcome() + if (outcome !== 'unexpected' && outcome !== 'flaky') continue + const last = t.results[t.results.length - 1] + const err = stripAnsi(last?.error?.message ?? last?.errors[0]?.message ?? '') + const file = relative(this.rootDir, t.location.file) + const title = t.titlePath().slice(1).join(' › ') + lines.push( + '', + `── ${outcome.toUpperCase()} ${file}:${t.location.line} ${title}`, + ` attempts=${t.results.length} duration=${Math.round(last?.duration ?? 0)}ms`, + '', + err + ) + } + + const stamp = new Date().toISOString().replace(/[:.]/g, '-') + mkdirSync('.e2e-logs', { recursive: true }) + writeFileSync(`.e2e-logs/${stamp}.log`, lines.join('\n') + '\n') + + // Keep only the 10 most recent runs. Timestamps sort lexicographically. + const KEEP = 10 + const old = readdirSync('.e2e-logs') + .filter((f) => f.endsWith('.log')) + .sort() + .slice(0, -KEEP) + for (const f of old) rmSync(`.e2e-logs/${f}`) + } +} From 9faa57711849be0cd57232b66a7e9bbd5255bfd0 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 22 Apr 2026 06:15:09 -0500 Subject: [PATCH 15/15] opt out of native scroll restore --- app/hooks/use-scroll-restoration.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/hooks/use-scroll-restoration.ts b/app/hooks/use-scroll-restoration.ts index cb2ff0c9e7..5d0a63c9f6 100644 --- a/app/hooks/use-scroll-restoration.ts +++ b/app/hooks/use-scroll-restoration.ts @@ -22,11 +22,22 @@ function setScrollPosition(key: string, pos: number) { * return (e.g., back/forward nav). Note that `location.key` is used in the * cache key, not `location.pathname`, so the same path navigated to at * different points in the history stack will not share the same scroll position. + * + * We tried RR's built-in `` and it didn't work — on + * back/forward nav, `window.scrollTo` was called with the right value but the + * document was still at viewport height at that moment, so the scroll got + * clamped to 0. We're not sure why; a theory is that RR restores in a + * `useLayoutEffect` which fires before some later render expands the content, + * and our `useEffect` after paint happens to catch that later render. */ export function useScrollRestoration() { const key = `scroll-position-${useLocation().key}` const { state } = useNavigation() useEffect(() => { + // opt out of the browser's native scroll restoration so it doesn't jump + // the still-visible old page to the new page's saved position on POP, + // before the new route's loader resolves. We restore manually below. + window.history.scrollRestoration = 'manual' if (state === 'loading') { setScrollPosition(key, window.scrollY) } else if (state === 'idle') {