diff --git a/docs/AI_USAGE.md b/docs/AI_USAGE.md index 44e324f..389ed05 100644 --- a/docs/AI_USAGE.md +++ b/docs/AI_USAGE.md @@ -590,6 +590,14 @@ - Not done on purpose: moving the error messages of the other pages (the elements results, the element page, the login, "Your gymnasts") to the three layers, because those files belong to other pull requests and the change would conflict with them. The menus of the Elements page still have their own copies of the menu classes that `menuStyles.ts` now holds. - **Limits:** The end-to-end tests need the database and the backend, and are run by hand: the course git server has no runners. A screen reader was not used: that has to be done by a person. Lighthouse was started on a production build in the last round, but the run did not finish (the computer was low on memory), so there are no Lighthouse scores from that round. "Your gymnasts" was fixed after the last check in a browser and has not been measured again, and the results table of the Elements page, which uses the changed badge, has not been looked at in dark mode. The test gymnast was made with the test login in the local database and deleted again. +## The focus after the visitor has acted (`src/components/FocusOnNavigate.tsx`, `e2e/focus.e2e.ts`) + +- **Tool:** Claude Code +- **What the team did:** Took the peer feedback that the focus is lost after "Load more", that the sort menu seemed to stay open after a choice with the keyboard, and that the focus does not move to the content when going to another page. Wrote issue #172. +- **What Claude did:** Checked each of the three before changing anything. The sort menu already closes (`closeOnClick`) and gives the focus back to its button, which names the new sort, so only an end-to-end test was added to keep it so. For the third, added `FocusOnNavigate`, which moves the focus to `#main-content` when the page (the path, not the search) changes, with a unit test that was checked by breaking the code on purpose. +- **Not done here:** "Load more" is being replaced by pagination in #162 (pull request #189), which changes the button and where the focus should go, so the focus after a page change belongs in that work. Doing it here would conflict with it. +- **Limits:** Tested with Playwright and not with a screen reader. + ## A shorter README (`README.md`, `docs/`) - **Tool:** Claude Code diff --git a/e2e/focus.e2e.ts b/e2e/focus.e2e.ts new file mode 100644 index 0000000..467fa07 --- /dev/null +++ b/e2e/focus.e2e.ts @@ -0,0 +1,35 @@ +// Where the focus is after the visitor has done something with the keyboard: it must not be +// lost (on the ), because a keyboard or screen reader user would start again from the top. + +import { expect, test } from '@playwright/test' +import { resultsLoaded } from './helpers.ts' + +test('after a choice in the sort menu with the keyboard, the menu is closed and the focus is on the button, which says the choice', async ({ + page, +}) => { + await page.goto('elements') + await resultsLoaded(page) + + const button = page.getByRole('button', { name: /Sort by Code: low to high/ }) + await button.focus() + await page.keyboard.press('Enter') + await expect(page.getByRole('menu')).toBeVisible() + await page.getByRole('menuitemradio', { name: 'Name: A to Z' }).focus() + await page.keyboard.press('Enter') + + await expect(page.getByRole('menu')).toBeHidden() + const chosen = page.getByRole('button', { name: /Sort by Name: A to Z/ }) + await expect(chosen).toBeFocused() +}) + +test('after a link to another page, the focus is at the start of that page', async ({ + page, +}) => { + await page.goto('elements') + await resultsLoaded(page) + + await page.getByRole('link', { name: 'About' }).click() + await expect(page.getByRole('heading', { level: 1 })).toBeVisible() + + await expect(page.locator('#main-content')).toBeFocused() +}) diff --git a/src/App.tsx b/src/App.tsx index 791a6a8..822fe51 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -3,6 +3,7 @@ import { Navigate, Route, Routes } from 'react-router-dom' import { Navbar } from './components/navbar/Navbar' import { SkipLink } from './components/skipLink/SkipLink' import { GoToTop } from './components/goToTop/GoToTop' +import FocusOnNavigate from './components/FocusOnNavigate' import { ProtectedRoute } from './components/ProtectedRoute' import { AuthProvider } from './components/auth/AuthProvider' @@ -40,6 +41,7 @@ export function App() { return ( + setIsDarkMode((current) => !current)} diff --git a/src/components/FocusOnNavigate.tsx b/src/components/FocusOnNavigate.tsx new file mode 100644 index 0000000..d631edd --- /dev/null +++ b/src/components/FocusOnNavigate.tsx @@ -0,0 +1,23 @@ +import { useEffect, useRef } from 'react' +import { useLocation } from 'react-router-dom' + +/** + * After the visitor goes to another page, puts the focus at the start of that page, as a full + * page load would. Without it the focus stays on the link that was used (which may be gone), or + * falls to the ``, and a keyboard or screen reader user starts again from the top of the + * menu. Only a change of the page counts: a change of the search, the filters or the sort is + * not a new page. Nothing is moved on the first load. + */ +export default function FocusOnNavigate({ targetId }: { targetId: string }) { + const { pathname } = useLocation() + const previous = useRef(null) + + useEffect(() => { + const was = previous.current + previous.current = pathname + if (was === null || was === pathname) return + document.getElementById(targetId)?.focus() + }, [pathname, targetId]) + + return null +} diff --git a/src/tests/components/FocusOnNavigate.test.tsx b/src/tests/components/FocusOnNavigate.test.tsx new file mode 100644 index 0000000..5d6cc16 --- /dev/null +++ b/src/tests/components/FocusOnNavigate.test.tsx @@ -0,0 +1,51 @@ +// @vitest-environment jsdom + +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { MemoryRouter, useNavigate } from 'react-router-dom' +import { afterEach, describe, expect, it } from 'vitest' +import FocusOnNavigate from '@/components/FocusOnNavigate' + +afterEach(cleanup) + +function Page() { + const navigate = useNavigate() + return ( + <> + + + +
+ + ) +} + +describe('FocusOnNavigate', () => { + it('moves the focus to the start of the page after a change of page, and not before', () => { + render( + + + , + ) + expect(document.body).toBe(document.activeElement) + + fireEvent.click(screen.getByRole('button', { name: 'to another page' })) + + expect(document.getElementById('main-content')).toBe(document.activeElement) + }) + + it('leaves the focus alone when only the search changes', () => { + render( + + + , + ) + const button = screen.getByRole('button', { name: 'change the search' }) + button.focus() + + fireEvent.click(button) + + expect(document.activeElement).toBe(button) + }) +})