auto-resolve sso provider icons via simple icons#8
Conversation
|
Firetiger deploy monitoring skipped This PR didn't match the auto-monitor filter configured on your GitHub connection:
Reason: PR modifies SSO provider icon handling in the UI layer, not kernel API endpoints or Temporal workflows as specified in the filter. To monitor this PR anyway, reply with |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0c8c226. Configure here.
| return { label: provider, icon: null }; | ||
| const key = slugify(provider); | ||
| const nonBrand = NON_BRAND_ICONS[key]; | ||
| if (nonBrand) return nonBrand; |
There was a problem hiding this comment.
Non-brand providers lose substring matching, breaking compound names
Medium Severity
The old code used p.includes("saml") || p.includes("sso") to match provider strings, so compound names like "enterprise-sso" or "saml-provider" would correctly get the building icon. The new code slugifys the provider (yielding e.g. "enterprisesso") and does an exact dictionary lookup against NON_BRAND_ICONS, which only has keys "passkey", "sso", and "saml". These compound names now miss the non-brand check entirely, hit the CDN path, fail to load, and degrade to a letter avatar instead of the intended BuildingIcon or KeyIcon.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0c8c226. Configure here.
| onError={() => setErrored(true)} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Stale error state persists across provider prop changes
Low Severity
SSOProviderIcon stores CDN load failure in errored state via useState, but never resets it when the provider prop changes. If a CDN request fails for one provider and then the component receives a different provider (e.g., ssoProvider changes in the parent while the tree position stays mounted), the stale errored = true causes the new provider to immediately render the letter-avatar fallback instead of attempting its own CDN fetch.
Reviewed by Cursor Bugbot for commit 0c8c226. Configure here.
1 participant
Summary
Replaces the hardcoded provider switch in
getSSOProviderInfowith a generic resolver, so any provider key works out of the box — no more per-provider PRs.provideris slugified (google,github,okta,auth0,discord,slack, etc.).https://cdn.simpleicons.org/<slug>) via a regular<img>— 3000+ brands, MIT, no API key, no bundle cost.<img>error, falls back to a circular letter avatar with the provider's first letter — so unknown providers still render something.passkey,sso,saml) keep their existing built-in icons.GoogleMark/GitHubMark/GitLabMark/MicrosoftMark/FacebookMark/AppleMarkSVGs fromicons.tsx— net-22lines and a smaller bundle.Trade-offs
simple-iconsnpm package with dynamic import.Test plan
bun run typecheckpassesNote
Medium Risk
Introduces runtime CDN-loaded provider icons (and a fallback) which can affect UI rendering and reliability if the network/CDN or provider slug is unavailable; otherwise changes are localized to SSO button display.
Overview
SSO provider rendering is generalized:
getSSOProviderInfonow slugifies the provider string, title-cases the label, and renders an icon by loadinghttps://cdn.simpleicons.org/<slug>with an on-error fallback to a circular first-letter avatar.Built-in icons are retained only for non-brand providers (
passkey,sso,saml), and the previously inlined brand SVG marks (Google/GitHub/Microsoft/Facebook/Apple) are removed fromicons.tsx. Styling addskma-sso-icon--letterto support the new fallback avatar.Reviewed by Cursor Bugbot for commit 0c8c226. Bugbot is set up for automated code reviews on this repo. Configure here.