* fix(web): ResourceControls single-scroll + right/down disclosure chevrons (#1462) Two fixes to the Resources controls panel: 1. Sections scrolled prematurely. `panelMaxHeight` divided the available height equally across open sections (`… / n`), so a section with many items scrolled while sparse siblings left their equal share unused. Replace the per-section ScrollAreas with a single viewport-bounded ScrollArea.Autosize around the whole accordion (matching ToolControls / PromptControls). Sections size to content; nothing scrolls until the open sections' combined height exceeds the cap. 2. Accordion chevrons pointed down/up. Add a `disclosure` Accordion theme variant + `.disclosure-chevron[data-rotate]` rule that rotates a right-pointing RiArrowRightSLine 90° (not Mantine's default 180°), so closed sections point right and open sections point down — matching the ResourceLink chevron convention. Also keep empty (disabled) sections out of the accordion's open set so they render collapsed with a right chevron instead of an open-but-empty panel. Verified all states in a real browser via Playwright. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(web): cover empty-section-collapsed behavior in ResourceControls (#1462) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): make Resources sidebar fill panel height before scrolling (#1462) The first pass capped the accordion scroll region at a fixed sub-viewport height (`100vh - … - 160px`), so the sidebar scrolled before reaching the height of the detail panel to its right. Make the sidebar card full-height instead: add a `sidebar` Card variant (height:100%, flex column, overflow hidden), give it `flex: 1`, and let ResourceControls' root Stack (`flex: 1; min-height: 0`) and a plain flex-filling ScrollArea claim the space below the title/search. The card now matches the detail panel's height exactly and the single inner scroll region scrolls only once the open sections overflow the full panel. Adds a ResourcesScreen ManyResources story asserting the sidebar/detail height match and that overflow scrolls, plus a unit test for the empty-section-collapsed behavior. Verified heights + scroll in a real browser via Playwright. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): pin Resources section headers, scroll each section in place (#1462) The previous pass scrolled the whole accordion as one block (headers scrolled away). Restore pinned headers with per-section scrolling, without the original equal-division premature-scroll bug. The `disclosure` Accordion now fills its full-height card as a flex column (`disclosure-sections`): controls are pinned, and each open section's panel scrolls within its own share of the remaining height. Per-item `flex` weights (shrink ∝ item count, grow 0) hand space from long sections to short ones, so nothing scrolls until the combined content overflows the panel — a long URIs list scrolls in place while Templates stays fully visible below it. Drops the single outer ScrollArea (and useScrollMemory). Verified in a real browser: 6 URIs → no scroll, headers pinned, empty space at the bottom; 40 URIs → URIs panel scrolls in place, Templates intact, all three headers pinned, sidebar still matches the detail height. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): Resources sidebar grows with content, capped at screen (#1462) The `sidebar` Card variant forced `height: 100%`, so the panel was always full-height with empty space below short content. Switch to `max-height: 100%` (and drop `flex: 1`) so the card hugs its content and only caps at the screen height — matching the Tools panel. When content overflows the cap, the flex accordion still takes over per-section scrolling with pinned headers. Verified in a real browser: 6 URIs → card hugs (638px, shorter than the detail panel); 40 URIs → card caps at the screen (949px == detail), headers pinned, URIs scrolls in place. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): preserve empty-section intent + kill accordion collapse jump (#1464 review) Address review feedback on PR #1464: - Intent loss (review #1): the accordion's `value` is the filtered `visibleOpenSections`, so Mantine's toggle dropped open-but-empty sections from the persisted intent. Merge them back in the onChange handler so an open empty section stays open and reappears with items. Adds a unit test. - Collapse animation jump (review #3): Mantine's Collapse briefly set the panel to its full natural height (fighting the flex layout) on open/close, flashing siblings. Set `transitionDuration={0}` so flex controls the height; restore a smooth chevron rotation via CSS. Verified no jump in a real browser (panel 501→0, no spike). Since closed sections now render no content, drop the redundant (and incorrect) "Templates" toggle clicks from 4 ResourcesScreen tests (the section is open by default) and add a section-toggle test to keep the openSections handler covered. - Nits: scope the stories' chevron query to the accordion root; tighten the Collapsed assertion to the identity transform. Native-vs-styled scrollbar (review #4) split out as #1466. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(web): clarify chevron-timing and intent-merge invariants (#1464 review) Address the two optional doc nits from the re-review: - Note that the chevron's 200ms restates Mantine's default accordion duration (keep in sync if accordion timing is themed). - Note that the unconditional empty-section merge is safe because empty-section controls are disabled, so `next` never contains one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall
GitHub
parent
3220d6239a
commit
d525fc9f54
3 changed files with 388 additions and 64 deletions
|
|
@ -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<typeof ResourceControls> = {
|
||||
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,
|
||||
},
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<ResourceControls
|
||||
{...baseProps}
|
||||
subscriptions={[]}
|
||||
openSections={["resources", "templates", "subscriptions"]}
|
||||
/>,
|
||||
);
|
||||
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(
|
||||
<ResourceControls
|
||||
{...baseProps}
|
||||
subscriptions={[]}
|
||||
openSections={["resources", "templates", "subscriptions"]}
|
||||
onOpenSectionsChange={onOpenSectionsChange}
|
||||
/>,
|
||||
);
|
||||
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[] = [
|
||||
|
|
|
|||
|
|
@ -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<string, number> = {
|
||||
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 (
|
||||
<Stack gap="sm">
|
||||
// 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).
|
||||
<Stack gap="sm" flex={1} mih={0}>
|
||||
<Group justify="space-between">
|
||||
<Title order={4}>Resources</Title>
|
||||
<ListChangedIndicator visible={listChanged} onRefresh={onRefreshList} />
|
||||
|
|
@ -140,71 +172,94 @@ export function ResourceControls({
|
|||
/>
|
||||
<ListToggle compact={!allExpanded} onToggle={handleToggleList} />
|
||||
</Group>
|
||||
<Accordion multiple value={openSections} onChange={onOpenSectionsChange}>
|
||||
<Accordion.Item value="resources">
|
||||
<Accordion
|
||||
multiple
|
||||
variant="disclosure"
|
||||
chevron={<RiArrowRightSLine />}
|
||||
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}
|
||||
>
|
||||
<Accordion.Item
|
||||
value="resources"
|
||||
flex={sectionFlex(
|
||||
visibleOpenSections.includes("resources"),
|
||||
filteredResources.length,
|
||||
)}
|
||||
>
|
||||
<Accordion.Control disabled={filteredResources.length === 0}>
|
||||
{formatSectionCount("URIs", filteredResources.length)}
|
||||
</Accordion.Control>
|
||||
<Accordion.Panel>
|
||||
<ScrollArea.Autosize mah={maxHeight}>
|
||||
<Stack gap="xs">
|
||||
{filteredResources.map((resource) => (
|
||||
<ResourceListItem
|
||||
key={resource.uri}
|
||||
resource={resource}
|
||||
selected={resource.uri === selectedUri}
|
||||
onClick={() => {
|
||||
if (resource.uri !== selectedUri)
|
||||
onSelectUri(resource.uri);
|
||||
}}
|
||||
/>
|
||||
))}
|
||||
</Stack>
|
||||
</ScrollArea.Autosize>
|
||||
<Stack gap="xs">
|
||||
{filteredResources.map((resource) => (
|
||||
<ResourceListItem
|
||||
key={resource.uri}
|
||||
resource={resource}
|
||||
selected={resource.uri === selectedUri}
|
||||
onClick={() => {
|
||||
if (resource.uri !== selectedUri) onSelectUri(resource.uri);
|
||||
}}
|
||||
/>
|
||||
))}
|
||||
</Stack>
|
||||
</Accordion.Panel>
|
||||
</Accordion.Item>
|
||||
|
||||
<Accordion.Item value="templates">
|
||||
<Accordion.Item
|
||||
value="templates"
|
||||
flex={sectionFlex(
|
||||
visibleOpenSections.includes("templates"),
|
||||
filteredTemplates.length,
|
||||
)}
|
||||
>
|
||||
<Accordion.Control disabled={filteredTemplates.length === 0}>
|
||||
{formatSectionCount("Templates", filteredTemplates.length)}
|
||||
</Accordion.Control>
|
||||
<Accordion.Panel>
|
||||
<ScrollArea.Autosize mah={maxHeight}>
|
||||
<Stack gap="xs">
|
||||
{filteredTemplates.map((template) => (
|
||||
<ResourceListItem
|
||||
key={template.uriTemplate}
|
||||
resource={template}
|
||||
selected={template.uriTemplate === selectedTemplateUri}
|
||||
onClick={() => {
|
||||
if (template.uriTemplate !== selectedTemplateUri)
|
||||
onSelectTemplate(template.uriTemplate);
|
||||
}}
|
||||
/>
|
||||
))}
|
||||
</Stack>
|
||||
</ScrollArea.Autosize>
|
||||
<Stack gap="xs">
|
||||
{filteredTemplates.map((template) => (
|
||||
<ResourceListItem
|
||||
key={template.uriTemplate}
|
||||
resource={template}
|
||||
selected={template.uriTemplate === selectedTemplateUri}
|
||||
onClick={() => {
|
||||
if (template.uriTemplate !== selectedTemplateUri)
|
||||
onSelectTemplate(template.uriTemplate);
|
||||
}}
|
||||
/>
|
||||
))}
|
||||
</Stack>
|
||||
</Accordion.Panel>
|
||||
</Accordion.Item>
|
||||
|
||||
<Accordion.Item value="subscriptions">
|
||||
<Accordion.Item
|
||||
value="subscriptions"
|
||||
flex={sectionFlex(
|
||||
visibleOpenSections.includes("subscriptions"),
|
||||
filteredSubscriptions.length,
|
||||
)}
|
||||
>
|
||||
<Accordion.Control disabled={filteredSubscriptions.length === 0}>
|
||||
{formatSectionCount("Subscriptions", filteredSubscriptions.length)}
|
||||
</Accordion.Control>
|
||||
<Accordion.Panel>
|
||||
<ScrollArea.Autosize mah={maxHeight}>
|
||||
<Stack gap="xs">
|
||||
{filteredSubscriptions.map((sub) => (
|
||||
<ResourceSubscribedItem
|
||||
key={sub.resource.uri}
|
||||
subscription={sub}
|
||||
onUnsubscribe={() =>
|
||||
onUnsubscribeResource(sub.resource.uri)
|
||||
}
|
||||
/>
|
||||
))}
|
||||
</Stack>
|
||||
</ScrollArea.Autosize>
|
||||
<Stack gap="xs">
|
||||
{filteredSubscriptions.map((sub) => (
|
||||
<ResourceSubscribedItem
|
||||
key={sub.resource.uri}
|
||||
subscription={sub}
|
||||
onUnsubscribe={() => onUnsubscribeResource(sub.resource.uri)}
|
||||
/>
|
||||
))}
|
||||
</Stack>
|
||||
</Accordion.Panel>
|
||||
</Accordion.Item>
|
||||
</Accordion>
|
||||
|
|
|
|||
Loading…
Reference in a new issue