-
Notifications
You must be signed in to change notification settings - Fork 426
feat(ui): Extract <ProviderIcon /> component
#7633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(ui): Extract <ProviderIcon /> component
#7633
Conversation
🦋 Changeset detectedLatest commit: fe0801e The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a new 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/ui/src/components/UserProfile/ConnectedAccountsSection.tsx`:
- Around line 156-170: The component dereferences
providerToDisplayData[account.provider] in multiple places but only guards it
inside ImageOrInitial, causing a runtime crash when provider data is missing;
centralize retrieval by creating a single providerData variable from
providerToDisplayData[account.provider] at the top of the render (used by
ImageOrInitial, label rendering, and descriptors), then use safe fallbacks
(e.g., a generic icon URL or a fallback name like account.provider or "Unknown
provider") wherever providerData is undefined; update ImageOrInitial, the label
text usage, and any
descriptors.socialButtonsProviderIcon.setId(account.provider) usage to reference
this providerData/fallback so no direct dereference of
providerToDisplayData[account.provider].name remains.
packages/ui/src/components/UserProfile/ConnectedAccountsSection.tsx
Outdated
Show resolved
Hide resolved
| isDisabled={card.isLoading} | ||
| src={strategyToDisplayData[strategy].iconUrl} | ||
| alt={`Connect ${strategyToDisplayData[strategy].name}`} | ||
| sx={theme => ({ width: theme.sizes.$5 })} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure why we'd increase the size of the web3 icons here. I removed, so its consistent with the SSO icons, but let me know if this was intentional.
brkalow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏 nice cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/ui/src/common/__tests__/ProviderIcon.test.tsx`:
- Line 49: Remove the unused "container" variable from the render()
destructuring in the ProviderIcon.test.tsx test cases; locate the render(...)
calls that currently do "const { container, ... } = render(...)" and delete
"container" from the destructured list (or simply use "render(...)" without
destructuring if nothing else is needed) so ESLint's no-unused-vars is not
triggered — apply this change for all test cases that currently destructure
container (the occurrences noted in the review).
855c0b4 to
0c61521
Compare
Description
Follow up to #7560
Extract
<ProviderIcon />component to ensure consistency across the usages. Ensures icons are inverted properly, not just within sign-in or sign-up components.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.