refactor(clerk-js): Expand ApplicationLogo usage for oAuth logo #6518
Conversation
🦋 Changeset detectedLatest commit: 8ad7ca8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 ↗︎
|
@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: |
📝 WalkthroughWalkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (1)
67-78: Add tests for external/internal link rendering and image selectionPlease add tests that:
- Verify that providing props.href renders an external Link when isExternalUrl is true, and a RouterLink otherwise.
- Verify src selection precedence: props.src > parsedLayout.logoImageUrl > logoImageUrl.
- Verify alt selection once alt? is added: props.alt > applicationName.
I can scaffold tests (Jest + React Testing Library) for ApplicationLogo. Want me to open a follow-up PR with test cases?
Also applies to: 80-106
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (2)
76-85: Minor: Consistency on alt for your app logoNo change required; ApplicationLogo can default to the environment’s applicationName. If you prefer explicitness:
- <ConnectionItem justify='start'> - <ApplicationLogo /> + <ConnectionItem justify='start'> + <ApplicationLogo /* alt defaults to applicationName */ /> </ConnectionItem>
45-52: Tests: cover header compositions for logo presence permutationsGiven the refactor, add tests that snapshot or assert DOM structures for:
- both logos present (two ApplicationLogo + ConnectionSeparator),
- only OAuth logo present (Box with overlaid ConnectionIcon + ApplicationLogo),
- only Clerk logo present (ConnectionIcon + ApplicationLogo),
- neither logo present (ConnectionIcon only).
I can scaffold React Testing Library tests with fixtures for logoImageUrl/oAuthApplicationLogoUrl permutations. Want me to push a test file?
Also applies to: 56-73, 76-85, 88-91
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx(5 hunks)packages/clerk-js/src/ui/elements/ApplicationLogo.tsx(3 hunks)packages/shared/src/url.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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/shared/src/url.tspackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.tsx
**/*.{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/shared/src/url.tspackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/url.tspackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.tsx
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/url.tspackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.tsx
**/*.{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/shared/src/url.tspackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.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/shared/src/url.tspackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.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.
Files:
packages/shared/src/url.tspackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.tsx
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/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.tsx
**/*.{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/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.tsx
**/*.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/components/OAuthConsent/OAuthConsent.tsxpackages/clerk-js/src/ui/elements/ApplicationLogo.tsx
🧬 Code Graph Analysis (2)
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (2)
packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (1)
ApplicationLogo(38-112)packages/clerk-js/src/ui/customizables/index.ts (1)
Flex(16-16)
packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (3)
packages/clerk-js/src/ui/styledSystem/types.ts (1)
PropsOfComponent(58-58)packages/clerk-js/src/ui/customizables/index.ts (2)
Flex(16-16)useAppearance(9-9)packages/shared/src/url.ts (1)
isExternalUrl(181-183)
⏰ 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). (4)
- GitHub Check: Build Packages
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (2)
80-93: SSR: external vs internal link decision relies on isExternalUrlThis branch executes during render and will run on the server in SSR environments. With the refactor proposed in shared/url.ts, isExternalUrl becomes SSR-safe. Ensure that change lands together with this usage to avoid ReferenceError on window during SSR.
If you prefer local shielding, you can also guard here:
- isExternalUrl(logoUrl) ? ( + (typeof window === 'undefined' ? isAbsoluteUrl(logoUrl) : isExternalUrl(logoUrl)) ? (Note: would require importing isAbsoluteUrl from @clerk/shared/url.
Do you want me to wire this guard now, or rely on the shared helper fix?
84-90: Ignoresxsuggestion for Link: unsupported by the primitive
TheLinkcomponent’s props (LinkProps) and implementation only support Emotion’scssprop and the styled‐system variants fromapplyVariants. There is nosxprop defined or consumed, so migrating tosxhere would have no effect. Continue usingcss(or, if you needsx, extend theLinkprimitive to support it).Likely an incorrect or invalid review comment.
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
Outdated
Show resolved
Hide resolved
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
Outdated
Show resolved
Hide resolved
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
Outdated
Show resolved
Hide resolved
📝 WalkthroughWalkthroughRefactors OAuthConsent header/logo rendering to use ApplicationLogo instead of Avatar, introducing ConnectionHeader and ConnectionItem layout helpers. Updates four display scenarios to consistently wrap logos and icons via the new header container. Extends ApplicationLogo to accept optional src and href props and conditionally render internal RouterLink or external Link based on a new isExternalUrl utility. Focus-ring styling is applied for both link types. Adds isExternalUrl to shared URL utilities, using isAbsoluteUrl and window.location.origin to detect external links. No exported signature changes except the expanded ApplicationLogo props and the newly exported isExternalUrl function. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (1)
19-386: Add missing tests for the OAuthConsent component
No existing test or spec files referenceOAuthConsentorOAuthConsentInternal. We need to cover the refactored logic and all rendering branches:– Render with both OAuth app and Clerk avatars present
– Render with only the OAuth app avatar
– Render with only the Clerk avatar
– Render with no avatars
– Verify the scopes list and localization keys
– Verify the warning text, domain extraction (getRootDomain) and modal opening behaviorPlease add Jest/React Testing Library specs in a new
OAuthConsent.test.tsx(or similar) that mount the component viawithCardStateProviderand mockuseOAuthConsentContext,useUser, anduseEnvironmentto drive each scenario.
🧹 Nitpick comments (3)
packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (2)
80-106: Extract shared focus styling to reduce duplication.The conditional rendering logic is correct and properly handles external vs internal links. However, the focus ring styling is duplicated between Link and RouterLink components.
Extract the shared focus styling to improve maintainability:
+const focusStyles = (theme: InternalTheme) => ({ + '&:focus': { + ...common.focusRing(theme), + outline: 'none', + }, +}); + {logoUrl ? ( isExternalUrl(logoUrl) ? ( <Link href={logoUrl} isExternal - css={(theme: InternalTheme) => ({ - '&:focus': { - ...common.focusRing(theme), - outline: 'none', - }, - })} + css={focusStyles} > {image} </Link> ) : ( <RouterLink sx={theme => ({ justifyContent: 'center', - '&:focus': { - ...common.focusRing(theme), - outline: 'none', - }, + ...focusStyles(theme), })} to={logoUrl} > {image} </RouterLink> ) ) : ( image )}
38-112: Add explicit return type annotation.Following the coding guidelines for TypeScript, add an explicit return type for the component function.
-export const ApplicationLogo = (props: ApplicationLogoProps) => { +export const ApplicationLogo = (props: ApplicationLogoProps): React.ReactElement | null => {packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (1)
309-321: Consider extracting ConnectionItem to a shared location.The
ConnectionItemcomponent is well-implemented with proper typing and flex layout. However, it could be useful in other parts of the codebase.Consider moving
ConnectionItemto a shared location likepackages/clerk-js/src/ui/elements/if it might be reused elsewhere:// packages/clerk-js/src/ui/elements/ConnectionItem.tsx import type { ComponentProps } from 'react'; import { Flex } from '../customizables'; export function ConnectionItem({ children, ...props }: ComponentProps<typeof Flex>) { return ( <Flex {...props} sx={{ flex: 1, ...(props.sx || {}), }} > {children} </Flex> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx(5 hunks)packages/clerk-js/src/ui/elements/ApplicationLogo.tsx(3 hunks)packages/shared/src/url.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
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/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/shared/src/url.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/shared/src/url.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/elements/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/shared/src/url.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/shared/src/url.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/shared/src/url.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/shared/src/url.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.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.
Files:
packages/clerk-js/src/ui/elements/ApplicationLogo.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsxpackages/shared/src/url.ts
🧬 Code Graph Analysis (2)
packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (3)
packages/clerk-js/src/ui/styledSystem/types.ts (1)
PropsOfComponent(58-58)packages/clerk-js/src/ui/customizables/index.ts (2)
Flex(16-16)useAppearance(9-9)packages/shared/src/url.ts (1)
isExternalUrl(181-183)
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (2)
packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (1)
ApplicationLogo(38-112)packages/clerk-js/src/ui/customizables/index.ts (1)
Flex(16-16)
⏰ 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). (20)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
🔇 Additional comments (9)
packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (4)
1-1: LGTM: Import addition is correct.The
isExternalUrlimport properly references the new utility function from the shared package.
6-8: LGTM: New imports support enhanced functionality.The additional imports for Link, type utilities, and styling system are correctly imported to support the external link handling and focus styling.
27-36: Excellent JSDoc documentation and type definition.The prop interface is well-documented with clear descriptions for both new optional properties. The type extension properly uses
PropsOfComponent<typeof Flex>for component composition.
43-44: LGTM: Proper prop prioritization logic.The logic correctly prioritizes props over layout configuration, maintaining backward compatibility while enabling override capabilities.
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (5)
2-2: LGTM: Import addition supports new component typing.The
ComponentPropsimport correctly supports the newConnectionItemcomponent typing.
44-52: Excellent refactoring: Both logos scenario improved.The refactoring properly uses
ApplicationLogowith thesrcprop for the OAuth logo while maintaining the layout structure with the newConnectionItemwrapper components.
62-62: LGTM: OAuth-only logo scenario properly updated.The
ApplicationLogowithsrcprop correctly replaces the previous Avatar usage while preserving the overlayConnectionIconpositioning.
76-84: Good refactoring: Clerk-only logo scenario improved.The layout is well-structured using
ConnectionItemcomponents with proper justification and the standardApplicationLogowithout props for the Clerk application logo.
88-90: LGTM: No-logos scenario properly updated.The
ConnectionHeaderwrapper maintains consistency across all scenarios while preserving the icon-only display.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
packages/clerk-js/src/ui/primitives/Link.tsx (1)
75-76: Add noreferrer when using target=_blank
Include noreferrer alongside noopener for security and privacy.- rel={href && isExternal ? 'noopener' : undefined} + rel={href && isExternal ? 'noopener noreferrer' : undefined}packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (1)
55-56: Honor alt override for accessibility
Prefer provided alt over applicationName for better accessible name control.- alt={applicationName} + alt={alt ?? applicationName}
🧹 Nitpick comments (6)
packages/clerk-js/src/ui/primitives/Link.tsx (1)
36-43: Prefer :focus-visible and guard against disabled state for focus ring
Use :focus-visible to avoid showing rings on mouse click, and prevent rings when disabled via data-state.- true: { - '&:focus': { - outline: 'none', - ...common.focusRing(theme), - }, - }, + true: { + '&:focus-visible:not([data-disabled="true"])': { + outline: 'none', + ...common.focusRing(theme), + }, + },packages/clerk-js/src/ui/elements/ApplicationLogo.tsx (4)
58-65: Handle image load failures to avoid a permanently hidden logo
If the image fails to load, display stays “none”. Set loaded onError too.size={200} onLoad={() => setLoaded(true)} + onError={() => setLoaded(true)}
73-76: Avoid transient NaN ratio and layout shift until image is loaded
Guard height calculation with loaded to avoid NaN (0/0) before image dimensions are known and reduce CLS.- theme => ({ - height: getContainerHeightForImageRatio(imageRef, theme.sizes.$6), + theme => ({ + height: loaded + ? getContainerHeightForImageRatio(imageRef, theme.sizes.$6) + : `calc(${theme.sizes.$6} * 2)`, justifyContent: 'center', }),
80-86: RouterLink focus ring usage LGTM; consider aria-label fallback
Using focusRing with RouterLink is consistent with the new Link variant. If alt might be empty, consider an aria-label on the link for an explicit accessible name.
24-37: Add JSDoc on the component itself and unit tests for new props path
Document ApplicationLogo’s behavior (wrapping with link when href is present, alt override) and add tests:
- renders Image with provided src
- alt precedence: alt > applicationName
- wraps in RouterLink when href is provided and applies focus ring class
- robust when image fails to load (onError)
Would you like me to draft React Testing Library tests for these scenarios?
.changeset/chilly-ravens-cover.md (1)
5-5: Fix capitalization (“OAuth”) and broaden the note to cover Link focus ring and new props
Tighten wording and include all user-impacting changes.-Refactor ApplicationLogo rendering logic to account for oAuth logos within OAuthConsent component. +Refactor ApplicationLogo rendering and OAuthConsent header composition to properly display OAuth logos (including non‑square assets). Add optional src/alt/href props to ApplicationLogo and wrap the logo with a focus‑ring Link when a URL is provided. Introduce a focusRing styling variant to Link.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/chilly-ravens-cover.md(1 hunks)packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx(6 hunks)packages/clerk-js/src/ui/elements/ApplicationLogo.tsx(2 hunks)packages/clerk-js/src/ui/primitives/Link.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
🧰 Additional context used
📓 Path-based instructions (11)
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/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.tsx
**/*.{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/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/elements/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.tsx
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/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.tsx
**/*.{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/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.tsx
**/*.{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/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.tsx
**/*.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/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.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.
Files:
packages/clerk-js/src/ui/elements/ApplicationLogo.tsxpackages/clerk-js/src/ui/primitives/Link.tsx
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/chilly-ravens-cover.md
🪛 LanguageTool
.changeset/chilly-ravens-cover.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ... '@clerk/clerk-js': patch --- Refactor ApplicationLogo rendering logic to account for oAuth lo...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (1)
packages/clerk-js/src/ui/primitives/Link.tsx (1)
48-49: Good default: focusRing is opt-in
Defaulting focusRing to false is a safe, non-breaking choice.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/chilly-ravens-cover.md (1)
5-5: Fix OAuth capitalization.The term should be "OAuth" (capital O, capital A) rather than "oAuth" as per the standard OAuth specification.
-Refactor ApplicationLogo rendering logic to account for oAuth logos within OAuthConsent component. +Refactor ApplicationLogo rendering logic to account for OAuth logos within OAuthConsent component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/chilly-ravens-cover.md(1 hunks)packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx(6 hunks)packages/clerk-js/src/ui/elements/ApplicationLogo.tsx(2 hunks)packages/clerk-js/src/ui/primitives/Link.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/elements/ApplicationLogo.tsx
🧰 Additional context used
📓 Path-based instructions (11)
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/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.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/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
**/*.{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/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
packages/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/ui/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
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/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
**/*.{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/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
**/*.{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/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.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/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
**/*.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/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.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.
Files:
packages/clerk-js/src/ui/primitives/Link.tsxpackages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx
.changeset/**
📄 CodeRabbit Inference Engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/chilly-ravens-cover.md
🪛 LanguageTool
.changeset/chilly-ravens-cover.md
[grammar] ~5-~5: Ensure spelling is correct
Context: ... '@clerk/clerk-js': patch --- Refactor ApplicationLogo rendering logic to account for oAuth lo...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (7)
packages/clerk-js/src/ui/primitives/Link.tsx (1)
36-48: LGTM! Clean focusRing implementation.The focusRing variant is well-implemented:
- Uses the shared
common.focusRing(theme)for consistency- Defaults to
falsefor opt-in behavior- Properly removes default outline when enabled
- Follows the established variant pattern
packages/clerk-js/src/ui/components/OAuthConsent/OAuthConsent.tsx (6)
2-2: LGTM! Appropriate React type import.Adding
ComponentPropsimport for typing theConnectionItemcomponent props is correct.
51-60: LGTM! Proper alt text provided for OAuth logo.The ApplicationLogo components now include appropriate alt text using
oAuthApplicationName, addressing accessibility concerns.
71-74: LGTM! Alt text provided for single OAuth logo case.Consistent with the dual logo case, the alt text is properly set for accessibility.
321-330: LGTM! Correct sx prop composition in ConnectionItem.The implementation properly handles the
sxprop by using array composition[{ flex: 1 }, props.sx]instead of object spreading. This correctly supportsThemableCssPropwhich can be a function or array, avoiding runtime errors and preserving composition behavior.
44-103: Well-structured conditional rendering for logo combinations.The refactor provides clean handling of all logo combination scenarios:
- Both logos present: ConnectionItem layout with separator
- Only OAuth logo: Overlay ConnectionIcon approach
- Only Clerk logo: ConnectionItem layout with icon and logo
- No logos: Simple ConnectionHeader with icon
The use of ConnectionHeader, ConnectionItem, and ConnectionSeparator provides consistent layout structure.
19-265: Suggest adding tests for the refactored component.The significant refactor from Avatar to ApplicationLogo usage and the introduction of ConnectionItem layout logic would benefit from test coverage to ensure all logo combination scenarios render correctly.
Consider adding tests that cover:
- All four logo combination scenarios (both, OAuth only, Clerk only, neither)
- Proper alt text rendering for accessibility
- ConnectionItem layout behavior
- ApplicationLogo src prop usage
Would you like me to help generate test cases for these scenarios?
Description
This refactor is to support work that @jfoshee is doing to enable linking to external App page in OAuth Consent in #6447
Update ApplicationLogo focus ring
Update oAuth logo to use ApplicationLogo vs Avatar
Ensures logos render properly, even when they are not perfectly squares
Fixes Logo alignment issues
When the ApplicationLogo was not square, the separator was not properly centered
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
UI/Style
Refactor
Chores