activity: select-only multi-client filter

the freetype client field is gone. the picker is a select-only menu of known clients with multi-select, chips for the active set, and a 32-client cap shared with the server. the api accepts a comma-separated client list and filters any-of with bound parameters. the trigger carets come from phosphor icons, newly adopted. bundle budget rises to 900000 bytes for the picker and the icon dependency.

admin/src/features/activity/ActivityFilters.test.tsx

@@ -10,14 +10,63 @@
  */
 
 import { useCallback, useState } from "react";
-import { afterAll, expect, test, vi } from "vitest";
-import { act, fireEvent, render, screen } from "@testing-library/react";
+import { afterAll, beforeEach, expect, test, vi } from "vitest";
+import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react";
+import { QueryClientProvider } from "@tanstack/react-query";
+import { createQueryClient } from "@/lib/queryClient";
+import { queryKeys } from "@/lib/queries";
+import type { Client } from "@/lib/types";
 import ActivityFilters, { NO_FILTERS, type AppliedFilters } from "./ActivityFilters";
 import { unixToDatetimeLocal } from "./datetime";
 
 vi.stubEnv("TZ", "Europe/Paris");
 afterAll(() => vi.unstubAllEnvs());
 
+function client(ip: string, name: string, learnedName: string): Client {
+	return {
+		id: 1,
+		ip,
+		name,
+		learned_name: learnedName,
+		group_id: 1,
+		group: "default",
+		hand_edited: name !== "",
+		first_seen: 1_700_000_000,
+		last_seen: 1_700_000_100,
+	};
+}
+
+/** Two named clients and one bare address: every case the picker has to show. */
+const CLIENTS: Client[] = [
+	client("192.0.2.10", "Kitchen Pi", "pi.lan"),
+	client("192.0.2.11", "", "laptop.lan"),
+	client("192.0.2.12", "", ""),
+];
+
+/** The picker reads the clients query the query tables already load. */
+beforeEach(() => {
+	vi.stubGlobal(
+		"fetch",
+		vi.fn((input: RequestInfo | URL) => {
+			const url = String(input);
+			if (url === "/api/clients") {
+				return Promise.resolve(
+					new Response(JSON.stringify({ clients: CLIENTS }), {
+						status: 200,
+						headers: { "content-type": "application/json" },
+					}),
+				);
+			}
+			return Promise.resolve(new Response(JSON.stringify({ error: "not stubbed" }), { status: 404 }));
+		}),
+	);
+});
+
+/** A household bigger than the chip row and, at 40, bigger than the 32 cap. */
+function manyClients(count: number): Client[] {
+	return Array.from({ length: count }, (_, index) => client(`198.51.100.${index + 1}`, `Device ${index + 1}`, ""));
+}
+
 /** 02:30 does not exist on this date in Paris; the clock jumps 02:00 to 03:00. */
 const GAP_WALL_TIME = "2026-03-29T02:30:00";
 
@@ -42,7 +91,12 @@ function describedText(input: HTMLElement): string {
  * land them in an order the network and the router can genuinely produce —
  * an early debounce arriving after a later one, over a draft that has moved on.
  */
-function renderFilters(initial: AppliedFilters = NO_FILTERS, deferred = false) {
+function renderFilters(
+	initial: AppliedFilters = NO_FILTERS,
+	deferred = false,
+	primeClients = true,
+	clients: Client[] = CLIENTS,
+) {
 	const patches: Array<Partial<AppliedFilters>> = [];
 	const state: { current: AppliedFilters } = { current: initial };
 	const setter: { current: ((next: AppliedFilters) => void) | null } = { current: null };
@@ -62,12 +116,75 @@ function renderFilters(initial: AppliedFilters = NO_FILTERS, deferred = false) {
 		return <ActivityFilters applied={applied} onApply={onApply} onClear={onClear} />;
 	}
 
-	render(<Harness />);
+	const queryClient = createQueryClient();
+	// Seeded rather than fetched, so the picker has its options on the first
+	// render and a test of the toolbar is not also a test of a request landing.
+	// The one test that is about that arrival opts out and waits for the stub.
+	if (primeClients) queryClient.setQueryData(queryKeys.clients, clients);
+	render(
+		<QueryClientProvider client={queryClient}>
+			<Harness />
+		</QueryClientProvider>,
+	);
 	/** The URL moving under the form: a commit landing, or the back button. */
 	function land(next: AppliedFilters) {
 		act(() => setter.current!(next));
 	}
-	return { patches, state, land };
+	return { patches, state, land, queryClient };
+}
+
+/**
+ * A press, as react-aria hears one. `usePress` works in pointer events, or in
+ * mouse events where jsdom has no `PointerEvent`; a bare `click` is neither, and
+ * RAC's own controls do not respond to it.
+ */
+function press(element: HTMLElement) {
+	fireEvent.mouseDown(element);
+	fireEvent.mouseUp(element);
+	fireEvent.click(element);
+}
+
+/**
+ * The picker's trigger: the first button of the client group, whatever it reads
+ * as. Its label is the selection, so it cannot be looked up by a fixed name.
+ */
+function clientTrigger(): HTMLElement {
+	return within(screen.getByRole("group", { name: "Clients" })).getAllByRole("button")[0] as HTMLElement;
+}
+
+/** The removable chips, in the order they are shown, by what each one reads as. */
+function clientChips(): string[] {
+	return within(screen.getByRole("group", { name: "Clients" }))
+		.getAllByRole("button")
+		.slice(1)
+		.map((chip) => chip.textContent?.replace("\u00d7", "") ?? "");
+}
+
+/**
+ * Opens the client menu, if it is not open already.
+ *
+ * The open state is read from the trigger rather than assumed: an open RAC
+ * popover hides the rest of the page from the accessibility tree — the trigger
+ * included — so a blind second click would either fail to find it or shut the
+ * menu it was meant to open.
+ */
+function openClientMenu() {
+	if (clientTrigger().getAttribute("aria-expanded") === "true") return;
+	fireEvent.click(clientTrigger());
+}
+
+/**
+ * Picks one client and shuts the menu again.
+ *
+ * A multiple-selection menu stays open on a pick, which is the point of it — but
+ * an open RAC popover hides the rest of the page from the accessibility tree, so
+ * a test that wants to read the chips has to close it first, exactly as a reader
+ * would before looking at them.
+ */
+function pickClient(name: string) {
+	openClientMenu();
+	press(screen.getByRole("menuitemcheckbox", { name }));
+	fireEvent.keyDown(screen.getByRole("menu"), { key: "Escape", code: "Escape" });
 }
 
 function openTimeMenu() {
@@ -284,10 +401,7 @@ test("a debounced commit landing late does not roll the field back over newer ty
 		act(() => vi.advanceTimersByTime(400));
 		fireEvent.change(domain, { target: { value: "ads" } });
 		act(() => vi.advanceTimersByTime(400));
-		expect(patches).toEqual([
-			{ domain: "ad", client: undefined },
-			{ domain: "ads", client: undefined },
-		]);
+		expect(patches).toEqual([{ domain: "ad" }, { domain: "ads" }]);
 		fireEvent.change(domain, { target: { value: "adsx" } });
 
 		// The older one lands first. It is this toolbar's own echo, two keystrokes
@@ -352,40 +466,293 @@ test("a window moved from outside clears an error left over from the old one", (
 	expect(screen.queryByRole("alert")).toBeNull();
 });
 
-test("the client field names the exact match it performs", () => {
+test("the picker offers every client and names the filter it is not yet applying", () => {
 	renderFilters();
-	const client = screen.getByLabelText("Client IP (exact match)") as HTMLInputElement;
-	expect(client.getAttribute("placeholder")).toBe("Client IP…");
-});
+	expect(clientTrigger().textContent).toContain("Clients");
+	expect(clientChips()).toEqual([]);
 
-test("an earlier range commit landing late does not unseat the pick that is current", () => {
-	const first = 1_800_000_000_000;
-	vi.spyOn(Date, "now").mockReturnValue(first);
-	const { patches, land } = renderFilters(NO_FILTERS, true);
-
-	openTimeMenu();
-	fireEvent.click(screen.getByRole("menuitem", { name: "Past hour" }));
-
-	// A second pick before the first has come back through the URL.
-	const second = first + 60_000;
-	vi.spyOn(Date, "now").mockReturnValue(second);
-	openTimeMenu();
-	fireEvent.click(screen.getByRole("menuitem", { name: "Past 24 hours" }));
-
-	expect(patches).toEqual([
-		{ since: first / 1000 - 3600, until: first / 1000 },
-		{ since: second / 1000 - 86_400, until: second / 1000 },
+	openClientMenu();
+	// The name is the way in; the address is still the filter.
+	expect(screen.getAllByRole("menuitemcheckbox").map((item) => item.textContent)).toEqual([
+		"192.0.2.12",
+		"Kitchen Pi — 192.0.2.10",
+		"laptop.lan — 192.0.2.11",
 	]);
+});
 
-	// The older window lands first. It is this toolbar's own echo, so it must not
-	// read as a range someone else chose: doing that would clear the preset and
-	// throw the custom row open over a window the reader has already moved past.
-	land({ ...NO_FILTERS, since: first / 1000 - 3600, until: first / 1000 });
-	expect(screen.queryByLabelText("Since")).toBeNull();
+test("picking clients commits them to the url at once, as one comma-separated value", () => {
+	vi.useFakeTimers();
+	try {
+		const { patches, state } = renderFilters();
 
-	land({ ...NO_FILTERS, since: second / 1000 - 86_400, until: second / 1000 });
-	expect(screen.getByRole("button", { name: "Time: Past 24 hours" })).toBeTruthy();
-	expect(screen.queryByLabelText("Since")).toBeNull();
+		pickClient("Kitchen Pi — 192.0.2.10");
+		// No debounce: a pick is a decision the reader has finished making, and
+		// nothing about it can be half-typed.
+		expect(patches).toEqual([{ client: "192.0.2.10" }]);
+		expect(state.current.client).toBe("192.0.2.10");
 
-	vi.restoreAllMocks();
+		pickClient("laptop.lan — 192.0.2.11");
+		expect(patches).toEqual([{ client: "192.0.2.10" }, { client: "192.0.2.10,192.0.2.11" }]);
+		expect(state.current.client).toBe("192.0.2.10,192.0.2.11");
+
+		// Nothing lands later either: there is no pending commit behind these.
+		act(() => vi.advanceTimersByTime(400));
+		expect(patches).toHaveLength(2);
+	} finally {
+		vi.useRealTimers();
+	}
+});
+
+test("the trigger names a single client and counts several", () => {
+	const { state } = renderFilters();
+
+	pickClient("Kitchen Pi — 192.0.2.10");
+	expect(clientTrigger().textContent).toContain("Kitchen Pi — 192.0.2.10");
+
+	pickClient("laptop.lan — 192.0.2.11");
+	expect(clientTrigger().textContent).toContain("2 clients");
+	expect(state.current.client).toBe("192.0.2.10,192.0.2.11");
+});
+
+test("a picked client is a chip, and the chip takes it back off", () => {
+	const { state } = renderFilters();
+
+	pickClient("Kitchen Pi — 192.0.2.10");
+	pickClient("laptop.lan — 192.0.2.11");
+	// A chip is one client beside a row of others, so it wears the short form: the
+	// address is the half that does not fit, and the menu still spells it out.
+	expect(clientChips()).toEqual(["Kitchen Pi", "laptop.lan"]);
+
+	fireEvent.click(screen.getByRole("button", { name: "Remove client Kitchen Pi — 192.0.2.10" }));
+
+	expect(state.current.client).toBe("192.0.2.11");
+	expect(clientChips()).toEqual(["laptop.lan"]);
+});
+
+test("an address no client claims still shows as a chip and can still be removed", () => {
+	// The log names devices the config has never heard of, and a link filtered on
+	// one has to stay readable and clearable even though the menu cannot offer it.
+	const { state } = renderFilters({ ...NO_FILTERS, client: "203.0.113.9,192.0.2.10" });
+
+	expect(clientChips()).toEqual(["203.0.113.9", "Kitchen Pi"]);
+	expect(clientTrigger().textContent).toContain("2 clients");
+
+	fireEvent.click(screen.getByRole("button", { name: "Remove client 203.0.113.9" }));
+	expect(state.current.client).toBe("192.0.2.10");
+});
+
+test("the menu ticks the clients the url names, not the ones last picked here", () => {
+	const { land } = renderFilters();
+	pickClient("Kitchen Pi — 192.0.2.10");
+
+	// The URL moves to a different client from somewhere else.
+	land({ ...NO_FILTERS, client: "192.0.2.11" });
+	expect(clientChips()).toEqual(["laptop.lan"]);
+
+	openClientMenu();
+	const ticked = screen
+		.getAllByRole("menuitemcheckbox")
+		.filter((item) => item.getAttribute("aria-checked") === "true");
+	expect(ticked.map((item) => item.textContent?.replace("✓", ""))).toEqual(["laptop.lan — 192.0.2.11"]);
+});
+
+test("a rename moves the label and leaves the filter on the address", async () => {
+	const { state, queryClient } = renderFilters();
+	pickClient("Kitchen Pi — 192.0.2.10");
+
+	act(() => {
+		queryClient.setQueryData(
+			queryKeys.clients,
+			CLIENTS.map((entry) => (entry.ip === "192.0.2.10" ? { ...entry, name: "Hallway Pi" } : entry)),
+		);
+	});
+
+	// Nothing here holds a label, so nothing here can commit one: the chip is a
+	// rendering of the address, re-rendered.
+	await waitFor(() => expect(clientChips()).toEqual(["Hallway Pi"]));
+	expect(state.current.client).toBe("192.0.2.10");
+});
+
+test("Clear empties the picker along with everything else", () => {
+	const { state } = renderFilters();
+	pickClient("Kitchen Pi — 192.0.2.10");
+	expect(clientChips()).toHaveLength(1);
+
+	fireEvent.click(screen.getByRole("button", { name: "Clear" }));
+
+	expect(clientChips()).toEqual([]);
+	expect(state.current).toEqual(NO_FILTERS);
+});
+
+test("with no clients loaded the trigger is disabled rather than opening on nothing", () => {
+	renderFilters({ ...NO_FILTERS, client: "192.0.2.10" }, false, false);
+
+	expect(clientTrigger().hasAttribute("disabled")).toBe(true);
+	// The filter the URL carries is still on screen and still removable, because
+	// none of that waits on the convenience that names it.
+	expect(clientChips()).toEqual(["192.0.2.10"]);
+});
+
+test("past three chips the rest are a count, and the count removes nothing", () => {
+	const clients = manyClients(12);
+	renderFilters({ ...NO_FILTERS, client: clients.map((entry) => entry.ip).join(",") }, false, true, clients);
+
+	// Three, and then how many more. A dozen chips are not more visible than
+	// three: they wrap the toolbar into a block and push the table off the screen.
+	expect(clientChips()).toEqual(["Device 1", "Device 2", "Device 3"]);
+	expect(screen.getByText("+9 more")).toBeTruthy();
+	expect(clientTrigger().textContent).toContain("12 clients");
+
+	// The summary is not a button, so there is no fourth thing to remove and no
+	// pointer target that does nothing.
+	expect(within(screen.getByRole("group", { name: "Clients" })).getAllByRole("button")).toHaveLength(4);
+	expect(screen.queryByRole("button", { name: /Remove client Device 4/ })).toBeNull();
+});
+
+test("every known client at once commits every one of their addresses", () => {
+	const { patches, state } = renderFilters();
+
+	pickClient("Kitchen Pi — 192.0.2.10");
+	pickClient("laptop.lan — 192.0.2.11");
+	expect(state.current.client).toBe("192.0.2.10,192.0.2.11");
+
+	// The whole household is still written out. What is picked is what is
+	// committed, so the ticks, the chips and the URL all say what the reader just
+	// did — and pruned clients keep history rows, so this is not "no filter".
+	pickClient("192.0.2.12");
+	expect(patches.at(-1)).toEqual({ client: "192.0.2.10,192.0.2.11,192.0.2.12" });
+	expect(state.current.client).toBe("192.0.2.10,192.0.2.11,192.0.2.12");
+	expect(clientChips()).toEqual(["Kitchen Pi", "laptop.lan", "192.0.2.12"]);
+});
+
+test("the only client on the network is still a client you can pick", () => {
+	// The household the owner hit: one known client, so picking it is picking all
+	// of them. A picker that answered that click by clearing itself would read as
+	// a control that did nothing at all.
+	const { state } = renderFilters(NO_FILTERS, false, true, [CLIENTS[0] as Client]);
+
+	pickClient("Kitchen Pi — 192.0.2.10");
+
+	expect(state.current.client).toBe("192.0.2.10");
+	expect(clientChips()).toEqual(["Kitchen Pi"]);
+	expect(clientTrigger().textContent).toContain("Kitchen Pi — 192.0.2.10");
+
+	openClientMenu();
+	expect(screen.getByRole("menuitemcheckbox", { name: "Kitchen Pi — 192.0.2.10" }).getAttribute("aria-checked")).toBe(
+		"true",
+	);
+});
+
+test("the menu stops at the cap the server enforces, and says so", () => {
+	const clients = manyClients(40);
+	const picked = clients.slice(0, 32).map((entry) => entry.ip);
+	const { patches } = renderFilters({ ...NO_FILTERS, client: picked.join(",") }, false, true, clients);
+
+	openClientMenu();
+	expect(screen.getByText("At most 32 clients at a time.")).toBeTruthy();
+	const item = screen.getByRole("menuitemcheckbox", { name: "Device 33 — 198.51.100.33" });
+	expect(item.getAttribute("aria-disabled")).toBe("true");
+
+	// A menu that took a 33rd pick and dropped it would look like it had worked.
+	press(item);
+	expect(patches).toEqual([]);
+
+	// What is already picked can still be unpicked, or the reader would be stuck.
+	const chosen = screen.getByRole("menuitemcheckbox", { name: "Device 1 — 198.51.100.1" });
+	expect(chosen.getAttribute("aria-disabled")).toBeNull();
+});
+
+test("the cap note appears when the cap binds, however few clients are loaded", () => {
+	const clients = manyClients(5);
+	// Two known and thirty unknown: the cap is full while the menu still has three
+	// rows it will not let anyone pick, and a disabled row with nothing to explain
+	// it is the one state this must not reach.
+	const unknown = Array.from({ length: 30 }, (_, index) => `203.0.113.${index + 1}`);
+	renderFilters(
+		{ ...NO_FILTERS, client: [clients[0]!.ip, clients[1]!.ip, ...unknown].join(",") },
+		false,
+		true,
+		clients,
+	);
+
+	openClientMenu();
+	expect(screen.getByText("At most 32 clients at a time.")).toBeTruthy();
+	expect(
+		screen.getByRole("menuitemcheckbox", { name: "Device 3 — 198.51.100.3" }).getAttribute("aria-disabled"),
+	).toBe("true");
+});
+
+test("with room left the cap note stays out of the menu", () => {
+	renderFilters();
+	openClientMenu();
+
+	// Three clients and nothing picked: a ceiling nobody can reach is not news.
+	expect(screen.queryByText("At most 32 clients at a time.")).toBeNull();
+});
+
+test("an address no client claims takes a slot in the cap like any other", () => {
+	const clients = manyClients(40);
+	// Thirty-one known and one the config has never heard of. The cap is on what
+	// the request may name, not on what this menu happens to be able to show.
+	const picked = [...clients.slice(0, 31).map((entry) => entry.ip), "203.0.113.9"];
+	renderFilters({ ...NO_FILTERS, client: picked.join(",") }, false, true, clients);
+
+	openClientMenu();
+	const item = screen.getByRole("menuitemcheckbox", { name: "Device 33 — 198.51.100.33" });
+	expect(item.getAttribute("aria-disabled")).toBe("true");
+});
+
+test("select-all is held to the cap, with the addresses already filtered keeping their slots", () => {
+	const clients = manyClients(40);
+	const unknown = ["203.0.113.1", "203.0.113.2", "203.0.113.3", "203.0.113.4", "203.0.113.5"];
+	const { state } = renderFilters({ ...NO_FILTERS, client: unknown.join(",") }, false, true, clients);
+
+	openClientMenu();
+	// Ctrl+A reaches the selection without pressing an item, so the disabled rows
+	// never see it and the cap has to hold here too.
+	fireEvent.keyDown(screen.getByRole("menu"), { key: "a", code: "KeyA", ctrlKey: true });
+	fireEvent.keyUp(screen.getByRole("menu"), { key: "a", code: "KeyA", ctrlKey: true });
+
+	const applied = state.current.client?.split(",") ?? [];
+	expect(applied).toHaveLength(32);
+	// What was already filtered keeps its slots and the new picks take what is
+	// left, rather than the list being cut wherever it happened to run out.
+	expect(applied.slice(0, 5)).toEqual(unknown);
+	// The rest are known clients, each once. Which 27 is the menu's own order and
+	// not something this test should restate.
+	const rest = applied.slice(5);
+	const addresses = new Set(clients.map((entry) => entry.ip));
+	expect(rest.every((ip) => addresses.has(ip))).toBe(true);
+	expect(new Set(rest).size).toBe(27);
+});
+
+test("removing a chip hands the focus on rather than dropping it", () => {
+	const clients = manyClients(5);
+	const { state } = renderFilters(
+		{
+			...NO_FILTERS,
+			client: clients
+				.slice(0, 4)
+				.map((entry) => entry.ip)
+				.join(","),
+		},
+		false,
+		true,
+		clients,
+	);
+
+	// The chip that takes the removed one's place, so a reader clearing three
+	// clients from the keyboard does not tab back in from the top each time.
+	fireEvent.click(screen.getByRole("button", { name: "Remove client Device 2 — 198.51.100.2" }));
+	expect(document.activeElement?.getAttribute("aria-label")).toBe("Remove client Device 3 — 198.51.100.3");
+
+	// Nothing after it, so the one before it.
+	fireEvent.click(screen.getByRole("button", { name: "Remove client Device 4 — 198.51.100.4" }));
+	fireEvent.click(screen.getByRole("button", { name: "Remove client Device 3 — 198.51.100.3" }));
+	expect(document.activeElement?.getAttribute("aria-label")).toBe("Remove client Device 1 — 198.51.100.1");
+
+	// No chips left, so the control the row belongs to.
+	fireEvent.click(screen.getByRole("button", { name: "Remove client Device 1 — 198.51.100.1" }));
+	expect(document.activeElement).toBe(clientTrigger());
+	expect(state.current.client).toBeUndefined();
 });

admin/src/features/activity/ActivityFilters.tsx

@@ -3,9 +3,9 @@
  *
  * The applied state is the URL, never this form: what the reader sees is what
  * the link they can paste to a housemate will show. So this holds a draft, and
- * every control writes through to the URL — the segments and the time presets
- * at once, the two text fields after a pause so a five-letter domain is one
- * navigation rather than five.
+ * every control writes through to the URL — the segments, the client picker and
+ * the time presets at once, the domain field after a pause so a five-letter word
+ * is one navigation rather than five.
  *
  * A time preset writes both bounds as absolute seconds, resolved once at the
  * click. Neither half may be left open: a bookmark has to describe the same
@@ -34,6 +34,7 @@ import * as stylex from "@stylexjs/stylex";
 import { Button, Menu, MenuItem, MenuTrigger, Popover, Radio, RadioGroup } from "react-aria-components";
 import { styles as shared } from "@/ui/styles";
 import { colors } from "@/ui/tokens.stylex";
+import ClientFilter, { joinClients, parseClients, useClientOptions } from "./ClientFilter";
 import { datetimeField, editDatetimeField, resolveDatetimeField, type DatetimeField } from "./datetime";
 import type { ActivitySearch } from "./search";
 
@@ -90,11 +91,6 @@ const styles = stylex.create({
 		width: "100%",
 		paddingInlineStart: "1.875rem",
 	},
-	clientInput: {
-		flexGrow: 0,
-		flexShrink: 1,
-		flexBasis: "10rem",
-	},
 	/** A button is text-sized by default; this is the hit area around the text. */
 	hitTarget: {
 		minHeight: HIT_TARGET,
@@ -217,12 +213,6 @@ interface Bounds {
 	until: number | undefined;
 }
 
-/** The two text filters as the URL spells them, with absent written as empty. */
-interface TextPair {
-	domain: string;
-	client: string;
-}
-
 function sameBounds(a: Bounds, b: Bounds): boolean {
 	return a.since === b.since && a.until === b.until;
 }
@@ -272,19 +262,22 @@ interface ChosenPreset extends Bounds {
  * What this toolbar knows about the URL, and what it is still waiting to see
  * come back from it.
  *
- * `url` and `bounds` are the last values looked at, so a change can be told
- * apart field by field. `pendingText` and `pendingBounds` are every commit made
- * here that the URL has not echoed yet. Both are queues rather than single
- * slots: two keystrokes either side of the debounce put two text commits in
+ * `domain` and `bounds` are the last values looked at, so a change can be told
+ * apart field by field. `pendingDomain` and `pendingBounds` are every commit
+ * made here that the URL has not echoed yet. Both are queues rather than single
+ * slots: two keystrokes either side of the debounce put two domain commits in
  * flight, and two menu picks in quick succession do the same to the range. In
  * both cases the older echo landing second must not be mistaken for someone
  * else's edit — that is what would clear the preset out from under the pick
  * that is actually current.
+ *
+ * Only the domain needs any of this. Every other control commits on the action
+ * itself, so the URL is never behind what the reader is doing with it.
  */
 interface Sync {
-	url: TextPair;
+	domain: string;
 	bounds: Bounds;
-	pendingText: TextPair[];
+	pendingDomain: string[];
 	pendingBounds: Bounds[];
 }
 
@@ -296,41 +289,41 @@ interface Props {
 }
 
 export default function ActivityFilters({ applied, onApply, onClear }: Props) {
-	const urlText: TextPair = { domain: applied.domain ?? "", client: applied.client ?? "" };
+	const urlDomain = applied.domain ?? "";
 	const urlBounds: Bounds = { since: applied.since, until: applied.until };
 
-	const [domain, setDomain] = useState(urlText.domain);
-	const [client, setClient] = useState(urlText.client);
+	const clientOptions = useClientOptions();
+	// The picked clients are read straight off the URL: there is no draft to hold,
+	// because there is no state here the reader can leave half-finished.
+	const clients = parseClients(applied.client);
+	const [domain, setDomain] = useState(urlDomain);
 	const [preset, setPreset] = useState<ChosenPreset | null>(null);
 	const [showCustom, setShowCustom] = useState(applied.since !== undefined || applied.until !== undefined);
 	const [since, setSince] = useState<DatetimeField>(() => datetimeField(applied.since));
 	const [until, setUntil] = useState<DatetimeField>(() => datetimeField(applied.until));
 	const [error, setError] = useState<BoundError | null>(null);
 	const [sync, setSync] = useState<Sync>({
-		url: urlText,
+		domain: urlDomain,
 		bounds: urlBounds,
-		pendingText: [],
+		pendingDomain: [],
 		pendingBounds: [],
 	});
 
-	const textMoved = sync.url.domain !== urlText.domain || sync.url.client !== urlText.client;
+	const domainMoved = sync.domain !== urlDomain;
 	const boundsMoved = !sameBounds(sync.bounds, urlBounds);
-	if (textMoved || boundsMoved) {
-		let pendingText = sync.pendingText;
+	if (domainMoved || boundsMoved) {
+		let pendingDomain = sync.pendingDomain;
 		let pendingBounds = sync.pendingBounds;
 
-		if (textMoved) {
+		if (domainMoved) {
 			// The newest commit the URL matches, and everything before it, has now
 			// been accounted for; an older one landing later is not new news.
-			const echo = pendingText.findLastIndex(
-				(sent) => sent.domain === urlText.domain && sent.client === urlText.client,
-			);
+			const echo = pendingDomain.findLastIndex((sent) => sent === urlDomain);
 			if (echo >= 0) {
-				pendingText = pendingText.slice(echo + 1);
+				pendingDomain = pendingDomain.slice(echo + 1);
 			} else {
-				pendingText = [];
-				if (sync.url.domain !== urlText.domain) setDomain(urlText.domain);
-				if (sync.url.client !== urlText.client) setClient(urlText.client);
+				pendingDomain = [];
+				setDomain(urlDomain);
 			}
 		}
 
@@ -352,21 +345,18 @@ export default function ActivityFilters({ applied, onApply, onClear }: Props) {
 			}
 		}
 
-		setSync({ url: urlText, bounds: urlBounds, pendingText, pendingBounds });
+		setSync({ domain: urlDomain, bounds: urlBounds, pendingDomain, pendingBounds });
 	}
 
-	const dirty = textFilter(domain) !== applied.domain || textFilter(client) !== applied.client;
+	const dirty = textFilter(domain) !== applied.domain;
 
-	const commitText = useCallback(
+	const commitDomain = useCallback(
 		(replace: boolean) => {
-			const next = { domain: textFilter(domain), client: textFilter(client) };
-			setSync((prev) => ({
-				...prev,
-				pendingText: [...prev.pendingText, { domain: next.domain ?? "", client: next.client ?? "" }],
-			}));
-			onApply(next, replace);
+			const next = textFilter(domain);
+			setSync((prev) => ({ ...prev, pendingDomain: [...prev.pendingDomain, next ?? ""] }));
+			onApply({ domain: next }, replace);
 		},
-		[domain, client, onApply],
+		[domain, onApply],
 	);
 
 	function commitBounds(bounds: Bounds) {
@@ -376,13 +366,13 @@ export default function ActivityFilters({ applied, onApply, onClear }: Props) {
 
 	useEffect(() => {
 		if (!dirty) return;
-		const id = setTimeout(() => commitText(true), DEBOUNCE_MS);
+		const id = setTimeout(() => commitDomain(true), DEBOUNCE_MS);
 		return () => clearTimeout(id);
-	}, [dirty, commitText]);
+	}, [dirty, commitDomain]);
 
 	function flush(event: FormEvent) {
 		event.preventDefault();
-		if (dirty) commitText(true);
+		if (dirty) commitDomain(true);
 	}
 
 	function selectPreset(label: string) {
@@ -439,7 +429,6 @@ export default function ActivityFilters({ applied, onApply, onClear }: Props) {
 
 	function clear() {
 		setDomain("");
-		setClient("");
 		setPreset(null);
 		setShowCustom(false);
 		setSince(datetimeField(undefined));
@@ -447,7 +436,7 @@ export default function ActivityFilters({ applied, onApply, onClear }: Props) {
 		setError(null);
 		setSync((prev) => ({
 			...prev,
-			pendingText: [],
+			pendingDomain: [],
 			pendingBounds: [...prev.pendingBounds, { since: undefined, until: undefined }],
 		}));
 		onClear();
@@ -455,7 +444,7 @@ export default function ActivityFilters({ applied, onApply, onClear }: Props) {
 
 	const active =
 		domain.trim() !== "" ||
-		client.trim() !== "" ||
+		clients.length > 0 ||
 		applied.blocked !== undefined ||
 		applied.since !== undefined ||
 		applied.until !== undefined;
@@ -537,20 +526,10 @@ export default function ActivityFilters({ applied, onApply, onClear }: Props) {
 						{...stylex.props(shared.smallInput, styles.field, styles.searchInput, shared.focusRing)}
 					/>
 				</div>
-				{/*
-				 * The client filter matches one address exactly — the server has no
-				 * substring match for it — so the name says so rather than leaving the
-				 * reader to discover it by typing half an address and getting nothing.
-				 */}
-				<input
-					type="text"
-					aria-label="Client IP (exact match)"
-					placeholder="Client IP…"
-					spellCheck={false}
-					autoComplete="off"
-					value={client}
-					onChange={(event) => setClient(event.target.value)}
-					{...stylex.props(shared.smallInput, styles.field, styles.clientInput, shared.focusRing)}
+				<ClientFilter
+					options={clientOptions}
+					selected={clients}
+					onChange={(next) => onApply({ client: joinClients(next) })}
 				/>
 				<RadioGroup
 					aria-label="Result"

admin/src/features/activity/ClientFilter.tsx

@@ -0,0 +1,365 @@
+/**
+ * The client filter: the loaded clients as a list you pick from, and the picked
+ * ones as chips you can take back off.
+ *
+ * There is nothing to type here, deliberately. The filter is exact — the server
+ * matches whole addresses — so a half-typed address is not a narrower filter but
+ * a wrong one, and a field that applied as you typed emptied the table under
+ * every reader who started with a digit. Choosing from a list cannot be
+ * half-done: every state this control can be in is a filter someone meant.
+ *
+ * Several clients at once, because the question is usually about a group — the
+ * two phones, the television and the console — and one address at a time makes
+ * that several passes over the same window.
+ *
+ * The chips carry the whole selection, including addresses no client claims. The
+ * log names devices the config has never heard of, and those are the ones an
+ * operator is most likely hunting; a link filtered on one has to stay readable
+ * and clearable even though the menu below cannot offer it.
+ *
+ * A list that has not loaded, or failed to, leaves the trigger disabled rather
+ * than opening on nothing. Chips from the URL still show, so the filter stays
+ * visible and removable either way.
+ */
+
+import { CaretDown } from "@phosphor-icons/react";
+import { useEffect, useMemo, useRef } from "react";
+import * as stylex from "@stylexjs/stylex";
+import { Button, Menu, MenuItem, MenuTrigger, Popover } from "react-aria-components";
+import { clientLabel, useClientNames } from "@/features/clients/clientNames";
+import { styles as shared } from "@/ui/styles";
+import { colors } from "@/ui/tokens.stylex";
+import { MAX_CLIENTS } from "./search";
+
+/**
+ * One client as the picker shows it.
+ *
+ * `label` identifies it outright and `name` is the short form. The menu is a
+ * list of every client at once, where two devices can share a name and only the
+ * address tells them apart; a chip is one client the reader picked a moment ago,
+ * beside a row of others, where the address is the part that does not fit.
+ */
+export interface ClientOption {
+	ip: string;
+	label: string;
+	name: string | null;
+}
+
+/** The pointer-target floor `ui/Checkbox` and the dialog Close button already set. */
+const HIT_TARGET = 44;
+
+/**
+ * How many chips are shown before the rest become a count.
+ *
+ * The chips exist so an active filter is visible without opening the menu. A
+ * dozen of them are not more visible than three — they wrap the toolbar into a
+ * block of its own and push the table off the screen — so past this the row says
+ * how many more there are and the menu remains where they are managed.
+ */
+const MAX_CHIPS = 3;
+
+const styles = stylex.create({
+	root: {
+		display: "flex",
+		flexWrap: "wrap",
+		alignItems: "center",
+		gap: "0.25rem",
+	},
+	caret: {
+		display: "inline-flex",
+		color: colors.textMuted,
+	},
+	trigger: {
+		minHeight: HIT_TARGET,
+		minWidth: HIT_TARGET,
+		display: "inline-flex",
+		alignItems: "center",
+		justifyContent: "center",
+		gap: "0.375rem",
+	},
+	popover: {
+		maxHeight: "16rem",
+		overflowY: "auto",
+		borderRadius: "0.25rem",
+		borderWidth: 1,
+		borderStyle: "solid",
+		borderColor: colors.border,
+		backgroundColor: colors.surfaceRaised,
+		color: colors.text,
+		boxShadow: "0 10px 15px -3px rgb(0 0 0 / 0.1), 0 4px 6px -4px rgb(0 0 0 / 0.1)",
+	},
+	menu: {
+		outlineStyle: "none",
+		paddingBlock: "0.25rem",
+	},
+	item: {
+		cursor: "pointer",
+		paddingInline: "0.75rem",
+		fontSize: "0.875rem",
+		lineHeight: "1.25rem",
+		whiteSpace: "nowrap",
+		minHeight: HIT_TARGET,
+		display: "flex",
+		alignItems: "center",
+		gap: "0.5rem",
+	},
+	/** Inset because an item flush against a scrolling popover clips an outset ring. */
+	itemFocused: {
+		backgroundColor: colors.primary,
+		color: colors.primaryText,
+		outlineColor: { default: null, ":focus-visible": colors.primaryText },
+	},
+	/** The tick keeps its column when absent, so the labels do not shift on select. */
+	tick: {
+		width: "0.75rem",
+		flexShrink: 0,
+	},
+	chip: {
+		cursor: "pointer",
+		display: "inline-flex",
+		alignItems: "center",
+		gap: "0.375rem",
+		minHeight: HIT_TARGET,
+		paddingInline: "0.625rem",
+		borderRadius: "999px",
+		borderWidth: 1,
+		borderStyle: "solid",
+		borderColor: colors.border,
+		backgroundColor: { default: colors.surfaceHover, ":hover": colors.surfaceRaised },
+		color: colors.text,
+		fontSize: "0.875rem",
+		lineHeight: "1.25rem",
+	},
+	chipCross: {
+		color: colors.textMuted,
+	},
+	/** Not a button: it removes nothing, and nothing about it is pressable. */
+	chipMore: {
+		display: "inline-flex",
+		alignItems: "center",
+		minHeight: HIT_TARGET,
+		paddingInline: "0.625rem",
+		fontSize: "0.875rem",
+		lineHeight: "1.25rem",
+		color: colors.textMuted,
+	},
+	capNote: {
+		paddingInline: "0.75rem",
+		paddingBlock: "0.375rem",
+		fontSize: "0.75rem",
+		lineHeight: "1rem",
+		color: colors.textMuted,
+	},
+});
+
+/** The em dash pairs the two halves without reading as part of either. */
+function optionLabel(ip: string, name: string | null): string {
+	return name === null ? ip : `${name} — ${ip}`;
+}
+
+/**
+ * The loaded clients, by the name the query tables already give them.
+ *
+ * This is the same cached query those tables read, so opening the list costs no
+ * request; an empty map — still loading, or failed — yields no options.
+ */
+export function useClientOptions(): ClientOption[] {
+	const names = useClientNames();
+	return useMemo(() => {
+		const options = [...names.keys()].map((ip) => {
+			const name = clientLabel(ip, names)?.text ?? null;
+			return { ip, name, label: optionLabel(ip, name) };
+		});
+		return options.sort((a, b) => a.label.localeCompare(b.label));
+	}, [names]);
+}
+
+/** How an address reads in full: its client's line, or the bare address. */
+export function displayFor(ip: string, options: readonly ClientOption[]): string {
+	return options.find((option) => option.ip === ip)?.label ?? ip;
+}
+
+/** How an address reads on a chip: its client's name, or the bare address. */
+export function chipFor(ip: string, options: readonly ClientOption[]): string {
+	return options.find((option) => option.ip === ip)?.name ?? ip;
+}
+
+/**
+ * The URL carries the selection as one comma-separated value; absent means none.
+ *
+ * A plain split, because `validateClients` has already made the value canonical
+ * — trimmed, no blanks, no repeats, within the cap. Dropping anything a second
+ * time here is what would let the chips and the request disagree.
+ */
+export function parseClients(value: string | undefined): string[] {
+	if (value === undefined) return [];
+	return value.split(",");
+}
+
+export function joinClients(ips: readonly string[]): string | undefined {
+	return ips.length === 0 ? undefined : ips.join(",");
+}
+
+/**
+ * The trigger's label. A single client is named outright, because that is the
+ * one case where the whole filter fits on the button; more than one would not,
+ * and the chips beside it say which ones anyway.
+ */
+function triggerLabel(selected: readonly string[], options: readonly ClientOption[]): string {
+	if (selected.length === 0) return "Clients";
+	if (selected.length === 1) return displayFor(selected[0] as string, options);
+	return `${selected.length} clients`;
+}
+
+interface Props {
+	options: readonly ClientOption[];
+	selected: readonly string[];
+	/** Every change here is a decision already made, so it applies at once. */
+	onChange: (next: string[]) => void;
+}
+
+export default function ClientFilter({ options, selected, onChange }: Props) {
+	const known = new Set(options.map((option) => option.ip));
+	// The menu is offered only the addresses it can account for. An address no
+	// client claims is not in its collection, so handing it over as a selected key
+	// would be handing over a key that resolves to nothing.
+	const chosen = new Set(selected.filter((ip) => known.has(ip)));
+	// Every address the URL carries takes a slot, whether or not a client claims
+	// it: the cap is on what the request may name, not on what this menu can show.
+	const atCap = selected.length >= MAX_CLIENTS;
+	// True only when the cap is actually holding something back. Unknown addresses
+	// spend slots too, so a short list of clients can be closed off while the menu
+	// still has room in it — and a disabled row with nothing to explain it is the
+	// one state this must not reach.
+	const capBinds = atCap && chosen.size < options.length;
+
+	const chips = useRef(new Map<string, HTMLButtonElement>());
+	const trigger = useRef<HTMLButtonElement>(null);
+	/** Where focus goes once the removed chip is gone; null means the trigger. */
+	const focusAfterRemoval = useRef<string | null | undefined>(undefined);
+
+	useEffect(() => {
+		const next = focusAfterRemoval.current;
+		if (next === undefined) return;
+		focusAfterRemoval.current = undefined;
+		// Removing a chip unmounts the element that had the focus. Left alone the
+		// browser drops focus to the document, and a reader clearing three clients
+		// from the keyboard would have to tab back in from the top each time.
+		(next === null ? trigger.current : (chips.current.get(next) ?? trigger.current))?.focus();
+	});
+
+	function remove(ip: string) {
+		const next = selected.filter((entry) => entry !== ip);
+		const visible = next.slice(0, MAX_CHIPS);
+		const index = selected.indexOf(ip);
+		// The chip that takes this one's place, or the one before it at the end of
+		// the row, or the trigger when the row is empty.
+		focusAfterRemoval.current = visible[index] ?? visible[index - 1] ?? null;
+		onChange(next);
+	}
+
+	/**
+	 * A selection from the menu, merged back over the addresses the menu could not
+	 * see and turned into what the URL should carry.
+	 *
+	 * What is picked is what is committed, always — even every known client at
+	 * once. The tick, the chip and the URL then say what the reader did, and a
+	 * picker whose feedback for "you selected everything" is to erase the
+	 * selection reads as a control that ignored the click; with one client on the
+	 * network that is every click it will ever get. Nor is the full set a no-op:
+	 * the history keeps rows for clients the inventory has since pruned, so "all
+	 * known clients" and "no filter" are different questions.
+	 *
+	 * The cap is enforced here as well as on the items, because select-all reaches
+	 * this without passing an item at all. What was already filtered keeps its
+	 * slots and the new picks take what is left, so a selection too big to send is
+	 * cut somewhere the reader can predict rather than wherever the URL ran out.
+	 */
+	function apply(next: Set<string>) {
+		const kept = selected.filter((ip) => !known.has(ip) || next.has(ip));
+		const added = [...next].filter((ip) => !chosen.has(ip));
+		onChange([...kept, ...added].slice(0, MAX_CLIENTS));
+	}
+
+	return (
+		// The picker and the chips it fills are one control between them: the group
+		// says so, and names the chips' "Remove …" buttons as part of it.
+		<div role="group" aria-label="Clients" {...stylex.props(styles.root)}>
+			<MenuTrigger>
+				<Button
+					// Nothing to open, and a trigger that opened on an empty popover would
+					// promise a list that is not there.
+					ref={trigger}
+					isDisabled={options.length === 0}
+					className={() => stylex.props(shared.button, styles.trigger, shared.focusRing).className ?? ""}
+				>
+					{triggerLabel(selected, options)}
+					<span aria-hidden="true" {...stylex.props(styles.caret)}>
+						<CaretDown size={12} />
+					</span>
+				</Button>
+				<Popover className={() => stylex.props(styles.popover).className ?? ""}>
+					{/* Said where it is doing something, and nowhere else. */}
+					{capBinds && <p {...stylex.props(styles.capNote)}>At most {MAX_CLIENTS} clients at a time.</p>}
+					<Menu
+						aria-label="Clients"
+						selectionMode="multiple"
+						selectedKeys={chosen}
+						onSelectionChange={(keys) => {
+							// Ctrl/Cmd+A hands back the literal "all"; taking every known
+							// client is its only reading, and apply() caps the result.
+							if (keys === "all") apply(new Set(options.map((option) => option.ip)));
+							else apply(new Set([...keys].map(String)));
+						}}
+						autoFocus={false}
+						{...stylex.props(styles.menu)}
+					>
+						{options.map((option) => (
+							<MenuItem
+								key={option.ip}
+								id={option.ip}
+								// At the cap, what is already picked can still be unpicked and
+								// nothing else can be added. A menu that took a 33rd pick and
+								// dropped it would look like it had worked.
+								isDisabled={atCap && !chosen.has(option.ip)}
+								textValue={option.label}
+								className={({ isFocused }) =>
+									stylex.props(styles.item, shared.insetFocusRing, isFocused && styles.itemFocused)
+										.className ?? ""
+								}
+							>
+								<span aria-hidden="true" {...stylex.props(styles.tick)}>
+									{chosen.has(option.ip) ? "✓" : ""}
+								</span>
+								{option.label}
+							</MenuItem>
+						))}
+					</Menu>
+				</Popover>
+			</MenuTrigger>
+			{selected.slice(0, MAX_CHIPS).map((ip) => (
+				<button
+					key={ip}
+					type="button"
+					ref={(node) => {
+						if (node === null) chips.current.delete(ip);
+						else chips.current.set(ip, node);
+					}}
+					// The chip reads as a name and removes an address, so the name alone
+					// would not say what the button does to a reader who cannot see it.
+					aria-label={`Remove client ${displayFor(ip, options)}`}
+					onClick={() => remove(ip)}
+					{...stylex.props(styles.chip, shared.focusRing)}
+				>
+					{chipFor(ip, options)}
+					<span aria-hidden="true" {...stylex.props(styles.chipCross)}>
+						×
+					</span>
+				</button>
+			))}
+			{selected.length > MAX_CHIPS && (
+				<span {...stylex.props(styles.chipMore)}>+{selected.length - MAX_CHIPS} more</span>
+			)}
+		</div>
+	);
+}

admin/src/features/activity/search.test.ts

@@ -1,4 +1,12 @@
-import { validateActivitySearch, validateBlocked, validateMode, validateText, validateTimestamp } from "./search";
+import {
+	validateActivitySearch,
+	validateBlocked,
+	validateMode,
+	validateText,
+	validateTimestamp,
+	MAX_CLIENTS,
+	validateClients,
+} from "./search";
 
 test("mode is the two-value union, defaulting to history", () => {
 	expect(validateMode("live")).toBe("live");
@@ -111,3 +119,23 @@ test("a search of junk applies nothing", () => {
 		blocked: undefined,
 	});
 });
+
+test("the client list is canonicalized once, so the chips and the request agree", () => {
+	expect(validateClients("192.0.2.10,192.0.2.11")).toBe("192.0.2.10,192.0.2.11");
+
+	// Blanks name no client and a repeat asks for the same client twice, so
+	// neither changes which rows come back: dropping them is the same filter
+	// written once, not a different one.
+	expect(validateClients(" 192.0.2.10 , ,192.0.2.11,192.0.2.10,")).toBe("192.0.2.10,192.0.2.11");
+	expect(validateClients(",,")).toBeUndefined();
+	expect(validateClients("")).toBeUndefined();
+	expect(validateClients(null)).toBeUndefined();
+});
+
+test("a pasted list past the cap is cut to what the api will accept", () => {
+	const addresses = Array.from({ length: MAX_CLIENTS + 8 }, (_, index) => `198.51.100.${index + 1}`);
+
+	// The API refuses a longer list outright, so keeping the extra addresses
+	// would show a filter that cannot be applied at all.
+	expect(validateClients(addresses.join(","))).toBe(addresses.slice(0, MAX_CLIENTS).join(","));
+});

admin/src/features/activity/search.ts

@@ -57,6 +57,37 @@ export function validateText(value: unknown): string | undefined {
 	return trimmed === "" ? undefined : trimmed;
 }
 
+/** `queries_repo.max_clients`: past this the API answers 400 rather than filter. */
+export const MAX_CLIENTS = 32;
+
+/**
+ * The client filter: a comma-separated list of exact addresses, canonicalized
+ * here and nowhere else.
+ *
+ * This is the one place the value is read, so it is the one place it can be made
+ * to mean exactly one thing. Everything downstream — the chips that show the
+ * filter and the request that applies it — reads what this returns, so the two
+ * cannot disagree about a link somebody pasted.
+ *
+ * The rule: entries are trimmed, blanks are dropped, repeats are dropped, and
+ * the list is cut to the cap. A blank entry names no client and a repeat asks
+ * for the same client twice, so neither changes which rows come back; dropping
+ * them is not a different filter, it is the same filter written once. The cut is
+ * a different filter, and it is the honest one available: the API refuses a
+ * longer list outright, so keeping the extra addresses would show a filter that
+ * cannot be applied at all.
+ */
+export function validateClients(value: unknown): string | undefined {
+	if (typeof value !== "string") return undefined;
+	const seen = new Set<string>();
+	for (const entry of value.split(",")) {
+		const trimmed = entry.trim();
+		if (trimmed !== "") seen.add(trimmed);
+		if (seen.size === MAX_CLIENTS) break;
+	}
+	return seen.size === 0 ? undefined : [...seen].join(",");
+}
+
 /**
  * The API filter for a validated search, built field by field.
  *
@@ -82,7 +113,7 @@ export function validateActivitySearch(search: Record<string, unknown>): Activit
 		since: validateTimestamp(search["since"]),
 		until: validateTimestamp(search["until"]),
 		domain: validateText(search["domain"]),
-		client: validateText(search["client"]),
+		client: validateClients(search["client"]),
 		blocked: validateBlocked(search["blocked"]),
 	};
 }