fix(web): hide resource subscription UI when server lacks subscribe capability (#1479)
* fix(web): hide resource subscription UI when server lacks subscribe capability (#1478) Thread a `subscriptionsSupported` flag derived from `capabilities?.resources?.subscribe === true` from App through InspectorView → ResourcesScreen to the leaf components. When false: - ResourcePreviewPanel hides the Subscribe/Unsubscribe button (Refresh stays). - ResourceControls omits the Subscriptions accordion section entirely, dropping it from `allSections` so the expand/collapse-all toggle and open-section bookkeeping stay correct even if a stale "subscriptions" entry persists from a prior subscription-capable session. The flag defaults to true at the leaf components so existing callers, tests, and stories keep rendering the subscription controls unless a server is explicitly marked unsupported. Adds unit tests and Storybook stories covering the hidden and shown states across all three components. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): give the Servers screen scroll the same gutter/idle-hidden treatment (#1474) The server grid's ScrollArea.Autosize used the default type="hover" with no offsetScrollbars, so the scrollbar popped in on hover and overlaid the right edge of the server cards (occluding their action icons / status badges) — the same problem #1474 fixed for the History/Network/Logging list panels. Apply the same `type="scroll"` + `offsetScrollbars` treatment, and reuse the shared `expectScrollbarGutterIdleHidden` story assertion to regression-protect the gutter + idle-hidden behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): stop the InspectorView shell from scrolling as a whole AppShell.Main carried Mantine's default `padding="md"` on top of the fixed-header offset, but every screen already sizes itself to `calc(100vh - header)` and supplies its own `xl` padding (with no allowance for Main's padding). That extra inset pushed screen content past the viewport, so the whole InspectorView gained a page-level scrollbar instead of confining overflow to each screen's inner ScrollArea. Set the shell `padding` to 0 (Main now contributes only the header offset, matching what the screens were built for) and clamp the Main slot to `height: 100dvh` with `overflow: hidden` in the theme, so the view can never scroll regardless of content — only the inner ScrollArea regions do. Adds an InspectorView story play assertion guarding the invariant (Main clips + is viewport-clamped) and a ManyServers fixture that confirms it holds under enough content to overflow, with the inner server-grid ScrollArea absorbing the overflow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(web): address review — align viewport units + drop stale subscriptions intent Responding to the @claude review on #1479: - Align all screen height calcs from `100vh` to `100dvh` so they match the Main slot's `100dvh` clamp. On a mobile viewport where `dvh < vh` a `100vh` child would have exceeded Main's `100dvh` content box and been clipped by `overflow: hidden`; the units are now in lockstep. - Restrict `intendedButEmptySections` in ResourceControls to `allSections` so a stale "subscriptions" entry persisted from a prior subscription-capable session is dropped from persisted open-sections instead of being perpetually re-appended once the section is no longer rendered. Adds a regression test. 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
f4157063e6
commit
ef921ba052
12 changed files with 346 additions and 47 deletions
|
|
@ -63,7 +63,7 @@ export interface AppsUiState {
|
|||
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100vh - var(--app-shell-header-height, 0px))",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
align: "flex-start",
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@ export interface HistoryUiState {
|
|||
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100vh - var(--app-shell-header-height, 0px))",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
});
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ export interface LogsUiState {
|
|||
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100vh - var(--app-shell-header-height, 0px))",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
});
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ export interface NetworkUiState {
|
|||
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100vh - var(--app-shell-header-height, 0px))",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
});
|
||||
|
|
|
|||
|
|
@ -59,7 +59,7 @@ export interface PromptsUiState {
|
|||
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100vh - var(--app-shell-header-height, 0px))",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
});
|
||||
|
|
@ -104,7 +104,7 @@ const EmptyState = Text.withProps({
|
|||
});
|
||||
|
||||
const SCROLL_MAX_HEIGHT =
|
||||
"calc(100vh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2)";
|
||||
"calc(100dvh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2)";
|
||||
|
||||
function hasArguments(prompt: Prompt): boolean {
|
||||
return !!prompt.arguments && prompt.arguments.length > 0;
|
||||
|
|
|
|||
|
|
@ -222,6 +222,21 @@ export const AllSections: Story = {
|
|||
},
|
||||
};
|
||||
|
||||
// Server without the resources.subscribe capability: the Subscriptions
|
||||
// accordion section is omitted and no Subscribe button appears (#1478).
|
||||
export const SubscriptionsUnsupported: Story = {
|
||||
args: {
|
||||
resources: sampleResources,
|
||||
templates: sampleTemplates,
|
||||
subscriptions: sampleSubscriptions,
|
||||
subscriptionsSupported: false,
|
||||
},
|
||||
play: async ({ canvasElement }) => {
|
||||
const canvas = within(canvasElement);
|
||||
expect(canvas.queryByText(/Subscriptions/)).not.toBeInTheDocument();
|
||||
},
|
||||
};
|
||||
|
||||
const manyResources: Resource[] = Array.from({ length: 40 }, (_, i) => ({
|
||||
name: `resource-${String(i + 1).padStart(2, "0")}.wav`,
|
||||
uri: `file:///kit/resource-${i + 1}.wav`,
|
||||
|
|
|
|||
|
|
@ -317,4 +317,27 @@ describe("ResourcesScreen", () => {
|
|||
await user.click(screen.getByRole("button", { name: "Unsubscribe" }));
|
||||
expect(onUnsubscribeResource).toHaveBeenCalledWith("file:///x");
|
||||
});
|
||||
|
||||
it("hides the Subscriptions section and Subscribe button when subscriptionsSupported is false", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithMantine(
|
||||
<ControlledResourcesScreen
|
||||
subscriptionsSupported={false}
|
||||
readState={{
|
||||
status: "ok",
|
||||
uri: "file:///x",
|
||||
result: okResult,
|
||||
isSubscribed: false,
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
// Sidebar Subscriptions accordion section is gone.
|
||||
expect(screen.queryByText(/Subscriptions/)).not.toBeInTheDocument();
|
||||
// Opening a resource preview shows Refresh but no Subscribe button.
|
||||
await user.click(screen.getByText("x.txt"));
|
||||
expect(screen.getByRole("button", { name: "Refresh" })).toBeInTheDocument();
|
||||
expect(
|
||||
screen.queryByRole("button", { name: "Subscribe" }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -35,6 +35,13 @@ export interface ResourcesScreenProps {
|
|||
ui: ResourcesUiState;
|
||||
listChanged: boolean;
|
||||
completionsSupported?: boolean;
|
||||
/**
|
||||
* Whether the connected server advertises the `resources.subscribe`
|
||||
* capability. When false, the Subscribe/Unsubscribe button and the
|
||||
* Subscriptions accordion section are hidden. Defaults to true so the
|
||||
* controls render unless a caller explicitly marks them unsupported.
|
||||
*/
|
||||
subscriptionsSupported?: boolean;
|
||||
onUiChange: (next: ResourcesUiState) => void;
|
||||
onRefreshList: () => void;
|
||||
onReadResource: (uri: string) => void;
|
||||
|
|
@ -67,7 +74,7 @@ export interface ResourcesUiState {
|
|||
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100vh - var(--app-shell-header-height, 0px))",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
});
|
||||
|
|
@ -119,7 +126,7 @@ const EmptyState = Text.withProps({
|
|||
});
|
||||
|
||||
const SCROLL_MAX_HEIGHT =
|
||||
"calc(100vh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2)";
|
||||
"calc(100dvh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2)";
|
||||
|
||||
export function ResourcesScreen({
|
||||
resources,
|
||||
|
|
@ -129,6 +136,7 @@ export function ResourcesScreen({
|
|||
ui,
|
||||
listChanged,
|
||||
completionsSupported,
|
||||
subscriptionsSupported = true,
|
||||
onUiChange,
|
||||
onRefreshList,
|
||||
onReadResource,
|
||||
|
|
@ -255,6 +263,7 @@ export function ResourcesScreen({
|
|||
contents={readState.result.contents}
|
||||
lastUpdated={readState.lastUpdated}
|
||||
isSubscribed={readState.isSubscribed ?? false}
|
||||
subscriptionsSupported={subscriptionsSupported}
|
||||
onRefresh={() => handleReadResource(readResource.uri)}
|
||||
onSubscribe={() => onSubscribeResource(readResource.uri)}
|
||||
onUnsubscribe={() => onUnsubscribeResource(readResource.uri)}
|
||||
|
|
@ -275,6 +284,7 @@ export function ResourcesScreen({
|
|||
resources={resources}
|
||||
templates={templates}
|
||||
subscriptions={subscriptions}
|
||||
subscriptionsSupported={subscriptionsSupported}
|
||||
selectedUri={selectedResourceUri}
|
||||
selectedTemplateUri={selectedTemplateUri}
|
||||
searchText={search}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||
import { expect, fn, userEvent, waitFor, within } from "storybook/test";
|
||||
import type { ServerEntry } from "@inspector/core/mcp/types.js";
|
||||
import { expectScrollbarGutterIdleHidden } from "../../../test/scrollAreaStoryAssertions";
|
||||
import { ServerListScreen } from "./ServerListScreen";
|
||||
|
||||
const meta: Meta<typeof ServerListScreen> = {
|
||||
|
|
@ -79,6 +80,11 @@ export const MultipleServers: Story = {
|
|||
args: {
|
||||
servers: [connectedStdioServer, disconnectedStdioServer, failedHttpServer],
|
||||
},
|
||||
// The server grid scroll region reserves a scrollbar gutter and hides the
|
||||
// bar when idle, matching the History/Network/Logging list panels (#1474).
|
||||
play: async ({ canvasElement }) => {
|
||||
expectScrollbarGutterIdleHidden(canvasElement);
|
||||
},
|
||||
};
|
||||
|
||||
export const SingleServer: Story = {
|
||||
|
|
|
|||
|
|
@ -137,7 +137,15 @@ export function ServerListScreen({
|
|||
onExport={onExport}
|
||||
/>
|
||||
|
||||
<ScrollArea.Autosize mah="calc(100vh - var(--app-shell-header-height, 60px) - var(--mantine-spacing-xl) * 2 - 60px)">
|
||||
<ScrollArea.Autosize
|
||||
mah="calc(100dvh - var(--app-shell-header-height, 60px) - var(--mantine-spacing-xl) * 2 - 60px)"
|
||||
// Same scrollbar treatment as the History/Network/Logging list panels
|
||||
// (#1474): reserve a gutter so the bar never overlays the right edge of
|
||||
// the server cards (occluding their action icons / status badges), and
|
||||
// only show it while actively scrolling rather than popping in on hover.
|
||||
type="scroll"
|
||||
offsetScrollbars
|
||||
>
|
||||
{servers.length === 0 ? (
|
||||
<EmptyState>
|
||||
No servers configured. Add a server to get started.
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ export interface TasksUiState {
|
|||
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100vh - var(--app-shell-header-height, 0px))",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
});
|
||||
|
|
|
|||
|
|
@ -61,7 +61,7 @@ export interface ToolsScreenProps {
|
|||
// viewport minus the app-shell header and the screen's top+bottom xl padding,
|
||||
// leaving the bottom margin the overflow used to eat.
|
||||
const SCROLL_MAX_HEIGHT =
|
||||
"calc(100vh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2)";
|
||||
"calc(100dvh - var(--app-shell-header-height, 0px) - var(--mantine-spacing-xl) * 2)";
|
||||
|
||||
// No `align` override: children stretch to the row's full height, giving each
|
||||
// column pane a definite height. That definite height is what lets a column's
|
||||
|
|
@ -69,7 +69,7 @@ const SCROLL_MAX_HEIGHT =
|
|||
// see the Prompts/Resources preview panes this mirrors).
|
||||
const ScreenLayout = Flex.withProps({
|
||||
variant: "screen",
|
||||
h: "calc(100vh - var(--app-shell-header-height, 0px))",
|
||||
h: "calc(100dvh - var(--app-shell-header-height, 0px))",
|
||||
gap: "md",
|
||||
p: "xl",
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue