Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates various provider icons by adding or modifying SVG files under public/providers/ and updating ProviderIcon.tsx to support an external SVG fallback from thesvg.org. Feedback on these changes highlights that prioritizing external SVGs over local PNGs introduces performance, reliability, and privacy concerns, and suggests prioritizing local assets instead. Additionally, the reviewer notes that using a standard <img> tag for the external SVG will trigger ESLint errors unless an ESLint disable comment is added, and points out that unit tests should be added to tests/ to accompany the production code changes under src/ as required by the repository style guide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const [remoteSrcFailed, setRemoteSrcFailed] = useState(false); | ||
| const pngKey = `${normalizedId}:png`; | ||
| const svgKey = `${normalizedId}:svg`; | ||
| const usePng = !lobeIcon && hasPng && !failedAssets[pngKey]; | ||
| const useSvg = !lobeIcon && hasSvg && !failedAssets[svgKey] && (!hasPng || failedAssets[pngKey]); | ||
| const theSvgKey = `${normalizedId}:thesvg`; |
There was a problem hiding this comment.
According to the Repository Style Guide (Rule 9), tests must always be included when changing production code under src/. Since ProviderIcon.tsx has been modified to support external SVG fallback logic, please ensure that corresponding unit tests are added or updated in the tests/ directory to maintain test coverage.
References
- Always include tests when changing production code (src/, open-sse/, electron/, bin/). (link)
| <img | ||
| src={`https://thesvg.org/icons/${normalizedId}/default.svg`} | ||
| alt={providerId} | ||
| width={size} | ||
| height={size} | ||
| style={{ objectFit: "contain" }} | ||
| onError={() => { | ||
| setFailedAssets((current) => ({ ...current, [pngKey]: true })); | ||
| }} | ||
| unoptimized | ||
| style={{ objectFit: "contain", flex: "none" }} | ||
| onError={() => setFailedAssets((current) => ({ ...current, [theSvgKey]: true }))} | ||
| /> |
There was a problem hiding this comment.
Using a standard <img> tag without an ESLint disable comment will trigger the @next/next/no-img-element rule, which can fail the build or CI checks. Since this is an external SVG from a third-party CDN, using <img> is appropriate, but we should add the ESLint disable comment as done elsewhere in this file.
{/* eslint-disable-next-line @next/next/no-img-element -- external SVG from thesvg.org, not a static/known asset */}
<img
src={`https://thesvg.org/icons/${normalizedId}/default.svg`}
alt={providerId}
width={size}
height={size}
style={{ objectFit: "contain", flex: "none" }}
onError={() => setFailedAssets((current) => ({ ...current, [theSvgKey]: true }))}
/>
ef0eeab to
0d02aa4
Compare
0d02aa4 to
63a6e24
Compare
- Adds thesvg.org to the external SVG resolution chain. - Includes eslint-disable comment for next/image external img.
3931f51 to
e1182f2
Compare
Summary
Migrates the application to prioritize local SVG icons over fetching from LobeHub or falling back to static PNGs.
Problem
Built-in providers currently rely on third-party remote assets (LobeHub) or pixelated static PNG files. This introduces unnecessary network latency, potential visual degradation on high-res displays, and a point of failure if the external registry is unreachable.
Files changed
src/shared/components/ProviderIcon.tsx� Updates the resolution chain to prioritize local SVG assets, while safely preserving customsrcoverrides (from Add metadata aggregation for combo models in /v1/models #2166) and downstream fallbacks.public/providers/*.svg� Adds 100+ local, crisp SVG icons for built-in providers.public/providers/*.png� Removes obsolete PNG fallbacks for providers that now have SVGs.Test Plan
anybudget and tracked artifacts CI gates locally).