diff --git a/clients/web/src/components/elements/EraBadge/EraBadge.tsx b/clients/web/src/components/elements/EraBadge/EraBadge.tsx index 37c4ebe4..d3caa1d0 100644 --- a/clients/web/src/components/elements/EraBadge/EraBadge.tsx +++ b/clients/web/src/components/elements/EraBadge/EraBadge.tsx @@ -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 ( - + {formatEra(era)} ); diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx index d181c36a..ba4fb132 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx @@ -567,6 +567,168 @@ describe("ConnectionInfoContent", () => { expect(screen.getByText("token-123")).toBeInTheDocument(); }); + it("weights labels bold and values normal, in both halves of the modal", () => { + renderWithMantine( + , + ); + + // The convention is the whole point of #2328: the label is the fixed + // scaffolding a reader scans down, the value is what differs. Asserted in + // Server Implementation *and* OAuth Details, because the defect being + // guarded against is the two halves disagreeing — checking one alone would + // pass on a modal that is internally inconsistent. + 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.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"); + } + }); + + it("gives Client ID and Auth URL the full width, with no inset surface", () => { + renderWithMantine( + , + ); + + // A CIMD client id IS a URL, so it is long by construction. In the + // two-column grid it got half the modal and broke mid-token + // (`…/client-metadata.` / `json`), which reads as a rendering fault rather + // than as one value. + for (const value of [ + "http://127.0.0.1:8093/client-metadata.json?profile=long-enough-to-wrap", + "https://auth.example.com/authorize", + ]) { + expect(screen.getByText(value)).toHaveStyle({ + 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( + , + ); + + // 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", () => { + renderWithMantine( + , + ); + + // Order is the assertion, so read the labels off the DOM rather than + // checking each is merely present: the inline two-column rows come first, + // then every label-over-value field together. Interleaving them is what + // this guards against — it broke the scan down the label column, and the + // tokens already used the full-width layout at the bottom. + const order = [ + "Client registration", + "Scopes", + "Auth URL", + "Client ID", + "Access Token", + ]; + const positions = order.map((label) => { + const node = screen.getAllByText(label)[0]; + expect(node).toBeInTheDocument(); + return ( + node.compareDocumentPosition(screen.getAllByText("Protocol")[0]) & + Node.DOCUMENT_POSITION_PRECEDING + ); + }); + expect(positions.every(Boolean)).toBe(true); + + const labelNode = (label: string) => screen.getAllByText(label)[0]; + for (let i = 0; i < order.length - 1; i++) { + const earlier = labelNode(order[i]); + const later = labelNode(order[i + 1]); + expect( + earlier.compareDocumentPosition(later) & + Node.DOCUMENT_POSITION_FOLLOWING, + ).toBeTruthy(); + } + }); + it("renders EMA idp session when provided", () => { renderWithMantine( void; } -const ValueText = Text.withProps({ +// Label/value pairs in this modal read label-bold, value-normal: the label is +// the fixed scaffolding a reader scans down, and the value is the thing that +// differs per connection. The reverse (which this was until #2328) bolded every +// answer, so nothing stood out and the two columns fought each other. +const FieldLabel = Text.withProps({ size: "sm", fw: 600, }); +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 @@ -104,7 +119,14 @@ const SectionHeading = Title.withProps({ // `Code` block — that keeps the whole value visible and removes a scroll region // that would otherwise need its own keyboard access (axe // `scrollable-region-focusable`). -const ValueCode = Code.withProps({ variant: "wrapping" }); +const ValueCode = Code.withProps({ variant: "wrapping-plain" }); + +// A long OAuth value (client id, auth URL) gets its label on its own line and +// the value across the full modal width beneath it, the way `OAuthTokenField` +// already lays out a token. In the two-column grid these values had roughly +// half the width and wrapped mid-token — `…/client-metadata.` / `json` — which +// reads as a rendering fault rather than as one URL (#2328). +const FullWidthField = Stack.withProps({ gap: 4 }); // One declared sub-option of an extension. Mirrors `CapabilityItem`'s ✓/✗ row // rather than reusing it: that element's `capability` prop is the closed union @@ -306,22 +328,22 @@ export function ConnectionInfoContent({ Server Implementation - Name + Name {displayName} - Version + Version {displayVersion} - Protocol + Protocol {protocolVersion || "—"} - Transport - {transport} + Transport + {transport} - Era - + Era + - Session + Session {formatSession(protocolEra, transport)} @@ -330,7 +352,7 @@ export function ConnectionInfoContent({ Discovery - Supported versions + Supported versions {discoverResult.supportedVersions.length > 0 ? discoverResult.supportedVersions.join(", ") @@ -433,26 +455,17 @@ export function ConnectionInfoContent({ OAuth Details - Protocol + Protocol {formatProtocol(oauth.protocol)} - Status - + Status + {oauth.authorized ? "Authorized" : "Not authorized"} - + - {oauth.clientId && ( - - Client ID - {oauth.clientId} - - )} {oauth.clientRegistrationKind && ( - Client registration + Client registration {formatClientRegistrationKind(oauth.clientRegistrationKind)} @@ -460,22 +473,34 @@ export function ConnectionInfoContent({ )} {oauth.protocol === "ema" && oauth.idpSession && ( - IdP session + IdP session {formatIdpSession(oauth.idpSession)} )} - {oauth.authUrl && ( - - Auth URL - {oauth.authUrl} - - )} {oauth.scopes && oauth.scopes.length > 0 && ( - Scopes + Scopes {formatScopes(oauth.scopes)} )} + {/* The full-width fields are kept together at the end of the + section, directly above the token rows, which use the same + label-over-value layout. Interleaved with the inline two-column + rows they broke the scan down the label column for every row + after them, and Client ID — the one most likely to be long — + was the worst offender (#2328). */} + {oauth.authUrl && ( + + Auth URL + {oauth.authUrl} + + )} + {oauth.clientId && ( + + Client ID + {oauth.clientId} + + )} {oauth.accessToken && ( { + it("renders the default variant", () => { + const { getByText } = renderWithMantine(default); + expect(getByText("default")).toBeTruthy(); + }); + + it("wraps long values in both wrapping variants", () => { + for (const variant of ["wrapping", "wrapping-plain"] as const) { + const { getByText } = renderWithMantine( + {variant}, + ); + const style = getByText(variant).style; + expect(style.wordBreak).toBe("break-all"); + expect(style.whiteSpace).toBe("pre-wrap"); + } + }); + + it("strips the inset surface for wrapping-plain only", () => { + const { getByText: getPlain } = renderWithMantine( + plain, + ); + expect(getPlain("plain").style.backgroundColor).toBe("transparent"); + expect(getPlain("plain").style.padding).toBe("0px"); + + // The guard that makes this change safe: `wrapping` keeps its surface, so + // every other consumer of it is untouched. + const { getByText: getInset } = renderWithMantine( + inset, + ); + expect(getInset("inset").style.backgroundColor).not.toBe("transparent"); + expect(getInset("inset").style.padding).not.toBe("0px"); + }); + + it("clips instead of wrapping for the nowrap variant", () => { + const { getByText } = renderWithMantine(n); + const style = getByText("n").style; + expect(style.whiteSpace).toBe("nowrap"); + expect(style.overflow).toBe("hidden"); + expect(style.textOverflow).toBe("ellipsis"); + }); + + it("drops the margin on a block Code", () => { + const { getByText } = renderWithMantine(b); + expect(getByText("b").style.margin).toBe("0px"); + }); +}); diff --git a/clients/web/src/theme/Code.ts b/clients/web/src/theme/Code.ts index b54a8eee..86efd014 100644 --- a/clients/web/src/theme/Code.ts +++ b/clients/web/src/theme/Code.ts @@ -10,10 +10,20 @@ export const ThemeCode = Code.extend({ backgroundColor: "var(--inspector-code-surface, var(--inspector-surface-code))", }; - if (props.variant === "wrapping") { + if (props.variant === "wrapping" || props.variant === "wrapping-plain") { root.wordBreak = "break-all"; root.whiteSpace = "pre-wrap"; } + if (props.variant === "wrapping-plain") { + // Wraps like `wrapping`, but with no inset surface behind it. Used for + // label/value rows that read as *values* rather than as quoted blobs — + // Connection Info's Client ID and Auth URL. A separate variant rather + // than a tweak to `wrapping`, because that one is shared with panels + // (elicitation URLs, load errors, CSV fallback) where the grey inset is + // doing real work separating a raw value from the prose around it. + root.backgroundColor = "transparent"; + root.padding = "0"; + } if (props.block) { root.margin = "0"; }