From f1de80477a8531caec0f25af3b6b7cc1958ed4c8 Mon Sep 17 00:00:00 2001
From: m5r <mokhtar@mial.net>
Date: Sat, 29 Aug 2026 12:33:00 +0200
Subject: [PATCH] admin: group sources and safe search become rac checkboxes

the assigned sources list is a rac checkboxgroup and safe search uses the same drawn checkbox, extracted to a shared ui component with grouped and standalone modes enforced by a discriminated union. the label carries a 44px pointer-target floor on both axes, the focus ring is driven from rac's focus-visible state and guarded by a test, and toggleSource is gone because the group hands back the whole set.
---
 .../configuration/GroupSourcesEditor.tsx      |  41 +++---
 .../configuration/ProtectionGroups.tsx        |  30 ++--
 .../configuration/ProtectionPage.test.tsx     |  23 +++
 .../features/configuration/sourceSet.test.ts  |  16 +-
 admin/src/features/configuration/sourceSet.ts |   5 -
 admin/src/ui/Checkbox.test.tsx                |  92 ++++++++++++
 admin/src/ui/Checkbox.tsx                     | 138 ++++++++++++++++++
 7 files changed, 281 insertions(+), 64 deletions(-)
 create mode 100644 admin/src/ui/Checkbox.test.tsx
 create mode 100644 admin/src/ui/Checkbox.tsx

diff --git a/admin/src/features/configuration/GroupSourcesEditor.tsx b/admin/src/features/configuration/GroupSourcesEditor.tsx
index a0a5291..c13315c 100644
--- a/admin/src/features/configuration/GroupSourcesEditor.tsx
+++ b/admin/src/features/configuration/GroupSourcesEditor.tsx
@@ -3,7 +3,8 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
 import * as stylex from "@stylexjs/stylex";
 import { groupSourcesPutMutation, groupSourcesQuery } from "@/lib/queries";
 import type { Blocklist } from "@/lib/types";
-import { sameSet, toggleSource } from "./sourceSet";
+import { sameSet } from "./sourceSet";
+import Checkbox, { CheckboxGroup } from "@/ui/Checkbox";
 import InlineError from "@/lib/InlineError";
 import { styles as shared } from "@/ui/styles";
 import { colors } from "@/ui/tokens.stylex";
@@ -28,13 +29,6 @@ const styles = stylex.create({
 		flexDirection: "column",
 		gap: "0.25rem",
 	},
-	checkboxLabel: {
-		display: "inline-flex",
-		alignItems: "center",
-		gap: "0.5rem",
-		fontSize: "0.875rem",
-		lineHeight: "1.25rem",
-	},
 	buttonRow: {
 		marginTop: "0.75rem",
 		display: "flex",
@@ -66,21 +60,22 @@ export default function GroupSourcesEditor({ groupId, blocklists }: Props) {
 
 	return (
 		<div {...stylex.props(styles.root)}>
-			<ul {...stylex.props(styles.list)}>
-				{blocklists.map((blocklist) => (
-					<li key={blocklist.id}>
-						<label {...stylex.props(styles.checkboxLabel)}>
-							<input
-								type="checkbox"
-								checked={current.includes(blocklist.id)}
-								onChange={() => setSelected(toggleSource(current, blocklist.id))}
-								{...stylex.props(shared.focusRing)}
-							/>
-							{blocklist.name}
-						</label>
-					</li>
-				))}
-			</ul>
+			{/* The section's own "Assigned sources" heading is the visible label; a
+			    Label here would put the same words on screen twice. Ids cross the
+			    React Aria boundary as strings, the same convention as Select. */}
+			<CheckboxGroup
+				aria-label="Assigned sources"
+				value={current.map(String)}
+				onChange={(values) => setSelected(values.map(Number).sort((a, b) => a - b))}
+			>
+				<ul {...stylex.props(styles.list)}>
+					{blocklists.map((blocklist) => (
+						<li key={blocklist.id}>
+							<Checkbox value={String(blocklist.id)}>{blocklist.name}</Checkbox>
+						</li>
+					))}
+				</ul>
+			</CheckboxGroup>
 			<InlineError error={mutation.error} />
 			<div {...stylex.props(styles.buttonRow)}>
 				<button
diff --git a/admin/src/features/configuration/ProtectionGroups.tsx b/admin/src/features/configuration/ProtectionGroups.tsx
index 0a85f5c..1a41433 100644
--- a/admin/src/features/configuration/ProtectionGroups.tsx
+++ b/admin/src/features/configuration/ProtectionGroups.tsx
@@ -18,6 +18,7 @@ import {
 	rulesQuery,
 } from "@/lib/queries";
 import type { Blocklist, ConfigStatus, Group, Rule, RuleAction, RuleKind } from "@/lib/types";
+import Checkbox from "@/ui/Checkbox";
 import ConfirmDialog from "@/ui/ConfirmDialog";
 import DefinitionList from "@/ui/DefinitionList";
 import Select from "@/ui/Select";
@@ -68,13 +69,6 @@ const styles = stylex.create({
 		alignItems: "center",
 		gap: "0.75rem",
 	},
-	checkboxLabel: {
-		display: "inline-flex",
-		alignItems: "center",
-		gap: "0.5rem",
-		fontSize: "0.875rem",
-		lineHeight: "1.25rem",
-	},
 	spacer: {
 		marginLeft: "auto",
 	},
@@ -355,21 +349,15 @@ function GroupDetailEditable({ group }: { group: Group }) {
 			)}
 
 			<div {...stylex.props(styles.controlRow)}>
-				<label {...stylex.props(styles.checkboxLabel)}>
-					<input
-						type="checkbox"
-						checked={group.safe_search}
-						disabled={update.isPending}
-						{...stylex.props(shared.focusRing)}
-						onChange={(event) =>
-							update.mutate({
-								id: group.id,
-								input: { name: group.name, safe_search: event.target.checked },
-							})
-						}
-					/>
+				<Checkbox
+					isSelected={group.safe_search}
+					isDisabled={update.isPending}
+					onChange={(safeSearch) =>
+						update.mutate({ id: group.id, input: { name: group.name, safe_search: safeSearch } })
+					}
+				>
 					Safe search
-				</label>
+				</Checkbox>
 				<span {...stylex.props(styles.spacer)}>
 					{!renaming && (
 						<button
diff --git a/admin/src/features/configuration/ProtectionPage.test.tsx b/admin/src/features/configuration/ProtectionPage.test.tsx
index ff23c8c..4b3e24d 100644
--- a/admin/src/features/configuration/ProtectionPage.test.tsx
+++ b/admin/src/features/configuration/ProtectionPage.test.tsx
@@ -89,6 +89,29 @@ test("the source assignment saves the full set via PUT", async () => {
 	});
 });
 
+test("the source checkboxes form one named group, and each carries its own state", async () => {
+	await openProtection(2);
+	await screen.findByRole("heading", { name: "kids", level: 2 });
+
+	// The section heading names the group; the boxes belong to it rather than
+	// sitting loose beside the group's other checkboxes.
+	const group = await screen.findByRole("group", { name: "Assigned sources" });
+	const ads = within(group).getByRole("checkbox", { name: "Ads" }) as HTMLInputElement;
+	expect(ads.checked).toBe(false);
+	// Safe search is the group's own field, not one of its sources.
+	expect(within(group).queryByRole("checkbox", { name: "Safe search" })).toBeNull();
+
+	fireEvent.click(ads);
+	await waitFor(() => expect(ads.checked).toBe(true));
+
+	// Discard returns the group to the server's set rather than clearing it.
+	fireEvent.click(screen.getByRole("button", { name: "Discard" }));
+	await waitFor(() =>
+		expect((within(group).getByRole("checkbox", { name: "Ads" }) as HTMLInputElement).checked).toBe(false),
+	);
+	expect(writes("PUT")).toEqual([]);
+});
+
 test("only the selected group's rules are listed", async () => {
 	await openProtection(2);
 	await screen.findByRole("heading", { name: "kids", level: 2 });
diff --git a/admin/src/features/configuration/sourceSet.test.ts b/admin/src/features/configuration/sourceSet.test.ts
index ed09533..f7ef6ec 100644
--- a/admin/src/features/configuration/sourceSet.test.ts
+++ b/admin/src/features/configuration/sourceSet.test.ts
@@ -1,18 +1,4 @@
-import { sameSet, toggleSource } from "./sourceSet";
-
-test("toggleSource adds a missing id keeping ascending order", () => {
-	expect(toggleSource([1, 3], 2)).toEqual([1, 2, 3]);
-	expect(toggleSource([], 5)).toEqual([5]);
-});
-
-test("toggleSource removes a present id", () => {
-	expect(toggleSource([1, 2, 3], 2)).toEqual([1, 3]);
-	expect(toggleSource([5], 5)).toEqual([]);
-});
-
-test("toggleSource twice is a no-op set-wise", () => {
-	expect(toggleSource(toggleSource([1, 2], 3), 3)).toEqual([1, 2]);
-});
+import { sameSet } from "./sourceSet";
 
 test("sameSet compares regardless of order", () => {
 	expect(sameSet([1, 2, 3], [3, 1, 2])).toBe(true);
diff --git a/admin/src/features/configuration/sourceSet.ts b/admin/src/features/configuration/sourceSet.ts
index f239b54..41f2677 100644
--- a/admin/src/features/configuration/sourceSet.ts
+++ b/admin/src/features/configuration/sourceSet.ts
@@ -1,8 +1,3 @@
-export function toggleSource(ids: number[], id: number): number[] {
-	if (ids.includes(id)) return ids.filter((existing) => existing !== id);
-	return [...ids, id].sort((a, b) => a - b);
-}
-
 export function sameSet(a: number[], b: number[]): boolean {
 	if (a.length !== b.length) return false;
 	const sortedA = [...a].sort((x, y) => x - y);
diff --git a/admin/src/ui/Checkbox.test.tsx b/admin/src/ui/Checkbox.test.tsx
new file mode 100644
index 0000000..3188451
--- /dev/null
+++ b/admin/src/ui/Checkbox.test.tsx
@@ -0,0 +1,92 @@
+import { act, fireEvent, render, screen, within } from "@testing-library/react";
+import Checkbox, { CheckboxGroup } from "./Checkbox";
+
+/**
+ * The drawn box, which is the label's one child that does not hold the hidden
+ * input. StyleX compiles to class names and jsdom loads no stylesheet, so the
+ * class list is the only place the composed ring is observable.
+ */
+function indicator(input: HTMLElement): HTMLElement {
+	const label = input.closest("label") as HTMLElement;
+	const spans = [...label.querySelectorAll("span")];
+	const box = spans.find((span) => !span.contains(input));
+	if (box === undefined) throw new Error("the checkbox drew no indicator");
+	return box;
+}
+
+test("a standalone checkbox reports its state and reads its label as its name", () => {
+	const onChange = vi.fn();
+	render(
+		<Checkbox isSelected onChange={onChange}>
+			Safe search
+		</Checkbox>,
+	);
+
+	const box = screen.getByRole("checkbox", { name: "Safe search" }) as HTMLInputElement;
+	expect(box.checked).toBe(true);
+
+	fireEvent.click(box);
+	// The caller owns the state, so the box reports the value it would move to
+	// rather than moving there itself.
+	expect(onChange).toHaveBeenCalledWith(false);
+});
+
+test("a disabled checkbox keeps its state on screen but takes no input", () => {
+	render(
+		<Checkbox isSelected isDisabled onChange={vi.fn()}>
+			Safe search
+		</Checkbox>,
+	);
+
+	const box = screen.getByRole("checkbox", { name: "Safe search" }) as HTMLInputElement;
+	expect(box.checked).toBe(true);
+	// The disabled input is the whole guard: a browser fires no click on one, and
+	// it is out of the tab order. Clicking it here would prove nothing either way,
+	// because `fireEvent` dispatches straight at the node and skips that check.
+	expect(box.disabled).toBe(true);
+});
+
+test("keyboard focus composes the ring onto the drawn box", () => {
+	render(
+		<Checkbox isSelected={false} onChange={vi.fn()}>
+			Ads
+		</Checkbox>,
+	);
+
+	const input = screen.getByRole("checkbox", { name: "Ads" });
+	const box = indicator(input);
+	const idle = box.className.split(" ");
+	expect(input.closest("label")?.getAttribute("data-focus-visible")).toBeNull();
+
+	// React Aria only calls focus visible after the modality is keyboard, which
+	// is why a bare focus() is not enough to raise the ring.
+	act(() => {
+		fireEvent.keyDown(document.body, { key: "Tab" });
+		input.focus();
+	});
+
+	expect(input.closest("label")?.getAttribute("data-focus-visible")).toBe("true");
+	const ringed = box.className.split(" ");
+	// The box gained classes it did not have: the ring is composed onto it, not
+	// merely reported by React Aria on the root.
+	expect(ringed.length).toBeGreaterThan(idle.length);
+	expect(idle.every((name) => ringed.includes(name))).toBe(true);
+});
+
+test("inside a group the group owns the selection, and the group carries the name", () => {
+	const onChange = vi.fn();
+	render(
+		<CheckboxGroup aria-label="Assigned sources" value={["1"]} onChange={onChange}>
+			<Checkbox value="1">Ads</Checkbox>
+			<Checkbox value="2">Trackers</Checkbox>
+		</CheckboxGroup>,
+	);
+
+	const group = screen.getByRole("group", { name: "Assigned sources" });
+	expect((within(group).getByRole("checkbox", { name: "Ads" }) as HTMLInputElement).checked).toBe(true);
+	expect((within(group).getByRole("checkbox", { name: "Trackers" }) as HTMLInputElement).checked).toBe(false);
+
+	// The group reports the whole set, not the box that moved.
+	fireEvent.click(within(group).getByRole("checkbox", { name: "Trackers" }));
+	expect(onChange).toHaveBeenCalledWith(["1", "2"]);
+});
diff --git a/admin/src/ui/Checkbox.tsx b/admin/src/ui/Checkbox.tsx
new file mode 100644
index 0000000..b53cc8c
--- /dev/null
+++ b/admin/src/ui/Checkbox.tsx
@@ -0,0 +1,138 @@
+/**
+ * The checkbox, wrapping React Aria's.
+ *
+ * React Aria hides the real input and leaves the mark to the call site, so the
+ * box below is what the reader sees. It is drawn to the native control's size
+ * so a row that held a native checkbox keeps its height and its baseline.
+ *
+ * One component covers both uses: pass `value` for a box inside a
+ * `CheckboxGroup`, which owns the selection, or `isSelected`/`onChange` for a
+ * standalone box that owns its own.
+ */
+
+import type { ReactNode } from "react";
+import * as stylex from "@stylexjs/stylex";
+import { Checkbox as AriaCheckbox } from "react-aria-components";
+import { colors } from "./tokens.stylex";
+
+/**
+ * Re-exported rather than wrapped: the group adds no styling of its own, and
+ * routing it through here keeps React Aria's checkbox parts to one import site,
+ * the same rule `Select` follows.
+ */
+export { CheckboxGroup } from "react-aria-components";
+
+interface Common {
+	/** The visible label, which is also the accessible name. */
+	children: ReactNode;
+	/** Visible but inert; React Aria also drops it from the tab order. */
+	isDisabled?: boolean;
+}
+
+/** Inside a `CheckboxGroup`, which holds the selection for every box in it. */
+interface Grouped extends Common {
+	value: string;
+	isSelected?: never;
+	onChange?: never;
+}
+
+/** On its own, where the caller holds the state and is told to move it. */
+interface Standalone extends Common {
+	value?: never;
+	isSelected: boolean;
+	onChange: (isSelected: boolean) => void;
+}
+
+/**
+ * The two modes are exclusive: a grouped box that also carried `isSelected`
+ * would have two sources of truth, and a standalone one without `onChange`
+ * could never move. The union is what makes both unrepresentable.
+ */
+type Props = Grouped | Standalone;
+
+const styles = stylex.create({
+	/**
+	 * The whole label is the hit area, so it carries the 44px pointer-target
+	 * floor the dialog's Close button already sets, on both axes. The text is
+	 * 20px tall, and a one-word source name is narrower than 44px again.
+	 */
+	label: {
+		minHeight: 44,
+		minWidth: 44,
+		display: "inline-flex",
+		alignItems: "center",
+		gap: "0.5rem",
+		fontSize: "0.875rem",
+		lineHeight: "1.25rem",
+	},
+	box: {
+		flexShrink: 0,
+		boxSizing: "border-box",
+		width: "0.875rem",
+		height: "0.875rem",
+		display: "inline-flex",
+		alignItems: "center",
+		justifyContent: "center",
+		borderRadius: "0.1875rem",
+		borderWidth: 1,
+		borderStyle: "solid",
+		borderColor: colors.borderStrong,
+		backgroundColor: colors.surfaceRaised,
+	},
+	boxSelected: {
+		borderColor: colors.primary,
+		backgroundColor: colors.primary,
+		color: colors.primaryText,
+	},
+	/**
+	 * The shared ring keys off `:focus-visible`, which lands on the hidden input
+	 * rather than on this box, so the ring follows the state React Aria reports.
+	 */
+	boxFocused: {
+		outlineWidth: 2,
+		outlineStyle: "solid",
+		outlineColor: colors.focus,
+		outlineOffset: 2,
+	},
+	/** The box dims, not the words: the label still has to be readable. */
+	boxDisabled: {
+		opacity: 0.5,
+	},
+});
+
+/** `currentColor` so the mark follows the selected box's foreground token. */
+function CheckMark() {
+	return (
+		<svg aria-hidden="true" viewBox="0 0 12 12" width="10" height="10" fill="none">
+			<path
+				d="M2.5 6.5 5 9l4.5-5.5"
+				stroke="currentColor"
+				strokeWidth="2"
+				strokeLinecap="round"
+				strokeLinejoin="round"
+			/>
+		</svg>
+	);
+}
+
+export default function Checkbox({ children, ...state }: Props) {
+	return (
+		<AriaCheckbox {...state} className={() => stylex.props(styles.label).className ?? ""}>
+			{(renderProps) => (
+				<>
+					<span
+						{...stylex.props(
+							styles.box,
+							renderProps.isSelected && styles.boxSelected,
+							renderProps.isFocusVisible && styles.boxFocused,
+							renderProps.isDisabled && styles.boxDisabled,
+						)}
+					>
+						{renderProps.isSelected && <CheckMark />}
+					</span>
+					{children}
+				</>
+			)}
+		</AriaCheckbox>
+	);
+}