Skip to content

Conversation

msabramo
Copy link
Contributor

@msabramo msabramo commented Jun 4, 2025

Motivation and Context

A quick way to see the full URL if it's cut off

Inspired by @cliffhall's comment

Screenshot 2025-06-03 at 11 35 55 PM

How Has This Been Tested?

  • Set Transport Type to SSE or Streamable HTTP
  • Hover over URL when URL is blank - displays tooltip with Enter URL to MCP server
  • Paste a long URL into URL field; hover over URL - displays tooltip with full URL.

Breaking Changes

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

A quick way to see the full URL if it's cut off

Inspired by @cliffhall's
[comment](modelcontextprotocol#459 (comment))
Copy link
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Display tooltip if sseUrl is set.

Comment on lines 288 to 301
<Tooltip>
<TooltipTrigger asChild>
<Input
id="sse-url-input"
placeholder="URL"
value={sseUrl}
onChange={(e) => setSseUrl(e.target.value)}
className="font-mono"
/>
</TooltipTrigger>
<TooltipContent>
{sseUrl || `Enter URL to MCP server`}
</TooltipContent>
</Tooltip>
Copy link
Member

@cliffhall cliffhall Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make the tooltip only display if sseUrl is not null/undefined. It would seem odd to have this one place where a tooltip comes up over an empty field.

Suggested change
<Tooltip>
<TooltipTrigger asChild>
<Input
id="sse-url-input"
placeholder="URL"
value={sseUrl}
onChange={(e) => setSseUrl(e.target.value)}
className="font-mono"
/>
</TooltipTrigger>
<TooltipContent>
{sseUrl || `Enter URL to MCP server`}
</TooltipContent>
</Tooltip>
{ sseUrl && <Tooltip>
<TooltipTrigger asChild>
<Input
id="sse-url-input"
placeholder="URL"
value={sseUrl}
onChange={(e) => setSseUrl(e.target.value)}
className="font-mono"
/>
</TooltipTrigger>
<TooltipContent>
{sseUrl}
</TooltipContent>
</Tooltip> }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 2ecfdb0.

@msabramo
Copy link
Contributor Author

msabramo commented Jun 5, 2025

Display tooltip if sseUrl is set.

Done in 2ecfdb0.

@msabramo msabramo requested a review from cliffhall June 5, 2025 07:26
cliffhall
cliffhall previously approved these changes Jun 5, 2025
Copy link
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but needs a prettier-fix run to pass CI.

@msabramo
Copy link
Contributor Author

msabramo commented Jun 6, 2025

Looks good, but needs a prettier-fix run to pass CI.

Updated. Thanks!

@msabramo msabramo requested a review from cliffhall June 6, 2025 13:39
Copy link
Member

@cliffhall cliffhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

Screenshot 2025-06-06 at 1 37 46 PM

@cliffhall cliffhall merged commit 03edbb0 into modelcontextprotocol:main Jun 6, 2025
2 checks passed
@msabramo msabramo deleted the tooltip-for-url-field branch June 7, 2025 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants