admin: title-only information becomes visible text

the client address follows its name as visible muted text in the query tables, the config lock indicator prints its reason beside the tag except in table rows where a page-level note explains the lock instead, and the locked delete buttons describe themselves through that one visible note. the chart legend tooltip is deleted because a named client is deliberately not addressed in the chart, and the dead series address field went with it. titles that merely repeat visible copyable text stay.
This commit is contained in:
2026-08-29 13:03:30 +02:00
parent 207252acee
commit c65d92d8f8
9 changed files with 154 additions and 51 deletions
@@ -163,7 +163,7 @@ test("renders the first page with the seven columns filled in", async () => {
expect(screen.getByText(/Showing 2 queries/)).toBeTruthy(); expect(screen.getByText(/Showing 2 queries/)).toBeTruthy();
}); });
test("resolves each row's client to its display name, keeping the IP as the tooltip", async () => { test("resolves each row's client to its display name, reading the IP out with it", async () => {
stubFetch((url) => { stubFetch((url) => {
if (url === "/api/clients") return json({ clients: CLIENTS }); if (url === "/api/clients") return json({ clients: CLIENTS });
if (url !== "/api/queries") return new Response(JSON.stringify({ error: "not stubbed" }), { status: 404 }); if (url !== "/api/queries") return new Response(JSON.stringify({ error: "not stubbed" }), { status: 404 });
@@ -183,19 +183,22 @@ test("resolves each row's client to its display name, keeping the IP as the tool
// A hand-typed name wins outright; the learned name never surfaces for it. // A hand-typed name wins outright; the learned name never surfaces for it.
const named = await screen.findByText("Kitchen Pi"); const named = await screen.findByText("Kitchen Pi");
expect(named.getAttribute("title")).toBe("192.0.2.10"); // The address reads out with the name it replaced, rather than sitting in a
// title only a mouse can reach.
expect(named.textContent).toBe("Kitchen Pi (192.0.2.10)");
expect(named.getAttribute("title")).toBeNull();
expect(screen.queryByText("pi.lan")).toBeNull(); expect(screen.queryByText("pi.lan")).toBeNull();
// A learned name reads muted and nothing more here: the "learned" tag would // A learned name reads muted and nothing more here: the "learned" tag would
// repeat on every row of the table, so the Clients page carries it instead. // repeat on every row of the table, so the Clients page carries it instead.
const learned = screen.getByText("laptop.lan"); const learned = screen.getByText("laptop.lan");
expect(learned.getAttribute("title")).toBe("192.0.2.11"); expect(learned.textContent).toBe("laptop.lan (192.0.2.11)");
expect(within(learned.closest("tr")!).queryByText("learned")).toBeNull(); expect(within(learned.closest("tr")!).queryByText("learned")).toBeNull();
// A known client with neither name, and a client the loaded list has never // A known client with neither name, and a client the loaded list has never
// seen, both fall back to the bare address with no tooltip standing in. // seen, both fall back to the bare address with nothing standing in for it.
expect(screen.getByText("192.0.2.12").getAttribute("title")).toBeNull(); expect(screen.getByText("192.0.2.12").textContent).toBe("192.0.2.12");
expect(screen.getByText("192.0.2.99").getAttribute("title")).toBeNull(); expect(screen.getByText("192.0.2.99").textContent).toBe("192.0.2.99");
}); });
test("load more appends the next page and stops at the end of the log", async () => { test("load more appends the next page and stops at the end of the log", async () => {
@@ -178,7 +178,7 @@ test("streams rows, flags blocked ones, and freezes the display", async () => {
expect(screen.getByText("later.example")).toBeTruthy(); expect(screen.getByText("later.example")).toBeTruthy();
}); });
test("resolves each row's client to its display name, keeping the IP as the tooltip", async () => { test("resolves each row's client to its display name, reading the IP out with it", async () => {
await openLive(); await openLive();
act(() => { act(() => {
sources[0]!.emit("query", frame(1000, "named.example", { request: { client: "192.0.2.10" } })); sources[0]!.emit("query", frame(1000, "named.example", { request: { client: "192.0.2.10" } }));
@@ -188,11 +188,14 @@ test("resolves each row's client to its display name, keeping the IP as the tool
}); });
const named = await screen.findByText("Kitchen Pi"); const named = await screen.findByText("Kitchen Pi");
expect(named.getAttribute("title")).toBe("192.0.2.10"); // The address reads out with the name it replaced, rather than sitting in a
// title only a mouse can reach.
expect(named.textContent).toBe("Kitchen Pi (192.0.2.10)");
expect(named.getAttribute("title")).toBeNull();
expect(screen.queryByText("pi.lan")).toBeNull(); expect(screen.queryByText("pi.lan")).toBeNull();
const learned = screen.getByText("laptop.lan"); const learned = screen.getByText("laptop.lan");
expect(learned.getAttribute("title")).toBe("192.0.2.11"); expect(learned.textContent).toBe("laptop.lan (192.0.2.11)");
expect(within(learned.closest("tr")!).queryByText("learned")).toBeNull(); expect(within(learned.closest("tr")!).queryByText("learned")).toBeNull();
expect(screen.getByText("192.0.2.12").getAttribute("title")).toBeNull(); expect(screen.getByText("192.0.2.12").getAttribute("title")).toBeNull();
+40 -10
View File
@@ -1,6 +1,6 @@
import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import { fireEvent, render, screen, waitFor, within } from "@testing-library/react";
import { ClientName, type ClientNames } from "./clientNames"; import { ClientName, type ClientNames } from "./clientNames";
import { BASE, MANAGED_FILE, NEVER, renderClientsPage, setConfigStatus } from "./testFixtures"; import { BASE, CLIENTS, MANAGED_FILE, NEVER, renderClientsPage, setConfigStatus } from "./testFixtures";
/** The text of the elements an input points at with `aria-describedby`. */ /** The text of the elements an input points at with `aria-describedby`. */
function describedText(input: HTMLElement): string { function describedText(input: HTMLElement): string {
@@ -187,9 +187,15 @@ test("an unknown group id filters to nothing and offers a way out", async () =>
expect(router.state.location.search).toEqual({}); expect(router.state.location.search).toEqual({});
}); });
// Both statuses lock the declared delete, but only file authority proves the
// file declares the row; the anchors keep the two sentences apart.
const DECLARED_NOTE = /^This client is declared in the configuration file/;
const UNKNOWN_NOTE = /^nxdns cannot say whether this client is declared/;
test("file mode drops every edit affordance and keeps the observed delete live (R2-4)", async () => { test("file mode drops every edit affordance and keeps the observed delete live (R2-4)", async () => {
await renderClientsPage({ ...BASE, "GET /api/config/status": MANAGED_FILE }); await renderClientsPage({ ...BASE, "GET /api/config/status": MANAGED_FILE });
await screen.findAllByLabelText(/Managed by \/etc\/nxdns\/config\.zon/); // The settled sentence, not the tag: "Locked" is already on screen while
// authority is pending, so waiting on it would not wait for this status.
await screen.findAllByText(/^Managed by \/etc\/nxdns\/config\.zon/);
expect(screen.queryAllByRole("button", { name: "Edit" })).toEqual([]); expect(screen.queryAllByRole("button", { name: "Edit" })).toEqual([]);
@@ -197,6 +203,34 @@ test("file mode drops every edit affordance and keeps the observed delete live (
const observed = clientRow("192.168.1.11"); const observed = clientRow("192.168.1.11");
expect((within(declared).getByRole("button", { name: "Delete" }) as HTMLButtonElement).disabled).toBe(true); expect((within(declared).getByRole("button", { name: "Delete" }) as HTMLButtonElement).disabled).toBe(true);
expect((within(observed).getByRole("button", { name: "Delete" }) as HTMLButtonElement).disabled).toBe(false); expect((within(observed).getByRole("button", { name: "Delete" }) as HTMLButtonElement).disabled).toBe(false);
// Why the locked Delete will not answer, in visible text and exactly once:
// per row it would repeat down the whole page, and on the button it was a
// title that a keyboard and a touch screen never reached.
expect(screen.getAllByText(DECLARED_NOTE, { selector: "p" })).toHaveLength(1);
expect(within(declared).queryByText(DECLARED_NOTE, { selector: "p" })).toBeNull();
// The description stays on the button itself too, for a reader on that control.
const locked = within(declared).getByRole("button", { name: "Delete" });
expect(document.getElementById(locked.getAttribute("aria-describedby") ?? "")?.textContent).toMatch(DECLARED_NOTE);
});
test("an all-observed page still says why Edit is gone, with no delete note to carry it", async () => {
// Every row observed, so no Delete is locked. The edit lock is still real, and
// "Locked" appearing with nothing to explain it is the failure this guards.
const observedOnly = { clients: [CLIENTS.clients[1]] };
await renderClientsPage({
...BASE,
"GET /api/clients": observedOnly,
"GET /api/config/status": MANAGED_FILE,
});
// The settled sentence is both the anchor and the assertion: it is the whole
// explanation for the missing Edit action.
await screen.findAllByText(/^Managed by \/etc\/nxdns\/config\.zon/);
expect(await screen.findAllByText("Locked")).not.toHaveLength(0);
// The delete sentence belongs only to a row that has one.
expect(screen.queryByText(DECLARED_NOTE)).toBeNull();
expect((screen.getByRole("button", { name: "Delete" }) as HTMLButtonElement).disabled).toBe(false);
}); });
test("file mode renders network assignments with no mutation control at all (R2-4)", async () => { test("file mode renders network assignments with no mutation control at all (R2-4)", async () => {
@@ -217,7 +251,7 @@ test("file mode renders network assignments with no mutation control at all (R2-
test("a failed config status exposes no configuration mutation, and still deletes an observed client (R3-4)", async () => { test("a failed config status exposes no configuration mutation, and still deletes an observed client (R3-4)", async () => {
await renderClientsPage({ ...BASE, "GET /api/config/status": undefined }); await renderClientsPage({ ...BASE, "GET /api/config/status": undefined });
await screen.findAllByLabelText(/Configuration status unavailable/); await screen.findAllByText(/^Configuration status unavailable/);
expect(screen.queryAllByRole("button", { name: "Edit" })).toEqual([]); expect(screen.queryAllByRole("button", { name: "Edit" })).toEqual([]);
expect(screen.queryByRole("button", { name: "Save assignments" })).toBeNull(); expect(screen.queryByRole("button", { name: "Save assignments" })).toBeNull();
@@ -233,10 +267,6 @@ test("a failed config status exposes no configuration mutation, and still delete
// confirmation is already open. `undefined` is the failed status: the fetch stub // confirmation is already open. `undefined` is the failed status: the fetch stub
// answers 404 for a key it does not hold. // answers 404 for a key it does not hold.
// //
// Both statuses lock the declared delete, but only file authority proves the
// file declares the row; the anchors keep the two sentences apart.
const DECLARED_NOTE = /^This client is declared in the configuration file/;
const UNKNOWN_NOTE = /^nxdns cannot say whether this client is declared/;
describe.each([ describe.each([
["file authority", MANAGED_FILE, DECLARED_NOTE, UNKNOWN_NOTE], ["file authority", MANAGED_FILE, DECLARED_NOTE, UNKNOWN_NOTE],
["a failed status", undefined, UNKNOWN_NOTE, DECLARED_NOTE], ["a failed status", undefined, UNKNOWN_NOTE, DECLARED_NOTE],
@@ -259,7 +289,7 @@ describe.each([
expect(within(dialog).queryByRole("button", { name: "Save" })).toBeNull(); expect(within(dialog).queryByRole("button", { name: "Save" })).toBeNull();
expect((within(dialog).getByLabelText("Name") as HTMLInputElement).value).toBe("laptop"); expect((within(dialog).getByLabelText("Name") as HTMLInputElement).value).toBe("laptop");
expect(within(dialog).getByText(/can no longer be saved/)).toBeTruthy(); expect(within(dialog).getByText(/can no longer be saved/)).toBeTruthy();
expect(within(dialog).getByLabelText(/^Locked\./)).toBeTruthy(); expect(within(dialog).getByText("Locked")).toBeTruthy();
fireEvent.click(within(dialog).getByRole("button", { name: "Cancel" })); fireEvent.click(within(dialog).getByRole("button", { name: "Cancel" }));
await waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); await waitFor(() => expect(screen.queryByRole("dialog")).toBeNull());
@@ -284,7 +314,7 @@ describe.each([
await waitFor(() => expect(within(dialog).queryByRole("button", { name: "Delete" })).toBeNull()); await waitFor(() => expect(within(dialog).queryByRole("button", { name: "Delete" })).toBeNull());
expect(within(dialog).getByText(lockNote)).toBeTruthy(); expect(within(dialog).getByText(lockNote)).toBeTruthy();
expect(within(dialog).queryByText(otherNote)).toBeNull(); expect(within(dialog).queryByText(otherNote)).toBeNull();
expect(within(dialog).getByLabelText(/^Locked\./)).toBeTruthy(); expect(within(dialog).getByText("Locked")).toBeTruthy();
fireEvent.click(within(dialog).getByRole("button", { name: "Cancel" })); fireEvent.click(within(dialog).getByRole("button", { name: "Cancel" }));
await waitFor(() => expect(screen.queryByRole("alertdialog")).toBeNull()); await waitFor(() => expect(screen.queryByRole("alertdialog")).toBeNull());
@@ -364,7 +394,7 @@ test("a pending config status holds the same line as a failed one (R3-4)", async
// The status request never settles, so authority stays pending for the whole // The status request never settles, so authority stays pending for the whole
// test: nothing configuration owns may be offered on that guess. // test: nothing configuration owns may be offered on that guess.
await renderClientsPage({ ...BASE, "GET /api/config/status": NEVER }); await renderClientsPage({ ...BASE, "GET /api/config/status": NEVER });
await screen.findAllByLabelText(/Checking which configuration source/); await screen.findAllByText(/^Checking which configuration source/);
expect(screen.queryAllByRole("button", { name: "Edit" })).toEqual([]); expect(screen.queryAllByRole("button", { name: "Edit" })).toEqual([]);
expect(screen.queryByRole("button", { name: "Save assignments" })).toBeNull(); expect(screen.queryByRole("button", { name: "Save assignments" })).toBeNull();
+30 -6
View File
@@ -10,7 +10,7 @@ import NetworkAssignments from "./NetworkAssignments";
import { ClientDisplayName } from "./clientIdentity"; import { ClientDisplayName } from "./clientIdentity";
import InlineError from "@/lib/InlineError"; import InlineError from "@/lib/InlineError";
import ConfirmDialog from "@/ui/ConfirmDialog"; import ConfirmDialog from "@/ui/ConfirmDialog";
import ConfigLockIndicator from "@/features/configuration/ConfigLockIndicator"; import ConfigLockIndicator, { lockReason } from "@/features/configuration/ConfigLockIndicator";
import { useAuthority, useReadOnlyConfig, type Authority } from "@/features/configuration/authority"; import { useAuthority, useReadOnlyConfig, type Authority } from "@/features/configuration/authority";
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";
@@ -25,6 +25,9 @@ import { colors } from "@/ui/tokens.stylex";
* alone cannot say which operator surface set the row — so the sentence names * alone cannot say which operator surface set the row — so the sentence names
* the doubt rather than asserting a declaration, the way `provenanceOf` does. * the doubt rather than asserting a declaration, the way `provenanceOf` does.
*/ */
/** The one note every locked Delete on this page describes itself with. */
const DELETE_LOCK_NOTE_ID = "clients-delete-locked-note";
function declaredDeleteNote(authority: Authority): string { function declaredDeleteNote(authority: Authority): string {
if (authority.state === "resolved") { if (authority.state === "resolved") {
return "This client is declared in the configuration file; remove it there and restart."; return "This client is declared in the configuration file; remove it there and restart.";
@@ -52,6 +55,15 @@ const styles = stylex.create({
marginTop: "1rem", marginTop: "1rem",
color: colors.textMuted, color: colors.textMuted,
}, },
lockNote: {
marginTop: "1rem",
display: "flex",
flexDirection: "column",
gap: "0.25rem",
fontSize: "0.875rem",
lineHeight: "1.25rem",
color: colors.textMuted,
},
filterBar: { filterBar: {
marginTop: "1rem", marginTop: "1rem",
display: "flex", display: "flex",
@@ -127,6 +139,11 @@ export default function ClientsPage() {
// question: the operator is told why, and only they close the dialog. // question: the operator is told why, and only they close the dialog.
const deleteLocked = pendingDelete !== null && readOnly && pendingDelete.hand_edited; const deleteLocked = pendingDelete !== null && readOnly && pendingDelete.hand_edited;
// The reason a locked Delete will not answer, printed once above the table.
// Per row it would repeat down the whole page; on the button it was a `title`
// that a keyboard and a touch screen never reached.
const deletesLocked = readOnly && rows.some((client) => client.hand_edited);
return ( return (
<section> <section>
<h1 {...stylex.props(styles.heading)}>Clients</h1> <h1 {...stylex.props(styles.heading)}>Clients</h1>
@@ -142,6 +159,15 @@ export default function ClientsPage() {
</Link> </Link>
</div> </div>
)} )}
{/* The whole explanation for this page's locks, printed once. Per row it
would repeat down the table; on the controls it was a `title` that a
keyboard and a touch screen never reached. */}
{readOnly && (
<div {...stylex.props(styles.lockNote)}>
<p>{lockReason(authority)}.</p>
{deletesLocked && <p id={DELETE_LOCK_NOTE_ID}>{declaredDeleteNote(authority)}</p>}
</div>
)}
{clients.length === 0 ? ( {clients.length === 0 ? (
<p {...stylex.props(styles.empty)}> <p {...stylex.props(styles.empty)}>
No clients yet. Rows appear automatically as devices on the network make DNS queries there is No clients yet. Rows appear automatically as devices on the network make DNS queries there is
@@ -187,7 +213,7 @@ export default function ClientsPage() {
{/* Naming a client writes configuration, so the affordance is {/* Naming a client writes configuration, so the affordance is
absent — not disabled — wherever the write cannot land. */} absent — not disabled — wherever the write cannot land. */}
{readOnly ? ( {readOnly ? (
<ConfigLockIndicator /> <ConfigLockIndicator compact />
) : ( ) : (
<button <button
type="button" type="button"
@@ -201,10 +227,8 @@ export default function ClientsPage() {
type="button" type="button"
onClick={() => setPendingDelete(client)} onClick={() => setPendingDelete(client)}
disabled={readOnly && client.hand_edited} disabled={readOnly && client.hand_edited}
title={ aria-describedby={
readOnly && client.hand_edited readOnly && client.hand_edited ? DELETE_LOCK_NOTE_ID : undefined
? declaredDeleteNote(authority)
: undefined
} }
{...stylex.props( {...stylex.props(
shared.smallButton, shared.smallButton,
+13 -3
View File
@@ -16,6 +16,14 @@ import * as stylex from "@stylexjs/stylex";
import { clientsQuery } from "@/lib/queries"; import { clientsQuery } from "@/lib/queries";
import type { Client } from "@/lib/types"; import type { Client } from "@/lib/types";
import { styles as shared } from "@/ui/styles"; import { styles as shared } from "@/ui/styles";
import { colors } from "@/ui/tokens.stylex";
const styles = stylex.create({
/** Secondary to the name it qualifies, and never the only thing in the cell. */
address: {
color: colors.textMuted,
},
});
export type ClientNames = ReadonlyMap<string, Pick<Client, "name" | "learned_name">>; export type ClientNames = ReadonlyMap<string, Pick<Client, "name" | "learned_name">>;
@@ -53,11 +61,13 @@ export function clientLabel(ip: string, names: ClientNames): { text: string; lea
export function ClientName({ ip, names }: { ip: string; names: ClientNames }) { export function ClientName({ ip, names }: { ip: string; names: ClientNames }) {
const label = clientLabel(ip, names); const label = clientLabel(ip, names);
if (label === null) return <span {...stylex.props(shared.mono)}>{ip}</span>; if (label === null) return <span {...stylex.props(shared.mono)}>{ip}</span>;
// The name replaces the address on screen, so the address stays reachable // The name replaces the address, so the address follows it as real text that
// as the tooltip rather than disappearing from the row entirely. // anyone can read and copy. A `title` carried it before, which reaches
// neither a keyboard nor a touch screen.
return ( return (
<span title={ip} {...stylex.props(label.learned && shared.learnedName)}> <span {...stylex.props(label.learned && shared.learnedName)}>
{label.text} {label.text}
<span {...stylex.props(styles.address)}> ({ip})</span>
</span> </span>
); );
} }
@@ -61,17 +61,20 @@ test("the lock is silent when the database owns the configuration", async () =>
test("under file authority the lock names the file, in words a reader hears", async () => { test("under file authority the lock names the file, in words a reader hears", async () => {
renderIndicator(MANAGED_FILE); renderIndicator(MANAGED_FILE);
// The reason is visible text beside the tag, not a title and not a label: a
// tooltip reaches neither a keyboard nor a touch screen, and screen-reader-only
// text is the same failure pointed the other way.
const lock = await screen.findByText("Locked"); const lock = await screen.findByText("Locked");
await waitFor(() => await waitFor(() =>
expect(lock.getAttribute("aria-label")).toBe( expect(screen.getByText(`Managed by ${CONFIG_PATH}; edit the file and restart nxdns`)).toBeTruthy(),
`Locked. Managed by ${CONFIG_PATH}; edit the file and restart nxdns.`,
),
); );
expect(lock.getAttribute("title")).toBeNull();
expect(lock.getAttribute("aria-label")).toBeNull();
}); });
test("an unanswered status still locks, and says that is why", async () => { test("an unanswered status still locks, and says that is why", async () => {
renderIndicator("failed"); renderIndicator("failed");
const lock = await screen.findByText("Locked"); await screen.findByText("Locked");
await waitFor(() => expect(lock.getAttribute("aria-label")).toContain("Configuration status unavailable")); await waitFor(() => expect(screen.getByText(/^Configuration status unavailable/)).toBeTruthy());
}); });
@@ -1,8 +1,20 @@
import * as stylex from "@stylexjs/stylex"; import * as stylex from "@stylexjs/stylex";
import { colors } from "@/ui/tokens.stylex"; import { colors } from "@/ui/tokens.stylex";
import { useAuthority } from "./authority"; import { useAuthority, type Authority } from "./authority";
const styles = stylex.create({ const styles = stylex.create({
row: {
display: "inline-flex",
alignItems: "baseline",
flexWrap: "wrap",
gap: "0.375rem",
},
/** The sentence is secondary to the word, and wraps rather than stretching a row. */
reason: {
fontSize: "0.75rem",
lineHeight: "1rem",
color: colors.textMuted,
},
tag: { tag: {
marginLeft: "0.5rem", marginLeft: "0.5rem",
borderWidth: 1, borderWidth: 1,
@@ -26,20 +38,34 @@ const styles = stylex.create({
* The word is real text, not colour or an icon, so a screen reader announces * The word is real text, not colour or an icon, so a screen reader announces
* the reason the control will not answer. * the reason the control will not answer.
*/ */
export default function ConfigLockIndicator() { /**
* Why a configuration control will not answer. Exported because a page that
* shows the compact tag has to print this sentence itself, once, somewhere the
* tag can point at.
*/
export function lockReason(authority: Authority): string {
if (authority.state === "pending") return "Checking which configuration source this server obeys";
if (authority.state === "failed") return "Configuration status unavailable, so edits are held back";
return `Managed by ${authority.status.path ?? "the configuration file"}; edit the file and restart nxdns`;
}
export default function ConfigLockIndicator({ compact = false }: { compact?: boolean }) {
const authority = useAuthority(); const authority = useAuthority();
if (authority.state === "resolved" && authority.status.authority === "database") return null; if (authority.state === "resolved" && authority.status.authority === "database") return null;
const reason = const reason = lockReason(authority);
authority.state === "pending"
? "Checking which configuration source this server obeys"
: authority.state === "failed"
? "Configuration status unavailable, so edits are held back"
: `Managed by ${authority.status.path ?? "the configuration file"}; edit the file and restart nxdns`;
// A compact caller has no room for the sentence and must print it once
// nearby instead: the Clients table would otherwise repeat it down every row.
if (compact) return <span {...stylex.props(styles.tag)}>Locked</span>;
// Everywhere else the reason is visible text rather than a `title` or an
// `aria-label`. A tooltip reaches neither a keyboard nor a touch screen, and
// screen-reader-only text is the same failure pointed the other way.
return ( return (
<span title={reason} aria-label={`Locked. ${reason}.`} {...stylex.props(styles.tag)}> <span {...stylex.props(styles.row)}>
Locked <span {...stylex.props(styles.tag)}>Locked</span>
<span {...stylex.props(styles.reason)}>{reason}</span>
</span> </span>
); );
} }
+2 -8
View File
@@ -69,8 +69,6 @@ const styles = stylex.create({
interface Series { interface Series {
key: string; key: string;
label: string; label: string;
/** Kept beside the label so a renamed client is still identifiable by address. */
address: string | null;
color: string; color: string;
buckets: number[]; buckets: number[];
} }
@@ -89,15 +87,11 @@ function seriesOf(data: ClientChartData, names: ClientNames): Series[] {
// the same precedence and the same lookup the query tables use. The colour // the same precedence and the same lookup the query tables use. The colour
// keys on the address regardless, so naming a client never repaints it. // keys on the address regardless, so naming a client never repaints it.
label: clientLabel(client.client, names)?.text ?? client.client, label: clientLabel(client.client, names)?.text ?? client.client,
address: client.client,
color: seriesColor(clientKey(client.client)), color: seriesColor(clientKey(client.client)),
buckets: client.buckets, buckets: client.buckets,
})); }));
if (data.other.every((count) => count === 0)) return named; if (data.other.every((count) => count === 0)) return named;
return [ return [...named, { key: OTHER_KEY, label: "Other", color: seriesColor(OTHER_KEY), buckets: data.other }];
...named,
{ key: OTHER_KEY, label: "Other", address: null, color: seriesColor(OTHER_KEY), buckets: data.other },
];
} }
/** /**
@@ -219,7 +213,7 @@ export default function ClientChart({ data }: { data: ClientChartData }) {
)} )}
<ul {...stylex.props(styles.legend)}> <ul {...stylex.props(styles.legend)}>
{series.map((one) => ( {series.map((one) => (
<li key={one.key} title={one.address ?? undefined} {...stylex.props(styles.legendItem)}> <li key={one.key} {...stylex.props(styles.legendItem)}>
<span aria-hidden="true" {...stylex.props(styles.swatch, styles.swatchColor(one.color))} /> <span aria-hidden="true" {...stylex.props(styles.swatch, styles.swatchColor(one.color))} />
{one.label} {one.label}
</li> </li>
@@ -214,6 +214,16 @@ test("a registered client is named in the chart, an unregistered one keeps its a
await waitFor(() => expect(within(chart).getAllByText("kitchen-pi")).toHaveLength(2)); await waitFor(() => expect(within(chart).getAllByText("kitchen-pi")).toHaveLength(2));
expect(within(chart).queryByText("192.0.2.30")).toBeNull(); expect(within(chart).queryByText("192.0.2.30")).toBeNull();
expect(within(chart).getAllByText("192.0.2.31")).toHaveLength(2); expect(within(chart).getAllByText("192.0.2.31")).toHaveLength(2);
// Nor on hover: a pointer-only tooltip would say what the design just chose
// not to, and only to a reader holding a mouse.
const legendItem = within(chart)
.getAllByText("kitchen-pi")
.map((node) => node.closest("li"))
.find((node) => node !== null);
expect(legendItem).toBeTruthy();
expect(legendItem?.getAttribute("title")).toBeNull();
expect(within(chart).getByRole("list").querySelectorAll("[title]")).toHaveLength(0);
}); });
test("a client named only by reverse DNS is named by it too", async () => { test("a client named only by reverse DNS is named by it too", async () => {