diff --git a/frontend/src/components/app-sidebar.tsx b/frontend/src/components/app-sidebar.tsx index 5c942253..cdc25092 100644 --- a/frontend/src/components/app-sidebar.tsx +++ b/frontend/src/components/app-sidebar.tsx @@ -93,52 +93,54 @@ export function AppSidebar() { - - Navigation - - - {navItems.map((item) => { - const isActive = currentPath === item.url - return ( - - - - {item.title} - - - ) - })} + diff --git a/frontend/src/components/ui/responsive-dialog.test.tsx b/frontend/src/components/ui/responsive-dialog.test.tsx new file mode 100644 index 00000000..7db2e9c3 --- /dev/null +++ b/frontend/src/components/ui/responsive-dialog.test.tsx @@ -0,0 +1,190 @@ +import { afterEach, expect, test } from 'vitest' +import { page, userEvent } from 'vitest/browser' +import { render } from 'vitest-browser-react' + +import { + ResponsiveDialog, + ResponsiveDialogClose, + ResponsiveDialogDescription, + ResponsiveDialogFooter, + ResponsiveDialogHeader, + ResponsiveDialogTitle, +} from '@/components/ui/responsive-dialog' + +const longTitle = 'Close work order WO-2026-00042 for the north depot vehicle' +const longDescription = + 'Closing this work order records the final odometer reading, marks every open task as done, and sends a notice to ' + + 'the fleet manager. You cannot reopen a closed work order later, so check each field first.' + +afterEach(async () => { + await page.viewport(414, 896) +}) + +function LongDialog({ title, description }: { title: string; description?: string }) { + return ( + + + {title} + {description && {description}} + + {Array.from({ length: 20 }, (_, index) => ( +

+ Field {index + 1} +

+ ))} + + Close work order + + + ) +} + +function FormDialog({ title, description }: { title: string; description?: string }) { + return ( + + + {title} + {description && {description}} + + {Array.from({ length: 20 }, (_, index) => ( + + ))} + + Cancel + Save + + + ) +} + +function isHidden(element: Element): boolean { + const { left, right, top, bottom } = element.getBoundingClientRect() + return [ + [left + 1, top + 1], + [right - 1, bottom - 1], + ].some(([x, y]) => !element.contains(document.elementFromPoint(x, y))) +} + +function closeButton(): Element { + return page.getByRole('button', { name: 'Close', exact: true }).element() +} + +function scrollParent(element: Element): Element { + let parent = element.parentElement + while (parent && getComputedStyle(parent).overflowY !== 'auto') parent = parent.parentElement + if (!parent) throw new Error('The content has no scroll container') + return parent +} + +async function openAnimationsFinished(): Promise { + await expect.element(page.getByRole('dialog')).toBeVisible() + // The modal starts its animations after it paints. + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))) + await Promise.all(document.getAnimations().map((animation) => animation.finished)) +} + +function paragraphTextUnderCloseButton(x: number, y: number): string | undefined { + const button = closeButton() + const shown = document.elementsFromPoint(x, y).find((element) => !button.contains(element)) + return shown?.closest('p')?.textContent ?? undefined +} + +for (const [width, height] of [ + [1280, 720], + [390, 844], +]) { + test(`at ${width}x${height} the title of a long dialog does not run under the close button`, async () => { + await page.viewport(width, height) + render() + await openAnimationsFinished() + + const button = closeButton().getBoundingClientRect() + const range = document.createRange() + range.selectNodeContents(page.getByRole('heading', { name: longTitle }).element()) + const linesUnderButton = Array.from(range.getClientRects()).filter( + (line) => line.left < button.right && button.left < line.right && line.top < button.bottom && button.top < line.bottom, + ) + expect(linesUnderButton).toEqual([]) + }) + + test(`at ${width}x${height} the content of a long dialog does not scroll under the close button`, async () => { + await page.viewport(width, height) + render() + await openAnimationsFinished() + + scrollParent(page.getByText('Field 1', { exact: true }).element()).scrollTop = 120 + + await expect.poll(() => { + const { left, right, top, bottom } = closeButton().getBoundingClientRect() + expect(closeButton().contains(document.elementFromPoint((left + right) / 2, (top + bottom) / 2))).toBe(true) + return [ + [left + 1, top + 1], + [right - 1, top + 1], + [left + 1, bottom - 1], + [right - 1, bottom - 1], + ].map(([x, y]) => paragraphTextUnderCloseButton(x, y)).filter((text) => text?.startsWith('Field ')) + }).toEqual([]) + }) + + test(`at ${width}x${height} the header and footer of a long form do not hide the field in focus`, async () => { + await page.viewport(width, height) + render() + await openAnimationsFinished() + + scrollParent(page.getByRole('textbox', { name: 'Field 1', exact: true }).element()).scrollTop = 40 + const focused: string[] = [] + const hidden: string[] = [] + async function press(keys: string) { + await userEvent.keyboard(keys) + const field = document.activeElement! + focused.push(field.ariaLabel!) + if (isHidden(field)) hidden.push(field.ariaLabel!) + } + for (let index = 1; index <= 20; index++) await press('{Tab}') + for (let index = 19; index >= 1; index--) await press('{Shift>}{Tab}{/Shift}') + + expect(focused[0]).toBe('Field 1') + expect(hidden).toEqual([]) + }) + + test(`at ${width}x${height} a field in focus that scrolls under the header does not show around the title`, async () => { + await page.viewport(width, height) + render() + await openAnimationsFinished() + + const field = page.getByRole('textbox', { name: 'Field 4', exact: true }).element() as HTMLElement + field.focus() + const box = scrollParent(field) + const title = page.getByRole('heading', { name: 'Edit filter' }).element() + box.scrollTop += field.getBoundingClientRect().top - title.getBoundingClientRect().top + expect(field.getBoundingClientRect().top).toBeCloseTo(title.getBoundingClientRect().top, 0) + + // A focus ring is 3 px wide, and the text of the title reaches about 3 px below its box. + await expect.poll(() => { + const header = title.closest('[data-slot="responsive-dialog-header"]')! + const { left } = box.getBoundingClientRect() + const right = left + box.clientWidth + const { top, bottom } = title.getBoundingClientRect() + const shown = new Set() + for (let y = top; y <= bottom + 3; y++) { + for (let x = left + 1; x < right; x += 2) { + const hit = document.elementFromPoint(x, y) + if (!hit || header.contains(hit) || closeButton().contains(hit)) continue + shown.add(hit === field ? 'the field' : hit.getAttribute('class') ?? hit.tagName) + } + } + return [...shown] + }).toEqual([]) + }) + + test(`at ${width}x${height} the footer of a long dialog stays in view`, async () => { + await page.viewport(width, height) + render() + + await expect.poll(() => { + const action = page.getByRole('button', { name: 'Close work order' }).element() + const { left, top, width: w, height: h } = action.getBoundingClientRect() + return action.contains(document.elementFromPoint(left + w / 2, top + h / 2)) + }).toBe(true) + }) +} diff --git a/frontend/src/components/ui/responsive-dialog.tsx b/frontend/src/components/ui/responsive-dialog.tsx index f0068f3c..dd286c22 100644 --- a/frontend/src/components/ui/responsive-dialog.tsx +++ b/frontend/src/components/ui/responsive-dialog.tsx @@ -45,6 +45,32 @@ function ResponsiveDialogClose({ ) } +type ScrollPadding = { top: number; bottom: number } + +const ScrollPaddingContext = React.createContext> | null>(null) + +// The header and the footer stick over the content that scrolls. The scroll box gets scroll padding of their heights, +// so the browser does not scroll a field that gets focus under them. +function useStickyHeight(side: keyof ScrollPadding) { + const setScrollPadding = React.useContext(ScrollPaddingContext) + const ref = React.useRef(null) + + React.useLayoutEffect(() => { + const element = ref.current + if (!element || !setScrollPadding) return + const setHeight = (height: number) => + setScrollPadding((padding) => (padding[side] === height ? padding : { ...padding, [side]: height })) + const observer = new ResizeObserver(() => setHeight(element.offsetHeight)) + observer.observe(element) + return () => { + observer.disconnect() + setHeight(0) + } + }, [setScrollPadding, side]) + + return ref +} + /** * A dialog that is a bottom sheet on mobile and a centered modal on desktop. * Put it inside a `ResponsiveDialogTrigger`, or control it with `isOpen`. @@ -62,6 +88,7 @@ function ResponsiveDialog({ }) { const isMobile = useIsMobile() const [descriptionId, setDescriptionId] = React.useState() + const [scrollPadding, setScrollPadding] = React.useState({ top: 0, bottom: 0 }) return ( {/* Drag handle for mobile */}
-
+ {/* The rounded corners clip the sticky header and footer, which cover the full width. */} +
- {children} + + {children} +
{showCloseButton && ( @@ -115,28 +151,43 @@ function ResponsiveDialog({ ) } +/** + * Sticks to the top of the content that scrolls. Put it first in the dialog. + * Its background covers the first 0.5rem below it. Leave a gap of at least 0.75rem, so that a focus ring there shows. + */ function ResponsiveDialogHeader({ className, ...props -}: React.ComponentProps<"div">) { +}: Omit, "ref">) { return (
) } +/** Sticks to the bottom of the content that scrolls. Put it last in the dialog. */ function ResponsiveDialogFooter({ className, ...props -}: React.ComponentProps<"div">) { +}: Omit, "ref">) { return (
) {
= { +const pageNameByRoutePath: Partial> = { '/': 'Home', '/about': 'About', '/components': 'Components', @@ -44,9 +45,9 @@ const routeNames: Record = { function RootComponent() { const router = useRouter() const routerState = useRouterState() - const currentPath = routerState.location.pathname + const routePath = routerState.matches[routerState.matches.length - 1]?.fullPath ?? '' const isNotFound = routerState.matches.some((match) => match.globalNotFound) - const pageName = isNotFound ? 'Not Found' : (routeNames[currentPath] ?? 'Page') + const pageName = isNotFound ? 'Not Found' : (pageNameByRoutePath[routePath] ?? 'Page') return ( @@ -76,7 +77,8 @@ function RootComponent() {
- + {/* The default container is a footer, which is a contentinfo landmark. */} + ) diff --git a/frontend/tests/features/app-shell/app-shell.locators.ts b/frontend/tests/features/app-shell/app-shell.locators.ts index 77391500..f323fa08 100644 --- a/frontend/tests/features/app-shell/app-shell.locators.ts +++ b/frontend/tests/features/app-shell/app-shell.locators.ts @@ -4,7 +4,14 @@ export const appShell = (page: Page) => ({ aboutHeading: page.getByRole('heading', { level: 1, name: 'About' }), aboutText: page.getByText('This is a demo application built with .NET Aspire and React'), currentPage: page.getByRole('navigation', { name: 'breadcrumb' }).getByRole('link'), + contentInfoLandmarks: page.getByRole('contentinfo'), + // The About page has no separator of its own, so the one separator in main is the one of the layout header. + headerSeparator: page.getByRole('main').getByRole('separator'), + routerDevtoolsButton: page.getByRole('button', { name: 'Open TanStack Router Devtools' }), + mainNavigation: page.getByRole('navigation', { name: 'Main' }), + sidebarTrigger: page.getByRole('main').getByRole('button', { name: 'Toggle Sidebar' }), mainLandmarks: page.getByRole('main'), + wideContent: page.getByRole('region', { name: 'Wide content' }), notFoundHeading: page.getByRole('heading', { name: 'Page not found' }), homeLink: page.getByRole('link', { name: 'Go to the home page' }), weatherHeading: page.getByRole('heading', { name: 'Weather Forecast' }), diff --git a/frontend/tests/features/app-shell/app-shell.spec.ts b/frontend/tests/features/app-shell/app-shell.spec.ts index 5959056b..bcf41ea4 100644 --- a/frontend/tests/features/app-shell/app-shell.spec.ts +++ b/frontend/tests/features/app-shell/app-shell.spec.ts @@ -52,6 +52,52 @@ test.describe('the landmarks of a page', () => { }) }) +test.describe('the layout header', () => { + test.use({ viewport: { width: 1280, height: 720 } }) + + test('centers the separator on the sidebar trigger', async ({ page }) => { + const shell = appShellPage(page) + + await shell.openAbout() + + expect(await shell.verticalCenter(shell.at.headerSeparator)).toBeCloseTo(await shell.verticalCenter(shell.at.sidebarTrigger), 0) + }) +}) + +test.describe('the landmarks of the layout', () => { + test('the router devtools add no contentinfo landmark', async ({ page }) => { + const shell = appShellPage(page) + + await shell.openAbout() + + await expect(shell.at.routerDevtoolsButton).toBeVisible() + expect(await shell.at.contentInfoLandmarks.count()).toBe(0) + }) + + test('the sidebar links are in a main navigation landmark', async ({ page }) => { + const shell = appShellPage(page) + + await shell.openAbout() + + await expect(shell.at.mainNavigation.getByRole('link', { name: 'Home' })).toBeVisible() + await expect(shell.at.mainNavigation.getByRole('link', { name: 'About' })).toBeVisible() + }) +}) + +test.describe('wide content next to the open sidebar', () => { + test.use({ viewport: { width: 1024, height: 720 } }) + + test('scrolls in its own box, and the page does not scroll sideways', async ({ page }) => { + const shell = appShellPage(page) + await shell.openAbout() + + await shell.addWideContent(1600) + + expect(await shell.sideScrollRange(shell.at.wideContent)).toBeGreaterThan(0) + expect(await shell.pageSideScrollRange()).toBe(0) + }) +}) + test.describe('an unknown address', () => { test('shows a page not found message in the app layout', async ({ page }) => { const shell = appShellPage(page) diff --git a/frontend/tests/features/app-shell/app-shell.tasks.ts b/frontend/tests/features/app-shell/app-shell.tasks.ts index ea6572d0..790f8483 100644 --- a/frontend/tests/features/app-shell/app-shell.tasks.ts +++ b/frontend/tests/features/app-shell/app-shell.tasks.ts @@ -1,4 +1,4 @@ -import { expect, type Page } from '@playwright/test' +import { expect, type Locator, type Page } from '@playwright/test' import { weatherPaths } from '@tests/support/api/weather-api' import { appShell } from './app-shell.locators' @@ -31,6 +31,34 @@ export function appShellPage(page: Page) { await settled }, + async addWideContent(width: number) { + await at.mainLandmarks.evaluate((main, width) => { + const scroller = document.createElement('div') + scroller.className = 'overflow-x-auto' + scroller.role = 'region' + scroller.ariaLabel = 'Wide content' + const content = document.createElement('div') + content.style.width = `${width}px` + content.textContent = 'Wide content' + scroller.append(content) + main.append(scroller) + }, width) + }, + + async pageSideScrollRange() { + return page.evaluate(() => document.documentElement.scrollWidth - document.documentElement.clientWidth) + }, + + async sideScrollRange(locator: Locator) { + return locator.evaluate((element) => element.scrollWidth - element.clientWidth) + }, + + async verticalCenter(locator: Locator) { + const box = await locator.boundingBox() + if (!box) throw new Error('The element has no box') + return box.y + box.height / 2 + }, + async pageScrollRange() { return page.evaluate(() => document.documentElement.scrollHeight - document.documentElement.clientHeight) }, diff --git a/frontend/tests/features/filter-demo/filter-demo.tasks.ts b/frontend/tests/features/filter-demo/filter-demo.tasks.ts index 24923abe..b14f0fee 100644 --- a/frontend/tests/features/filter-demo/filter-demo.tasks.ts +++ b/frontend/tests/features/filter-demo/filter-demo.tasks.ts @@ -190,13 +190,13 @@ export function filterDemoPage(page: Page) { const { width, height } = element.getBoundingClientRect() return width > 1 && height > 1 } - const headerBottom = title.parentElement!.getBoundingClientRect().bottom + const titleBottom = title.getBoundingClientRect().bottom let top = Infinity let bottom = -Infinity for (const element of sheet!.querySelectorAll('*')) { const box = element.getBoundingClientRect() const isLeaf = ['BUTTON', 'INPUT'].includes(element.tagName) || ![...element.children].some(shows) - if (!isLeaf || !shows(element) || box.top < headerBottom) continue + if (!isLeaf || !shows(element) || box.top < titleBottom) continue top = Math.min(top, box.top) bottom = Math.max(bottom, box.bottom) } @@ -205,7 +205,7 @@ export function filterDemoPage(page: Page) { scroller = scroller.parentElement } const contentEnd = scroller.getBoundingClientRect().top + scroller.clientTop - scroller.scrollTop + scroller.scrollHeight - return { top: Math.round(top - headerBottom), bottom: Math.round(contentEnd - bottom) } + return { top: Math.round(top - titleBottom), bottom: Math.round(contentEnd - bottom) } }, sheet) },