fix(ui): Back finds the item again past the focus keeper; nested drags stay in their list
Deep review of the debt burn, each measured in Chrome: - Back from a sub-view whose focused control the swap removed: the focus keeper put the focus on a stand-in (the returning list's first item) and `restoreNavPosition`, finding the focus "already somewhere", gave up — Plex show → Back landed on tile 0, not the tile it was opened from. A stand-in the keeper placed now counts as untouched, like <body>. The keeper's walk up also stops at its container: a slot left with nothing focusable no longer sends the focus to the first control of the whole document. - DataTable enabled both layouts' keyboard lists with the same flag, and `useListNav` reads its ref only when the flag changes: after a resize across `md`, the layout that mounted was never registered (no j/k) and the one that left stayed registered. Each is enabled with its layout. - A Plex playlist nests blocks and each show's episodes in one DndContext; closestCenter and the keyboard coordinates weighed every droppable, so a movie landed "over" an episode — announced as a place in the other list, dropped somewhere else or nowhere. `sameListCollision` and `sameListKeyboardCoordinates` keep a drag among its siblings; onDragEnd's episode → block resolution is gone with the case. - dnd-kit reports the item over itself in the pick-up's own tick; announced, "moved to position 1" replaced "picked up … use the arrow keys" before a screen reader read it.
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { useEffect, useRef, useState, type RefObject } from "react";
|
||||
import { dispatch, useKeyScope } from "./keymap";
|
||||
import { scrollParent } from "./listPaging";
|
||||
import { isStandInFocus } from "./useFocusKeeper";
|
||||
|
||||
// ── 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
|
||||
@@ -378,13 +379,14 @@ function restoreTarget(list: NavList, pos: NavPosition): HTMLElement | null {
|
||||
* place, and only once the page has exactly as many lists as when it was left: with more or fewer,
|
||||
* list N may be another list, and <body> is the honest outcome. Gives up when the time is up, and at
|
||||
* once when focus is already somewhere (the reader moved on, a dialog took it). The item is focused
|
||||
* where the scroll restore put it, never scrolled to. Returns a cancel. */
|
||||
* where the scroll restore put it, never scrolled to. (A stand-in the focus keeper placed when the view
|
||||
* it held was swapped away counts as untouched, like <body>.) Returns a cancel. */
|
||||
export function restoreNavPosition(pos: NavPosition): () => void {
|
||||
const deadline = Date.now() + RESTORE_MS;
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const attempt = () => {
|
||||
const active = document.activeElement;
|
||||
if (active && active !== document.body) return;
|
||||
if (active && active !== document.body && !isStandInFocus(active)) return;
|
||||
const all = ordered();
|
||||
const list = pos.label
|
||||
? all.find((l) => labelOf(l) === pos.label)
|
||||
|
||||
@@ -46,6 +46,29 @@ describe("useFocusKeeper", () => {
|
||||
expect(document.activeElement).toBe(btn("after the list"));
|
||||
});
|
||||
|
||||
// A page swapped for its loading fallback leaves nothing focusable in the slot. The walk up used to
|
||||
// run past the container and hand the focus to the first control of the whole document.
|
||||
it("never puts the focus outside its own container", async () => {
|
||||
function Swapped() {
|
||||
const ref = useRef<HTMLDivElement>(null);
|
||||
useFocusKeeper(ref);
|
||||
const [loaded, setLoaded] = useState(true);
|
||||
return (
|
||||
<>
|
||||
<button>rail</button>
|
||||
<div ref={ref}>
|
||||
{loaded ? <button onClick={() => setLoaded(false)}>go</button> : <p>Loading…</p>}
|
||||
</div>
|
||||
</>
|
||||
);
|
||||
}
|
||||
render(<Swapped />);
|
||||
btn("go").focus();
|
||||
act(() => btn("go").click());
|
||||
await flush();
|
||||
expect(document.activeElement).toBe(document.body);
|
||||
});
|
||||
|
||||
it("leaves a keyboard list's rows to listNav", async () => {
|
||||
render(<Rows nav />);
|
||||
btn("approve b").focus();
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// Back from a sub-view the focus keeper ALSO sees: the control that held the focus in the sub-view
|
||||
// is removed by the swap, the keeper puts the focus on a stand-in in its slot — the first item of
|
||||
// the returning list — and `restoreNavPosition` then found the focus "already somewhere" and gave
|
||||
// up. Back landed on the first tile instead of the one the view was opened from (measured in Chrome,
|
||||
// deep review of the debt burn). Its own file: listNav keeps module-level state.
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { act, cleanup, render } from "@testing-library/react";
|
||||
import { useRef, useState } from "react";
|
||||
|
||||
import { navItemAttrs, navPosition, restoreNavPosition, useListNav } from "./listNav";
|
||||
import { useFocusKeeper } from "./useFocusKeeper";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
function Grid() {
|
||||
const ref = useRef<HTMLDivElement>(null);
|
||||
useListNav(ref, { enabled: true });
|
||||
return (
|
||||
<div ref={ref}>
|
||||
{["a", "b", "c"].map((t, i) => (
|
||||
<div key={t} {...navItemAttrs(i, t)}>
|
||||
<button data-nav-open="">{t}</button>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
let show: (v: "grid" | "detail") => void = () => {};
|
||||
|
||||
function Page() {
|
||||
const ref = useRef<HTMLElement>(null);
|
||||
useFocusKeeper(ref);
|
||||
const [view, setView] = useState<"grid" | "detail">("grid");
|
||||
show = setView;
|
||||
return <main ref={ref}>{view === "grid" ? <Grid /> : <button>Back to library</button>}</main>;
|
||||
}
|
||||
|
||||
const button = (name: string) =>
|
||||
[...document.querySelectorAll("button")].find((b) => b.textContent === name)!;
|
||||
const wait = (ms: number) => act(() => new Promise((r) => setTimeout(r, ms)));
|
||||
|
||||
describe("Back from a sub-view whose held control the swap removed", () => {
|
||||
it("returns the focus to the item the view was opened from, not the keeper's stand-in", async () => {
|
||||
render(<Page />);
|
||||
button("c").focus();
|
||||
const pos = navPosition()!;
|
||||
expect(pos.index).toBe(2);
|
||||
|
||||
act(() => show("detail"));
|
||||
button("Back to library").focus(); // a control the keeper holds
|
||||
|
||||
act(() => show("grid")); // popstate's view swap…
|
||||
const cancel = restoreNavPosition(pos); // …and its restore
|
||||
await wait(120);
|
||||
cancel();
|
||||
|
||||
expect(
|
||||
document.activeElement?.closest("[data-nav-index]")?.getAttribute("data-nav-index"),
|
||||
).toBe("2");
|
||||
});
|
||||
});
|
||||
@@ -29,8 +29,22 @@ function firstFocusable(node: Node | null, fromEnd = false): HTMLElement | null
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Where the focus goes once `lost` (removed with the nodes of `record`) has left the page. */
|
||||
function slotFor(record: MutationRecord, lost: Node): HTMLElement | null {
|
||||
// The element the keeper last put the focus on, while it still holds it. A Back that swaps the page
|
||||
// or a sub-view removes the held control too, and the keeper's stand-in then sat where
|
||||
// `restoreNavPosition` looks for an untouched <body>: the restore gave up and Back left the focus on
|
||||
// the first tile instead of the one the view was opened from (measured in Chrome). A stand-in is
|
||||
// not the reader's choice, so the restore may take the focus from it.
|
||||
let standIn: HTMLElement | null = null;
|
||||
|
||||
/** Whether `el` holds the focus only because the keeper put it there. */
|
||||
export function isStandInFocus(el: Element | null): boolean {
|
||||
return el !== null && el === standIn;
|
||||
}
|
||||
|
||||
/** 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
|
||||
* not send the focus to the first control of the whole document. */
|
||||
function slotFor(record: MutationRecord, lost: Node, root: HTMLElement): HTMLElement | null {
|
||||
const removed = [...record.removedNodes].find((n) => n === lost || n.contains(lost));
|
||||
if (!removed) return null;
|
||||
const after = firstFocusable(record.nextSibling);
|
||||
@@ -38,7 +52,7 @@ function slotFor(record: MutationRecord, lost: Node): HTMLElement | null {
|
||||
const before = firstFocusable(record.previousSibling, true);
|
||||
if (before) return before;
|
||||
let up: Node | null = record.target;
|
||||
while (up && up instanceof HTMLElement) {
|
||||
while (up && up instanceof HTMLElement && root.contains(up)) {
|
||||
const inside = firstFocusable(up.firstChild);
|
||||
if (inside) return inside;
|
||||
up = up.parentElement;
|
||||
@@ -53,6 +67,7 @@ export function useFocusKeeper(ref: RefObject<HTMLElement | null>): void {
|
||||
let held: HTMLElement | null = null;
|
||||
const onFocusIn = (e: FocusEvent) => {
|
||||
const t = e.target as HTMLElement | null;
|
||||
if (t !== standIn) standIn = null;
|
||||
held = t && root.contains(t) && !t.closest("[data-nav-index]") ? t : null;
|
||||
};
|
||||
const watch = new MutationObserver((records) => {
|
||||
@@ -62,8 +77,9 @@ export function useFocusKeeper(ref: RefObject<HTMLElement | null>): void {
|
||||
const active = document.activeElement;
|
||||
if (active && active !== document.body) return; // something already took the focus
|
||||
for (const r of records) {
|
||||
const target = slotFor(r, lost);
|
||||
const target = slotFor(r, lost, root);
|
||||
if (target) {
|
||||
standIn = target;
|
||||
target.focus({ preventScroll: true });
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user