diff --git a/components/site/Chrome.tsx b/components/site/Chrome.tsx index 9be3b17..3358c05 100644 --- a/components/site/Chrome.tsx +++ b/components/site/Chrome.tsx @@ -6,8 +6,11 @@ import { usePathname } from "next/navigation"; import { links } from "@/lib/links"; import { HOME_SECTION_ID, + focusSection, homeNavTarget, + queueSectionFocus, shouldInterceptNavClick, + takeSectionFocus, } from "@/lib/scroll"; import { MobileMenu } from "@/components/site/MobileMenu"; import { resolveActiveSection } from "@/components/site/chrome-active"; @@ -56,8 +59,10 @@ export function Chrome() { // hash (`/#method#method…`, #116) under output: "export" + // trailingSlash; Next 16.3.8 fixed that upstream. Off this route the // section isn't in the DOM (e.g. /blog/*), so the native - // soft-navs home and scrolls to the hash itself. Modified clicks (new - // tab, middle-click) also fall through to the native . + // soft-navs home and scrolls to the hash itself; the id is queued so + // the arrival effect below moves focus into the section once home has + // committed. Modified clicks (new tab, middle-click) also fall through + // to the native . const handleNavClick = useCallback( (e: React.MouseEvent) => { if (!shouldInterceptNavClick(e)) return; @@ -89,8 +94,11 @@ export function Chrome() { const { id } = target; const el = document.getElementById(id); // Off this route: fall through to the native , as the top - // branch does. - if (el === null) return; + // branch does, and focus the section on arrival. + if (el === null) { + queueSectionFocus(id); + return; + } e.preventDefault(); // Write the exact single hash so it can never accumulate; // replaceState keeps a same-hash click a URL no-op. @@ -101,15 +109,9 @@ export function Chrome() { // desktop, so the drawer's custom rAF isn't needed. el.scrollIntoView({ block: "start" }); // Unlike a real anchor nav, the intercepted scroll doesn't move - // keyboard / SR focus — without this, tab order and the reading - // cursor stay stuck on the nav. Add tabindex only when the target - // isn't already focusable (don't pull a focusable el out of tab - // order); leaving the injected tabindex="-1" in place is the - // established programmatic-focus pattern (focusable via script, - // not via Tab). preventScroll so .focus() doesn't run its own - // scroll and race the scrollIntoView above. - if (el.tabIndex < 0) el.setAttribute("tabindex", "-1"); - el.focus({ preventScroll: true }); + // keyboard / SR focus; focusSection does, without racing the + // scrollIntoView above. + focusSection(el); }, [], ); @@ -198,6 +200,19 @@ export function Chrome() { return () => window.removeEventListener("scroll", onScroll); }, [pathname]); + // Cross-route arrival: a section tap from another route (this nav or + // the drawer) queued its id before soft-navving to `/#id`. Router + // navigations commit the new page together with the new pathname, and + // this passive effect runs after Next's layout-phase hash scroll, so + // the section is in the DOM and already scrolled into place. Same-page + // taps focus directly and never queue. + useEffect(() => { + const id = takeSectionFocus(window.location.hash); + if (id === null) return; + const el = document.getElementById(id); + if (el) focusSection(el); + }, [pathname]); + const effectiveActive = resolveActiveSection( pathname, scrolled, diff --git a/components/site/MobileMenu.tsx b/components/site/MobileMenu.tsx index 416d3ae..c6bb0cd 100644 --- a/components/site/MobileMenu.tsx +++ b/components/site/MobileMenu.tsx @@ -10,7 +10,12 @@ import { import { createPortal } from "react-dom"; import { useRouter } from "next/navigation"; import { links } from "@/lib/links"; -import { HOME_SECTION_ID, scrollToAnchor } from "@/lib/scroll"; +import { + HOME_SECTION_ID, + focusSection, + queueSectionFocus, + scrollToAnchor, +} from "@/lib/scroll"; type SectionId = "intro" | "compare" | "method" | "faq" | "contact"; @@ -81,11 +86,13 @@ export function MobileMenu({ activeSection, tone, onOpenChange }: Props) { // the prior scroll position — otherwise the body unlock would override // the anchor jump. const pendingAnchorRef = useRef(null); - // An in-page anchor-select close (target in this DOM) moves focus into - // the target section from the scroll-lock effect's cleanup, which runs - // *before* the toggle-refocus effect's setup in the same commit. This - // flag — set there, consumed there — stops that effect from yanking - // focus back to the hamburger. + // A section-select close moves focus into the target section — at once + // from the scroll-lock effect's cleanup when the target is in this DOM, + // or after the route commits (Chrome's arrival effect) when it isn't. + // That cleanup runs *before* the toggle-refocus effect's setup in the + // same commit. This flag — set in that cleanup, consumed by the + // toggle-refocus effect — stops it from yanking focus back to the + // hamburger. const focusMovedToSectionRef = useRef(false); useEffect(() => { @@ -180,8 +187,16 @@ export function MobileMenu({ activeSection, tone, onOpenChange }: Props) { // no-op against a still-pinned body. Historically a full reload, // because soft-nav hash handling under output: "export" was // untrusted; its one concrete failure, `/#id#id` (#116), was fixed - // upstream in Next 16.3.8. Skip the local restore and focus move — - // the router owns scroll and the URL. + // upstream in Next 16.3.8. Skip the local restore — the router + // owns scroll and the URL. A section can't be focused until the + // route commits, so queue it for Chrome's arrival effect and set + // the flag so the toggle-refocus effect doesn't grab focus + // meanwhile; a home tap leaves the flag false, so focus returns + // to the toggle. + if (!isHome) { + queueSectionFocus(anchor); + focusMovedToSectionRef.current = true; + } router.push(isHome ? "/" : `/#${anchor}`); return; } @@ -215,13 +230,9 @@ export function MobileMenu({ activeSection, tone, onOpenChange }: Props) { history.replaceState(null, "", `#${anchor}`); scrollToAnchor(targetEl); // Move keyboard / SR focus into the section — scrollToAnchor - // only moves the viewport. Add tabindex only when the target - // isn't already focusable (don't pull a focusable el out of - // tab order); leaving the injected tabindex="-1" in place is - // the established programmatic-focus pattern. preventScroll so - // .focus() doesn't race the in-flight rAF scroll. - if (targetEl.tabIndex < 0) targetEl.setAttribute("tabindex", "-1"); - targetEl.focus({ preventScroll: true }); + // only moves the viewport. focusSection won't race the + // in-flight rAF scroll. + focusSection(targetEl); focusMovedToSectionRef.current = true; } }; @@ -232,10 +243,10 @@ export function MobileMenu({ activeSection, tone, onOpenChange }: Props) { const wasOpenRef = useRef(false); useEffect(() => { if (wasOpenRef.current && !open) { - // An anchor-select close already moved focus into the section; - // leave it there. Normal closes (Escape, scrim, toggle, resize) and - // an off-route anchor close (soft-navs home) leave the flag false - // and still return focus to the toggle. + // A section-select close already moved focus into the section, or + // will once the route lands on its hash; leave it alone. Normal closes + // (Escape, scrim, toggle, resize) and a home-select close leave the + // flag false and still return focus to the toggle. if (focusMovedToSectionRef.current) { focusMovedToSectionRef.current = false; } else { diff --git a/lib/scroll.test.ts b/lib/scroll.test.ts index e588e27..61c1b39 100644 --- a/lib/scroll.test.ts +++ b/lib/scroll.test.ts @@ -1,9 +1,12 @@ -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { HOME_SECTION_ID, + focusSection, homeNavTarget, parseScrollMarginTop, + queueSectionFocus, shouldInterceptNavClick, + takeSectionFocus, } from "./scroll"; describe("homeNavTarget", () => { @@ -134,3 +137,89 @@ describe("shouldInterceptNavClick", () => { ); }); }); + +describe("focusSection", () => { + const fakeEl = (tabIndex: number) => ({ + tabIndex, + setAttribute: vi.fn(), + focus: vi.fn(), + }); + + it("makes a non-focusable section programmatically focusable", () => { + const el = fakeEl(-1); + focusSection(el); + expect(el.setAttribute).toHaveBeenCalledWith("tabindex", "-1"); + }); + + it("adds the tabindex before focusing (focus is a no-op without it)", () => { + const el = fakeEl(-1); + focusSection(el); + expect(el.setAttribute.mock.invocationCallOrder[0]).toBeLessThan( + el.focus.mock.invocationCallOrder[0]!, + ); + }); + + it("leaves an already-focusable element in the tab order", () => { + const el = fakeEl(0); + focusSection(el); + expect(el.setAttribute).not.toHaveBeenCalled(); + }); + + it("focuses without scrolling, so it can't race the nav's scroll", () => { + for (const el of [fakeEl(-1), fakeEl(0)]) { + focusSection(el); + expect(el.focus).toHaveBeenCalledWith({ preventScroll: true }); + } + }); +}); + +describe("queueSectionFocus / takeSectionFocus", () => { + // The queue is module state: drain it so no case depends on another + // having taken what it queued. + beforeEach(() => { + takeSectionFocus(""); + }); + + it("returns nothing when no section is queued", () => { + expect(takeSectionFocus("#method")).toBeNull(); + }); + + it("returns the queued id when the URL landed on its hash", () => { + queueSectionFocus("method"); + expect(takeSectionFocus("#method")).toBe("method"); + }); + + it("is single-use", () => { + queueSectionFocus("method"); + takeSectionFocus("#method"); + expect(takeSectionFocus("#method")).toBeNull(); + }); + + it("ignores a stale id on an arrival with a different hash", () => { + // e.g. an aborted section nav followed by a home nav (no hash). + queueSectionFocus("method"); + expect(takeSectionFocus("")).toBeNull(); + queueSectionFocus("method"); + expect(takeSectionFocus("#faq")).toBeNull(); + }); + + it("keeps only the latest queued section", () => { + queueSectionFocus("method"); + queueSectionFocus("faq"); + expect(takeSectionFocus("#faq")).toBe("faq"); + queueSectionFocus("method"); + queueSectionFocus("faq"); + expect(takeSectionFocus("#method")).toBeNull(); + }); + + it("matches the hash exactly", () => { + queueSectionFocus("method"); + expect(takeSectionFocus("#methodology")).toBeNull(); + }); + + it("clears the queue even when the hash doesn't match", () => { + queueSectionFocus("method"); + takeSectionFocus(""); + expect(takeSectionFocus("#method")).toBeNull(); + }); +}); diff --git a/lib/scroll.ts b/lib/scroll.ts index e7c5c66..867af26 100644 --- a/lib/scroll.ts +++ b/lib/scroll.ts @@ -55,6 +55,45 @@ export function shouldInterceptNavClick(e: { ); } +// Moves keyboard / SR focus into a section after a nav tap — scrolling +// alone only moves the viewport, so tab order and the reading cursor +// would stay on the nav. Add tabindex only when the target isn't already +// focusable (don't pull a focusable el out of tab order); leaving the +// injected tabindex="-1" in place is the established programmatic-focus +// pattern (focusable via script, not via Tab). preventScroll so .focus() +// doesn't run its own scroll and race whichever scroll got us here. +// Typed structurally so it's unit-testable without a DOM. +export function focusSection( + el: Pick, +) { + if (el.tabIndex < 0) el.setAttribute("tabindex", "-1"); + el.focus({ preventScroll: true }); +} + +// A section tap from another route (e.g. /blog/*) soft-navs to `/#id`, +// so the target isn't in the DOM yet when the nav is tapped. The tap +// queues the id here; Chrome — mounted in the root layout, so it +// survives the route change — takes it in a pathname effect once the +// home page has committed and focuses the section with focusSection. +// Next's hash scroll doesn't move focus — 16.3.8's default +// appNewScrollHandler leaves it untouched — so without this, focus stays +// on the tapped link (or the drawer). +let pendingSectionFocus: string | null = null; + +export function queueSectionFocus(id: string) { + pendingSectionFocus = id; +} + +// Single-use: always clears the queue. Returns the id only when the URL +// actually landed on that hash, so a queued id that outlives an aborted +// nav can't focus a section on a later, unrelated arrival (e.g. home, +// which lands on `/` with no hash). +export function takeSectionFocus(hash: string): string | null { + const id = pendingSectionFocus; + pendingSectionFocus = null; + return id !== null && hash === `#${id}` ? id : null; +} + // `` scroll-behavior is forced to `auto` for the lifetime of // the rAF: without it, each per-frame `window.scrollTo` defers to // `motion-safe:scroll-smooth` and queues yet another browser