diff --git a/admin/src/features/activity/ActivityDetailPage.test.tsx b/admin/src/features/activity/ActivityDetailPage.test.tsx index 1837cf1..ff84d32 100644 --- a/admin/src/features/activity/ActivityDetailPage.test.tsx +++ b/admin/src/features/activity/ActivityDetailPage.test.tsx @@ -1,4 +1,4 @@ -import { cleanup, render, screen, waitFor, within } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; import { QueryClientProvider } from "@tanstack/react-query"; import { RouterProvider, createMemoryHistory } from "@tanstack/react-router"; import { AuthProvider } from "@/auth/store"; @@ -313,16 +313,17 @@ test("a row retention has pruned explains the 404 and keeps the way back to the }); }); -/** - * The related-actions region of a query detail. Scoped on purpose: the sidebar - * carries a Pause of its own, and this is the one that answers "this query was - * blocked and should not have been". - */ +/** The related-actions region of a query detail. */ function related(): HTMLElement { return screen.getByRole("region", { name: "Related" }); } -test("a blocked query's Related offers Pause; an allowed one has nothing to pause about", async () => { +/** + * Related is links only. Pause is a resolver-wide control and lives in the + * sidebar alone, so a blocked query — the case that used to carry one here — + * offers no button of any kind. + */ +test("Related carries its four links and no control, blocked query or not", async () => { responses["/api/queries/50"] = detail(50, { policy: { action: "block", reason: "blocklist_domain", matched: "ads.example" }, route: { kind: "blocked", upstream: "" }, @@ -330,25 +331,12 @@ test("a blocked query's Related offers Pause; an allowed one has nothing to paus renderDetail(50); await screen.findByRole("heading", { name: "example.com" }); - await waitFor(() => expect(within(related()).getByRole("button", { name: "Pause" })).toBeTruthy()); - - cleanup(); - responses["/api/queries/51"] = detail(51, { policy: { action: "allow", reason: "no_match", matched: "" } }); - renderDetail(51); - - await screen.findByRole("heading", { name: "example.com" }); - expect(within(related()).queryByRole("button", { name: "Pause" })).toBeNull(); -}); - -test("the Pause action stays away while protection is unavailable", async () => { - responses["/api/health"] = health({ protection: { state: "unavailable", until: null } }); - responses["/api/queries/52"] = detail(52, { - policy: { action: "block", reason: "blocklist_domain", matched: "ads.example" }, - route: { kind: "blocked", upstream: "" }, - }); - renderDetail(52); - - await screen.findByRole("heading", { name: "example.com" }); - await waitFor(() => expect(screen.getByText("Diagnostics around this query")).toBeTruthy()); - expect(within(related()).queryByRole("button", { name: "Pause" })).toBeNull(); + await waitFor(() => expect(within(related()).getByText("Diagnostics around this query")).toBeTruthy()); + expect(within(related()).getAllByRole("link").map((link) => link.textContent)).toEqual([ + "Test this domain against current policy", + "All activity for this domain", + "All activity from this client", + "Diagnostics around this query", + ]); + expect(within(related()).queryByRole("button")).toBeNull(); }); diff --git a/admin/src/features/activity/ActivityDetailPage.tsx b/admin/src/features/activity/ActivityDetailPage.tsx index 3e2fde6..a35afca 100644 --- a/admin/src/features/activity/ActivityDetailPage.tsx +++ b/admin/src/features/activity/ActivityDetailPage.tsx @@ -73,9 +73,7 @@ export default function ActivityDetailPage() { domain={domain} client={client} ts={time} - origin={origin} - blocked={detail.policy.action === "block"} - /> + origin={origin} /> } /> diff --git a/admin/src/features/activity/LiveActivity.test.tsx b/admin/src/features/activity/LiveActivity.test.tsx index 9e117ef..a72fd7b 100644 --- a/admin/src/features/activity/LiveActivity.test.tsx +++ b/admin/src/features/activity/LiveActivity.test.tsx @@ -446,16 +446,16 @@ test("leaving live closes the stream, and coming back opens exactly one fresh on expect(sources[1]!.closed).toBe(false); }); -/** - * The related-actions region of a query detail. Scoped on purpose: the sidebar - * carries a Pause of its own, and this is the one that answers "this query was - * blocked and should not have been". - */ +/** The related-actions region of a query detail. */ function related(): HTMLElement { return screen.getByRole("region", { name: "Related" }); } -test("a streamed blocked row carries the same Pause action as the persisted detail", async () => { +/** + * The streamed detail carries the same Related as the persisted one: four links + * and no control. Pause is resolver-wide and lives in the sidebar alone. + */ +test("a streamed blocked row's Related carries links only", async () => { await openLive(); act(() => sources[0]!.emit( @@ -468,14 +468,7 @@ test("a streamed blocked row carries the same Pause action as the persisted deta ); fireEvent.click(screen.getByRole("button", { name: "streamed.example" })); - await waitFor(() => expect(within(related()).getByRole("button", { name: "Pause" })).toBeTruthy()); -}); - -test("a streamed row that was allowed offers nothing to pause", async () => { - await openLive(); - act(() => sources[0]!.emit("query", frame(1001, "allowed.example", { policy: { action: "allow" } }))); - fireEvent.click(screen.getByRole("button", { name: "allowed.example" })); - - await screen.findByRole("heading", { level: 1, name: "allowed.example" }); - expect(within(related()).queryByRole("button", { name: "Pause" })).toBeNull(); + await waitFor(() => expect(within(related()).getByText("Diagnostics around this query")).toBeTruthy()); + expect(within(related()).getAllByRole("link")).toHaveLength(4); + expect(within(related()).queryByRole("button")).toBeNull(); }); diff --git a/admin/src/features/activity/LiveActivity.tsx b/admin/src/features/activity/LiveActivity.tsx index 5bf6102..7e505fc 100644 --- a/admin/src/features/activity/LiveActivity.tsx +++ b/admin/src/features/activity/LiveActivity.tsx @@ -268,9 +268,7 @@ function LiveDetail({ row, origin, onClose }: { row: StreamedRow; origin: Activi domain={summary.domain} client={summary.client_ip} ts={summary.ts} - origin={origin} - blocked={row.event.policy.action === "block"} - /> + origin={origin} /> } /> diff --git a/admin/src/features/activity/RelatedActions.tsx b/admin/src/features/activity/RelatedActions.tsx index a8ebce1..bd04d05 100644 --- a/admin/src/features/activity/RelatedActions.tsx +++ b/admin/src/features/activity/RelatedActions.tsx @@ -11,7 +11,6 @@ import { Link } from "@tanstack/react-router"; import * as stylex from "@stylexjs/stylex"; -import PauseControl from "@/features/pause/PauseControl"; import { styles as shared } from "@/ui/styles"; import { provenanceRelatedLink } from "./ProvenanceDetail"; import { diagnosticsBounds, relatedBounds } from "./relatedLinks"; @@ -24,15 +23,9 @@ interface Props { ts: number; /** The Activity search the reader came from; its bounds win over the defaults. */ origin: Pick; - /** - * This query was blocked. Pausing is a valid answer to a block the reader - * disagrees with, and to nothing else here — so the control appears for a - * block and not beside an allowed query it could not have caused. - */ - blocked: boolean; } -export default function RelatedActions({ domain, client, ts, origin, blocked }: Props) { +export default function RelatedActions({ domain, client, ts, origin }: Props) { const bounds = relatedBounds(ts, origin); const window = diagnosticsBounds(ts); return ( @@ -61,7 +54,6 @@ export default function RelatedActions({ domain, client, ts, origin, blocked }: > Diagnostics around this query - {blocked && } ); } diff --git a/admin/src/features/pause/PauseControl.test.tsx b/admin/src/features/pause/PauseControl.test.tsx index 1b14254..b3e0872 100644 --- a/admin/src/features/pause/PauseControl.test.tsx +++ b/admin/src/features/pause/PauseControl.test.tsx @@ -91,11 +91,14 @@ test("unpaused: duration menu pauses with the picked duration_seconds", async () fireEvent.click(trigger); expect(trigger.getAttribute("aria-expanded")).toBe("true"); - for (const label of ["60 seconds", "5 minutes", "30 minutes", "Indefinitely"]) { - expect(screen.getByRole("button", { name: label })).toBeTruthy(); - } + expect(screen.getAllByRole("menuitem").map((item) => item.textContent)).toEqual([ + "60 seconds", + "5 minutes", + "30 minutes", + "Indefinitely", + ]); - fireEvent.click(screen.getByRole("button", { name: "5 minutes" })); + fireEvent.click(screen.getByRole("menuitem", { name: "5 minutes" })); await waitFor(() => expect(postBodies).toEqual([{ paused: true, duration_seconds: 300 }])); await screen.findByRole("button", { name: "Resume" }); @@ -105,7 +108,7 @@ test("indefinite pause sends no duration_seconds", async () => { renderControl(); fireEvent.click(await findPauseTrigger()); - fireEvent.click(screen.getByRole("button", { name: "Indefinitely" })); + fireEvent.click(screen.getByRole("menuitem", { name: "Indefinitely" })); await waitFor(() => expect(postBodies).toEqual([{ paused: true }])); await screen.findByRole("button", { name: "Resume" }); @@ -151,11 +154,15 @@ test("an active resolver states nothing: the button already says Pause", async ( test("escape closes the duration menu", async () => { renderControl(); - const trigger = await findPauseTrigger(); - fireEvent.click(trigger); - expect(screen.getByRole("button", { name: "Indefinitely" })).toBeTruthy(); - fireEvent.keyDown(trigger, { key: "Escape" }); - expect(screen.queryByRole("button", { name: "Indefinitely" })).toBeNull(); + fireEvent.click(await findPauseTrigger()); + expect(screen.getByRole("menuitem", { name: "Indefinitely" })).toBeTruthy(); + + // Opening the menu moves focus into it, so Escape is pressed where the reader + // is, not on the trigger they left. + fireEvent.keyDown(screen.getByRole("menu"), { key: "Escape" }); + + await waitFor(() => expect(screen.queryByRole("menuitem", { name: "Indefinitely" })).toBeNull()); + expect(screen.getByRole("button", { name: "Pause" }).getAttribute("aria-expanded")).toBe("false"); }); test("failed pause with 429 shows a ticking retry countdown", async () => { @@ -168,7 +175,7 @@ test("failed pause with 429 shows a ticking retry countdown", async () => { renderControl(); fireEvent.click(await findPauseTrigger()); - fireEvent.click(screen.getByRole("button", { name: "5 minutes" })); + fireEvent.click(screen.getByRole("menuitem", { name: "5 minutes" })); const alert = await screen.findByRole("alert"); expect(alert.textContent).toBe("Rate limited. Try again in 30s."); @@ -187,7 +194,7 @@ test("failed pause with 503 shows the degraded message", async () => { renderControl(); fireEvent.click(await findPauseTrigger()); - fireEvent.click(screen.getByRole("button", { name: "60 seconds" })); + fireEvent.click(screen.getByRole("menuitem", { name: "60 seconds" })); const alert = await screen.findByRole("alert"); expect(alert.textContent).toBe("The server is starting or degraded. Try again shortly."); @@ -202,12 +209,12 @@ test("a successful pause clears the previous mutation error", async () => { renderControl(); fireEvent.click(await findPauseTrigger()); - fireEvent.click(screen.getByRole("button", { name: "5 minutes" })); + fireEvent.click(screen.getByRole("menuitem", { name: "5 minutes" })); await screen.findByRole("alert"); postFailure = null; fireEvent.click(screen.getByRole("button", { name: "Pause" })); - fireEvent.click(screen.getByRole("button", { name: "5 minutes" })); + fireEvent.click(screen.getByRole("menuitem", { name: "5 minutes" })); await screen.findByRole("button", { name: "Resume" }); expect(screen.queryByRole("alert")).toBeNull(); @@ -240,7 +247,7 @@ test("a menu left open when the control withdraws does not come back open", asyn vi.useFakeTimers({ shouldAdvanceTime: true }); renderControl(); fireEvent.click(await findPauseTrigger()); - expect(screen.getByRole("button", { name: "Indefinitely" })).toBeTruthy(); + expect(screen.getByRole("menuitem", { name: "Indefinitely" })).toBeTruthy(); protection = { state: "unavailable", until: null }; await vi.advanceTimersByTimeAsync(11_000); @@ -253,7 +260,7 @@ test("a menu left open when the control withdraws does not come back open", asyn // reader opened, and nobody opened this one. const trigger = await vi.waitFor(() => screen.getByRole("button", { name: "Pause" })); expect(trigger.getAttribute("aria-expanded")).toBe("false"); - expect(screen.queryByRole("button", { name: "Indefinitely" })).toBeNull(); + expect(screen.queryByRole("menuitem", { name: "Indefinitely" })).toBeNull(); vi.useRealTimers(); }); @@ -261,7 +268,7 @@ test("a menu open when someone else pauses does not reopen when that pause ends" vi.useFakeTimers({ shouldAdvanceTime: true }); renderControl(); fireEvent.click(await findPauseTrigger()); - expect(screen.getByRole("button", { name: "Indefinitely" })).toBeTruthy(); + expect(screen.getByRole("menuitem", { name: "Indefinitely" })).toBeTruthy(); // Filtering is paused from somewhere else, and this browser learns it from // the poll. The Resume rendering has no menu. @@ -274,7 +281,7 @@ test("a menu open when someone else pauses does not reopen when that pause ends" const trigger = await vi.waitFor(() => screen.getByRole("button", { name: "Pause" })); expect(trigger.getAttribute("aria-expanded")).toBe("false"); - expect(screen.queryByRole("button", { name: "Indefinitely" })).toBeNull(); + expect(screen.queryByRole("menuitem", { name: "Indefinitely" })).toBeNull(); vi.useRealTimers(); }); diff --git a/admin/src/features/pause/PauseControl.tsx b/admin/src/features/pause/PauseControl.tsx index 3fcd700..2e9a401 100644 --- a/admin/src/features/pause/PauseControl.tsx +++ b/admin/src/features/pause/PauseControl.tsx @@ -1,8 +1,7 @@ /** - * The Pause/Resume control, in the two places a pause is a valid answer to what - * the reader is looking at: the foot of the sidebar, where it belongs to the - * resolver rather than to any page, and beside the detail of a query that was - * blocked. + * The Pause/Resume control, at the foot of the sidebar. A pause stops filtering + * for the whole resolver, so it belongs to the shell rather than to any page, + * and it has no second placement. * * It reads `Health.protection` rather than `/api/pause` so it cannot contradict * the Diagnostics health strip, and it renders nothing at all while protection @@ -20,9 +19,10 @@ * line: the button says Pause, which is the whole message. */ -import { useEffect, useState } from "react"; +import { useEffect } from "react"; import { useMutation, useQueryClient } from "@tanstack/react-query"; import * as stylex from "@stylexjs/stylex"; +import { Button, Menu, MenuItem, MenuTrigger, Popover } from "react-aria-components"; import { formatClock } from "@/lib/format"; import { pauseMutation } from "@/lib/queries"; import InlineError from "@/lib/InlineError"; @@ -48,6 +48,8 @@ const styles = stylex.create({ ":disabled": "oklch(70.5% 0.015 286.067)", "@media (prefers-color-scheme: dark)": { default: null, ":disabled": "oklch(44.2% 0.017 285.786)" }, }, + /** The sidebar foot is the only placement, and there Log out sets the width. */ + width: "100%", }, row: { display: "flex", @@ -61,44 +63,44 @@ const styles = stylex.create({ lineHeight: "1rem", color: colors.textSecondary, }, - anchor: { - position: "relative", - }, - menu: { - position: "absolute", - left: 0, - top: "100%", - zIndex: 10, - marginTop: "0.25rem", - display: "flex", - width: "9rem", - flexDirection: "column", + /** `--trigger-width` is RAC's: the menu is as wide as the button that opened it. */ + popover: { + width: "var(--trigger-width)", borderRadius: "0.25rem", borderWidth: 1, borderStyle: "solid", borderColor: colors.border, backgroundColor: colors.surfaceRaised, - paddingBlock: "0.25rem", + color: colors.text, boxShadow: "0 1px 3px 0 rgb(0 0 0 / 0.1), 0 1px 2px -1px rgb(0 0 0 / 0.1)", }, + menu: { + outlineStyle: "none", + paddingBlock: "0.25rem", + }, menuItem: { - cursor: { default: "pointer", ":disabled": "not-allowed" }, - borderStyle: "none", - backgroundColor: { default: "transparent", ":hover": colors.surfaceHover }, - color: "inherit", + cursor: "pointer", paddingInline: "0.75rem", paddingBlock: "0.375rem", - textAlign: "left", fontSize: "0.875rem", lineHeight: "1.25rem", }, + /** + * RAC focuses the item's own node, so the shared ring applies; it is inset + * because an item flush against the popover edge clips an outset one, and + * recoloured because the focus token is the blue this row just painted. + */ + menuItemFocused: { + backgroundColor: colors.primary, + color: colors.primaryText, + outlineColor: { default: null, ":focus-visible": colors.primaryText }, + }, }); export default function PauseControl() { const queryClient = useQueryClient(); const protection = useProtection(); const mutation = useMutation(pauseMutation(queryClient)); - const [menuOpen, setMenuOpen] = useState(false); const paused = protection.state === "paused"; const { reset } = mutation; useEffect(() => reset(), [paused, reset]); @@ -109,15 +111,6 @@ export default function PauseControl() { // the wrong action, which is the contradiction this control exists to end. const actionable = protection.state === "active" || protection.state === "paused"; - // Leaving `active` unmounts the menu but not the state that opened it, and - // the menu belongs to the active rendering alone — a pause someone else - // started, seen through the poll, takes it away exactly as a withdrawal does. - // Closing on the way out rather than on the way back means the trigger can - // only ever come back shut, however long it was gone. - useEffect(() => { - if (protection.state !== "active") setMenuOpen(false); - }, [protection.state]); - if (!actionable) return null; if (paused) { @@ -141,41 +134,40 @@ export default function PauseControl() { } return ( -
{ - if (e.key === "Escape") setMenuOpen(false); - }} - > - - {menuOpen && ( -
- {DURATIONS.map(({ label, seconds }) => ( - - ))} -
- )} +
+ + + stylex.props(styles.popover).className ?? ""}> + + {DURATIONS.map(({ label, seconds }) => ( + + mutation.mutate( + seconds === null + ? { paused: true } + : { paused: true, duration_seconds: seconds }, + ) + } + className={({ isFocused }) => + stylex.props( + styles.menuItem, + shared.insetFocusRing, + isFocused && styles.menuItemFocused, + ).className ?? "" + } + > + {label} + + ))} + + +
); diff --git a/admin/src/shell/AppShell.test.tsx b/admin/src/shell/AppShell.test.tsx index 76058e3..e7bfb99 100644 --- a/admin/src/shell/AppShell.test.tsx +++ b/admin/src/shell/AppShell.test.tsx @@ -214,12 +214,45 @@ test("mount probe reveals the logout button and a failed logout surfaces inline" renderShell(); - fireEvent.click(await screen.findByRole("button", { name: "Log out" })); + // Both renderings are mounted; the viewport paints the sidebar one on WIDE + // and the header one below it. + await waitFor(() => expect(screen.getAllByRole("button", { name: "Log out" })).toHaveLength(2)); + const aside = document.querySelector("aside") as HTMLElement; + fireEvent.click(within(aside).getByRole("button", { name: "Log out" })); - await screen.findByText("Rate limited. Try again in 7s."); + await within(aside).findByText("Rate limited. Try again in 7s."); expect(screen.getByRole("heading", { name: "Overview" })).toBeTruthy(); }); +test("Log out sits in the sidebar on wide, and only in the header below it", async () => { + stubFetch((url) => + url === "/api/auth/login" + ? new Response(JSON.stringify({ error: "password required" }), { + status: 401, + headers: { "content-type": "application/json" }, + }) + : null, + ); + renderShell(); + await screen.findByRole("heading", { name: "Overview" }); + + const aside = document.querySelector("aside") as HTMLElement; + const header = document.querySelector("header") as HTMLElement; + await waitFor(() => expect(within(aside).getByRole("button", { name: "Log out" })).toBeTruthy()); + expect(within(header).getByRole("button", { name: "Log out" })).toBeTruthy(); + // The header is the narrow rendering now: the Menu button is its only nav. + expect(within(header).getByRole("button", { name: "Menu" })).toBeTruthy(); + + // In the sidebar the control precedes the version label rather than crowding it. + const version = within(aside).getByText(/^nxdns v/); + const logout = within(aside).getByRole("button", { name: "Log out" }); + expect(logout.compareDocumentPosition(version) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + // The drawer keeps no third copy: the header already carries the narrow one. + fireEvent.click(within(header).getByRole("button", { name: "Menu" })); + const drawer = document.getElementById("mobile-nav") as HTMLElement; + expect(within(drawer).queryByRole("button", { name: "Log out" })).toBeNull(); +}); + test("the header carries no protection display at all any more", async () => { renderShell(); await screen.findByRole("heading", { name: "Overview" }); diff --git a/admin/src/shell/AppShell.tsx b/admin/src/shell/AppShell.tsx index ce1e0b5..c95bdb2 100644 --- a/admin/src/shell/AppShell.tsx +++ b/admin/src/shell/AppShell.tsx @@ -110,6 +110,17 @@ const styles = stylex.create({ alignItems: "center", gap: "0.5rem", }, + /** + * In a 14rem column the button and its error stack, so the button spans the + * sidebar and matches the Pause trigger below it. + */ + sidebarLogout: { + flexDirection: "column", + alignItems: "stretch", + gap: "0.25rem", + paddingInline: "1rem", + paddingTop: "0.75rem", + }, shell: { minHeight: "100dvh", height: { default: null, [WIDE]: "100dvh" }, @@ -147,8 +158,9 @@ const styles = stylex.create({ minWidth: { default: null, [WIDE]: 0 }, flexDirection: "column", }, + /** Narrow only: on WIDE the sidebar carries everything this row held. */ header: { - display: "flex", + display: { default: "flex", [WIDE]: "none" }, alignItems: "center", gap: "0.75rem", borderBottomWidth: 1, @@ -284,13 +296,13 @@ function VersionFooter() { ); } -function LogoutButton() { +function LogoutButton({ style }: { style?: stylex.StyleXStyles }) { const { authRequired, logout } = useAuth(); const navigate = useNavigate(); const [error, setError] = useState(null); if (authRequired !== true) return null; return ( -
+