diff --git a/app/components/Sidebar.tsx b/app/components/Sidebar.tsx index 0cc380ca40..c4705f50e2 100644 --- a/app/components/Sidebar.tsx +++ b/app/components/Sidebar.tsx @@ -56,7 +56,7 @@ const JumpToButton = () => { export function Sidebar({ children }: { children: React.ReactNode }) { return ( -
+
diff --git a/app/components/TopBar.tsx b/app/components/TopBar.tsx index 8c33db531e..527bf44cb4 100644 --- a/app/components/TopBar.tsx +++ b/app/components/TopBar.tsx @@ -31,13 +31,12 @@ export function TopBar({ children }: { children: React.ReactNode }) { // 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 ( - <> -
+
+
{cornerPicker}
- {/* Height is governed by PageContainer grid */} - {/* shrink-0 is needed to prevent getting squished by body content */} -
+
+ {/* shrink-0 is needed to prevent getting squished by body content */}
{otherPickers}
@@ -65,6 +64,6 @@ export function TopBar({ children }: { children: React.ReactNode }) {
- +
) } diff --git a/app/hooks/use-scroll-restoration.ts b/app/hooks/use-scroll-restoration.ts index 8dbd8a5848..5653257fe9 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 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 ?? 0) } 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/AuthenticatedLayout.tsx b/app/layouts/AuthenticatedLayout.tsx index b4cbc16682..e47a1b63b8 100644 --- a/app/layouts/AuthenticatedLayout.tsx +++ b/app/layouts/AuthenticatedLayout.tsx @@ -5,7 +5,7 @@ * * Copyright Oxide Computer Company */ -import { Outlet } from 'react-router-dom' +import { Outlet, ScrollRestoration } from 'react-router-dom' import { apiQueryClient, useApiQueryErrorsAllowed, usePrefetchedApiQuery } from '@oxide/api' @@ -43,6 +43,7 @@ export function AuthenticatedLayout() { <> + ) } diff --git a/app/layouts/helpers.tsx b/app/layouts/helpers.tsx index 64a00c9cb0..fc0d1e376a 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-dom' import { PageActionsTarget } from '~/components/PageActions' @@ -14,20 +13,21 @@ 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-[60px,1fr]` +export const PageContainer = classed.div`min-h-screen pt-[var(--navigation-height)] [overscroll-behavior-y:none]` +// TODO: this doesn't go tall enough on a tall screen to get the pagination bar to the bottom +// http://localhost:4000/projects/mock-project/disks export function ContentPane() { - const ref = useRef(null) - useScrollRestoration(ref) + useScrollRestoration() return ( -
-
+
+
-
+
@@ -42,7 +42,10 @@ export function ContentPane() { * `
` because we don't need it. */ export const SerialConsoleContentPane = () => ( -
+
diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index a6f3bdacfe..bf536f3450 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -135,7 +135,7 @@ export function SerialConsolePage() { }, [canConnect]) return ( -
+
container.evaluate((el: HTMLElement) => el.scrollTop) - await expect.poll(getScrollTop).toBe(expected) + await page.waitForFunction((expected) => window.scrollY === expected, expected) } 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((y) => { + window.scrollTo(0, y) + }, to) } test('scroll restore', async ({ page }) => { - // open small window to make scrolling easier - await page.setViewportSize({ width: 800, height: 500 }) + // open short window to make scrolling easier + // needs to be wide enough to not require the nav to be opened + await page.setViewportSize({ width: 1024, height: 500 }) // nav to disks and scroll it await page.goto('/projects/mock-project/disks') + // this helps us wait till the page is ready to scroll + await expectVisible(page, ['role=heading[name*="Disks"]']) await expectScrollTop(page, 0) await scrollTo(page, 143) // nav to snapshots await page.getByRole('link', { name: 'Snapshots' }).click() + await expectVisible(page, ['role=heading[name*="Snapshots"]']) + await expect(page).toHaveURL('/projects/mock-project/snapshots') await expectScrollTop(page, 0) // go back to disks, scroll is restored, scroll it some more diff --git a/test/e2e/z-index.e2e.ts b/test/e2e/z-index.e2e.ts index 6379685bc3..a76538da00 100644 --- a/test/e2e/z-index.e2e.ts +++ b/test/e2e/z-index.e2e.ts @@ -27,7 +27,7 @@ test('Dropdown content can scroll off page and doesn’t hide TopBar', async ({ // scroll the page down just enough that the button and the top item are off // screen, but the bottom item is not - await page.mouse.wheel(0, 480) + await page.mouse.wheel(0, 500) // if we don't do this, the test doesn't wait long enough for the following // assertions to become true