From 12c00d8c63288fd515bd24f60699737481605cbf Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 27 Aug 2026 03:16:44 -0400 Subject: [PATCH] fix(web): re-apply the editor's accessible name, and correct the README's scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot's third pass generated no new inline comments; all three of its suppressed ones were right. - `ariaLabel` is now re-applied from the wiring effect, not only at mount. `setOptions` updates Ace's option when the prop changes, but Ace recomputes the textarea's `aria-label` from it inside `TextInput.setAriaLabel()`, which it calls when the cursor moves — so a name that changed while the editor stayed mounted (a tool refreshed in place under a new field title, which `SchemaJsonField` passes straight through) left the visible label and the accessible name disagreeing until the user clicked into the box. `onLoad` keeps doing it too, since an effect runs after paint and this is the control's only name. - The README claimed this editor is on "every surface where JSON is typed or displayed", which is false in two places it now names instead: `ExperimentalFeaturesPanel`'s JSON-RPC *response* is still a read-only `Textarea` (only its request box moved), and `OAuthTokenField` renders a decoded JWT in a `Code` block beside the raw token. Both bypass `ContentViewer` and so are not reached by its JSON branch. Documenting them as exceptions is what keeps the section from sending a maintainer to the wrong abstraction. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UL9retfvAXRgvi6EWk4SY4 Signed-off-by: cliffhall --- clients/web/README.md | 6 +++-- .../elements/JsonEditor/JsonEditor.test.tsx | 19 ++++++++++++++++ .../elements/JsonEditor/JsonEditor.tsx | 22 ++++++++++++++++--- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/clients/web/README.md b/clients/web/README.md index 02fe4d89..0fa61943 100644 --- a/clients/web/README.md +++ b/clients/web/README.md @@ -285,7 +285,7 @@ Each customized Mantine component has a `Theme.ts` file (`Button.ts`, `Tex ## JSON editing and display (`JsonEditor`) -Every surface in this client where JSON is typed or displayed renders one element: **`elements/JsonEditor`**, an **Ace** editor (`react-ace` + `ace-builds`, declared in this client because they render React). Ace brings code folding, line numbers, brace auto-closing, and per-line error annotation from its JSON worker — the last three are why hand-writing a nested payload in a bare textarea was the actual pain ([#2151](https://github.com/modelcontextprotocol/inspector/issues/2151)). +Every surface in this client where JSON is **typed**, plus `ContentViewer`'s read-only JSON branch, renders one element: **`elements/JsonEditor`**, an **Ace** editor (`react-ace` + `ace-builds`, declared in this client because they render React). Ace brings code folding, line numbers, brace auto-closing, and per-line error annotation from its JSON worker — the last three are why hand-writing a nested payload in a bare textarea was the actual pain ([#2151](https://github.com/modelcontextprotocol/inspector/issues/2151)). `JsonEditor` is deliberately **text in, text out**: it never parses. The editing contracts above it disagree about what an unparseable draft means, and neither can be expressed by a component that decides for them: @@ -298,7 +298,9 @@ Every surface in this client where JSON is typed or displayed renders one elemen | `groups/ImportServerJsonPanel`, `groups/ExperimentalFeaturesPanel` | The panel owns the text and validates it itself. | | `elements/ContentViewer`'s JSON branch | Read-only — see below. | -**Read-only mode is what `ContentViewer` renders JSON as**, and through it every JSON payload in the app: Protocol and Network entries, tool results, structured output, resource previews, server cards. Highlighting is *not* what that buys — JSON already highlighted, via the lazily-imported Prism grammar `CodeHighlight` loads. What Ace adds is **folding**, line numbers and a gutter on a large payload. The Prism `json` grammar was dropped in the same change rather than kept beside it: two highlighters for one language drift, and nothing else asks for `json`. Two cases stay on the plain renderer — a `wrap={false}` caller (the server card's fixed-height, single-line box) and untyped text that only *looks* like JSON but does not parse, which in an editor would frame a server's prose as a malformed document. +**Read-only mode is what `ContentViewer` renders JSON as**, and through it every payload that reaches the app that way: Protocol and Network entries, tool results, structured output, resource previews, server cards. Highlighting is *not* what that buys — JSON already highlighted, via the lazily-imported Prism grammar `CodeHighlight` loads. What Ace adds is **folding**, line numbers and a gutter on a large payload. The Prism `json` grammar was dropped in the same change rather than kept beside it: two highlighters for one language drift, and nothing else asks for `json`. Two cases stay on the plain renderer — a `wrap={false}` caller (the server card's fixed-height, single-line box) and untyped text that only *looks* like JSON but does not parse, which in an editor would frame a server's prose as a malformed document. + +Two read-only JSON displays deliberately do **not** go through `ContentViewer`, and so are not on this editor — worth knowing before assuming it is the route for all of them. `ExperimentalFeaturesPanel` renders its JSON-RPC **response** in a read-only `Textarea` (its *request* box is on the editor), and `ConnectionInfoContent/OAuthTokenField` renders a decoded JWT in a `Code` block beside the raw token, where the decoded/raw toggle and the copy affordance belong to the field rather than to a viewer. Both are candidates for the same treatment; neither is in [#2151](https://github.com/modelcontextprotocol/inspector/issues/2151)'s scope. Five integration details are load-bearing: diff --git a/clients/web/src/components/elements/JsonEditor/JsonEditor.test.tsx b/clients/web/src/components/elements/JsonEditor/JsonEditor.test.tsx index ae84b0c3..f50b36b0 100644 --- a/clients/web/src/components/elements/JsonEditor/JsonEditor.test.tsx +++ b/clients/web/src/components/elements/JsonEditor/JsonEditor.test.tsx @@ -36,6 +36,25 @@ describe("JsonEditor", () => { expect(screen.getByLabelText(aceLabel(LABEL))).toBeInTheDocument(); }); + // Ace recomputes the textarea's `aria-label` from its option only when the + // cursor moves, so a name that changes while the editor stays mounted — a + // tool refreshed in place under a new field title — would leave the visible + // label and the accessible name disagreeing until the user clicked into it. + it("re-applies the accessible name when it changes in place", () => { + const { rerender } = renderWithMantine( + , + ); + expect(screen.getByLabelText(aceLabel("Series JSON"))).toBeInTheDocument(); + + rerender( + , + ); + expect( + screen.getByLabelText(aceLabel("Data points JSON")), + ).toBeInTheDocument(); + expect(screen.queryByLabelText(aceLabel("Series JSON"))).toBeNull(); + }); + it("shows the text it is given, verbatim", () => { // Braces, not a bare JSX string: a JSX string attribute does not process // escapes, so `'…\n…'` written inline would be a literal backslash-n. diff --git a/clients/web/src/components/elements/JsonEditor/JsonEditor.tsx b/clients/web/src/components/elements/JsonEditor/JsonEditor.tsx index 17f436cd..751c1ecf 100644 --- a/clients/web/src/components/elements/JsonEditor/JsonEditor.tsx +++ b/clients/web/src/components/elements/JsonEditor/JsonEditor.tsx @@ -149,12 +149,28 @@ export function JsonEditor({ // wrapper's own id is what makes the `