Conversation
📝 WalkthroughWalkthroughThis PR adds a persistent light/dark theme (tokens, global CSS, and a Zustand store) and applies theme-driven colors across screens and UI components, plus a root AppThemeBoundary and a profile theme selector. ChangesDark/Light Theme System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/auth-screen.tsx (1)
1-650:⚠️ Potential issue | 🟠 MajorBlock merge:
npm run lint/npm run typecheckdo not pass
npm run lintfails (44 errors), includingcomponents/auth-screen.tsxunresolved imports like@clerk/expo,expo-linking,expo-router,expo-web-browser,react-native-safe-area-context, andposthog-react-native.npm run typecheckfails forcomponents/auth-screen.tsxtoo (missing module type declarations +implicit any/appThemeColorsindexing issues), plus many similar failures across other files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/auth-screen.tsx` around lines 1 - 650, The build is failing due to missing type declarations for several third‑party imports and unsafe indexing/implicit-any usage in this file; add proper types (install `@types` packages where available or add ambient .d.ts module declarations for `@clerk/expo`, expo-linking, expo-router, expo-web-browser, react-native-safe-area-context, posthog-react-native) and then tighten the local types: ensure the theme variable returned by useThemeStore is a typed union and index appThemeColors with that union (or cast as keyof typeof appThemeColors) wherever used (e.g., in AuthScreen and VerificationModal), and import/annotate Clerk/SSO/SignIn/SignUp return types for signIn, signUp, startSSOFlow and usePostHog so functions like handleEmailAuth, handleVerifyCode and handleSocialAuth no longer rely on implicit any types.
🧹 Nitpick comments (2)
store/use-theme-store.ts (1)
44-61: ⚡ Quick winSimplify redundant hydration tracking.
The
hasHydratedflag is being set in three separate places:
onRehydrateStoragecallback (lines 44-46)persist.onFinishHydrationcallback (lines 55-57)- Immediate synchronous check (lines 59-61)
This creates unnecessary duplication. The
persist.onFinishHydrationcallback should be sufficient to track when hydration completes.Simplified hydration tracking
persist( (set) => ({ hasHydrated: false, setHasHydrated: (hasHydrated) => set({ hasHydrated }), setTheme: (theme) => set({ theme }), theme: "light", toggleTheme: () => set((state) => ({ theme: state.theme === "light" ? "dark" : "light" })), }), { name: "lingua-theme-storage", - onRehydrateStorage: () => (state) => { - state?.setHasHydrated(true); - }, partialize: (state) => ({ theme: state.theme, }), storage: createJSONStorage(getThemeStorage), }, ), ); useThemeStore.persist.onFinishHydration((state) => { state.setHasHydrated(true); }); - -if (useThemeStore.persist.hasHydrated()) { - useThemeStore.setState({ hasHydrated: true }); -}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@store/use-theme-store.ts` around lines 44 - 61, The hydration flag is being set in three places; keep only the persist.onFinishHydration handler and remove the redundant updates: delete the onRehydrateStorage callback that calls state?.setHasHydrated(true) and remove the immediate synchronous hasHydrated check that calls useThemeStore.setState({ hasHydrated: true }); retain useThemeStore.persist.onFinishHydration((state) => { state.setHasHydrated(true); }) along with the existing partialize and storage setup (e.g. createJSONStorage/getThemeStorage).app/index.tsx (1)
12-12: 💤 Low valueConsider applying theme for loading placeholder consistency.
The loading View uses
bg-lingua-backgroundbut doesn't apply thetheme-darkclass conditionally likeAppThemeBoundarydoes. While this loading state is brief and defaults to light theme, for visual consistency you could apply the theme class here as well.Optional theme consistency
+import { useThemeStore } from "`@/store/use-theme-store`"; + export default function Index() { const { isLoaded, isSignedIn } = useAuth(); const hasHydratedLanguage = useLanguageStore((state) => state.hasHydrated); const selectedLanguageId = useLanguageStore((state) => state.selectedLanguageId); + const theme = useThemeStore((state) => state.theme); + const hasHydratedTheme = useThemeStore((state) => state.hasHydrated); + const activeTheme = hasHydratedTheme ? theme : "light"; if (!isLoaded || !hasHydratedLanguage) { - return <View className="flex-1 bg-lingua-background" />; + return ( + <View + className={`flex-1 bg-lingua-background ${activeTheme === "dark" ? "theme-dark" : ""}`} + /> + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/index.tsx` at line 12, Loading placeholder lacks the conditional theme class used elsewhere; update the View returned in app/index.tsx (the loading placeholder JSX) to include the same conditional theme class logic used by AppThemeBoundary so it applies "theme-dark" when appropriate, by mirroring the theme-detection/className composition that AppThemeBoundary uses for consistent light/dark styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@constants/theme.ts`:
- Around line 14-15: The dark-theme entry uses the Chinese label "黑色" which may
be incorrect for dark mode; update the label in the theme definition where the
object contains icon: "moon" to a more appropriate term like "深色" or "暗色" (e.g.,
change label: "黑色" to label: "深色") to match typical Chinese UI terminology for
dark mode.
---
Outside diff comments:
In `@components/auth-screen.tsx`:
- Around line 1-650: The build is failing due to missing type declarations for
several third‑party imports and unsafe indexing/implicit-any usage in this file;
add proper types (install `@types` packages where available or add ambient .d.ts
module declarations for `@clerk/expo`, expo-linking, expo-router,
expo-web-browser, react-native-safe-area-context, posthog-react-native) and then
tighten the local types: ensure the theme variable returned by useThemeStore is
a typed union and index appThemeColors with that union (or cast as keyof typeof
appThemeColors) wherever used (e.g., in AuthScreen and VerificationModal), and
import/annotate Clerk/SSO/SignIn/SignUp return types for signIn, signUp,
startSSOFlow and usePostHog so functions like handleEmailAuth, handleVerifyCode
and handleSocialAuth no longer rely on implicit any types.
---
Nitpick comments:
In `@app/index.tsx`:
- Line 12: Loading placeholder lacks the conditional theme class used elsewhere;
update the View returned in app/index.tsx (the loading placeholder JSX) to
include the same conditional theme class logic used by AppThemeBoundary so it
applies "theme-dark" when appropriate, by mirroring the
theme-detection/className composition that AppThemeBoundary uses for consistent
light/dark styling.
In `@store/use-theme-store.ts`:
- Around line 44-61: The hydration flag is being set in three places; keep only
the persist.onFinishHydration handler and remove the redundant updates: delete
the onRehydrateStorage callback that calls state?.setHasHydrated(true) and
remove the immediate synchronous hasHydrated check that calls
useThemeStore.setState({ hasHydrated: true }); retain
useThemeStore.persist.onFinishHydration((state) => { state.setHasHydrated(true);
}) along with the existing partialize and storage setup (e.g.
createJSONStorage/getThemeStorage).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d386a8e-2b72-49ec-ac2f-5f01c27c91c6
📒 Files selected for processing (19)
app/(tabs)/chat.tsxapp/(tabs)/home.tsxapp/(tabs)/learn.tsxapp/(tabs)/profile.tsxapp/LanguageSelection.tsxapp/_layout.tsxapp/index.tsxapp/lesson/[lessonId].tsxapp/onboarding.tsxcomponents/audio-teacher-session.tsxcomponents/auth-screen.tsxcomponents/bottom-tab-bar.tsxcomponents/chat-input-bar.tsxcomponents/chat-message-bubble.tsxcomponents/lesson-card.tsxcomponents/tab-placeholder-screen.tsxconstants/theme.tsglobal.cssstore/use-theme-store.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/index.tsx (1)
1-33:⚠️ Potential issue | 🔴 CriticalLint/typecheck are currently failing for this branch.
npm run lintexits with code 2 due toimport/no-unresolved(e.g.,@clerk/expo,expo-router,react-native-safe-area-context,posthog-react-native).npm run typecheckfails withTS2307missing modules (same imports) and manyimplicit any/typing errors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/index.tsx` around lines 1 - 33, The lint/typecheck failures come from unresolved imports (e.g., `@clerk/expo`, expo-router, react-native-safe-area-context, posthog-react-native) used by symbols like useAuth, Redirect, View, useLanguageStore and useThemeStore; fix by either installing the appropriate packages and their type defs (npm install `@clerk/expo` expo-router react-native-safe-area-context posthog-react-native and add any `@types/`* packages if available) or, if those libs have no TS types, create a project-level ambient declaration file (e.g., globals.d.ts) that declares missing modules with lines like declare module "expo-router"; declare module "`@clerk/expo`"; declare module "posthog-react-native"; declare module "react-native-safe-area-context"; ensure this .d.ts is included by tsconfig (include or typeRoots) so the compiler and ESLint resolve the imports and the symbols useAuth/Redirect/View resolve without TS2307/import/no-unresolved errors.
🧹 Nitpick comments (1)
app/index.tsx (1)
18-18: ⚡ Quick winAvoid dynamic NativeWind class interpolation for theme state.
Line 18 builds
classNamedynamically; this conflicts with the project rule for dynamic styles. Prefer inline/StyleSheet for the theme-dependent part.Suggested change
+import { appThemeColors } from "`@/constants/theme`"; import { View } from "react-native"; @@ - <View - className={`flex-1 bg-lingua-background ${activeTheme === "dark" ? "theme-dark" : ""}`} - /> + <View + style={{ flex: 1, backgroundColor: appThemeColors[activeTheme].background }} + />As per coding guidelines, "Use StyleSheet or inline styles instead of NativeWind classes for ... dynamic styles".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/index.tsx` at line 18, The current JSX uses a template string in the className (className={`flex-1 bg-lingua-background ${activeTheme === "dark" ? "theme-dark" : ""}`}) which mixes dynamic theme state into NativeWind classes; replace that with a static className for the layout (e.g., "flex-1 bg-lingua-background") and move the theme-dependent styling into a StyleSheet or inline style applied via the style prop, using the existing activeTheme variable to select the appropriate style object (e.g., styles.darkContainer vs styles.lightContainer); update the element where className is set and add the new style lookup, and define the corresponding StyleSheet entries instead of toggling NativeWind classes dynamically.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/index.tsx`:
- Around line 1-33: The lint/typecheck failures come from unresolved imports
(e.g., `@clerk/expo`, expo-router, react-native-safe-area-context,
posthog-react-native) used by symbols like useAuth, Redirect, View,
useLanguageStore and useThemeStore; fix by either installing the appropriate
packages and their type defs (npm install `@clerk/expo` expo-router
react-native-safe-area-context posthog-react-native and add any `@types/`*
packages if available) or, if those libs have no TS types, create a
project-level ambient declaration file (e.g., globals.d.ts) that declares
missing modules with lines like declare module "expo-router"; declare module
"`@clerk/expo`"; declare module "posthog-react-native"; declare module
"react-native-safe-area-context"; ensure this .d.ts is included by tsconfig
(include or typeRoots) so the compiler and ESLint resolve the imports and the
symbols useAuth/Redirect/View resolve without TS2307/import/no-unresolved
errors.
---
Nitpick comments:
In `@app/index.tsx`:
- Line 18: The current JSX uses a template string in the className
(className={`flex-1 bg-lingua-background ${activeTheme === "dark" ? "theme-dark"
: ""}`}) which mixes dynamic theme state into NativeWind classes; replace that
with a static className for the layout (e.g., "flex-1 bg-lingua-background") and
move the theme-dependent styling into a StyleSheet or inline style applied via
the style prop, using the existing activeTheme variable to select the
appropriate style object (e.g., styles.darkContainer vs styles.lightContainer);
update the element where className is set and add the new style lookup, and
define the corresponding StyleSheet entries instead of toggling NativeWind
classes dynamically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 87e4110d-beea-449a-8bdb-b3b60074bbe4
📒 Files selected for processing (3)
app/index.tsxconstants/theme.tsstore/use-theme-store.ts
💤 Files with no reviewable changes (1)
- store/use-theme-store.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes