fix(keys): epic-end review round 2 — focus comes back to the card, the right rail and the reason
Whole-epic deep-review of E11 (18 raised, 12 kept; B and G refuted, four latent test-hardening notes dropped), plus the admin refusal copy gap round 1 surfaced. - A context menu opened from a card the keyboard is on gives the focus back to the card. floating-ui returns focus through the opener's first tabbable descendant even when handed a ref, so for a non-tabbable opener the surface restores it itself (measured in Chrome: j, Shift+F10, Escape left it on the thumbnail link). - Back finds a named list (a rail) by its name: a rail that appeared above it after the cache expired sent the focus into the wrong rail. An unnamed list is matched only when the page has the same number of lists. - The slot kept for a removed item is forgotten once focus moves on to page content (search, a chip), so j after new results starts at the first visible item; a menu or dialog opened from the item keeps it. - The list keys reach the cheat sheet only while a list has an item. - A j-focused card root is a named group while it holds the focus. - The channel page's Block and Hide toggles keep the focus while their request runs. - A split export that fails part-way says how many clips were added and deselects them, so a retry adds only the rest. - Config save, the one-click share link and "log out other sessions" report failures with their reason. - VirtualGrid uses the one scroll-parent walk listNav measures against. - Admin refusals (activate, resend, invite, set-password link) say why in HU and EN instead of "try again"; the email-off special cases collapse into that one reason. The contract reads the codes from localizeDetail. HU copy points to Konfiguráció → E-mail / SMTP; HU "unreachable" matches the EN change of round 1.
This commit is contained in:
@@ -3,6 +3,7 @@ import {
|
||||
useContext,
|
||||
useEffect,
|
||||
useId,
|
||||
useLayoutEffect,
|
||||
useMemo,
|
||||
useRef,
|
||||
useState,
|
||||
@@ -600,6 +601,41 @@ function PopoverSurfaceInner({
|
||||
// in the accessible-name computation — so an explicit ariaLabel is dead unless we drop the ref.
|
||||
if (ariaLabel) delete floatingProps["aria-labelledby"];
|
||||
|
||||
// Where focus goes back on close. floating-ui returns it to the element focused before the open —
|
||||
// but when that element is not a Tab stop itself, to its first TABBABLE descendant instead, even
|
||||
// when handed the element as a ref (its return runs through `getFirstTabbableElement` either way).
|
||||
// A card the keyboard is on (lib/listNav gives its root tabindex=-1) is exactly that: measured in
|
||||
// Chrome (deep-review E11 round 2), j, Shift+F10, Escape left focus on the card's thumbnail link, the
|
||||
// ring off the card and the next Enter on a control. So for such an element the library's return is
|
||||
// off and this gives the focus back to it as it is — captured at open, before focus moves into the
|
||||
// panel, and only when the focus was still in a floating layer (or nowhere) as the panel closed:
|
||||
// focus the reader moved on to stays where they put it. A menu row's action runs a task later
|
||||
// (`closeThenAct`), so a dialog it opens still captures the card as the element to return to.
|
||||
const [returnTo] = useState(() => {
|
||||
const active = document.activeElement;
|
||||
return active instanceof HTMLElement && active !== document.body && active.tabIndex < 0
|
||||
? active
|
||||
: null;
|
||||
});
|
||||
const floatingRef = refs.floating;
|
||||
useLayoutEffect(() => {
|
||||
if (!returnTo) return;
|
||||
// Read once here: the panel's ref is attached before layout effects run, and cleared by the time
|
||||
// this cleanup does.
|
||||
const floating = floatingRef.current;
|
||||
return () => {
|
||||
const active = document.activeElement;
|
||||
const stillOurs =
|
||||
!active ||
|
||||
active === document.body ||
|
||||
!!floating?.contains(active) ||
|
||||
!!active.closest("[data-floating-ui-portal]");
|
||||
queueMicrotask(() => {
|
||||
if (stillOurs && returnTo.isConnected) returnTo.focus({ preventScroll: true });
|
||||
});
|
||||
};
|
||||
}, [returnTo, floatingRef]);
|
||||
|
||||
const zClass = mode === "hover" ? "z-tooltip" : "z-popover";
|
||||
// A menu/listbox/dialog traps focus into the surface (and returns it on close). A hover card is
|
||||
// passive, and a COMBOBOX must leave focus in the input (its list is navigated virtually) — neither
|
||||
@@ -666,7 +702,7 @@ function PopoverSurfaceInner({
|
||||
// — in an item-less menu (e.g. a first-run tag editor with only a create input) — the
|
||||
// input, so such a menu still has a keyboard entry point instead of stranding focus.
|
||||
initialFocus={initialFocus ?? (isNav && selectedIndex != null ? -1 : undefined)}
|
||||
returnFocus
|
||||
returnFocus={!returnTo}
|
||||
>
|
||||
{positioned}
|
||||
</FloatingFocusManager>
|
||||
|
||||
@@ -130,6 +130,42 @@ describe("useContextMenu / ContextMenu", () => {
|
||||
expect(menuitems()).toEqual([]);
|
||||
});
|
||||
|
||||
// A card the keyboard is on is focused as a whole, through a tabindex=-1 root (lib/listNav), and
|
||||
// holds tabbable controls. floating-ui handed the focus back to the first of them instead of the
|
||||
// card — measured in Chrome (deep-review E11 round 2): j, Shift+F10, Escape left it on the thumbnail.
|
||||
// Closed here by picking the row (the other way out; jsdom delivers no Escape to a keyboard-opened
|
||||
// menu — Escape is measured in the browser).
|
||||
it("gives the focus back to a card focused as a whole, not to a control inside it", async () => {
|
||||
function Card() {
|
||||
const menu = useContextMenu<string>();
|
||||
return (
|
||||
<>
|
||||
<div tabIndex={-1} data-testid="card" {...menu.triggerProps("card")}>
|
||||
<a href="#thumb">thumbnail</a>
|
||||
</div>
|
||||
<ContextMenu
|
||||
menu={menu}
|
||||
ariaLabel="Card actions"
|
||||
items={menu.target ? [{ id: "a", label: "Pin", onSelect: vi.fn() }] : []}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
render(
|
||||
<FloatingTree>
|
||||
<Card />
|
||||
</FloatingTree>,
|
||||
);
|
||||
const card = screen.getByTestId("card");
|
||||
act(() => card.focus());
|
||||
fireEvent.keyDown(card, { key: "F10", shiftKey: true });
|
||||
expect(menuitems()).toEqual(["Pin"]);
|
||||
await vi.waitFor(() => expect(card.contains(document.activeElement)).toBe(false));
|
||||
fireEvent.click(item("Pin"));
|
||||
expect(menuitems()).toEqual([]);
|
||||
await vi.waitFor(() => expect(document.activeElement).toBe(card));
|
||||
});
|
||||
|
||||
describe("long press (touch)", () => {
|
||||
const press = (el: HTMLElement, x = 10, y = 10) =>
|
||||
fireEvent.pointerDown(el, { pointerType: "touch", clientX: x, clientY: y });
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from "react";
|
||||
import { useVirtualizer } from "@tanstack/react-virtual";
|
||||
import { useListNav } from "../../lib/listNav";
|
||||
import { scrollParent } from "../../lib/listPaging";
|
||||
import { GRID_GAP, columnsFor, itemWidthFor } from "./gridColumns";
|
||||
import { shouldPrefetchNext } from "./readAhead";
|
||||
|
||||
@@ -33,18 +34,6 @@ function chunk<T>(arr: T[], size: number): T[][] {
|
||||
return out;
|
||||
}
|
||||
|
||||
// The list scrolls inside an <main overflow-y-auto> (the page scroller), not the window, so the
|
||||
// virtualizer needs that element. Walk up to the nearest scrollable ancestor.
|
||||
function getScrollParent(node: HTMLElement | null): HTMLElement {
|
||||
let el = node?.parentElement ?? null;
|
||||
while (el) {
|
||||
const oy = getComputedStyle(el).overflowY;
|
||||
if (oy === "auto" || oy === "scroll") return el;
|
||||
el = el.parentElement;
|
||||
}
|
||||
return document.documentElement;
|
||||
}
|
||||
|
||||
export interface VirtualGridProps<T> {
|
||||
items: T[];
|
||||
getKey: (item: T) => string;
|
||||
@@ -105,8 +94,11 @@ export default function VirtualGrid<T>({
|
||||
// this wrong before. This is measured here anyway, for colCount.
|
||||
const [itemWidth, setItemWidth] = useState(0);
|
||||
|
||||
// The list scrolls inside an <main overflow-y-auto> (the page scroller), not the window, so the
|
||||
// virtualizer needs that element. The SAME walk lib/listNav measures its visible band against: the
|
||||
// virtualizer and the keyboard must agree on which element scrolls.
|
||||
useLayoutEffect(() => {
|
||||
if (listRef.current) setScrollEl(getScrollParent(listRef.current));
|
||||
if (listRef.current) setScrollEl(scrollParent(listRef.current) ?? document.documentElement);
|
||||
}, []);
|
||||
|
||||
// Re-measure the column count and start offset on container/scroll-area resize.
|
||||
|
||||
Reference in New Issue
Block a user