diff --git a/clients/web/src/App.css b/clients/web/src/App.css index f0435662..0e89cde2 100644 --- a/clients/web/src/App.css +++ b/clients/web/src/App.css @@ -182,6 +182,62 @@ } } +/* ── Accordion disclosure chevron (right when closed, down when open) ── + * + * Mantine's default chevron rotation flips 180° on open (e.g. down → up). For + * the `disclosure` Accordion variant we use a right-pointing chevron and rotate + * it only 90° so closed sections point right and open sections point down — the + * standard disclosure direction, matching ResourceLink's chevron (issue #1462). + * `[data-rotate]` is set by Mantine on the chevron slot when the item is open; + * the class+attribute selector outranks Mantine's `:where()`-wrapped default. */ + +/* Restore a smooth rotation: the accordion sets `transitionDuration={0}` to + * disable its panel height animation (which fights the flex layout), and that + * zeroes the shared `--accordion-transition-duration` the chevron would use. + * The 200ms restates Mantine's default accordion duration — keep it in sync if + * accordion timing is ever themed. */ +.disclosure-chevron { + transition: transform 200ms ease; +} + +.disclosure-chevron[data-rotate] { + transform: rotate(90deg); +} + +/* ── Full-height sectioned accordion (Resources) ───────────────── + * + * The `disclosure` Accordion fills its full-height card as a flex column. The + * section controls (headers) stay pinned while each OPEN section's panel scrolls + * within its own share of the remaining height — the per-item `flex` weight + * (set in ResourceControls) hands space from long sections to short ones. With + * `flex-grow: 0`, nothing expands or scrolls until the combined content + * overflows the panel, so sections never scroll while the panel has free space + * (issue #1462). Closed sections collapse to their header height. */ + +.disclosure-sections { + display: flex; + flex-direction: column; + min-height: 0; +} + +.disclosure-sections > .mantine-Accordion-item[data-active] { + display: flex; + flex-direction: column; + min-height: 0; +} + +.disclosure-sections .mantine-Accordion-control { + flex: 0 0 auto; +} + +.disclosure-sections + > .mantine-Accordion-item[data-active] + .mantine-Accordion-panel { + flex: 1 1 auto; + min-height: 0; + overflow-y: auto; +} + /* ── List item hover ────────────────────────────────────── */ .list-item:hover { diff --git a/clients/web/src/components/groups/ResourceControls/ResourceControls.stories.tsx b/clients/web/src/components/groups/ResourceControls/ResourceControls.stories.tsx index 73518d52..a35420d8 100644 --- a/clients/web/src/components/groups/ResourceControls/ResourceControls.stories.tsx +++ b/clients/web/src/components/groups/ResourceControls/ResourceControls.stories.tsx @@ -4,9 +4,24 @@ import type { ResourceTemplate, } from "@modelcontextprotocol/sdk/types.js"; import type { InspectorResourceSubscription } from "../../../../../../core/mcp/types.js"; -import { fn } from "storybook/test"; +import { expect, fn, within } from "storybook/test"; import { ResourceControls } from "./ResourceControls"; +// rotate(90deg) — Mantine sets `data-rotate` on the chevron slot of an open +// section; App.css rotates the right-pointing arrow 90° so it points down. +const ROTATED_DOWN = "matrix(0, 1, -1, 0, 0, 0)"; +// Identity — a closed section's right-pointing chevron at its natural 0°. +const NOT_ROTATED = "matrix(1, 0, 0, 1, 0, 0)"; + +function chevronTransforms(canvasElement: HTMLElement): string[] { + // Scoped to the accordion root so it still works if a story ever renders more + // than one disclosure accordion on the canvas. + const root = canvasElement.querySelector(".disclosure-sections"); + return [ + ...(root ?? canvasElement).querySelectorAll(".disclosure-chevron"), + ].map((c) => getComputedStyle(c).transform); +} + const meta: Meta = { title: "Groups/ResourceControls", component: ResourceControls, @@ -65,6 +80,13 @@ export const Default: Story = { templates: sampleTemplates, subscriptions: sampleSubscriptions, }, + // All three populated sections open by default (compact=false) → every + // chevron points down. + play: async ({ canvasElement }) => { + const transforms = chevronTransforms(canvasElement); + expect(transforms).toHaveLength(3); + for (const t of transforms) expect(t).toBe(ROTATED_DOWN); + }, }; export const WithSearch: Story = { @@ -87,3 +109,54 @@ export const Empty: Story = { subscriptions: [], }, }; + +// Sections explicitly collapsed: every chevron points right (no rotation). +export const Collapsed: Story = { + args: { + resources: sampleResources, + templates: sampleTemplates, + subscriptions: sampleSubscriptions, + openSections: [], + }, + play: async ({ canvasElement }) => { + const transforms = chevronTransforms(canvasElement); + expect(transforms).toHaveLength(3); + // Identity matrix = the right-pointing arrow's natural 0° (not rotated). + for (const t of transforms) expect(t).toBe(NOT_ROTATED); + }, +}; + +// Populated URIs/Templates but no Subscriptions, all "open" in intent. The +// empty Subscriptions section is kept out of the open set, so its chevron +// points right while the two populated ones point down. +export const EmptySectionCollapsed: Story = { + args: { + resources: sampleResources, + templates: sampleTemplates, + subscriptions: [], + openSections: ["resources", "templates", "subscriptions"], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect(canvas.getByText("Subscriptions (0)")).toBeInTheDocument(); + const transforms = chevronTransforms(canvasElement); + const downCount = transforms.filter((t) => t === ROTATED_DOWN).length; + expect(downCount).toBe(2); // URIs + Templates down, Subscriptions right + }, +}; + +// Many URIs with sparse Templates/Subscriptions: under the old equal `/ n` +// height split, URIs scrolled while the others left their share unused. Now the +// sections size to content inside one bounded scroll region. +const manyResources: Resource[] = Array.from({ length: 30 }, (_, i) => ({ + name: `resource-${String(i + 1).padStart(2, "0")}.md`, + uri: `file:///docs/resource-${i + 1}.md`, +})); + +export const ManyResources: Story = { + args: { + resources: manyResources, + templates: sampleTemplates, + subscriptions: sampleSubscriptions, + }, +}; diff --git a/clients/web/src/components/groups/ResourceControls/ResourceControls.test.tsx b/clients/web/src/components/groups/ResourceControls/ResourceControls.test.tsx index 1ee1e07a..261833d4 100644 --- a/clients/web/src/components/groups/ResourceControls/ResourceControls.test.tsx +++ b/clients/web/src/components/groups/ResourceControls/ResourceControls.test.tsx @@ -222,6 +222,51 @@ describe("ResourceControls", () => { expect(onCompactChange).toHaveBeenLastCalledWith(false); }); + it("keeps an empty section collapsed even when it's in openSections", () => { + // All three sections requested open, but Subscriptions has no items: its + // control must render collapsed (aria-expanded=false) so the chevron points + // right, while the populated sections stay expanded (#1462). + renderWithMantine( + , + ); + expect(screen.getByRole("button", { name: /URIs \(2\)/ })).toHaveAttribute( + "aria-expanded", + "true", + ); + expect( + screen.getByRole("button", { name: /Templates \(1\)/ }), + ).toHaveAttribute("aria-expanded", "true"); + expect( + screen.getByRole("button", { name: /Subscriptions \(0\)/ }), + ).toHaveAttribute("aria-expanded", "false"); + }); + + it("preserves an open-but-empty section's intent when toggling another section", async () => { + // Subscriptions is open-in-intent but empty (excluded from the accordion's + // value). Collapsing a populated section must not drop subscriptions from + // the persisted intent, so it reopens once it has items again (#1462). + const user = userEvent.setup(); + const onOpenSectionsChange = vi.fn(); + renderWithMantine( + , + ); + await user.click(screen.getByRole("button", { name: /Templates \(1\)/ })); + // Mantine emits ["resources"]; "subscriptions" is merged back in. + expect(onOpenSectionsChange).toHaveBeenCalledWith( + expect.arrayContaining(["resources", "subscriptions"]), + ); + expect(onOpenSectionsChange.mock.calls[0][0]).not.toContain("templates"); + }); + it("filters by resource title when title is set", async () => { const user = userEvent.setup(); const resourcesWithTitle: Resource[] = [ diff --git a/clients/web/src/components/groups/ResourceControls/ResourceControls.tsx b/clients/web/src/components/groups/ResourceControls/ResourceControls.tsx index 47b1f80c..108aff35 100644 --- a/clients/web/src/components/groups/ResourceControls/ResourceControls.tsx +++ b/clients/web/src/components/groups/ResourceControls/ResourceControls.tsx @@ -2,11 +2,11 @@ import { Accordion, CloseButton, Group, - ScrollArea, Stack, TextInput, Title, } from "@mantine/core"; +import { RiArrowRightSLine } from "react-icons/ri"; import type { Resource, ResourceTemplate, @@ -46,17 +46,19 @@ export interface ResourceControlsProps { onCompactChange: (next: boolean) => void; } -function panelMaxHeight(openCount: number): string { - const n = Math.max(openCount, 1); - const fixedChrome = 300; - const perPanelChrome = 20; - return `calc((100vh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2 - ${fixedChrome + perPanelChrome * n}px) / ${n})`; -} - function formatSectionCount(label: string, count: number): string { return `${label} (${count})`; } +// Per-section flex for the full-height accordion. Open sections share the +// remaining height; `flex-shrink` is weighted by item count (so a long section +// gives up space to shorter ones before they have to scroll) and `flex-grow` is +// 0 so nothing expands — or scrolls — until the combined content overflows the +// panel. Closed/empty sections stay at their header height (#1462). +function sectionFlex(open: boolean, count: number): string { + return open && count > 0 ? `0 ${count} auto` : "0 0 auto"; +} + export function ResourceControls({ resources, templates, @@ -104,7 +106,34 @@ export function ResourceControls({ const openSections = controlledOpenSections ?? (initialCompact ? [] : [...allSections]); const allExpanded = openSections.length === allSections.length; - const maxHeight = panelMaxHeight(openSections.length); + + // Empty sections have a disabled control and nothing to show, so keep them + // out of the accordion's open set — they render collapsed (chevron points + // right) rather than as an open-but-empty panel (#1462). `openSections` still + // tracks the user's intent (and seeds the ListToggle), so a section re-opens + // on its own once it has items again. + const sectionItemCounts: Record = { + resources: filteredResources.length, + templates: filteredTemplates.length, + subscriptions: filteredSubscriptions.length, + }; + const visibleOpenSections = openSections.filter( + (section) => (sectionItemCounts[section] ?? 0) > 0, + ); + // Open-in-intent but currently empty (so excluded from the accordion's + // `value`). Mantine derives the next open-array by toggling the clicked + // section against the `value` we hand it, which omits these — so without + // merging them back, toggling any populated section would silently drop an + // empty section's intent and it wouldn't reappear once it has items again. + const intendedButEmptySections = openSections.filter( + (section) => !visibleOpenSections.includes(section), + ); + function handleOpenSectionsChange(next: string[]) { + // Safe to append unconditionally: empty-section controls are `disabled`, so + // the user can never toggle one and `next` never contains an empty section + // — no double-add, and a section the user just closed can't be resurrected. + onOpenSectionsChange([...next, ...intendedButEmptySections]); + } function handleToggleList() { // Compute the next compact value from what the click will produce so a @@ -117,7 +146,10 @@ export function ResourceControls({ } return ( - + // Fills the full-height `sidebar` Card (flex column) so the scroll region + // below can claim the remaining space; `mih: 0` lets that child shrink and + // scroll instead of overflowing the card (#1462). + Resources @@ -140,71 +172,94 @@ export function ResourceControls({ /> - - + } + value={visibleOpenSections} + onChange={handleOpenSectionsChange} + flex={1} + mih={0} + // Disable Mantine's panel height animation: its Collapse drives the + // open/close via an inline `height` that briefly jumps the panel to its + // full natural height, fighting the flex sizing (the panels are + // flex-distributed and scroll). With it off, flex controls the height + // cleanly. The chevron still rotates smoothly (CSS, in App.css). #1462 + transitionDuration={0} + > + {formatSectionCount("URIs", filteredResources.length)} - - - {filteredResources.map((resource) => ( - { - if (resource.uri !== selectedUri) - onSelectUri(resource.uri); - }} - /> - ))} - - + + {filteredResources.map((resource) => ( + { + if (resource.uri !== selectedUri) onSelectUri(resource.uri); + }} + /> + ))} + - + {formatSectionCount("Templates", filteredTemplates.length)} - - - {filteredTemplates.map((template) => ( - { - if (template.uriTemplate !== selectedTemplateUri) - onSelectTemplate(template.uriTemplate); - }} - /> - ))} - - + + {filteredTemplates.map((template) => ( + { + if (template.uriTemplate !== selectedTemplateUri) + onSelectTemplate(template.uriTemplate); + }} + /> + ))} + - + {formatSectionCount("Subscriptions", filteredSubscriptions.length)} - - - {filteredSubscriptions.map((sub) => ( - - onUnsubscribeResource(sub.resource.uri) - } - /> - ))} - - + + {filteredSubscriptions.map((sub) => ( + onUnsubscribeResource(sub.resource.uri)} + /> + ))} + diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.stories.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.stories.tsx index ac35aeb0..4c5e0313 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.stories.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.stories.tsx @@ -6,7 +6,7 @@ import type { ResourceTemplate, } from "@modelcontextprotocol/sdk/types.js"; import type { InspectorResourceSubscription } from "../../../../../../core/mcp/types.js"; -import { fn, userEvent, within } from "storybook/test"; +import { expect, fn, userEvent, within } from "storybook/test"; import { ResourcesScreen } from "./ResourcesScreen"; import type { ReadResourceState, ResourcesUiState } from "./ResourcesScreen"; import { EMPTY_RESOURCES_UI } from "../screenUiState"; @@ -222,6 +222,51 @@ export const AllSections: Story = { }, }; +const manyResources: Resource[] = Array.from({ length: 40 }, (_, i) => ({ + name: `resource-${String(i + 1).padStart(2, "0")}.wav`, + uri: `file:///kit/resource-${i + 1}.wav`, +})); + +// Enough URIs to overflow the panel. The sidebar card stays the same height as +// the detail panel (no selection → full-height empty card). The section headers +// stay pinned and the long URIs section scrolls *within its own panel* — the +// whole accordion doesn't scroll, and a section doesn't scroll until the panel +// is full (#1462). +export const ManyResources: Story = { + args: { + resources: manyResources, + templates: sampleTemplates, + }, + play: async ({ canvasElement }) => { + const [sidebarCard, detailCard] = + canvasElement.querySelectorAll(".mantine-Card-root"); + const sidebar = sidebarCard.getBoundingClientRect(); + const detail = detailCard.getBoundingClientRect(); + // The sidebar matches the detail panel's height (same bottom baseline). + expect(Math.abs(sidebar.bottom - detail.bottom)).toBeLessThanOrEqual(1); + expect(Math.abs(sidebar.height - detail.height)).toBeLessThanOrEqual(1); + + // The section headers stay pinned (visible, in document order), so the + // whole accordion doesn't scroll as one block. + const controls = canvasElement.querySelectorAll( + ".disclosure-sections .mantine-Accordion-control", + ); + expect(controls).toHaveLength(3); + const tops = [...controls].map((c) => c.getBoundingClientRect().top); + expect(tops[0]).toBeLessThan(tops[1]); + expect(tops[1]).toBeLessThan(tops[2]); + + // The long URIs section scrolls within its own panel. + const urisPanel = canvasElement.querySelector( + ".disclosure-sections .mantine-Accordion-panel", + ); + if (!(urisPanel instanceof HTMLElement)) { + throw new Error("URIs panel not found"); + } + expect(urisPanel.scrollHeight).toBeGreaterThan(urisPanel.clientHeight); + }, +}; + export const Empty: Story = { args: { resources: [], diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx index a94d9633..ebc4b5bf 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.test.tsx @@ -138,10 +138,21 @@ describe("ResourcesScreen", () => { ).toBeInTheDocument(); }); + it("toggles a section's open state through the lifted openSections handler", async () => { + const user = userEvent.setup(); + renderWithMantine(); + const urisHeader = screen.getByRole("button", { name: /URIs \(2\)/ }); + // Open by default (compact=false); clicking routes through ResourcesScreen's + // onOpenSectionsChange → onUiChange and collapses it. + expect(urisHeader).toHaveAttribute("aria-expanded", "true"); + await user.click(urisHeader); + expect(urisHeader).toHaveAttribute("aria-expanded", "false"); + }); + it("renders the template panel when a template is selected", async () => { const user = userEvent.setup(); renderWithMantine(); - await user.click(screen.getByText("Templates (1)")); + // Templates section is open by default; select the template to open its form. await user.click(screen.getByText("files")); expect( screen.getByRole("button", { name: "Read Resource" }), @@ -154,7 +165,6 @@ describe("ResourcesScreen", () => { renderWithMantine( , ); - await user.click(screen.getByText("Templates (1)")); await user.click(screen.getByText("files")); await user.type(screen.getByLabelText("path"), "alpha"); await user.click(screen.getByRole("button", { name: "Read Resource" })); @@ -210,8 +220,7 @@ describe("ResourcesScreen", () => { onReadResource={onReadResource} />, ); - // Open the template form. - await user.click(screen.getByText("Templates (1)")); + // Templates is open by default; select the template to open its form. await user.click(screen.getByText("files")); // Submit it — the screen calls onReadResource and remembers the // template URI for the close handler. @@ -249,7 +258,6 @@ describe("ResourcesScreen", () => { const { rerender } = renderWithMantine( , ); - await user.click(screen.getByText("Templates (1)")); await user.click(screen.getByText("Dynamic")); await user.type(screen.getByLabelText("id"), "asdf"); await user.click(screen.getByRole("button", { name: "Read Resource" })); diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx index ac3099b0..d98bed36 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx @@ -77,9 +77,14 @@ const Sidebar = Stack.withProps({ flex: "0 0 auto", }); +// Card that grows with its content but is capped at the screen height by the +// `sidebar` variant (`max-height: 100%`), like the Tools panel. The column +// layout lets ResourceControls' accordion take over per-section scrolling once +// the content would overflow that cap. const SidebarCard = Card.withProps({ withBorder: true, padding: "lg", + variant: "sidebar", }); const DetailCard = Card.withProps({ diff --git a/clients/web/src/theme/Accordion.ts b/clients/web/src/theme/Accordion.ts new file mode 100644 index 00000000..39b247aa --- /dev/null +++ b/clients/web/src/theme/Accordion.ts @@ -0,0 +1,17 @@ +import { Accordion } from "@mantine/core"; + +export const ThemeAccordion = Accordion.extend({ + // The `disclosure` variant drives two behaviours via App.css (see #1462): + // - `disclosure-chevron` on the chevron slot rotates a right-pointing arrow + // 90° (right → down) on open, instead of Mantine's default 180° flip. + // - `disclosure-sections` on the root makes the accordion a full-height + // flex column: section headers stay pinned and each open section's panel + // scrolls within its own (item-count-weighted) share of the space, so + // nothing scrolls until the panel is full. + // Pair it with `chevron={}` and per-item `flex` weights. + classNames: (_theme, props) => { + if (props.variant === "disclosure") + return { root: "disclosure-sections", chevron: "disclosure-chevron" }; + return {}; + }, +}); diff --git a/clients/web/src/theme/Card.ts b/clients/web/src/theme/Card.ts index e00765c5..eaa5b586 100644 --- a/clients/web/src/theme/Card.ts +++ b/clients/web/src/theme/Card.ts @@ -20,6 +20,23 @@ export const ThemeCard = Card.extend({ }, }; } + if (props.variant === "sidebar") { + // Sidebar container that grows with its content but never taller than the + // screen's available area (`max-height: 100%` of the full-height column + // wrapper) — like the Tools panel. Lays its content out as a column and + // hides overflow so that, once capped, the flex accordion below the fixed + // title/search takes over per-section scrolling rather than the card + // bleeding past the viewport (#1462). + return { + root: { + backgroundColor: "var(--inspector-surface-card)", + maxHeight: "100%", + display: "flex", + flexDirection: "column", + overflow: "hidden", + }, + }; + } if (props.variant === "preview") { // Container for the resource preview / template form panels: sizes to // content (no forced height) but caps at the screen's available area diff --git a/clients/web/src/theme/index.ts b/clients/web/src/theme/index.ts index 4bb47db7..72b07cd9 100644 --- a/clients/web/src/theme/index.ts +++ b/clients/web/src/theme/index.ts @@ -1,3 +1,4 @@ +export { ThemeAccordion } from "./Accordion"; export { ThemeActionIcon } from "./ActionIcon"; export { ThemeAlert } from "./Alert"; export { ThemeAppShell } from "./AppShell"; diff --git a/clients/web/src/theme/theme.ts b/clients/web/src/theme/theme.ts index 40da99cf..a23bc668 100644 --- a/clients/web/src/theme/theme.ts +++ b/clients/web/src/theme/theme.ts @@ -1,5 +1,6 @@ import { createTheme, type MantineColorsTuple } from "@mantine/core"; import { + ThemeAccordion, ThemeActionIcon, ThemeAlert, ThemeAppShell, @@ -56,6 +57,7 @@ export const theme = createTheme({ /* ── Component overrides ────────────────────────────────── */ components: { + Accordion: ThemeAccordion, ActionIcon: ThemeActionIcon, Alert: ThemeAlert, AppShell: ThemeAppShell,