Open
Conversation
📝 WalkthroughWalkthroughThis PR integrates circular theme-switch animations via Changes
Sequence DiagramsequenceDiagram
participant User
participant Pressable as UI Control<br/>(Pressable)
participant Handler as Event Handler<br/>(handleModeChange/<br/>handleThemeSelect)
participant Animation as switchTheme<br/>(Animation)
participant ThemeSystem as Theme System<br/>(useTheme/MMKV)
User->>Pressable: Press theme/mode selector
Pressable->>Handler: onPress(event)
Handler->>Handler: event.currentTarget.measure(...)
Handler->>ThemeSystem: Update THEME_MODE or APP_THEME_ID
Handler->>Animation: switchTheme(startingPoint)
Animation->>Animation: Calculate circle center & radius
Animation->>User: Render circular animation
Animation->>ThemeSystem: Apply new theme
ThemeSystem->>User: Update UI with new theme
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 15
🧹 Nitpick comments (14)
src/screens/novel/components/ChapterItem.tsx (1)
96-99: MemoizereleaseTimeStylefor consistency with the component's optimization pattern.All other derived style values in this component (
selectedStyle,titleColor,releaseColor,ripple) are memoized withuseMemo. This inline object is created fresh on every render, which is inconsistent with the established pattern.♻️ Suggested fix
- const releaseTimeStyle = { - color: theme.outline, - marginStart: chapter.releaseTime ? 5 : 0, - } as const; + const releaseTimeStyle = useMemo( + () => ({ + color: theme.outline, + marginStart: chapter.releaseTime ? 5 : 0, + }), + [theme.outline, chapter.releaseTime], + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/novel/components/ChapterItem.tsx` around lines 96 - 99, The inline style object releaseTimeStyle in ChapterItem is created every render and should be memoized like the other derived values (selectedStyle, titleColor, releaseColor, ripple); replace the inline const releaseTimeStyle with a useMemo that returns the same object based on theme.outline and chapter.releaseTime so the reference is stable across renders and consistent with the component’s optimization pattern.src/screens/novel/components/Info/NovelInfoHeader.tsx (1)
184-196: Consider keeping width values inline for single-use styles.The extraction of
width: 130andwidth: 180into separate StyleSheet objects (styles.w130,styles.w180) adds indirection for values that are each used only once. Since these widths are specific to this skeleton component and unlikely to be reused elsewhere, inline styles would be simpler and more maintainable:<View style={[styles.infoSkeletonBar, { width: 130, backgroundColor }]} >If these widths are meant to become reusable across components or are related to the theme system refactor, consider using more descriptive names that indicate their purpose (e.g.,
styles.skeletonBarShort,styles.skeletonBarMedium).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/novel/components/Info/NovelInfoHeader.tsx` around lines 184 - 196, The styles.w130 and styles.w180 entries used only once in NovelInfoHeader's skeleton should be inlined to reduce indirection: replace usages of styles.w130 and styles.w180 in the View style arrays (which also include styles.infoSkeletonBar and backgroundColor) with inline width values ({ width: 130 } and { width: 180 }) or rename them to descriptive names if you intend reuse (e.g., skeletonBarShort/skeletonBarMedium); update the two View elements in NovelInfoHeader.tsx accordingly and remove the now-unused styles.w130/styles.w180 from the StyleSheet.android/app/src/debug/AndroidManifest.xml (1)
6-6:android:allowBackupin debug manifest is redundant.Line 6 duplicates the same value already declared in
android/app/src/main/AndroidManifest.xml(Line 22). Consider removing it from debug overlay to keep one source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/debug/AndroidManifest.xml` at line 6, The debug manifest contains a redundant android:allowBackup="true" attribute that duplicates the declaration in the main AndroidManifest.xml; remove the android:allowBackup attribute from the debug overlay manifest (the debug AndroidManifest.xml) and leave the single source of truth in the main AndroidManifest.xml so the app uses the main manifest's setting.android/app/src/main/java/com/rajarsheechatterjee/LNReader/MainApplication.kt (1)
10-11: Remove unused imports.
DefaultNewArchitectureEntryPoint.loadandDefaultReactHost.getDefaultReactHostare imported but never used after the migration toExpoReactHostFactory.Proposed fix
-import com.facebook.react.defaults.DefaultNewArchitectureEntryPoint.load -import com.facebook.react.defaults.DefaultReactHost.getDefaultReactHost🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/src/main/java/com/rajarsheechatterjee/LNReader/MainApplication.kt` around lines 10 - 11, Remove the unused imports DefaultNewArchitectureEntryPoint.load and DefaultReactHost.getDefaultReactHost from MainApplication.kt: locate the import lines that reference DefaultNewArchitectureEntryPoint.load and DefaultReactHost.getDefaultReactHost and delete them so only necessary imports remain after migrating to ExpoReactHostFactory.src/components/ThemePicker/ThemePicker.tsx (2)
19-19: UnusedisDarkprop in interface.The
isDarkprop is defined inThemePickerProps(Line 19) but is never destructured or used in the component. Either remove it from the interface or implement its intended functionality.Proposed fix - remove unused prop
interface ThemePickerProps { theme: ThemeColors; currentTheme: ThemeColors; onPress: (event: GestureResponderEvent) => void; horizontal?: boolean; - isDark?: boolean; }Also applies to: 22-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ThemePicker/ThemePicker.tsx` at line 19, ThemePickerProps defines an unused isDark property; remove isDark from the ThemePickerProps interface (and any related type exports) or, if intended, wire it into the ThemePicker component by destructuring isDark from props and using it to set initial state/theme selection. Specifically update the ThemePickerProps type to omit isDark and adjust any callers/prop usages accordingly, or alternatively modify the ThemePicker function to accept ({ isDark, ... }) and apply isDark to the component's state or className so the prop is actually used.
158-158: Commented-out elevation may affect Android shadows.The
elevation: 2is commented out. On Android, this was providing the shadow effect for the theme cards. The iOS-specific shadow properties (Lines 153-156) remain, but Android users will no longer see card shadows. If this is intentional, consider removing the commented line entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ThemePicker/ThemePicker.tsx` at line 158, The commented-out "elevation: 2" in ThemePicker.tsx removes Android shadows for the theme cards; locate the style object in the ThemePicker component where "//elevation: 2" appears (the card/item style for theme cards) and either uncomment and restore elevation: 2 to re-enable Android shadow rendering or remove the commented line entirely if the absence of elevation is intentional, ensuring the iOS shadow props (shadowColor, shadowOffset, shadowOpacity, shadowRadius) remain unchanged.src/screens/onboarding/ThemeSelectionStep.tsx (1)
177-196: Remove unused styles after AmoledToggle refactor.The styles
toggle,toggleThumb, andtoggleThumbActive(Lines 177-196) are no longer used after replacing the custom toggle implementation with theSwitchcomponent.Proposed fix
amoledLabel: { fontSize: 16, fontWeight: '400', }, - toggle: { - width: 52, - height: 32, - borderRadius: 16, - padding: 2, - justifyContent: 'center', - }, - toggleThumb: { - width: 28, - height: 28, - borderRadius: 14, - elevation: 2, - shadowColor: '#000', - shadowOffset: { width: 0, height: 2 }, - shadowOpacity: 0.2, - shadowRadius: 2, - }, - toggleThumbActive: { - alignSelf: 'flex-end', - }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/onboarding/ThemeSelectionStep.tsx` around lines 177 - 196, The styles object in ThemeSelectionStep still contains unused keys from the old AmoledToggle: remove the unused style entries "toggle", "toggleThumb", and "toggleThumbActive" from the exported styles object in ThemeSelectionStep.tsx (where styles are defined) and run a quick search for any remaining references to those symbols to ensure nothing else depends on them; leave the Switch-based styling intact.src/hooks/persisted/useTheme.ts (1)
76-97: Type inconsistency: Record keys are numbers but literals are strings.The maps are typed as
Record<number, number>but the keys are string literals ('1','2', etc.). While JavaScript coerces these correctly at runtime, the typing is misleading. Consider using numeric keys for consistency.Proposed fix
- const lightIdMap: Record<number, number> = { - '1': 100, - '8': 102, - '9': 108, - '10': 101, - '12': 103, - '14': 104, - '16': 105, - '18': 106, - '20': 107, - }; - const darkIdMap: Record<number, number> = { - '2': 100, - '9': 102, - '10': 108, - '11': 101, - '13': 103, - '15': 104, - '17': 105, - '19': 106, - '21': 107, - }; + const lightIdMap: Record<number, number> = { + 1: 100, + 8: 102, + 9: 108, + 10: 101, + 12: 103, + 14: 104, + 16: 105, + 18: 106, + 20: 107, + }; + const darkIdMap: Record<number, number> = { + 2: 100, + 9: 102, + 10: 108, + 11: 101, + 13: 103, + 15: 104, + 17: 105, + 19: 106, + 21: 107, + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/persisted/useTheme.ts` around lines 76 - 97, The lightIdMap and darkIdMap objects are declared as Record<number, number> but use string key literals; change the keys from quoted strings to numeric literals (e.g., '1': 100 -> 1: 100) so the object shapes match the Record<number, number> type, leaving the map names (lightIdMap, darkIdMap) and values unchanged.src/theme/md3/index.ts (1)
11-17: Position-based theme IDs are fragile—new themes lack protection against reordering.The auto-assignment via
.map((theme, i) => ({ ...theme, id: 100 + i }))is true and the warning that themes must be appended is correct. However, the system provides no enforcement. If a theme is inserted in the middle rather than at the end, all subsequent IDs shift and break user preferences for new themes (those added after the legacytransformThemeIdmapping was created).Legacy themes are protected by the
transformThemeIdmapping inuseTheme.ts, but new themes are not. Add a unit test that asserts theme count/order hasn't changed unexpectedly, or use explicit IDs per theme instead of relying on array position.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/theme/md3/index.ts` around lines 11 - 17, The current ID assignment in md3 index uses position-based mapping (.map((theme, i) => ({ ...theme, id: 100 + i }))) which is fragile; change to explicit, stable IDs by adding an id property to each theme object in the exported theme list and update the mapping logic in src/theme/md3/index.ts to use theme.id when present (fall back to 100+i only for legacy entries). Also add a unit test asserting the exported theme list length and that each theme has a numeric id and that the sequence of ids for new themes matches the expected explicit list to prevent accidental reordering regressions; reference transformThemeId in useTheme.ts to ensure legacy mappings remain compatible.src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx (4)
34-37: Type assertion masks potential runtime issues.Casting
useMMKVString('THEME_MODE')to[ThemeMode, (mode: ThemeMode) => void]hides that:
- The stored value could be any string (not just valid
ThemeModevalues)- The original setter accepts
undefinedfor deletionConsider adding runtime validation or using a type guard to ensure the stored value is actually a valid
ThemeMode.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 34 - 37, The current type assertion on useMMKVString('THEME_MODE') hides invalid runtime values and that the setter can accept undefined; replace the cast with runtime validation and a proper type guard: read the raw string from useMMKVString into a variable (e.g., rawTheme), validate it against the ThemeMode union (create an isThemeMode(value): value is ThemeMode function), set themeMode to a safe default ('system') when invalid, and wrap or narrow the setter so callers use (mode: ThemeMode | undefined) to allow deletion while preserving correct typings for setThemeMode and preventing invalid string values from being treated as ThemeMode.
131-152: Remove commented-out code before merging.Large blocks of commented-out code (old handler implementations) clutter the file. If these are no longer needed, remove them. If they're kept for reference during review, ensure they're cleaned up before the PR is merged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 131 - 152, Remove the dead commented-out handler implementations to clean up the file: delete the commented block containing handleModeChange and handleThemeSelect (and their internal calls to setThemeMode, setThemeId, setCustomAccentColor and references to actualThemeMode) so only active code remains; if any logic is still required migrate it into the existing active handlers, otherwise remove the entire commented section before merging.
217-219: Remove or restore commented-out UI elements.The
themeSectionTextshowing "Light Theme" / "Dark Theme" labels is commented out. If the UI intentionally no longer shows these labels, remove the dead code. If this is temporary, add a TODO comment explaining when it should be restored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 217 - 219, The commented-out Text node rendering getString('appearanceScreen.lightTheme') using the themeSectionText style in SettingsAppearanceScreen should either be removed or explicitly marked for future restoration: if the label is intentionally removed, delete the commented block (the commented Text and its surrounding JSX); if it's temporary, replace the comment with a clear TODO comment explaining when/why to restore it and reference the getString('appearanceScreen.lightTheme') usage and styles.themeSectionText so reviewers can find it easily.
153-168: Consider adding fallback theme validation inhandleModeChange.The old commented-out implementation (lines 134-141) validated that the current theme exists in the new mode's theme list, falling back to the first theme if not found. Without this, switching from dark to light mode while using a dark-only theme could leave the user with an invalid theme selection.
♻️ Suggested enhancement
const handleModeChange = (mode: ThemeMode, event: GestureResponderEvent) => { setThemeMode(mode); event.currentTarget.measure((_x1, _y1, width, height, px, py) => { switchTheme({ - switchThemeFunction: () => {}, + switchThemeFunction: () => { + if (mode !== 'system') { + const themes = mode === 'dark' ? darkThemes : lightThemes; + const currentThemeInMode = themes.find(t => t.id === theme.id); + if (!currentThemeInMode) { + setThemeId(themes[0].id); + } + } + }, animationConfig: { type: 'circular', duration: 900, startingPoint: { cy: py + height / 2, cx: px + width / 2, }, }, }); }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 153 - 168, In handleModeChange, before calling setThemeMode and switchTheme, validate that the current theme is available for the target ThemeMode; if it isn't, set a safe fallback theme (e.g., the first theme in the new mode's theme list) so the app never retains an invalid theme for that mode; implement this by checking the current theme against your themesByMode[mode] (or equivalent theme list) and calling the theme setter (e.g., setTheme or setSelectedTheme) with themesByMode[mode][0] when missing, then proceed to setThemeMode(mode) and invoke switchTheme as currently implemented.env.d.ts (1)
2-8: Consider marking config properties as potentially undefined.
react-native-configreads values from environment variables at runtime. If a variable is missing from the.envfile, accessing it will returnundefined. The current typing assumes all values are always present, which could mask runtime errors.♻️ Suggested change to reflect runtime reality
export interface NativeConfig { - MYANIMELIST_CLIENT_ID: string; - ANILIST_CLIENT_ID: string; - GIT_HASH: string; - RELEASE_DATE: string; - BUILD_TYPE: 'Debug' | 'Release' | 'Beta' | 'Github Action'; + MYANIMELIST_CLIENT_ID?: string; + ANILIST_CLIENT_ID?: string; + GIT_HASH?: string; + RELEASE_DATE?: string; + BUILD_TYPE?: 'Debug' | 'Release' | 'Beta' | 'Github Action'; }Alternatively, if you're confident the build system guarantees these values, keep them required but ensure CI validates
.envcompleteness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@env.d.ts` around lines 2 - 8, The NativeConfig interface currently forces all env keys to be present; update the types in NativeConfig (e.g., MYANIMELIST_CLIENT_ID, ANILIST_CLIENT_ID, GIT_HASH, RELEASE_DATE, and BUILD_TYPE) to reflect that they may be missing at runtime by making each property optional or a union with undefined (e.g., string | undefined and 'Debug' | 'Release' | 'Beta' | 'Github Action' | undefined); if you prefer keeping them required instead, add a note or alternative check to ensure CI validates .env completeness before build.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__mocks__/index.js`:
- Line 2: Create a runtime Jest mock for react-native-config by adding a new
file named __mocks__/react-native-config.js that exports the minimal mocked
shape used by About.tsx, aniList.ts, and myAnimeList.ts (e.g., an object with
the env keys your code reads); then update the existing __mocks__/index.js to
also load that mock by adding require('./react-native-config'); alongside the
existing require('./react-native-nitro-modules'); so Jest will register the mock
at test runtime.
In `@android/app/src/main/AndroidManifest.xml`:
- Line 22: The Android release manifest currently enables global cleartext and
backup via android:usesCleartextTraffic="true" and android:allowBackup="true" on
the <application> element (.MainApplication), which weakens production security;
remove or set these attributes to false in the main manifest for release builds
and instead: 1) restrict cleartext to debug by moving
android:usesCleartextTraffic="true" into your debug AndroidManifest (or
implement a networkSecurityConfig with an explicit domain allowlist and
reference it from the release manifest), and 2) disable backups for release by
setting android:allowBackup="false" (or enable only for specific components if
required). Ensure changes target the <application> element referencing
.MainApplication and validate behavior in debug vs release build variants.
In `@android/app/src/main/res/values/styles.xml`:
- Around line 2-7: MainActivity is still using the deprecated systemUiVisibility
flags (SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION / SYSTEM_UI_FLAG_LAYOUT_STABLE);
update it to use modern edge-to-edge APIs by removing references to
systemUiVisibility and instead call enableEdgeToEdge() (or use
WindowCompat.setDecorFitsSystemWindows(window, false)) and install proper window
insets handling; update or add any corresponding insets listeners (e.g.,
WindowInsetsCompat) to adjust view paddings/margins so UI doesn’t overlap
status/navigation bars and ensure this change aligns with the AppTheme parent
Theme.EdgeToEdge.
- Line 7: The app currently sets android:windowLightStatusBar=true globally
which makes light status icons hard to see against the dark splash
colorPrimaryDark; fix by either changing the global item
android:windowLightStatusBar to false, or better add a values-night styles
override that sets android:windowLightStatusBar=false for night mode (create a
values-night/styles.xml that defines your app theme and overrides
android:windowLightStatusBar to false while keeping other theme attributes),
ensuring day mode keeps the original value if needed.
In `@android/gradle/wrapper/gradle-wrapper.properties`:
- Line 3: The Gradle wrapper distributionUrl was bumped to Gradle 9.0.0 which is
incompatible with the project's AGP/React Native versions; revert the wrapper to
a compatible 8.x release (recommend 8.13) by updating the distributionUrl back
to a Gradle 8.x distribution, or alternatively upgrade React Native and Android
Gradle Plugin (AGP) to versions that support Gradle 9 (e.g., RN 0.76+ and AGP
9+) if you intend to move to Gradle 9; update the distributionUrl reference in
the gradle-wrapper.properties file accordingly and ensure the change aligns with
the versions used by the build (run a clean build after changing to confirm
compatibility).
In `@babel.config.js`:
- Line 5: The babel.config.js currently uses ESM syntax ("export default
function (api) { ... }") while your package is being loaded as CommonJS; replace
that export with a CommonJS export by assigning the function to module.exports
(i.e., export the same function via module.exports = function(api) { ... }) so
Node can load the file as .cjs; update the top-level export only and leave the
function body and name untouched.
In `@env.d.ts`:
- Around line 1-11: Tests fail because the runtime module 'react-native-config'
isn't available to Jest; add a Jest mock that exports a Config object matching
the NativeConfig interface (keys: MYANIMELIST_CLIENT_ID, ANILIST_CLIENT_ID,
GIT_HASH, RELEASE_DATE, BUILD_TYPE with allowed values
'Debug'|'Release'|'Beta'|'Github Action'). Implement this by creating a Jest
manual mock for module 'react-native-config' (or adding a jest.mock() in your
test mocks entry) that exports const Config = { MYANIMELIST_CLIENT_ID: '...',
ANILIST_CLIENT_ID: '...', GIT_HASH: '...', RELEASE_DATE: '...', BUILD_TYPE:
'Debug' } and default-exports Config so imports in myAnimeList.ts, aniList.ts,
and About.tsx resolve during tests.
In `@metro.config.js`:
- Around line 1-2: The import for mergeConfig is coming from
'@react-native/metro-config' but should be from 'expo/metro-config'; update the
require so both getDefaultConfig and mergeConfig are imported from
'expo/metro-config' (replace the require of '@react-native/metro-config' with
one that pulls mergeConfig from 'expo/metro-config') and remove the unused
'@react-native/metro-config' import; verify usage of getDefaultConfig and
mergeConfig still works in metro.config.js.
In `@package.json`:
- Around line 74-94: Update the package.json engines.node constraint to require
at least Node.js 20.19.4 (change the "engines.node" value from ">=20" to
">=20.19.4") so it meets Expo SDK 55's minimum; locate the engines.node field in
package.json and replace the current version range accordingly, and ensure any
local/CI Node version managers (nvmrc, .github/workflows or Dockerfiles) are
aligned with the new minimum if present.
In `@src/database/db.ts`:
- Around line 127-139: The migration repair must be idempotent for each column:
instead of only checking `results.some(... row[1] === 'applied_at')` and gating
both alterations, inspect `results` for each column separately (e.g., check for
'applied_at' and for 'name') and call `_db.executeRawSync("ALTER TABLE
'__drizzle_migrations' ADD COLUMN ...")` only for the missing column(s); use the
existing `db.executeRawSync`/`_db.executeRawSync` calls and `results` variable
to implement per-column guards so repeated runs are safe and partial prior runs
are corrected.
In `@src/hooks/persisted/useNovel.ts`:
- Around line 679-687: The delete flow currently removes only the legacy tracked
key `${TRACKED_NOVEL_PREFIX}_${novel.id}` but not tracker-scoped keys, so extend
the cleanup to remove per-tracker entries: for each tracker name associated with
the novel (e.g., iterate novel.trackers or obtain tracker names from the tracker
registry), call
MMKVStorage.remove(`${TRACKED_NOVEL_PREFIX}_${novel.id}_${trackerName}`) in
addition to the existing removals (MMKVStorage.remove for
NOVEL_PAGE_INDEX_PREFIX, NOVEL_SETTINGS_PREFIX, LAST_READ_PREFIX) and keep the
legacy removal; reference MMKVStorage.remove, TRACKED_NOVEL_PREFIX,
NOVEL_PAGE_INDEX_PREFIX, NOVEL_SETTINGS_PREFIX, LAST_READ_PREFIX, NOVEL_STORAGE
and the novel.{id,pluginId,path} identifiers when implementing the loop.
In `@src/plugins/pluginManager.ts`:
- Around line 127-130: The current uninstall loop uses
key.startsWith(_plugin.id) which will match other plugin keys sharing the same
prefix and risk deleting their data; change the matching to a precise namespace
check (e.g., require a delimiter after the plugin id or compare the plugin id
segment) before calling store.remove. Update the logic around
store.getAllKeys(), key.startsWith(_plugin.id), and store.remove to only remove
keys that equal the plugin id namespace or begin with a canonical separator like
`${_plugin.id}:` (or split the key and compare the first segment to _plugin.id)
so only the target plugin's keys are deleted.
In `@src/screens/onboarding/ThemeSelectionStep.tsx`:
- Around line 101-121: The theme ID is being set immediately in
handleThemeSelect while setThemeMode is deferred inside switchThemeFunction,
causing inconsistent ordering with handleModeChange; move the
setThemeId(selectedTheme.id) call into the switchThemeFunction callback passed
to switchTheme so both themeId and themeMode are applied together during the
animation (i.e., inside the same switchTheme({ switchThemeFunction: () => { ...
} }) block), ensuring you update themeId and call setThemeMode in the same place
to avoid visual glitches.
In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx`:
- Around line 51-52: The code uses Omit<ThemeMode, 'system'> incorrectly for a
union exclusion; update the declaration of actualThemeMode to use
Exclude<ThemeMode, 'system'> instead (i.e., replace Omit with Exclude) so the
'system' member is removed from the ThemeMode union correctly; modify the line
that defines actualThemeMode (and any similar occurrences) to use
Exclude<ThemeMode, 'system'> while keeping the existing conditional logic with
themeMode and Appearance.getColorScheme().
- Around line 170-188: handleThemeSelect currently calls switchTheme with an
empty switchThemeFunction, losing two behaviors: clearing custom accent and
syncing theme mode; update handleThemeSelect to call
setCustomAccentColor(undefined) and pass a switchThemeFunction that calls
setThemeMode(selectedTheme.isDark ? 'dark' : 'light') (so switchTheme still
performs animation while the function updates persisted THEME_MODE), keeping
setThemeId(selectedTheme.id) as-is.
---
Nitpick comments:
In `@android/app/src/debug/AndroidManifest.xml`:
- Line 6: The debug manifest contains a redundant android:allowBackup="true"
attribute that duplicates the declaration in the main AndroidManifest.xml;
remove the android:allowBackup attribute from the debug overlay manifest (the
debug AndroidManifest.xml) and leave the single source of truth in the main
AndroidManifest.xml so the app uses the main manifest's setting.
In
`@android/app/src/main/java/com/rajarsheechatterjee/LNReader/MainApplication.kt`:
- Around line 10-11: Remove the unused imports
DefaultNewArchitectureEntryPoint.load and DefaultReactHost.getDefaultReactHost
from MainApplication.kt: locate the import lines that reference
DefaultNewArchitectureEntryPoint.load and DefaultReactHost.getDefaultReactHost
and delete them so only necessary imports remain after migrating to
ExpoReactHostFactory.
In `@env.d.ts`:
- Around line 2-8: The NativeConfig interface currently forces all env keys to
be present; update the types in NativeConfig (e.g., MYANIMELIST_CLIENT_ID,
ANILIST_CLIENT_ID, GIT_HASH, RELEASE_DATE, and BUILD_TYPE) to reflect that they
may be missing at runtime by making each property optional or a union with
undefined (e.g., string | undefined and 'Debug' | 'Release' | 'Beta' | 'Github
Action' | undefined); if you prefer keeping them required instead, add a note or
alternative check to ensure CI validates .env completeness before build.
In `@src/components/ThemePicker/ThemePicker.tsx`:
- Line 19: ThemePickerProps defines an unused isDark property; remove isDark
from the ThemePickerProps interface (and any related type exports) or, if
intended, wire it into the ThemePicker component by destructuring isDark from
props and using it to set initial state/theme selection. Specifically update the
ThemePickerProps type to omit isDark and adjust any callers/prop usages
accordingly, or alternatively modify the ThemePicker function to accept ({
isDark, ... }) and apply isDark to the component's state or className so the
prop is actually used.
- Line 158: The commented-out "elevation: 2" in ThemePicker.tsx removes Android
shadows for the theme cards; locate the style object in the ThemePicker
component where "//elevation: 2" appears (the card/item style for theme cards)
and either uncomment and restore elevation: 2 to re-enable Android shadow
rendering or remove the commented line entirely if the absence of elevation is
intentional, ensuring the iOS shadow props (shadowColor, shadowOffset,
shadowOpacity, shadowRadius) remain unchanged.
In `@src/hooks/persisted/useTheme.ts`:
- Around line 76-97: The lightIdMap and darkIdMap objects are declared as
Record<number, number> but use string key literals; change the keys from quoted
strings to numeric literals (e.g., '1': 100 -> 1: 100) so the object shapes
match the Record<number, number> type, leaving the map names (lightIdMap,
darkIdMap) and values unchanged.
In `@src/screens/novel/components/ChapterItem.tsx`:
- Around line 96-99: The inline style object releaseTimeStyle in ChapterItem is
created every render and should be memoized like the other derived values
(selectedStyle, titleColor, releaseColor, ripple); replace the inline const
releaseTimeStyle with a useMemo that returns the same object based on
theme.outline and chapter.releaseTime so the reference is stable across renders
and consistent with the component’s optimization pattern.
In `@src/screens/novel/components/Info/NovelInfoHeader.tsx`:
- Around line 184-196: The styles.w130 and styles.w180 entries used only once in
NovelInfoHeader's skeleton should be inlined to reduce indirection: replace
usages of styles.w130 and styles.w180 in the View style arrays (which also
include styles.infoSkeletonBar and backgroundColor) with inline width values ({
width: 130 } and { width: 180 }) or rename them to descriptive names if you
intend reuse (e.g., skeletonBarShort/skeletonBarMedium); update the two View
elements in NovelInfoHeader.tsx accordingly and remove the now-unused
styles.w130/styles.w180 from the StyleSheet.
In `@src/screens/onboarding/ThemeSelectionStep.tsx`:
- Around line 177-196: The styles object in ThemeSelectionStep still contains
unused keys from the old AmoledToggle: remove the unused style entries "toggle",
"toggleThumb", and "toggleThumbActive" from the exported styles object in
ThemeSelectionStep.tsx (where styles are defined) and run a quick search for any
remaining references to those symbols to ensure nothing else depends on them;
leave the Switch-based styling intact.
In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx`:
- Around line 34-37: The current type assertion on useMMKVString('THEME_MODE')
hides invalid runtime values and that the setter can accept undefined; replace
the cast with runtime validation and a proper type guard: read the raw string
from useMMKVString into a variable (e.g., rawTheme), validate it against the
ThemeMode union (create an isThemeMode(value): value is ThemeMode function), set
themeMode to a safe default ('system') when invalid, and wrap or narrow the
setter so callers use (mode: ThemeMode | undefined) to allow deletion while
preserving correct typings for setThemeMode and preventing invalid string values
from being treated as ThemeMode.
- Around line 131-152: Remove the dead commented-out handler implementations to
clean up the file: delete the commented block containing handleModeChange and
handleThemeSelect (and their internal calls to setThemeMode, setThemeId,
setCustomAccentColor and references to actualThemeMode) so only active code
remains; if any logic is still required migrate it into the existing active
handlers, otherwise remove the entire commented section before merging.
- Around line 217-219: The commented-out Text node rendering
getString('appearanceScreen.lightTheme') using the themeSectionText style in
SettingsAppearanceScreen should either be removed or explicitly marked for
future restoration: if the label is intentionally removed, delete the commented
block (the commented Text and its surrounding JSX); if it's temporary, replace
the comment with a clear TODO comment explaining when/why to restore it and
reference the getString('appearanceScreen.lightTheme') usage and
styles.themeSectionText so reviewers can find it easily.
- Around line 153-168: In handleModeChange, before calling setThemeMode and
switchTheme, validate that the current theme is available for the target
ThemeMode; if it isn't, set a safe fallback theme (e.g., the first theme in the
new mode's theme list) so the app never retains an invalid theme for that mode;
implement this by checking the current theme against your themesByMode[mode] (or
equivalent theme list) and calling the theme setter (e.g., setTheme or
setSelectedTheme) with themesByMode[mode][0] when missing, then proceed to
setThemeMode(mode) and invoke switchTheme as currently implemented.
In `@src/theme/md3/index.ts`:
- Around line 11-17: The current ID assignment in md3 index uses position-based
mapping (.map((theme, i) => ({ ...theme, id: 100 + i }))) which is fragile;
change to explicit, stable IDs by adding an id property to each theme object in
the exported theme list and update the mapping logic in src/theme/md3/index.ts
to use theme.id when present (fall back to 100+i only for legacy entries). Also
add a unit test asserting the exported theme list length and that each theme has
a numeric id and that the sequence of ids for new themes matches the expected
explicit list to prevent accidental reordering regressions; reference
transformThemeId in useTheme.ts to ensure legacy mappings remain compatible.
🪄 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
Run ID: 2b58e2e4-f2fa-4909-a55d-a9e4e3b5229a
⛔ Files ignored due to path filters (2)
android/gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jarpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (55)
.gitignore__mocks__/index.js__mocks__/react-native-nitro-modules.jsandroid/app/src/debug/AndroidManifest.xmlandroid/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/com/rajarsheechatterjee/LNReader/MainApplication.ktandroid/app/src/main/res/values/strings.xmlandroid/app/src/main/res/values/styles.xmlandroid/build.gradleandroid/gradle/wrapper/gradle-wrapper.propertiesandroid/gradlewandroid/settings.gradlebabel.config.jsdrizzle/migrations.jsenv.d.tsios/LNReader.xcodeproj/project.pbxprojios/LNReader/LNReader-Bridging-Header.hmetro.config.jspackage.jsonsrc/components/BottomSheet/BottomSheet.tsxsrc/components/List/List.tsxsrc/components/SegmentedControl/SegmentedControl.tsxsrc/components/ThemePicker/ThemePicker.tsxsrc/database/db.tssrc/database/queries/__tests__/testDbManager.tssrc/hooks/persisted/useNovel.tssrc/hooks/persisted/useNovelSettings.tssrc/hooks/persisted/usePlugins.tssrc/hooks/persisted/useTheme.tssrc/hooks/persisted/useTrackedNovel.tssrc/plugins/helpers/storage.tssrc/plugins/pluginManager.tssrc/screens/Categories/components/CategoryCard.tsxsrc/screens/more/About.tsxsrc/screens/novel/components/ChapterItem.tsxsrc/screens/novel/components/Info/NovelInfoHeader.tsxsrc/screens/novel/components/JumpToChapterModal.tsxsrc/screens/novel/components/NovelScreenList.tsxsrc/screens/onboarding/ThemeSelectionStep.tsxsrc/screens/reader/components/ReaderBottomSheet/ReaderSheetPreferenceItem.tsxsrc/screens/settings/SettingsAdvancedScreen.tsxsrc/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsxsrc/services/Trackers/aniList.tssrc/services/Trackers/myAnimeList.tssrc/theme/md3/catppuccin.tssrc/theme/md3/defaultTheme.tssrc/theme/md3/index.tssrc/theme/md3/lavender.tssrc/theme/md3/mignightDusk.tssrc/theme/md3/strawberry.tssrc/theme/md3/tako.tssrc/theme/md3/tealTurquoise.tssrc/theme/md3/yinyang.tssrc/theme/md3/yotsuba.tssrc/utils/mmkv/mmkv.ts
💤 Files with no reviewable changes (10)
- src/theme/md3/mignightDusk.ts
- drizzle/migrations.js
- src/theme/md3/defaultTheme.ts
- src/theme/md3/yotsuba.ts
- src/theme/md3/yinyang.ts
- src/theme/md3/tealTurquoise.ts
- src/theme/md3/strawberry.ts
- src/theme/md3/lavender.ts
- src/theme/md3/tako.ts
- src/theme/md3/catppuccin.ts
| @@ -1,4 +1,4 @@ | |||
| require('./nativeModules'); | |||
| require('./react-native-mmkv'); | |||
| require('./react-native-nitro-modules'); | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Possible react-native-config mock files:"
fd -HI 'react-native-config.js' .
echo
echo "Jest registrations for react-native-config:"
rg -n --glob '!node_modules/**' "jest\\.mock\\(['\"]react-native-config['\"]\\)|require\\(['\"]./react-native-config['\"]\\)" .Repository: CD-Z/lnreader
Length of output: 142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if react-native-config is actually imported
echo "=== Checking for react-native-config imports ==="
rg -n "react-native-config" --type ts --type tsx --type js .
echo
echo "=== Current __mocks__/index.js content ==="
cat -n __mocks__/index.js
echo
echo "=== Jest configuration ==="
fd -HI "jest.config.*" . || echo "No jest.config found"
echo
echo "=== setupTests or similar setup files ==="
fd -HI "setup.*test" . || echo "No setup test files found"
echo
echo "=== All mock files in __mocks__ ==="
ls -la __mocks__/ 2>/dev/null || echo "__mocks__ directory not found"Repository: CD-Z/lnreader
Length of output: 150
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if react-native-config is actually imported
echo "=== Checking for react-native-config imports ==="
rg -n "react-native-config" . --max-count=20 || echo "No matches found"
echo
echo "=== Current __mocks__/index.js content ==="
if [ -f "__mocks__/index.js" ]; then
cat -n "__mocks__/index.js"
else
echo "File not found"
fi
echo
echo "=== Jest configuration files ==="
fd -H "jest" . -type f | head -20
echo
echo "=== All files in __mocks__ directory ==="
if [ -d "__mocks__" ]; then
ls -1 __mocks__/
else
echo "__mocks__ directory not found"
fi
echo
echo "=== Looking for react-native-config.js in entire repo ==="
find . -name "*react-native-config*" -type f 2>/dev/null | head -20 || echo "Not found"Repository: CD-Z/lnreader
Length of output: 1262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if react-native-config.js mock exists ==="
if [ -f "__mocks__/react-native-config.js" ]; then
echo "EXISTS"
cat -n "__mocks__/react-native-config.js"
else
echo "DOES NOT EXIST"
fi
echo
echo "=== All files in __mocks__ directory ==="
ls -1 __mocks__/
echo
echo "=== Content of env.d.ts ==="
if [ -f "env.d.ts" ]; then
cat -n env.d.ts | head -30
else
echo "File not found"
fi
echo
echo "=== Jest configuration (if any) ==="
if [ -f "jest.config.js" ]; then
cat -n jest.config.js | head -50
elif [ -f "jest.config.json" ]; then
cat -n jest.config.json | head -50
elif grep -q "jest" package.json 2>/dev/null; then
echo "Jest config found in package.json:"
cat -n package.json | grep -A 20 '"jest"'
else
echo "No jest config found"
fi
echo
echo "=== setupFilesAfterEnv or test setup configuration ==="
rg -n "setupFilesAfterEnv|setupFiles|testEnvironment" . --max-count=10 || echo "Not found"Repository: CD-Z/lnreader
Length of output: 3605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Content of __tests__/jest.setup.ts ==="
if [ -f "__tests__/jest.setup.ts" ]; then
cat -n "__tests__/jest.setup.ts"
else
echo "File not found"
fi
echo
echo "=== Check if react-native-config is mocked anywhere in __tests__ ==="
if [ -d "__tests__" ]; then
rg -n "react-native-config" __tests__/ || echo "Not found in __tests__"
else
echo "__tests__ directory not found"
fiRepository: CD-Z/lnreader
Length of output: 287
Add react-native-config mock to shared Jest setup.
The PR imports react-native-config in three files (About.tsx, aniList.ts, myAnimeList.ts), but no Jest mock is registered. Currently, __mocks__/index.js only loads the Nitro modules mock, and __mocks__/react-native-config.js does not exist. The TypeScript declaration in env.d.ts provides types but not a runtime mock. Tests will fail when these modules load react-native-config.
Create __mocks__/react-native-config.js and add require('./react-native-config'); to __mocks__/index.js.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@__mocks__/index.js` at line 2, Create a runtime Jest mock for
react-native-config by adding a new file named __mocks__/react-native-config.js
that exports the minimal mocked shape used by About.tsx, aniList.ts, and
myAnimeList.ts (e.g., an object with the env keys your code reads); then update
the existing __mocks__/index.js to also load that mock by adding
require('./react-native-config'); alongside the existing
require('./react-native-nitro-modules'); so Jest will register the mock at test
runtime.
| <action android:name="android.intent.action.TTS_SERVICE"/> | ||
| </intent> | ||
| </queries> | ||
| <application android:name=".MainApplication" android:largeHeap="true" android:label="@string/app_name" android:icon="@mipmap/ic_launcher" android:roundIcon="@mipmap/ic_launcher" android:allowBackup="true" android:theme="@style/AppTheme" android:usesCleartextTraffic="true" android:supportsRtl="true"> |
There was a problem hiding this comment.
Harden release manifest: avoid global cleartext + backup in main application config.
Line 22 keeps android:usesCleartextTraffic="true" and android:allowBackup="true" in the main manifest, which weakens transport/data protection for non-debug builds. Prefer restricting cleartext to debug-only (or explicit domain allowlist via networkSecurityConfig) and disabling backup for release unless explicitly required.
🔧 Suggested direction
- <application ... android:allowBackup="true" ... android:usesCleartextTraffic="true" ...>
+ <application ... android:allowBackup="false" ... android:usesCleartextTraffic="false" ...>If HTTP is needed for local/dev endpoints, move cleartext allowance to debug-only manifest or scoped network security config.
📝 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.
Suggested change
| <application android:name=".MainApplication" android:largeHeap="true" android:label="@string/app_name" android:icon="@mipmap/ic_launcher" android:roundIcon="@mipmap/ic_launcher" android:allowBackup="true" android:theme="@style/AppTheme" android:usesCleartextTraffic="true" android:supportsRtl="true"> | |
| <application android:name=".MainApplication" android:largeHeap="true" android:label="@string/app_name" android:icon="@mipmap/ic_launcher" android:roundIcon="@mipmap/ic_launcher" android:allowBackup="false" android:theme="@style/AppTheme" android:usesCleartextTraffic="false" android:supportsRtl="true"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@android/app/src/main/AndroidManifest.xml` at line 22, The Android release
manifest currently enables global cleartext and backup via
android:usesCleartextTraffic="true" and android:allowBackup="true" on the
<application> element (.MainApplication), which weakens production security;
remove or set these attributes to false in the main manifest for release builds
and instead: 1) restrict cleartext to debug by moving
android:usesCleartextTraffic="true" into your debug AndroidManifest (or
implement a networkSecurityConfig with an explicit domain allowlist and
reference it from the release manifest), and 2) disable backups for release by
setting android:allowBackup="false" (or enable only for specific components if
required). Ensure changes target the <application> element referencing
.MainApplication and validate behavior in debug vs release build variants.
Comment on lines
+2
to
+7
| <style name="AppTheme" parent="Theme.EdgeToEdge"> | ||
| <item name="android:editTextBackground">@drawable/rn_edit_text_material</item> | ||
| <item name="colorPrimary">@color/colorPrimary</item> | ||
| <item name="colorPrimaryDark">@color/colorPrimaryDark</item> | ||
| <item name="android:navigationBarColor">@color/colorPrimaryDark</item> | ||
| <item name="android:statusBarColor">@color/colorPrimaryDark</item> | ||
| <item name="colorControlActivated">#2596be</item> | ||
| <item name="android:windowLightStatusBar">true</item> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether edge-to-edge APIs are used and deprecated flags remain.
fd -i MainActivity.kt --exec sed -n '1,180p' {}
rg -n -C2 'enableEdgeToEdge|setDecorFitsSystemWindows|systemUiVisibility|SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION|SYSTEM_UI_FLAG_LAYOUT_STABLE' android srcRepository: CD-Z/lnreader
Length of output: 3730
Theme.EdgeToEdge requires modern window inset handling, but MainActivity still uses deprecated systemUiVisibility flags.
Lines 2-7 of styles.xml define AppTheme with Theme.EdgeToEdge parent. However, MainActivity.kt (lines 25–29) continues relying on the deprecated systemUiVisibility mechanism with SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION and SYSTEM_UI_FLAG_LAYOUT_STABLE instead of calling enableEdgeToEdge() or using modern WindowCompat.setDecorFitsSystemWindows() APIs. This mismatch can cause content overlap under system bars on modern Android versions. Replace deprecated flags with proper edge-to-edge setup via enableEdgeToEdge() and modern inset handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@android/app/src/main/res/values/styles.xml` around lines 2 - 7, MainActivity
is still using the deprecated systemUiVisibility flags
(SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION / SYSTEM_UI_FLAG_LAYOUT_STABLE); update
it to use modern edge-to-edge APIs by removing references to systemUiVisibility
and instead call enableEdgeToEdge() (or use
WindowCompat.setDecorFitsSystemWindows(window, false)) and install proper window
insets handling; update or add any corresponding insets listeners (e.g.,
WindowInsetsCompat) to adjust view paddings/margins so UI doesn’t overlap
status/navigation bars and ensure this change aligns with the AppTheme parent
Theme.EdgeToEdge.
| <item name="android:navigationBarColor">@color/colorPrimaryDark</item> | ||
| <item name="android:statusBarColor">@color/colorPrimaryDark</item> | ||
| <item name="colorControlActivated">#2596be</item> | ||
| <item name="android:windowLightStatusBar">true</item> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if a night override exists for status-bar icon appearance.
fd -i styles.xml android/app/src/main/res
rg -n -C2 'windowLightStatusBar|Theme.EdgeToEdge|SplashScreenTheme' android/app/src/main/resRepository: CD-Z/lnreader
Length of output: 1516
🏁 Script executed:
# Look for color definitions and night-mode variants
find android/app/src/main/res -type d -name "values*" | sort
echo "---"
fd colors.xml android/app/src/main/res
rg -n 'colorPrimary|colorPrimaryDark' android/app/src/main/res/values/Repository: CD-Z/lnreader
Length of output: 920
Hardcoded light status bar icons conflict with dark splash background.
Line 7 sets android:windowLightStatusBar=true globally, but the splash screen uses colorPrimaryDark (#202125—nearly black) for its background (line 23). This creates poor contrast: light status bar icons become invisible against the dark splash background during app startup.
No values-night override exists. Either set windowLightStatusBar=false or add a values-night/styles.xml override with appropriate contrast for dark mode.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@android/app/src/main/res/values/styles.xml` at line 7, The app currently sets
android:windowLightStatusBar=true globally which makes light status icons hard
to see against the dark splash colorPrimaryDark; fix by either changing the
global item android:windowLightStatusBar to false, or better add a values-night
styles override that sets android:windowLightStatusBar=false for night mode
(create a values-night/styles.xml that defines your app theme and overrides
android:windowLightStatusBar to false while keeping other theme attributes),
ensuring day mode keeps the original value if needed.
| distributionBase=GRADLE_USER_HOME | ||
| distributionPath=wrapper/dists | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.3-bin.zip | ||
| distributionUrl=https\://services.gradle.org/distributions/gradle-9.0.0-bin.zip |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Wrapper version =="
rg -n "distributionUrl|distributionSha256Sum" android/gradle/wrapper/gradle-wrapper.properties
echo
echo "== Android/Gradle plugin declarations =="
rg -n --glob '*.gradle' --glob '*.gradle.kts' --glob 'settings.gradle' --glob 'settings.gradle.kts' \
'com\.android\.tools\.build:gradle|id\s*\(\s*"com\.android\.application"\s*\)|kotlin-gradle-plugin|org\.jetbrains\.kotlin' android
echo
echo "== Expo / RN versions from package manifests (if present) =="
fd -i '^package\.json$' | xargs -r rg -n '"expo"|"react-native"'Repository: CD-Z/lnreader
Length of output: 559
🌐 Web query:
For Android Gradle Plugin 8.x (and current Expo SDK 55 / React Native 0.83.x ecosystem), is Gradle 9.0.0 officially supported? Please provide the compatibility matrix links.
💡 Result:
No, Gradle 9.0.0 is not officially supported by Android Gradle Plugin (AGP) 8.x. AGP 8.x requires Gradle versions starting from 8.0 (for AGP 8.0) up to 8.13 (for AGP 8.11+), but not 9.x. AGP 9.0 requires Gradle 9.1+, and AGP 9.1 requires Gradle 9.3+. React Native 0.83 (used in Expo SDK 55) templates use AGP from com.android.tools.build:gradle (version unspecified in snippets, but compatible with AGP 8.x per RN Gradle plugin) and Gradle unspecified in gradle.properties (likely 8.x per compatibility). Expo SDK 55 docs do not specify Gradle/AGP versions but align with RN 0.83, which uses AGP 8.x. Issues exist with third-party libs (e.g., Sentry, Okta) on Gradle 9 + RN 0.83, indicating no official support. Compatibility matrix links: - Official AGP/Gradle: https://developer.android.com/build/releases/about-agp (or https://developer.android.com/build/releases/past-releases/agp-*-release-notes for specifics) - Gradle/AGP tested: https://docs.gradle.org/current/userguide/compatibility.html (tests AGP 8.13-9.1.0-alpha) - RN 0.83 template build.gradle: https://github.com/react-native-community/template/blob/0.83-stable/template/android/build.gradle
Citations:
- 1: https://docs.gradle.org/current/userguide/compatibility.html
- 2: https://developer.android.com/build/releases/about-agp
- 3: https://gradle.org/releases/
- 4: [Android] Build fails when upgrading react-native 0.83.x due to gradle 9 okta/okta-react-native#468
- 5: https://github.com/react-native-community/template/blob/0.83-stable/template/android/gradle.properties
- 6: https://github.com/react-native-community/template/blob/0.83-stable/template/android/build.gradle
🏁 Script executed:
#!/bin/bash
# Get the exact AGP version specification
echo "== Checking build.gradle for AGP version =="
cd android && find . -name "build.gradle*" -type f | head -5 | while read f; do
echo "=== $f ==="
head -30 "$f" | cat -n
doneRepository: CD-Z/lnreader
Length of output: 2621
🏁 Script executed:
#!/bin/bash
echo "== gradle.properties (if exists) =="
cat android/gradle.properties 2>/dev/null || echo "Not found"
echo
echo "== gradle/libs.versions.toml (if exists) =="
cat android/gradle/libs.versions.toml 2>/dev/null || echo "Not found"
echo
echo "== settings.gradle / settings.gradle.kts =="
cat android/settings.gradle 2>/dev/null || cat android/settings.gradle.kts 2>/dev/null || echo "Not found"Repository: CD-Z/lnreader
Length of output: 3830
Gradle 9.0.0 is incompatible with the current Android stack—downgrade or upgrade AGP/React Native before merging.
The project uses React Native 0.83.4 and Expo SDK 55.0.9, which rely on Android Gradle Plugin 8.x. AGP 8.x requires Gradle 8.0–8.13 but does not support Gradle 9.x. This change will break the build. Either downgrade the Gradle wrapper to 8.13 (or compatible 8.x version) or upgrade React Native and AGP to versions that support Gradle 9 (e.g., RN 0.76+ with AGP 9+), understanding this may introduce other compatibility issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@android/gradle/wrapper/gradle-wrapper.properties` at line 3, The Gradle
wrapper distributionUrl was bumped to Gradle 9.0.0 which is incompatible with
the project's AGP/React Native versions; revert the wrapper to a compatible 8.x
release (recommend 8.13) by updating the distributionUrl back to a Gradle 8.x
distribution, or alternatively upgrade React Native and Android Gradle Plugin
(AGP) to versions that support Gradle 9 (e.g., RN 0.76+ and AGP 9+) if you
intend to move to Gradle 9; update the distributionUrl reference in the
gradle-wrapper.properties file accordingly and ensure the change aligns with the
versions used by the build (run a clean build after changing to confirm
compatibility).
Comment on lines
+679
to
687
| MMKVStorage.remove(`${TRACKED_NOVEL_PREFIX}_${novel.id}`); | ||
| MMKVStorage.remove( | ||
| `${NOVEL_PAGE_INDEX_PREFIX}_${novel.pluginId}_${novel.path}`, | ||
| ); | ||
| MMKVStorage.delete( | ||
| MMKVStorage.remove( | ||
| `${NOVEL_SETTINGS_PREFIX}_${novel.pluginId}_${novel.path}`, | ||
| ); | ||
| MMKVStorage.delete(`${LAST_READ_PREFIX}_${novel.pluginId}_${novel.path}`); | ||
| MMKVStorage.remove(`${LAST_READ_PREFIX}_${novel.pluginId}_${novel.path}`); | ||
| const novelDir = NOVEL_STORAGE + '/' + novel.pluginId + '/' + novel.id; |
There was a problem hiding this comment.
Delete flow misses tracker-scoped tracked-novel keys.
Line 679 removes only the legacy single key, but tracked entries are stored per tracker (${TRACKED_NOVEL_PREFIX}_${novelId}_${trackerName}). This leaves stale tracking data after cache deletion.
💡 Proposed fix
for (const novel of cachedNovels) {
- MMKVStorage.remove(`${TRACKED_NOVEL_PREFIX}_${novel.id}`);
+ const trackedPrefix = `${TRACKED_NOVEL_PREFIX}_${novel.id}_`;
+ MMKVStorage
+ .getAllKeys()
+ .filter(key => key.startsWith(trackedPrefix))
+ .forEach(key => MMKVStorage.remove(key));
+ MMKVStorage.remove(`TRACKED_NOVEL_MIGRATION_V1_${novel.id}`);
+
MMKVStorage.remove(
`${NOVEL_PAGE_INDEX_PREFIX}_${novel.pluginId}_${novel.path}`,
);
MMKVStorage.remove(
`${NOVEL_SETTINGS_PREFIX}_${novel.pluginId}_${novel.path}`,
);
MMKVStorage.remove(`${LAST_READ_PREFIX}_${novel.pluginId}_${novel.path}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/persisted/useNovel.ts` around lines 679 - 687, The delete flow
currently removes only the legacy tracked key
`${TRACKED_NOVEL_PREFIX}_${novel.id}` but not tracker-scoped keys, so extend the
cleanup to remove per-tracker entries: for each tracker name associated with the
novel (e.g., iterate novel.trackers or obtain tracker names from the tracker
registry), call
MMKVStorage.remove(`${TRACKED_NOVEL_PREFIX}_${novel.id}_${trackerName}`) in
addition to the existing removals (MMKVStorage.remove for
NOVEL_PAGE_INDEX_PREFIX, NOVEL_SETTINGS_PREFIX, LAST_READ_PREFIX) and keep the
legacy removal; reference MMKVStorage.remove, TRACKED_NOVEL_PREFIX,
NOVEL_PAGE_INDEX_PREFIX, NOVEL_SETTINGS_PREFIX, LAST_READ_PREFIX, NOVEL_STORAGE
and the novel.{id,pluginId,path} identifiers when implementing the loop.
Comment on lines
127
to
130
| store.getAllKeys().forEach(key => { | ||
| if (key.startsWith(_plugin.id)) { | ||
| store.delete(key); | ||
| store.remove(key); | ||
| } |
There was a problem hiding this comment.
Tighten key matching to avoid cross-plugin data deletion.
At Line 128, startsWith(_plugin.id) can remove keys for other plugins whose IDs share a prefix (e.g., foo vs foobar). This is a data-loss bug during uninstall.
💡 Proposed fix
- store.getAllKeys().forEach(key => {
- if (key.startsWith(_plugin.id)) {
- store.remove(key);
- }
- });
+ const keyPrefixes = [
+ `${_plugin.id}_DB_`,
+ `${_plugin.id}_LocalStorage`,
+ `${_plugin.id}_SessionStorage`,
+ ];
+ store.getAllKeys().forEach(key => {
+ if (keyPrefixes.some(prefix => key.startsWith(prefix))) {
+ store.remove(key);
+ }
+ });📝 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.
Suggested change
| store.getAllKeys().forEach(key => { | |
| if (key.startsWith(_plugin.id)) { | |
| store.delete(key); | |
| store.remove(key); | |
| } | |
| const keyPrefixes = [ | |
| `${_plugin.id}_DB_`, | |
| `${_plugin.id}_LocalStorage`, | |
| `${_plugin.id}_SessionStorage`, | |
| ]; | |
| store.getAllKeys().forEach(key => { | |
| if (keyPrefixes.some(prefix => key.startsWith(prefix))) { | |
| store.remove(key); | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/pluginManager.ts` around lines 127 - 130, The current uninstall
loop uses key.startsWith(_plugin.id) which will match other plugin keys sharing
the same prefix and risk deleting their data; change the matching to a precise
namespace check (e.g., require a delimiter after the plugin id or compare the
plugin id segment) before calling store.remove. Update the logic around
store.getAllKeys(), key.startsWith(_plugin.id), and store.remove to only remove
keys that equal the plugin id namespace or begin with a canonical separator like
`${_plugin.id}:` (or split the key and compare the first segment to _plugin.id)
so only the target plugin's keys are deleted.
Comment on lines
+101
to
121
| const handleThemeSelect = ( | ||
| selectedTheme: ThemeColors, | ||
| event: GestureResponderEvent, | ||
| ) => { | ||
| setThemeId(selectedTheme.id); | ||
| setThemeMode(selectedTheme.isDark ? 'dark' : 'light'); | ||
| event.currentTarget.measure((_x1, _y1, width, height, px, py) => { | ||
| switchTheme({ | ||
| switchThemeFunction: () => { | ||
| setThemeMode(selectedTheme.isDark ? 'dark' : 'light'); | ||
| }, | ||
| animationConfig: { | ||
| type: 'circular', | ||
| duration: 900, | ||
| startingPoint: { | ||
| cy: py + height / 2, | ||
| cx: px + width / 2, | ||
| }, | ||
| }, | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Consider consistency: setThemeId is outside animation callback, setThemeMode is inside.
In handleThemeSelect, setThemeId is called immediately (Line 105), while setThemeMode is called inside switchThemeFunction (Line 109). This differs from handleModeChange where setThemeMode is called immediately (Line 85).
This may cause a brief visual glitch where the theme ID updates before the animation completes. Consider moving setThemeId inside switchThemeFunction for consistency, or verify this ordering is intentional.
Proposed fix for consistency
const handleThemeSelect = (
selectedTheme: ThemeColors,
event: GestureResponderEvent,
) => {
- setThemeId(selectedTheme.id);
event.currentTarget.measure((_x1, _y1, width, height, px, py) => {
switchTheme({
switchThemeFunction: () => {
+ setThemeId(selectedTheme.id);
setThemeMode(selectedTheme.isDark ? 'dark' : 'light');
},📝 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.
Suggested change
| const handleThemeSelect = ( | |
| selectedTheme: ThemeColors, | |
| event: GestureResponderEvent, | |
| ) => { | |
| setThemeId(selectedTheme.id); | |
| setThemeMode(selectedTheme.isDark ? 'dark' : 'light'); | |
| event.currentTarget.measure((_x1, _y1, width, height, px, py) => { | |
| switchTheme({ | |
| switchThemeFunction: () => { | |
| setThemeMode(selectedTheme.isDark ? 'dark' : 'light'); | |
| }, | |
| animationConfig: { | |
| type: 'circular', | |
| duration: 900, | |
| startingPoint: { | |
| cy: py + height / 2, | |
| cx: px + width / 2, | |
| }, | |
| }, | |
| }); | |
| }); | |
| }; | |
| const handleThemeSelect = ( | |
| selectedTheme: ThemeColors, | |
| event: GestureResponderEvent, | |
| ) => { | |
| event.currentTarget.measure((_x1, _y1, width, height, px, py) => { | |
| switchTheme({ | |
| switchThemeFunction: () => { | |
| setThemeId(selectedTheme.id); | |
| setThemeMode(selectedTheme.isDark ? 'dark' : 'light'); | |
| }, | |
| animationConfig: { | |
| type: 'circular', | |
| duration: 900, | |
| startingPoint: { | |
| cy: py + height / 2, | |
| cx: px + width / 2, | |
| }, | |
| }, | |
| }); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/onboarding/ThemeSelectionStep.tsx` around lines 101 - 121, The
theme ID is being set immediately in handleThemeSelect while setThemeMode is
deferred inside switchThemeFunction, causing inconsistent ordering with
handleModeChange; move the setThemeId(selectedTheme.id) call into the
switchThemeFunction callback passed to switchTheme so both themeId and themeMode
are applied together during the animation (i.e., inside the same switchTheme({
switchThemeFunction: () => { ... } }) block), ensuring you update themeId and
call setThemeMode in the same place to avoid visual glitches.
src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+170
to
188
| const handleThemeSelect = ( | ||
| selectedTheme: ThemeColors, | ||
| event: GestureResponderEvent, | ||
| ) => { | ||
| setThemeId(selectedTheme.id); | ||
| setCustomAccentColor(undefined); | ||
|
|
||
| if (currentMode !== 'system') { | ||
| setThemeMode(selectedTheme.isDark ? 'dark' : 'light'); | ||
| } | ||
| event.currentTarget.measure((_x1, _y1, width, height, px, py) => { | ||
| switchTheme({ | ||
| switchThemeFunction: () => {}, | ||
| animationConfig: { | ||
| type: 'circular', | ||
| duration: 900, | ||
| startingPoint: { | ||
| cy: py + height / 2, | ||
| cx: px + width / 2, | ||
| }, | ||
| }, | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Missing theme mode synchronization and custom accent reset.
The new handleThemeSelect removed two behaviors present in the commented-out implementation:
- Custom accent reset (old line 146):
setCustomAccentColor(undefined)ensured selecting a new theme cleared any custom accent color. - Theme mode sync (old lines 148-150): When a user explicitly selects a light/dark theme, the mode should update accordingly.
The onboarding screen (ThemeSelectionStep.tsx, context snippet 3) uses switchThemeFunction to call setThemeMode(selectedTheme.isDark ? 'dark' : 'light'). This screen's empty switchThemeFunction will leave the persisted THEME_MODE stale, causing inconsistencies with useTheme hook consumers (context snippet 4).
🐛 Proposed fix to restore missing functionality
const handleThemeSelect = (
selectedTheme: ThemeColors,
event: GestureResponderEvent,
) => {
setThemeId(selectedTheme.id);
+ setCustomAccentColor(undefined);
event.currentTarget.measure((_x1, _y1, width, height, px, py) => {
switchTheme({
- switchThemeFunction: () => {},
+ switchThemeFunction: () => {
+ if (themeMode !== 'system') {
+ setThemeMode(selectedTheme.isDark ? 'dark' : 'light');
+ }
+ },
animationConfig: {
type: 'circular',
duration: 900,
startingPoint: {
cy: py + height / 2,
cx: px + width / 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.
Suggested change
| const handleThemeSelect = ( | |
| selectedTheme: ThemeColors, | |
| event: GestureResponderEvent, | |
| ) => { | |
| setThemeId(selectedTheme.id); | |
| setCustomAccentColor(undefined); | |
| if (currentMode !== 'system') { | |
| setThemeMode(selectedTheme.isDark ? 'dark' : 'light'); | |
| } | |
| event.currentTarget.measure((_x1, _y1, width, height, px, py) => { | |
| switchTheme({ | |
| switchThemeFunction: () => {}, | |
| animationConfig: { | |
| type: 'circular', | |
| duration: 900, | |
| startingPoint: { | |
| cy: py + height / 2, | |
| cx: px + width / 2, | |
| }, | |
| }, | |
| }); | |
| }); | |
| }; | |
| const handleThemeSelect = ( | |
| selectedTheme: ThemeColors, | |
| event: GestureResponderEvent, | |
| ) => { | |
| setThemeId(selectedTheme.id); | |
| setCustomAccentColor(undefined); | |
| event.currentTarget.measure((_x1, _y1, width, height, px, py) => { | |
| switchTheme({ | |
| switchThemeFunction: () => { | |
| if (themeMode !== 'system') { | |
| setThemeMode(selectedTheme.isDark ? 'dark' : 'light'); | |
| } | |
| }, | |
| animationConfig: { | |
| type: 'circular', | |
| duration: 900, | |
| startingPoint: { | |
| cy: py + height / 2, | |
| cx: px + width / 2, | |
| }, | |
| }, | |
| }); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx`
around lines 170 - 188, handleThemeSelect currently calls switchTheme with an
empty switchThemeFunction, losing two behaviors: clearing custom accent and
syncing theme mode; update handleThemeSelect to call
setCustomAccentColor(undefined) and pass a switchThemeFunction that calls
setThemeMode(selectedTheme.isDark ? 'dark' : 'light') (so switchTheme still
performs animation while the function updates persisted THEME_MODE), keeping
setThemeId(selectedTheme.id) as-is.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/onboarding/ThemeSelectionStep.tsx`:
- Around line 40-50: The row Pressable and the Switch both call toggle(),
causing duplicate toggles; remove one binding so only a single interactive
surface triggers the change. Either delete onPress={toggle} from the Pressable
and keep Switch's onValueChange={toggle}, or keep onPress on the Pressable and
remove onValueChange from the Switch (so the Switch becomes non-interactive and
the whole row handles taps); locate the Pressable element and the Switch
component usage and remove the redundant toggle reference accordingly.
🪄 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
Run ID: 1eaab210-f186-4b4f-bb1c-8fd0e3cf5bbe
📒 Files selected for processing (1)
src/screens/onboarding/ThemeSelectionStep.tsx
Comment on lines
+40
to
+50
| <Pressable | ||
| style={[ | ||
| styles.amoledContainer, | ||
| { backgroundColor: theme.surfaceVariant }, | ||
| ]} | ||
| onPress={toggle} | ||
| > | ||
| <Text style={[styles.amoledLabel, { color: theme.onSurface }]}> | ||
| {getString('appearanceScreen.pureBlackDarkMode')} | ||
| </Text> | ||
| <Pressable | ||
| onPress={() => setAmoledBlack(!isAmoledBlack)} | ||
| style={[ | ||
| styles.toggle, | ||
| { | ||
| backgroundColor: isAmoledBlack | ||
| ? theme.primary | ||
| : theme.surfaceVariant, | ||
| }, | ||
| ]} | ||
| > | ||
| <View | ||
| style={[ | ||
| styles.toggleThumb, | ||
| isAmoledBlack && styles.toggleThumbActive, | ||
| { | ||
| backgroundColor: isAmoledBlack | ||
| ? theme.onPrimary | ||
| : theme.onSurfaceVariant, | ||
| }, | ||
| ]} | ||
| /> | ||
| </Pressable> | ||
| </View> | ||
| <Switch value={isAmoledBlack} onValueChange={toggle} /> |
There was a problem hiding this comment.
Don’t bind toggle() to both press targets.
@components/Switch/Switch already wraps itself in a Pressable (src/components/Switch/Switch.tsx:109-111). With onPress={toggle} on the row and onValueChange={toggle} on the switch, a tap on the switch fires toggle() twice and writes AMOLED_BLACK twice for one interaction. Keep a single interactive surface here.
One local option if the whole row should stay tappable
<Pressable
style={[
styles.amoledContainer,
{ backgroundColor: theme.surfaceVariant },
]}
onPress={toggle}
+ accessibilityRole="switch"
+ accessibilityState={{ checked: isAmoledBlack }}
>
<Text style={[styles.amoledLabel, { color: theme.onSurface }]}>
{getString('appearanceScreen.pureBlackDarkMode')}
</Text>
- <Switch value={isAmoledBlack} onValueChange={toggle} />
+ <View pointerEvents="none">
+ <Switch value={isAmoledBlack} />
+ </View>
</Pressable>📝 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.
Suggested change
| <Pressable | |
| style={[ | |
| styles.amoledContainer, | |
| { backgroundColor: theme.surfaceVariant }, | |
| ]} | |
| onPress={toggle} | |
| > | |
| <Text style={[styles.amoledLabel, { color: theme.onSurface }]}> | |
| {getString('appearanceScreen.pureBlackDarkMode')} | |
| </Text> | |
| <Pressable | |
| onPress={() => setAmoledBlack(!isAmoledBlack)} | |
| style={[ | |
| styles.toggle, | |
| { | |
| backgroundColor: isAmoledBlack | |
| ? theme.primary | |
| : theme.surfaceVariant, | |
| }, | |
| ]} | |
| > | |
| <View | |
| style={[ | |
| styles.toggleThumb, | |
| isAmoledBlack && styles.toggleThumbActive, | |
| { | |
| backgroundColor: isAmoledBlack | |
| ? theme.onPrimary | |
| : theme.onSurfaceVariant, | |
| }, | |
| ]} | |
| /> | |
| </Pressable> | |
| </View> | |
| <Switch value={isAmoledBlack} onValueChange={toggle} /> | |
| <Pressable | |
| style={[ | |
| styles.amoledContainer, | |
| { backgroundColor: theme.surfaceVariant }, | |
| ]} | |
| onPress={toggle} | |
| accessibilityRole="switch" | |
| accessibilityState={{ checked: isAmoledBlack }} | |
| > | |
| <Text style={[styles.amoledLabel, { color: theme.onSurface }]}> | |
| {getString('appearanceScreen.pureBlackDarkMode')} | |
| </Text> | |
| <View pointerEvents="none"> | |
| <Switch value={isAmoledBlack} /> | |
| </View> | |
| </Pressable> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/onboarding/ThemeSelectionStep.tsx` around lines 40 - 50, The row
Pressable and the Switch both call toggle(), causing duplicate toggles; remove
one binding so only a single interactive surface triggers the change. Either
delete onPress={toggle} from the Pressable and keep Switch's
onValueChange={toggle}, or keep onPress on the Pressable and remove
onValueChange from the Switch (so the Switch becomes non-interactive and the
whole row handles taps); locate the Pressable element and the Switch component
usage and remove the redundant toggle reference accordingly.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx (2)
175-193:⚠️ Potential issue | 🟠 MajorRestore the removed side effects on theme selection.
This still drops
setCustomAccentColor(undefined)and the non-systemsetThemeMode(selectedTheme.isDark ? 'dark' : 'light')sync, so picking a concrete theme can leave the accent override and persistedTHEME_MODEout of sync with the selected palette.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 175 - 193, handleThemeSelect currently only sets themeId and calls switchTheme, but it dropped the side effects that clear any custom accent and persist the selected theme mode; restore calling setCustomAccentColor(undefined) and setThemeMode(selectedTheme.isDark ? 'dark' : 'light') (in the same logical order as before) inside handleThemeSelect so selecting a concrete theme clears the accent override and updates persisted THEME_MODE, then proceed to call switchTheme and setThemeId as before.
158-172:⚠️ Potential issue | 🟠 MajorMove theme state mutations into
switchThemeFunction.Both
handleModeChangeandhandleThemeSelectupdate state before callingswitchTheme, leavingswitchThemeFunctionempty. The library's documented example and circular-animation pattern show that theme mutations should occur insideswitchThemeFunction. Mutating state before the animation starts causes the component tree to update immediately rather than synchronizing with the animation, defeating the animation's purpose.🔧 Proposed fix
const handleModeChange = (mode: ThemeMode, event: GestureResponderEvent) => { - setThemeMode(mode); event.currentTarget.measure((_x1, _y1, width, height, px, py) => { switchTheme({ - switchThemeFunction: () => {}, + switchThemeFunction: () => { + setThemeMode(mode); + }, animationConfig: { type: 'circular', duration: 900, startingPoint: { cy: py + height / 2, cx: px + width / 2, }, }, }); }); }; const handleThemeSelect = ( selectedTheme: ThemeColors, event: GestureResponderEvent, ) => { - setThemeId(selectedTheme.id); event.currentTarget.measure((_x1, _y1, width, height, px, py) => { switchTheme({ - switchThemeFunction: () => {}, + switchThemeFunction: () => { + setThemeId(selectedTheme.id); + }, animationConfig: { type: 'circular', duration: 900, startingPoint: { cy: py + height / 2, cx: px + width / 2, }, }, }); }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 158 - 172, The theme state (setThemeMode/setThemeName) is being mutated before starting the animated transition; move those mutations into the switchThemeFunction so the component updates are synchronized with the circular animation. In handleModeChange and handleThemeSelect, keep measuring the event target and calling switchTheme, but remove the direct setThemeMode/setThemeName calls and instead invoke setThemeMode(mode) or setThemeName(theme) inside the provided switchThemeFunction callback passed to switchTheme; ensure the animationConfig (type/duration/startingPoint) remains unchanged and that switchThemeFunction closes over the correct mode/theme argument.
🧹 Nitpick comments (1)
src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx (1)
51-57: Drop the unreachable'unspecified'branch.
Appearance.getColorScheme()returns'light','dark', ornull, so after the?? 'light'fallback this extra branch can never fire. Simplifying it keepsactualThemeModealigned with the React Native API and removes dead code. (reactnative.dev)♻️ Proposed simplification
const actualThemeMode: Exclude<ThemeMode, 'system'> = - themeMode !== 'system' - ? themeMode - : colorScheme === 'unspecified' - ? 'light' - : colorScheme; + themeMode !== 'system' ? themeMode : colorScheme;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 51 - 57, The branch checking for 'unspecified' is dead code because Appearance.getColorScheme() returns 'light' | 'dark' | null and you already fallback to 'light'; update the logic that computes actualThemeMode (the variable actualThemeMode, using Appearance.getColorScheme() and themeMode, and the ThemeMode type) to remove the 'unspecified' case and simply use the colorScheme fallback directly when themeMode === 'system', ensuring the result is typed as Exclude<ThemeMode, 'system'>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx`:
- Around line 175-193: handleThemeSelect currently only sets themeId and calls
switchTheme, but it dropped the side effects that clear any custom accent and
persist the selected theme mode; restore calling setCustomAccentColor(undefined)
and setThemeMode(selectedTheme.isDark ? 'dark' : 'light') (in the same logical
order as before) inside handleThemeSelect so selecting a concrete theme clears
the accent override and updates persisted THEME_MODE, then proceed to call
switchTheme and setThemeId as before.
- Around line 158-172: The theme state (setThemeMode/setThemeName) is being
mutated before starting the animated transition; move those mutations into the
switchThemeFunction so the component updates are synchronized with the circular
animation. In handleModeChange and handleThemeSelect, keep measuring the event
target and calling switchTheme, but remove the direct setThemeMode/setThemeName
calls and instead invoke setThemeMode(mode) or setThemeName(theme) inside the
provided switchThemeFunction callback passed to switchTheme; ensure the
animationConfig (type/duration/startingPoint) remains unchanged and that
switchThemeFunction closes over the correct mode/theme argument.
---
Nitpick comments:
In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx`:
- Around line 51-57: The branch checking for 'unspecified' is dead code because
Appearance.getColorScheme() returns 'light' | 'dark' | null and you already
fallback to 'light'; update the logic that computes actualThemeMode (the
variable actualThemeMode, using Appearance.getColorScheme() and themeMode, and
the ThemeMode type) to remove the 'unspecified' case and simply use the
colorScheme fallback directly when themeMode === 'system', ensuring the result
is typed as Exclude<ThemeMode, 'system'>.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c04e9169-509e-429b-a386-b3d7f5d92f08
📒 Files selected for processing (1)
src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx
CD-Z
force-pushed
the
theme-switcher-refactor
branch
from
93e9e3e to
eadb954
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx (1)
158-173:⚠️ Potential issue | 🟡 MinorEmpty
switchThemeFunction— same timing concern as onboarding screen.State changes (
setThemeMode) occur beforeswitchThemeis called, with an empty callback. For proper before/after screenshot capture by the animation library, consider movingsetThemeMode(mode)insideswitchThemeFunction.🔧 Proposed fix
const handleModeChange = (mode: ThemeMode, event: GestureResponderEvent) => { - setThemeMode(mode); event.currentTarget.measure((_x1, _y1, width, height, px, py) => { switchTheme({ - switchThemeFunction: () => {}, + switchThemeFunction: () => { + setThemeMode(mode); + }, animationConfig: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 158 - 173, In handleModeChange, don't call setThemeMode before triggering the transition; instead pass a non-empty switchThemeFunction to switchTheme that performs setThemeMode(mode) so the animation library can capture before/after states; update the call in handleModeChange (the switchTheme invocation) to include switchThemeFunction: () => setThemeMode(mode) and keep the same animationConfig (type/duration/startingPoint) so timing and circular animation behavior remain unchanged.
🧹 Nitpick comments (5)
src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx (2)
136-157: Remove commented-out code.This large block of commented code (old
handleModeChangeandhandleThemeSelectimplementations) adds noise. If needed for reference, it's preserved in version control. Consider removing it for cleaner code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 136 - 157, Remove the commented-out legacy implementations of handleModeChange and handleThemeSelect from SettingsAppearanceScreen to reduce noise: delete the entire commented block containing the old handleModeChange (that sets setThemeMode, chooses darkThemes/lightThemes, checks currentThemeInMode, and calls setThemeId) and the old handleThemeSelect (that calls setThemeId, setCustomAccentColor, and conditionally setThemeMode based on actualThemeMode). Keep only the active implementations and rely on VCS for history.
344-355: Commented-out style property.Line 346 has
//marginHorizontal: 8commented out. Either remove it or restore it if needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx` around lines 344 - 355, The style object themePickerRow contains a commented-out property "//marginHorizontal: 8" — either remove this commented line to keep the code clean or restore it as an active property if horizontal margins are required; update the themePickerRow style (in SettingsAppearanceScreen's themePickerRow) accordingly and ensure layout still matches design after re-enabling or removing the margin.src/screens/onboarding/ThemeSelectionStep.tsx (1)
175-195: Unused styles remain after refactor.The styles
toggle,toggleThumb, andtoggleThumbActive(lines 175-194) appear to be remnants from the previous custom toggle implementation that was replaced with<Switch>. Consider removing these unused styles.🧹 Proposed cleanup
amoledLabel: { fontSize: 16, fontWeight: '400', }, - toggle: { - width: 52, - height: 32, - borderRadius: 16, - padding: 2, - justifyContent: 'center', - }, - toggleThumb: { - width: 28, - height: 28, - borderRadius: 14, - elevation: 2, - shadowColor: '#000', - shadowOffset: { width: 0, height: 2 }, - shadowOpacity: 0.2, - shadowRadius: 2, - }, - toggleThumbActive: { - alignSelf: 'flex-end', - }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/onboarding/ThemeSelectionStep.tsx` around lines 175 - 195, Remove the unused style entries left from the old custom toggle in ThemeSelectionStep: delete the toggle, toggleThumb, and toggleThumbActive keys from the StyleSheet object (the styles created in ThemeSelectionStep.tsx, often referenced as styles). After removing them, search for any remaining references to styles.toggle, styles.toggleThumb, or styles.toggleThumbActive in the component and remove or replace those references so only the current <Switch>-based styling remains.src/components/ThemePicker/ThemePicker.tsx (2)
19-27: UnusedisDarkprop declared but never used.The
isDarkprop is defined inThemePickerProps(line 19) but is not destructured in the component parameters (lines 22-27) and is never referenced in the component body. Since neither call site passes this prop (per context snippets), consider removing it to avoid confusion.🧹 Proposed fix to remove unused prop
interface ThemePickerProps { theme: ThemeColors; currentTheme: ThemeColors; onPress: (event: GestureResponderEvent) => void; horizontal?: boolean; - isDark?: boolean; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ThemePicker/ThemePicker.tsx` around lines 19 - 27, The ThemePickerProps interface declares an unused isDark prop that is never destructured or referenced in the ThemePicker component; remove isDark from the ThemePickerProps type declaration and any other type declarations, and update any call sites or tests (if present) to stop passing isDark so the ThemePicker component signature ({ theme, currentTheme, onPress, horizontal }: ThemePickerProps) stays consistent with the props actually used in the ThemePicker function.
152-159: Commented-out elevation creates platform inconsistency.iOS shadow properties (lines 153-156) are active while Android elevation is commented out. This results in shadows appearing on iOS but not on Android. If intentional for visual reasons, consider adding a brief comment explaining why; otherwise, either enable both or remove both.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ThemePicker/ThemePicker.tsx` around lines 152 - 159, The Android elevation is commented out while iOS shadow props (shadowColor, shadowOffset, shadowOpacity, shadowRadius) are enabled, causing inconsistent shadows across platforms; update the ThemePicker style (where shadowColor/shadowOffset/shadowOpacity/shadowRadius are defined) to either enable elevation: 2 for Android or remove/disable the iOS shadow props so both platforms match, or add a brief inline comment explaining the intentional visual difference—ensure you edit the style block that contains the elevation property and shadow* properties.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/onboarding/ThemeSelectionStep.tsx`:
- Around line 84-99: The theme is being set before the animation captures a
screenshot, so move the state change into the switchThemeFunction passed to
switchTheme inside handleModeChange: remove setThemeMode(mode) from the top of
handleModeChange and instead call setThemeMode(mode) inside the
switchThemeFunction callback you pass to switchTheme within the
event.currentTarget.measure callback (keep the circular animationConfig and
startingPoint calculation as-is); this ensures
react-native-theme-switch-animation takes the screenshot of the old theme before
applying setThemeMode in switchThemeFunction.
---
Duplicate comments:
In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx`:
- Around line 158-173: In handleModeChange, don't call setThemeMode before
triggering the transition; instead pass a non-empty switchThemeFunction to
switchTheme that performs setThemeMode(mode) so the animation library can
capture before/after states; update the call in handleModeChange (the
switchTheme invocation) to include switchThemeFunction: () => setThemeMode(mode)
and keep the same animationConfig (type/duration/startingPoint) so timing and
circular animation behavior remain unchanged.
---
Nitpick comments:
In `@src/components/ThemePicker/ThemePicker.tsx`:
- Around line 19-27: The ThemePickerProps interface declares an unused isDark
prop that is never destructured or referenced in the ThemePicker component;
remove isDark from the ThemePickerProps type declaration and any other type
declarations, and update any call sites or tests (if present) to stop passing
isDark so the ThemePicker component signature ({ theme, currentTheme, onPress,
horizontal }: ThemePickerProps) stays consistent with the props actually used in
the ThemePicker function.
- Around line 152-159: The Android elevation is commented out while iOS shadow
props (shadowColor, shadowOffset, shadowOpacity, shadowRadius) are enabled,
causing inconsistent shadows across platforms; update the ThemePicker style
(where shadowColor/shadowOffset/shadowOpacity/shadowRadius are defined) to
either enable elevation: 2 for Android or remove/disable the iOS shadow props so
both platforms match, or add a brief inline comment explaining the intentional
visual difference—ensure you edit the style block that contains the elevation
property and shadow* properties.
In `@src/screens/onboarding/ThemeSelectionStep.tsx`:
- Around line 175-195: Remove the unused style entries left from the old custom
toggle in ThemeSelectionStep: delete the toggle, toggleThumb, and
toggleThumbActive keys from the StyleSheet object (the styles created in
ThemeSelectionStep.tsx, often referenced as styles). After removing them, search
for any remaining references to styles.toggle, styles.toggleThumb, or
styles.toggleThumbActive in the component and remove or replace those references
so only the current <Switch>-based styling remains.
In `@src/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsx`:
- Around line 136-157: Remove the commented-out legacy implementations of
handleModeChange and handleThemeSelect from SettingsAppearanceScreen to reduce
noise: delete the entire commented block containing the old handleModeChange
(that sets setThemeMode, chooses darkThemes/lightThemes, checks
currentThemeInMode, and calls setThemeId) and the old handleThemeSelect (that
calls setThemeId, setCustomAccentColor, and conditionally setThemeMode based on
actualThemeMode). Keep only the active implementations and rely on VCS for
history.
- Around line 344-355: The style object themePickerRow contains a commented-out
property "//marginHorizontal: 8" — either remove this commented line to keep the
code clean or restore it as an active property if horizontal margins are
required; update the themePickerRow style (in SettingsAppearanceScreen's
themePickerRow) accordingly and ensure layout still matches design after
re-enabling or removing the margin.
🪄 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
Run ID: b676b407-2f21-4720-b1ce-92d3c65876c1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
package.jsonsrc/components/SegmentedControl/SegmentedControl.tsxsrc/components/ThemePicker/ThemePicker.tsxsrc/hooks/persisted/useTheme.tssrc/screens/onboarding/ThemeSelectionStep.tsxsrc/screens/settings/SettingsAppearanceScreen/SettingsAppearanceScreen.tsxsrc/theme/md3/catppuccin.tssrc/theme/md3/defaultTheme.tssrc/theme/md3/index.tssrc/theme/md3/lavender.tssrc/theme/md3/mignightDusk.tssrc/theme/md3/strawberry.tssrc/theme/md3/tako.tssrc/theme/md3/tealTurquoise.tssrc/theme/md3/yinyang.tssrc/theme/md3/yotsuba.ts
💤 Files with no reviewable changes (9)
- src/theme/md3/yinyang.ts
- src/theme/md3/yotsuba.ts
- src/theme/md3/lavender.ts
- src/theme/md3/mignightDusk.ts
- src/theme/md3/tealTurquoise.ts
- src/theme/md3/defaultTheme.ts
- src/theme/md3/strawberry.ts
- src/theme/md3/catppuccin.ts
- src/theme/md3/tako.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- src/hooks/persisted/useTheme.ts
- src/components/SegmentedControl/SegmentedControl.tsx
- src/theme/md3/index.ts
Comment on lines
+84
to
99
| const handleModeChange = (mode: ThemeMode, event: GestureResponderEvent) => { | ||
| setThemeMode(mode); | ||
|
|
||
| if (mode !== 'system') { | ||
| const themes = mode === 'dark' ? darkThemes : lightThemes; | ||
| const currentThemeInMode = themes.find(t => t.id === theme.id); | ||
|
|
||
| if (!currentThemeInMode) { | ||
| setThemeId(themes[0].id); | ||
| } | ||
| } | ||
| event.currentTarget.measure((_x1, _y1, width, height, px, py) => { | ||
| switchTheme({ | ||
| switchThemeFunction: () => {}, | ||
| animationConfig: { | ||
| type: 'circular', | ||
| duration: 900, | ||
| startingPoint: { | ||
| cy: py + height / 2, | ||
| cx: px + width / 2, | ||
| }, | ||
| }, | ||
| }); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
How does react-native-theme-switch-animation switchThemeFunction callback work? When is the screenshot taken relative to the callback execution?
💡 Result:
The switchThemeFunction callback in react-native-theme-switch-animation is a user-provided function that performs the actual theme state change in your app (e.g., setTheme('dark'), Redux dispatch, Context update, etc.). It is called by the library during the animation sequence to switch the underlying theme while an overlay/screenshot of the old theme animates out, creating a smooth transition effect. The screenshot (or native snapshot) of the current screen (old theme) is taken before executing the switchThemeFunction callback. The library first captures the screen state, overlays it, starts the animation (fade/circular/etc.), then calls your switchThemeFunction to update the theme underneath the overlay, and finally animates the overlay out to reveal the new theme. This ensures the transition appears seamless without flicker. This mechanism is standard for theme transition libraries and matches the "plug & play" description in docs, where your callback handles the theme logic after library setup.
Citations:
- 1: https://github.com/WadhahEssam/react-native-theme-switch-animation/blob/main/README.md
- 2: https://registry.npmjs.org/react-native-theme-switch-animation
- 3: https://socket.dev/npm/package/react-native-theme-switch-animation
- 4: https://medium.com/@wadahesam/smooth-dark-light-theme-transition-animations-in-react-native-17c0632ecec4
- 5: Circular animation not work most of the time WadhahEssam/react-native-theme-switch-animation#20
Move theme change into switchThemeFunction to match library expectations.
The react-native-theme-switch-animation library takes a screenshot of the current screen state before executing switchThemeFunction. Calling setThemeMode(mode) at line 85 (before the measure callback) means the screenshot captures the new theme state, defeating the animation's visual transition effect. Move setThemeMode(mode) into the callback:
Proposed fix
const handleModeChange = (mode: ThemeMode, event: GestureResponderEvent) => {
- setThemeMode(mode);
event.currentTarget.measure((_x1, _y1, width, height, px, py) => {
switchTheme({
- switchThemeFunction: () => {},
+ switchThemeFunction: () => {
+ setThemeMode(mode);
+ },
animationConfig: {
type: 'circular',
duration: 900,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/onboarding/ThemeSelectionStep.tsx` around lines 84 - 99, The
theme is being set before the animation captures a screenshot, so move the state
change into the switchThemeFunction passed to switchTheme inside
handleModeChange: remove setThemeMode(mode) from the top of handleModeChange and
instead call setThemeMode(mode) inside the switchThemeFunction callback you pass
to switchTheme within the event.currentTarget.measure callback (keep the
circular animationConfig and startingPoint calculation as-is); this ensures
react-native-theme-switch-animation takes the screenshot of the old theme before
applying setThemeMode in switchThemeFunction.
2547835 to
06f0f66
Compare
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
New Features
Improvements