(DO NOT MERGE) feat: update starter kit with route protection and additional guides#28
(DO NOT MERGE) feat: update starter kit with route protection and additional guides#28peterphanouvong wants to merge 25 commits intomainfrom
Conversation
…arter-kit into peter/make-starter
WalkthroughAdds optional Kinde-based authentication toggle and docs, migrates styling to Tailwind with a new semantic token surface, introduces many Tailwind/Radix UI primitives and utilities, refactors app flow to AppContent/LandingPage with improved loading/error handling, relaxes TypeScript strictness, updates build/tooling and path aliases, and removes a pnpm local override. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant App as App.tsx
participant KA as useKindeAuth
participant UI as View
U->>App: Load app
App->>KA: useKindeAuth()
KA-->>App: { isLoading, isAuthenticated, error }
alt error
App-->>UI: Render "Error"
else isLoading
App-->>UI: Render full-screen spinner
else isAuthenticated
App-->>UI: Render AppContent (dashboard)
else
App-->>UI: Render LandingPage (Sign in / Sign up)
end
sequenceDiagram
autonumber
participant Router as React Router
participant PR as ProtectedRoute
participant K as KindeAuth.has()
participant Page as Protected Page
Router->>PR: Navigate to /protected
alt has prop provided
PR->>K: has(params)
K-->>PR: boolean
alt allowed
PR-->>Router: allow render
Router->>Page: render
else denied
PR-->>Router: redirect to fallbackPath
end
else no has
PR-->>Router: allow render
Router->>Page: render
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (14)
vite.config.ts (1)
8-10: Add strictPort to avoid OIDC redirect mismatchesKinde redirect/logout URLs in .env_sample use port 3000. Without
strictPort, Vite may auto-select another port if 3000 is taken, breaking redirects.Apply this diff:
server: { - port: 3000, + port: 3000, + strictPort: true, },package.json (1)
14-20: Consider moving Tailwind tooling to devDependencies
tailwindcssand@tailwindcss/viteare build-time tools. Keeping them indevDependenciesreduces your prod dependency surface.Example diff:
- "dependencies": { + "dependencies": { "@kinde-oss/kinde-auth-react": "^5.6.0", - "@tailwindcss/vite": "^4.1.11", "react": "^19.1.0", "react-dom": "^19.1.0", "react-router-dom": "^7.7.0", - "tailwindcss": "^4.1.11" + // ... }, "devDependencies": { + "@tailwindcss/vite": "^4.1.11", + "tailwindcss": "^4.1.11", "@eslint/js": "^9.28.0", "@types/react": "^19.1.6", "@types/react-dom": "^19.1.5", "@vitejs/plugin-react": "^4.5.1", "eslint": "^9.28.0", "eslint-plugin-react-hooks": "^5.2.0", "eslint-plugin-react-refresh": "^0.4.20", "globals": "^16.2.0", "typescript": "^5.8.3", "typescript-eslint": "^8.33.1", "vite": "^6.3.5" }Also applies to: 21-33
src/components/LoggedOut.tsx (2)
12-14: Make the brand clickable (route to home)Small UX/a11y win and parity with common nav patterns.
Apply this diff:
- <div className="text-xl font-semibold text-gray-800"> - React Starter Kit - </div> + <div className="text-xl font-semibold text-gray-800"> + <Link to="/" className="hover:opacity-90 focus:outline-none focus:ring"> + React Starter Kit + </Link> + </div>Add the import (outside the selected range):
import { Link } from "react-router-dom";
10-11: Tailwind v4 “container” utility may not be present by defaultIf you notice no centering/max-width effect, switch to explicit width utilities.
Apply this diff as a resilient alternative:
- <div className="container mx-auto px-4 py-4"> + <div className="mx-auto max-w-screen-xl px-4 py-4"> ... - <main className="container mx-auto px-4 py-8"> + <main className="mx-auto max-w-screen-xl px-4 py-8">Also applies to: 27-29
src/config/auth.ts (1)
1-12: Harden config: add types, normalize boolean parsing, and validate required env when auth is enabledAs-is, enabling auth with missing VITE_KINDE_* values will propagate undefined into the provider at runtime. Tighten types and surface a clear warning early.
Apply:
+type AuthConfig = { + ENABLED: boolean; + KINDE: { + CLIENT_ID?: string; + DOMAIN?: string; + REDIRECT_URL?: string; + LOGOUT_URL?: string; + }; +}; -export const AUTH_CONFIG = { - // Enable/disable authentication with environment variable - ENABLED: import.meta.env.VITE_ENABLE_AUTH === "true", +export const AUTH_CONFIG: AuthConfig = { + // Enable/disable authentication with environment variable + ENABLED: String(import.meta.env.VITE_ENABLE_AUTH).toLowerCase() === "true", // Kinde configuration KINDE: { CLIENT_ID: import.meta.env.VITE_KINDE_CLIENT_ID, DOMAIN: import.meta.env.VITE_KINDE_DOMAIN, REDIRECT_URL: import.meta.env.VITE_KINDE_REDIRECT_URL, LOGOUT_URL: import.meta.env.VITE_KINDE_LOGOUT_URL, }, }; + +export function validateAuthConfig(): void { + if (!AUTH_CONFIG.ENABLED) return; + const required = ["CLIENT_ID", "DOMAIN", "REDIRECT_URL"] as const; + const missing = required.filter((k) => !AUTH_CONFIG.KINDE[k]); + if (missing.length) { + console.warn( + `[auth] Missing required Kinde config: ${missing.join(", ")}. ` + + "Set VITE_KINDE_* env vars or disable auth (VITE_ENABLE_AUTH=false)." + ); + } +}Optionally, call validateAuthConfig() once in main.tsx during App startup (see that file’s suggestion).
PROTECTED_ROUTE_USAGE.md (2)
128-133: Tighten prop table wording and fix minor grammarSmall copyedits for clarity and to address grammar tool flags.
Apply:
-| `children` | `React.ReactNode` | Yes | - | The component to render if access is granted | -| `has` | `HasParams` | No | - | KindeAuth has() parameters to check permissions/roles | -| `fallbackPath` | `string` | No | `"/"` | Path to redirect to if access is denied | +| `children` | `React.ReactNode` | Yes | — | Component to render when access is granted | +| `has` | `HasParams` | No | — | Parameters forwarded to your has() implementation (e.g., permissions/roles) | +| `fallbackPath` | `string` | No | `"/"` | Redirect path when access is denied |
153-160: Align best-practices phrasing with the new APIThe list still mentions “access functions” even though the doc promotes has(). Suggest updating wording.
Apply:
-1. **Always provide fallback paths** for better user experience -2. **Use descriptive access function names** for better code readability -3. **Handle undefined users gracefully** in your access functions -4. **Combine multiple conditions** when needed for complex access control -5. **Test your access functions** with different user scenarios +1. **Always provide a fallbackPath** for a better user experience. +2. **Keep has() checks descriptive and localized** (e.g., name helpers/constants for permission keys). +3. **Handle undefined users gracefully** in your guard logic. +4. **Combine multiple conditions** when needed (roles + permissions + flags). +5. **Test access scenarios** with different users, orgs, roles, and feature flags.src/index.css (1)
101-104: Improve focus visibility: add ring offset for contrast across themesAdding a ring offset improves UX on both light and dark backgrounds.
Apply:
@utility focus-ring { - @apply focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring; + @apply focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring + focus-visible:ring-offset-2 focus-visible:ring-offset-background; }src/App.tsx (1)
10-18: Add basic a11y to the loading UIExpose loading state to assistive tech.
Apply:
- if (isLoading) { + if (isLoading) { return ( - <div className="min-h-screen flex items-center justify-center"> + <div + className="min-h-screen flex items-center justify-center" + role="status" + aria-live="polite" + aria-busy="true" + > <div className="text-center"> - <div className="animate-spin rounded-full h-12 w-12 border-b-2 border-gray-600 mx-auto mb-4"></div> + <div className="animate-spin rounded-full h-12 w-12 border-b-2 border-gray-600 mx-auto mb-4" aria-hidden="true"></div> <p className="text-gray-600">Loading...</p> </div> </div> ); }src/components/LoggedIn.tsx (2)
7-13: Prevent double-click sign-out; surface in-progress state and improve a11yDisable the button while awaiting
logout()and reflect busy state to avoid multiple calls and confusing UX.Apply within these ranges:
const handleLogout = async () => { try { - await logout(); + setIsLoggingOut(true); + await logout(); } catch (error) { console.error("Logout failed:", error); + } finally { + setIsLoggingOut(false); } };- <button - onClick={handleLogout} - className="px-3 py-2 bg-gray-600 text-white rounded-md hover:bg-gray-700 font-medium" - > + <button + onClick={handleLogout} + disabled={isLoggingOut} + aria-busy={isLoggingOut} + aria-disabled={isLoggingOut} + className="px-3 py-2 bg-gray-600 text-white rounded-md hover:bg-gray-700 font-medium disabled:opacity-50 disabled:cursor-not-allowed" + > Sign out </button>Additional changes outside the selected ranges (add once):
// add at top-level imports import { useState } from "react"; // inside LoggedIn(), after useKindeAuth() const [isLoggingOut, setIsLoggingOut] = useState(false);Also applies to: 51-56
28-33: Tighten img accessibility and performance
- Use a descriptive alt, preferably the user’s name.
- Add lazy loading and async decoding to defer offscreen work.
- Consider fixed width/height to reduce CLS.
- <img - src={user.picture} - alt="user profile avatar" - className="w-10 h-10 rounded-full" - /> + <img + src={user.picture} + alt={`${(user?.givenName ?? user?.given_name ?? "User")} profile avatar`} + loading="lazy" + decoding="async" + width={40} + height={40} + className="w-10 h-10 rounded-full" + />AUTH_SETUP.md (3)
50-57: Clarify that VITE_ are public and add a brief security note*Variables prefixed with
VITE_are exposed to client code; ensure readers don’t put secrets here (client IDs are fine; client secrets are not).Suggested addition immediately below the table:
> Note: VITE_* variables are embedded into the client bundle at build time. Do not place secrets (like client secrets or API keys intended for server-side use) in these variables. Client IDs and domains are safe to expose.
60-65: Be explicit about exact Redirect/Logout URIs in KindeKinde requires exact matches for Redirect and Logout URLs. Call this out to reduce setup friction and link it to whatever you pass into
KindeProvider(e.g.,redirectUri/logoutUri).Proposed wording tweak:
4. Configure your Redirect and Logout URLs in Kinde to EXACTLY match the values you use in your app (e.g., the `redirectUri` and `logoutUri` passed to `KindeProvider`). Mismatches (including port or path) will prevent sign-in/sign-out from completing.If helpful, I can add a short snippet showing where these values are wired in
src/main.tsxorsrc/config/auth.ts.
39-47: Keep guide and UI terminology consistentThe guide mentions “account management,” while the UI shows an “Account”
PortalLink. Consider standardizing phrasing (e.g., “Account” everywhere) to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.env_sample(1 hunks)AUTH_SETUP.md(1 hunks)PROTECTED_ROUTE_USAGE.md(1 hunks)package.json(1 hunks)pnpm-workspace.yaml(0 hunks)src/App.tsx(1 hunks)src/components/LoggedIn.tsx(1 hunks)src/components/LoggedOut.tsx(1 hunks)src/config/auth.ts(1 hunks)src/index.css(1 hunks)src/main.tsx(1 hunks)tsconfig.app.json(1 hunks)vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- pnpm-workspace.yaml
🧰 Additional context used
🧬 Code graph analysis (2)
src/main.tsx (2)
src/config/auth.ts (1)
AUTH_CONFIG(1-12)src/App.tsx (1)
App(24-44)
src/App.tsx (3)
src/components/LoggedIn.tsx (1)
LoggedIn(4-69)src/components/LoggedOut.tsx (1)
LoggedOut(6-32)src/config/auth.ts (1)
AUTH_CONFIG(1-12)
🪛 LanguageTool
PROTECTED_ROUTE_USAGE.md
[grammar] ~130-~130: There might be a mistake here.
Context: ...mponent to render if access is granted | | has | HasParams | No | - | KindeAu...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ... parameters to check permissions/roles | | fallbackPath | string | No | "/"...
(QB_NEW_EN)
[grammar] ~207-~207: There might be a mistake here.
Context: ... and roles** in your KindeAuth dashboard 2. Creating test users with specific perm...
(QB_NEW_EN)
[grammar] ~208-~208: There might be a mistake here.
Context: ... users** with specific permissions/roles 3. Using the examples page at /examples...
(QB_NEW_EN)
[grammar] ~209-~209: There might be a mistake here.
Context: ... see different access controls in action 4. Testing feature flags by enabling/disa...
(QB_NEW_EN)
AUTH_SETUP.md
[grammar] ~50-~50: There might be a mistake here.
Context: ... | Default | | ------------------------- | ----------...
(QB_NEW_EN)
[grammar] ~51-~51: There might be a mistake here.
Context: ...--------- | -------------------------- | | VITE_ENABLE_AUTH | Enable/dis...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...ntication | false | | VITE_KINDE_CLIENT_ID | Kinde clie...
(QB_NEW_EN)
[grammar] ~53-~53: There might be a mistake here.
Context: ... | Required when auth enabled | | VITE_KINDE_DOMAIN | Kinde doma...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ... | Required when auth enabled | | VITE_KINDE_REDIRECT_URL | Redirect U...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...uth | Required when auth enabled | | VITE_KINDE_LOGOUT_URL | Logout red...
(QB_NEW_EN)
🔇 Additional comments (11)
vite.config.ts (1)
3-3: Tailwind Vite plugin integration looks goodImporting and registering
@tailwindcss/viteis correct for Tailwind v4 + Vite 6.Also applies to: 7-7
.env_sample (2)
1-3: Good addition of feature flag for auth
VITE_ENABLE_AUTH=falsemakes local dev opt-in and reduces setup friction. Keep as a sample default.
5-9: Env variables parsing & runtime alignment confirmed
VITE_ENABLE_AUTHis centrally parsed as a boolean insrc/config/auth.tsvia
ENABLED: import.meta.env.VITE_ENABLE_AUTH === "true"—no scattered string comparisons.- The dev server port is explicitly set to 3000 in
vite.config.tsand matches the redirect/logout URLs in.env_sample.- No router basename is configured under
src/, so the default root path is in use.No changes required.
src/components/LoggedOut.tsx (2)
8-30: LGTM on the simplified, consistent logged-out layoutClean nav, obvious auth CTAs, and Tailwind utility classes are consistent with the rest of the kit.
1-4: LoggedOut always rendered within KindeProvider when using LoginLink/RegisterLinkI’ve verified that:
- In
main.tsx, whenAUTH_CONFIG.ENABLEDis true, the entire<App />(and by extension<LoggedOut />) is wrapped in<KindeProvider>; when disabled,<App />returns a non-auth view without rendering<LoggedOut />at all.- In
App.tsx, theAuthenticatedApp(which returns<LoggedOut />on unauthenticated) only mounts under the same enabled flag, ensuring the provider is always present for those links.Since
LoginLink/RegisterLinkcan only render when the provider is active, no additional fallback logic or guards are required here.Likely an incorrect or invalid review comment.
PROTECTED_ROUTE_USAGE.md (1)
86-124: Missing ProtectedRoute and has() Implementation – Please VerifyI wasn’t able to find any
ProtectedRoutecomponent or ahas()helper in the codebase. Please confirm:
- That a
ProtectedRoutecomponent is defined and exported (e.g. in a.ts,.tsx,.js, or.jsxfile).- That a
has()wrapper exists and is invoked byProtectedRoutefor permission, role, feature-flag, and billing checks.If these are not yet implemented, either:
- Add the
has()helper (and haveProtectedRouteforward to it), or- Update the documentation to use the official SDK methods (
getPermission/getPermissionsandgetFlag) instead ofhas().Let me know once you’ve verified or added these pieces, and I can help adjust the examples accordingly.
src/index.css (1)
1-104: Verified: Project is on Tailwind CSS v4
package.json specifies tailwindcss ^4.1.11, and src/index.css correctly uses v4-only at-rules (@import "tailwindcss",@theme,@utility). No changes required.src/main.tsx (1)
24-24: LGTM on root renderMounting AppWrapper at #root is straightforward and keeps provider logic centralized.
src/App.tsx (1)
24-44: Good separation to avoid conditional hook callsSplitting AuthenticatedApp keeps useKindeAuth isolated from disabled-auth code paths. Clean, readable, and avoids React hook rule pitfalls.
src/components/LoggedIn.tsx (1)
48-50: PortalLink support confirmed in v5.6.0
Version 5.6.0 of@kinde-oss/kinde-auth-reactexportsPortalLinkfrom itscomponentsentrypoint, andPortalLinkPropsextendsReact.ButtonHTMLAttributes<HTMLButtonElement>(includingclassName) plus optionalreturnUrlandsubNav. Your usage with aclassNameprop is valid, so no changes are needed.AUTH_SETUP.md (1)
20-24: Verify Dev Server Port ConfigurationIt looks like there’s no explicit port override in your Vite setup (via
vite.config.*) or in thedevscript ofpackage.json, which means you’re most likely running on Vite’s default port 5173. However, your AUTH_SETUP.md example still useslocalhost:3000, which can lead to “redirect mismatch” errors if the actual dev server port differs.Please confirm your actual development server port and update the environment variable examples accordingly:
• vite.config.* – no
server.portsetting found; defaults to 5173
• package.json – noscripts.devoverride detected (you may be using a different script name; please verify)
• AUTH_SETUP.md (lines 20–24) currently shows port 3000Suggested change to AUTH_SETUP.md:
- VITE_KINDE_REDIRECT_URL=http://localhost:3000 - VITE_KINDE_LOGOUT_URL=http://localhost:3000 + VITE_KINDE_REDIRECT_URL=http://localhost:5173 + VITE_KINDE_LOGOUT_URL=http://localhost:5173If you are running on a custom port (e.g., via an alternate script or environment file), please adjust these URLs to match your actual configuration.
| "start": "vite", | ||
| "dev": "vite", | ||
| "build": "tsc -b && vite build", | ||
| "build": "vite build", | ||
| "lint": "eslint .", | ||
| "preview": "vite preview" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reintroduce type-checking in the build pipeline
Switching build to only vite build drops TypeScript checks entirely. Keep fast builds, but fail early on type errors.
Apply this diff:
"scripts": {
"start": "vite",
"dev": "vite",
- "build": "vite build",
+ "typecheck": "tsc -b --noEmit --pretty false",
+ "build": "npm run typecheck && vite build",
"lint": "eslint .",
"preview": "vite preview"
},(Optional) If you prefer IDE-only typechecking, consider vite-plugin-checker to run TS checks in dev instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "start": "vite", | |
| "dev": "vite", | |
| "build": "tsc -b && vite build", | |
| "build": "vite build", | |
| "lint": "eslint .", | |
| "preview": "vite preview" | |
| }, | |
| "start": "vite", | |
| "dev": "vite", | |
| "typecheck": "tsc -b --noEmit --pretty false", | |
| "build": "npm run typecheck && vite build", | |
| "lint": "eslint .", | |
| "preview": "vite preview" | |
| }, |
🤖 Prompt for AI Agents
In package.json around lines 6-11, the "build" script currently runs only "vite
build" which skips TypeScript type-checking; update the build script to run the
TypeScript compiler with no emit before Vite (for example: "tsc --noEmit && vite
build") so builds fail early on type errors, and ensure tsconfig.json exists and
"typescript" is in devDependencies; optionally add a separate "typecheck" script
("tsc --noEmit") or use vite-plugin-checker for dev-time checks.
| ## Component Props | ||
|
|
||
| | Prop | Type | Required | Default | Description | | ||
| |------|------|----------|---------|-------------| | ||
| | `children` | `React.ReactNode` | Yes | - | The component to render if access is granted | | ||
| | `has` | `HasParams` | No | - | KindeAuth has() parameters to check permissions/roles | | ||
| | `fallbackPath` | `string` | No | `"/"` | Path to redirect to if access is denied | | ||
|
|
||
| ## HasParams Type | ||
|
|
||
| The `has` prop accepts the same parameters as KindeAuth's `has()` function: | ||
|
|
||
| ```tsx | ||
| interface HasParams { | ||
| roles?: string[]; | ||
| permissions?: string[]; | ||
| featureFlags?: string[]; | ||
| billingEntitlements?: string[]; | ||
| forceApi?: boolean | { | ||
| roles?: boolean; | ||
| permissions?: boolean; | ||
| featureFlags?: boolean; | ||
| billingEntitlements?: true; | ||
| }; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Fix type bug: forceApi.billingEntitlements should be boolean, not the literal type true
This would mislead consumers and breaks accurate typing.
Apply:
interface HasParams {
roles?: string[];
permissions?: string[];
featureFlags?: string[];
billingEntitlements?: string[];
forceApi?: boolean | {
roles?: boolean;
permissions?: boolean;
featureFlags?: boolean;
- billingEntitlements?: true;
+ billingEntitlements?: boolean;
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Component Props | |
| | Prop | Type | Required | Default | Description | | |
| |------|------|----------|---------|-------------| | |
| | `children` | `React.ReactNode` | Yes | - | The component to render if access is granted | | |
| | `has` | `HasParams` | No | - | KindeAuth has() parameters to check permissions/roles | | |
| | `fallbackPath` | `string` | No | `"/"` | Path to redirect to if access is denied | | |
| ## HasParams Type | |
| The `has` prop accepts the same parameters as KindeAuth's `has()` function: | |
| ```tsx | |
| interface HasParams { | |
| roles?: string[]; | |
| permissions?: string[]; | |
| featureFlags?: string[]; | |
| billingEntitlements?: string[]; | |
| forceApi?: boolean | { | |
| roles?: boolean; | |
| permissions?: boolean; | |
| featureFlags?: boolean; | |
| billingEntitlements?: true; | |
| }; | |
| } | |
| ``` | |
| interface HasParams { | |
| roles?: string[]; | |
| permissions?: string[]; | |
| featureFlags?: string[]; | |
| billingEntitlements?: string[]; | |
| forceApi?: boolean | { | |
| roles?: boolean; | |
| permissions?: boolean; | |
| featureFlags?: boolean; | |
| billingEntitlements?: boolean; | |
| }; | |
| } |
🧰 Tools
🪛 LanguageTool
[grammar] ~130-~130: There might be a mistake here.
Context: ...mponent to render if access is granted | | has | HasParams | No | - | KindeAu...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ... parameters to check permissions/roles | | fallbackPath | string | No | "/"...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In PROTECTED_ROUTE_USAGE.md around lines 126 to 151, the HasParams type
incorrectly declares forceApi.{billingEntitlements} as the literal type true;
change that property to boolean so forceApi is typed as boolean | { roles?:
boolean; permissions?: boolean; featureFlags?: boolean; billingEntitlements?:
boolean; } to accurately reflect possible values and avoid misleading consumers.
| {user?.givenName?.[0] || user?.familyName?.[0] || "U"} | ||
| </div> | ||
| )} | ||
| <div className="text-sm"> | ||
| <p className="font-medium text-gray-800"> | ||
| {user?.givenName} {user?.familyName} | ||
| </p> | ||
| <p className="text-gray-500">{user?.email}</p> | ||
| </div> |
There was a problem hiding this comment.
Fix Kinde user field names (camelCase vs snake_case) to avoid empty UI and potential TS errors
Kinde’s React SDK typically exposes user.given_name and user.family_name. The current code uses givenName/familyName, which may render blanks and can trigger TS property errors depending on SDK typings.
Minimal, safe inline change:
- {user?.givenName?.[0] || user?.familyName?.[0] || "U"}
+ {(user?.givenName ?? user?.given_name ?? user?.familyName ?? user?.family_name ?? user?.email ?? "U")[0]?.toUpperCase()}- {user?.givenName} {user?.familyName}
+ {(user?.givenName ?? user?.given_name) || ""}
+ {" "}
+ {(user?.familyName ?? user?.family_name) || ""}As a follow-up, consider a small helper to centralize formatting and keep JSX clean:
// inside component, above return
function getDisplayName(u?: any) {
if (!u) return "";
const first = u.givenName ?? u.given_name ?? "";
const last = u.familyName ?? u.family_name ?? "";
return [first, last].filter(Boolean).join(" ");
}
function getInitials(u?: any) {
const name = getDisplayName(u) || u?.email || "U";
const parts = name.trim().split(/\s+/).filter(Boolean);
const initials = (parts[0]?.[0] || "") + (parts[1]?.[0] || "");
return (initials || name[0] || "U").toUpperCase();
}Then use {getInitials(user)} and {getDisplayName(user)} in JSX.
🤖 Prompt for AI Agents
In src/components/LoggedIn.tsx around lines 35 to 43, the component currently
uses user.givenName and user.familyName which Kinde's SDK often exposes as
given_name and family_name; update the JSX to safely prefer camelCase but fall
back to snake_case (e.g. use user?.givenName ?? user?.given_name and
user?.familyName ?? user?.family_name) when rendering initials and the display
name, or implement the suggested helper functions getDisplayName and getInitials
inside the component and replace the inline expressions with {getInitials(user)}
and {getDisplayName(user)} to avoid empty UI and TypeScript property errors.
src/main.tsx
Outdated
| const AppWrapper = () => { | ||
| if (AUTH_CONFIG.ENABLED) { | ||
| return ( | ||
| <KindeProvider | ||
| clientId={AUTH_CONFIG.KINDE.CLIENT_ID} | ||
| domain={AUTH_CONFIG.KINDE.DOMAIN} | ||
| logoutUri={AUTH_CONFIG.KINDE.LOGOUT_URL} | ||
| redirectUri={AUTH_CONFIG.KINDE.REDIRECT_URL} | ||
| > | ||
| <App /> | ||
| </KindeProvider> | ||
| ); | ||
| } | ||
|
|
||
| return <App />; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prevent runtime failures when auth is enabled but env is incomplete; default redirect/logout URIs
KindeProvider requires clientId, domain, and redirectUri; logoutUri is optional. Passing undefined will cause misconfig at runtime. Add a small guard and reasonable defaults for URIs. (docs.kinde.com)
Apply:
-import { AUTH_CONFIG } from "./config/auth.ts";
+import { AUTH_CONFIG, validateAuthConfig } from "./config/auth.ts";
const AppWrapper = () => {
+ validateAuthConfig();
if (AUTH_CONFIG.ENABLED) {
+ const { CLIENT_ID, DOMAIN, REDIRECT_URL, LOGOUT_URL } = AUTH_CONFIG.KINDE;
+ if (!CLIENT_ID || !DOMAIN || !REDIRECT_URL) {
+ console.error(
+ "[auth] Missing Kinde config. Set VITE_KINDE_CLIENT_ID, VITE_KINDE_DOMAIN, and VITE_KINDE_REDIRECT_URL."
+ );
+ // Fall back to unauthenticated app to avoid a hard crash.
+ return <App />;
+ }
return (
<KindeProvider
- clientId={AUTH_CONFIG.KINDE.CLIENT_ID}
- domain={AUTH_CONFIG.KINDE.DOMAIN}
- logoutUri={AUTH_CONFIG.KINDE.LOGOUT_URL}
- redirectUri={AUTH_CONFIG.KINDE.REDIRECT_URL}
+ clientId={CLIENT_ID}
+ domain={DOMAIN}
+ logoutUri={LOGOUT_URL ?? window.location.origin}
+ redirectUri={REDIRECT_URL ?? window.location.origin}
>
<App />
</KindeProvider>
);
}
return <App />;
};Optionally, wire KindeProvider callbacks (onSuccess/onError) to surface auth errors in dev. (docs.kinde.com)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main.tsx around lines 7-22, currently KindeProvider is rendered when
AUTH_CONFIG.ENABLED even if required env values may be missing; add a guard that
verifies AUTH_CONFIG.KINDE.CLIENT_ID and AUTH_CONFIG.KINDE.DOMAIN are present
and non-empty before rendering KindeProvider, otherwise log an error/warning and
fall back to returning <App /> without the provider; when rendering
KindeProvider, ensure redirectUri and logoutUri default to a safe value (e.g.,
window.location.origin or `${window.location.origin}/`), avoid passing
undefined, and optionally wire simple onSuccess and onError callbacks
(console.log or dev-only logger) to surface auth errors during development.
| "strict": false, | ||
| "noImplicitAny": false, | ||
| "strictNullChecks": false, | ||
| "noUnusedLocals": false, | ||
| "noUnusedParameters": false, | ||
| "noFallthroughCasesInSwitch": false, | ||
| "noUncheckedSideEffectImports": false |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reconsider disabling core type-safety flags
Turning off strict, noImplicitAny, strictNullChecks, and unused checks removes meaningful safeguards. Combined with the build no longer running tsc, this can let type errors ship unnoticed.
Minimal, safer alternative:
- Keep
strict: true. - If env shapes are the blocker, declare a typed
ImportMetaEnvinsrc/vite-env.d.tsand narrow once in a config module instead of relaxing global flags.
Apply this diff to restore core checks:
- "strict": false,
- "noImplicitAny": false,
- "strictNullChecks": false,
- "noUnusedLocals": false,
- "noUnusedParameters": false,
- "noFallthroughCasesInSwitch": false,
- "noUncheckedSideEffectImports": false
+ "strict": true,
+ "noImplicitAny": true,
+ "strictNullChecks": true,
+ "noUnusedLocals": true,
+ "noUnusedParameters": true,
+ "noFallthroughCasesInSwitch": true,
+ "noUncheckedSideEffectImports": trueIf you want, I can draft a vite-env.d.ts and a small safeEnv() helper that validates the required vars at startup.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "strict": false, | |
| "noImplicitAny": false, | |
| "strictNullChecks": false, | |
| "noUnusedLocals": false, | |
| "noUnusedParameters": false, | |
| "noFallthroughCasesInSwitch": false, | |
| "noUncheckedSideEffectImports": false | |
| "strict": true, | |
| "noImplicitAny": true, | |
| "strictNullChecks": true, | |
| "noUnusedLocals": true, | |
| "noUnusedParameters": true, | |
| "noFallthroughCasesInSwitch": true, | |
| "noUncheckedSideEffectImports": true |
🤖 Prompt for AI Agents
In tsconfig.app.json around lines 19-25, core TypeScript safety flags were
disabled (strict, noImplicitAny, strictNullChecks, noUnusedLocals,
noUnusedParameters, noFallthroughCasesInSwitch, noUncheckedSideEffectImports);
revert these to safe defaults by setting "strict": true and removing or setting
the other flags to true (or deleting explicit false entries) to restore compiler
checks, and instead address any env typing issues by adding a typed
ImportMetaEnv declaration file (e.g., src/vite-env.d.ts) and a single
config/helper module that narrows/validates process or import.meta.env at
startup (safeEnv) so you don't relax global compiler guarantees.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (4)
tsconfig.app.json (1)
19-25: Fix tsconfig: move baseUrl/paths under compilerOptions and restore core safety flags
- baseUrl and paths are at the top level; TypeScript only recognizes them inside compilerOptions, so the alias may not be honored here.
- Re-enabling strict checks prevents silent type regressions. Also consider enabling noUncheckedSideEffectImports to catch typos in bare side‑effect imports. (typescriptlang.org)
Apply these diffs:
- Re-enable safety flags (Lines 19–25):
- "strict": false, - "noImplicitAny": false, - "strictNullChecks": false, - "noUnusedLocals": false, - "noUnusedParameters": false, - "noFallthroughCasesInSwitch": false, - "noUncheckedSideEffectImports": false + "strict": true, + "noImplicitAny": true, + "strictNullChecks": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "noFallthroughCasesInSwitch": true, + "noUncheckedSideEffectImports": true
- Remove mis-scoped fields (Lines 27–31) and place them inside compilerOptions:
- "include": ["src"], - "baseUrl": ".", - "paths": { - "@/*": ["./src/*"] - } + "include": ["src"]Then add inside compilerOptions (right before its closing brace):
"baseUrl": ".", "paths": { "@/*": ["./src/*"] }Note: noUncheckedSideEffectImports is a valid TS 5.x option to error on missing side‑effect imports. (typescriptlang.org)
#!/usr/bin/env bash # Confirm baseUrl/paths are under compilerOptions python - <<'PY' import json, sys with open("tsconfig.app.json") as f: j = json.load(f) co = j.get("compilerOptions", {}) print("baseUrl in compilerOptions:", "baseUrl" in co) print("paths in compilerOptions:", "paths" in co) PYAlso applies to: 27-31
src/components/AppContent.tsx (1)
22-24: Duplicate app name hardcodingSame issue as in LandingPage - the app name is hardcoded and should be centralized.
Apply the same configurable app name solution as suggested for LandingPage.
package.json (2)
8-8: Reinstate TypeScript checks in builds (same feedback as earlier).Dropping to only "vite build" skips type-checking and lets type errors ship. Add a separate typecheck step and gate builds on it.
"scripts": { "start": "vite", "dev": "vite", - "build": "vite build", + "typecheck": "tsc --noEmit --pretty false", + "build": "npm run typecheck && vite build", "lint": "eslint .", "preview": "vite preview" },
16-16: Remove @types/react-router-dom (v5 types) — RRD v7 ships its own types.Keeping it causes duplicate/incorrect typings with react-router-dom@^7.7.0.
"dependencies": { "@kinde-oss/kinde-auth-react": "^5.6.0", "@radix-ui/react-slot": "^1.2.3", "@tailwindcss/vite": "^4.1.12", - "@types/react-router-dom": "^5.3.3", "class-variance-authority": "^0.7.1",
🧹 Nitpick comments (9)
src/lib/utils.ts (1)
1-6: LGTM; add explicit return type for consistencyMinor: match the explicit return type used elsewhere and make intent clear.
-export function cn(...inputs: ClassValue[]) { +export function cn(...inputs: ClassValue[]): string { return twMerge(clsx(inputs)); }tsconfig.json (1)
7-12: Alias mapping looks good; consider centralizing via a base configTo avoid drift across tsconfig.app.json and tsconfig.node.json, extract shared settings (baseUrl, paths) into tsconfig.base.json and have others extend it.
Example tsconfig.base.json:
{ "compilerOptions": { "baseUrl": ".", "paths": { "@/*": ["./src/*"] } } }Then in tsconfig.app.json:
{ "extends": "./tsconfig.base.json", "compilerOptions": { // app-specific overrides... } }src/components/LandingPage.tsx (1)
7-31: Consider making the app name configurableThe hardcoded "App name" should be configurable through environment variables or a config file for better maintainability.
Add to your environment configuration:
// src/config/app.ts export const APP_CONFIG = { name: process.env.VITE_APP_NAME || 'React Starter Kit' };Then use it in the component:
+import { APP_CONFIG } from '../config/app'; -<div className="text-xl font-semibold text-gray-800">App name</div> +<div className="text-xl font-semibold text-gray-800">{APP_CONFIG.name}</div>src/components/AppContent.tsx (1)
36-36: Potential issue with initials logicThe code
user?.familyName?.[0]should beuser?.familyName?.[0]but the logicuser?.givenName?.[0] || user?.familyName?.[0]only shows one initial. For proper initials, both first and last name initials should be displayed.Consider showing both initials:
-{user?.givenName?.[0] || user?.familyName?.[0] || "U"} +{(user?.givenName?.[0] || '') + (user?.familyName?.[0] || '') || "U"}src/components/ui/button.tsx (3)
63-66: Avoid two ways to mark “destructive”: boolean and variant conflict. Keep only variant="destructive".Having both leads to class conflicts (and surprises when combined with other variants).
iconOnly: { true: "gap-0 p-0", false: "", }, - destructive: { - true: "bg-[hsl(0,100%,38%)] text-[hsl(0,0%,100%)] hover:bg-[hsl(0,100%,25%)]", - false: "", - }, loading: { true: "pointer-events-none", false: "", }, @@ iconOnly: false, - destructive: false, loading: false,If you must support “destructive + secondary”, prefer cva compoundVariants to explicitly resolve class precedence.
Also applies to: 78-79
84-93: Remove redundant prop declarations; VariantProps already provides these.Simplifies types and avoids divergence.
export interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement>, VariantProps<typeof buttonVariants> { asChild?: boolean; - fullWidth?: boolean; - iconLeading?: boolean; - iconOnly?: boolean; - destructive?: boolean; - loading?: boolean; }
45-62: iconOnly semantics: enforce consistent geometry.iconOnly=true with size≠"icon" yields odd padding/shape. Either coerce size="icon" when iconOnly is true, or add a compoundVariant to normalize.
Example compoundVariant (add under the cva options):
{ variants: { ... }, + compoundVariants: [ + { + iconOnly: true, + class: "h-12 w-12 p-0 rounded-[50%]" + } + ], defaultVariants: { ... } }Or document that callers should pass size="icon" when iconOnly is true.
package.json (2)
15-15: Move Tailwind build-time tooling to devDependencies.tailwindcss and @tailwindcss/vite are build-only; keeping them in dependencies bloats prod installs/containers.
"dependencies": { "@kinde-oss/kinde-auth-react": "^5.6.0", "@radix-ui/react-slot": "^1.2.3", - "@tailwindcss/vite": "^4.1.12", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "lucide-react": "^0.542.0", "react": "^19.1.0", "react-dom": "^19.1.0", "react-router-dom": "^7.7.0", "tailwind-merge": "^3.3.1", - "tailwindcss": "^4.1.12" + // keep runtime UI deps here }, "devDependencies": { "@eslint/js": "^9.28.0", "@types/node": "^24.3.0", "@types/react": "^19.1.6", "@types/react-dom": "^19.1.5", "@vitejs/plugin-react": "^4.5.1", "eslint": "^9.28.0", "eslint-plugin-react-hooks": "^5.2.0", "eslint-plugin-react-refresh": "^0.4.20", "globals": "^16.2.0", "tw-animate-css": "^1.3.7", "typescript": "^5.8.3", "typescript-eslint": "^8.33.1", "vite": "^6.3.5", + "@tailwindcss/vite": "^4.1.12", + "tailwindcss": "^4.1.12" }Also applies to: 24-24, 26-40
28-28: Add an engines field to enforce a supported Node runtime.Vite 6 and Tailwind v4 expect modern Node. Recommend pinning Node to avoid CI/prod drift.
{ "name": "kinde-react-starter-kit", "version": "2.0.0", "type": "module", + "engines": { + "node": ">=18.18.0 || >=20" + }, "scripts": {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
components.json(1 hunks)lib/utils.ts(1 hunks)package.json(1 hunks)src/App.tsx(1 hunks)src/components/AppContent.tsx(1 hunks)src/components/LandingPage.tsx(1 hunks)src/components/ui/button.tsx(1 hunks)src/index.css(1 hunks)src/lib/utils.ts(1 hunks)src/main.tsx(1 hunks)tsconfig.app.json(1 hunks)tsconfig.json(1 hunks)vite.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main.tsx
- vite.config.ts
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/LandingPage.tsx (2)
src/components/LoggedOut.tsx (1)
LoggedOut(6-56)src/components/LoggedIn.tsx (1)
LoggedIn(4-75)
src/components/AppContent.tsx (2)
src/components/ui/button.tsx (1)
Button(135-135)src/components/LoggedIn.tsx (1)
LoggedIn(4-75)
src/components/ui/button.tsx (2)
lib/utils.ts (1)
cn(4-6)src/lib/utils.ts (1)
cn(4-6)
src/App.tsx (1)
src/components/LandingPage.tsx (1)
LandingPage(7-31)
🔇 Additional comments (16)
components.json (1)
6-12: Verify Tailwind config presence before setting the path
I didn’t find anytailwind.config.*file in the repository. Before pointing"config"at a non-existent file, please confirm or add your Tailwind configuration.• File:
components.json, lines 6–12
• Suggested change (once a config exists):"tailwind": { - "config": "", + "config": "tailwind.config.ts", # or "tailwind.config.js"/".cjs" as appropriate "css": "src/index.css", "baseColor": "neutral", "cssVariables": true, "prefix": "" },If you haven’t generated a config yet, you can run:
npx tailwindcss init [tailwind.config.ts]Then update
components.jsonaccordingly.src/index.css (4)
1-2: LGTM on Tailwind CSS integration!The import statements properly enable Tailwind CSS and its animation utilities. The structure follows best practices for modern CSS architecture.
7-36: Well-structured semantic token systemThe
@themeblock establishes a clean semantic token system using HSL values. The color naming convention (background, foreground, primary, etc.) is consistent and follows design system best practices.
38-83: Comprehensive light theme implementationThe color system uses modern OKLCH color space for better perceptual uniformity. The semantic token organization is thorough, covering all UI components including charts and sidebar elements.
119-121: Excellent focus ring utility implementationThe custom
focus-ringutility provides consistent focus styling across the application. Usingfocus-visibleensures it only shows for keyboard navigation.src/components/LandingPage.tsx (2)
1-5: Clean authentication component integrationThe imports properly separate Kinde auth components from local UI components. Good use of the
asChildpattern for composability.
15-20: Good button variant usageProper semantic usage of button variants -
uncontainedfor sign in andprimaryfor sign up, guiding users toward registration.src/components/AppContent.tsx (3)
8-14: Robust logout error handlingThe async logout handler properly catches and logs errors. Good defensive programming approach.
28-38: Solid avatar fallback logicGood progressive enhancement: user picture → initials → generic "U". The fallback logic is comprehensive and handles edge cases.
49-53: Consistent button styling and functionalityGood use of the
asChildpattern for the Account link and proper onclick handler for the logout button. The styling is consistent across navigation elements.src/App.tsx (3)
6-6: Good error handling additionAdding
errorto the destructuring fromuseKindeAuthenables proper error state handling in the authentication flow.
11-20: Excellent loading UI improvementThe new loading screen is much more user-friendly with a proper spinner animation and centered layout. Great UX improvement over a simple "Loading..." text.
22-22: Clean component rendering logicThe conditional rendering is clean and follows React best practices. The authentication state properly drives the UI.
package.json (3)
14-14: UI deps LGTM.class-variance-authority, clsx, lucide-react align with the new UI surface.
Also applies to: 17-19
20-22: React 19 + react-dom 19 + RRD 7 alignment looks good.Versions are in lock-step; no action needed.
36-36: tw-animate-css usage confirmed
The package is imported insrc/index.cssvia:@import "tw-animate-css";Since it’s actively used, please keep
"tw-animate-css": "^1.3.7"in your dependencies.
| import { clsx, type ClassValue } from "clsx"; | ||
| import { twMerge } from "tailwind-merge"; | ||
|
|
||
| export function cn(...inputs: ClassValue[]): string { | ||
| return twMerge(clsx(inputs)); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid duplicating cn; keep a single source of truth under src/lib/utils.ts
You now have two identical cn implementations (here and in src/lib/utils.ts). This risks drift and confusing imports (plain vs "@/lib/utils"). Prefer one canonical module and re-export (or remove this file).
Apply one of the following:
Option A — re-export to preserve any existing imports:
-import { clsx, type ClassValue } from "clsx";
-import { twMerge } from "tailwind-merge";
-
-export function cn(...inputs: ClassValue[]): string {
- return twMerge(clsx(inputs));
-}
+// Deprecated: prefer importing from "@/lib/utils"
+export { cn } from "@/lib/utils";Option B — delete this file and update any imports to "@/lib/utils".
Run to find call sites that may still import from lib/utils:
🏁 Script executed:
#!/usr/bin/env bash
rg -nP --type ts --type tsx -C1 $'from\\s+[\'"](?:\\.?\\.?/)?lib/utils[\'"]|from\\s+[\'"]@/lib/utils[\'"]'Length of output: 139
Here’s an updated search that skips the broken --type flags and explicitly targets .ts and .tsx files:
🏁 Script executed:
#!/usr/bin/env bash
# Look for imports of either relative lib/utils or the canonical @/lib/utils
rg -n -C1 -g '*.ts' -g '*.tsx' "from\s+['\"](?:\.\.?\/)?lib/utils['\"]|from\s+['\"]@/lib/utils['\"]"Length of output: 259
Here’s a simpler, fixed-string search that should catch any remaining imports of lib/utils or @/lib/utils without regex errors:
🏁 Script executed:
#!/usr/bin/env bash
# Search .ts/.tsx files for literal imports of lib/utils or @/lib/utils
rg -n -C1 -g '*.ts' -g '*.tsx' \
-e "from 'lib/utils'" \
-e "from \"lib/utils\"" \
-e "from '@/lib/utils'" \
-e "from \"@/lib/utils\""Length of output: 136
🏁 Script executed:
#!/usr/bin/env bash
# Search for any references to lib/utils or src/lib/utils in TypeScript files
rg -n -C1 -g '*.ts' -g '*.tsx' -e 'lib/utils' -e 'src/lib/utils'Length of output: 195
Consolidate cn into a single source of truth
The codebase currently has two identical cn functions—one in lib/utils.ts and another in src/lib/utils.ts—which risks API drift and confusing imports. We found at least one remaining import of the duplicate in:
- src/components/ui/button.tsx:5
import { cn } from "../../lib/utils";
Please consolidate by choosing one of the following:
Option A — Re-export the canonical implementation to preserve existing imports and mark this file deprecated:
--- a/lib/utils.ts
+++ b/lib/utils.ts
@@ -1,6 +1,3 @@
- import { clsx, type ClassValue } from "clsx";
- import { twMerge } from "tailwind-merge";
-
- export function cn(...inputs: ClassValue[]): string {
- return twMerge(clsx(inputs));
- }
+ // Deprecated: use the implementation in "@/lib/utils"
+ export { cn } from "@/lib/utils";Then update call sites like src/components/ui/button.tsx to:
-import { cn } from "../../lib/utils";
+import { cn } from "@/lib/utils";Option B — Delete lib/utils.ts entirely and update all imports to point at "@/lib/utils" (or "src/lib/utils" if not aliased).
Either approach ensures there’s one canonical cn and avoids future divergence.
🤖 Prompt for AI Agents
In lib/utils.ts around lines 1-6, this duplicate cn implementation should be
consolidated; implement Option A by re-exporting the canonical cn from the
single source (src/lib/utils.ts or "@/lib/utils") and mark this file deprecated:
replace the function body with a single export that imports cn from the
canonical module and re-exports it, add a brief deprecation comment, then update
remaining call sites (e.g., src/components/ui/button.tsx) to import from the
canonical "@/lib/utils" path; run a global search/replace to update any other
../../lib/utils imports and run the typecheck/build to ensure no broken imports
remain.
| if (error) { | ||
| return <div>"Error"</div>; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error display
The error message <div>"Error"</div> is too generic and includes unnecessary quotes. Users need more helpful error information.
Provide a more informative error display:
-return <div>"Error"</div>;
+return (
+ <div className="min-h-screen flex items-center justify-center">
+ <div className="text-center">
+ <p className="text-red-600 mb-2">Authentication Error</p>
+ <p className="text-gray-600 text-sm">Please refresh the page and try again.</p>
+ </div>
+ </div>
+);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (error) { | |
| return <div>"Error"</div>; | |
| } | |
| if (error) { | |
| return ( | |
| <div className="min-h-screen flex items-center justify-center"> | |
| <div className="text-center"> | |
| <p className="text-red-600 mb-2">Authentication Error</p> | |
| <p className="text-gray-600 text-sm">Please refresh the page and try again.</p> | |
| </div> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/App.tsx around lines 8 to 10, the error branch returns a generic string
literal with unnecessary quotes ("Error"); replace this with a user-friendly
error UI that displays the actual error message and a brief action (e.g.,
"Something went wrong: {error.message}" plus an optional Retry button or link).
Ensure you safely access the error message (guarding for null/unknown shapes)
and style the container consistently with the app (add role="alert" for
accessibility).
src/components/ui/button.tsx
Outdated
| import { cn } from "../../lib/utils"; | ||
|
|
||
| const buttonVariants = cva( | ||
| "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:ring-2 focus-visible:ring-offset-2", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure disabled visuals work with asChild and avoid animating “all”.
Add aria-disabled styles for anchor-based buttons and prefer transition-colors.
- "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:ring-2 focus-visible:ring-offset-2",
+ "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-colors disabled:pointer-events-none disabled:opacity-50 aria-disabled:pointer-events-none aria-disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:ring-2 focus-visible:ring-offset-2",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:ring-2 focus-visible:ring-offset-2", | |
| "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-colors disabled:pointer-events-none disabled:opacity-50 aria-disabled:pointer-events-none aria-disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:ring-2 focus-visible:ring-offset-2", |
🤖 Prompt for AI Agents
In src/components/ui/button.tsx around line 8, the button class string uses
transition-all and lacks aria-disabled rules so disabled visuals break when
using asChild (e.g., anchor tags); change transition-all to transition-colors
and add aria-disabled:pointer-events-none and aria-disabled:opacity-50 (or
equivalent aria-disabled styles) so anchor-based buttons reflect disabled state,
and keep existing disabled: rules for native elements to preserve behavior.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
src/components/ui/button.tsx (3)
8-8: Avoid transition-all; add aria-disabled styles (asChild-safe).- "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:ring-2 focus-visible:ring-offset-2", + "inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md text-sm font-medium transition-colors disabled:pointer-events-none disabled:opacity-50 aria-disabled:pointer-events-none aria-disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:ring-2 focus-visible:ring-offset-2",
12-32: Tailwind has no “loading:” variant; use data-[loading]:…- "loading:bg-[var(--color-button-primary-bg-loading)]", + "data-[loading]:bg-[var(--color-button-primary-bg-loading)]", @@ - "loading:bg-[var(--color-button-secondary-bg-loading)]", + "data-[loading]:bg-[var(--color-button-secondary-bg-loading)]", @@ - "loading:bg-[var(--color-button-uncontained-bg-loading)]", + "data-[loading]:bg-[var(--color-button-uncontained-bg-loading)]",
125-143: Fix className drop, default type, and loading/disabled semantics.Preserves consumer className, prevents accidental form submits, and makes asChild anchors “disabled” correctly.
) => { - const Comp = asChild ? Slot : "button"; + const Comp = asChild ? Slot : "button"; + const isDisabled = Boolean(loading || props.disabled); return ( <Comp - className={cn( - buttonVariants({ - variant, - size, - fullWidth, - iconLeading, - iconOnly, - destructive, - loading, - className, - }) - )} - ref={ref} - {...props} + ref={ref} + className={cn( + buttonVariants({ + variant, + size, + fullWidth, + iconLeading, + iconOnly, + destructive, + loading, + }), + className + )} + disabled={!asChild ? isDisabled : undefined} + aria-disabled={isDisabled || undefined} + aria-busy={loading || undefined} + data-loading={loading ? "" : undefined} + data-slot="button" + type={!asChild ? (props.type ?? "button") : undefined} + {...props} /> ); }
🧹 Nitpick comments (2)
src/components/ui/button.tsx (2)
5-5: Unify utils import path with other UI files.Other components use "@/lib/utils". Prefer the alias for consistency.
-import { cn } from "../../lib/utils"; +import { cn } from "@/lib/utils";
77-80: Remove boolean “destructive” variant to avoid duplication.You already have variant="destructive". The boolean duplicates styles and increases API surface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.json(1 hunks)src/components/ui/avatar.tsx(1 hunks)src/components/ui/button.tsx(1 hunks)src/components/ui/input.tsx(1 hunks)src/index.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/index.css
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/ui/avatar.tsx (1)
src/components/LoggedIn.tsx (1)
LoggedIn(4-75)
src/components/ui/button.tsx (2)
lib/utils.ts (1)
cn(4-6)src/lib/utils.ts (1)
cn(4-6)
| function Avatar({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof AvatarPrimitive.Root>) { | ||
| return ( | ||
| <AvatarPrimitive.Root | ||
| data-slot="avatar" | ||
| className={cn( | ||
| "relative flex size-8 shrink-0 overflow-hidden rounded-full", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Forward refs to match Radix primitives.
Prevents ref loss and aligns with Radix patterns.
-function Avatar({
- className,
- ...props
-}: React.ComponentProps<typeof AvatarPrimitive.Root>) {
- return (
- <AvatarPrimitive.Root
- data-slot="avatar"
- className={cn(
- "relative flex size-8 shrink-0 overflow-hidden rounded-full",
- className
- )}
- {...props}
- />
- )
-}
+const Avatar = React.forwardRef<
+ React.ElementRef<typeof AvatarPrimitive.Root>,
+ React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Root>
+>(({ className, ...props }, ref) => (
+ <AvatarPrimitive.Root
+ ref={ref}
+ data-slot="avatar"
+ className={cn("relative flex size-8 shrink-0 overflow-hidden rounded-full", className)}
+ {...props}
+ />
+));
+Avatar.displayName = "Avatar";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function Avatar({ | |
| className, | |
| ...props | |
| }: React.ComponentProps<typeof AvatarPrimitive.Root>) { | |
| return ( | |
| <AvatarPrimitive.Root | |
| data-slot="avatar" | |
| className={cn( | |
| "relative flex size-8 shrink-0 overflow-hidden rounded-full", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| const Avatar = React.forwardRef< | |
| React.ElementRef<typeof AvatarPrimitive.Root>, | |
| React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Root> | |
| >(({ className, ...props }, ref) => ( | |
| <AvatarPrimitive.Root | |
| ref={ref} | |
| data-slot="avatar" | |
| className={cn( | |
| "relative flex size-8 shrink-0 overflow-hidden rounded-full", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| )); | |
| Avatar.displayName = "Avatar"; |
🤖 Prompt for AI Agents
In src/components/ui/avatar.tsx around lines 6-20, the Avatar component
currently doesn't forward refs which breaks Radix patterns and can cause ref
loss; wrap the component with React.forwardRef and accept a ref param typed as
React.ElementRef<typeof AvatarPrimitive.Root>, change the props type to
React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Root>, pass the ref
through to AvatarPrimitive.Root (ref={ref}), and set Avatar.displayName for
better DevTools labeling (e.g., Avatar.displayName =
AvatarPrimitive.Root.displayName ?? "Avatar").
| function AvatarImage({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof AvatarPrimitive.Image>) { | ||
| return ( | ||
| <AvatarPrimitive.Image | ||
| data-slot="avatar-image" | ||
| className={cn("aspect-square size-full", className)} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Forward ref for AvatarImage.
-function AvatarImage({
- className,
- ...props
-}: React.ComponentProps<typeof AvatarPrimitive.Image>) {
- return (
- <AvatarPrimitive.Image
- data-slot="avatar-image"
- className={cn("aspect-square size-full", className)}
- {...props}
- />
- )
-}
+const AvatarImage = React.forwardRef<
+ React.ElementRef<typeof AvatarPrimitive.Image>,
+ React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Image>
+>(({ className, ...props }, ref) => (
+ <AvatarPrimitive.Image
+ ref={ref}
+ data-slot="avatar-image"
+ className={cn("aspect-square size-full", className)}
+ {...props}
+ />
+));
+AvatarImage.displayName = "AvatarImage";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function AvatarImage({ | |
| className, | |
| ...props | |
| }: React.ComponentProps<typeof AvatarPrimitive.Image>) { | |
| return ( | |
| <AvatarPrimitive.Image | |
| data-slot="avatar-image" | |
| className={cn("aspect-square size-full", className)} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| const AvatarImage = React.forwardRef< | |
| React.ElementRef<typeof AvatarPrimitive.Image>, | |
| React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Image> | |
| >(({ className, ...props }, ref) => ( | |
| <AvatarPrimitive.Image | |
| ref={ref} | |
| data-slot="avatar-image" | |
| className={cn("aspect-square size-full", className)} | |
| {...props} | |
| /> | |
| )); | |
| AvatarImage.displayName = "AvatarImage"; |
🤖 Prompt for AI Agents
In src/components/ui/avatar.tsx around lines 22 to 33, the AvatarImage component
is declared as a plain function and doesn't forward refs; change it to use
React.forwardRef so callers can get a ref to the underlying
AvatarPrimitive.Image, update the props/ref typing to match
AvatarPrimitive.Image (e.g., forwardRef<HTMLImageElement,
React.ComponentProps<typeof AvatarPrimitive.Image>>), pass the ref through to
<AvatarPrimitive.Image ref={ref} ... /> and set a displayName for the forwarded
component for better debugging.
| function AvatarFallback({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof AvatarPrimitive.Fallback>) { | ||
| return ( | ||
| <AvatarPrimitive.Fallback | ||
| data-slot="avatar-fallback" | ||
| className={cn( | ||
| "bg-muted flex size-full items-center justify-center rounded-full", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Forward ref for AvatarFallback.
-function AvatarFallback({
- className,
- ...props
-}: React.ComponentProps<typeof AvatarPrimitive.Fallback>) {
- return (
- <AvatarPrimitive.Fallback
- data-slot="avatar-fallback"
- className={cn(
- "bg-muted flex size-full items-center justify-center rounded-full",
- className
- )}
- {...props}
- />
- )
-}
+const AvatarFallback = React.forwardRef<
+ React.ElementRef<typeof AvatarPrimitive.Fallback>,
+ React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Fallback>
+>(({ className, ...props }, ref) => (
+ <AvatarPrimitive.Fallback
+ ref={ref}
+ data-slot="avatar-fallback"
+ className={cn("bg-muted flex size-full items-center justify-center rounded-full", className)}
+ {...props}
+ />
+));
+AvatarFallback.displayName = "AvatarFallback";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function AvatarFallback({ | |
| className, | |
| ...props | |
| }: React.ComponentProps<typeof AvatarPrimitive.Fallback>) { | |
| return ( | |
| <AvatarPrimitive.Fallback | |
| data-slot="avatar-fallback" | |
| className={cn( | |
| "bg-muted flex size-full items-center justify-center rounded-full", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| ) | |
| } | |
| const AvatarFallback = React.forwardRef< | |
| React.ElementRef<typeof AvatarPrimitive.Fallback>, | |
| React.ComponentPropsWithoutRef<typeof AvatarPrimitive.Fallback> | |
| >(({ className, ...props }, ref) => ( | |
| <AvatarPrimitive.Fallback | |
| ref={ref} | |
| data-slot="avatar-fallback" | |
| className={cn( | |
| "bg-muted flex size-full items-center justify-center rounded-full", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| )); | |
| AvatarFallback.displayName = "AvatarFallback"; |
🤖 Prompt for AI Agents
In src/components/ui/avatar.tsx around lines 35 to 49, the AvatarFallback
component is a plain functional component and should accept and forward a ref to
the underlying AvatarPrimitive.Fallback; change it to use React.forwardRef with
the correct typing (React.ComponentProps<typeof AvatarPrimitive.Fallback> and
HTML element or appropriate element type) and forward the ref to the
AvatarPrimitive.Fallback element via the ref prop while preserving props and
className composition.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/components/AppContent.tsx (5)
6-6: Align component name with filename for clarity/devtoolsRename to match AppContent.tsx.
-export default function LoggedIn() { +export default function AppContent() {
20-21: Remove empty className and add a landmark labelAvoid empty attrs; add aria-label for a11y.
- <nav className=""> + <nav aria-label="Primary">
31-38: Improve avatar a11y and fallback initialsUse a meaningful alt, lazy-load the image, and show both initials.
- <AvatarImage - src={user.picture} - alt="user profile avatar" - /> + <AvatarImage + src={user.picture} + alt={[user?.givenName, user?.familyName].filter(Boolean).join(" ") || "User avatar"} + loading="lazy" + />- <AvatarFallback className="bg-gray-600 text-white font-semibold"> - {user?.givenName?.[0] || user?.familyName?.[0] || "U"} - </AvatarFallback> + <AvatarFallback className="bg-gray-600 text-white font-semibold"> + {(((user?.givenName?.[0] || "") + (user?.familyName?.[0] || "")) || "U").toUpperCase()} + </AvatarFallback>
9-15: Harden logout: type-safe error and double-click guardType the caught error and disable the button while logging out.
const handleLogout = async () => { - try { - await logout(); - } catch (error) { - console.error("Logout failed:", error); - } + setIsLoggingOut(true); + try { + await logout(); + } catch (error: unknown) { + console.error("Logout failed:", error); + } finally { + setIsLoggingOut(false); + } };- <Button onClick={handleLogout}>Sign out</Button> + <Button onClick={handleLogout} disabled={isLoggingOut} aria-busy={isLoggingOut}> + {isLoggingOut ? "Signing out..." : "Sign out"} + </Button>Additions required outside the shown ranges:
// at top-level imports import { useState } from "react"; // after extracting { user, logout } const [isLoggingOut, setIsLoggingOut] = useState(false);Also applies to: 53-54
23-25: Replace placeholder brand with env-backed valueAvoid hardcoding app name.
- <div className="text-xl font-semibold text-gray-800"> - App name - </div> + <div className="text-xl font-semibold text-gray-800"> + {import.meta.env.VITE_APP_NAME ?? "App name"} + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/components/AppContent.tsx(1 hunks)src/components/LandingPage.tsx(1 hunks)src/components/ui/input.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/LandingPage.tsx
- src/components/ui/input.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/AppContent.tsx (2)
src/components/LoggedIn.tsx (1)
LoggedIn(4-75)src/App.tsx (1)
App(5-11)
🔇 Additional comments (2)
src/components/AppContent.tsx (2)
17-66: LGTM overallClean composition, good use of Kinde hooks, and sensible UI primitives.
50-53: No changes needed:uncontainedvariant exists
Theuncontainedvariant is already defined in Button’s variants map (see button.tsx line 26), so usingvariant="uncontained"is valid.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main.tsx (1)
7-16: Guard required config; avoid passing undefined; safe defaults for URIs.Ensure clientId/domain/redirectUri exist; default logout/redirect to window.location.origin. If missing, avoid mounting the provider to prevent runtime misconfig. Verify that is safe when auth is disabled (App currently calls useKindeAuth in src/App.tsx lines 4-10).
-import { KindeProvider } from "@kinde-oss/kinde-auth-react"; +import { KindeProvider } from "@kinde-oss/kinde-auth-react"; const AppWrapper = () => { - return ( - <KindeProvider - clientId={import.meta.env.VITE_KINDE_CLIENT_ID} - domain={import.meta.env.VITE_KINDE_DOMAIN} - logoutUri={import.meta.env.VITE_KINDE_LOGOUT_URL} - redirectUri={import.meta.env.VITE_KINDE_REDIRECT_URL} - // When running local against a custom domain, include the line below - useInsecureForRefreshToken={INSECURE_REFRESH} - > - <App /> - </KindeProvider> - ); + const CLIENT_ID = import.meta.env.VITE_KINDE_CLIENT_ID; + const DOMAIN = import.meta.env.VITE_KINDE_DOMAIN; + const REDIRECT_URL = import.meta.env.VITE_KINDE_REDIRECT_URL ?? window.location.origin; + const LOGOUT_URL = import.meta.env.VITE_KINDE_LOGOUT_URL ?? window.location.origin; + + if (!CLIENT_ID || !DOMAIN || !REDIRECT_URL) { + console.error( + "[auth] Missing Kinde config. Set VITE_KINDE_CLIENT_ID, VITE_KINDE_DOMAIN, VITE_KINDE_REDIRECT_URL." + ); + return <App />; // Ensure App does not call useKindeAuth when auth is disabled. + } + return ( + <KindeProvider + clientId={CLIENT_ID} + domain={DOMAIN} + logoutUri={LOGOUT_URL} + redirectUri={REDIRECT_URL} + useInsecureForRefreshToken={INSECURE_REFRESH} + > + <App /> + </KindeProvider> + ); } -createRoot(document.getElementById("root")!).render( - <KindeProvider ...> - <App /> - </KindeProvider> -); +createRoot(document.getElementById("root")!).render(<AppWrapper />);
🧹 Nitpick comments (2)
src/main.tsx (2)
7-16: Align with PR’s config/toggle (AUTH_CONFIG.ENABLED) instead of raw env usage.This file bypasses the new src/config/auth.ts and VITE_ENABLE_AUTH toggle mentioned in the PR; unify to avoid drift.
-import { KindeProvider } from "@kinde-oss/kinde-auth-react"; +import { KindeProvider } from "@kinde-oss/kinde-auth-react"; +import { AUTH_CONFIG } from "./config/auth.ts"; const AppWrapper = () => { - const CLIENT_ID = import.meta.env.VITE_KINDE_CLIENT_ID; - const DOMAIN = import.meta.env.VITE_KINDE_DOMAIN; - const REDIRECT_URL = import.meta.env.VITE_KINDE_REDIRECT_URL ?? window.location.origin; - const LOGOUT_URL = import.meta.env.VITE_KINDE_LOGOUT_URL ?? window.location.origin; + const { ENABLED, KINDE } = AUTH_CONFIG; + const { CLIENT_ID, DOMAIN, REDIRECT_URL, LOGOUT_URL } = KINDE; - if (!CLIENT_ID || !DOMAIN || !REDIRECT_URL) { + if (!ENABLED || !CLIENT_ID || !DOMAIN || !REDIRECT_URL) { console.error( - "[auth] Missing Kinde config. Set VITE_KINDE_CLIENT_ID, VITE_KINDE_DOMAIN, VITE_KINDE_REDIRECT_URL." + "[auth] Auth disabled or incomplete config; rendering app without provider." ); return <App />; } return ( <KindeProvider clientId={CLIENT_ID} domain={DOMAIN} logoutUri={LOGOUT_URL} redirectUri={REDIRECT_URL} useInsecureForRefreshToken={INSECURE_REFRESH} > <App /> </KindeProvider> ); }
6-17: Optional: wrap in React.StrictMode for dev-only checks.+import { StrictMode } from "react"; -createRoot(document.getElementById("root")!).render(<AppWrapper />); +createRoot(document.getElementById("root")!).render( + <StrictMode> + <AppWrapper /> + </StrictMode> +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main.tsx (1)
src/App.tsx (1)
App(5-11)
| redirectUri={import.meta.env.VITE_KINDE_REDIRECT_URL} | ||
| // When running local against a custom domain, include the line below | ||
| // useInsecureForRefreshToken={true} | ||
| useInsecureForRefreshToken={true} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not commit insecure refresh tokens; gate to dev-only and make opt-in.
Hardcoding useInsecureForRefreshToken={true} weakens token security in production.
Apply:
+// Dev-only, explicit opt-in for custom domain local testing
+const INSECURE_REFRESH =
+ import.meta.env.DEV && import.meta.env.VITE_KINDE_USE_INSECURE_REFRESH === "true";
+if (!import.meta.env.DEV && import.meta.env.VITE_KINDE_USE_INSECURE_REFRESH === "true") {
+ console.warn("[auth] Ignoring VITE_KINDE_USE_INSECURE_REFRESH outside development.");
+}
+
createRoot(document.getElementById("root")!).render(
<KindeProvider
clientId={import.meta.env.VITE_KINDE_CLIENT_ID}
domain={import.meta.env.VITE_KINDE_DOMAIN}
logoutUri={import.meta.env.VITE_KINDE_LOGOUT_URL}
redirectUri={import.meta.env.VITE_KINDE_REDIRECT_URL}
// When running local against a custom domain, include the line below
- useInsecureForRefreshToken={true}
+ useInsecureForRefreshToken={INSECURE_REFRESH}
>
<App />
</KindeProvider>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useInsecureForRefreshToken={true} | |
| // Dev-only, explicit opt-in for custom domain local testing | |
| const INSECURE_REFRESH = | |
| import.meta.env.DEV && import.meta.env.VITE_KINDE_USE_INSECURE_REFRESH === "true"; | |
| if (!import.meta.env.DEV && import.meta.env.VITE_KINDE_USE_INSECURE_REFRESH === "true") { | |
| console.warn( | |
| "[auth] Ignoring VITE_KINDE_USE_INSECURE_REFRESH outside development." | |
| ); | |
| } | |
| createRoot(document.getElementById("root")!).render( | |
| <KindeProvider | |
| clientId={import.meta.env.VITE_KINDE_CLIENT_ID} | |
| domain={import.meta.env.VITE_KINDE_DOMAIN} | |
| logoutUri={import.meta.env.VITE_KINDE_LOGOUT_URL} | |
| redirectUri={import.meta.env.VITE_KINDE_REDIRECT_URL} | |
| // When running local against a custom domain, include the line below | |
| useInsecureForRefreshToken={INSECURE_REFRESH} | |
| > | |
| <App /> | |
| </KindeProvider> | |
| ); |
🤖 Prompt for AI Agents
In src/main.tsx around line 13, the prop useInsecureForRefreshToken is hardcoded
true; change it so insecure refresh tokens are disabled by default and only
enabled in non-production when explicitly opted-in. Replace the literal true
with a boolean expression that checks NODE_ENV === 'development' and an explicit
opt-in environment flag (for example process.env.ENABLE_INSECURE_REFRESH ===
'true'), defaulting to false; ensure the code path cannot enable insecure
refreshes in production, add a brief comment documenting the env var, and update
any README/tests to reflect the opt-in behavior.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
package.json (2)
18-18: Remove @types/react-router-dom — RRD v7 includes its own typesPrevents conflicting/incorrect typings.
"dependencies": { "@kinde-oss/kinde-auth-react": "^5.6.0", "@radix-ui/react-avatar": "^1.1.10", "@radix-ui/react-label": "^2.1.7", "@radix-ui/react-slot": "^1.2.3", "@tailwindcss/vite": "^4.1.12", - "@types/react-router-dom": "^5.3.3", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "lucide-react": "^0.542.0", "react": "^19.1.0",
8-8: Reintroduce type-checking in the build pipelineDropping TS checks hides errors (several exist in this PR). Fail builds on type errors.
"scripts": { "start": "vite", "dev": "vite", - "build": "vite build", + "typecheck": "tsc -b --noEmit --pretty false", + "build": "npm run typecheck && vite build", "lint": "eslint .", "preview": "vite preview" },src/components/ui/button.tsx (1)
34-53: Fix className drop and default type; improve asChild-disabled semantics
- Passing
classNameintocvadrops consumer classes.- Default button type should be “button” to avoid accidental form submits.
- Add
aria-disabledforasChild.-function Button({ - className, - variant, - size, - asChild = false, - ...props -}: React.ComponentProps<"button"> & - VariantProps<typeof buttonVariants> & { - asChild?: boolean; - }) { - const Comp = asChild ? Slot : "button"; - - return ( - <Comp - data-slot="button" - className={cn(buttonVariants({ variant, size, className }))} - {...props} - /> - ); -} +const Button = React.forwardRef< + HTMLButtonElement, + React.ComponentProps<"button"> & VariantProps<typeof buttonVariants> & { asChild?: boolean } +>(({ className, variant, size, asChild = false, ...props }, ref) => { + const Comp = asChild ? Slot : "button"; + const isDisabled = Boolean((props as any).disabled); + return ( + <Comp + ref={ref as any} + data-slot="button" + className={cn(buttonVariants({ variant, size }), className)} + aria-disabled={asChild ? (isDisabled || undefined) : undefined} + type={!asChild ? ((props as React.ComponentProps<"button">).type ?? "button") : undefined} + {...props} + /> + ); +}); +Button.displayName = "Button";src/index.css (1)
164-196: Nice: duplicate dark-mode strategy removedPrevious duplication (.dark plus @media prefers) appears resolved; single strategy improves maintainability.
🧹 Nitpick comments (11)
src/components/ui/checkbox.tsx (2)
15-18: Fix invalid Tailwind important syntax in disabled border class
disabled:border-border-form-control-disabled!won’t work. Usedisabled:!border-….- "disabled:bg-background-form-control-disabled disabled:data-[state=checked]:bg-background-form-control-disabled-checked disabled:border-border-form-control-disabled!", + "disabled:bg-background-form-control-disabled disabled:data-[state=checked]:bg-background-form-control-disabled-checked disabled:!border-border-form-control-disabled",
7-29: Forward ref for focus/imperative usage; no behavior changeKeeps API consistent with other Radix-wrapped components and enables
.focus().-function Checkbox({ - className, - ...props -}: React.ComponentProps<typeof CheckboxPrimitive.Root>) { +const Checkbox = React.forwardRef< + React.ElementRef<typeof CheckboxPrimitive.Root>, + React.ComponentPropsWithoutRef<typeof CheckboxPrimitive.Root> +>(({ className, ...props }, ref) => { return ( <CheckboxPrimitive.Root + ref={ref} data-slot="checkbox" className={cn( "peer cursor-pointer border-border-form-control-default hover:border-border-form-control-hover dark:bg-input/30 data-[state=checked]:bg-primary data-[state=checked]:text-primary-foreground dark:data-[state=checked]:bg-primary data-[state=checked]:border-primary focus-visible:ring-[2px] focus-visible:ring-button-ring focus-visible:ring-offset-2 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive size-6 shrink-0 rounded-[4px] border transition-shadow outline-none disabled:cursor-not-allowed", "disabled:bg-background-form-control-disabled disabled:data-[state=checked]:bg-background-form-control-disabled-checked disabled:!border-border-form-control-disabled", className )} {...props} > <CheckboxPrimitive.Indicator data-slot="checkbox-indicator" className="flex items-center justify-center text-current transition-none" > <CheckIcon className="size-4" /> </CheckboxPrimitive.Indicator> </CheckboxPrimitive.Root> ); -} +}); +Checkbox.displayName = "Checkbox";src/components/ui/spinner.tsx (2)
34-35: className is being dropped; pass it outside cva resolver
cvaignores unknown keys;classNamemust be merged separately.- className={cn(spinnerVariants({ variant, size, className }))} + className={cn(spinnerVariants({ variant, size }), className)}
31-31: Minor a11y: announce politely and mark busyImprove SR feedback without visual changes.
- <div role="status"> + <div role="status" aria-live="polite" aria-busy="true">src/components/ui/radio-group.tsx (2)
7-18: Forward ref for Root to match Radix ergonomicsEnables refs for focus mgmt and parity with other primitives.
-function RadioGroup({ - className, - ...props -}: React.ComponentProps<typeof RadioGroupPrimitive.Root>) { - return ( - <RadioGroupPrimitive.Root - data-slot="radio-group" - className={cn("grid gap-3", className)} - {...props} - /> - ); -} +const RadioGroup = React.forwardRef< + React.ElementRef<typeof RadioGroupPrimitive.Root>, + React.ComponentPropsWithoutRef<typeof RadioGroupPrimitive.Root> +>(({ className, ...props }, ref) => ( + <RadioGroupPrimitive.Root + ref={ref} + data-slot="radio-group" + className={cn("grid gap-3", className)} + {...props} + /> +)); +RadioGroup.displayName = "RadioGroup";
20-44: Forward ref for Item; keep current styles as-isSmall ergonomics improvement; no behavior change.
-function RadioGroupItem({ - className, - ...props -}: React.ComponentProps<typeof RadioGroupPrimitive.Item>) { - return ( - <RadioGroupPrimitive.Item +const RadioGroupItem = React.forwardRef< + React.ElementRef<typeof RadioGroupPrimitive.Item>, + React.ComponentPropsWithoutRef<typeof RadioGroupPrimitive.Item> +>(({ className, ...props }, ref) => ( + <RadioGroupPrimitive.Item + ref={ref} data-slot="radio-group-item" className={cn( "group border-border-form-control-default text-primary focus-visible:border-ring focus-visible:ring-ring focus-visible:ring-2 focus-visible:ring-offset-2 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive dark:bg-input/30 aspect-square size-6 shrink-0 rounded-full border shadow-xs transition-[color,box-shadow] outline-none disabled:cursor-not-allowed", "hover:border-border-form-control-hover", "data-[state=checked]:border-2 data-[state=checked]:border-primary", "disabled:bg-background-form-control-disabled disabled:border-border-form-control-disabled disabled:data-[state=checked]:border-border-form-control-disabled disabled:data-[state=checked]:text-text-form-control-disabled-checked", className )} {...props} > <RadioGroupPrimitive.Indicator data-slot="radio-group-indicator" className="relative flex items-center justify-center" > <CircleIcon className="fill-primary group-disabled:fill-text-form-control-disabled-checked absolute top-1/2 left-1/2 size-3 -translate-x-1/2 -translate-y-1/2" /> </RadioGroupPrimitive.Indicator> </RadioGroupPrimitive.Item> - ); -} +)); +RadioGroupItem.displayName = "RadioGroupItem";src/components/ui/button.tsx (1)
8-8: Trim “transition-all”; prefer “transition-colors”Reduces unnecessary animated properties; avoids layout/box-shadow jank.
- "cursor-pointer inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md font-medium transition-all disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:border-ring focus-visible:ring-button-ring focus-visible:ring-[2px] focus-visible:ring-offset-2 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive", + "cursor-pointer inline-flex items-center justify-center gap-2 whitespace-nowrap rounded-md font-medium transition-colors disabled:pointer-events-none disabled:opacity-50 [&_svg]:pointer-events-none [&_svg:not([class*='size-'])]:size-4 shrink-0 [&_svg]:shrink-0 outline-none focus-visible:border-ring focus-visible:ring-button-ring focus-visible:ring-[2px] focus-visible:ring-offset-2 aria-invalid:ring-destructive/20 dark:aria-invalid:ring-destructive/40 aria-invalid:border-destructive",src/components/ui/table.tsx (1)
; pass children explicitly for clarity
5-16: Avoid self-closingJSX will accept
childrenvia props, but an explicit pair is clearer and avoids accidental shadowing.-function Table({ className, ...props }: React.ComponentProps<"table">) { +function Table({ + className, + children, + ...props +}: React.PropsWithChildren<React.ComponentProps<"table">>) { return ( <div data-slot="table-container" className="relative w-full overflow-x-auto" > - <table + <table data-slot="table" className={cn("w-full caption-bottom", className)} {...props} - /> + > + {children} + </table> </div> ); }src/index.css (3)
198-201: Confirm “outline-ring/50” compiles as intendedThis utility may not resolve unless your Tailwind mapping exposes “ring” as a color. If it doesn’t, set outline-color directly using an arbitrary value.
* { - @apply border-border outline-ring/50; + @apply border-border; + outline-color: color-mix(in oklab, var(--ring) 50%, transparent); }
98-129: Standardize color space for palette tokensConsider converting the rgba palette to OKLCH to match the rest of the theme for consistency and perceptual tuning.
130-162: Expose color-scheme for UA widgetsHelps native form controls and scrollbars match theme automatically.
:root { + color-scheme: light; --radius: 0.625rem; ... } .dark { + color-scheme: dark; --background: oklch(0.145 0 0); ... }Also applies to: 164-196
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml📒 Files selected for processing (10)
package.json(1 hunks)src/components/ui/badge.tsx(1 hunks)src/components/ui/button.tsx(1 hunks)src/components/ui/checkbox.tsx(1 hunks)src/components/ui/input.tsx(1 hunks)src/components/ui/label.tsx(1 hunks)src/components/ui/radio-group.tsx(1 hunks)src/components/ui/spinner.tsx(1 hunks)src/components/ui/table.tsx(1 hunks)src/index.css(1 hunks)🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ui/input.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ui/button.tsx (2)
lib/utils.ts (1)
cn(4-6)src/lib/utils.ts (1)
cn(4-6)🔇 Additional comments (3)
src/components/ui/badge.tsx (1)
30-46: LGTM — solid variant system and Slot supportAPI and class merging look good; consistent with the rest of the UI kit.
src/components/ui/table.tsx (1)
66-85: Verify Tailwind custom height token "h-15" is definedNo repository evidence for "h-15" in the provided search output — it’s not a standard Tailwind utility. Confirm "h-15" is added in tailwind.config.js (theme.extend.height) or defined in your CSS, or replace with a standard/arb utility.
File: src/components/ui/table.tsx (lines 66–85)
src/index.css (1)
5-5: Verify Tailwind 4 “@custom-variant” support in your toolchainEnsure your Tailwind version/plugins support @custom-variant dark (&:is(.dark *)); or fall back to the built‑in dark mode config.
| function Label({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof LabelPrimitive.Root>) { | ||
| return ( | ||
| <LabelPrimitive.Root | ||
| data-slot="label" | ||
| className={cn( | ||
| "flex items-center gap-2 text-primary text-base leading-none font-medium select-none group-data-[disabled=true]:pointer-events-none group-data-[disabled=true]:opacity-50 peer-disabled:cursor-not-allowed ", | ||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Forward ref and simplify disabled styling; current group-data variant likely never applies
Forwarding the ref aligns with Radix patterns and enables programmatic focus. Also, the group-data-[disabled=true]:… classes require a group parent; keep peer-disabled:* and drop the unused group selector.
-function Label({
- className,
- ...props
-}: React.ComponentProps<typeof LabelPrimitive.Root>) {
- return (
- <LabelPrimitive.Root
- data-slot="label"
- className={cn(
- "flex items-center gap-2 text-primary text-base leading-none font-medium select-none group-data-[disabled=true]:pointer-events-none group-data-[disabled=true]:opacity-50 peer-disabled:cursor-not-allowed ",
- className
- )}
- {...props}
- />
- );
-}
+const Label = React.forwardRef<
+ React.ElementRef<typeof LabelPrimitive.Root>,
+ React.ComponentPropsWithoutRef<typeof LabelPrimitive.Root>
+>(({ className, ...props }, ref) => (
+ <LabelPrimitive.Root
+ ref={ref}
+ data-slot="label"
+ className={cn(
+ "flex items-center gap-2 text-primary text-base leading-none font-medium select-none peer-disabled:cursor-not-allowed peer-disabled:opacity-50",
+ className
+ )}
+ {...props}
+ />
+));
+Label.displayName = LabelPrimitive.Root.displayName ?? "Label";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function Label({ | |
| className, | |
| ...props | |
| }: React.ComponentProps<typeof LabelPrimitive.Root>) { | |
| return ( | |
| <LabelPrimitive.Root | |
| data-slot="label" | |
| className={cn( | |
| "flex items-center gap-2 text-primary text-base leading-none font-medium select-none group-data-[disabled=true]:pointer-events-none group-data-[disabled=true]:opacity-50 peer-disabled:cursor-not-allowed ", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| ); | |
| } | |
| const Label = React.forwardRef< | |
| React.ElementRef<typeof LabelPrimitive.Root>, | |
| React.ComponentPropsWithoutRef<typeof LabelPrimitive.Root> | |
| >(({ className, ...props }, ref) => ( | |
| <LabelPrimitive.Root | |
| ref={ref} | |
| data-slot="label" | |
| className={cn( | |
| "flex items-center gap-2 text-primary text-base leading-none font-medium select-none peer-disabled:cursor-not-allowed peer-disabled:opacity-50", | |
| className | |
| )} | |
| {...props} | |
| /> | |
| )); | |
| Label.displayName = LabelPrimitive.Root.displayName ?? "Label"; |
🤖 Prompt for AI Agents
In src/components/ui/label.tsx around lines 6 to 20, forward the ref to
LabelPrimitive.Root and simplify disabled styling by removing the
group-data-[disabled=true]:* classnames (which only apply when a parent has
class "group") and keep the peer-disabled:* variants; update the component
signature to accept a ref (use React.forwardRef) and pass that ref to
LabelPrimitive.Root, merge className as before, and ensure props typing remains
React.ComponentProps<typeof LabelPrimitive.Root> while preserving existing class
order.
| --color-background-neutral: var(--color-white); | ||
| --color-background-informational: var(--color-blue-100); |
There was a problem hiding this comment.
Undefined CSS variable --color-white breaks multiple tokens
Several tokens use var(--color-white) but it’s never defined. Define it in :root to avoid invalid computed values.
:root {
+ --color-white: rgba(255, 255, 255, 1);
--color-grey-50: rgba(245, 245, 245, 1);
--color-grey-100: rgba(235, 235, 235, 1);Also applies to: 131-136, 158-159
🤖 Prompt for AI Agents
In src/index.css around lines 35-36 (and also affecting lines 131-136 and
158-159), multiple tokens reference var(--color-white) but that variable is not
defined; add a --color-white declaration in the :root selector (e.g. set to
#ffffff or your chosen white value) so var(--color-white) resolves to a valid
color and prevents invalid computed values; update :root to include this
variable and ensure any theme variants override it if needed.
| --color-destructive: var(--destructive); | ||
| --color-border: var(--border); | ||
| --color-input: var(--input); | ||
| --color-ring: var(--ring); |
There was a problem hiding this comment.
Missing mapping: --color-destructive-foreground
You map --color-destructive but not its foreground counterpart, which many component styles expect.
--color-destructive: var(--destructive);
+ --color-destructive-foreground: var(--destructive-foreground);
--color-border: var(--border);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --color-destructive: var(--destructive); | |
| --color-border: var(--border); | |
| --color-input: var(--input); | |
| --color-ring: var(--ring); | |
| --color-destructive: var(--destructive); | |
| --color-destructive-foreground: var(--destructive-foreground); | |
| --color-border: var(--border); | |
| --color-input: var(--input); | |
| --color-ring: var(--ring); |
🤖 Prompt for AI Agents
In src/index.css around lines 79 to 82, you declare --color-destructive but not
its expected foreground counterpart; add a mapping for
--color-destructive-foreground (e.g., set --color-destructive-foreground:
var(--destructive-foreground); or provide a sensible fallback like
var(--destructive-foreground, #fff)) alongside the existing color variables so
component styles that reference the destructive foreground resolve correctly.
| --muted-foreground: oklch(0.556 0 0); | ||
| --accent: var(--color-grey-50); | ||
| --accent-foreground: oklch(0.205 0 0); | ||
| --destructive: oklch(0.577 0.245 27.325); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add base tokens for destructive foreground in light/dark
Without these, text/icons on destructive surfaces may be unreadable.
:root {
...
--destructive: oklch(0.577 0.245 27.325);
+ --destructive-foreground: oklch(0.985 0 0);
--border: var(--color-grey-100); .dark {
...
--destructive: oklch(0.704 0.191 22.216);
+ --destructive-foreground: oklch(0.205 0 0);
--border: oklch(1 0 0 / 10%);Also applies to: 171-171
🤖 Prompt for AI Agents
In src/index.css around lines 145 and 171, the stylesheet defines a destructive
background color but does not define matching destructive foreground tokens for
light and dark themes, which can make text/icons unreadable; add base foreground
token(s) for destructive (e.g. --destructive-foreground for light and
--destructive-foreground-dark or a theme-specific override for dark mode) next
to the existing destructive background token(s), choosing accessible color
values (ensure sufficient contrast with the destructive background using your
contrast target) and place them in the same theme blocks so text/icons on
destructive surfaces use these tokens.
2 participants
…arter-kit into peter/make-starter# Explain your changes
Suppose there is a related issue with enough detail for a reviewer to understand your changes fully. In that case, you can omit an explanation and instead include either “Fixes #XX” or “Updates #XX” where “XX” is the issue number.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.
Summary by CodeRabbit
New Features
Documentation
Style
Chores