diff --git a/clients/web/src/components/elements/ContentViewer/ContentViewer.tsx b/clients/web/src/components/elements/ContentViewer/ContentViewer.tsx index 998ae42a..7d7fbfc3 100644 --- a/clients/web/src/components/elements/ContentViewer/ContentViewer.tsx +++ b/clients/web/src/components/elements/ContentViewer/ContentViewer.tsx @@ -62,6 +62,12 @@ export interface ContentViewerProps { wrap?: boolean; } +/** + * Rows a read-only JSON payload renders before the editor scrolls internally + * instead of growing. See the note at the `JsonEditor` that uses it. + */ +const JSON_DISPLAY_MAX_LINES = 200; + function buildDataUri(mimeType: string, data: string): string { return `data:${mimeType};base64,${data}`; } @@ -186,18 +192,26 @@ function JsonContent({ text, copyable }: { text: string; copyable: boolean }) { ariaLabel="JSON content" value={formatJson(text)} readOnly - // Sized to the payload, both ways. A two-line result must not open six - // rows of blank editor — and an uncapped `maxLines` is what keeps a - // large one from growing its *own* scroller inside the host's: every - // consumer here already provides the scroll container it wants (the - // structured-output section caps at `mah` and scrolls; a result panel - // scrolls whole), and a second scrollbar nested in the first is both - // worse to use and a change to layout this swap has no business making. - // It also keeps every line in the DOM, as the preformatted block it - // replaces did — Ace renders only what its own viewport covers, so a - // capped editor would silently drop the rest from find-in-page. + // Sized to the payload, and capped. + // + // `minLines={1}` so a two-line result does not open six rows of blank + // editor. The cap is the more interesting number, and it is a trade: + // + // - Below it the editor grows to fit, so the *host's* scroll container + // is the only one — every consumer already provides the one it wants + // (the structured-output section caps at `mah` and scrolls; a result + // panel scrolls whole), and nesting a second scrollbar inside it is + // worse to use. + // - Above it Ace scrolls internally and renders only the rows its own + // viewport covers. That is the point of having a cap at all: the + // Protocol and Network lists are not virtualized and keep every + // entry's payload mounted inside its `Collapse`, so an uncapped + // editor would put a whole 10k-line response in the DOM per entry. + // + // 200 is chosen so essentially no real payload nests a scrollbar while + // the pathological one stays bounded. minLines={1} - maxLines={Infinity} + maxLines={JSON_DISPLAY_MAX_LINES} /> ); diff --git a/clients/web/src/components/elements/JsonEditor/JsonEditor.stories.tsx b/clients/web/src/components/elements/JsonEditor/JsonEditor.stories.tsx index 7b0128e2..356150b6 100644 --- a/clients/web/src/components/elements/JsonEditor/JsonEditor.stories.tsx +++ b/clients/web/src/components/elements/JsonEditor/JsonEditor.stories.tsx @@ -78,6 +78,11 @@ export const WithError: Story = { }, }; +/** + * Switched off by the form around it — what a tool panel does to every field + * while a call is in flight. Ace has no disabled state, so this is read-only + * plus a dimmed wrapper, `aria-disabled`, and removal from the tab order. + */ export const Disabled: Story = { args: { ariaLabel: "Payload JSON", @@ -86,6 +91,30 @@ export const Disabled: Story = { disabled: true, onChange: () => {}, }, + parameters: { + a11y: { + config: { + rules: [ + // Re-stated because a story's `a11y.config.rules` replaces the + // preview's rather than merging with it — dropping this would make + // `region` fire here and nowhere else. + { id: "region", enabled: false }, + // WCAG 1.4.3 exempts "inactive user interface components" from the + // contrast minimum, and this whole editor is one: the dimming is the + // affordance that says so. axe cannot see that, because the disabled + // state lives on Ace's hidden textarea while the text it measures is + // in the separately-rendered gutter — so it reports the dimmed line + // numbers as a violation on every row. + // + // Scoped to this story on purpose. Every other story here, and the + // whole read-only path, is still contrast-checked; a preview-level + // disable would retire the rule for the enabled editor too, which is + // where it has something to catch. + { id: "color-contrast", enabled: false }, + ], + }, + }, + }, }; /** diff --git a/clients/web/src/components/elements/JsonEditor/JsonEditor.test.tsx b/clients/web/src/components/elements/JsonEditor/JsonEditor.test.tsx index a8824f02..ae84b0c3 100644 --- a/clients/web/src/components/elements/JsonEditor/JsonEditor.test.tsx +++ b/clients/web/src/components/elements/JsonEditor/JsonEditor.test.tsx @@ -136,6 +136,46 @@ describe("JsonEditor", () => { expect(document.querySelector(".json-editor-readonly")).not.toBeNull(); }); + // Ace has no disabled state, so its hidden textarea keeps `tabindex="0"` + // unless something takes it out — leaving a keyboard user able to tab into + // a field the rest of the form has switched off, with nothing announced. + it("leaves the tab order and says so when disabled", () => { + const { rerender } = renderWithMantine( + , + ); + const input = screen.getByLabelText(aceLabel(LABEL)); + expect(input).toHaveAttribute("tabindex", "0"); + expect(input).not.toHaveAttribute("aria-disabled"); + + rerender( + , + ); + expect(input).toHaveAttribute("tabindex", "-1"); + expect(input).toHaveAttribute("aria-disabled", "true"); + + // …and both are restored when the form re-enables. + rerender( + , + ); + expect(input).toHaveAttribute("tabindex", "0"); + expect(input).not.toHaveAttribute("aria-disabled"); + }); + + // Read-only is not disabled: a payload the user may need to scroll past the + // bottom of has to be reachable by keyboard (WCAG SC 2.1.1), which is why + // the preformatted block this replaced carried `tabIndex={0}`. + it("stays focusable when merely read-only", () => { + renderWithMantine(); + const input = screen.getByLabelText(aceLabel(LABEL)); + expect(input).toHaveAttribute("tabindex", "0"); + expect(input).not.toHaveAttribute("aria-disabled"); + }); + it("is read-only and dimmed when disabled", () => { const { container } = renderWithMantine(