diff --git a/frontend/src/components/AccountPanel.tsx b/frontend/src/components/AccountPanel.tsx index 42298122..7f91ac25 100644 --- a/frontend/src/components/AccountPanel.tsx +++ b/frontend/src/components/AccountPanel.tsx @@ -7,9 +7,9 @@ import { LoadingState, StateMessage } from "./ui/QueryState"; import { api } from "../lib/api"; import { qk } from "../lib/queryKeys"; import { notify } from "../lib/notifications"; -import { notifyActionError, notifyKnownFailure } from "../lib/actionErrors"; +import { notifyActionError } from "../lib/actionErrors"; import { copyText } from "../lib/clipboard"; -import { invalidateUserAccess, isEmailOff } from "../lib/adminUsers"; +import { invalidateUserAccess } from "../lib/adminUsers"; import type { AdminUserAccess, AdminUserRow } from "../lib/api/admin"; // The admin's answer to "why can this person not get in?", and the place every recovery action for @@ -73,10 +73,7 @@ export default function AccountPanel({ const resend = useMutation({ mutationFn: () => api.resendUserVerification(user.id), onSuccess: () => notify({ level: "success", message: t("users.account.resendSent") }), - onError: (err) => - isEmailOff(err) - ? notifyKnownFailure(err, t("users.pendingAccess.resendEmailOff")) - : notifyActionError(err, t("users.pendingAccess.resendFailed")), + onError: (err) => notifyActionError(err, t("users.pendingAccess.resendFailed")), }); const activate = useMutation({ diff --git a/frontend/src/components/AdminUsers.tsx b/frontend/src/components/AdminUsers.tsx index 2bd5a7d2..916d5508 100644 --- a/frontend/src/components/AdminUsers.tsx +++ b/frontend/src/components/AdminUsers.tsx @@ -24,7 +24,7 @@ import { type PendingAccessRow, } from "../lib/api"; import { notify } from "../lib/notifications"; -import { invalidateUserAccess, isEmailOff } from "../lib/adminUsers"; +import { invalidateUserAccess } from "../lib/adminUsers"; import { ADMIN_USERS_TAB_KEY } from "../lib/adminUsersTab"; import { Button } from "./ui/Button"; import Tooltip from "./ui/Tooltip"; @@ -319,12 +319,6 @@ function UsersRoles({ me }: { me: Me }) { ); } -/** Did the invitation fail because this instance has no working SMTP? */ -function inviteEmailOff(err: HttpError): boolean { - const d = err.detailData as { code?: string; reason?: string } | null | undefined; - return d?.code === "invite_send" && d.reason === "email_off"; -} - // --- Sign-ups that are still outside (roadmap UX-25) -------------------------------------------- // // The gap this closes: approve an address BEFORE the person registers and their sign-up produces no @@ -360,12 +354,9 @@ function PendingAccess() { refresh(); notify({ level: "success", message: t("users.pendingAccess.resent", { email: u.email }) }); }, - // The public resend is deliberately silent; this one is not, so say WHY it failed — an instance - // with no working SMTP would otherwise show a success toast for a mail that was never sent. - onError: (err) => - isEmailOff(err) - ? notifyKnownFailure(err, t("users.pendingAccess.resendEmailOff")) - : notifyActionError(err, t("users.pendingAccess.resendFailed")), + // The public resend is deliberately silent; this one is not, and the server's reason says WHY + // (no working SMTP, a refused send) — a success toast for a mail never sent would be the lie. + onError: (err) => notifyActionError(err, t("users.pendingAccess.resendFailed")), }); const list = rows.data ?? []; @@ -591,10 +582,7 @@ function AdminInvites() { refresh(); notify({ level: "success", message: t("settings.invites.invited", { email: v.inv.email }) }); }, - onError: (err) => - err instanceof HttpError && inviteEmailOff(err) - ? notifyKnownFailure(err, t("settings.invites.inviteEmailOff")) - : notifyActionError(err, t("settings.invites.inviteFailed")), + onError: (err) => notifyActionError(err, t("settings.invites.inviteFailed")), }); // The one recipient the app has no language signal about: the admin typed this address in, so // nobody ever told us how to write to its owner. The dialog is where that gets decided — and it diff --git a/frontend/src/components/ChannelPage.busyFocus.dom.test.tsx b/frontend/src/components/ChannelPage.busyFocus.dom.test.tsx new file mode 100644 index 00000000..e676af89 --- /dev/null +++ b/frontend/src/components/ChannelPage.busyFocus.dom.test.tsx @@ -0,0 +1,93 @@ +// @vitest-environment jsdom +// +// The channel page's Block and Hide toggles while their request runs (deep-review E11 round 2). A +// focused button that turns natively `disabled` drops the keyboard to in Chrome (measured for +// ui/Button in E11 S2), so a busy toggle blocks the click with aria-disabled instead — focusable, and +// still pressed only once. +import type { ReactNode } from "react"; +import { describe, it, expect, vi, beforeAll, afterEach } from "vitest"; +import { render, screen, fireEvent, waitFor, cleanup, act } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +vi.mock("react-i18next", async (importOriginal) => ({ + ...(await importOriginal()), + useTranslation: () => ({ t: (key: string) => key, i18n: { language: "en" } }), +})); +vi.mock("./NavigationProvider", () => ({ + useNavigationActions: () => ({ closeChannel: vi.fn(), openChannel: vi.fn() }), +})); +vi.mock("./ConfirmProvider", () => ({ useConfirm: () => async () => true })); +vi.mock("./FeedFiltersProvider", () => ({ + useFeedFilters: () => ({ includeNormal: true, includeShorts: false, includeLive: false }), +})); +vi.mock("./Feed", () => ({ default: () => null })); +vi.mock("./ChannelAbout", () => ({ default: () => null })); +// The page's chrome portals into the shell's fixed toolbar band, which this test has no shell for. +vi.mock("./ui/PageShell", async (importOriginal) => ({ + ...(await importOriginal()), + PageToolbar: ({ children }: { children?: ReactNode }) => <>{children}, +})); + +import ChannelPage from "./ChannelPage"; +import { api, type ChannelDetail, type Me } from "../lib/api"; + +beforeAll(() => { + class Stub { + observe() {} + unobserve() {} + disconnect() {} + takeRecords() { + return []; + } + } + globalThis.ResizeObserver ??= Stub as unknown as typeof ResizeObserver; + globalThis.IntersectionObserver ??= Stub as unknown as typeof IntersectionObserver; +}); +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const CH = "UCsXVk37bltHxD1rDPwtNM8Q"; + +describe("ChannelPage — a busy toggle keeps the keyboard", () => { + it.each([ + ["channel.block", "blockChannel"], + ["channel.hide", "updateChannel"], + ] as const)("%s stays focusable and fires once while its request runs", async (label, call) => { + vi.spyOn(api, "channelDetail").mockResolvedValue({ + id: CH, + title: "LEMMiNO", + subscribed: true, + blocked: false, + hidden: false, + deep_requested: false, + backfill_done: true, + stored_videos: 10, + } as unknown as ChannelDetail); + vi.spyOn(api, "tags").mockResolvedValue([] as never); + const pending = vi.spyOn(api, call).mockReturnValue(new Promise(() => {}) as never); + render( + + {}} + onShowInFeed={() => {}} + /> + , + ); + const button = await screen.findByRole("button", { name: label }); + act(() => button.focus()); + fireEvent.click(button); + await waitFor(() => expect(button.getAttribute("aria-disabled")).toBe("true")); + expect(button.hasAttribute("disabled")).toBe(false); + expect(document.activeElement).toBe(button); + fireEvent.click(button); + expect(pending).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/components/ChannelPage.tsx b/frontend/src/components/ChannelPage.tsx index fc7bca47..5982111f 100644 --- a/frontend/src/components/ChannelPage.tsx +++ b/frontend/src/components/ChannelPage.tsx @@ -479,11 +479,13 @@ export default function ChannelPage({ )} {!me.is_demo && ( + + ))} + + ); + } + + // `s`, a query, Escape: the feed stays mounted with new results and the old card is gone. The slot + // kept for a REMOVED card sent the next `j` to the third new result instead of the first. + it("starts from the first visible item after focus went on to a field and the items changed", () => { + withLayout( + (_, n) => ({ left: 0, top: n * 100 }), + () => { + const page = (ids: number[]) => ( + <> + + + + ); + const { rerender } = render(page([0, 1, 2, 3])); + press("j"); + press("j"); + press("j"); + expect(focused()).toBe("r2"); + act(() => screen.getByTestId("field").focus()); + rerender(page([4, 5, 6, 7])); + act(() => screen.getByTestId("field").blur()); + press("j"); + expect(focused()).toBe("r4"); + }, + ); + }); + + // A confirm (or the item's own menu) takes the focus before the delete it confirms: back from it, the + // next `j` still goes on from the deleted row's slot. + it("keeps the slot when the removal was confirmed in a dialog", () => { + withLayout( + (_, n) => ({ left: 0, top: n * 100 }), + () => { + const page = (ids: number[]) => ( + <> +
+ +
+ + + ); + const { rerender } = render(page([0, 1, 2, 3])); + press("j"); + press("j"); + press("j"); + act(() => screen.getByTestId("ok").focus()); + rerender(page([0, 1, 3])); + act(() => screen.getByTestId("ok").blur()); + press("j"); + expect(focused()).toBe("r3"); + }, + ); + }); + + // A card root is a plain
: focused by `j` it had no role and no name, so a screen reader read + // an unlabelled run of text. + it("names a focused card after what it opens while it holds the focus, then gives the name back", () => { + render(); + press("j"); + const a0 = screen.getByTestId("a0"); + expect(a0.getAttribute("role")).toBe("group"); + expect(a0.getAttribute("aria-label")).toBe("title"); + press("j"); + expect(a0.hasAttribute("role")).toBe(false); + expect(a0.hasAttribute("aria-label")).toBe(false); + expect(screen.getByTestId("a1").getAttribute("aria-label")).toBe("title"); + }); +}); diff --git a/frontend/src/lib/listNav.scope.dom.test.tsx b/frontend/src/lib/listNav.scope.dom.test.tsx new file mode 100644 index 00000000..49af4087 --- /dev/null +++ b/frontend/src/lib/listNav.scope.dom.test.tsx @@ -0,0 +1,54 @@ +// @vitest-environment jsdom +// +// The `list` key scope — what puts j/k, the arrows and Enter on the cheat sheet — is live only while a +// registered list has an item to land on. An empty download queue, a list still loading or one that +// failed mounts its container all the same, and the sheet listed the keys over nothing (deep-review +// E11 round 2). Its own file: listNav keeps module state. +import { useRef } from "react"; +import { afterEach, describe, expect, it } from "vitest"; +import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import { navItemAttrs, useListNav } from "./listNav"; +import { useActiveScopes } from "./keymap"; + +afterEach(cleanup); + +function Scopes() { + return {useActiveScopes().join(" ")}; +} + +function Queue({ count }: { count: number }) { + const ref = useRef(null); + useListNav(ref, { enabled: true }); + return ( +
+ {count === 0 ? ( +

Nothing queued

+ ) : ( + Array.from({ length: count }, (_, i) => ( +
+ item +
+ )) + )} +
+ ); +} + +const page = (count: number) => ( + <> + + + +); +const scopes = () => screen.getByTestId("scopes").textContent!.split(" "); + +describe("useListNav · the list key scope", () => { + it("is not live over a list with nothing in it, and is while it has items", async () => { + const { rerender } = render(page(0)); + expect(scopes()).not.toContain("list"); + rerender(page(2)); + await waitFor(() => expect(scopes()).toContain("list")); + rerender(page(0)); + await waitFor(() => expect(scopes()).not.toContain("list")); + }); +}); diff --git a/frontend/src/lib/listNav.ts b/frontend/src/lib/listNav.ts index fa3cf1ec..dda79144 100644 --- a/frontend/src/lib/listNav.ts +++ b/frontend/src/lib/listNav.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, type RefObject } from "react"; +import { useEffect, useRef, useState, type RefObject } from "react"; import { dispatch, useKeyScope } from "./keymap"; import { scrollParent } from "./listPaging"; @@ -35,8 +35,9 @@ interface NavList { } const lists = new Map(); -// The item we gave a tabindex to, so the next move can take it back. +// The item we gave a tabindex to, so the next move can take it back — and whether we named it too. let roved: HTMLElement | null = null; +let named = false; const INDEX = "data-nav-index"; const LIST = "data-nav-list"; @@ -125,10 +126,17 @@ function firstVisible(): { list: NavList; index: number } | null { let last: { list: NavList; index: number; el: HTMLElement } | null = null; function onFocusIn(e: FocusEvent): void { - const item = (e.target as HTMLElement | null)?.closest?.(`[${INDEX}]`); + const target = e.target as HTMLElement | null; + const item = target?.closest?.(`[${INDEX}]`); const list = item && lists.get(item.parentElement?.closest(`[${LIST}]`) as HTMLElement); if (item && list) last = { list, index: indexOf(item), el: item }; + // 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 + // E11 round 2). A layer opened FROM the item — its menu, a confirm — does not count: focus comes + // back from it, and what it did (a hidden card, a deleted row) is the removal the slot is kept for. + // Removing the focused node itself fires no focusin. + else if (!target?.closest?.('[data-floating-ui-portal], [role="dialog"]')) last = null; } /** Where `j` (`k`: the item before) lands after the last-focused item left the DOM: the item now in @@ -150,13 +158,31 @@ function slotOfRemoved(delta: 1 | -1): { list: NavList; index: number } | null { function focusItem(el: HTMLElement, reveal = true): void { if (roved !== el) { + if (named) roved?.removeAttribute("role"); + if (named) roved?.removeAttribute("aria-label"); roved?.removeAttribute("tabindex"); roved = null; + named = false; // An item that is a Tab stop in its own right (a native