Skip to content

Commit 5b54ce1

Browse files
sumeetpardeshisumeetpardeshi
committed
adding fixes as buttons were not visible for streamable-http transport type, as per PR review comment
1 parent 1067f4d commit 5b54ce1

File tree

2 files changed

+156
-95
lines changed

2 files changed

+156
-95
lines changed

‎client/src/components/Sidebar.tsx

Lines changed: 51 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,22 @@ const Sidebar = ({
119119
args: args.trim() ? args.split(/\s+/) : [],
120120
env: { ...env },
121121
};
122-
} else {
122+
}
123+
if (transportType === "sse") {
123124
return {
124125
type: "sse",
125126
url: sseUrl,
126127
note: "For SSE connections, add this URL directly in Client",
127128
};
128129
}
130+
if (transportType === "streamable-http") {
131+
return {
132+
type: "streamable-http",
133+
url: sseUrl,
134+
note: "For Streamable HTTP connections, add this URL directly in Client",
135+
};
136+
}
137+
return {};
129138
}, [transportType, command, args, env, sseUrl]);
130139

131140
// Memoized config entry generator
@@ -266,44 +275,6 @@ const Sidebar = ({
266275
className="font-mono"
267276
/>
268277
</div>
269-
<div className="grid grid-cols-2 gap-2 mt-2">
270-
<Tooltip>
271-
<TooltipTrigger asChild>
272-
<Button
273-
variant="outline"
274-
size="sm"
275-
onClick={handleCopyServerEntry}
276-
className="w-full"
277-
>
278-
{copiedServerEntry ? (
279-
<CheckCheck className="h-4 w-4 mr-2" />
280-
) : (
281-
<Copy className="h-4 w-4 mr-2" />
282-
)}
283-
Server Entry
284-
</Button>
285-
</TooltipTrigger>
286-
<TooltipContent>Copy Server Entry</TooltipContent>
287-
</Tooltip>
288-
<Tooltip>
289-
<TooltipTrigger asChild>
290-
<Button
291-
variant="outline"
292-
size="sm"
293-
onClick={handleCopyServerFile}
294-
className="w-full"
295-
>
296-
{copiedServerFile ? (
297-
<CheckCheck className="h-4 w-4 mr-2" />
298-
) : (
299-
<Copy className="h-4 w-4 mr-2" />
300-
)}
301-
Servers File
302-
</Button>
303-
</TooltipTrigger>
304-
<TooltipContent>Copy Servers File</TooltipContent>
305-
</Tooltip>
306-
</div>
307278
</>
308279
) : (
309280
<>
@@ -319,22 +290,6 @@ const Sidebar = ({
319290
className="font-mono"
320291
/>
321292
</div>
322-
<div className="w-full mt-2">
323-
<Button
324-
variant="outline"
325-
size="sm"
326-
onClick={handleCopyServerFile}
327-
className="w-full"
328-
title="Copy SSE URL Configuration"
329-
>
330-
{copiedServerFile ? (
331-
<CheckCheck className="h-4 w-4 mr-2" />
332-
) : (
333-
<Copy className="h-4 w-4 mr-2" />
334-
)}
335-
Copy Servers File
336-
</Button>
337-
</div>
338293
<div className="space-y-2">
339294
<Button
340295
variant="outline"
@@ -382,6 +337,7 @@ const Sidebar = ({
382337
</div>
383338
</>
384339
)}
340+
385341
{transportType === "stdio" && (
386342
<div className="space-y-2">
387343
<Button
@@ -507,6 +463,46 @@ const Sidebar = ({
507463
</div>
508464
)}
509465

466+
{/* Always show both copy buttons for all transport types */}
467+
<div className="grid grid-cols-2 gap-2 mt-2">
468+
<Tooltip>
469+
<TooltipTrigger asChild>
470+
<Button
471+
variant="outline"
472+
size="sm"
473+
onClick={handleCopyServerEntry}
474+
className="w-full"
475+
>
476+
{copiedServerEntry ? (
477+
<CheckCheck className="h-4 w-4 mr-2" />
478+
) : (
479+
<Copy className="h-4 w-4 mr-2" />
480+
)}
481+
Server Entry
482+
</Button>
483+
</TooltipTrigger>
484+
<TooltipContent>Copy Server Entry</TooltipContent>
485+
</Tooltip>
486+
<Tooltip>
487+
<TooltipTrigger asChild>
488+
<Button
489+
variant="outline"
490+
size="sm"
491+
onClick={handleCopyServerFile}
492+
className="w-full"
493+
>
494+
{copiedServerFile ? (
495+
<CheckCheck className="h-4 w-4 mr-2" />
496+
) : (
497+
<Copy className="h-4 w-4 mr-2" />
498+
)}
499+
Servers File
500+
</Button>
501+
</TooltipTrigger>
502+
<TooltipContent>Copy Servers File</TooltipContent>
503+
</Tooltip>
504+
</div>
505+
510506
{/* Configuration */}
511507
<div className="space-y-2">
512508
<Button

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

Lines changed: 105 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,31 @@ describe("Sidebar Environment Variables", () => {
649649
jest.clearAllTimers();
650650
});
651651

652+
const getCopyButtons = () => {
653+
return {
654+
serverEntry: screen.getByRole("button", { name: /server entry/i }),
655+
serversFile: screen.getByRole("button", { name: /servers file/i }),
656+
};
657+
};
658+
659+
it("should render both copy buttons for all transport types", () => {
660+
["stdio", "sse", "streamable-http"].forEach((transportType) => {
661+
renderSidebar({ transportType });
662+
// There should be exactly one Server Entry and one Servers File button per render
663+
const serverEntryButtons = screen.getAllByRole("button", {
664+
name: /server entry/i,
665+
});
666+
const serversFileButtons = screen.getAllByRole("button", {
667+
name: /servers file/i,
668+
});
669+
expect(serverEntryButtons).toHaveLength(1);
670+
expect(serversFileButtons).toHaveLength(1);
671+
// Clean up DOM for next iteration
672+
// (Testing Library's render does not auto-unmount in a loop)
673+
document.body.innerHTML = "";
674+
});
675+
});
676+
652677
it("should copy server entry configuration to clipboard for STDIO transport", async () => {
653678
const command = "node";
654679
const args = "--inspect server.js";
@@ -661,20 +686,13 @@ describe("Sidebar Environment Variables", () => {
661686
env,
662687
});
663688

664-
// Use act to properly wrap the clipboard operations
665689
await act(async () => {
666-
const copyServerEntryButton = screen.getByRole("button", {
667-
name: /server entry/i,
668-
});
669-
fireEvent.click(copyServerEntryButton);
670-
671-
// Fast-forward timers to handle the setTimeout
690+
const { serverEntry } = getCopyButtons();
691+
fireEvent.click(serverEntry);
672692
jest.runAllTimers();
673693
});
674694

675-
// Check clipboard API was called with the correct configuration
676695
expect(mockClipboardWrite).toHaveBeenCalledTimes(1);
677-
678696
const expectedConfig = JSON.stringify(
679697
{
680698
command,
@@ -684,7 +702,6 @@ describe("Sidebar Environment Variables", () => {
684702
null,
685703
2,
686704
);
687-
688705
expect(mockClipboardWrite).toHaveBeenCalledWith(expectedConfig);
689706
});
690707

@@ -701,18 +718,12 @@ describe("Sidebar Environment Variables", () => {
701718
});
702719

703720
await act(async () => {
704-
const copyServersFileButton = screen.getByRole("button", {
705-
name: /servers file/i,
706-
});
707-
fireEvent.click(copyServersFileButton);
708-
709-
// Fast-forward timers to handle the setTimeout
721+
const { serversFile } = getCopyButtons();
722+
fireEvent.click(serversFile);
710723
jest.runAllTimers();
711724
});
712725

713-
// Check clipboard API was called with the correct configuration
714726
expect(mockClipboardWrite).toHaveBeenCalledTimes(1);
715-
716727
const expectedConfig = JSON.stringify(
717728
{
718729
mcpServers: {
@@ -726,31 +737,43 @@ describe("Sidebar Environment Variables", () => {
726737
null,
727738
2,
728739
);
729-
730740
expect(mockClipboardWrite).toHaveBeenCalledWith(expectedConfig);
731741
});
732742

733-
it("should copy servers file configuration to clipboard for SSE transport", async () => {
743+
it("should copy server entry configuration to clipboard for SSE transport", async () => {
734744
const sseUrl = "http://localhost:3000/events";
745+
renderSidebar({ transportType: "sse", sseUrl });
735746

736-
renderSidebar({
737-
transportType: "sse",
738-
sseUrl,
747+
await act(async () => {
748+
const { serverEntry } = getCopyButtons();
749+
fireEvent.click(serverEntry);
750+
jest.runAllTimers();
739751
});
740752

741-
await act(async () => {
742-
const copyServersFileButton = screen.getByRole("button", {
743-
name: /servers file/i,
744-
});
745-
fireEvent.click(copyServersFileButton);
753+
expect(mockClipboardWrite).toHaveBeenCalledTimes(1);
754+
const expectedConfig = JSON.stringify(
755+
{
756+
type: "sse",
757+
url: sseUrl,
758+
note: "For SSE connections, add this URL directly in Client",
759+
},
760+
null,
761+
2,
762+
);
763+
expect(mockClipboardWrite).toHaveBeenCalledWith(expectedConfig);
764+
});
746765

747-
// Fast-forward timers to handle the setTimeout
766+
it("should copy servers file configuration to clipboard for SSE transport", async () => {
767+
const sseUrl = "http://localhost:3000/events";
768+
renderSidebar({ transportType: "sse", sseUrl });
769+
770+
await act(async () => {
771+
const { serversFile } = getCopyButtons();
772+
fireEvent.click(serversFile);
748773
jest.runAllTimers();
749774
});
750775

751-
// Check clipboard API was called with the correct configuration
752776
expect(mockClipboardWrite).toHaveBeenCalledTimes(1);
753-
754777
const expectedConfig = JSON.stringify(
755778
{
756779
mcpServers: {
@@ -764,7 +787,56 @@ describe("Sidebar Environment Variables", () => {
764787
null,
765788
2,
766789
);
790+
expect(mockClipboardWrite).toHaveBeenCalledWith(expectedConfig);
791+
});
792+
793+
it("should copy server entry configuration to clipboard for streamable-http transport", async () => {
794+
const sseUrl = "http://localhost:3001/sse";
795+
renderSidebar({ transportType: "streamable-http", sseUrl });
796+
797+
await act(async () => {
798+
const { serverEntry } = getCopyButtons();
799+
fireEvent.click(serverEntry);
800+
jest.runAllTimers();
801+
});
802+
803+
expect(mockClipboardWrite).toHaveBeenCalledTimes(1);
804+
const expectedConfig = JSON.stringify(
805+
{
806+
type: "streamable-http",
807+
url: sseUrl,
808+
note: "For Streamable HTTP connections, add this URL directly in Client",
809+
},
810+
null,
811+
2,
812+
);
813+
expect(mockClipboardWrite).toHaveBeenCalledWith(expectedConfig);
814+
});
815+
816+
it("should copy servers file configuration to clipboard for streamable-http transport", async () => {
817+
const sseUrl = "http://localhost:3001/sse";
818+
renderSidebar({ transportType: "streamable-http", sseUrl });
819+
820+
await act(async () => {
821+
const { serversFile } = getCopyButtons();
822+
fireEvent.click(serversFile);
823+
jest.runAllTimers();
824+
});
767825

826+
expect(mockClipboardWrite).toHaveBeenCalledTimes(1);
827+
const expectedConfig = JSON.stringify(
828+
{
829+
mcpServers: {
830+
"default-server": {
831+
type: "streamable-http",
832+
url: sseUrl,
833+
note: "For Streamable HTTP connections, add this URL directly in Client",
834+
},
835+
},
836+
},
837+
null,
838+
2,
839+
);
768840
expect(mockClipboardWrite).toHaveBeenCalledWith(expectedConfig);
769841
});
770842

@@ -779,18 +851,12 @@ describe("Sidebar Environment Variables", () => {
779851
});
780852

781853
await act(async () => {
782-
const copyServerEntryButton = screen.getByRole("button", {
783-
name: /server entry/i,
784-
});
785-
fireEvent.click(copyServerEntryButton);
786-
787-
// Fast-forward timers to handle the setTimeout
854+
const { serverEntry } = getCopyButtons();
855+
fireEvent.click(serverEntry);
788856
jest.runAllTimers();
789857
});
790858

791-
// Check clipboard API was called with empty args array
792859
expect(mockClipboardWrite).toHaveBeenCalledTimes(1);
793-
794860
const expectedConfig = JSON.stringify(
795861
{
796862
command,
@@ -800,7 +866,6 @@ describe("Sidebar Environment Variables", () => {
800866
null,
801867
2,
802868
);
803-
804869
expect(mockClipboardWrite).toHaveBeenCalledWith(expectedConfig);
805870
});
806871
});

0 commit comments

Comments
 (0)