fix(ui): the reader's own use of a stand-in stops the restore; the pill's name carries its cap
Round 2 of the deep review: - A click on, or a key typed into, the focus keeper's stand-in fires no focusin, so a restore still polling for its list could take the focus from under the reader. A pointerdown or keydown now makes it the reader's own focus. - The Jump-to pill hid its "Ctrl K" from its name (the voice command "click Ctrl K" still matched nothing); the key cap's tooltip now equals its name, so it is not read twice as a description. - listNav's comment still cited DataTable's two layouts as the reason for its visibility filter.
This commit is contained in:
@@ -57,6 +57,8 @@ describe("the command palette's visible entry", () => {
|
||||
const cap = screen.getByRole("button", { name: /^palette\.open \((Ctrl|⌘) K\)$/ });
|
||||
// Label in Name: the name carries what the cap shows, so "click Ctrl K" finds it.
|
||||
expect(cap.getAttribute("aria-label")).toContain(cap.textContent);
|
||||
// …and a tooltip equal to that name, so a screen reader does not read it again as a description.
|
||||
expect(cap.getAttribute("title")).toBe(cap.getAttribute("aria-label"));
|
||||
expect(screen.getByRole("textbox")).toBeTruthy();
|
||||
fireEvent.click(cap);
|
||||
expect(isOpen()).toBe(true);
|
||||
@@ -66,8 +68,8 @@ describe("the command palette's visible entry", () => {
|
||||
page = "home";
|
||||
const isOpen = mount();
|
||||
expect(screen.queryByRole("textbox")).toBeNull();
|
||||
// Named by what it shows ("Jump to…"), not by a label that replaced it.
|
||||
const pill = screen.getByRole("button", { name: "palette.title" });
|
||||
// Named by what it shows ("Jump to…" and its key cap), not by a label that replaced it.
|
||||
const pill = screen.getByRole("button", { name: /^palette\.title ?(Ctrl|⌘) K$/ });
|
||||
expect(pill.getAttribute("title")).toBe("palette.open");
|
||||
fireEvent.click(pill);
|
||||
expect(isOpen()).toBe(true);
|
||||
|
||||
@@ -34,7 +34,7 @@ export function PaletteKeycap() {
|
||||
type="button"
|
||||
onClick={openPalette}
|
||||
aria-label={`${t("palette.open")} (${cap})`}
|
||||
title={t("palette.open")}
|
||||
title={`${t("palette.open")} (${cap})`}
|
||||
className="shrink-0 rounded-md border border-border px-1.5 py-0.5 font-mono text-3xs text-muted transition hover:border-accent/50 hover:text-fg @max-[640px]/appmain:hidden"
|
||||
>
|
||||
{cap}
|
||||
@@ -55,10 +55,7 @@ export function PaletteLauncher() {
|
||||
>
|
||||
<Command className="h-4 w-4 shrink-0" aria-hidden="true" />
|
||||
<span>{t("palette.title")}</span>
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className="rounded-md border border-border px-1.5 py-0.5 font-mono text-3xs @max-[640px]/appmain:hidden"
|
||||
>
|
||||
<span className="rounded-md border border-border px-1.5 py-0.5 font-mono text-3xs @max-[640px]/appmain:hidden">
|
||||
{cap}
|
||||
</span>
|
||||
</button>
|
||||
|
||||
@@ -66,8 +66,8 @@ function itemAt(list: NavList, index: number): HTMLElement | null {
|
||||
function ordered(): NavList[] {
|
||||
return (
|
||||
[...lists.values()]
|
||||
// Only lists on screen: a responsive table renders its rows twice — a table and a card list, one
|
||||
// of them display:none at any width (ui/DataTable) — and `j` must not walk into the hidden copy.
|
||||
// Only lists on screen: a list can be mounted yet hidden (display:none in a collapsed panel or
|
||||
// an inactive tab), and `j` must not walk into rows nobody can see.
|
||||
.filter((l) => l.el.isConnected && l.el.checkVisibility?.() !== false)
|
||||
.sort((a, b) =>
|
||||
a.el.compareDocumentPosition(b.el) & Node.DOCUMENT_POSITION_FOLLOWING ? -1 : 1,
|
||||
|
||||
@@ -61,4 +61,26 @@ describe("Back from a sub-view whose held control the swap removed", () => {
|
||||
document.activeElement?.closest("[data-nav-index]")?.getAttribute("data-nav-index"),
|
||||
).toBe("2");
|
||||
});
|
||||
|
||||
it("leaves the focus alone once the reader has used the stand-in", async () => {
|
||||
render(<Page />);
|
||||
button("c").focus();
|
||||
const pos = navPosition()!;
|
||||
act(() => show("detail"));
|
||||
button("Back to library").focus();
|
||||
|
||||
act(() => show("grid"));
|
||||
await act(() => Promise.resolve()); // the keeper places its stand-in
|
||||
const standIn = document.activeElement;
|
||||
expect(standIn?.textContent).toBe("a");
|
||||
// A key typed into it fires no focusin — but it is the reader's focus now.
|
||||
act(() => {
|
||||
standIn!.dispatchEvent(new KeyboardEvent("keydown", { key: "x", bubbles: true }));
|
||||
});
|
||||
const cancel = restoreNavPosition(pos);
|
||||
await wait(120);
|
||||
cancel();
|
||||
|
||||
expect(document.activeElement).toBe(standIn);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -70,6 +70,12 @@ export function useFocusKeeper(ref: RefObject<HTMLElement | null>): void {
|
||||
if (t !== standIn) standIn = null;
|
||||
held = t && root.contains(t) && !t.closest("[data-nav-index]") ? t : null;
|
||||
};
|
||||
// Using the stand-in makes it the reader's own focus: a click on it or a key typed into it fires
|
||||
// no focusin, and a restore still polling for its list would otherwise take the focus from
|
||||
// under the reader's hands.
|
||||
const onUse = () => {
|
||||
standIn = null;
|
||||
};
|
||||
const watch = new MutationObserver((records) => {
|
||||
const lost = held;
|
||||
if (!lost || lost.isConnected) return;
|
||||
@@ -86,9 +92,13 @@ export function useFocusKeeper(ref: RefObject<HTMLElement | null>): void {
|
||||
}
|
||||
});
|
||||
document.addEventListener("focusin", onFocusIn);
|
||||
document.addEventListener("pointerdown", onUse, true);
|
||||
document.addEventListener("keydown", onUse, true);
|
||||
watch.observe(root, { childList: true, subtree: true });
|
||||
return () => {
|
||||
document.removeEventListener("focusin", onFocusIn);
|
||||
document.removeEventListener("pointerdown", onUse, true);
|
||||
document.removeEventListener("keydown", onUse, true);
|
||||
watch.disconnect();
|
||||
};
|
||||
}, [ref]);
|
||||
|
||||
Reference in New Issue
Block a user