Skip to content

Conversation

bfmvsa
Copy link

@bfmvsa bfmvsa commented Jul 19, 2025

Added loading state indication to buttons. Implemented loading/loadingText props for the button component and updated the Connect/Restart buttons.

Motivation and Context

Users had no visual feedback when clicking buttons that triggered connect/restart operations, leading to confusion and potential duplicate clicks. This change provides immediate loading feedback.

How Has This Been Tested?

  • Tested loading states on the button component
  • Verified default "Loading..." text and custom loading text
  • Tested both buttons with loading prop
  • Confirmed buttons are disabled during loading

Breaking Changes

None.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

screenshot.2025-07-19.at.18.17.44.mov

@olishiz
Copy link

olishiz commented Aug 1, 2025

This looks good!

@richardkmichael
Copy link
Collaborator

I like this too. Usability ✨ are great additions. 👍

What about going all the way, and indicating exactly what was clicked?

diff --git a/client/src/components/Sidebar.tsx b/client/src/components/Sidebar.tsx
index 134397c..bdd785c 100644
--- a/client/src/components/Sidebar.tsx
+++ b/client/src/components/Sidebar.tsx
@@ -243,6 +243,10 @@ const Sidebar = ({
     }
   }, [generateMCPServerFile, toast, reportError]);
 
+  const connectButtonConfig = transportType === "stdio"
+    ? { label: "Restart", loadingText: "Restarting" }
+    : { label: "Reconnect", loadingText: "Reconnecting" };
+
   return (
     <div className="bg-card border-r border-border flex flex-col h-full">
       <div className="flex items-center justify-between p-4 border-b border-gray-200 dark:border-border">
@@ -700,9 +704,10 @@ const Sidebar = ({
                   data-testid="connect-button"
                   onClick={handleRestart}
                   loading={isConnecting}
+                  loadingText={connectButtonConfig.loadingText}
                 >
                   <RotateCcw className="w-4 h-4 mr-2" />
-                  {transportType === "stdio" ? "Restart" : "Reconnect"}
+                  {connectButtonConfig.label}
                 </Button>
                 <Button onClick={onDisconnect}>
                   <RefreshCwOff className="w-4 h-4 mr-2" />
@@ -715,6 +720,7 @@ const Sidebar = ({
                 className="w-full"
                 onClick={handleConnect}
                 loading={isConnecting}
+                loadingText="Connecting"
               >
                 <Play className="w-4 h-4 mr-2" />
                 Connect

@olaservo
Copy link
Member

Hi @bfmvsa I think this is a great idea, however I was testing this locally and ran into this situation:

  • I tried connecting to a server over STDIO with incorrect arguments
  • Since the button stayed in a "Loading" state while the connection kept failing, there was no way to retry with new args by clicking Connect again, until I force quit inspector

Could we add something to handle that scenario more gracefully?

@olaservo olaservo added the enhancement New feature request label Aug 12, 2025
@bfmvsa
Copy link
Author

bfmvsa commented Aug 12, 2025

Hi @olaservo, good catch!
I was able to reproduce the issue. I'll try to fix this.

BTW, the same issue is the root of the problem in the current release

screenshot.2025-08-12.at.15.42.37.mp4

@olaservo olaservo added the waiting on submitter Waiting for the submitter to provide more info label Aug 18, 2025
@olaservo
Copy link
Member

olaservo commented Sep 1, 2025

@claude could you help suggest how to handle the scenario described in the comments, where incorrect arguments will lead to the user getting stuck in a 'loading' state unless they restart the app?

(Edit - forgot that Claude currently can't access branches from forks)

Copy link

claude bot commented Sep 1, 2025

Claude encountered an error —— View job

Failed with exit code 128

I'll analyze this and get back to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request waiting on submitter Waiting for the submitter to provide more info
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants