Skip to content

fix(ui): make provider icons follow the theme and open the icon picker in the content pane - #8438

Open
Owlbay wants to merge 3 commits into
farion1231:mainfrom
Owlbay:fix/provider-icon-theme
Open

Owlbay wants to merge 3 commits into
farion1231:mainfrom
Owlbay:fix/provider-icon-theme

Conversation

@Owlbay

@Owlbay Owlbay commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Provider cards no longer force a white tile behind every icon in dark mode. Near-black strokes/fills in inline SVGs are rendered with currentColor; a near-black default icon color is left to the surrounding text color. Self-contained icons (tile icons, icons with their own light ink) are untouched.
  • The icon picker opens as a FullScreenPanel over the content area instead of a full-window dialog, so the sidebar and window controls stay in place and the back button no longer overlaps the traffic lights.
  • The upload hint in the icon picker is plain text under the grid instead of a long inverted tooltip.

Test plan

  • pnpm typecheck, prettier
  • IconPicker / BasicFormFields / ProviderCard / ProviderIcon(Box) unit tests
  • Visual check of provider list and icon picker in light and dark mode
  • Note: UsageDashboard and rememberedChoice tests fail on main as well, unrelated to this change.

…r in the content pane

Provider cards no longer force a white tile behind every icon in dark mode.
Near-black strokes in inline SVGs are rendered with currentColor, and a
near-black default icon color is left to the surrounding text color.

The icon picker now opens as a FullScreenPanel over the content area instead
of a full-window dialog, so the sidebar and window controls stay in place.
The upload hint is shown as text instead of a long tooltip.
@Owlbay
Owlbay requested a review from farion1231 as a code owner October 16, 2026 18:19
@github-actions github-actions Bot added the frontend Frontend (React/TypeScript) label Oct 16, 2026

@farion1231 farion1231 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this — moving the icon picker into the content pane so the back button stops colliding with the window controls is a nice fix, and the merge tree passes typecheck and the full unit suite. I checked every built-in icon (96 inline SVGs, 45 URL icons) plus the picker flow against main and found three regressions:

[P2] Esc in the icon picker now closes the whole provider editor

Both the editor and the new picker are FullScreenPanels, and each one registers its own window keydown listener (FullScreenPanel.tsx#L123). stopPropagation() does not stop other listeners on the same target, and the outer panel registered first, so it runs first. Opening the picker leaves focus on the icon button behind the panel, so pressing Esc reaches the outer handlePanelClose (EditProviderDialog.tsx#L129): the edit page closes and unsaved edits are lost (on the Add page it jumps back to the preset step). I reproduced it with BasicFormFields inside a FullScreenPanel: on this branch the outer onClose fires once; on main the Radix dialog moves focus inside and consumes Esc, so the outer panel is not touched.
Fix: let only the topmost panel handle Esc (e.g. a small open-panel stack in FullScreenPanel), or have the picker panel handle Esc in the capture phase with stopImmediatePropagation(), and move focus into the panel when it opens.

[P2] adaptSvgToTheme breaks multi-color and tile icons, in light mode too

index.ts#L228-L260 swaps dark fills for currentColor, but ProviderIcon sets style.color to the icon's defaultColor, so the swapped parts get the brand color:

  • Hugging Face (used by presets): the eyes and mouth (#3A3B45) become #FFD21E, the same yellow as the face, so the face has no features in either theme. LionCC is affected the same way.
  • OpenClaw (the sidebar app icon): the eyes (#050810) become #ff4f40, the same red as the body.
  • lum < 60 on raw sRGB counts saturated reds as near-black: Huawei (#C7000B, and its #FF0000 default color) renders entirely in the text color, and Stability's red part does too.
  • Tile icons aren't excluded: SSAI Code's background <rect fill="#08080e"> becomes currentColor, so in dark mode it turns into a light-gray tile with pale glyphs.

Fix: only recolor single-ink icons (every painted color is near-black). Skip anything with other non-gray colors, skip isTileIcon, and use a real near-black test (e.g. max channel ≤ ~0x40) instead of weighted luminance.

[P2] Without the white tile, icons that can't be recolored become invisible on dark cards

ProviderCard.tsx#L329 removes the background for every icon, but several have no themable path:

  • Raster icons with a transparent background and dark ink, e.g. atlascloud (used by presets: a solid black "A" on transparent).
  • Custom uploaded icons: transparency is kept when the file is encoded, and they render through <img>, so a black logo on a transparent background disappears.
  • Novita and OpenCode: the fill="white" inside their <clipPath>/<mask> defs counts as "light ink", so adaptSvgToTheme skips them and #000000 / #211E1E stay as they are.

Fix: keep a contrasting background in dark mode for URL/custom icons (and for any SVG the adapter leaves untouched), and ignore paints inside <defs>, <clipPath> and <mask> when deciding whether to adapt.

Once these three are fixed I'm happy to take another look quickly.

- Esc closes only the most recently opened FullScreenPanel, so the icon
  picker no longer takes the provider editor down with it.
- Only near-black paints (all channels low) are recolored; masks, clip paths
  and defs are ignored, tile icons are skipped, and recolored icons no longer
  get the brand color applied over them.
- Keep a light backing in dark mode for raster/uploaded icons and SVGs that
  still carry near-black ink after adaptation.
@Owlbay

Owlbay commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks for the thorough check. Pushed a follow-up commit:

  • Esc: FullScreenPanel keeps a stack of open panels and only the most recently opened one handles Esc (test added).
  • Recoloring: only near-black paints (all channels ≤ 0x50, so saturated reds like Huawei are not affected) are swapped to currentColor; paints inside <defs>/<clipPath>/<mask> are ignored, tile icons are skipped, and icons that were recolored no longer get defaultColor applied (Hugging Face / OpenClaw keep distinct eyes).
  • Backing: raster/URL non-tile icons, uploaded icons, and SVGs that still contain near-black ink after adaptation get a white backing in dark mode.

🤖 Addressed by Claude Code

@farion1231 farion1231 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the quick follow-up. I re-checked ab99c57 merged into current main (typecheck and the full unit suite pass) and rendered every built-in icon again in light and dark mode against main. Esc now closes only the icon picker, saturated reds (Huawei, Stability) and tile icons are left alone, and raster / uploaded icons as well as Novita and OpenCode now read fine on the dark card. One case from the last round is still off:

[P2] Hugging Face loses its eyes and mouth in dark mode

Its face details are #3A3B45 drawn on a #FFD21E face. They are now swapped to currentColor, which on a dark card resolves to the light text color (#e4e4e7). Light-on-yellow is about 1.1:1 contrast, so at card size the eyes and mouth practically disappear (light mode is fine). Hugging Face is a visible preset in six apps, and the icon picker grid shows the same thing. LionCC has the same pattern (#111111 linework on #F9DA3C).

  • Recolor logic:
    export function adaptSvgToTheme(svg: string): string {
    const cached = adaptedSvgCache.get(svg);
    if (cached !== undefined) return cached;
    const paints = paintedColors(svg);
    const skip =
    paints.some((c) => c.trim().toLowerCase() === "currentcolor") ||
    paints.some(isLightInk);
    const result = skip
    ? svg
    : svg
    .split(MASK_BLOCK)
    // split 带捕获组:[正文, 整块, 标签名, 正文, ...],只改正文
    .map((part, i) =>
    i % 3 === 0
    ? part.replace(PAINT_ATTR, (whole, attr, color) =>
    isDarkInk(color) ? `${attr}="currentColor"` : whole,
    )
    : part,
    )
    .join("");
    adaptedSvgCache.set(svg, result);
    return result;
    }
    /**
  • The new test asserts this recolor:
    it("recolors near-black paints in multi-color icons", () => {
    const out = adaptSvgToTheme(getIcon("huggingface"));
    expect(out).toContain('fill="currentColor"');
    expect(out).toContain("#FFD21E");
    });

One way to handle it: when near-black paint sits next to other non-near-black fills, treat the icon like the "has light ink" case and leave it as-is, so needsLightBacking gives it the light backing instead (something along these lines, untested):

const skip =
  paints.some((c) => c.trim().toLowerCase() === "currentcolor") ||
  paints.some(isLightInk) ||
  // dark details on a coloured fill (Hugging Face, LionCC): keep the original colors and back it instead
  (paints.some(isDarkInk) && paints.some((c) => c.startsWith("#") && !isDarkInk(c)));

nit: split(MASK_BLOCK) re-inserts the tag name into the SVG

MASK_BLOCK has two capture groups, so split yields [text, block, tagName, text, ...], and the .join("") puts the tag name back as bare text, e.g. ...</linearGradient></defs>defs<path .... That affects 21 built-in SVGs. It is not rendered, but it also makes adapted true for icons where nothing was recolored (Azure, Gemini, Meta, Qwen, ...), so their defaultColor gets dropped. Skipping the tag-name parts fixes it:

.split(MASK_BLOCK)
.filter((_, i) => i % 3 !== 2)
.map((part, i) => (i % 2 === 0 ? part.replace(PAINT_ATTR, ...) : part))

Happy to re-check quickly once the Hugging Face case is addressed.

Multi-color icons keep their near-black details (Hugging Face eyes, LionCC
linework), which read fine on the colored shapes. Inline SVGs no longer get a
light backing; only raster/URL icons do.
@Owlbay

Owlbay commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Good catch. adaptSvgToTheme now only recolors single-ink icons (every painted color near-black); multi-color icons such as Hugging Face and LionCC keep their dark details on the colored shapes. The test now asserts they are unchanged, and inline SVGs no longer get a light backing (only raster/URL icons do).

🤖 Addressed by Claude Code

@Owlbay

Owlbay commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator Author

@farion1231 the follow-up from the second review is pushed (multi-color icons are left untouched, only single-ink icons are recolored). Could you take another look when you have time?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Frontend (React/TypeScript)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants