diff --git a/app/src/components/RootLayout.test.tsx b/app/src/components/RootLayout.test.tsx new file mode 100644 index 0000000000..4274635fea --- /dev/null +++ b/app/src/components/RootLayout.test.tsx @@ -0,0 +1,80 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render } from '@testing-library/react'; +import { ThemeProvider, createTheme } from '@mui/material/styles'; +import { MemoryRouter, Routes, Route, Link } from 'react-router-dom'; +import userEvent from '@testing-library/user-event'; +import { RootLayout } from './RootLayout'; + +vi.mock('../hooks', () => ({ + useAnalytics: () => ({ trackEvent: vi.fn(), trackPageview: vi.fn() }), +})); + +vi.mock('./MastheadRule', () => ({ + MastheadRule: () =>
, +})); + +vi.mock('./NavBar', () => ({ + NavBar: () =>
, +})); + +vi.mock('./Footer', () => ({ + Footer: () =>
, +})); + +const theme = createTheme(); + +function renderAt(initialPath: string) { + return render( + + + + }> + + go-legal + go-about-hash +
+ } + /> + legal-page
} /> + about-page
} /> + + + + , + ); +} + +describe('RootLayout', () => { + let scrollSpy: ReturnType; + + beforeEach(() => { + scrollSpy = vi.spyOn(window, 'scrollTo').mockImplementation(() => {}); + }); + + afterEach(() => { + scrollSpy.mockRestore(); + }); + + it('scrolls to top when navigating to a new path (PUSH)', async () => { + const user = userEvent.setup(); + renderAt('/'); + scrollSpy.mockClear(); + + await user.click(document.querySelector('a[href="/legal"]') as HTMLElement); + + expect(scrollSpy).toHaveBeenCalledWith(0, 0); + }); + + it('does not scroll to top when the destination has a hash anchor', async () => { + const user = userEvent.setup(); + renderAt('/'); + scrollSpy.mockClear(); + + await user.click(document.querySelector('a[href="/about#section"]') as HTMLElement); + + expect(scrollSpy).not.toHaveBeenCalledWith(0, 0); + }); +}); diff --git a/app/src/components/RootLayout.tsx b/app/src/components/RootLayout.tsx index e0b398a38f..94e80b5e2f 100644 --- a/app/src/components/RootLayout.tsx +++ b/app/src/components/RootLayout.tsx @@ -1,4 +1,5 @@ -import { Outlet, useLocation } from 'react-router-dom'; +import { useEffect } from 'react'; +import { Outlet, useLocation, useNavigationType } from 'react-router-dom'; import Box from '@mui/material/Box'; import Container from '@mui/material/Container'; @@ -22,9 +23,22 @@ const containerSx = { */ export function RootLayout() { const { trackEvent } = useAnalytics(); - const { pathname } = useLocation(); + const { pathname, hash } = useLocation(); + const navigationType = useNavigationType(); const mastheadSticks = pathname !== '/plots'; + // Reset scroll on forward navigation (PUSH/REPLACE). Without this, short + // pages like /legal inherit the previous page's scroll position because + // PlotsPage sets scrollRestoration='manual'. Skip on POP so browser + // back/forward keeps native scroll restoration; skip when a hash anchor + // is present so in-page anchors still work. PlotsPage runs its own + // saved-scroll restore in a later effect, so it still overrides this. + useEffect(() => { + if (hash) return; + if (navigationType === 'POP') return; + window.scrollTo(0, 0); + }, [pathname, hash, navigationType]); + return ( { render(); expect(screen.getByTestId('helmet')).toBeInTheDocument(); }); + + describe('scrollRestoration', () => { + const original = history.scrollRestoration; + + afterEach(() => { + history.scrollRestoration = original; + }); + + it("sets scrollRestoration to 'manual' on mount and restores 'auto' on unmount", () => { + history.scrollRestoration = 'auto'; + const { unmount } = render(); + expect(history.scrollRestoration).toBe('manual'); + unmount(); + expect(history.scrollRestoration).toBe('auto'); + }); + }); }); diff --git a/app/src/pages/HomePage.tsx b/app/src/pages/HomePage.tsx index 3a832cd160..13e0cf2a88 100644 --- a/app/src/pages/HomePage.tsx +++ b/app/src/pages/HomePage.tsx @@ -28,11 +28,15 @@ export function HomePage() { const { specsData, librariesData, stats } = useAppData(); const { homeStateRef, saveScrollPosition } = useHomeState(); - // Disable browser's automatic scroll restoration + // Disable browser's automatic scroll restoration so we can restore from + // our persisted state (homeStateRef.scrollY) instead. Restore on unmount + // so other routes get native back/forward behavior. useEffect(() => { - if ('scrollRestoration' in history) { - history.scrollRestoration = 'manual'; - } + if (!('scrollRestoration' in history)) return; + history.scrollRestoration = 'manual'; + return () => { + history.scrollRestoration = 'auto'; + }; }, []); // Custom hooks diff --git a/app/src/pages/PlotsPage.test.tsx b/app/src/pages/PlotsPage.test.tsx index cc27bfe507..b2229b44a8 100644 --- a/app/src/pages/PlotsPage.test.tsx +++ b/app/src/pages/PlotsPage.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { render, screen } from '../test-utils'; vi.mock('../hooks', () => ({ @@ -68,4 +68,20 @@ describe('PlotsPage', () => { render(); expect(screen.getByTestId('helmet')).toBeInTheDocument(); }); + + describe('scrollRestoration', () => { + const original = history.scrollRestoration; + + afterEach(() => { + history.scrollRestoration = original; + }); + + it("sets scrollRestoration to 'manual' on mount and restores 'auto' on unmount", () => { + history.scrollRestoration = 'auto'; + const { unmount } = render(); + expect(history.scrollRestoration).toBe('manual'); + unmount(); + expect(history.scrollRestoration).toBe('auto'); + }); + }); }); diff --git a/app/src/pages/PlotsPage.tsx b/app/src/pages/PlotsPage.tsx index 19c22dfb23..ca579da667 100644 --- a/app/src/pages/PlotsPage.tsx +++ b/app/src/pages/PlotsPage.tsx @@ -21,11 +21,15 @@ export function PlotsPage() { const { specsData, librariesData } = useAppData(); const { homeStateRef, saveScrollPosition } = useHomeState(); - // Disable browser's automatic scroll restoration + // Disable browser's automatic scroll restoration so we can restore from + // our persisted state (homeStateRef.scrollY) instead. Restore on unmount + // so other routes get native back/forward behavior. useEffect(() => { - if ('scrollRestoration' in history) { - history.scrollRestoration = 'manual'; - } + if (!('scrollRestoration' in history)) return; + history.scrollRestoration = 'manual'; + return () => { + history.scrollRestoration = 'auto'; + }; }, []); const { trackPageview, trackEvent } = useAnalytics();