Repository navigation
feat(ui): display server icons in the registry web client - #1586
tounsils wants to merge 1 commit into
Conversation
Sorry, something went wrong.
Servers can declare `icons` in server.json and the schema has supported it for some time, but the web client never rendered them, so every card looked identical apart from its text. Closes modelcontextprotocol#784. Selection, in order: - HTTPS only. The publish API already rejects other schemes, but this UI can be pointed at a staging, local or custom API base URL from its own settings, so the scheme is re-checked at render time rather than trusted. - `theme: "dark"` icons are used only when nothing else is offered, since this UI has a light background. - Scalable icons (`sizes: ["any"]`) win outright. Otherwise the smallest icon at least as wide as the rendered size is chosen, so it stays crisp without fetching a needlessly large file. Rendering notes: - `src` goes through the existing `escapeAttr`, so a quote in a URL cannot break out of the attribute. - `alt` is empty on purpose. The icon sits directly beside the server name, so announcing it again is noise for screen readers. - `loading="lazy"` keeps a grid of cards from fetching every icon at once, and `referrerpolicy="no-referrer"` avoids leaking registry URLs to icon hosts. - A broken or blocked image removes itself instead of leaving a placeholder. Verified against the first 100 servers in the production registry, which exercises every shape currently published: no `sizes` field, a single 512x512, `["any"]`, and a 256/512 pair. Also checked that http, `javascript:`, `data:` and null sources render nothing, and that a quote injected into `src` is escaped. Unit tests pass, including internal/api/handlers/v0, which is the package that embeds this file. `make validate` and `make lint` both fail identically on a clean checkout of upstream main with these changes absent, so those failures are pre-existing rather than introduced here; the lint ones are gofmt objecting to CRLF from a Windows checkout. Tests ran without `-race`, which needs cgo and a C toolchain this machine does not have.
|
Following up after three weeks, since this hasn't picked up a reviewer yet. It's still a single file at +54 lines and it closes #784. main has moved since I opened it, so I'm happy to rebase onto current main if that's what's holding it up. Equally happy to narrow the scope if the icon-selection rules are more than you want in the web client right now. The core of the change is just rendering icons on the cards; the size and theme selection could land separately, or drop to "first HTTPS icon" if that's simpler to review. Let me know either way and I'll turn it around quickly. |
Sorry, something went wrong.
|
Flagging an overlap on my own PR so a maintainer doesn't have to find it: #1208 implements this same feature and has been open since April. I missed it when I opened this one. The two overlap almost entirely, and each has one thing the other lacks:
Both enforce HTTPS at render time, escape the src, set an empty alt next to the server name, and use loading="lazy" with referrerpolicy="no-referrer". #1208 came first and is the better base of the two. If that's the direction you'd prefer, I'm happy to close this and send the size-selection logic as a follow-up on top of it, or as a patch to that branch if @MukundaKatta would rather take it that way. Either is fine by me, I'd just rather one of these land than both sit. |
Sorry, something went wrong.
Closes #784.
Servers can declare icons in server.json and the schema has supported it for some time, but the web client never rendered them, so every card looks identical apart from its text. This renders them in both the "Recently Updated" cards and the main search results.
Selection
Rendering
SVG is rendered via <img src>, where scripts do not execute, which is the safe path for the image/svg+xml case the schema warns about.
Testing
Ran the selection logic against the first 100 servers in the production registry. That covers every icon shape currently published:
Also verified that http://, javascript:, data: and null sources render nothing, that a dark-only icon set still renders, that light is preferred when both are offered, and that a quote injected into src is escaped rather than breaking the attribute.
Unit tests pass, including internal/api/handlers/v0, the package that embeds this file.
Two pre-existing failures, noted for transparency: make validate and make lint both fail identically on a clean checkout of main with these changes absent. The lint ones are gofmt objecting to CRLF from a Windows checkout. Neither is introduced here and neither is touched by this change. Tests ran without -race, which needs cgo and a C toolchain unavailable on my machine.