fix: carry the value convention to the badges, and pin the layout it claims
Copilot review round 1 on #2329. Four findings, all correct. The whole-modal claim was false for the three rows whose value is a Badge: `ThemeBadge` defaults to `fw: 600`, so Transport, Era and Status still rendered bold-label/bold-value. A `ValueBadge` constant carries the normal weight for Transport and Status, and `EraBadge` takes an optional `fw` so Connection Info can ask for it without changing the standalone chip in ProtocolListPanel. The chip still reads as a chip — its emphasis is the outline and colour, not the font weight. Three test weaknesses, each of which let a regression through: - The weight assertion used `queryByText(...)?.style.fontWeight ?? ""`, which passes vacuously when the row is absent entirely. Now `getByText`, and deleting a value row fails three tests instead of none. - The full-width test asserted only the transparent background, so swapping `FullWidthField` back for the two-column `SimpleGrid` would still have passed. It now asserts the structure: label and value are siblings, and the value is in no SimpleGrid. - `OAuthTokenField`'s bold caption was asserted nowhere — the weight test renders no token. A case now covers Access Token and ID Token through it. Note for anyone asserting a Badge's weight later: `getByText` lands on the inner label span and `fw` is applied to the root, so the assertion has to walk up or it reads an empty string and passes against anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GfHQ8vMm5NBgzbUJ6UwxmB Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
664c9f8800
commit
c3d9c665a6
3 changed files with 75 additions and 10 deletions
|
|
@ -5,14 +5,22 @@ import { formatEra, isModernEra } from "./eraUtils";
|
|||
export interface EraBadgeProps {
|
||||
/** The negotiated protocol era; `undefined` renders as Legacy. */
|
||||
era: ProtocolEra | undefined;
|
||||
/**
|
||||
* Font weight override. The app-wide `ThemeBadge` defaults to `fw: 600`,
|
||||
* which is right where this badge is a standalone chip (ProtocolListPanel's
|
||||
* header). Connection Info renders it as the *value* half of a label/value
|
||||
* row, where the convention is bold label / normal value, so it passes 400.
|
||||
* Left undefined the theme default applies (#2328).
|
||||
*/
|
||||
fw?: number;
|
||||
}
|
||||
|
||||
// Labels a connection's negotiated protocol era (SEP §7.8). Feed it from
|
||||
// connection state only — see the note in `eraUtils` on why the era must never
|
||||
// be inferred from individual message frames.
|
||||
export function EraBadge({ era }: EraBadgeProps) {
|
||||
export function EraBadge({ era, fw }: EraBadgeProps) {
|
||||
return (
|
||||
<Badge variant="outline" color={isModernEra(era) ? "blue" : "gray"}>
|
||||
<Badge variant="outline" color={isModernEra(era) ? "blue" : "gray"} fw={fw}>
|
||||
{formatEra(era)}
|
||||
</Badge>
|
||||
);
|
||||
|
|
|
|||
|
|
@ -590,8 +590,22 @@ describe("ConnectionInfoContent", () => {
|
|||
for (const label of ["Name", "Protocol", "Client ID", "Scopes"]) {
|
||||
expect(screen.getAllByText(label)[0]).toHaveStyle({ fontWeight: "600" });
|
||||
}
|
||||
// `getByText`, not `queryByText` + `?? ""`: an absent value would make the
|
||||
// optional form pass vacuously (undefined → "" → not "600"), so the test
|
||||
// would go green on a row that had stopped rendering at all.
|
||||
for (const value of ["Everything Server", "read"]) {
|
||||
expect(screen.queryByText(value)?.style.fontWeight ?? "").not.toBe("600");
|
||||
expect(screen.getByText(value).style.fontWeight).not.toBe("600");
|
||||
}
|
||||
|
||||
// Badge values count too. `ThemeBadge` defaults to `fw: 600`, so Status,
|
||||
// Transport and Era read bold-label/bold-value unless overridden — the
|
||||
// whole-modal claim is false without this.
|
||||
for (const badge of ["streamable-http", "Legacy", "Authorized"]) {
|
||||
// `getByText` lands on the Badge's inner label span; `fw` is applied to
|
||||
// the root, so walk up to it or the assertion reads an empty string and
|
||||
// passes against anything.
|
||||
const root = screen.getByText(badge).closest('[class*="Badge-root"]');
|
||||
expect((root as HTMLElement | null)?.style.fontWeight).toBe("400");
|
||||
}
|
||||
});
|
||||
|
||||
|
|
@ -623,6 +637,45 @@ describe("ConnectionInfoContent", () => {
|
|||
backgroundColor: "transparent",
|
||||
});
|
||||
}
|
||||
|
||||
// The background alone does not pin the *layout*: swapping FullWidthField
|
||||
// back for the two-column SimpleGrid would keep it transparent and still
|
||||
// pass. Assert the structure that makes the value full width — label and
|
||||
// value are siblings in a column, and neither sits in a SimpleGrid.
|
||||
for (const [label, value] of [
|
||||
[
|
||||
"Client ID",
|
||||
"http://127.0.0.1:8093/client-metadata.json?profile=long-enough-to-wrap",
|
||||
],
|
||||
["Auth URL", "https://auth.example.com/authorize"],
|
||||
]) {
|
||||
const labelNode = screen.getByText(label);
|
||||
const valueNode = screen.getByText(value);
|
||||
expect(valueNode.parentElement).toBe(labelNode.parentElement);
|
||||
expect(valueNode.closest('[class*="SimpleGrid"]')).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it("bolds the token captions, which are field labels in the same list", () => {
|
||||
renderWithMantine(
|
||||
<ConnectionInfoContent
|
||||
initializeResult={fullResult}
|
||||
clientCapabilities={fullClientCaps}
|
||||
transport="streamable-http"
|
||||
oauth={{
|
||||
protocol: "standard",
|
||||
authorized: true,
|
||||
accessToken: "token-123",
|
||||
idToken: "id-token-456",
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
|
||||
// OAuthTokenField renders its own caption, so the weight convention has to
|
||||
// be asserted through it — the other weight test renders no token at all.
|
||||
for (const caption of ["Access Token", "ID Token"]) {
|
||||
expect(screen.getByText(caption).style.fontWeight).toBe("600");
|
||||
}
|
||||
});
|
||||
|
||||
it("groups the full-width fields at the end, Client ID directly above Access Token", () => {
|
||||
|
|
|
|||
|
|
@ -92,6 +92,13 @@ const ValueText = Text.withProps({
|
|||
size: "sm",
|
||||
});
|
||||
|
||||
// A badge standing in as the *value* half of a label/value row. The app-wide
|
||||
// `ThemeBadge` defaults to `fw: 600`, which is right for a standalone chip but
|
||||
// makes these rows read bold-label/bold-value — the one convention this modal
|
||||
// is not supposed to have. The chip still reads as a chip: its emphasis comes
|
||||
// from the outline and colour, not the font weight (#2328).
|
||||
const ValueBadge = Badge.withProps({ variant: "outline", fw: 400 });
|
||||
|
||||
// Shown for Name/Version when the server didn't report `serverInfo` — an em dash
|
||||
// plus an explicit note so the client-side catalog fallback is never mistaken
|
||||
// for a value the server sent. Exported so tests/stories assert against it
|
||||
|
|
@ -331,10 +338,10 @@ export function ConnectionInfoContent({
|
|||
<ValueText>{protocolVersion || "—"}</ValueText>
|
||||
|
||||
<FieldLabel>Transport</FieldLabel>
|
||||
<Badge variant="outline">{transport}</Badge>
|
||||
<ValueBadge>{transport}</ValueBadge>
|
||||
|
||||
<FieldLabel>Era</FieldLabel>
|
||||
<EraBadge era={protocolEra} />
|
||||
<EraBadge era={protocolEra} fw={400} />
|
||||
|
||||
<FieldLabel>Session</FieldLabel>
|
||||
<ValueText>{formatSession(protocolEra, transport)}</ValueText>
|
||||
|
|
@ -452,12 +459,9 @@ export function ConnectionInfoContent({
|
|||
<ValueText>{formatProtocol(oauth.protocol)}</ValueText>
|
||||
|
||||
<FieldLabel>Status</FieldLabel>
|
||||
<Badge
|
||||
variant="outline"
|
||||
color={oauth.authorized ? "green" : "gray"}
|
||||
>
|
||||
<ValueBadge color={oauth.authorized ? "green" : "gray"}>
|
||||
{oauth.authorized ? "Authorized" : "Not authorized"}
|
||||
</Badge>
|
||||
</ValueBadge>
|
||||
</SimpleGrid>
|
||||
{oauth.clientRegistrationKind && (
|
||||
<SimpleGrid cols={2}>
|
||||
|
|
|
|||
Loading…
Reference in a new issue