-
Notifications
You must be signed in to change notification settings - Fork 402
refactor(clerk-js,types): Refactor base theme approach #6371
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
Conversation
Changed border color from $borderAlpha100 to $borderAlpha150 across Card, PopoverCard, ProfileCardContent, Button, and Table components for improved visual consistency. Also added hoverAsFocus prop to SocialButtonBlock and refined focus/hover styles in Button.
Changed border colors in ActionCard and MenuList components to use $borderAlpha150 for improved visual consistency. Removed boxShadow styles from ActionCard and MenuList. Minor formatting adjustment in Button focus style.
Added the focusRing prop to several action buttons in OrganizationSwitcherPopover and SessionActions components to improve keyboard accessibility and visual focus indication.
🦋 Changeset detectedLatest commit: 320e592 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 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 Git ↗︎
|
Introduces support for setting `baseTheme: false` in appearance and globalAppearance, allowing users to disable the default base theme. Updates tests, documentation, and internal logic to handle the new option and renames `polishedAppearance` to `baseTheme` for clarity.
Changed border color references from $borderAlpha100 to $borderAlpha150 in PricingTableDefault and Drawer components for improved visual consistency.
Introduces 'clerk' and 'simple' string-based themes and a getBaseTheme function to select them. Updates the Theme type to support a new 'theme' property, deprecates 'baseTheme', and refactors theme expansion logic to use the new property. This enables easier theme selection and future extensibility.
Refactored parseCaptcha to only merge defined captcha properties (theme, size, language) from appearance objects, preventing undefined values from overwriting defaults.
Expanded and refactored tests for AppearanceProvider to cover new theme and captcha properties, including support for string, object, and array-based themes. Updated type documentation to remove mention of disabling baseTheme with 'false', reflecting deprecation and new theme precedence.
Added an empty elements object to simpleTheme in baseTheme.ts. Updated parseAppearance.test.tsx to expect two parsed elements (simple theme and base theme) instead of one, reflecting changes in theme composition.
Adds border color to button focus state when hoverAsFocus is enabled, improving visual feedback. Also enables hoverAsFocus for SocialButtonIcon.
📝 WalkthroughWalkthroughThis change set introduces a new theming mechanism for UI components, replacing the previous Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Complexity label: Complex
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (2)
packages/clerk-js/sandbox/app.ts (1)
354-356: Commented code for testing theme configurations.The commented-out appearance configuration suggests testing of different theme options during development. Consider removing this commented code if it's no longer needed for testing.
packages/clerk-js/src/ui/baseTheme.ts (1)
270-274: Clarify the non-public API usage.The
simpleStylesproperty uses a TypeScript suppression comment indicating it's "not public api". Consider documenting why this internal property is necessary and whether there's a more type-safe approach.const simpleTheme: Appearance = { - //@ts-expect-error not public api + // Internal property to indicate simple styling mode + //@ts-expect-error Internal API for simple theme detection simpleStyles: true, elements: {}, } satisfies Appearance;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/clerk-js/sandbox/app.ts(2 hunks)packages/clerk-js/src/ui/baseTheme.ts(2 hunks)packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsx(2 hunks)packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx(3 hunks)packages/clerk-js/src/ui/components/UserButton/SessionActions.tsx(2 hunks)packages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsx(3 hunks)packages/clerk-js/src/ui/customizables/parseAppearance.ts(5 hunks)packages/clerk-js/src/ui/elements/Action/ActionCard.tsx(1 hunks)packages/clerk-js/src/ui/elements/Card/CardContent.tsx(1 hunks)packages/clerk-js/src/ui/elements/Card/CardRoot.tsx(1 hunks)packages/clerk-js/src/ui/elements/Drawer.tsx(2 hunks)packages/clerk-js/src/ui/elements/Menu.tsx(1 hunks)packages/clerk-js/src/ui/elements/PopoverCard.tsx(1 hunks)packages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsx(1 hunks)packages/clerk-js/src/ui/elements/SocialButtons.tsx(2 hunks)packages/clerk-js/src/ui/primitives/Button.tsx(1 hunks)packages/clerk-js/src/ui/primitives/Table.tsx(3 hunks)packages/types/src/appearance.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
packages/clerk-js/src/ui/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/clerk-js-ui.mdc)
packages/clerk-js/src/ui/**/*.{ts,tsx}: Element descriptors should always be camelCase
Use element descriptors in UI components to enable consistent theming and styling via appearance.elements
Element descriptors should generate unique, stable CSS classes for theming
Element descriptors should handle state classes (e.g., cl-loading, cl-active, cl-error, cl-open) automatically based on component state
Do not render hard-coded values; all user-facing strings must be localized using provided localization methods
Use the useLocalizations hook and localizationKeys utility for all text and error messages
Use the styled system (sx prop, theme tokens, responsive values) for custom component styling
Use useCardState for card-level state, useFormState for form-level state, and useLoadingStatus for loading states
Always use handleError utility for API errors and use translateError for localized error messages
Use useFormControl for form field state, implement proper validation, and handle loading and error states in forms
Use localization keys for all form labels and placeholders
Use element descriptors for consistent styling and follow the theme token system
Use the Card and FormContainer patterns for consistent UI structure
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/baseTheme.tspackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/customizables/parseAppearance.tspackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/baseTheme.tspackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/customizables/parseAppearance.tspackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsxpackages/types/src/appearance.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/baseTheme.tspackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/customizables/parseAppearance.tspackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsxpackages/types/src/appearance.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/baseTheme.tspackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/customizables/parseAppearance.tspackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsxpackages/types/src/appearance.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/baseTheme.tspackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/customizables/parseAppearance.tspackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsxpackages/types/src/appearance.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/baseTheme.tspackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/customizables/parseAppearance.tspackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsxpackages/types/src/appearance.ts
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{jsx,tsx}: Use error boundaries in React components
Minimize re-renders in React components
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components:UserProfile,NavigationMenu
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Use useState for simple state management
Use useReducer for complex state logic
Implement proper state initialization
Use proper state updates with callbacks
Implement proper state cleanup
Use Context API for theme/authentication
Implement proper state selectors
Use proper state normalization
Implement proper state persistence
Use React.memo for expensive components
Implement proper useCallback for handlers
Use proper useMemo for expensive computations
Implement proper virtualization for lists
Use proper code splitting with React.lazy
Implement proper cleanup in useEffect
Use proper refs for DOM access
Implement proper event listener cleanup
Use proper abort controllers for fetch
Implement proper subscription cleanup
Use proper HTML elements
Implement proper ARIA attributes
Use proper heading hierarchy
Implement proper form labels
Use proper button types
Implement proper focus management
Use proper keyboard shortcuts
Implement proper tab order
Use proper skip links
Implement proper focus traps
Implement proper error boundaries
Use proper error logging
Implement proper error recovery
Use proper error messages
Implement proper error fallbacks
Use proper form validation
Implement proper error states
Use proper error messages
Implement proper form submission
Use proper form reset
Use proper component naming
Implement proper file naming
Use proper prop naming
Implement proper...
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsx
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/baseTheme.tspackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/customizables/parseAppearance.tspackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsxpackages/types/src/appearance.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
**/*.tsx: Use proper type definitions for props and state
Leverage TypeScript's type inference where possible
Use proper event types for handlers
Implement proper generic types for reusable components
Use proper type guards for conditional rendering
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsx
**/*
⚙️ CodeRabbit Configuration File
**/*: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.Whenever reviewing a pull request, if there are any changes that could impact security, always tag
@clerk/securityin the PR.Security-impacting changes include, but are not limited to:
- Changes to authentication logic or mechanisms (e.g. login, session handling, token issuance)
- Any modification to access control, authorization checks, or role-based permissions
- Introduction or modification of hashing algorithms, signature verification, or cryptographic primitives
- Handling of sensitive data (e.g. passwords, tokens, secrets, PII)
- Integration with external identity providers (e.g. SSO, OAuth, OpenID Connect)
- Modifications to security headers, cookie flags, CORS policies, or CSRF protections
- Bypass mechanisms (e.g. feature flags, testing overrides) that could weaken protections
- Changes to rate limiting, abuse prevention, or input validation
If you're unsure whether a change is security-relevant, err on the side of caution and tag
@clerk/security.Any time that you tag
@clerk/security, please do so explicitly in a code comment, rather than within a collapsed section in a coderabbit comment, such as the "recent review details" section. If you do use the team name in any thinking or non-direct-code-comment content, it can be referred to as "clerk security team" to avoid accidentally printing the tag which sends a notification to the team.
Files:
packages/clerk-js/src/ui/elements/Menu.tsxpackages/clerk-js/sandbox/app.tspackages/clerk-js/src/ui/elements/Card/CardRoot.tsxpackages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsxpackages/clerk-js/src/ui/elements/Drawer.tsxpackages/clerk-js/src/ui/elements/SocialButtons.tsxpackages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsxpackages/clerk-js/src/ui/components/UserButton/SessionActions.tsxpackages/clerk-js/src/ui/elements/PopoverCard.tsxpackages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsxpackages/clerk-js/src/ui/baseTheme.tspackages/clerk-js/src/ui/primitives/Table.tsxpackages/clerk-js/src/ui/primitives/Button.tsxpackages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsxpackages/clerk-js/src/ui/customizables/parseAppearance.tspackages/clerk-js/src/ui/elements/Action/ActionCard.tsxpackages/clerk-js/src/ui/elements/Card/CardContent.tsxpackages/types/src/appearance.ts
packages/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Unit tests should use Jest or Vitest as the test runner.
Files:
packages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsx
packages/{clerk-js,elements,themes}/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Visual regression testing should be performed for UI components.
Files:
packages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsx
**/*.test.{jsx,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/react.mdc)
**/*.test.{jsx,tsx}: Use React Testing Library
Test component behavior, not implementation
Use proper test queries
Implement proper test isolation
Use proper test coverage
Test component interactions
Use proper test data
Implement proper test setup
Use proper test cleanup
Implement proper test assertions
Use proper test structure
Files:
packages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/typescript.mdc)
**/__tests__/**/*.{ts,tsx}: Create type-safe test builders/factories
Use branded types for test isolation
Implement proper mock types that match interfaces
Files:
packages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsx
packages/types/src/appearance.ts
📄 CodeRabbit Inference Engine (.cursor/rules/clerk-js-ui.mdc)
Add new element descriptor types in packages/types/src/appearance.ts before using them elsewhere
Files:
packages/types/src/appearance.ts
🧬 Code Graph Analysis (2)
packages/clerk-js/src/ui/baseTheme.ts (1)
packages/types/src/appearance.ts (1)
Appearance(986-1052)
packages/clerk-js/src/ui/customizables/parseAppearance.ts (2)
packages/clerk-js/src/ui/baseTheme.ts (2)
baseTheme(286-286)getBaseTheme(276-284)packages/types/src/appearance.ts (2)
Theme(811-860)CaptchaAppearanceOptions(941-957)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (28)
packages/clerk-js/src/ui/elements/Action/ActionCard.tsx (1)
32-32: Visual refinement approved.The border color opacity increase from
$borderAlpha100to$borderAlpha150improves visual clarity and aligns with the broader UI styling updates across components.packages/clerk-js/src/ui/components/UserButton/SessionActions.tsx (1)
185-185: Accessibility improvement approved.Adding
focusRingprops to the SmallAction components enhances keyboard navigation and focus visibility for the manage account and sign out actions.Also applies to: 197-197
packages/clerk-js/src/ui/elements/SocialButtons.tsx (1)
214-214: UI interaction enhancement approved.Adding
hoverAsFocusprops to both social button variants creates consistent hover and focus behavior across social authentication options, improving user experience.Also applies to: 237-237
packages/clerk-js/src/ui/elements/Menu.tsx (1)
133-133: Menu styling refinement approved.The border color change from
$borderAlpha50to$borderAlpha150significantly improves menu visibility and aligns with the coordinated visual updates across the UI component library.packages/clerk-js/src/ui/elements/Card/CardRoot.tsx (1)
43-43: Card styling enhancement approved.The border color update from
$borderAlpha100to$borderAlpha150improves the visual definition of card boundaries, contributing to better content organization and visual hierarchy.packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsx (1)
120-120: LGTM! Good accessibility improvement.Adding the
focusRingprop to both SmallAction components enhances keyboard navigation and focus visibility, which aligns with accessibility best practices.Also applies to: 129-129
packages/clerk-js/src/ui/elements/Drawer.tsx (1)
267-267: LGTM! Consistent styling update.The border color updates from
$borderAlpha100to$borderAlpha150align with the coordinated visual refresh across the UI components, providing better visual definition and consistency.Also applies to: 306-306
packages/clerk-js/src/ui/elements/ProfileCard/ProfileCardContent.tsx (1)
44-46: LGTM! Consistent styling refinement.The border color update to
$borderAlpha150and the negative margins align with the broader UI refresh. The negative margins likely help with proper border alignment when components are positioned adjacently.packages/clerk-js/sandbox/app.ts (1)
195-196: Good backward compatibility enhancement.Preserving existing appearance properties when updating variables ensures that configurations like
baseThemeare maintained, supporting the new theming approach while maintaining backward compatibility.packages/clerk-js/src/ui/elements/PopoverCard.tsx (1)
61-63: LGTM! Coordinated styling update.The border color update to
$borderAlpha150and negative margins align with the broader UI refresh across components. These changes improve visual consistency and proper border alignment.packages/clerk-js/src/ui/primitives/Table.tsx (1)
16-16: LGTM! Consistent border opacity improvements.The border color changes from
$borderAlpha100to$borderAlpha150provide improved visual consistency across the table component. These styling updates align with the broader theme refinements described in the PR objectives.Also applies to: 25-25, 36-36, 48-48
packages/clerk-js/src/ui/components/PricingTable/PricingTableDefault.tsx (1)
172-172: LGTM! Consistent border styling improvements.The border color updates from
$borderAlpha100to$borderAlpha150across the card, features section, and footer provide better visual definition and consistency with the theme refactor.Also applies to: 207-207, 227-227
packages/clerk-js/src/ui/elements/Card/CardContent.tsx (2)
42-42: LGTM! Border color improvement aligns with theme updates.The border color change from
$borderAlpha50to$borderAlpha150provides better visual consistency with the broader theme refactor.
48-49: Verify that negative margins don't cause layout issues.The addition of negative margins (
marginBlockStart: '-1px'andmarginInline: '-1px') appears to be related to the border changes, but negative margins can sometimes cause unexpected layout behaviors or overlapping elements.Please ensure these negative margins are necessary and don't introduce visual artifacts or layout issues across different use cases of the CardContent component.
packages/clerk-js/src/ui/baseTheme.ts (1)
96-96: LGTM! Clean implementation of the new theme system.The refactor from
polishedAppearancetoclerkTheme, addition ofgetBaseThemefunction, andbaseThemeexport properly implement the new theming mechanism described in the PR objectives. This enables users to explicitly opt into different themes while maintaining backward compatibility.Also applies to: 276-286
packages/clerk-js/src/ui/customizables/__tests__/parseAppearance.test.tsx (2)
341-341: LGTM! Test descriptions updated to reflect API changes.The test descriptions correctly reflect the renaming from
polishedAppearancetobaseTheme.Also applies to: 362-362
468-587: Excellent test coverage for the new theme system!The comprehensive test suite covers all the key scenarios from the PR objectives:
- String-based theme support (
'clerk'and'simple')- Theme property precedence over deprecated
baseTheme- Backward compatibility with existing
baseThemeusage- Object and array-based theme support
The test assertions properly verify both the new functionality and maintain backward compatibility. This thorough testing gives confidence in the theme refactor implementation.
packages/clerk-js/src/ui/customizables/parseAppearance.ts (5)
4-4: LGTM! Import changes align with new theme architecture.The change from importing
polishedAppearancetobaseTheme, getBaseThemeis consistent with the new centralized theme handling approach described in the PR objectives.
24-24: Correct addition of 'theme' to omitted keys.Adding
'theme'to thePublicAppearanceTopLevelKeyomission list is appropriate since it's now handled specially in the parsing logic, similar tobaseTheme.
86-86: LGTM! Consistent use of new baseTheme export.The replacement of
polishedAppearancewithbaseThememaintains the same functionality while using the new centralized theme exports.
107-118: Excellent implementation of new theme handling.This implementation properly handles the new theme system with:
- Backward compatibility via fallback to deprecated
baseTheme- Support for string-based themes (
'clerk','simple') viagetBaseTheme- Support for object-based and array-based themes
- Proper recursive expansion for nested themes
The logic correctly prioritizes the new
themeproperty while maintaining compatibility.
134-146: Improved CAPTCHA appearance merging logic.The selective property copying approach is better than the previous shallow merge as it:
- Explicitly handles only the expected properties (
theme,size,language)- Avoids copying undefined values
- Makes the merging behavior more predictable and maintainable
packages/types/src/appearance.ts (4)
809-809: Correct expansion of BaseTheme type for string-based themes.The union type properly enables both object-based themes (
BaseThemeTaggedType) and the new string-based themes ('clerk'|'simple') while maintaining the optionalcssLayerNameproperty.
813-826: Excellent documentation and type definition for new theme property.The new
themeproperty is well-documented with:
- Clear examples for both object-based and string-based usage
- Proper JSDoc formatting
- Support for both single themes and arrays
- Comprehensive explanation of the functionality
828-835: Proper deprecation of baseTheme with migration guidance.The deprecation is handled correctly with:
- Clear deprecation notice
- Explicit migration path to use
themeinstead- Maintained backward compatibility in the type definition
857-857: Improved clarity in captcha example.The example change from
theme: 'dark'tocaptcha: { theme: 'dark' }better illustrates the correct usage of the captcha property structure.packages/clerk-js/src/ui/primitives/Button.tsx (2)
98-98: Improved border visibility with increased opacity.The change to
theme.colors.$borderAlpha150increases border opacity for better visual definition, consistent with the broader UI styling refresh across multiple components.Also applies to: 108-108
101-103: Enhanced focus styles for better accessibility.The addition of
borderColor: theme.colors.$borderAlpha300in focus states whenhoverAsFocusis true improves accessibility by providing clearer visual feedback during keyboard navigation.Also applies to: 112-114
|
@alexcarpenter The one nit from CodeRabbit makes sense: const simpleTheme: Appearance = {
- //@ts-expect-error not public api
+ // Internal property to indicate simple styling mode
+ //@ts-expect-error Internal API for simple theme detection
simpleStyles: true,
elements: {},
} satisfies Appearance; |
Co-authored-by: Tom Milewski <me@tm.codes>
Updated the comment above the ts-expect-error directive in simpleTheme to clarify that it is for internal API usage related to simple theme detection.
Updated processCssLayerNameExtraction to handle both the new 'theme' property and the deprecated 'baseTheme' property in the Appearance object. The function now extracts cssLayerName from either property, supports arrays and string themes, and ensures backward compatibility.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@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: 0
🧹 Nitpick comments (2)
packages/clerk-js/src/utils/appearance.ts (2)
22-51: Consider optimizing cssLayerName extraction for better performance.The array handling logic is correct and properly handles both string and object themes. However, the forEach loop could be more efficient.
Consider using
find()for early termination once the first cssLayerName is found:- themeProperty.forEach((theme: BaseTheme) => { - if (!cssLayerNameFromTheme && typeof theme === 'object' && theme.cssLayerName) { - cssLayerNameFromTheme = theme.cssLayerName; - } - }); + const themeWithCssLayer = themeProperty.find((theme: BaseTheme) => + typeof theme === 'object' && theme.cssLayerName + ); + cssLayerNameFromTheme = typeof themeWithCssLayer === 'object' ? themeWithCssLayer.cssLayerName : undefined;
62-68: Simplify the theme processing logic.The IIFE pattern works but can be simplified for better readability.
Consider this cleaner approach:
- const processedTheme = - typeof themeProperty === 'string' - ? themeProperty - : (() => { - const { cssLayerName, ...rest } = themeProperty; - return rest; - })(); + let processedTheme: BaseTheme; + if (typeof themeProperty === 'string') { + processedTheme = themeProperty; + } else { + const { cssLayerName, ...rest } = themeProperty; + processedTheme = rest; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/clerk-js/src/utils/appearance.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/clerk-js/src/utils/appearance.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/src/utils/appearance.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/utils/appearance.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/clerk-js/src/utils/appearance.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/clerk-js/src/utils/appearance.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/clerk-js/src/utils/appearance.ts
**/*
⚙️ CodeRabbit Configuration File
**/*: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.Whenever reviewing a pull request, if there are any changes that could impact security, always tag
@clerk/securityin the PR.Security-impacting changes include, but are not limited to:
- Changes to authentication logic or mechanisms (e.g. login, session handling, token issuance)
- Any modification to access control, authorization checks, or role-based permissions
- Introduction or modification of hashing algorithms, signature verification, or cryptographic primitives
- Handling of sensitive data (e.g. passwords, tokens, secrets, PII)
- Integration with external identity providers (e.g. SSO, OAuth, OpenID Connect)
- Modifications to security headers, cookie flags, CORS policies, or CSRF protections
- Bypass mechanisms (e.g. feature flags, testing overrides) that could weaken protections
- Changes to rate limiting, abuse prevention, or input validation
If you're unsure whether a change is security-relevant, err on the side of caution and tag
@clerk/security.Any time that you tag
@clerk/security, please do so explicitly in a code comment, rather than within a collapsed section in a coderabbit comment, such as the "recent review details" section. If you do use the team name in any thinking or non-direct-code-comment content, it can be referred to as "clerk security team" to avoid accidentally printing the tag which sends a notification to the team.
Files:
packages/clerk-js/src/utils/appearance.ts
🧬 Code Graph Analysis (1)
packages/clerk-js/src/utils/appearance.ts (1)
packages/types/src/appearance.ts (2)
Appearance(986-1052)BaseTheme(809-809)
🔇 Additional comments (2)
packages/clerk-js/src/utils/appearance.ts (2)
3-18: LGTM! Clean backward compatibility implementation.The JSDoc documentation clearly explains the dual property support, and the logic correctly prioritizes the new
themeproperty while maintaining backward compatibility withbaseTheme. The early return logic is sound.
7-84: Test coverage located in appearance.spec.ts – please confirm completeness.Tests for
processCssLayerNameExtractionlive in
packages/clerk-js/src/utils/__tests__/appearance.spec.tsand cover many scenarios. Review that file to ensure it includes:
- Handling of both
themeand deprecatedbaseTheme- Array vs. single theme inputs
- String vs. object theme types
- CSS layer name extraction and prioritization (appearance-level vs. theme-level)
- Edge cases such as
undefinedor other non-object inputs
|
!snapshot |
|
Hey @alexcarpenter - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.1.14-snapshot.v20250728160410 --save-exact
npm i @clerk/astro@2.10.11-snapshot.v20250728160410 --save-exact
npm i @clerk/backend@2.6.0-snapshot.v20250728160410 --save-exact
npm i @clerk/chrome-extension@2.5.13-snapshot.v20250728160410 --save-exact
npm i @clerk/clerk-js@5.77.0-snapshot.v20250728160410 --save-exact
npm i @clerk/elements@0.23.46-snapshot.v20250728160410 --save-exact
npm i @clerk/clerk-expo@2.14.12-snapshot.v20250728160410 --save-exact
npm i @clerk/expo-passkeys@0.3.23-snapshot.v20250728160410 --save-exact
npm i @clerk/express@1.7.13-snapshot.v20250728160410 --save-exact
npm i @clerk/fastify@2.4.13-snapshot.v20250728160410 --save-exact
npm i @clerk/localizations@3.20.4-snapshot.v20250728160410 --save-exact
npm i @clerk/nextjs@6.27.0-snapshot.v20250728160410 --save-exact
npm i @clerk/nuxt@1.7.14-snapshot.v20250728160410 --save-exact
npm i @clerk/clerk-react@5.37.0-snapshot.v20250728160410 --save-exact
npm i @clerk/react-router@1.8.7-snapshot.v20250728160410 --save-exact
npm i @clerk/remix@4.10.7-snapshot.v20250728160410 --save-exact
npm i @clerk/shared@3.15.1-snapshot.v20250728160410 --save-exact
npm i @clerk/tanstack-react-start@0.21.3-snapshot.v20250728160410 --save-exact
npm i @clerk/testing@1.10.7-snapshot.v20250728160410 --save-exact
npm i @clerk/themes@2.4.2-snapshot.v20250728160410 --save-exact
npm i @clerk/types@4.71.0-snapshot.v20250728160410 --save-exact
npm i @clerk/vue@1.8.21-snapshot.v20250728160410 --save-exact |
Description
Refactor base theme approach to enable opting into simple theme.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
themeproperty in appearance customization, enabling selection between "clerk" and "simple" themes, with backward compatibility for the deprecatedbaseThemeproperty.Enhancements
Bug Fixes
Documentation
Tests