Conversation
|
Important Review skippedToo many files! This PR contains 150 files, which is 50 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (150)
You can disable this status message by setting the 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 |
This comment has been minimized.
This comment has been minimized.
| const isActive = segment.value === activeFilter; | ||
| return ( | ||
| <Pressable key={segment.value} onPress={() => handleFilterChange(segment.value)} className={`flex-1 items-center rounded-md px-3 py-2 ${isActive ? 'bg-white shadow-sm dark:bg-gray-600' : ''}`}> | ||
| <Pressable key={segment.value} onPress={() => handleFilterChange(segment.value)} className={`flex-1 items-center rounded-md px-3 py-2 ${isActive ? 'bg-white shadow-xs dark:bg-gray-600' : ''}`}> |
There was a problem hiding this comment.
Render performance issue: Inline arrow functions in JSX props, such as onPress={() => handleFilterChange(segment.value)}, create new function instances on every render. Move these function definitions outside the render method to prevent unnecessary re-renders.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/maps/search.tsx:
Line 176:
Render performance issue: Inline arrow functions in JSX props, such as `onPress={() => handleFilterChange(segment.value)}`, create new function instances on every render. Move these function definitions outside the render method to prevent unnecessary re-renders.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } catch { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Silent exception swallowing: The catch block in map-view.web.tsx returns false without logging the initialization or projection failure. Log the caught error with structured context before returning false to allow debugging of the retry logic.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/components/maps/map-view.web.tsx:
Line 560 to 562:
Silent exception swallowing: The `catch` block in `map-view.web.tsx` returns `false` without logging the initialization or projection failure. Log the caught error with structured context before returning `false` to allow debugging of the retry logic.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| backgroundColor: Appearance.getColorScheme() === 'dark' ? '#171717' : '#fff', | ||
| shadowColor: Appearance.getColorScheme() === 'dark' ? '#262626' : '#e5e5e5', |
There was a problem hiding this comment.
Incorrect theme detection: NotificationDetail.tsx and NotificationInbox.tsx use Appearance.getColorScheme() inside StyleSheet.create, which ignores manual in-app theme overrides and evaluates only once at module load. Read the effective scheme via useColorScheme() or nativewind at render time to correctly apply user-selected themes.
// Compute styles at render time from the active scheme so manual
// theme overrides are reflected:
// const isDark = useColorScheme() === 'dark';
// sidebarContainer: { backgroundColor: isDark ? '#171717' : '#fff', ... }Prompt for LLM
File src/components/notifications/NotificationDetail.tsx:
Line 206 to 207:
Incorrect theme detection: NotificationDetail.tsx and NotificationInbox.tsx use `Appearance.getColorScheme()` inside `StyleSheet.create`, which ignores manual in-app theme overrides and evaluates only once at module load. Read the effective scheme via `useColorScheme()` or `nativewind` at render time to correctly apply user-selected themes.
Suggested Code:
// Compute styles at render time from the active scheme so manual
// theme overrides are reflected:
// const isDark = useColorScheme() === 'dark';
// sidebarContainer: { backgroundColor: isDark ? '#171717' : '#fff', ... }
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| height: '100%', | ||
| backgroundColor: colorScheme.get() === 'dark' ? '#171717' : '#fff', | ||
| shadowColor: colorScheme.get() === 'dark' ? '#262626' : '#e5e5e5', | ||
| backgroundColor: Appearance.getColorScheme() === 'dark' ? '#171717' : '#fff', |
There was a problem hiding this comment.
Code duplication: The boolean expression Appearance.getColorScheme() === 'dark' is redundantly evaluated across multiple style properties in NotificationDetail.tsx. Hoist this logic into a single isDark variable before StyleSheet.create to avoid drift and simplify maintenance.
Kody rule violation: Use computed/derived properties for repeated calculations
Prompt for LLM
File src/components/notifications/NotificationDetail.tsx:
Line 206:
Code duplication: The boolean expression `Appearance.getColorScheme() === 'dark'` is redundantly evaluated across multiple style properties in `NotificationDetail.tsx`. Hoist this logic into a single `isDark` variable before `StyleSheet.create` to avoid drift and simplify maintenance.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| height: '100%', | ||
| backgroundColor: colorScheme.get() === 'dark' ? '#171717' : '#fff', | ||
| shadowColor: colorScheme.get() === 'dark' ? '#262626' : '#e5e5e5', | ||
| backgroundColor: Appearance.getColorScheme() === 'dark' ? '#171717' : '#fff', |
There was a problem hiding this comment.
Code duplication: The expression Appearance.getColorScheme() === 'dark' is duplicated across numerous style properties in NotificationInbox.tsx and NotificationDetail.tsx. Extract this into a single isDark constant or helper function before referencing it in the ternary operations.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/components/notifications/NotificationInbox.tsx:
Line 446:
Code duplication: The expression `Appearance.getColorScheme() === 'dark'` is duplicated across numerous style properties in `NotificationInbox.tsx` and `NotificationDetail.tsx`. Extract this into a single `isDark` constant or helper function before referencing it in the ternary operations.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| type IAnimatedPressableProps = React.ComponentProps<typeof Pressable> & MotionComponentProps<typeof Pressable, ViewStyle, unknown, unknown, unknown>; | ||
|
|
||
| const AnimatedPressable = createMotionAnimatedComponent(Pressable) as React.ComponentType<IAnimatedPressableProps>; |
There was a problem hiding this comment.
Unsafe type cast: createMotionAnimatedComponent(Pressable) asserts a type without proper validation. Ensure safe type casting by utilizing pattern matching or verifying the result before usage.
Kody rule violation: Use safe type casting with as operator
Prompt for LLM
File src/components/ui/actionsheet/index.tsx:
Line 23:
Unsafe type cast: `createMotionAnimatedComponent(Pressable)` asserts a type without proper validation. Ensure safe type casting by utilizing pattern matching or verifying the result before usage.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }), | ||
| [] | ||
| ); | ||
| const exitingAnimation = parentAnchor === 'left' ? SlideOutLeft.duration(200) : parentAnchor === 'right' ? SlideOutRight.duration(200) : parentAnchor === 'top' ? SlideOutUp.duration(200) : SlideOutDown.duration(200); |
There was a problem hiding this comment.
Unguarded string literals: Anchor positions like 'left', 'right', and 'top' are hardcoded in long ternary chains across multiple files. Define an enum or a const tuple for anchor values to prevent typos and centralize maintenance.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/ui/drawer/index.tsx:
Line 171:
Unguarded string literals: Anchor positions like 'left', 'right', and 'top' are hardcoded in long ternary chains across multiple files. Define an enum or a const tuple for anchor values to prevent typos and centralize maintenance.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| useEffect(() => { | ||
| Appearance.setColorScheme(mode === 'system' ? 'unspecified' : mode); | ||
| }, [mode]); |
There was a problem hiding this comment.
Theme override bug: GluestackUIProvider's useEffect pins Appearance to the resolved value, preventing the app from following OS dark↔light switches when 'system' is selected. Gate the setColorScheme call so it only forces an override when mode !== 'system'.
// Only force the override when a concrete mode is requested;
// 'system' should leave Appearance unset so the OS value wins.
useEffect(() => {
if (mode !== 'system') {
Appearance.setColorScheme(mode);
} else {
Appearance.setColorScheme('unspecified');
}
}, [mode]);Prompt for LLM
File src/components/ui/gluestack-ui-provider/index.tsx:
Line 16 to 18:
Theme override bug: GluestackUIProvider's `useEffect` pins Appearance to the resolved value, preventing the app from following OS dark↔light switches when 'system' is selected. Gate the `setColorScheme` call so it only forces an override when `mode !== 'system'`.
Suggested Code:
// Only force the override when a concrete mode is requested;
// 'system' should leave Appearance unset so the OS value wins.
useEffect(() => {
if (mode !== 'system') {
Appearance.setColorScheme(mode);
} else {
Appearance.setColorScheme('unspecified');
}
}, [mode]);
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| import { Appearance, useColorScheme, View, type ViewProps } from 'react-native'; | ||
|
|
||
| import { config } from './config'; | ||
| export type ModeType = 'light' | 'dark' | 'system'; |
There was a problem hiding this comment.
Fragile type definition: ModeType is a hand-maintained string-literal union, risking drift with its inline usages. Derive the type from a runtime constant using typeof MODES[number] to establish a single source of truth.
Kody rule violation: Derive TypeScript types from validation schemas
Prompt for LLM
File src/components/ui/gluestack-ui-provider/index.tsx:
Line 7:
Fragile type definition: `ModeType` is a hand-maintained string-literal union, risking drift with its inline usages. Derive the type from a runtime constant using `typeof MODES[number]` to establish a single source of truth.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // drives the class-based `dark:` variant (see @custom-variant in global.css). | ||
| const osScheme = useColorScheme(); | ||
| const colorSchemeName: 'light' | 'dark' = mode === 'system' ? (osScheme === 'dark' ? 'dark' : 'light') : mode === 'dark' ? 'dark' : 'light'; | ||
| const resolvedScheme: 'light' | 'dark' = mode === 'system' ? (osScheme === 'dark' ? 'dark' : 'light') : mode; |
There was a problem hiding this comment.
String literal duplication: Shared literals 'light', 'dark', and 'system' are scattered inline across gluestack-ui-provider/index.tsx. Reference centralized constants or enum members instead of bare strings to prevent errors and maintain a single source of truth.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/components/ui/gluestack-ui-provider/index.tsx:
Line 14:
String literal duplication: Shared literals 'light', 'dark', and 'system' are scattered inline across `gluestack-ui-provider/index.tsx`. Reference centralized constants or enum members instead of bare strings to prevent errors and maintain a single source of truth.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }); | ||
| const StyledUIIcon = styled(UIIcon, { className: 'style' }); | ||
|
|
||
| type IIConProps = IPrimitiveIcon & VariantProps<typeof iconStyle> & React.ComponentPropsWithoutRef<typeof UIIcon>; |
There was a problem hiding this comment.
Typographical error: The type name IIConProps incorrectly capitalizes the 'C', violating naming conventions and reducing searchability. Rename it to IIconProps consistently across the codebase, including consumer files like video-feed-card.test.tsx.
Kody rule violation: Use proper naming conventions
Prompt for LLM
File src/components/ui/icon/index.tsx:
Line 28:
Typographical error: The type name `IIConProps` incorrectly capitalizes the 'C', violating naming conventions and reducing searchability. Rename it to `IIconProps` consistently across the codebase, including consumer files like `video-feed-card.test.tsx`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| }, | ||
| }} | ||
| entering={FadeIn.duration(200).easing(Easing.linear)} | ||
| exiting={FadeOut.duration(200).easing(Easing.linear)} |
There was a problem hiding this comment.
Unnamed magic number: The numeric literal 200 is used inline for animation durations across multiple components. Extract this value to a module-level constant, such as MODAL_ANIMATION_DURATION = 200, to improve maintainability and readability.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/components/ui/modal/index.tsx:
Line 93:
Unnamed magic number: The numeric literal `200` is used inline for animation durations across multiple components. Extract this value to a module-level constant, such as `MODAL_ANIMATION_DURATION = 200`, to improve maintainability and readability.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| applyColorScheme(theme as ColorSchemeType); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to load selected theme:', error); |
There was a problem hiding this comment.
Unstructured error logging: The catch block in use-selected-theme.tsx uses console.error with a plain message string and raw error object. Replace this with a structured logger call that includes the operation name and relevant identifiers, such as logger.error('loadSelectedTheme failed', { op: 'loadSelectedTheme', key: SELECTED_THEME, err: error }).
Kody rule violation: Include error context in structured logs
Prompt for LLM
File src/lib/hooks/use-selected-theme.tsx:
Line 58:
Unstructured error logging: The `catch` block in `use-selected-theme.tsx` uses `console.error` with a plain message string and raw error object. Replace this with a structured logger call that includes the operation name and relevant identifiers, such as `logger.error('loadSelectedTheme failed', { op: 'loadSelectedTheme', key: SELECTED_THEME, err: error })`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| // Helper function to get tag style based on notification type | ||
| const getTypeTagStyle = (type: string): StyleProp<ViewStyle> => { | ||
| // Helper function to get the themed tag style key based on notification type | ||
| type TypeTagStyleKey = 'typeTagDefault' | 'typeTagInfo' | 'typeTagSuccess' | 'typeTagWarning' | 'typeTagAlert'; |
There was a problem hiding this comment.
The type union duplicates property keys defined in createThemedStyles, risking drift where adding or renaming a key requires manual updates in both places without compiler detection. Declare a const tuple and derive the type (const TYPE_TAG_KEYS = ['typeTagDefault','typeTagInfo','typeTagSuccess','typeTagWarning','typeTagAlert'] as const; type TypeTagStyleKey = typeof TYPE_TAG_KEYS[number];), or introduce a string enum that serves as both the type and the value source.
Kody rule violation: Derive TypeScript types from validation schemas
Prompt for LLM
File src/components/notifications/NotificationDetail.tsx:
Line 179:
The type union duplicates property keys defined in createThemedStyles, risking drift where adding or renaming a key requires manual updates in both places without compiler detection. Declare a const tuple and derive the type (const TYPE_TAG_KEYS = ['typeTagDefault','typeTagInfo','typeTagSuccess','typeTagWarning','typeTagAlert'] as const; type TypeTagStyleKey = typeof TYPE_TAG_KEYS[number];), or introduce a string enum that serves as both the type and the value source.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| if (lowerType.includes('alert') || lowerType.includes('emergency')) { | ||
| return styles.typeTagAlert; | ||
| return 'typeTagAlert'; |
There was a problem hiding this comment.
Magic strings returned from the helper function are typo-prone and not refactor-safe; changing a key name silently desynchronizes call sites and the themed-styles object. Define a string enum (e.g. enum TypeTagStyleKey { Alert='typeTagAlert', Warning='typeTagWarning', Info='typeTagInfo', Success='typeTagSuccess', Default='typeTagDefault' }) and return the enum member instead of the raw string.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/components/notifications/NotificationDetail.tsx:
Line 185:
Magic strings returned from the helper function are typo-prone and not refactor-safe; changing a key name silently desynchronizes call sites and the themed-styles object. Define a string enum (e.g. enum TypeTagStyleKey { Alert='typeTagAlert', Warning='typeTagWarning', Info='typeTagInfo', Success='typeTagSuccess', Default='typeTagDefault' }) and return the enum member instead of the raw string.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| export const NotificationInbox = ({ isOpen, onClose }: NotificationInboxProps) => { | ||
| const { t } = useTranslation(); | ||
| const { colorScheme } = useColorScheme(); |
There was a problem hiding this comment.
Identical hook sequences duplicating useColorScheme() and React.useMemo appear in both NotificationRow (lines 87–88) and NotificationInbox (lines 142–143), violating DRY and increasing the maintenance surface. Extract a custom hook, e.g. function useThemedStyles() { const { colorScheme } = useColorScheme(); return React.useMemo(() => createThemedStyles(colorScheme === 'dark'), [colorScheme]); }, and call const themed = useThemedStyles(); in both components.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/components/notifications/NotificationInbox.tsx:
Line 142:
Identical hook sequences duplicating useColorScheme() and React.useMemo appear in both NotificationRow (lines 87–88) and NotificationInbox (lines 142–143), violating DRY and increasing the maintenance surface. Extract a custom hook, e.g. function useThemedStyles() { const { colorScheme } = useColorScheme(); return React.useMemo(() => createThemedStyles(colorScheme === 'dark'), [colorScheme]); }, and call const themed = useThemedStyles(); in both components.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
Pull Request Description
This PR upgrades the Resgrid IC application to Expo SDK 56, migrates the styling system to NativeWind v5 (with Tailwind CSS v4), updates Gluestack UI to v5, and bumps LiveKit to 2.10.
Key Changes
Expo SDK 56 Upgrade
expo-image,expo-sharing, andexpo-status-barpluginsFocusAwareStatusBarsimplified accordinglyNativeWind v5 + Tailwind CSS v4 Migration
jsxImportSource)global.cssrewritten with Tailwind v4@theme/@layersyntax and comprehensive light/dark design tokens driven by CSS custom properties and class-based dark mode (@custom-variant dark)cssInteropreplaced bystyled()across all components;withNativeWind→withNativewindin Metro config@tailwindcss/postcssAppearance.setColorScheme(native) and<html>class toggling (web) instead of NativeWind'scolorScheme.set()Gluestack UI v5 Migration
@gluestack-ui/*to@gluestack-ui/core/*/creator@gluestack-ui/nativewind-utilsto@gluestack-ui/utils/nativewind-utilsPrimitiveIcondefinitions; now usesUIIconfrom gluestack core wrapped withstyled()withStyleContextAndStatesandwithStateswrappersGluestackUIProvidersimplified to use class-based theming instead of inline style config objectsViewwithout the gluestack creator wrapperAnimation Library Migration
@legendapp/motion(Motion.View,AnimatePresence) replaced withreact-native-reanimated(entering/exitingprops) across Modal, Drawer, AlertDialog, Popover, Menu, and Toast componentsNavigation Consolidation
useFocusEffect,useIsFocused, and navigation re-exports consolidated from@react-navigation/nativetoexpo-routerNavigationContainerwrapper andcreateNavigationContainerRefusageAdditional Fixes & Cleanup
ActionsheetContentto use reactive window dimensions (fixes landscape rendering)shadow-sm→shadow-xs,rounded-sm→rounded-xstestIDsupport added (Box, Card, Heading, Text, HStack, VStack, Grid, Skeleton)numberOfLines/line-clamp support added to webTextcomponentMessageCommanderSheetfrom the command board@keyframesto react-native-webanimationKeyframes