Skip to content

Commit 1b99ed6

Browse files
its-mashclaude
andcommitted
fix: render server icon URLs as images instead of raw text
Server definitions use HTTP URLs for the `icon` field (e.g., GitHub avatars), but ServerDetailModal and ServerInstallModal rendered them as raw text. Created a shared ServerIcon component that detects URL icons and renders them as <img> elements with error fallback. - Add ServerIcon component with URL detection and error handling - Fix ServerDetailModal to use ServerIcon instead of raw text - Fix ServerInstallModal to use ServerIcon instead of raw text - Refactor ServerCard to use shared ServerIcon (was inline) - Add unit tests for ServerIcon, ServerCard, ServerDetailModal - Add E2E tests verifying icons render as images in registry Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Mohammod Al Amin Ashik <maa.ashik00@gmail.com>
1 parent afd4010 commit 1b99ed6

8 files changed

Lines changed: 484 additions & 9 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* Shared server icon component that handles both URL-based and emoji icons.
3+
*
4+
* Server definitions may have an `icon` field that is either:
5+
* - An HTTP(S) URL to an image (e.g., GitHub avatar)
6+
* - An emoji string (e.g., "📦")
7+
* - null/undefined
8+
*/
9+
10+
import { useState } from 'react';
11+
12+
interface ServerIconProps {
13+
icon: string | null | undefined;
14+
/** CSS classes for the img element when rendering a URL icon */
15+
className?: string;
16+
/** Fallback emoji when icon is missing or fails to load (default: '📦') */
17+
fallback?: string;
18+
}
19+
20+
export function ServerIcon({ icon, className = 'w-9 h-9 object-contain', fallback = '📦' }: ServerIconProps) {
21+
const [failed, setFailed] = useState(false);
22+
23+
if (!icon || failed) {
24+
return <span data-testid="server-icon-fallback">{fallback}</span>;
25+
}
26+
27+
if (icon.startsWith('http')) {
28+
return (
29+
<img
30+
src={icon}
31+
alt=""
32+
className={className}
33+
data-testid="server-icon-img"
34+
onError={() => setFailed(true)}
35+
/>
36+
);
37+
}
38+
39+
return <span data-testid="server-icon-emoji">{icon}</span>;
40+
}

apps/desktop/src/components/ServerInstallModal.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
} from '@/lib/api/registry';
3030
import type { ServerDefinition } from '@/types/registry';
3131
import { useViewSpace } from '@/stores';
32+
import { ServerIcon } from '@/components/ServerIcon';
3233

3334
/** Deep link payload from backend */
3435
interface ServerInstallDeepLinkPayload {
@@ -243,9 +244,9 @@ export function ServerInstallModal() {
243244
{/* Server Info */}
244245
<div className="p-4 rounded-lg bg-surface-hover border border-[rgb(var(--border))]" data-testid="install-modal-server-info">
245246
<div className="flex items-center gap-3">
246-
{server.icon && (
247-
<span className="text-2xl">{server.icon}</span>
248-
)}
247+
<div className="flex-shrink-0 flex items-center justify-center text-2xl">
248+
<ServerIcon icon={server.icon} className="w-8 h-8 object-contain rounded" />
249+
</div>
249250
<div className="flex-1 min-w-0">
250251
<div className="font-medium text-lg" data-testid="install-modal-server-name">{server.name}</div>
251252
{server.description && (

apps/desktop/src/features/registry/ServerCard.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import type { ServerViewModel } from '../../types/registry';
6+
import { ServerIcon } from '../../components/ServerIcon';
67

78
interface ServerCardProps {
89
server: ServerViewModel;
@@ -103,11 +104,7 @@ export function ServerCard({
103104
{/* Header */}
104105
<div className="flex items-start gap-3 mb-3">
105106
<div className="text-3xl flex-shrink-0 flex items-center justify-center">
106-
{server.icon?.startsWith('http') ? (
107-
<img src={server.icon} alt="" className="w-9 h-9 object-contain" onError={(e) => { e.currentTarget.style.display = 'none'; e.currentTarget.parentElement!.append(document.createTextNode('📦')); }} />
108-
) : (
109-
server.icon || '📦'
110-
)}
107+
<ServerIcon icon={server.icon} />
111108
</div>
112109
<div className="flex-1 min-w-0">
113110
<div className="flex items-center gap-1.5">

apps/desktop/src/features/registry/ServerDetailModal.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import type { ServerViewModel } from '../../types/registry';
6+
import { ServerIcon } from '../../components/ServerIcon';
67

78
interface ServerDetailModalProps {
89
server: ServerViewModel;
@@ -31,7 +32,9 @@ export function ServerDetailModal({
3132
<div className="dropdown-menu relative w-full max-w-lg max-h-[90vh] overflow-hidden animate-in fade-in scale-in duration-150">
3233
{/* Header */}
3334
<div className="flex items-start gap-4 p-6 border-b border-[rgb(var(--border))]">
34-
<div className="text-5xl">{server.icon || '📦'}</div>
35+
<div className="flex-shrink-0 flex items-center justify-center">
36+
<ServerIcon icon={server.icon} className="w-12 h-12 object-contain rounded-lg" />
37+
</div>
3538
<div className="flex-1 min-w-0">
3639
<div className="flex items-center gap-2 flex-wrap">
3740
<h2 className="text-xl font-bold">

tests/e2e/specs/registry.spec.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,80 @@ test.describe('Registry/Discover Page', () => {
8686
});
8787
});
8888

89+
test.describe('Registry Server Icon Rendering', () => {
90+
test('should render server icons as images not raw URLs', async ({ page }) => {
91+
const dashboard = new DashboardPage(page);
92+
93+
await dashboard.navigate();
94+
await page.locator('nav button:has-text("Discover")').click();
95+
96+
// Wait for content to load
97+
await page.waitForTimeout(500);
98+
99+
// Server cards with URL icons should render img elements, not raw URL text
100+
const serverIconImages = page.locator('[data-testid="server-icon-img"]');
101+
const serverIconFallbacks = page.locator('[data-testid="server-icon-fallback"]');
102+
const serverIconEmojis = page.locator('[data-testid="server-icon-emoji"]');
103+
104+
const imgCount = await serverIconImages.count();
105+
const fallbackCount = await serverIconFallbacks.count();
106+
const emojiCount = await serverIconEmojis.count();
107+
108+
// At least some icons should be rendered (either as img or fallback/emoji)
109+
expect(imgCount + fallbackCount + emojiCount).toBeGreaterThan(0);
110+
111+
// Verify img elements have valid src attributes
112+
if (imgCount > 0) {
113+
const firstImg = serverIconImages.first();
114+
const src = await firstImg.getAttribute('src');
115+
expect(src).toMatch(/^https?:\/\//);
116+
}
117+
118+
// Ensure no raw URL text is shown in place of icons
119+
const cardTexts = await page.locator('[data-testid^="server-card-"]').allTextContents();
120+
for (const text of cardTexts) {
121+
expect(text).not.toMatch(/^https?:\/\/avatars\./);
122+
}
123+
});
124+
125+
test('should render icon as img in server detail modal', async ({ page }) => {
126+
const dashboard = new DashboardPage(page);
127+
const registry = new RegistryPage(page);
128+
129+
await dashboard.navigate();
130+
await page.locator('nav button:has-text("Discover")').click();
131+
await page.waitForTimeout(500);
132+
133+
// Click first server card to open detail modal
134+
const firstCard = page.locator('[data-testid^="server-card-"]').first();
135+
if (await firstCard.isVisible().catch(() => false)) {
136+
await firstCard.click();
137+
await page.waitForTimeout(300);
138+
139+
// The detail modal should render icons properly
140+
const modalIconImg = page.locator('.fixed [data-testid="server-icon-img"]');
141+
const modalIconFallback = page.locator('.fixed [data-testid="server-icon-fallback"]');
142+
const modalIconEmoji = page.locator('.fixed [data-testid="server-icon-emoji"]');
143+
144+
const hasImg = await modalIconImg.isVisible().catch(() => false);
145+
const hasFallback = await modalIconFallback.isVisible().catch(() => false);
146+
const hasEmoji = await modalIconEmoji.isVisible().catch(() => false);
147+
148+
// At least one icon rendering approach should be used
149+
expect(hasImg || hasFallback || hasEmoji).toBe(true);
150+
151+
// If img, verify it has valid src
152+
if (hasImg) {
153+
const src = await modalIconImg.getAttribute('src');
154+
expect(src).toMatch(/^https?:\/\//);
155+
}
156+
157+
// Close modal
158+
await page.keyboard.press('Escape');
159+
}
160+
});
161+
});
162+
89163
test.describe('Registry Filters and Sorting', () => {
90164
test('should have filter elements', async ({ page }) => {
91165
const dashboard = new DashboardPage(page);
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import { describe, it, expect, vi } from 'vitest';
2+
import { render, screen, fireEvent } from '@testing-library/react';
3+
import { ServerCard } from '../../../apps/desktop/src/features/registry/ServerCard';
4+
import type { ServerViewModel } from '../../../apps/desktop/src/types/registry';
5+
6+
function makeServer(overrides: Partial<ServerViewModel> = {}): ServerViewModel {
7+
return {
8+
id: 'com.test-server',
9+
name: 'Test Server',
10+
description: 'A test MCP server',
11+
alias: 'test',
12+
icon: null,
13+
auth: { type: 'none' },
14+
transport: {
15+
type: 'http',
16+
url: 'https://example.com/mcp',
17+
headers: {},
18+
metadata: { inputs: [] },
19+
},
20+
categories: ['developer-tools'],
21+
publisher: null,
22+
source: { type: 'Registry', url: 'https://registry.mcpmux.com', name: 'McpMux Registry' },
23+
is_installed: false,
24+
enabled: false,
25+
oauth_connected: false,
26+
input_values: {},
27+
connection_status: 'disconnected',
28+
missing_required_inputs: false,
29+
last_error: null,
30+
...overrides,
31+
};
32+
}
33+
34+
describe('ServerCard', () => {
35+
const defaultProps = {
36+
onInstall: vi.fn(),
37+
onUninstall: vi.fn(),
38+
onViewDetails: vi.fn(),
39+
};
40+
41+
describe('icon rendering', () => {
42+
it('should render fallback icon when icon is null', () => {
43+
const server = makeServer({ icon: null });
44+
render(<ServerCard server={server} {...defaultProps} />);
45+
expect(screen.getByTestId('server-icon-fallback')).toHaveTextContent('📦');
46+
});
47+
48+
it('should render emoji icon as text', () => {
49+
const server = makeServer({ icon: '🔐' });
50+
render(<ServerCard server={server} {...defaultProps} />);
51+
expect(screen.getByTestId('server-icon-emoji')).toHaveTextContent('🔐');
52+
});
53+
54+
it('should render URL icon as img element', () => {
55+
const server = makeServer({
56+
icon: 'https://avatars.githubusercontent.com/u/314135?v=4',
57+
});
58+
render(<ServerCard server={server} {...defaultProps} />);
59+
const img = screen.getByTestId('server-icon-img');
60+
expect(img.tagName).toBe('IMG');
61+
expect(img).toHaveAttribute(
62+
'src',
63+
'https://avatars.githubusercontent.com/u/314135?v=4'
64+
);
65+
});
66+
67+
it('should show fallback when image fails to load', () => {
68+
const server = makeServer({
69+
icon: 'https://example.com/broken-icon.png',
70+
});
71+
render(<ServerCard server={server} {...defaultProps} />);
72+
const img = screen.getByTestId('server-icon-img');
73+
fireEvent.error(img);
74+
expect(screen.getByTestId('server-icon-fallback')).toHaveTextContent('📦');
75+
});
76+
});
77+
78+
describe('server info', () => {
79+
it('should render server name', () => {
80+
const server = makeServer({ name: 'GitHub MCP Server' });
81+
render(<ServerCard server={server} {...defaultProps} />);
82+
expect(screen.getByText('GitHub MCP Server')).toBeInTheDocument();
83+
});
84+
85+
it('should render description', () => {
86+
const server = makeServer({ description: 'Manage GitHub repos' });
87+
render(<ServerCard server={server} {...defaultProps} />);
88+
expect(screen.getByText('Manage GitHub repos')).toBeInTheDocument();
89+
});
90+
91+
it('should render categories', () => {
92+
const server = makeServer({
93+
categories: ['cloud', 'developer-tools', 'productivity'],
94+
});
95+
render(<ServerCard server={server} {...defaultProps} />);
96+
expect(screen.getByText('cloud')).toBeInTheDocument();
97+
expect(screen.getByText('developer-tools')).toBeInTheDocument();
98+
expect(screen.getByText('productivity')).toBeInTheDocument();
99+
});
100+
101+
it('should truncate categories beyond 3', () => {
102+
const server = makeServer({
103+
categories: ['cloud', 'developer-tools', 'productivity', 'extra'],
104+
});
105+
render(<ServerCard server={server} {...defaultProps} />);
106+
expect(screen.getByText('+1')).toBeInTheDocument();
107+
});
108+
});
109+
110+
describe('actions', () => {
111+
it('should render Install button for non-installed server', () => {
112+
const server = makeServer({ is_installed: false });
113+
render(<ServerCard server={server} {...defaultProps} />);
114+
expect(screen.getByText('Install')).toBeInTheDocument();
115+
});
116+
117+
it('should render Uninstall button for installed server', () => {
118+
const server = makeServer({ is_installed: true });
119+
render(<ServerCard server={server} {...defaultProps} />);
120+
expect(screen.getByText('Uninstall')).toBeInTheDocument();
121+
});
122+
123+
it('should call onViewDetails when card is clicked', () => {
124+
const server = makeServer();
125+
render(<ServerCard server={server} {...defaultProps} />);
126+
fireEvent.click(screen.getByTestId(`server-card-${server.id}`));
127+
expect(defaultProps.onViewDetails).toHaveBeenCalledWith(server);
128+
});
129+
});
130+
});

0 commit comments

Comments
 (0)