Skip to content

Commit d7c3ef3

Browse files
cliffhallcliffhall
committed
In AppsTab.tsx, if an app has no inputSchema, just show it, otherwise show the form and the "back to form" button when showing the app.
* In AppsTab.test.tsx - test new behavior * In ListPane.tsx - remove the Clear button * In ListPane.test.tsx - remove test for clear button
1 parent 8b70df5 commit d7c3ef3

4 files changed

Lines changed: 127 additions & 45 deletions

File tree

‎client/src/components/AppsTab.tsx‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,10 @@ const AppsTab = ({
136136

137137
const handleSelectTool = useCallback((tool: Tool) => {
138138
setSelectedTool(tool);
139-
setIsAppOpen(false);
139+
const hasFields =
140+
tool.inputSchema.properties &&
141+
Object.keys(tool.inputSchema.properties).length > 0;
142+
setIsAppOpen(!hasFields);
140143
}, []);
141144

142145
const handleDeselectTool = useCallback(() => {
@@ -158,7 +161,6 @@ const AppsTab = ({
158161
<ListPane
159162
items={appTools}
160163
listItems={handleRefresh}
161-
clearItems={handleDeselectTool}
162164
setSelectedItem={handleSelectTool}
163165
renderItem={(tool) => {
164166
const resourceUri = getToolUiResourceUri(tool) || "";
@@ -174,7 +176,7 @@ const AppsTab = ({
174176
{tool.description}
175177
</span>
176178
)}
177-
<span className="text-[10px] text-muted-foreground font-mono mt-1 truncate">
179+
<span className="text-[10px] text-muted-foreground text-right font-mono mt-1 truncate">
178180
{resourceUri}
179181
</span>
180182
</div>
@@ -242,6 +244,9 @@ const AppsTab = ({
242244
(() => {
243245
const resourceUri = getToolUiResourceUri(selectedTool) || "";
244246
const content = resourceContentMap[resourceUri] || "";
247+
const hasFields =
248+
selectedTool.inputSchema.properties &&
249+
Object.keys(selectedTool.inputSchema.properties).length > 0;
245250

246251
return (
247252
<div className="space-y-4">
@@ -482,15 +487,17 @@ const AppsTab = ({
482487
</div>
483488
) : (
484489
<div className="space-y-4">
485-
<div className="flex justify-end">
486-
<Button
487-
onClick={handleCloseApp}
488-
variant="outline"
489-
size="sm"
490-
>
491-
Back to Input
492-
</Button>
493-
</div>
490+
{hasFields && (
491+
<div className="flex justify-end">
492+
<Button
493+
onClick={handleCloseApp}
494+
variant="outline"
495+
size="sm"
496+
>
497+
Back to Input
498+
</Button>
499+
</div>
500+
)}
494501
<div className="h-[600px]">
495502
<AppRenderer
496503
tool={selectedTool}

‎client/src/components/ListPane.tsx‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { useState, useMemo, useRef } from "react";
66
type ListPaneProps<T> = {
77
items: T[];
88
listItems: () => void;
9-
clearItems: () => void;
9+
clearItems?: () => void;
1010
setSelectedItem: (item: T) => void;
1111
renderItem: (item: T) => React.ReactNode;
1212
title: string;
@@ -101,14 +101,16 @@ const ListPane = <T extends object>({
101101
>
102102
{buttonText}
103103
</Button>
104-
<Button
105-
variant="outline"
106-
className="w-full mb-4"
107-
onClick={clearItems}
108-
disabled={items.length === 0}
109-
>
110-
Clear
111-
</Button>
104+
{clearItems && (
105+
<Button
106+
variant="outline"
107+
className="w-full mb-4"
108+
onClick={clearItems}
109+
disabled={items.length === 0}
110+
>
111+
Clear
112+
</Button>
113+
)}
112114
<div className="space-y-2 overflow-y-auto max-h-96">
113115
{filteredItems.map((item, index) => (
114116
<div

‎client/src/components/__tests__/AppsTab.test.tsx‎

Lines changed: 90 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,24 @@ describe("AppsTab", () => {
137137
expect(screen.getByText(errorMessage)).toBeInTheDocument();
138138
});
139139

140-
it("should open app renderer when an app card is clicked and Open App button is clicked", () => {
140+
it("should open app renderer when an app card is clicked and Open App button is clicked if fields exist", () => {
141+
const toolWithFields: Tool = {
142+
name: "fieldsApp",
143+
inputSchema: {
144+
type: "object",
145+
properties: {
146+
field1: { type: "string" },
147+
},
148+
},
149+
_meta: { ui: { resourceUri: "ui://fields" } },
150+
} as Tool & { _meta?: { ui?: { resourceUri?: string } } };
151+
141152
renderAppsTab({
142-
tools: [mockAppTool],
143-
resourceContentMap: { "ui://weather-app": "<html>test</html>" },
153+
tools: [toolWithFields],
154+
resourceContentMap: { "ui://fields": "<html>test</html>" },
144155
});
145156

146-
const appCard = screen.getByText("weatherApp").closest("div");
157+
const appCard = screen.getByText("fieldsApp").closest("div");
147158
expect(appCard).toBeTruthy();
148159
fireEvent.click(appCard!);
149160

@@ -157,7 +168,7 @@ describe("AppsTab", () => {
157168

158169
// AppRenderer should be rendered
159170
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();
160-
expect(screen.getByText("Tool: weatherApp")).toBeInTheDocument();
171+
expect(screen.getByText("Tool: fieldsApp")).toBeInTheDocument();
161172
});
162173

163174
it("should close app renderer when close button is clicked", () => {
@@ -169,7 +180,7 @@ describe("AppsTab", () => {
169180
// Open the app
170181
const appCard = screen.getByText("weatherApp").closest("div");
171182
fireEvent.click(appCard!);
172-
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
183+
// weatherApp has no properties in mockAppTool, so it should be open immediately
173184
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();
174185

175186
// Close the app (Deselect tool)
@@ -190,7 +201,7 @@ describe("AppsTab", () => {
190201
// Open the app
191202
const appCard = screen.getByText("weatherApp").closest("div");
192203
fireEvent.click(appCard!);
193-
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
204+
// weatherApp has no properties in mockAppTool, so it should be open immediately
194205

195206
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();
196207
expect(screen.getByText(`Content: ${resourceContent}`)).toBeInTheDocument();
@@ -227,7 +238,7 @@ describe("AppsTab", () => {
227238
// Select the app
228239
const appCard = screen.getByText("weatherApp").closest("div");
229240
fireEvent.click(appCard!);
230-
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
241+
// weatherApp has no properties in mockAppTool, so it should be open immediately
231242
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();
232243

233244
// Update tools list to remove the selected tool
@@ -254,7 +265,7 @@ describe("AppsTab", () => {
254265
// Select the app
255266
const appCard = screen.getByText("weatherApp").closest("div");
256267
fireEvent.click(appCard!);
257-
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
268+
// weatherApp has no properties in mockAppTool, so it should be open immediately
258269
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();
259270

260271
// Update tools list with the same tool
@@ -280,14 +291,7 @@ describe("AppsTab", () => {
280291
// Select the app
281292
const appCard = screen.getByText("weatherApp").closest("div");
282293
fireEvent.click(appCard!);
283-
284-
// Initially, Maximize button should not be visible (app not open)
285-
expect(
286-
screen.queryByRole("button", { name: /maximize/i }),
287-
).not.toBeInTheDocument();
288-
289-
// Open the app
290-
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
294+
// weatherApp has no properties in mockAppTool, so it should be open immediately
291295

292296
// Now Maximize button should be visible
293297
expect(
@@ -320,9 +324,7 @@ describe("AppsTab", () => {
320324
// Select the app
321325
const appCard = screen.getByText("weatherApp").closest("div");
322326
fireEvent.click(appCard!);
323-
324-
// Open the app
325-
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
327+
// weatherApp has no properties in mockAppTool, so it should be open immediately
326328

327329
// Maximize
328330
fireEvent.click(screen.getByRole("button", { name: /maximize/i }));
@@ -353,7 +355,7 @@ describe("AppsTab", () => {
353355
// Open the app
354356
const appCard = screen.getByText("weatherApp").closest("div");
355357
fireEvent.click(appCard!);
356-
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
358+
// weatherApp has no properties in mockAppTool, so it should be open immediately
357359

358360
// AppRenderer should still be rendered but with no content
359361
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();
@@ -433,11 +435,76 @@ describe("AppsTab", () => {
433435
});
434436

435437
it("should allow going back to input form from app renderer", () => {
438+
const toolWithFields: Tool = {
439+
name: "fieldsApp",
440+
inputSchema: {
441+
type: "object",
442+
properties: {
443+
field1: { type: "string" },
444+
},
445+
},
446+
_meta: { ui: { resourceUri: "ui://fields" } },
447+
} as Tool & { _meta?: { ui?: { resourceUri?: string } } };
448+
436449
renderAppsTab({
437-
tools: [mockAppTool],
450+
tools: [toolWithFields],
451+
});
452+
453+
fireEvent.click(screen.getByText("fieldsApp"));
454+
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
455+
456+
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();
457+
458+
const backButton = screen.queryByRole("button", { name: /back to input/i });
459+
expect(backButton).toBeInTheDocument();
460+
});
461+
462+
it("should skip input form if tool has no input fields", () => {
463+
const toolNoFields: Tool = {
464+
name: "noFieldsApp",
465+
inputSchema: {
466+
type: "object",
467+
properties: {},
468+
},
469+
_meta: { ui: { resourceUri: "ui://no-fields" } },
470+
} as Tool & { _meta?: { ui?: { resourceUri?: string } } };
471+
472+
renderAppsTab({
473+
tools: [toolNoFields],
438474
});
439475

440-
fireEvent.click(screen.getByText("weatherApp"));
476+
fireEvent.click(screen.getByText("noFieldsApp"));
477+
478+
// Should see the renderer immediately
479+
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();
480+
expect(screen.getByText("Tool: noFieldsApp")).toBeInTheDocument();
481+
482+
// Should NOT see the back button
483+
expect(
484+
screen.queryByRole("button", { name: /back to input/i }),
485+
).not.toBeInTheDocument();
486+
});
487+
488+
it("should allow going back to input form from app renderer if fields exist", () => {
489+
const toolWithFields: Tool = {
490+
name: "fieldsApp",
491+
inputSchema: {
492+
type: "object",
493+
properties: {
494+
field1: { type: "string" },
495+
},
496+
},
497+
_meta: { ui: { resourceUri: "ui://fields" } },
498+
} as Tool & { _meta?: { ui?: { resourceUri?: string } } };
499+
500+
renderAppsTab({
501+
tools: [toolWithFields],
502+
});
503+
504+
fireEvent.click(screen.getByText("fieldsApp"));
505+
506+
// Should see input form first
507+
expect(screen.getByText("App Input")).toBeInTheDocument();
441508
fireEvent.click(screen.getByRole("button", { name: /open app/i }));
442509

443510
expect(screen.getByTestId("app-renderer")).toBeInTheDocument();

‎client/src/components/__tests__/ListPane.test.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ describe("ListPane", () => {
1313
const defaultProps = {
1414
items: mockItems,
1515
listItems: jest.fn(),
16-
clearItems: jest.fn(),
1716
setSelectedItem: jest.fn(),
1817
renderItem: (item: (typeof mockItems)[0]) => <div>{item.name}</div>,
1918
title: "List tools",
@@ -36,6 +35,13 @@ describe("ListPane", () => {
3635
expect(
3736
screen.getByRole("button", { name: "Load Tools" }),
3837
).toBeInTheDocument();
38+
expect(
39+
screen.queryByRole("button", { name: "Clear" }),
40+
).not.toBeInTheDocument();
41+
});
42+
43+
it("should render Clear button when clearItems prop is provided", () => {
44+
renderListPane({ clearItems: jest.fn() });
3945
expect(screen.getByRole("button", { name: "Clear" })).toBeInTheDocument();
4046
});
4147

0 commit comments

Comments
 (0)