fix(nav): Back restores the focus after a click on the page's blank area
The page region is <main tabIndex=-1>, so a background click focused it and the Back restore took that for the reader's choice. A resting place is marked and one predicate (focusIsUnclaimed) decides "the focus is nowhere" everywhere.
This commit is contained in:
@@ -4,6 +4,7 @@ import { useTranslation } from "react-i18next";
|
|||||||
|
|
||||||
import { api, type DiagnosticsLevel } from "../lib/api";
|
import { api, type DiagnosticsLevel } from "../lib/api";
|
||||||
import { levelInForce } from "../lib/diagnostics";
|
import { levelInForce } from "../lib/diagnostics";
|
||||||
|
import { focusIsUnclaimed } from "../lib/useFocusKeeper";
|
||||||
import { updateMeCache, useMe } from "../lib/useMe";
|
import { updateMeCache, useMe } from "../lib/useMe";
|
||||||
import { Button } from "./ui/Button";
|
import { Button } from "./ui/Button";
|
||||||
import InlineNotice from "./ui/InlineNotice";
|
import InlineNotice from "./ui/InlineNotice";
|
||||||
@@ -61,7 +62,7 @@ export default function DiagnosticsSettings() {
|
|||||||
const from = closedFrom.current;
|
const from = closedFrom.current;
|
||||||
if (!from || from.isConnected) return;
|
if (!from || from.isConnected) return;
|
||||||
closedFrom.current = null;
|
closedFrom.current = null;
|
||||||
if (document.activeElement && document.activeElement !== document.body) return;
|
if (!focusIsUnclaimed()) return;
|
||||||
pickerRef.current?.querySelector<HTMLElement>('[aria-pressed="true"]')?.focus();
|
pickerRef.current?.querySelector<HTMLElement>('[aria-pressed="true"]')?.focus();
|
||||||
});
|
});
|
||||||
const watchFocus = () => {
|
const watchFocus = () => {
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import {
|
|||||||
import { moduleDef } from "../lib/modules";
|
import { moduleDef } from "../lib/modules";
|
||||||
import type { Page } from "../lib/urlState";
|
import type { Page } from "../lib/urlState";
|
||||||
import { qk } from "../lib/queryKeys";
|
import { qk } from "../lib/queryKeys";
|
||||||
|
import { focusIsUnclaimed } from "../lib/useFocusKeeper";
|
||||||
import { LEVEL_TAG, logClock } from "./logLineParts";
|
import { LEVEL_TAG, logClock } from "./logLineParts";
|
||||||
import { Button } from "./ui/Button";
|
import { Button } from "./ui/Button";
|
||||||
import { IconButton } from "./ui/IconButton";
|
import { IconButton } from "./ui/IconButton";
|
||||||
@@ -54,7 +55,7 @@ export default function IssueComposer({ id }: { id: string }) {
|
|||||||
useEffect(
|
useEffect(
|
||||||
() => () => {
|
() => () => {
|
||||||
window.setTimeout(() => {
|
window.setTimeout(() => {
|
||||||
if (document.activeElement === document.body)
|
if (focusIsUnclaimed())
|
||||||
document.querySelector<HTMLElement>('nav [aria-current="page"]')?.focus();
|
document.querySelector<HTMLElement>('nav [aria-current="page"]')?.focus();
|
||||||
}, 0);
|
}, 0);
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { useCallback, useLayoutEffect, useRef, useState, type ReactNode } from "react";
|
import { useCallback, useLayoutEffect, useRef, useState, type ReactNode } from "react";
|
||||||
import { useScrollFade } from "../../lib/useScrollFade";
|
import { useScrollFade } from "../../lib/useScrollFade";
|
||||||
import { useFocusKeeper } from "../../lib/useFocusKeeper";
|
import { FOCUS_REST_ATTRS, useFocusKeeper } from "../../lib/useFocusKeeper";
|
||||||
import { StickyTopProvider } from "./stickyTop";
|
import { StickyTopProvider } from "./stickyTop";
|
||||||
import { CONTENT_ID } from "./landmarks";
|
import { CONTENT_ID } from "./landmarks";
|
||||||
import { ensureNavId, peekNavId, subscribeNav } from "../../lib/history";
|
import { ensureNavId, peekNavId, subscribeNav } from "../../lib/history";
|
||||||
@@ -158,11 +158,13 @@ export default function PageScroller({
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
// `id` + `tabIndex={-1}`: the skip link's target (ui/SkipLink) takes the focus, so the next Tab
|
// `id` + `tabIndex={-1}`: the skip link's target (ui/SkipLink) takes the focus, so the next Tab
|
||||||
// lands on the page's first control.
|
// lands on the page's first control. A click on the page's background focuses it too, which is
|
||||||
|
// the focus RESTING, not a choice a Back restore must respect (`FOCUS_REST_ATTRS`).
|
||||||
<main
|
<main
|
||||||
ref={setRef}
|
ref={setRef}
|
||||||
id={CONTENT_ID}
|
id={CONTENT_ID}
|
||||||
tabIndex={-1}
|
tabIndex={-1}
|
||||||
|
{...FOCUS_REST_ATTRS}
|
||||||
style={fade.style}
|
style={fade.style}
|
||||||
className="flex-1 min-w-0 overflow-y-auto scrollbar-gutter-stable focus:outline-none"
|
className="flex-1 min-w-0 overflow-y-auto scrollbar-gutter-stable focus:outline-none"
|
||||||
>
|
>
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import { afterEach, describe, expect, it } from "vitest";
|
|||||||
import { act, cleanup, render, waitFor } from "@testing-library/react";
|
import { act, cleanup, render, waitFor } from "@testing-library/react";
|
||||||
import { rememberNavPosition } from "./history";
|
import { rememberNavPosition } from "./history";
|
||||||
import { navItemAttrs, useListNav } from "./listNav";
|
import { navItemAttrs, useListNav } from "./listNav";
|
||||||
|
import { FOCUS_REST_ATTRS } from "./useFocusKeeper";
|
||||||
|
|
||||||
function List({ id, count, label }: { id: string; count: number; label?: string }) {
|
function List({ id, count, label }: { id: string; count: number; label?: string }) {
|
||||||
const ref = useRef<HTMLDivElement>(null);
|
const ref = useRef<HTMLDivElement>(null);
|
||||||
@@ -146,6 +147,22 @@ describe("rememberNavPosition · Back focuses the item a view was left from", ()
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// E28 review H1: the page region is `<main tabIndex={-1}>` (the skip link's target), so a mouse click
|
||||||
|
// on the page's blank area focuses it. The restore took that for a control the reader had chosen and
|
||||||
|
// gave up: Back left the focus on the page instead of on the item the view was opened from.
|
||||||
|
describe("rememberNavPosition · the focus resting on the page region", () => {
|
||||||
|
it("still restores when the page region holds the focus", async () => {
|
||||||
|
const { unmount } = render(<Page />);
|
||||||
|
const stamped = leaveFrom("b1", 4);
|
||||||
|
unmount();
|
||||||
|
const region = render(<main tabIndex={-1} data-testid="main" {...FOCUS_REST_ATTRS} />);
|
||||||
|
region.getByTestId("main").focus();
|
||||||
|
landOn(stamped);
|
||||||
|
render(<Page />);
|
||||||
|
await waitFor(() => expect(focused()).toBe("b1"));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("rememberNavPosition · the list it comes back to (deep-review E11 round 2)", () => {
|
describe("rememberNavPosition · the list it comes back to (deep-review E11 round 2)", () => {
|
||||||
// Discover: a title opened from "Recently added" and watched — the page's cache expires and
|
// Discover: a title opened from "Recently added" and watched — the page's cache expires and
|
||||||
// "Continue watching" fills with that very title, a rail ABOVE. Found by its place, focus landed in
|
// "Continue watching" fills with that very title, a rail ABOVE. Found by its place, focus landed in
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { useEffect, useRef, useState, type RefObject } from "react";
|
import { useEffect, useRef, useState, type RefObject } from "react";
|
||||||
import { dispatch, useKeyScope } from "./keymap";
|
import { dispatch, useKeyScope } from "./keymap";
|
||||||
import { scrollParent } from "./listPaging";
|
import { scrollParent } from "./listPaging";
|
||||||
import { isStandInFocus } from "./useFocusKeeper";
|
import { focusIsUnclaimed } from "./useFocusKeeper";
|
||||||
|
|
||||||
// ── Keyboard navigation over lists and grids (E11 S4, UX-6) ──────────────────────────────────────
|
// ── Keyboard navigation over lists and grids (E11 S4, UX-6) ──────────────────────────────────────
|
||||||
// Before this, a feed card cost ~10 Tab stops (thumb, play, info, title, channel, five actions), so
|
// Before this, a feed card cost ~10 Tab stops (thumb, play, info, title, channel, five actions), so
|
||||||
@@ -148,6 +148,9 @@ function onFocusIn(e: FocusEvent): void {
|
|||||||
const list =
|
const list =
|
||||||
item && lists.get(item.parentElement?.closest<HTMLElement>(`[${LIST}]`) as HTMLElement);
|
item && lists.get(item.parentElement?.closest<HTMLElement>(`[${LIST}]`) as HTMLElement);
|
||||||
if (item && list) last = { list, index: indexOf(item), el: item };
|
if (item && list) last = { list, index: indexOf(item), el: item };
|
||||||
|
// The focus coming to REST on the page region (a click on its background) is not leaving the list
|
||||||
|
// for other content — like a click on <body>, which fires no focusin at all.
|
||||||
|
else if (focusIsUnclaimed(target)) return;
|
||||||
// Focus went on to page content that is not an item — the search box, a filter chip: the reader
|
// Focus went on to page content that is not an item — the search box, a filter chip: the reader
|
||||||
// left the list, and whatever it shows next (new results) is not "the slot they were in" (deep-review
|
// left the list, and whatever it shows next (new results) is not "the slot they were in" (deep-review
|
||||||
// E11 round 2). A layer opened FROM the item — its menu, a confirm, the Undo toast its removal put
|
// E11 round 2). A layer opened FROM the item — its menu, a confirm, the Undo toast its removal put
|
||||||
@@ -375,8 +378,7 @@ function refocusSlot(list: NavList, index: number, removed: HTMLElement): void {
|
|||||||
if (++frames < REMOVE_FRAMES) requestAnimationFrame(wait);
|
if (++frames < REMOVE_FRAMES) requestAnimationFrame(wait);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const active = document.activeElement;
|
if (!focusIsUnclaimed()) return;
|
||||||
if (active && active !== document.body && !isStandInFocus(active)) return;
|
|
||||||
const n = list.count();
|
const n = list.count();
|
||||||
if (n > 0) focusIndex(list, Math.min(index, n - 1));
|
if (n > 0) focusIndex(list, Math.min(index, n - 1));
|
||||||
};
|
};
|
||||||
@@ -450,8 +452,7 @@ export function restoreNavPosition(pos: NavPosition): () => void {
|
|||||||
const deadline = Date.now() + RESTORE_MS;
|
const deadline = Date.now() + RESTORE_MS;
|
||||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||||
const attempt = () => {
|
const attempt = () => {
|
||||||
const active = document.activeElement;
|
if (!focusIsUnclaimed()) return;
|
||||||
if (active && active !== document.body && !isStandInFocus(active)) return;
|
|
||||||
const all = ordered();
|
const all = ordered();
|
||||||
const list = pos.label
|
const list = pos.label
|
||||||
? all.find((l) => labelOf(l) === pos.label)
|
? all.find((l) => labelOf(l) === pos.label)
|
||||||
|
|||||||
@@ -41,6 +41,19 @@ export function isStandInFocus(el: Element | null): boolean {
|
|||||||
return el !== null && el === standIn;
|
return el !== null && el === standIn;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Marks an element that can hold the focus without being a control: the page's scroll region
|
||||||
|
* (`<main tabIndex={-1}>`, the skip link's target) takes it from a mouse click on the page's blank
|
||||||
|
* area. Holding it there is the focus RESTING on the page, the same as <body>. */
|
||||||
|
export const FOCUS_REST_ATTRS = { "data-focus-rest": "" } as const;
|
||||||
|
|
||||||
|
/** Whether the focus is nowhere the reader chose — <body>, the keeper's stand-in, or a resting place
|
||||||
|
* (`FOCUS_REST_ATTRS`) — so a restore may take it. The page region used to count as a choice: a
|
||||||
|
* click on its background focused it, and Back then left the focus there instead of on the item
|
||||||
|
* the view was opened from (E28 review H1). */
|
||||||
|
export function focusIsUnclaimed(el: Element | null = document.activeElement): boolean {
|
||||||
|
return !el || el === document.body || isStandInFocus(el) || el.hasAttribute("data-focus-rest");
|
||||||
|
}
|
||||||
|
|
||||||
/** Where the focus goes once `lost` (removed with the nodes of `record`) has left the page — never
|
/** Where the focus goes once `lost` (removed with the nodes of `record`) has left the page — never
|
||||||
* outside `root`: a slot with nothing focusable left (a page swapped for its loading fallback) must
|
* outside `root`: a slot with nothing focusable left (a page swapped for its loading fallback) must
|
||||||
* not send the focus to the first control of the whole document. */
|
* not send the focus to the first control of the whole document. */
|
||||||
@@ -80,8 +93,7 @@ export function useFocusKeeper(ref: RefObject<HTMLElement | null>): void {
|
|||||||
const lost = held;
|
const lost = held;
|
||||||
if (!lost || lost.isConnected) return;
|
if (!lost || lost.isConnected) return;
|
||||||
held = null;
|
held = null;
|
||||||
const active = document.activeElement;
|
if (!focusIsUnclaimed()) return; // something already took the focus
|
||||||
if (active && active !== document.body) return; // something already took the focus
|
|
||||||
for (const r of records) {
|
for (const r of records) {
|
||||||
const target = slotFor(r, lost, root);
|
const target = slotFor(r, lost, root);
|
||||||
if (target) {
|
if (target) {
|
||||||
|
|||||||
Reference in New Issue
Block a user