Skip to content

Commit 2e7098a

Browse files
authored
Merge pull request #645 from aesslinger/fix/plugin-manifest-icon-priority
fix: getDriverIcon never checked for a URL/data: manifest icon (#632)
2 parents 5b1abe3 + e00a189 commit 2e7098a

8 files changed

Lines changed: 205 additions & 57 deletions

File tree

File renamed without changes.

src/components/modals/NewConnectionModal.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ import {
6969
type ConnectionTestProgressPayload,
7070
} from "../../utils/connectionTest";
7171
import { fetchConnectionWithCredentials } from "../../utils/credentials";
72-
import { getDriverIcon, getDriverColorStyle } from "../../utils/driverUI";
72+
import { getDriverIcon, getDriverColorStyle, isUrlIcon } from "../../utils/driverUI";
7373
import {
7474
parseConnectionString,
7575
toConnectionParams,
@@ -326,7 +326,7 @@ export const NewConnectionModal = ({
326326
"#64748b";
327327
const renderDriverGlyph = (size: number) => {
328328
const icon = activeCatalogueDriver?.icon ?? activeDriver?.icon ?? "";
329-
if (/^https?:\/\//.test(icon) || icon.startsWith("data:")) {
329+
if (isUrlIcon(icon)) {
330330
return (
331331
<img
332332
src={icon}

src/components/modals/connection/EngineCard.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ import { useTranslation } from "react-i18next";
66
import type { PluginManifest } from "../../../types/plugins";
77
import type { CatalogueDriver, EngineGroup } from "../../../utils/connectionCatalogue";
88
import { labelForParadigm } from "../../../utils/connectionCatalogue";
9-
import { getDriverIcon } from "../../../utils/driverUI";
10-
import { RegistryDriverIcon } from "./RegistryDriverIcon";
9+
import { getDriverIcon, isUrlIcon } from "../../../utils/driverUI";
10+
import { RegistryDriverIcon } from "../../RegistryDriverIcon";
1111

1212
interface EngineCardProps {
1313
group: EngineGroup;
@@ -33,7 +33,7 @@ function accentFor(group: EngineGroup, rep: CatalogueDriver): string {
3333

3434
function renderIcon(rep: CatalogueDriver) {
3535
const icon = rep.icon ?? "";
36-
if (/^https?:\/\//.test(icon) || icon.startsWith("data:")) {
36+
if (isUrlIcon(icon)) {
3737
return <RegistryDriverIcon src={icon} size={24} fallback={<Database size={20} />} />;
3838
}
3939
if (rep.isBuiltin) {

src/components/modals/connection/InstallGate.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ import { useState } from "react";
44
import { useTranslation } from "react-i18next";
55

66
import type { CatalogueDriver } from "../../../utils/connectionCatalogue";
7+
import { isUrlIcon } from "../../../utils/driverUI";
78
import { PluginReadmeModal } from "../PluginReadmeModal";
8-
import { RegistryDriverIcon } from "./RegistryDriverIcon";
9+
import { RegistryDriverIcon } from "../../RegistryDriverIcon";
910

1011
export type InstallStatus = "idle" | "installing" | "error";
1112

@@ -33,7 +34,7 @@ function accentFor(driver: CatalogueDriver): string {
3334

3435
function renderIcon(driver: CatalogueDriver) {
3536
const icon = driver.icon ?? "";
36-
if (/^https?:\/\//.test(icon) || icon.startsWith("data:")) {
37+
if (isUrlIcon(icon)) {
3738
return <RegistryDriverIcon src={icon} size={32} fallback={<Database size={26} />} />;
3839
}
3940
return <Database size={26} />;

src/types/plugins.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,10 @@ export interface PluginManifest {
9595
default_username?: string;
9696
/** CSS hex color for UI accents (e.g. "#f97316"). Undefined falls back to a neutral color. */
9797
color?: string;
98-
/** Icon name: built-in values are "mysql" | "postgres" | "sqlite" | "network" | "database" | "folder-open".
99-
* External plugins can reference a file bundled in the plugin package. */
98+
/** Icon: a hosted URL (`https://...`) or `data:` URI, resolved to an
99+
* `<img>` (see the Tabularium manifest docs); or one of the built-in
100+
* lookup keys "mysql" | "postgres" | "sqlite" | "network" | "database" |
101+
* "folder-open", resolved to an inline brand/lucide icon. */
100102
icon?: string;
101103
/** Plugin-declared setting definitions. Empty/absent for built-in drivers. */
102104
settings?: PluginSettingDefinition[];

src/utils/driverUI.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { ReactNode } from "react";
55
import type { PluginManifest } from "../types/plugins";
66
import type { SavedConnection } from "../contexts/DatabaseContext";
77
import { PostgreSQLIcon, MySQLIcon, SQLiteIcon } from "./driverIcons";
8+
import { RegistryDriverIcon } from "../components/RegistryDriverIcon";
89

910
const FALLBACK_COLOR = "#64748b"; // slate-500
1011

@@ -15,13 +16,27 @@ export function getDriverColor(manifest: PluginManifest | undefined | null): str
1516
return manifest?.color || FALLBACK_COLOR;
1617
}
1718

19+
/**
20+
* True when a manifest `icon` value is a hosted URL or `data:` URI to render
21+
* as an `<img>`, rather than a built-in icon lookup key. URI schemes are
22+
* case-insensitive (RFC 3986), so `HTTPS://` must match too.
23+
*/
24+
export function isUrlIcon(icon: string): boolean {
25+
return /^(https?:\/\/|data:)/i.test(icon);
26+
}
27+
1828
/**
1929
* Returns a ReactNode icon for a driver.
20-
* Priority: brand SVG icon → lucide icon → generic fallback.
30+
* Priority: manifest-supplied URL/data: URI icon → brand SVG icon → lucide
31+
* icon → generic fallback.
2132
*/
2233
export function getDriverIcon(manifest: PluginManifest | undefined | null, size = 14): ReactNode {
2334
const iconName = manifest?.icon || "";
2435

36+
if (isUrlIcon(iconName)) {
37+
return <RegistryDriverIcon src={iconName} size={size} fallback={<Plug size={size} />} />;
38+
}
39+
2540
// Brand icons for built-in drivers
2641
switch (iconName) {
2742
case "postgres":
Lines changed: 6 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,12 @@
1-
import { describe, it, expect, vi } from "vitest";
2-
import { render, screen, waitFor } from "@testing-library/react";
3-
import { getConnectionAccent, getConnectionIcon } from "./driverUI";
4-
import { camelToKebab, getLucideIconComponent, CONNECTION_ICON_PACK } from "./connectionIconPack";
5-
import type { SavedConnection } from "../contexts/DatabaseContext";
6-
import type { PluginManifest } from "../types/plugins";
7-
8-
// Avoid loading the lazy ConnectionIconImage during tests (it pulls Tauri APIs that aren't available in vitest)
9-
vi.mock("../components/ConnectionIconImage", () => ({
10-
ConnectionIconImage: (props: { path: string; size: number }) =>
11-
<img data-testid="conn-icon-image" alt="" src={`mock://${props.path}`} width={props.size} height={props.size} />,
12-
}));
1+
import { describe, it, expect } from "vitest";
2+
import { render } from "@testing-library/react";
3+
import { camelToKebab, getLucideIconComponent, CONNECTION_ICON_PACK } from "../../src/utils/connectionIconPack";
4+
import { getConnectionIcon } from "../../src/utils/driverUI";
5+
import type { SavedConnection } from "../../src/contexts/DatabaseContext";
6+
import type { PluginManifest } from "../../src/types/plugins";
137

148
const manifest = { id: "mysql", color: "#0000ff", icon: "database" } as unknown as PluginManifest;
159

16-
describe("getConnectionAccent", () => {
17-
it("uses override when present", () => {
18-
const c = { appearance: { accentColor: "#ff0000" } } as SavedConnection;
19-
expect(getConnectionAccent(c, manifest)).toBe("#ff0000");
20-
});
21-
it("falls back to manifest color when override missing", () => {
22-
expect(getConnectionAccent({} as SavedConnection, manifest)).toBe("#0000ff");
23-
});
24-
it("falls back to grey when both missing", () => {
25-
expect(getConnectionAccent(null, null)).toBe("#64748b");
26-
});
27-
});
28-
29-
describe("getConnectionIcon", () => {
30-
it("renders emoji when override is emoji", () => {
31-
const c = { id: "1", appearance: { icon: { type: "emoji", value: "🐘" } } } as SavedConnection;
32-
render(<>{getConnectionIcon(c, manifest, 16)}</>);
33-
expect(screen.getByText("🐘")).toBeInTheDocument();
34-
});
35-
it("falls back to manifest icon when override missing", () => {
36-
// Smoke test: should not throw; icons are mocked to null in test env
37-
expect(() => render(<>{getConnectionIcon({ id: "1" } as SavedConnection, manifest, 16)}</>)).not.toThrow();
38-
});
39-
it("renders the mocked image component for image overrides", async () => {
40-
const c = { id: "1", appearance: { icon: { type: "image", path: "connection-icons/foo.png" } } } as SavedConnection;
41-
render(<>{getConnectionIcon(c, manifest, 16)}</>);
42-
await waitFor(() => expect(screen.getByTestId("conn-icon-image")).toBeInTheDocument());
43-
});
44-
it("falls back to manifest when pack id is unknown", () => {
45-
const c = { id: "1", appearance: { icon: { type: "pack", id: "this-icon-does-not-exist-xyz" } } } as SavedConnection;
46-
// Smoke test: should not throw; icons are mocked to null in test env
47-
expect(() => render(<>{getConnectionIcon(c, manifest, 16)}</>)).not.toThrow();
48-
});
49-
});
50-
5110
describe("camelToKebab / getLucideIconComponent — legacy id normalization", () => {
5211
it("converts camelCase to kebab-case correctly", () => {
5312
expect(camelToKebab("shieldCheck")).toBe("shield-check");

tests/utils/driverUI.test.tsx

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
import { describe, it, expect, vi } from "vitest";
2+
import { render, screen, waitFor } from "@testing-library/react";
3+
import {
4+
getConnectionAccent,
5+
getConnectionIcon,
6+
getDriverIcon,
7+
isUrlIcon,
8+
} from "../../src/utils/driverUI";
9+
import type { PluginManifest } from "../../src/types/plugins";
10+
import type { SavedConnection } from "../../src/contexts/DatabaseContext";
11+
12+
// Avoid loading the lazy ConnectionIconImage during tests (it pulls Tauri APIs that aren't available in vitest)
13+
vi.mock("../../src/components/ConnectionIconImage", () => ({
14+
ConnectionIconImage: (props: { path: string; size: number }) =>
15+
<img data-testid="conn-icon-image" alt="" src={`mock://${props.path}`} width={props.size} height={props.size} />,
16+
}));
17+
18+
describe("isUrlIcon", () => {
19+
it("matches http(s) URLs and data: URIs regardless of scheme case", () => {
20+
expect(isUrlIcon("http://example.com/icon.svg")).toBe(true);
21+
expect(isUrlIcon("https://example.com/icon.svg")).toBe(true);
22+
expect(isUrlIcon("HTTPS://example.com/icon.svg")).toBe(true);
23+
expect(isUrlIcon("data:image/png;base64,iVBORw0KGgo=")).toBe(true);
24+
expect(isUrlIcon("DATA:image/png;base64,iVBORw0KGgo=")).toBe(true);
25+
});
26+
27+
it("rejects built-in lookup keys and non-URL strings", () => {
28+
expect(isUrlIcon("postgres")).toBe(false);
29+
// "database" starts with "data" but is not a data: URI
30+
expect(isUrlIcon("database")).toBe(false);
31+
expect(isUrlIcon("ftp://example.com/icon.svg")).toBe(false);
32+
expect(isUrlIcon("")).toBe(false);
33+
});
34+
});
35+
36+
/**
37+
* getDriverIcon's priority is documented as: manifest-supplied URL/data:
38+
* URI icon -> brand SVG icon -> lucide icon -> generic fallback (issue
39+
* #632). getConnectionIcon layers a per-connection override on top of that,
40+
* for the full 3-tier priority @debba specified: Connection Custom Icon >
41+
* Manifest Icon (if present) > Plugin Icon.
42+
*/
43+
describe("getDriverIcon", () => {
44+
const manifest = (icon?: string): PluginManifest =>
45+
({ icon } as PluginManifest);
46+
47+
const renderIcon = (icon?: string, size = 14) =>
48+
render(<div data-testid="wrap">{getDriverIcon(manifest(icon), size)}</div>);
49+
50+
it("renders a registry <img> for an https:// manifest icon", () => {
51+
const { getByTestId } = renderIcon("https://example.com/icon.svg");
52+
const img = getByTestId("wrap").querySelector("img");
53+
expect(img).not.toBeNull();
54+
expect(img?.getAttribute("src")).toBe("https://example.com/icon.svg");
55+
});
56+
57+
it("renders a registry <img> for a data: URI manifest icon", () => {
58+
const dataUri = "data:image/png;base64,iVBORw0KGgo=";
59+
const { getByTestId } = renderIcon(dataUri);
60+
const img = getByTestId("wrap").querySelector("img");
61+
expect(img).not.toBeNull();
62+
expect(img?.getAttribute("src")).toBe(dataUri);
63+
});
64+
65+
it("still renders the built-in Postgres brand icon for the literal string 'postgres'", () => {
66+
const { getByTestId } = renderIcon("postgres");
67+
expect(getByTestId("wrap").querySelector("svg")).not.toBeNull();
68+
expect(getByTestId("wrap").querySelector("img")).toBeNull();
69+
});
70+
71+
it("still falls through to the legacy lucide branch (not the URL branch) for the literal string 'database'", () => {
72+
// lucide-react icons are globally mocked to render null in this test
73+
// environment (see tests/setup.ts), so we can't distinguish *which*
74+
// lucide icon rendered by inspecting the DOM — but we CAN confirm it
75+
// didn't take the new URL/data: branch (no <img>) and isn't one of the
76+
// hand-written brand SVGs (no real <svg>, since those aren't mocked).
77+
const { getByTestId } = renderIcon("database");
78+
expect(getByTestId("wrap").querySelector("img")).toBeNull();
79+
expect(getByTestId("wrap").querySelector("svg")).toBeNull();
80+
});
81+
82+
it("falls back to the generic Plug branch for an unrecognized non-URL string", () => {
83+
const { getByTestId } = renderIcon("some-random-value");
84+
expect(getByTestId("wrap").querySelector("img")).toBeNull();
85+
expect(getByTestId("wrap").querySelector("svg")).toBeNull();
86+
});
87+
88+
it("falls back to the generic Plug branch when no icon is set at all", () => {
89+
const { getByTestId } = renderIcon(undefined);
90+
expect(getByTestId("wrap").querySelector("img")).toBeNull();
91+
expect(getByTestId("wrap").querySelector("svg")).toBeNull();
92+
});
93+
});
94+
95+
describe("getConnectionAccent", () => {
96+
const manifest = { id: "mysql", color: "#0000ff", icon: "database" } as unknown as PluginManifest;
97+
98+
it("uses override when present", () => {
99+
const c = { appearance: { accentColor: "#ff0000" } } as SavedConnection;
100+
expect(getConnectionAccent(c, manifest)).toBe("#ff0000");
101+
});
102+
it("falls back to manifest color when override missing", () => {
103+
expect(getConnectionAccent({} as SavedConnection, manifest)).toBe("#0000ff");
104+
});
105+
it("falls back to grey when both missing", () => {
106+
expect(getConnectionAccent(null, null)).toBe("#64748b");
107+
});
108+
});
109+
110+
describe("getConnectionIcon", () => {
111+
const manifest = (icon?: string): PluginManifest => ({ icon } as PluginManifest);
112+
const connection = (
113+
icon?: SavedConnection["appearance"] extends { icon?: infer T } ? T : never,
114+
): Pick<SavedConnection, "appearance"> => (icon ? { appearance: { icon } } : {});
115+
116+
const renderConnectionIcon = (
117+
conn: Pick<SavedConnection, "appearance">,
118+
icon?: string,
119+
size = 14,
120+
) =>
121+
render(
122+
<div data-testid="wrap">{getConnectionIcon(conn, manifest(icon), size)}</div>,
123+
);
124+
125+
it("prefers a Connection Custom Icon (emoji override) over a URL manifest icon", () => {
126+
const { getByTestId } = renderConnectionIcon(
127+
connection({ type: "emoji", value: "🐘" }),
128+
"https://example.com/icon.svg",
129+
);
130+
expect(getByTestId("wrap").textContent).toBe("🐘");
131+
expect(getByTestId("wrap").querySelector("img")).toBeNull();
132+
});
133+
134+
it("falls back to the Manifest Icon (URL) when there is no connection override", () => {
135+
const { getByTestId } = renderConnectionIcon(
136+
connection(undefined),
137+
"https://example.com/icon.svg",
138+
);
139+
const img = getByTestId("wrap").querySelector("img");
140+
expect(img?.getAttribute("src")).toBe("https://example.com/icon.svg");
141+
});
142+
143+
it("falls back to the Plugin Icon (built-in brand icon) when there is neither a connection override nor a manifest icon URL", () => {
144+
const { getByTestId } = renderConnectionIcon(connection(undefined), "postgres");
145+
expect(getByTestId("wrap").querySelector("svg")).not.toBeNull();
146+
expect(getByTestId("wrap").querySelector("img")).toBeNull();
147+
});
148+
149+
it("renders emoji when override is emoji", () => {
150+
const c = { id: "1", appearance: { icon: { type: "emoji", value: "🐘" } } } as SavedConnection;
151+
render(<>{getConnectionIcon(c, manifest("database"), 16)}</>);
152+
expect(screen.getByText("🐘")).toBeInTheDocument();
153+
});
154+
155+
it("falls back to manifest icon when override missing", () => {
156+
// Smoke test: should not throw; icons are mocked to null in test env
157+
expect(() => render(<>{getConnectionIcon({ id: "1" } as SavedConnection, manifest("database"), 16)}</>)).not.toThrow();
158+
});
159+
160+
it("renders the mocked image component for image overrides", async () => {
161+
const c = { id: "1", appearance: { icon: { type: "image", path: "connection-icons/foo.png" } } } as SavedConnection;
162+
render(<>{getConnectionIcon(c, manifest("database"), 16)}</>);
163+
await waitFor(() => expect(screen.getByTestId("conn-icon-image")).toBeInTheDocument());
164+
});
165+
166+
it("falls back to manifest when pack id is unknown", () => {
167+
const c = { id: "1", appearance: { icon: { type: "pack", id: "this-icon-does-not-exist-xyz" } } } as SavedConnection;
168+
// Smoke test: should not throw; icons are mocked to null in test env
169+
expect(() => render(<>{getConnectionIcon(c, manifest("database"), 16)}</>)).not.toThrow();
170+
});
171+
});

0 commit comments

Comments
 (0)