From 4c1d2f2c34b889fea66b808e706165e7c587e03b Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 26 Apr 2026 09:27:22 -0400 Subject: [PATCH] Phase 4: Move screen selection state into the screens themselves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ToolsScreen, PromptsScreen, and ResourcesScreen each tracked the user's current selection via `selectedXxxName` / `onSelectXxx` props, hoisting screen-local UI state into whatever rendered them. InspectorView never threaded those callbacks anywhere, so clicks in the controls panel did not display the matching detail panel — the detail panel only rendered if a parent fed selection back in. Move selection into `useState` inside each screen and drop the `selectedXxxName` / `onSelectXxx` props. MCP-derived data (`callState`, `getPromptState`, `readState`) keeps flowing in from `InspectorView` as props. ResourcesScreen wraps `onReadResource` so a template-expanded URI becomes the selected resource for preview rendering. Update each screen's stories to drop the deleted props. Stories that need a pre-selected item use a `play` function to click it — no component coupling to Storybook. Add Phase 4.3 to the interfaces plan describing the rule. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../PromptsScreen/PromptsScreen.stories.tsx | 24 +++-- .../screens/PromptsScreen/PromptsScreen.tsx | 9 +- .../ResourcesScreen.stories.tsx | 94 ++++++++++--------- .../ResourcesScreen/ResourcesScreen.tsx | 29 +++--- .../ToolsScreen/ToolsScreen.stories.tsx | 38 ++++++-- .../screens/ToolsScreen/ToolsScreen.tsx | 9 +- .../views/InspectorView/InspectorView.tsx | 4 - specification/v2_ux_interfaces_plan.md | 50 ++++++++++ 8 files changed, 171 insertions(+), 86 deletions(-) diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.stories.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.stories.tsx index bffb91d5..f947a071 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.stories.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.stories.tsx @@ -1,6 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import type { Prompt } from "@modelcontextprotocol/sdk/types.js"; -import { fn } from "storybook/test"; +import { fn, userEvent, within } from "storybook/test"; import { PromptsScreen } from "./PromptsScreen"; import type { GetPromptState } from "./PromptsScreen"; @@ -10,7 +10,6 @@ const meta: Meta = { parameters: { layout: "fullscreen" }, args: { onRefreshList: fn(), - onSelectPrompt: fn(), onGetPrompt: fn(), onCopyMessages: fn(), listChanged: false, @@ -70,6 +69,11 @@ const translateResult: GetPromptState = { }, }; +async function selectPromptByName(canvasElement: HTMLElement, name: string) { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByText(name)); +} + export const NoSelection: Story = { args: { prompts: samplePrompts, @@ -79,36 +83,44 @@ export const NoSelection: Story = { export const PromptSelected: Story = { args: { prompts: samplePrompts, - selectedPromptName: "translate", + }, + play: async ({ canvasElement }) => { + await selectPromptByName(canvasElement, "translate"); }, }; export const WithResult: Story = { args: { prompts: samplePrompts, - selectedPromptName: "translate", getPromptState: translateResult, }, + play: async ({ canvasElement }) => { + await selectPromptByName(canvasElement, "translate"); + }, }; export const Loading: Story = { args: { prompts: samplePrompts, - selectedPromptName: "translate", getPromptState: { status: "pending" }, }, + play: async ({ canvasElement }) => { + await selectPromptByName(canvasElement, "translate"); + }, }; export const WithError: Story = { args: { prompts: samplePrompts, - selectedPromptName: "translate", getPromptState: { status: "error", error: 'Prompt "translate" requires argument "text" but none was provided. Please fill in all required arguments before submitting.', }, }, + play: async ({ canvasElement }) => { + await selectPromptByName(canvasElement, "translate"); + }, }; export const ListChanged: Story = { diff --git a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx index 17cc8cf7..3e1484ed 100644 --- a/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx +++ b/clients/web/src/components/screens/PromptsScreen/PromptsScreen.tsx @@ -25,11 +25,9 @@ export interface GetPromptState { export interface PromptsScreenProps { prompts: Prompt[]; - selectedPromptName?: string; getPromptState?: GetPromptState; listChanged: boolean; onRefreshList: () => void; - onSelectPrompt: (name: string) => void; onGetPrompt: (name: string, args: Record) => void; onCopyMessages?: () => void; } @@ -65,14 +63,15 @@ const EmptyState = Text.withProps({ export function PromptsScreen({ prompts, - selectedPromptName, getPromptState, listChanged, onRefreshList, - onSelectPrompt, onGetPrompt, onCopyMessages, }: PromptsScreenProps) { + const [selectedPromptName, setSelectedPromptName] = useState< + string | undefined + >(undefined); const [argumentValues, setArgumentValues] = useState>( {}, ); @@ -91,7 +90,7 @@ export function PromptsScreen({ onRefreshList={onRefreshList} onSelectPrompt={(name) => { setArgumentValues({}); - onSelectPrompt(name); + setSelectedPromptName(name); }} /> diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.stories.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.stories.tsx index b6cb9c66..4867920d 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.stories.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.stories.tsx @@ -4,7 +4,7 @@ import type { ResourceTemplate, } from "@modelcontextprotocol/sdk/types.js"; import type { InspectorResourceSubscription } from "../../../../../../core/mcp/types.js"; -import { fn } from "storybook/test"; +import { fn, userEvent, within } from "storybook/test"; import { ResourcesScreen } from "./ResourcesScreen"; import type { ReadResourceState } from "./ResourcesScreen"; @@ -14,8 +14,6 @@ const meta: Meta = { parameters: { layout: "fullscreen" }, args: { onRefreshList: fn(), - onSelectUri: fn(), - onSelectTemplate: fn(), onReadResource: fn(), onSubscribeResource: fn(), onUnsubscribeResource: fn(), @@ -105,6 +103,40 @@ const readConfigState: ReadResourceState = { isSubscribed: true, }; +const readUserProfileState: ReadResourceState = { + status: "ok", + uri: "file:///users/42/profile", + result: { + contents: [ + { + uri: "file:///users/42/profile", + mimeType: "application/json", + text: JSON.stringify( + { id: 42, name: "Alice", email: "alice@example.com" }, + null, + 2, + ), + }, + ], + }, + isSubscribed: false, +}; + +async function clickByText(canvasElement: HTMLElement, label: string) { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByText(label)); +} + +async function expandUserProfileTemplate(canvasElement: HTMLElement) { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByText("User Profile")); + const userIdInput = await canvas.findByLabelText("userId"); + await userEvent.type(userIdInput, "42"); + await userEvent.click( + await canvas.findByRole("button", { name: "Read Resource" }), + ); +} + export const WithResources: Story = { args: { resources: sampleResources, @@ -115,9 +147,11 @@ export const ResourceSelected: Story = { args: { resources: sampleResources, subscriptions: sampleSubscriptions, - selectedResourceUri: "file:///config.json", readState: readConfigState, }, + play: async ({ canvasElement }) => { + await clickByText(canvasElement, "config.json"); + }, }; export const WithTemplates: Story = { @@ -131,7 +165,9 @@ export const TemplateSelected: Story = { args: { resources: sampleResources, templates: sampleTemplates, - selectedTemplateUri: "file:///users/{userId}/profile", + }, + play: async ({ canvasElement }) => { + await clickByText(canvasElement, "User Profile"); }, }; @@ -139,26 +175,10 @@ export const TemplateWithResource: Story = { args: { resources: sampleResources, templates: sampleTemplates, - selectedTemplateUri: "file:///users/{userId}/profile", - readState: { - status: "ok", - uri: "file:///users/42/profile", - result: { - contents: [ - { - uri: "file:///users/42/profile", - mimeType: "application/json", - text: JSON.stringify( - { id: 42, name: "Alice", email: "alice@example.com" }, - null, - 2, - ), - }, - ], - }, - isSubscribed: false, - }, - selectedResourceUri: "file:///users/42/profile", + readState: readUserProfileState, + }, + play: async ({ canvasElement }) => { + await expandUserProfileTemplate(canvasElement); }, }; @@ -167,26 +187,10 @@ export const AllSections: Story = { resources: sampleResources, templates: sampleTemplates, subscriptions: sampleSubscriptions, - selectedTemplateUri: "file:///users/{userId}/profile", - readState: { - status: "ok", - uri: "file:///users/42/profile", - result: { - contents: [ - { - uri: "file:///users/42/profile", - mimeType: "application/json", - text: JSON.stringify( - { id: 42, name: "Alice", email: "alice@example.com" }, - null, - 2, - ), - }, - ], - }, - isSubscribed: false, - }, - selectedResourceUri: "file:///users/42/profile", + readState: readUserProfileState, + }, + play: async ({ canvasElement }) => { + await expandUserProfileTemplate(canvasElement); }, }; diff --git a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx index 85ce0816..0bbc0891 100644 --- a/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx +++ b/clients/web/src/components/screens/ResourcesScreen/ResourcesScreen.tsx @@ -1,3 +1,4 @@ +import { useState } from "react"; import { Alert, Card, @@ -31,13 +32,9 @@ export interface ResourcesScreenProps { resources: Resource[]; templates: ResourceTemplate[]; subscriptions: InspectorResourceSubscription[]; - selectedResourceUri?: string; - selectedTemplateUri?: string; readState?: ReadResourceState; listChanged: boolean; onRefreshList: () => void; - onSelectUri: (uri: string) => void; - onSelectTemplate: (uriTemplate: string) => void; onReadResource: (uri: string) => void; onSubscribeResource: (uri: string) => void; onUnsubscribeResource: (uri: string) => void; @@ -78,17 +75,20 @@ export function ResourcesScreen({ resources, templates, subscriptions, - selectedResourceUri, - selectedTemplateUri, readState, listChanged, onRefreshList, - onSelectUri, - onSelectTemplate, onReadResource, onSubscribeResource, onUnsubscribeResource, }: ResourcesScreenProps) { + const [selectedResourceUri, setSelectedResourceUri] = useState< + string | undefined + >(undefined); + const [selectedTemplateUri, setSelectedTemplateUri] = useState< + string | undefined + >(undefined); + const selectedResource = selectedResourceUri ? resources.find((r) => r.uri === selectedResourceUri) : undefined; @@ -104,6 +104,11 @@ export function ResourcesScreen({ ? { name: readState.uri, uri: readState.uri } : undefined); + function handleReadResource(uri: string) { + setSelectedResourceUri(uri); + onReadResource(uri); + } + function renderReadState() { if (!readState) return null; @@ -136,7 +141,7 @@ export function ResourcesScreen({ contents={readState.result.contents} lastUpdated={readState.lastUpdated} isSubscribed={readState.isSubscribed ?? false} - onRefresh={() => onReadResource(readResource.uri)} + onRefresh={() => handleReadResource(readResource.uri)} onSubscribe={() => onSubscribeResource(readResource.uri)} onUnsubscribe={() => onUnsubscribeResource(readResource.uri)} /> @@ -159,8 +164,8 @@ export function ResourcesScreen({ selectedTemplateUri={selectedTemplateUri} listChanged={listChanged} onRefreshList={onRefreshList} - onSelectUri={onSelectUri} - onSelectTemplate={onSelectTemplate} + onSelectUri={setSelectedResourceUri} + onSelectTemplate={setSelectedTemplateUri} onUnsubscribeResource={onUnsubscribeResource} /> @@ -172,7 +177,7 @@ export function ResourcesScreen({ diff --git a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.stories.tsx b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.stories.tsx index 935fc684..24dc86b7 100644 --- a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.stories.tsx +++ b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.stories.tsx @@ -1,7 +1,7 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import type { ElicitRequest, Tool } from "@modelcontextprotocol/sdk/types.js"; import { Modal } from "@mantine/core"; -import { fn } from "storybook/test"; +import { fn, userEvent, within } from "storybook/test"; import { ToolsScreen } from "./ToolsScreen"; import type { ToolCallState } from "./ToolsScreen"; import { SamplingRequestPanel } from "../../groups/SamplingRequestPanel/SamplingRequestPanel"; @@ -17,7 +17,6 @@ const meta: Meta = { args: { listChanged: false, onRefreshList: fn(), - onSelectTool: fn(), onCallTool: fn(), onCancelCall: fn(), onClearResult: fn(), @@ -77,6 +76,11 @@ const resultState: ToolCallState = { }, }; +async function selectToolByLabel(canvasElement: HTMLElement, label: string) { + const canvas = within(canvasElement); + await userEvent.click(await canvas.findByText(label)); +} + export const NoSelection: Story = { args: { tools: sampleTools, @@ -86,16 +90,20 @@ export const NoSelection: Story = { export const ToolSelected: Story = { args: { tools: sampleTools, - selectedToolName: "create_record", + }, + play: async ({ canvasElement }) => { + await selectToolByLabel(canvasElement, "Create Record"); }, }; export const WithResult: Story = { args: { tools: sampleTools, - selectedToolName: "create_record", callState: resultState, }, + play: async ({ canvasElement }) => { + await selectToolByLabel(canvasElement, "Create Record"); + }, }; export const LongToolName: Story = { @@ -130,15 +138,18 @@ export const LongToolName: Story = { }, }, ], - selectedToolName: + }, + play: async ({ canvasElement }) => { + await selectToolByLabel( + canvasElement, "organization_internal_database_complex_multi_table_join_query_with_aggregation_and_filtering", + ); }, }; export const WithError: Story = { args: { tools: sampleTools, - selectedToolName: "create_record", callState: { status: "error", result: { @@ -152,6 +163,9 @@ export const WithError: Story = { }, }, }, + play: async ({ canvasElement }) => { + await selectToolByLabel(canvasElement, "Create Record"); + }, }; export const WithListChanged: Story = { @@ -176,9 +190,11 @@ const elicitFormRequest = { export const WithSamplingModal: Story = { args: { tools: sampleTools, - selectedToolName: "create_record", callState: { status: "pending" }, }, + play: async ({ canvasElement }) => { + await selectToolByLabel(canvasElement, "Create Record"); + }, render: (args) => ( <> @@ -217,9 +233,11 @@ export const WithSamplingModal: Story = { export const WithElicitationModal: Story = { args: { tools: sampleTools, - selectedToolName: "create_record", callState: { status: "pending" }, }, + play: async ({ canvasElement }) => { + await selectToolByLabel(canvasElement, "Create Record"); + }, render: (args) => ( <> @@ -240,9 +258,11 @@ export const WithElicitationModal: Story = { export const WithPendingRequests: Story = { args: { tools: sampleTools, - selectedToolName: "create_record", callState: { status: "pending" }, }, + play: async ({ canvasElement }) => { + await selectToolByLabel(canvasElement, "Create Record"); + }, render: (args) => ( <> diff --git a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx index e4458eee..875be843 100644 --- a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx +++ b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx @@ -17,11 +17,9 @@ export interface ToolCallState { export interface ToolsScreenProps { tools: Tool[]; - selectedToolName?: string; callState?: ToolCallState; listChanged: boolean; onRefreshList: () => void; - onSelectTool: (name: string) => void; onCallTool: (name: string, args: Record) => void; onCancelCall?: () => void; onClearResult?: () => void; @@ -58,15 +56,16 @@ const EmptyState = Text.withProps({ export function ToolsScreen({ tools, - selectedToolName, callState, listChanged, onRefreshList, - onSelectTool, onCallTool, onCancelCall, onClearResult, }: ToolsScreenProps) { + const [selectedToolName, setSelectedToolName] = useState( + undefined, + ); const [formValues, setFormValues] = useState>({}); const selectedTool = selectedToolName ? tools.find((t) => t.name === selectedToolName) @@ -84,7 +83,7 @@ export function ToolsScreen({ onRefreshList={onRefreshList} onSelectTool={(name) => { setFormValues({}); - onSelectTool(name); + setSelectedToolName(name); }} /> diff --git a/clients/web/src/components/views/InspectorView/InspectorView.tsx b/clients/web/src/components/views/InspectorView/InspectorView.tsx index a15881c1..77815688 100644 --- a/clients/web/src/components/views/InspectorView/InspectorView.tsx +++ b/clients/web/src/components/views/InspectorView/InspectorView.tsx @@ -289,7 +289,6 @@ export function InspectorView({ tools={tools} listChanged={false} onRefreshList={noop} - onSelectTool={noop} onCallTool={noop} /> @@ -298,7 +297,6 @@ export function InspectorView({ prompts={prompts} listChanged={false} onRefreshList={noop} - onSelectPrompt={noop} onGetPrompt={noop} /> @@ -309,8 +307,6 @@ export function InspectorView({ subscriptions={subscriptions} listChanged={false} onRefreshList={noop} - onSelectUri={noop} - onSelectTemplate={noop} onReadResource={noop} onSubscribeResource={noop} onUnsubscribeResource={noop} diff --git a/specification/v2_ux_interfaces_plan.md b/specification/v2_ux_interfaces_plan.md index 26e07a31..2fc71087 100644 --- a/specification/v2_ux_interfaces_plan.md +++ b/specification/v2_ux_interfaces_plan.md @@ -532,6 +532,56 @@ existing stories) rather than a screen refactor. - No prop changes needed. - Optional: add `connectionStatus` if the embedded header needs it. +### 4.3 Move selection state into the screens + +The Phase 2 cluster work threaded `selectedXxxName` / `onSelectXxx` props +through every list-and-detail screen, leaving selection as caller-owned +state. That choice contradicts Phase 3's "selected name stays in `useState` +inside the screen" rule and creates a real wiring gap: nothing above the +screen has any reason to know which tool/prompt/resource the user clicked +on, so under `InspectorView` clicks would have to be round-tripped through +`App.tsx` purely to get the detail panel to render. + +Selection is screen-local UI state. MCP-derived data (`callState` / fetched +prompt messages / resource read results) keeps flowing in from +`InspectorView` as props. Refactor each screen accordingly. + +Screens to fix: + +- **`ToolsScreen`** — drop `selectedToolName` and `onSelectTool` from the + prop interface; track the selected tool name in `useState` inside the + screen. `ToolControls` continues to receive `selectedName` and + `onSelectTool` as props from the screen. `callState` (and any future + result/progress payload) keeps coming in from `InspectorView`. + `InspectorView` only needs to know the selected tool name when it + triggers `onCallTool` — which the screen already owns, so it can pass + the name up at call time rather than maintaining it in the parent. +- **`PromptsScreen`** — same pattern for `selectedPromptName`. Fetched + `GetPromptResult` continues to arrive as a prop from `InspectorView`. +- **`ResourcesScreen`** — same pattern for the selected resource URI and + selected resource template URI. `ReadResourceResult` continues to + arrive as a prop. + +For each screen: + +1. Remove the `selectedXxxName` / `onSelectXxx` props from the screen's + prop interface. +2. Add a `useState` for the selection inside the screen and pass it down + to the controls group. +3. When the user triggers a request (`onCallTool` / `onGetPrompt` / + `onReadResource`), pass the currently selected name/uri up alongside + any other arguments — the parent does not need to track selection + between calls. +4. Update each screen's stories to remove the now-deleted props. The + stories will start working without `useArgs` selection wiring, because + selection is now genuinely internal. +5. Audit the controls groups (`ToolControls`, `PromptControls`, + `ResourceControls`) — their `selectedName` / `onSelectXxx` props are + correct (they ARE caller-owned, because the screen owns them) and + should not change. +6. Update `InspectorView` and any wiring fixtures to drop the now-removed + selection props they were passing to each screen. + ## Phase 5 — Cleanup pass After all four phases, sweep: