feat(ui): show deprovisioned organization members - #9202
Conversation
🦋 Changeset detectedLatest commit: a353533 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 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.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@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: |
API Changes Report
Summary
@clerk/sharedCurrent version: 4.25.8 Subpath
|
📝 WalkthroughWalkthroughThe change adds an optional Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsLinked repositories: Couldn't analyze
Errors logged to '/home/jailuser/git/.git/lfs/logs/20260728T161515.688957582.log'. Errors logged to '/home/jailuser/git/.git/lfs/logs/20260728T161637.116584545.log'. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/clerk-js/src/core/resources/__tests__/PublicUserData.test.ts (1)
17-18: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover snapshot serialization too.
This verifies JSON-to-resource mapping, but not the changed
__internal_toSnapshot()contract. Add an assertion such asexpect(pud.__internal_toSnapshot()).toMatchObject({ deprovisioned: true })so snapshot regressions are caught.As per coding guidelines, unit tests are required for new functionality and should verify edge cases.
Also applies to: 25-29
🤖 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/clerk-js/src/core/resources/__tests__/PublicUserData.test.ts` around lines 17 - 18, Extend the PublicUserData test covering the deprovisioned resource to assert that __internal_toSnapshot() includes deprovisioned: true, preserving the existing JSON-to-resource assertions and covering the changed snapshot serialization contract.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@packages/clerk-js/src/core/resources/__tests__/PublicUserData.test.ts`:
- Around line 17-18: Extend the PublicUserData test covering the deprovisioned
resource to assert that __internal_toSnapshot() includes deprovisioned: true,
preserving the existing JSON-to-resource assertions and covering the changed
snapshot serialization contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b1deb87-32c4-4b69-814f-0c94f9698009
📒 Files selected for processing (58)
.changeset/wise-dolphins-explain.mdpackages/clerk-js/src/core/resources/PublicUserData.tspackages/clerk-js/src/core/resources/__tests__/PublicUserData.test.tspackages/localizations/src/ar-SA.tspackages/localizations/src/be-BY.tspackages/localizations/src/bg-BG.tspackages/localizations/src/bn-IN.tspackages/localizations/src/ca-ES.tspackages/localizations/src/cs-CZ.tspackages/localizations/src/da-DK.tspackages/localizations/src/de-DE.tspackages/localizations/src/el-GR.tspackages/localizations/src/en-GB.tspackages/localizations/src/en-US.tspackages/localizations/src/es-CR.tspackages/localizations/src/es-ES.tspackages/localizations/src/es-MX.tspackages/localizations/src/es-UY.tspackages/localizations/src/fa-IR.tspackages/localizations/src/fi-FI.tspackages/localizations/src/fr-FR.tspackages/localizations/src/he-IL.tspackages/localizations/src/hi-IN.tspackages/localizations/src/hr-HR.tspackages/localizations/src/hu-HU.tspackages/localizations/src/id-ID.tspackages/localizations/src/is-IS.tspackages/localizations/src/it-IT.tspackages/localizations/src/ja-JP.tspackages/localizations/src/kk-KZ.tspackages/localizations/src/ko-KR.tspackages/localizations/src/mn-MN.tspackages/localizations/src/ms-MY.tspackages/localizations/src/nb-NO.tspackages/localizations/src/nl-BE.tspackages/localizations/src/nl-NL.tspackages/localizations/src/pl-PL.tspackages/localizations/src/pt-BR.tspackages/localizations/src/pt-PT.tspackages/localizations/src/ro-RO.tspackages/localizations/src/ru-RU.tspackages/localizations/src/sk-SK.tspackages/localizations/src/sr-RS.tspackages/localizations/src/sv-SE.tspackages/localizations/src/ta-IN.tspackages/localizations/src/te-IN.tspackages/localizations/src/th-TH.tspackages/localizations/src/tr-TR.tspackages/localizations/src/uk-UA.tspackages/localizations/src/vi-VN.tspackages/localizations/src/zh-CN.tspackages/localizations/src/zh-TW.tspackages/shared/src/types/json.tspackages/shared/src/types/localization.tspackages/shared/src/types/session.tspackages/ui/src/components/OrganizationProfile/ActiveMembersList.tsxpackages/ui/src/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsxpackages/ui/src/components/OrganizationProfile/__tests__/utils.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/clerk-ios(auto-detected)clerk/cli(auto-detected)clerk/clerk-android(auto-detected)
dstaley
left a comment
dstaley
left a comment
There was a problem hiding this comment.
approved, but let's retain usage of the RowContainer component for consistency
|
|
||
| return ( | ||
| <RowContainer> | ||
| <Tr |
There was a problem hiding this comment.
let's update the RowContainer component to handle this logic, that way we retain one single source for defining the hover background color
There was a problem hiding this comment.
Addressed in fix(ui): centralize disabled member row styles
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/ActiveMembersList.tsx (1)
99-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winShow the deprovisioned badge for deprovisioned current users.
The
isCurrentUserbranch runs beforeisDeprovisioned, so a deprovisioned current user never receives the new badge. Make deprovisioned state take precedence, or render both badges if both statuses must remain visible.🤖 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/ActiveMembersList.tsx` around lines 99 - 107, Update the badge conditional in ActiveMembersList so isDeprovisioned is evaluated before isCurrentUser, ensuring deprovisioned current users receive the deprovisioned badge. Preserve the existing “you” badge behavior for current users who are not deprovisioned.
🧹 Nitpick comments (1)
packages/ui/src/components/OrganizationProfile/MemberListTable.tsx (1)
133-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return type to
RowContainer.This exported TypeScript component should declare its JSX return type, following the repository’s explicit-return-type requirement. Use the project-standard JSX element type.
🤖 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/MemberListTable.tsx` around lines 133 - 147, Update the exported RowContainer component to declare an explicit return type using the repository-standard JSX element type, while preserving its existing props handling and rendered Tr behavior.Source: Coding guidelines
🤖 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/MemberListTable.tsx`:
- Around line 133-144: Update RowContainer so its generated disabled/hover
styles are merged with any caller-provided sx from rest instead of replacing
them. Preserve the existing conditional styles and ensure consumer-provided
styles remain applied.
---
Outside diff comments:
In `@packages/ui/src/components/OrganizationProfile/ActiveMembersList.tsx`:
- Around line 99-107: Update the badge conditional in ActiveMembersList so
isDeprovisioned is evaluated before isCurrentUser, ensuring deprovisioned
current users receive the deprovisioned badge. Preserve the existing “you” badge
behavior for current users who are not deprovisioned.
---
Nitpick comments:
In `@packages/ui/src/components/OrganizationProfile/MemberListTable.tsx`:
- Around line 133-147: Update the exported RowContainer component to declare an
explicit return type using the repository-standard JSX element type, while
preserving its existing props handling and rendered Tr behavior.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d1f0807-bcf2-477b-bc81-74ed60fb1730
📒 Files selected for processing (2)
packages/ui/src/components/OrganizationProfile/ActiveMembersList.tsxpackages/ui/src/components/OrganizationProfile/MemberListTable.tsx
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
| export const RowContainer = (props: PropsOfComponent<typeof Tr> & { isDisabled?: boolean }) => { | ||
| const { isDisabled, ...rest } = props; | ||
|
|
||
| return ( | ||
| <Tr | ||
| {...props} | ||
| sx={t => ({ ':hover': { backgroundColor: t.colors.$neutralAlpha50 } })} | ||
| {...rest} | ||
| aria-disabled={isDisabled} | ||
| sx={t => | ||
| isDisabled | ||
| ? { opacity: t.opacity.$disabled, ':hover': { backgroundColor: 'transparent' } } | ||
| : { ':hover': { backgroundColor: t.colors.$neutralAlpha50 } } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve caller-provided sx styles.
rest can contain sx, but the later sx={...} assignment replaces it. Merge the caller’s styles with the disabled/hover styles so existing RowContainer consumers do not silently lose custom styling.
🤖 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/MemberListTable.tsx` around
lines 133 - 144, Update RowContainer so its generated disabled/hover styles are
merged with any caller-provided sx from rest instead of replacing them. Preserve
the existing conditional styles and ensure consumer-provided styles remain
applied.
4 participants
Summary
Testing