diff --git a/js/app/components/sidebar/sidebar-item.tsx b/js/app/components/sidebar/sidebar-item.tsx index dd3044323..303512f7e 100644 --- a/js/app/components/sidebar/sidebar-item.tsx +++ b/js/app/components/sidebar/sidebar-item.tsx @@ -74,6 +74,9 @@ export default function SidebarItem({ onHoverIn={() => setHover(true)} onHoverOut={() => setHover(false)} role="link" + // The current destination. The filled pill is the only other cue, which + // assistive tech (and e2e) can't see. + aria-current={active ? "page" : undefined} accessibilityLabel={typeof label === "string" ? label : "Link to " + href} // @ts-ignore This makes it render as on web! href={href} diff --git a/js/e2e-web/flows/about-link.spec.ts b/js/e2e-web/flows/about-link.spec.ts index 57a686b42..e7aa82e9a 100644 --- a/js/e2e-web/flows/about-link.spec.ts +++ b/js/e2e-web/flows/about-link.spec.ts @@ -1,4 +1,4 @@ -import { expect, test } from "@playwright/test"; +import { expect, test, type Page } from "@playwright/test"; import { loginThroughPds } from "./login"; // The About entry is a top-level sidebar destination while signed out — the @@ -6,6 +6,9 @@ import { loginThroughPds } from "./login"; // folds back into Settings > About once you are signed in. const HTTPS_URL = process.env.SERVER_HTTPS_URL; +const sidebarLink = (page: Page, name: string) => + page.getByRole("link", { name, exact: true }).first(); + test("about-link: signed out, the sidebar links straight to the About page", async ({ page, }) => { @@ -14,9 +17,32 @@ test("about-link: signed out, the sidebar links straight to the About page", asy timeout: 30_000, }); - await page.getByRole("link", { name: "About", exact: true }).first().click(); + await sidebarLink(page, "About").click(); await expect(page).toHaveURL(/\/settings\/about$/); await expect(page.getByText(/^Streamplace v/).first()).toBeVisible(); + + // Only the destination you're on lights up, though Settings matches every + // /settings/* path by prefix. + await expect(sidebarLink(page, "About")).toHaveAttribute( + "aria-current", + "page", + ); + await expect(sidebarLink(page, "Settings")).not.toHaveAttribute( + "aria-current", + "page", + ); + + // ...and that prefix still carries every other settings page. + await page.goto("/settings/advanced"); + await expect(page.getByText("Advanced").first()).toBeVisible(); + await expect(sidebarLink(page, "Settings")).toHaveAttribute( + "aria-current", + "page", + ); + await expect(sidebarLink(page, "About")).not.toHaveAttribute( + "aria-current", + "page", + ); }); test("about-link: signed in, About stays in the settings menu", async ({ @@ -31,17 +57,10 @@ test("about-link: signed in, About stays in the settings menu", async ({ }); // the sidebar drops "Log in" once the session lands, so it is showing the // signed-in list by the time the absence check runs - await expect( - page.getByRole("link", { name: "Log in", exact: true }), - ).toHaveCount(0); - await expect( - page.getByRole("link", { name: "About", exact: true }), - ).toHaveCount(0); - - await page - .getByRole("link", { name: "Settings", exact: true }) - .first() - .click(); + await expect(sidebarLink(page, "Log in")).toHaveCount(0); + await expect(sidebarLink(page, "About")).toHaveCount(0); + + await sidebarLink(page, "Settings").click(); await expect(page.getByText("Advanced").first()).toBeVisible(); await page.getByText("About", { exact: true }).first().click(); await expect(page).toHaveURL(/\/settings\/about$/);