diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx index d181c36a..53b225e1 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.test.tsx @@ -567,6 +567,64 @@ 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" }); + } + for (const value of ["Everything Server", "read"]) { + expect(screen.queryByText(value)?.style.fontWeight ?? "").not.toBe("600"); + } + }); + + 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", + }); + } + }); + 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", +}); + // 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 +112,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 +321,22 @@ export function ConnectionInfoContent({ Server Implementation - Name + Name {displayName} - Version + Version {displayVersion} - Protocol + Protocol {protocolVersion || "—"} - Transport + Transport {transport} - Era + Era - Session + Session {formatSession(protocolEra, transport)} @@ -330,7 +345,7 @@ export function ConnectionInfoContent({ Discovery - Supported versions + Supported versions {discoverResult.supportedVersions.length > 0 ? discoverResult.supportedVersions.join(", ") @@ -433,10 +448,10 @@ export function ConnectionInfoContent({ OAuth Details - Protocol + Protocol {formatProtocol(oauth.protocol)} - Status + Status {oauth.clientId && ( - - Client ID + + Client ID {oauth.clientId} - + )} {oauth.clientRegistrationKind && ( - Client registration + Client registration {formatClientRegistrationKind(oauth.clientRegistrationKind)} @@ -460,19 +475,19 @@ export function ConnectionInfoContent({ )} {oauth.protocol === "ema" && oauth.idpSession && ( - IdP session + IdP session {formatIdpSession(oauth.idpSession)} )} {oauth.authUrl && ( - - Auth URL + + Auth URL {oauth.authUrl} - + )} {oauth.scopes && oauth.scopes.length > 0 && ( - Scopes + Scopes {formatScopes(oauth.scopes)} )} diff --git a/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.tsx b/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.tsx index abd5c4a9..d2031938 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/OAuthTokenField.tsx @@ -26,7 +26,10 @@ const CaptionRow = Flex.withProps({ wrap: "nowrap", }); -const Caption = Text.withProps({ size: "sm" }); +// Bold, matching every other field label in Connection Info (#2328) — the +// token rows sit in the same OAuth Details list as Client ID and Scopes, so a +// lighter caption here would break the column a reader scans down. +const Caption = Text.withProps({ size: "sm", fw: 600 }); const Toolbar = Flex.withProps({ gap: 4, diff --git a/clients/web/src/theme/Code.test.tsx b/clients/web/src/theme/Code.test.tsx new file mode 100644 index 00000000..a78fea4b --- /dev/null +++ b/clients/web/src/theme/Code.test.tsx @@ -0,0 +1,60 @@ +import { describe, it, expect } from "vitest"; +import { Code } from "@mantine/core"; +import { renderWithMantine } from "../test/renderWithMantine"; + +// `ThemeCode.extend` is applied transitively wherever a Code renders, but no +// single component exercises every branch of its styles callback. Same reason +// as `Paper.test.tsx` (#1787 brought theme/** under the coverage gate). +// +// The load-bearing assertion here is the `wrapping` / `wrapping-plain` split +// (#2328): `wrapping-plain` exists so Connection Info can drop the inset +// surface behind a label/value pair *without* changing `wrapping`, which +// ElicitationUrlPanel, ListLoadError, MalformedItemsWarning and the CSV +// fallback all rely on to separate a raw value from the prose around it. + +describe("ThemeCode variants", () => { + 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"; }