fix(ui,clerk-js,localizations): General OrganizationProfile UI fixes#8898
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughAdds permission-and-data-aware gating to ChangesOrganizationProfile domain gating, section rename, and UserProfile alignment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
🦋 Changeset detectedLatest commit: 0d1b192 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@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: |
OrganizationProfileOrganizationProfile
OrganizationProfileOrganizationProfile UI fixes
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/OrganizationProfile/OrganizationGeneralPage.tsx (1)
139-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefer domains fetching until the domains feature is confirmed enabled.
At Line 139, the domains query hook runs before the feature-flag guard at Line 146, so
getDomainscan be triggered even when domains are disabled. Please move the domains-fetching hook into a child component rendered only after the enabled checks pass.Suggested refactor
const OrganizationDomainsSection = () => { const { organizationSettings } = useEnvironment(); - const { organization, domains } = useOrganization({ domains: { infinite: true } }); - const canManageDomains = useProtect({ permission: 'org:sys_domains:manage' }); + const { organization } = useOrganization(); if (!organizationSettings || !organization) { return null; } if (!organizationSettings.domains.enabled) { return null; } + return <OrganizationDomainsSectionContent />; +}; + +const OrganizationDomainsSectionContent = () => { + const { domains } = useOrganization({ domains: { infinite: true } }); + const canManageDomains = useProtect({ permission: 'org:sys_domains:manage' }); + // Hide the section when there are no domains to show and the user cannot add // any if (!domains?.data?.length && !canManageDomains) { return null; } return ( <ProfileSection.Root title={localizationKeys('organizationProfile.profilePage.domainSection.title')} id='organizationDomains' centered={false} > ... </ProfileSection.Root> ); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/OrganizationProfile/OrganizationGeneralPage.tsx` around lines 139 - 147, The useOrganization hook with the domains query parameter is being called before checking if domains are enabled, causing unnecessary queries. Move the useOrganization hook call that includes the domains configuration into a separate child component that is only rendered when organizationSettings.domains.enabled is true. This ensures the domains fetching is deferred until after the feature flag guard confirms domains are actually enabled.
🧹 Nitpick comments (1)
packages/ui/src/components/UserProfile/EmailsSection.tsx (1)
64-64: ⚡ Quick winUse
align='center'prop for consistency across aligned sections.EmailsSection applies
alignItems: 'center'directly insx, while ConnectedAccountsSection (line 162) and EnterpriseAccountsSection (line 177) both use the Flexalign='center'prop. For consistency and maintainability, use the Flex variant prop instead of embedding the CSS property directly.♻️ Proposed refactor for consistency
- <Flex sx={t => ({ overflow: 'hidden', gap: t.space.$1, alignItems: 'center' })}> + <Flex + align='center' + sx={t => ({ overflow: 'hidden', gap: t.space.$1 })} + >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/components/UserProfile/EmailsSection.tsx` at line 64, In the EmailsSection component, the Flex component at line 64 uses alignItems: 'center' inside the sx prop, which is inconsistent with how ConnectedAccountsSection and EnterpriseAccountsSection apply vertical alignment. Remove the alignItems: 'center' property from the sx object in the Flex component and instead use the align='center' prop directly on the Flex component to match the consistency pattern used throughout the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/ui/src/components/OrganizationProfile/__tests__/OrganizationGeneralPage.test.tsx`:
- Around line 177-180: Replace the hardcoded `await new Promise(r =>
setTimeout(r, 100))` delay in the test with `waitFor` to provide stable async
assertions. Remove the setTimeout promise and instead use `waitFor` to wait for
the expected DOM state where both 'Verified domains' and 'Add domain' text
elements should not be in the document, wrapping the expect statements within
the waitFor callback.
---
Outside diff comments:
In `@packages/ui/src/components/OrganizationProfile/OrganizationGeneralPage.tsx`:
- Around line 139-147: The useOrganization hook with the domains query parameter
is being called before checking if domains are enabled, causing unnecessary
queries. Move the useOrganization hook call that includes the domains
configuration into a separate child component that is only rendered when
organizationSettings.domains.enabled is true. This ensures the domains fetching
is deferred until after the feature flag guard confirms domains are actually
enabled.
---
Nitpick comments:
In `@packages/ui/src/components/UserProfile/EmailsSection.tsx`:
- Line 64: In the EmailsSection component, the Flex component at line 64 uses
alignItems: 'center' inside the sx prop, which is inconsistent with how
ConnectedAccountsSection and EnterpriseAccountsSection apply vertical alignment.
Remove the alignItems: 'center' property from the sx object in the Flex
component and instead use the align='center' prop directly on the Flex component
to match the consistency pattern used throughout the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 428a5065-485a-4208-bf9d-23ffe5ef5aea
📒 Files selected for processing (10)
.changeset/frank-spoons-stick.mdpackages/localizations/src/bg-BG.tspackages/localizations/src/en-US.tspackages/localizations/src/uk-UA.tspackages/localizations/src/utils/enUS_v4.tspackages/ui/src/components/OrganizationProfile/OrganizationGeneralPage.tsxpackages/ui/src/components/OrganizationProfile/__tests__/OrganizationGeneralPage.test.tsxpackages/ui/src/components/UserProfile/ConnectedAccountsSection.tsxpackages/ui/src/components/UserProfile/EmailsSection.tsxpackages/ui/src/components/UserProfile/EnterpriseAccountsSection.tsx
3bd5c8a to
759d7b1
Compare
759d7b1 to
0d1b192
Compare
Description
This PR introduces some UI fixes around
OrganizationProfile, such as:Verified domainssection when there are no domains and the user lacks permission to add themOrganization profilesection toProfilefor consistency withUserProfileChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Summary of changes
Bug Fixes
Localization
Style
Tests