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.
This commit is contained in:
2026-08-29 12:33:00 +02:00
parent 3c674966be
commit f1de80477a
7 changed files with 281 additions and 64 deletions
@@ -3,7 +3,8 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import * as stylex from "@stylexjs/stylex"; import * as stylex from "@stylexjs/stylex";
import { groupSourcesPutMutation, groupSourcesQuery } from "@/lib/queries"; import { groupSourcesPutMutation, groupSourcesQuery } from "@/lib/queries";
import type { Blocklist } from "@/lib/types"; 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 InlineError from "@/lib/InlineError";
import { styles as shared } from "@/ui/styles"; import { styles as shared } from "@/ui/styles";
import { colors } from "@/ui/tokens.stylex"; import { colors } from "@/ui/tokens.stylex";
@@ -28,13 +29,6 @@ const styles = stylex.create({
flexDirection: "column", flexDirection: "column",
gap: "0.25rem", gap: "0.25rem",
}, },
checkboxLabel: {
display: "inline-flex",
alignItems: "center",
gap: "0.5rem",
fontSize: "0.875rem",
lineHeight: "1.25rem",
},
buttonRow: { buttonRow: {
marginTop: "0.75rem", marginTop: "0.75rem",
display: "flex", display: "flex",
@@ -66,21 +60,22 @@ export default function GroupSourcesEditor({ groupId, blocklists }: Props) {
return ( return (
<div {...stylex.props(styles.root)}> <div {...stylex.props(styles.root)}>
{/* 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)}> <ul {...stylex.props(styles.list)}>
{blocklists.map((blocklist) => ( {blocklists.map((blocklist) => (
<li key={blocklist.id}> <li key={blocklist.id}>
<label {...stylex.props(styles.checkboxLabel)}> <Checkbox value={String(blocklist.id)}>{blocklist.name}</Checkbox>
<input
type="checkbox"
checked={current.includes(blocklist.id)}
onChange={() => setSelected(toggleSource(current, blocklist.id))}
{...stylex.props(shared.focusRing)}
/>
{blocklist.name}
</label>
</li> </li>
))} ))}
</ul> </ul>
</CheckboxGroup>
<InlineError error={mutation.error} /> <InlineError error={mutation.error} />
<div {...stylex.props(styles.buttonRow)}> <div {...stylex.props(styles.buttonRow)}>
<button <button
@@ -18,6 +18,7 @@ import {
rulesQuery, rulesQuery,
} from "@/lib/queries"; } from "@/lib/queries";
import type { Blocklist, ConfigStatus, Group, Rule, RuleAction, RuleKind } from "@/lib/types"; import type { Blocklist, ConfigStatus, Group, Rule, RuleAction, RuleKind } from "@/lib/types";
import Checkbox from "@/ui/Checkbox";
import ConfirmDialog from "@/ui/ConfirmDialog"; import ConfirmDialog from "@/ui/ConfirmDialog";
import DefinitionList from "@/ui/DefinitionList"; import DefinitionList from "@/ui/DefinitionList";
import Select from "@/ui/Select"; import Select from "@/ui/Select";
@@ -68,13 +69,6 @@ const styles = stylex.create({
alignItems: "center", alignItems: "center",
gap: "0.75rem", gap: "0.75rem",
}, },
checkboxLabel: {
display: "inline-flex",
alignItems: "center",
gap: "0.5rem",
fontSize: "0.875rem",
lineHeight: "1.25rem",
},
spacer: { spacer: {
marginLeft: "auto", marginLeft: "auto",
}, },
@@ -355,21 +349,15 @@ function GroupDetailEditable({ group }: { group: Group }) {
)} )}
<div {...stylex.props(styles.controlRow)}> <div {...stylex.props(styles.controlRow)}>
<label {...stylex.props(styles.checkboxLabel)}> <Checkbox
<input isSelected={group.safe_search}
type="checkbox" isDisabled={update.isPending}
checked={group.safe_search} onChange={(safeSearch) =>
disabled={update.isPending} update.mutate({ id: group.id, input: { name: group.name, safe_search: safeSearch } })
{...stylex.props(shared.focusRing)}
onChange={(event) =>
update.mutate({
id: group.id,
input: { name: group.name, safe_search: event.target.checked },
})
} }
/> >
Safe search Safe search
</label> </Checkbox>
<span {...stylex.props(styles.spacer)}> <span {...stylex.props(styles.spacer)}>
{!renaming && ( {!renaming && (
<button <button
@@ -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 () => { test("only the selected group's rules are listed", async () => {
await openProtection(2); await openProtection(2);
await screen.findByRole("heading", { name: "kids", level: 2 }); await screen.findByRole("heading", { name: "kids", level: 2 });
@@ -1,18 +1,4 @@
import { sameSet, toggleSource } from "./sourceSet"; import { sameSet } 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]);
});
test("sameSet compares regardless of order", () => { test("sameSet compares regardless of order", () => {
expect(sameSet([1, 2, 3], [3, 1, 2])).toBe(true); expect(sameSet([1, 2, 3], [3, 1, 2])).toBe(true);
@@ -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 { export function sameSet(a: number[], b: number[]): boolean {
if (a.length !== b.length) return false; if (a.length !== b.length) return false;
const sortedA = [...a].sort((x, y) => x - y); const sortedA = [...a].sort((x, y) => x - y);
+92
View File
@@ -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"]);
});
+138
View File
@@ -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>
);
}