Develop #193
Develop #193
Conversation
📝 WalkthroughWalkthroughCentralizes app-reset/logout cleanup and confirmation, adds proactive token refresh scheduling with network vs non-recoverable refresh error handling, renames protocol identifier Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Settings as Settings UI
participant Dialog as AlertDialog
participant Handler as handleLogoutConfirm
participant ResetService as AppResetService
participant Storage as Persisted Storage
participant Stores as Zustand Stores
participant AuthStore as Auth Store
participant SignOut as signOut()
User->>Settings: Tap "Logout"
Settings->>Dialog: Show confirmation
User->>Dialog: Confirm
Dialog->>Handler: handleLogoutConfirm()
Handler->>ResetService: clearAllAppData()
ResetService->>Storage: clearPersistedStorage()
ResetService->>Storage: clearAppStorageItems()
ResetService->>Stores: resetAllStores()
ResetService-->>Handler: resolved
Handler->>AuthStore: signOut()
AuthStore->>SignOut: clear tokens & navigate
SignOut-->>User: Redirect to login
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/api/common/client.tsx`:
- Around line 101-118: Replace the fragile message-based network check for
refreshError with an Axios-aware check: use axios.isAxiosError(refreshError) &&
!refreshError.response to determine network errors, keeping the same
logger.error/logger.warn branches and the call to
useAuthStore.getState().logout() for non-network failures; ensure
axios.isAxiosError is imported from 'axios' and remove the old
message.includes(...) logic while preserving existing log/context payloads.
In `@src/app/`(app)/settings.tsx:
- Around line 67-265: clearAllAppData misses awaiting the async LiveKit
disconnect and uses many hardcoded resets; update clearAllAppData to await
useLiveKitStore.getState().disconnectFromRoom() (replace the current un-awaited
call), and where possible call existing reset/clear methods or exported
initial-state helpers instead of inlining state objects (e.g., prefer
useStatusBottomSheetStore.getState().reset(),
useOfflineQueueStore.getState().clearAllEvents(), or exported initial state
constants from useCoreStore, useCallsStore, etc.); lastly, consider moving this
function into a reusable module (e.g., app-reset) for reuse and testing.
In `@src/components/contacts/contact-details-sheet.tsx`:
- Around line 128-131: The interactive row rendering when isActionable uses
Pressable but lacks accessibility metadata; update the Pressable (in the
isActionable branch that wraps content and handlePress) to include
accessibilityRole="button" and a descriptive accessibilityLabel (derive from the
row's visible label/value, e.g., use the same text shown in content or a prop
like label/value) and optionally accessibilityState={{ disabled: ... }} if the
action can be disabled, so screen readers expose it correctly.
- Around line 75-109: The handlePress function should wrap the
Linking.canOpenURL and Linking.openURL calls in a try/catch to log errors
(matching the error-handling pattern in src/lib/navigation.ts), and when action
=== 'address' implement a web fallback: if Linking.canOpenURL(nativeMapUrl) is
false or open fails, attempt opening the Google Maps web URL (default map URL)
as a fallback; also treat other open failures by logging the error and
optionally notifying the user. Additionally, update the Pressable that triggers
handlePress to include accessibility props (e.g., accessibilityRole="button" and
an accessibilityLabel describing the action/value such as `${action} ${value}`)
so screen readers get proper metadata. Ensure you reference and modify the
handlePress function and the Pressable component in contact-details-sheet.tsx
accordingly.
In `@src/stores/auth/store.tsx`:
- Around line 60-67: The token refresh timer is scheduled with setTimeout in the
login/refresh flow but the timeout ID isn't stored or cleared, causing leaks and
duplicate timers; add a state property (e.g., refreshTimeoutId) to the store,
when scheduling the timer save the returned ID via set({ refreshTimeoutId:
timeoutId }), always clear any existing timer before creating a new one
(clearTimeout(get().refreshTimeoutId)), and clear the stored ID and any pending
timer in the logout path (and when cancelling tokens) so
get().refreshAccessToken() cannot spawn stacked timeouts.
- Around line 146-199: The hydrate() method can schedule a token refresh that
may duplicate the refresh scheduled in onRehydrateStorage; add a single boolean
guard (e.g., isRefreshScheduled) either in the store state or as a module-scoped
variable and check it before calling setTimeout(() =>
get().refreshAccessToken(), ...) or before invoking get().refreshAccessToken();
set the flag when scheduling and clear it when the refresh completes/fails, or
alternatively remove the scheduling from hydrate() and keep refresh scheduling
only in onRehydrateStorage to ensure refreshAccessToken() is invoked exactly
once during initialization.
🧹 Nitpick comments (10)
src/services/__tests__/push-notification.test.ts (1)
438-451: Ensure fake timers are always restored.
If the test throws beforejest.useRealTimers(), later tests may run under fake timers. Atry/finallykeeps this isolated.♻️ Suggested hardening
it('should store listener handles on initialization', async () => { - jest.useFakeTimers(); - - await pushNotificationService.initialize(); - - // Run all timers to trigger getInitialNotification which is called in setTimeout - jest.runAllTimers(); - - // Verify listeners were registered - expect(mockOnMessage).toHaveBeenCalled(); - expect(mockOnNotificationOpenedApp).toHaveBeenCalled(); - expect(mockGetInitialNotification).toHaveBeenCalled(); - expect(mockSetBackgroundMessageHandler).toHaveBeenCalled(); - - jest.useRealTimers(); + jest.useFakeTimers(); + try { + await pushNotificationService.initialize(); + + // Run all timers to trigger getInitialNotification which is called in setTimeout + jest.runAllTimers(); + + // Verify listeners were registered + expect(mockOnMessage).toHaveBeenCalled(); + expect(mockOnNotificationOpenedApp).toHaveBeenCalled(); + expect(mockGetInitialNotification).toHaveBeenCalled(); + expect(mockSetBackgroundMessageHandler).toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } });src/components/settings/unit-selection-bottom-sheet.tsx (3)
150-151: Use the gluestack-ui ScrollView wrapper for consistent theming.Prefer the UI ScrollView component (exported from components/ui) over react-native’s ScrollView to keep styling and theming consistent across the app. Adjust the import path to the actual UI export in your codebase.
♻️ Proposed change
-import { ScrollView } from 'react-native'; +import { ScrollView } from '../ui/scroll-view'; // or '../ui' if re-exported thereAs per coding guidelines, use gluestack-ui components when available.
158-161: Avoid inline onPress handlers in the list.Consider extracting a memoized UnitItem component (or a stable handler factory) so each row can own a stable onPress handler, reducing re-renders in the list. As per coding guidelines, avoid anonymous functions in renderItem or event handlers.
173-173: Use a ternary for the check icon render.♻️ Proposed change
- {activeUnit?.UnitId === unit.UnitId && <Check size={20} className="text-primary-600" />} + {activeUnit?.UnitId === unit.UnitId ? <Check size={20} className="text-primary-600" /> : null}As per coding guidelines, use the conditional operator (?:) instead of && for conditional rendering.
src/components/calls/__tests__/close-call-bottom-sheet.test.tsx (1)
43-67: Consider typing mock props instead ofany. It keeps tests aligned with the TS guidelines and prevents accidental prop misuse.♻️ Suggested typing pattern
-jest.mock('@/components/ui/actionsheet', () => ({ - Actionsheet: ({ children, isOpen }: any) => { +type ActionsheetProps = { children?: React.ReactNode; isOpen?: boolean }; +type ChildrenOnly = { children?: React.ReactNode }; +jest.mock('@/components/ui/actionsheet', () => ({ + Actionsheet: ({ children, isOpen }: ActionsheetProps) => { const { View } = require('react-native'); return isOpen ? <View testID="actionsheet">{children}</View> : null; }, - ActionsheetBackdrop: () => null, - ActionsheetContent: ({ children }: any) => { + ActionsheetBackdrop: () => null, + ActionsheetContent: ({ children }: ChildrenOnly) => { const { View } = require('react-native'); return <View testID="actionsheet-content">{children}</View>; }, ActionsheetDragIndicator: () => null, - ActionsheetDragIndicatorWrapper: ({ children }: any) => { + ActionsheetDragIndicatorWrapper: ({ children }: ChildrenOnly) => { const { View } = require('react-native'); return <View>{children}</View>; }, })); -jest.mock('react-native-keyboard-controller', () => ({ - KeyboardAwareScrollView: ({ children }: any) => { +jest.mock('react-native-keyboard-controller', () => ({ + KeyboardAwareScrollView: ({ children }: ChildrenOnly) => { const { View } = require('react-native'); return <View>{children}</View>; }, }));As per coding guidelines, please avoid
anyin TS/TSX where possible.src/components/status/status-bottom-sheet.tsx (1)
651-666: Prefer ternary over&&for conditional rendering. This aligns with the project TSX guideline for rendering conditions.♻️ Suggested change
- {isSubmitting && <Spinner size="small" color="white" />} + {isSubmitting ? <Spinner size="small" color="white" /> : null}- {isSubmitting && <Spinner size="small" color="white" />} + {isSubmitting ? <Spinner size="small" color="white" /> : null}As per coding guidelines, conditional rendering should use the ternary operator rather than
&&.Also applies to: 698-706
src/components/calls/close-call-bottom-sheet.tsx (1)
98-99: Move inline styles toStyleSheet.createfor consistency.Inline styles on the scroll view should be centralized to align with the styling guideline for non-gluestack components.
♻️ Suggested refactor
-import React, { useState } from 'react'; +import React, { useState } from 'react'; +import { StyleSheet } from 'react-native'; @@ interface CloseCallBottomSheetProps { @@ } +const styles = StyleSheet.create({ + scrollView: { width: '100%' }, + scrollContent: { flexGrow: 1, paddingBottom: 80 }, +}); + export const CloseCallBottomSheet: React.FC<CloseCallBottomSheetProps> = ({ isOpen, onClose, callId, isLoading = false }) => { @@ - <KeyboardAwareScrollView style={{ width: '100%' }} contentContainerStyle={{ flexGrow: 1, paddingBottom: 80 }} showsVerticalScrollIndicator={false} keyboardShouldPersistTaps="handled" bottomOffset={120}> + <KeyboardAwareScrollView + style={styles.scrollView} + contentContainerStyle={styles.scrollContent} + showsVerticalScrollIndicator={false} + keyboardShouldPersistTaps="handled" + bottomOffset={120} + >As per coding guidelines, avoid inline styling when a StyleSheet is preferred.
src/components/protocols/protocol-card.tsx (1)
19-19: Avoid inline onPress handler to reduce re-renders.Consider memoizing the handler to align with the guideline against anonymous event handlers.
♻️ Suggested refactor
-import { Pressable } from 'react-native'; +import { useCallback } from 'react'; +import { Pressable } from 'react-native'; @@ export const ProtocolCard: React.FC<ProtocolCardProps> = ({ protocol, onPress }) => { + const handlePress = useCallback(() => onPress(protocol.ProtocolId), [onPress, protocol.ProtocolId]); + return ( - <Pressable onPress={() => onPress(protocol.ProtocolId)} testID={`protocol-card-${protocol.ProtocolId}`}> + <Pressable onPress={handlePress} testID={`protocol-card-${protocol.ProtocolId}`}>As per coding guidelines, avoid anonymous functions in event handlers.
src/app/(app)/settings.tsx (1)
362-382: LGTM - AlertDialog implementation looks good.The confirmation dialog properly uses gluestack-ui components with appropriate actions (outline/secondary for cancel, negative for destructive logout action).
Minor suggestion: Consider adding
space="sm"or margin toAlertDialogFooterfor button spacing if not handled by the component internally.src/stores/auth/store.tsx (1)
93-144: Enhanced refresh error handling is well-designed.The distinction between network errors (retry) and non-recoverable errors (logout) is appropriate. The 30-second retry delay for network errors provides resilience without overwhelming the server.
Note: The network error detection via string matching is pragmatic but may miss some error variants. Consider also checking for
error.codevalues likeENOTFOUNDor axios-specific error properties if using axios.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
expo-env.d.tssrc/api/common/client.tsxsrc/app/(app)/__tests__/protocols.test.tsxsrc/app/(app)/protocols.tsxsrc/app/(app)/settings.tsxsrc/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/components/calls/close-call-bottom-sheet.tsxsrc/components/contacts/contact-details-sheet.tsxsrc/components/protocols/__tests__/protocol-card.test.tsxsrc/components/protocols/__tests__/protocol-details-sheet.test.tsxsrc/components/protocols/protocol-card.tsxsrc/components/protocols/protocol-details-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/sidebar/sidebar.tsxsrc/components/status/status-bottom-sheet.tsxsrc/models/v4/callProtocols/callProtocolsResultData.tssrc/services/__tests__/push-notification.test.tssrc/stores/auth/store.tsxsrc/stores/protocols/__tests__/store.test.tssrc/translations/ar.jsonsrc/translations/en.jsonsrc/translations/es.json
🧰 Additional context used
📓 Path-based instructions (9)
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/translations/ar.jsonsrc/app/(app)/__tests__/protocols.test.tsxsrc/components/protocols/__tests__/protocol-card.test.tsxsrc/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/api/common/client.tsxsrc/components/protocols/__tests__/protocol-details-sheet.test.tsxsrc/components/sidebar/sidebar.tsxsrc/app/(app)/protocols.tsxsrc/app/(app)/settings.tsxsrc/services/__tests__/push-notification.test.tssrc/translations/en.jsonsrc/models/v4/callProtocols/callProtocolsResultData.tssrc/translations/es.jsonsrc/components/protocols/protocol-details-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/protocols/protocol-card.tsxsrc/stores/protocols/__tests__/store.test.tssrc/components/calls/close-call-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/stores/auth/store.tsxexpo-env.d.tssrc/components/contacts/contact-details-sheet.tsx
src/translations/**
📄 CodeRabbit inference engine (.cursorrules)
Store translation dictionaries in src/translations
Files:
src/translations/ar.jsonsrc/translations/en.jsonsrc/translations/es.json
src/translations/**/*.{json,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store translation dictionaries in src/translations
Files:
src/translations/ar.jsonsrc/translations/en.jsonsrc/translations/es.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/app/(app)/__tests__/protocols.test.tsxsrc/components/protocols/__tests__/protocol-card.test.tsxsrc/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/api/common/client.tsxsrc/components/protocols/__tests__/protocol-details-sheet.test.tsxsrc/components/sidebar/sidebar.tsxsrc/app/(app)/protocols.tsxsrc/app/(app)/settings.tsxsrc/services/__tests__/push-notification.test.tssrc/models/v4/callProtocols/callProtocolsResultData.tssrc/components/protocols/protocol-details-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/protocols/protocol-card.tsxsrc/stores/protocols/__tests__/store.test.tssrc/components/calls/close-call-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/stores/auth/store.tsxexpo-env.d.tssrc/components/contacts/contact-details-sheet.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for React component names (e.g., UserProfile, ChatScreen)
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo() for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Optimize images using react-native-fast-image
Use React Navigation for navigation and deep linking with best practices
Wrap all user-facing text in t() from react-i18next
Use react-hook-form for form handling
Use@rnmapbox/mapsfor maps or vehicle navigation
Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Use the conditional operator (?:) for conditional rendering instead of &&
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect/useState and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers
Optimize image handling using react-native-fast-image
Wrap all user-facing text in t() from react-i18next
Ensure support for dark mode and light mode
Ensure the app is accessible following WCAG for mobile
Use react-hook-form for form handling
Use@rnmapbox/mapsfor maps and navigation
Use lucide-react-native for icons directly in markup; do not us...
Files:
src/app/(app)/__tests__/protocols.test.tsxsrc/components/protocols/__tests__/protocol-card.test.tsxsrc/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/api/common/client.tsxsrc/components/protocols/__tests__/protocol-details-sheet.test.tsxsrc/components/sidebar/sidebar.tsxsrc/app/(app)/protocols.tsxsrc/app/(app)/settings.tsxsrc/components/protocols/protocol-details-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/protocols/protocol-card.tsxsrc/components/calls/close-call-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/stores/auth/store.tsxsrc/components/contacts/contact-details-sheet.tsx
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit inference engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/app/(app)/__tests__/protocols.test.tsxsrc/components/protocols/__tests__/protocol-card.test.tsxsrc/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/components/protocols/__tests__/protocol-details-sheet.test.tsxsrc/services/__tests__/push-notification.test.tssrc/stores/protocols/__tests__/store.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/app/(app)/__tests__/protocols.test.tsxsrc/components/protocols/__tests__/protocol-card.test.tsxsrc/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/components/protocols/__tests__/protocol-details-sheet.test.tsxsrc/services/__tests__/push-notification.test.tssrc/stores/protocols/__tests__/store.test.ts
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/app/(app)/__tests__/protocols.test.tsxsrc/components/protocols/__tests__/protocol-card.test.tsxsrc/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/api/common/client.tsxsrc/components/protocols/__tests__/protocol-details-sheet.test.tsxsrc/components/sidebar/sidebar.tsxsrc/app/(app)/protocols.tsxsrc/app/(app)/settings.tsxsrc/components/protocols/protocol-details-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/protocols/protocol-card.tsxsrc/components/calls/close-call-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/stores/auth/store.tsxsrc/components/contacts/contact-details-sheet.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/app/(app)/__tests__/protocols.test.tsxsrc/components/protocols/__tests__/protocol-card.test.tsxsrc/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/components/protocols/__tests__/protocol-details-sheet.test.tsxsrc/services/__tests__/push-notification.test.tssrc/stores/protocols/__tests__/store.test.ts
🧠 Learnings (12)
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/components/calls/__tests__/close-call-bottom-sheet.test.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated components
Applied to files:
src/components/calls/__tests__/close-call-bottom-sheet.test.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to {components/ui/**/*.{ts,tsx},**/*.tsx} : Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Applied to files:
src/components/calls/__tests__/close-call-bottom-sheet.test.tsxsrc/components/sidebar/sidebar.tsxsrc/app/(app)/settings.tsxsrc/components/calls/close-call-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize
Applied to files:
src/app/(app)/protocols.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Applied to files:
src/app/(app)/protocols.tsxsrc/components/settings/unit-selection-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use React Navigation for navigation and deep linking with best practices
Applied to files:
src/components/calls/close-call-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/components/contacts/contact-details-sheet.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{ts,tsx} : Use React Navigation for handling navigation and deep linking
Applied to files:
src/components/calls/close-call-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{ts,tsx} : Use react-native-mmkv for local storage
Applied to files:
src/components/status/status-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{ts,tsx} : Use Expo's SecureStore for sensitive data
Applied to files:
expo-env.d.ts
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{ts,tsx} : Use Expo SecureStore for sensitive data
Applied to files:
expo-env.d.ts
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use gluestack-ui icon component
Applied to files:
src/components/contacts/contact-details-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Applied to files:
src/components/contacts/contact-details-sheet.tsx
🧬 Code graph analysis (12)
src/app/(app)/__tests__/protocols.test.tsx (1)
src/components/ui/index.tsx (1)
Pressable(18-18)
src/components/calls/__tests__/close-call-bottom-sheet.test.tsx (1)
src/components/ui/index.tsx (1)
View(18-18)
src/api/common/client.tsx (1)
src/lib/logging/index.tsx (1)
logger(80-80)
src/components/sidebar/sidebar.tsx (2)
src/components/ui/button/index.tsx (1)
ButtonText(334-334)src/lib/utils.ts (1)
invertColor(38-62)
src/app/(app)/protocols.tsx (1)
src/components/protocols/protocol-card.tsx (1)
ProtocolCard(17-38)
src/app/(app)/settings.tsx (5)
src/lib/storage/app.tsx (3)
removeActiveUnitId(21-21)removeActiveCallId(32-32)removeDeviceUuid(43-43)src/lib/storage/index.tsx (1)
storage(5-5)src/stores/status/store.ts (1)
useStatusBottomSheetStore(48-109)src/stores/app/loading-store.ts (1)
useLoadingStore(25-39)src/components/ui/button/index.tsx (2)
Button(334-334)ButtonText(334-334)
src/services/__tests__/push-notification.test.ts (1)
src/services/push-notification.ts (1)
pushNotificationService(504-504)
src/components/protocols/protocol-details-sheet.tsx (1)
src/services/aptabase.service.ts (1)
trackEvent(36-69)
src/components/settings/unit-selection-bottom-sheet.tsx (1)
src/components/ui/index.tsx (1)
ScrollView(18-18)
src/components/status/status-bottom-sheet.tsx (2)
src/components/ui/button/index.tsx (2)
Button(334-334)ButtonText(334-334)src/lib/utils.ts (1)
invertColor(38-62)
src/stores/auth/store.tsx (4)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)src/lib/auth/api.tsx (1)
refreshTokenRequest(60-82)src/lib/storage/index.tsx (1)
setItem(23-25)src/lib/auth/types.tsx (2)
AuthResponse(11-18)ProfileModel(25-37)
src/components/contacts/contact-details-sheet.tsx (3)
src/components/ui/index.tsx (2)
View(18-18)Pressable(18-18)src/components/ui/icon/index.tsx (3)
MailIcon(672-672)PhoneIcon(760-760)GlobeIcon(525-525)src/components/ui/icon/index.web.tsx (3)
MailIcon(702-702)PhoneIcon(790-790)GlobeIcon(555-555)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (29)
src/services/__tests__/push-notification.test.ts (1)
101-112: LGTM: Notifee event mocks are wired correctly.expo-env.d.ts (1)
3-3: No functional change; looks fine.src/components/sidebar/sidebar.tsx (1)
70-86: Layout tweak is solid. The full-width button plus single-line text should prevent wrap/overflow issues.src/components/status/status-bottom-sheet.tsx (2)
508-516: Cancel action on status step is a good UX improvement.
621-636: Consistent previous/cancel + next layout looks good.src/components/calls/close-call-bottom-sheet.tsx (2)
91-91: Confirm ActionsheetsnapPointsunits match the wrapper API.If the Actionsheet wrapper expects percentage strings (e.g.,
'90%') or pixel values, a bare90could render incorrectly. Please confirm the accepted type in@/components/ui/actionsheet.
4-6: Move inline styles fromKeyboardAwareScrollViewtoStyleSheet.create().The
react-native-keyboard-controllerdependency is correctly declared in package.json and configured. However, inline styles on lines 98 should be extracted toStyleSheet.create()per coding guidelines:<KeyboardAwareScrollView style={{ width: '100%' }} contentContainerStyle={{ flexGrow: 1, paddingBottom: 80 }} ...>Move these styles to a StyleSheet object at the bottom of the file instead.
⛔ Skipped due to learnings
Learnt from: CR Repo: Resgrid/Unit PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-12T03:34:25.992Z Learning: Applies to {components/ui/**/*.{ts,tsx},**/*.tsx} : Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-componentsLearnt from: CR Repo: Resgrid/Unit PR: 0 File: .cursorrules:0-0 Timestamp: 2025-08-12T03:33:40.238Z Learning: Applies to **/*.tsx : Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-componentsLearnt from: CR Repo: Resgrid/Unit PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-12T03:34:25.992Z Learning: Applies to **/*.tsx : Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSizesrc/translations/en.json (1)
534-574: Translation additions look good.New settings keys align with the logout confirmation flow.
src/translations/ar.json (1)
534-574: Arabic translations look good.Keys are consistent with the settings flow additions.
src/translations/es.json (1)
534-574: Spanish translations look good.Keys are aligned with the new logout confirmation copy.
src/models/v4/callProtocols/callProtocolsResultData.ts (1)
2-2: Verify API payload/mapping forProtocolId.Renaming the public field is fine, but please confirm the API response uses
ProtocolId(or that a mapper translates legacyIdintoProtocolId) so IDs don’t end up undefined.src/stores/protocols/__tests__/store.test.ts (1)
15-51: ProtocolId fixture update looks good.Tests now align with the renamed data shape.
src/app/(app)/__tests__/protocols.test.tsx (2)
31-36: ProtocolCard mock now keys by ProtocolId.Keeps testIDs and onPress IDs consistent with the new model field.
108-182: Protocol fixtures aligned to ProtocolId.Consistent with the renamed property across the suite.
src/components/protocols/__tests__/protocol-card.test.tsx (2)
22-77: Fixture updates to ProtocolId look good.Keeps tests aligned with the model rename.
222-224: Empty ProtocolId edge case retained.Good coverage for fallback behavior.
src/components/protocols/protocol-details-sheet.tsx (5)
25-25: Selection now keyed by ProtocolId.Consistent with the updated data model.
29-37: Analytics payload uses ProtocolId.Keeps tracking consistent with the new identifier.
41-47: Graceful empty-selection Actionsheet.Nice fallback to avoid null renders while still allowing dismissal.
55-55: Keyed ActionsheetContent looks good.Forces remount on protocol changes as intended.
89-89: Keyed WebView ensures content refresh.Helps reinitialize the WebView when switching protocols.
src/app/(app)/protocols.tsx (3)
42-47: LGTM - Clean callback extraction.The
handleProtocolPresscallback is correctly memoized withuseCallbackand properly includesselectProtocolin the dependency array. This pattern enables passing a stable reference to child components.
79-80: LGTM - Correct migration to ProtocolId.The
keyExtractorproperly usesitem.ProtocolIdwith an appropriate fallback, andrenderItemcorrectly passes the memoized handler.
84-85: The custom FlatList component uses Shopify FlashList, not standard React Native FlatList.
estimatedItemSizeis a valid and recommended prop for FlashList (v1) to provide size hints and reduce blank cells during rendering. This is correct.
extraData={handleProtocolPress}is unnecessary sincehandleProtocolPressis already memoized withuseCallback(line 42). Removing it would be a minor optimization.Do not apply the suggested diff. The proposed props (
removeClippedSubviews,maxToRenderPerBatch,windowSize) are React Native FlatList-specific and do not apply to FlashList. FlashList has different optimization strategies. Consider these FlashList-appropriate alternatives instead:
- Provide
getItemTypeif you have heterogeneous item layouts to improve view recycling- Ensure
estimatedItemSizeis accurate to your actual item heights- Verify that item components and
renderItemare properly memoized to avoid unnecessary re-rendersLikely an incorrect or invalid review comment.
src/components/protocols/__tests__/protocol-details-sheet.test.tsx (3)
77-114: LGTM - Test fixtures correctly updated to use ProtocolId.The mock protocol data is properly updated to reflect the
Id→ProtocolIdrename in the data model. The test data structure is comprehensive and covers both full and minimal protocol scenarios.
415-432: LGTM - Minimal protocol test fixture updated.The
minimalProtocolfixture correctly usesProtocolIdand properly tests the scenario where optional fields are empty.
514-519: LGTM - Second protocol test fixture updated.The spread pattern with
ProtocolIdoverride is correct for creating the test variant.src/app/(app)/settings.tsx (1)
267-282: LGTM - Logout confirmation flow is well-structured.The sequence is correct: close dialog → track analytics → clear data → sign out. Dependencies in
useCallbackare properly declared.src/stores/auth/store.tsx (1)
228-266: onRehydrateStorage implementation is thorough.Good handling of both scenarios:
- Previously signed-in users with valid tokens
- Users with refresh tokens but non-signed-in status
The 2-second delay allows app initialization before refresh attempts.
However, ensure this doesn't conflict with the
hydrate()function to avoid duplicate refresh scheduling (see previous comment).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/react-native-cicd.yml:
- Around line 322-328: The grep -v commands that filter cleaned_body (the lines
using grep -v '✏️ Tip: You can customize this high-level summary...' and the two
coderabbit HTML comment patterns) can fail under set -eo pipefail when they
output no lines; append "|| true" to each of those grep -v pipelines so the
commands never return a non-zero exit status and the script continues gracefully
while still filtering cleaned_body.
In `@src/components/contacts/contact-details-sheet.tsx`:
- Around line 159-171: The accessibility hints currently fall back to English
because the translation keys used in contact-details-sheet.tsx
(accessibilityHint using t(`accessibility.action.${action}`)) are missing; add
the keys "accessibility.action.email", "accessibility.action.phone",
"accessibility.action.address", and "accessibility.action.website" with the
provided English strings to src/translations/en.json and add appropriate Spanish
and Arabic translations to src/translations/es.json and src/translations/ar.json
so t(...) resolves in each locale instead of relying on the defaultValue
fallback.
In `@src/services/app-reset.service.ts`:
- Around line 218-228: Wrap each async cleanup call with its own try-catch (and
set the store state in a finally or after the catch) so one failing operation
doesn't prevent the others; specifically, call await
liveKitState.disconnectFromRoom() inside a try that catches/logs the error
(reference disconnectFromRoom and useLiveKitStore) and ensure
useLiveKitStore.setState(INITIAL_LIVEKIT_STATE) still runs, and do the same for
await audioStreamState.cleanup() (reference cleanup and useAudioStreamStore)
with its own try-catch and guaranteed
useAudioStreamStore.setState(INITIAL_AUDIO_STREAM_STATE) execution.
🧹 Nitpick comments (12)
.vscode/settings.json (1)
49-68: Confirm shared intent for editor theme overrides.
These UI color overrides affect every contributor’s VSCode workspace. If the goal isn’t team-wide standardization, consider moving them to personal settings (or a local, git-ignored settings file) to avoid overriding individual preferences.src/components/status/status-bottom-sheet.tsx (1)
685-724: Consider extracting inline styles to StyleSheet.The
KeyboardAwareScrollViewwrapping is a good improvement for keyboard handling. However, inline styles are used here while the other file (close-call-bottom-sheet.tsx) usesStyleSheet.create()for the same pattern.♻️ Suggested refactor for consistency
+const styles = StyleSheet.create({ + scrollView: { + width: '100%', + }, + scrollViewContent: { + flexGrow: 1, + paddingBottom: 80, + }, +}); // Then in the component: -<KeyboardAwareScrollView style={{ width: '100%' }} contentContainerStyle={{ flexGrow: 1, paddingBottom: 80 }} showsVerticalScrollIndicator={false} keyboardShouldPersistTaps="handled" bottomOffset={120}> +<KeyboardAwareScrollView style={styles.scrollView} contentContainerStyle={styles.scrollViewContent} showsVerticalScrollIndicator={false} keyboardShouldPersistTaps="handled" bottomOffset={120}>src/components/settings/unit-selection-bottom-sheet.tsx (4)
36-42: Remove redundantkeyprop inside UnitItem.The
keyprop on line 37 inside theUnitItemcomponent is ineffective. React'skeymust be specified at the call site (in the.map()at line 194), not inside the component. The key at line 194 is correct; this one can be removed.Suggested fix
<ActionsheetItem - key={unit.UnitId} onPress={handlePress} disabled={isLoading} className={isSelected ? 'data-[checked=true]:bg-background-100' : ''} testID={`unit-item-${unit.UnitId}`} >
72-81: Consider showing a toast on fetch failure.When
fetchUnits()fails, the error is logged but the user isn't notified. Consider adding a toast to inform the user that loading units failed, improving the user experience.Suggested enhancement
React.useEffect(() => { if (isOpen && units.length === 0) { fetchUnits().catch((error) => { logger.error({ message: 'Failed to fetch units', context: { error }, }); + showToast('error', t('settings.fetch_units_failed')); }); } - }, [isOpen, units.length, fetchUnits]); + }, [isOpen, units.length, fetchUnits, showToast, t]);
139-143: Remove unreachable outer try-catch block.The outer
catchblock (lines 139-143) is unreachable because the inner try-catch (lines 112-138) already handles all exceptions from the async operations, and the code between the innerfinallyand outercatchcannot throw. This is dead code.Suggested fix
const handleUnitSelection = React.useCallback( async (unit: UnitResultData) => { if (isLoading || isProcessingRef.current) { return; } if (activeUnit?.UnitId === unit.UnitId) { logger.info({ message: 'Same unit already selected, closing modal', context: { unitId: unit.UnitId }, }); handleClose(); return; } - try { isProcessingRef.current = true; setIsLoading(true); let hasError = false; try { await setActiveUnit(unit.UnitId); await useRolesStore.getState().fetchRolesForUnit(unit.UnitId); logger.info({ message: 'Active unit updated successfully', context: { unitId: unit.UnitId, unitName: unit.Name }, }); showToast('success', t('settings.unit_selected_successfully', { unitName: unit.Name })); } catch (error) { hasError = true; logger.error({ message: 'Failed to update active unit', context: { error, unitId: unit.UnitId, unitName: unit.Name }, }); showToast('error', t('settings.unit_selection_failed')); } finally { setIsLoading(false); isProcessingRef.current = false; if (!hasError) { handleClose(); } } - } catch (outerError) { - // This should not happen, but just in case - setIsLoading(false); - isProcessingRef.current = false; - } }, [setActiveUnit, handleClose, isLoading, activeUnit, showToast, t] );
162-173: Use ternary operator for conditional rendering per coding guidelines.The coding guidelines specify using
?:instead of&&for conditional rendering. This applies to the current selection box.Suggested fix
{/* Current Selection */} - {activeUnit && ( + {activeUnit ? ( <Box className="rounded-lg border border-outline-200 bg-background-50 p-3"> <VStack space="xs"> <Text size="sm" className="font-medium text-typography-900"> {t('settings.current_unit')} </Text> <Text size="sm" className="text-typography-600"> {activeUnit.Name} </Text> </VStack> </Box> - )} + ) : null}Based on coding guidelines, conditional rendering should use the conditional operator (
?:) instead of&&.src/components/contacts/contact-details-sheet.tsx (1)
359-367: Consider adding actions for social media fields.The website field correctly uses
action="website", but other social media fields (Twitter, Instagram, etc.) remain non-actionable. If these fields store usernames (as suggested bylinkPrefix="@"), consider addingaction="website"with URL construction for each platform in a future enhancement.src/services/__tests__/app-reset.service.test.ts (1)
211-374: Consider extracting expected state shapes or using snapshot tests.The initial state shape assertions are comprehensive but tightly coupled to implementation details. If state shapes change in the service, these tests require parallel updates.
Consider either:
- Using Jest snapshot tests:
expect(INITIAL_CORE_STATE).toMatchSnapshot()- Testing only critical fields rather than full shape equality
This is a nice-to-have improvement and not blocking.
src/stores/auth/store.tsx (3)
144-168: Network error detection is fragile.String matching on error messages (
'Network Error','timeout', etc.) is brittle and may miss error types or break if Axios changes its error messages. Consider checking error properties or status codes for more reliable detection.♻️ Suggested approach using Axios error properties
- const isNetworkError = error instanceof Error && (error.message.includes('Network Error') || error.message.includes('timeout') || error.message.includes('ECONNREFUSED') || error.message.includes('ETIMEDOUT')); + import axios from 'axios'; + // ... + const isNetworkError = axios.isAxiosError(error) && (!error.response || error.code === 'ECONNABORTED' || error.code === 'ERR_NETWORK');This checks if:
- It's an Axios error with no response (network failure)
- The error code indicates connection abort or network error
273-277: Consider extracting the initialization delay as a named constant.The 2000ms delay appears twice and represents the same concept (app initialization time). Extract it as a named constant for clarity and maintainability.
♻️ Suggested refactor
// At the top of the file or in a constants file const APP_INITIALIZATION_DELAY_MS = 2000; // Then use it in both places: const timeoutId = setTimeout(() => { useAuthStore.getState().refreshAccessToken(); }, APP_INITIALIZATION_DELAY_MS);Also applies to: 294-297
171-221: Remove or clarify the deadhydrate()method.The
hydrate()method is commented out at the call site insrc/app/_layout.tsx(//useAuth().hydrate()), making it unused code. Token refresh is currently handled entirely byonRehydrateStorageduring Zustand persist middleware initialization. Ifhydrate()will not be used, remove it to reduce code clutter. If it's intended for future use or manual hydration scenarios, document that assumption and ensure the refresh scheduling logic inonRehydrateStorageremains the only entry point for refresh scheduling to avoid duplicate refresh attempts.src/services/app-reset.service.ts (1)
51-64: Usingas never[]hides type mismatches.The
as never[]cast for empty arrays is a type escape hatch that bypasses TypeScript's type checking. If the actual store expects a different array element type, this won't catch the mismatch at compile time.Consider either:
- Importing the actual initial state from each store
- Using proper generic types that match the store's expected types
♻️ Suggested approach
// Option 1: Import from stores (preferred - single source of truth) import { initialState as initialCallsState } from '@/stores/calls/store'; // Option 2: Define with proper types import type { Call, CallPriority, CallType } from '@/models/...'; export const INITIAL_CALLS_STATE = { calls: [] as Call[], callPriorities: [] as CallPriority[], callTypes: [] as CallType[], isLoading: false, error: null, };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/react-native-cicd.yml.vscode/settings.jsonsrc/api/common/client.tsxsrc/app/(app)/settings.tsxsrc/components/calls/close-call-bottom-sheet.tsxsrc/components/contacts/contact-details-sheet.tsxsrc/components/protocols/protocol-card.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/lib/auth/types.tsxsrc/services/__tests__/app-reset.service.test.tssrc/services/app-reset.service.tssrc/stores/auth/store.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/components/contacts/contact-details-sheet.tsxsrc/app/(app)/settings.tsxsrc/api/common/client.tsxsrc/services/__tests__/app-reset.service.test.tssrc/components/calls/close-call-bottom-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/lib/auth/types.tsxsrc/services/app-reset.service.tssrc/components/protocols/protocol-card.tsxsrc/stores/auth/store.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for React component names (e.g., UserProfile, ChatScreen)
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo() for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Optimize images using react-native-fast-image
Use React Navigation for navigation and deep linking with best practices
Wrap all user-facing text in t() from react-i18next
Use react-hook-form for form handling
Use@rnmapbox/mapsfor maps or vehicle navigation
Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Use the conditional operator (?:) for conditional rendering instead of &&
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect/useState and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers
Optimize image handling using react-native-fast-image
Wrap all user-facing text in t() from react-i18next
Ensure support for dark mode and light mode
Ensure the app is accessible following WCAG for mobile
Use react-hook-form for form handling
Use@rnmapbox/mapsfor maps and navigation
Use lucide-react-native for icons directly in markup; do not us...
Files:
src/components/contacts/contact-details-sheet.tsxsrc/app/(app)/settings.tsxsrc/api/common/client.tsxsrc/components/calls/close-call-bottom-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/lib/auth/types.tsxsrc/components/protocols/protocol-card.tsxsrc/stores/auth/store.tsx
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/components/contacts/contact-details-sheet.tsxsrc/app/(app)/settings.tsxsrc/api/common/client.tsxsrc/services/__tests__/app-reset.service.test.tssrc/components/calls/close-call-bottom-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/lib/auth/types.tsxsrc/services/app-reset.service.tssrc/components/protocols/protocol-card.tsxsrc/stores/auth/store.tsx
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/components/contacts/contact-details-sheet.tsxsrc/app/(app)/settings.tsxsrc/api/common/client.tsxsrc/components/calls/close-call-bottom-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/lib/auth/types.tsxsrc/components/protocols/protocol-card.tsxsrc/stores/auth/store.tsx
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit inference engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/services/__tests__/app-reset.service.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/services/__tests__/app-reset.service.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/services/__tests__/app-reset.service.test.ts
🧠 Learnings (15)
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Ensure the app is accessible following WCAG for mobile
Applied to files:
src/components/contacts/contact-details-sheet.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{ts,tsx} : Handle errors gracefully and provide user feedback
Applied to files:
src/components/contacts/contact-details-sheet.tsxsrc/api/common/client.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use gluestack-ui icon component
Applied to files:
src/components/contacts/contact-details-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Applied to files:
src/components/contacts/contact-details-sheet.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to {components/ui/**/*.{ts,tsx},**/*.tsx} : Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Applied to files:
src/app/(app)/settings.tsxsrc/components/calls/close-call-bottom-sheet.tsxsrc/components/settings/unit-selection-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to src/components/ui/**/*.{ts,tsx} : Build and maintain shared Gluestack UI wrappers in src/components/ui
Applied to files:
src/app/(app)/settings.tsxsrc/components/settings/unit-selection-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{ts,tsx} : Use axios for API requests
Applied to files:
src/api/common/client.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Generate tests for all components, services, and logic; ensure tests run without errors
Applied to files:
src/services/__tests__/app-reset.service.test.ts
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/services/__tests__/app-reset.service.test.ts
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use React Navigation for navigation and deep linking with best practices
Applied to files:
src/components/calls/close-call-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{ts,tsx} : Use React Navigation for handling navigation and deep linking
Applied to files:
src/components/calls/close-call-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Wrap all user-facing text in t() from react-i18next
Applied to files:
src/components/calls/close-call-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Applied to files:
src/components/calls/close-call-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Use getItemLayout for FlatList when items have consistent size
Applied to files:
src/components/settings/unit-selection-bottom-sheet.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{ts,tsx} : Use react-native-mmkv for local storage
Applied to files:
src/components/status/status-bottom-sheet.tsx
🧬 Code graph analysis (8)
src/components/contacts/contact-details-sheet.tsx (4)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)src/components/ui/index.tsx (2)
View(18-18)Pressable(18-18)src/components/ui/icon/index.tsx (3)
MailIcon(672-672)PhoneIcon(760-760)GlobeIcon(525-525)src/components/ui/icon/index.web.tsx (3)
MailIcon(702-702)PhoneIcon(790-790)GlobeIcon(555-555)
src/app/(app)/settings.tsx (3)
src/services/app-reset.service.ts (1)
clearAllAppData(243-269)src/lib/logging/index.tsx (2)
error(74-76)logger(80-80)src/components/ui/button/index.tsx (2)
Button(334-334)ButtonText(334-334)
src/api/common/client.tsx (1)
src/lib/logging/index.tsx (1)
logger(80-80)
src/services/__tests__/app-reset.service.test.ts (1)
src/services/app-reset.service.ts (18)
INITIAL_CORE_STATE(35-49)INITIAL_CALLS_STATE(51-57)INITIAL_UNITS_STATE(59-64)INITIAL_CONTACTS_STATE(66-75)INITIAL_NOTES_STATE(77-84)INITIAL_ROLES_STATE(86-92)INITIAL_PROTOCOLS_STATE(94-101)INITIAL_DISPATCH_STATE(103-120)INITIAL_SECURITY_STATE(122-125)INITIAL_LOCATION_STATE(127-135)INITIAL_LIVEKIT_STATE(137-145)INITIAL_AUDIO_STREAM_STATE(147-154)INITIAL_BLUETOOTH_AUDIO_STATE(156-163)INITIAL_PUSH_NOTIFICATION_MODAL_STATE(165-168)clearAppStorageItems(188-192)clearPersistedStorage(176-183)resetAllStores(198-235)clearAllAppData(243-269)
src/components/calls/close-call-bottom-sheet.tsx (2)
src/components/ui/select/index.tsx (8)
Select(225-225)SelectTrigger(238-238)SelectInput(232-232)SelectIcon(231-231)SelectPortal(234-234)SelectBackdrop(226-226)SelectContent(227-227)SelectItem(233-233)src/components/ui/button/index.tsx (2)
Button(334-334)ButtonText(334-334)
src/components/settings/unit-selection-bottom-sheet.tsx (1)
src/components/ui/index.tsx (1)
ScrollView(18-18)
src/components/status/status-bottom-sheet.tsx (2)
src/components/ui/button/index.tsx (2)
Button(334-334)ButtonText(334-334)src/lib/utils.ts (1)
invertColor(38-62)
src/components/protocols/protocol-card.tsx (1)
src/components/ui/index.tsx (1)
Pressable(18-18)
🪛 GitHub Check: test
src/components/settings/unit-selection-bottom-sheet.tsx
[warning] 36-36:
Replace ⏎······key={unit.UnitId}⏎······onPress={handlePress}⏎······disabled={isLoading}⏎······className={isSelected·?·'data-[checked=true]:bg-background-100'·:·''}⏎······testID={unit-item-${unit.UnitId}}⏎···· with ·key={unit.UnitId}·onPress={handlePress}·disabled={isLoading}·className={isSelected·?·'data-[checked=true]:bg-background-100'·:·''}·testID={unit-item-${unit.UnitId}}
[warning] 193-193:
Replace ⏎························key={unit.UnitId}⏎························unit={unit}⏎························isSelected={activeUnit?.UnitId·===·unit.UnitId}⏎························isLoading={isLoading}⏎························onSelect={handleUnitSelection}⏎····················· with ·key={unit.UnitId}·unit={unit}·isSelected={activeUnit?.UnitId·===·unit.UnitId}·isLoading={isLoading}·onSelect={handleUnitSelection}
🔇 Additional comments (36)
src/components/calls/close-call-bottom-sheet.tsx (5)
1-18: LGTM!The imports are well-organized, using the appropriate components from gluestack-ui and the new
KeyboardAwareScrollViewfromreact-native-keyboard-controllerfor improved keyboard handling. All imports align with coding guidelines.
27-52: LGTM!Component setup is clean with proper use of hooks. The analytics tracking effect has correct dependencies, and
handleCloseis appropriately memoized withuseCallback.
54-85: LGTM!The
handleSubmitlogic correctly handles validation, API calls, error handling, and navigation. The dependency array is complete and the flow (validate → API → toast → close → navigate → refresh) is appropriate.
89-144: LGTM!The Actionsheet-based UI is well-structured with proper dark mode support via className. All user-facing text is correctly wrapped in
t()calls. The form controls use appropriate components from gluestack-ui.
147-155: LGTM!The StyleSheet follows the coding guidelines for styling when gluestack-ui components need custom styles. The
paddingBottom: 80ensures content isn't hidden behind keyboard or bottom UI elements.src/components/status/status-bottom-sheet.tsx (6)
68-80: LGTM!The effect correctly sets the default tab based on
DetailType: stations for type 1, calls for types 2 and 3. The dependency onselectedStatusensures this runs when the status changes.
523-531: LGTM!The button layout update provides a consistent UX with Cancel on the left and Next on the right. All button text is properly internationalized.
553-566: LGTM!The tab header rendering correctly shows tabs only for
DetailType 3(both calls and stations), which is the expected behavior.
570-631: LGTM!The tab content rendering logic correctly handles all three DetailType cases:
- DetailType 1: Shows stations only
- DetailType 2: Shows calls only
- DetailType 3: Shows based on selected tab
The loading states and empty states are properly handled with appropriate feedback.
636-651: LGTM!The conditional rendering of Previous/Cancel buttons based on
cameFromStatusSelectionprovides correct navigation context for users.
666-681: LGTM!The button layout in the skip-destination step is consistent with other steps, with proper disabled states for both note validation and submission status.
.github/workflows/react-native-cicd.yml (1)
435-442: LGTM!The jq syntax is correct—using
$noteswithout quotes is the proper way to reference a string variable declared with--arg. This correctly handles JSON escaping of the release notes content.src/components/settings/unit-selection-bottom-sheet.tsx (3)
30-56: Well-structured memoized component.Good use of
React.memowith a properly memoizedhandlePresscallback to prevent unnecessary re-renders. ThedisplayNameassignment aids debugging. The conditional rendering of the checkmark correctly uses the ternary operator.
63-68: Appropriate use of React patterns.Good implementation using
React.memo, proper state management withuseStateanduseReffor the processing guard. The component follows functional component patterns as required by coding guidelines.
193-199:isLoadingprop is alwaysfalsein this render branch.At line 197,
isLoading={isLoading}is passed toUnitItem, but this code only executes whenisLoadingisfalse(see the ternary at line 177). The prop will always befalse, making thedisabled={isLoading}insideUnitItemineffective in this context.If the intent is to disable items during selection, consider using a different mechanism or removing this prop since it's currently non-functional.
⛔ Skipped due to learnings
Learnt from: CR Repo: Resgrid/Unit PR: 0 File: .cursorrules:0-0 Timestamp: 2025-08-12T03:33:40.238Z Learning: Applies to **/*.tsx : Avoid anonymous functions in renderItem or event handlers to prevent re-renderssrc/components/contacts/contact-details-sheet.tsx (7)
19-25: LGTM!The imports are well-organized. The addition of
useCallbackfor memoization,LinkingandPlatformfor cross-platform URL handling, andloggerfor error logging aligns with the coding guidelines for error handling and best practices.
63-73: LGTM!The
ContactFieldActiontype and extendedContactFieldPropsinterface are well-defined. The action types cover all necessary cases, and the optionalfullAddressprop enables proper map navigation with complete address information.
77-141: Well-implemented URL handling with proper error recovery.The
handlePressimplementation addresses the previous review feedback comprehensively:
- Error handling with try/catch and appropriate logging levels (
warnfor unsupported URLs,errorfor failures)- Web fallback for address actions when native maps can't open
- Phone number sanitization removes non-numeric characters while preserving
+for international format- Protocol handling for websites ensures URLs are valid
The early return on line 78 correctly guards against null/undefined values before the switch statement executes.
143-157: LGTM!The rendering logic is clean:
- Early return handles empty values appropriately
- Visual feedback via
text-primary-600for actionable fields provides good affordance- Content extraction avoids JSX duplication
- Conditional rendering uses ternary operator per coding guidelines
323-323: Verify intent: Fax phone number marked as callable.Line 323 sets
action="phone"for the fax number, which will trigger a phone call when tapped. This may be intentional (some users may want to call fax lines), but consider if this is the expected behavior or if fax numbers should remain non-actionable.
348-350: Verify GPS coordinate format compatibility with maps applications.The coordinate fields (
LocationGpsCoordinates,EntranceGpsCoordinates,ExitGpsCoordinates) useaction="address", which will pass the raw coordinate string to maps apps. Ensure the stored format (e.g.,"40.7128,-74.0060"vs"40.7128, -74.0060") is compatible with the platform map URL schemes.For better reliability, consider using coordinate-specific URL formats:
case 'address': { const addressToOpen = fullAddress || value.toString(); const encodedAddress = encodeURIComponent(addressToOpen); + // For coordinates, consider: geo:lat,lng or maps:?ll=lat,lng webFallbackUrl = `https://maps.google.com/?q=${encodedAddress}`;
174-186: LGTM!The
ContactDetailsSheetcomponent follows best practices:
React.FCpattern per guidelinesuseMemocorrectly memoizes the selected contact lookup- All user-facing text properly wrapped in
t()for internationalizationsrc/api/common/client.tsx (2)
1-1: LGTM - Proper axios error type guard import.The
isAxiosErrorimport enables reliable error classification for the token refresh flow.
101-117: LGTM - Robust network error detection implemented.The updated error handling correctly uses
isAxiosError(refreshError) && !refreshError.responseto distinguish network errors from application errors. This addresses the previous fragility concern with message-based detection and ensures:
- Network errors (timeout, connection issues) don't trigger unnecessary logouts
- Invalid refresh tokens and server errors (400/401) properly trigger logout
- Appropriate log levels are used for each case
src/components/protocols/protocol-card.tsx (1)
19-24: LGTM - Proper memoization of press handler.The
useCallbackpattern with correct dependencies prevents unnecessary re-renders when this component is used in lists. This aligns with the coding guidelines to avoid anonymous functions in event handlers. ThetestIDupdate toProtocolIdis consistent with the broader protocol identifier refactoring.src/lib/auth/types.tsx (1)
49-49: LGTM - Correct type for timeout tracking.Using
ReturnType<typeof setTimeout>is the portable approach that works across Node.js and browser environments. The nullable type appropriately represents the absence of a scheduled refresh.src/app/(app)/settings.tsx (2)
52-74: LGTM - Well-structured logout confirmation handler.Good implementation:
- Uses centralized
clearAllAppDataservice as recommended in past review- Error handling allows logout to proceed even if cleanup partially fails
- Analytics tracking captures useful context
- Proper
useCallbackwith correct dependenciesOne minor consideration: if
signOut()(line 73) throws, the error is unhandled. However, since this is the final action after cleanup, and the user initiated logout, allowing it to propagate or crash may be acceptable behavior.
156-174: LGTM - Accessible logout confirmation dialog.The AlertDialog implementation follows gluestack-ui patterns correctly with proper structure (Header, Body, Footer). User-facing text uses
t()for i18n compliance per coding guidelines.src/services/__tests__/app-reset.service.test.ts (2)
1-9: LGTM - Proper mock placement before imports.Placing
jest.mock()calls before imports ensures modules are mocked before they're loaded. The logger mock covers all commonly used log levels.
444-468: LGTM - Good coverage of clearAllAppData including error propagation.The tests verify:
- All clearing functions are called in sequence
- Errors during cleanup are properly propagated
This ensures calling code can handle failures appropriately.
src/stores/auth/store.tsx (3)
61-74: Timer cleanup properly implemented.The previous review concern about timer leaks has been addressed. The code now:
- Clears any existing timeout before scheduling a new one
- Stores the timeout ID in state for later cleanup
89-104: Logout cleanup properly implemented.Timer cleanup on logout prevents the stacked timeout issue flagged in the previous review.
24-24: No type mismatch—the current implementation is correct for React Native.The type declaration uses
ReturnType<typeof setTimeout> | null, which is the correct approach for handlingsetTimeoutacross platforms. In React Native, this resolves tonumber, andclearTimeoutis called with the correct type throughout the code. The existing implementation follows best practices and requires no changes.Likely an incorrect or invalid review comment.
src/services/app-reset.service.ts (3)
243-269: Well-structured main cleanup function.The function has proper logging, error handling with re-throw for caller handling, and clear ordering of operations. The orchestration of storage clearing before store resets is logical.
271-291: Dual export pattern provides flexibility.Both named exports and default object export allow consumers to choose their preferred import style. Exporting initial states enables testing and potential store synchronization.
170-183: No issues found. TheclearPersistedStorage()function handles auth-storage correctly in the logout flow. The sequence in settings.tsx properly awaitsclearAllAppData()before callingsignOut(), ensuring all operations are sequential. Whenlogout()is called afterclearPersistedStorage()deletes 'auth-storage', the auth store simply updates its in-memory state viaset(), which automatically triggers Zustand's persist middleware to re-create 'auth-storage' with the signed-out state. There is no race condition or cleanup interference.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| return isActionable ? ( | ||
| <Pressable | ||
| onPress={handlePress} | ||
| className="active:opacity-70" | ||
| accessibilityRole="button" | ||
| accessibilityLabel={`${label}, ${displayValue}`} | ||
| accessibilityHint={t(`accessibility.action.${action}`, { defaultValue: `Activate to ${action}` })} | ||
| > | ||
| {content} | ||
| </Pressable> | ||
| ) : ( | ||
| content | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find i18n/localization configuration files
find . -type f \( -name "*.json" -o -name "*.ts" -o -name "*.js" \) | grep -i "i18n\|locale\|translation\|lang" | head -20Repository: Resgrid/Unit
Length of output: 261
🏁 Script executed:
# Search for accessibility.action translation keys
rg "accessibility\.action\.(email|phone|address|website)" --type json -A 2 -B 2Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Alternative: search for any accessibility translation keys
rg "accessibility" --type json -A 2 | head -50Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Check if there's an i18n configuration in common locations
find . -type f -name "*.json" | xargs grep -l "accessibility" 2>/dev/null | head -10Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Check en.json for accessibility keys
rg "accessibility" ./src/translations/en.json -A 3 -B 1Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Read resources.ts to understand i18n structure
cat ./src/lib/i18n/resources.tsRepository: Resgrid/Unit
Length of output: 284
🏁 Script executed:
# Look at the contact-details-sheet.tsx file to see what actions are possible
rg "handlePress\|action" ./src/components/contacts/contact-details-sheet.tsx -B 2 -A 2 | head -40Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Read en.json to see overall structure
head -100 ./src/translations/en.jsonRepository: Resgrid/Unit
Length of output: 3767
🏁 Script executed:
# Search for action parameter/prop in contact-details-sheet.tsx
rg "action" ./src/components/contacts/contact-details-sheet.tsx -B 2 -A 1 | head -50Repository: Resgrid/Unit
Length of output: 1574
🏁 Script executed:
# Check file size of en.json to know if we can read it entirely
wc -l ./src/translations/en.jsonRepository: Resgrid/Unit
Length of output: 86
🏁 Script executed:
# Search the entire en.json for "accessibility" key
cat ./src/translations/en.json | grep -i "accessibility"Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Also check if the translations exist in es.json or ar.json
rg "accessibility" ./src/translations/Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Check what action values are actually used by searching ContactFieldAction type
rg "ContactFieldAction\|action.*=" ./src/components/contacts/contact-details-sheet.tsx -B 1 -A 1 | head -30Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Read the contact-details-sheet.tsx file to see the type definition and full context
cat ./src/components/contacts/contact-details-sheet.tsx | head -200Repository: Resgrid/Unit
Length of output: 7430
🏁 Script executed:
# Search for ContactFieldAction type definition
rg "type ContactFieldAction\|interface ContactFieldAction" -A 5Repository: Resgrid/Unit
Length of output: 38
Translation keys for accessibility hints are missing from i18n files.
The accessibility hints currently rely on defaultValue fallback (Activate to email, Activate to phone, etc.), which means all users see English text regardless of language setting. To properly support internationalization per the coding guidelines, add the following translation keys to your i18n files:
{
"accessibility": {
"action": {
"email": "Activate to send email",
"phone": "Activate to call",
"address": "Activate to view on map",
"website": "Activate to open website"
}
}
}Add these to src/translations/en.json, src/translations/es.json, and src/translations/ar.json with appropriate translations for each language.
🤖 Prompt for AI Agents
In `@src/components/contacts/contact-details-sheet.tsx` around lines 159 - 171,
The accessibility hints currently fall back to English because the translation
keys used in contact-details-sheet.tsx (accessibilityHint using
t(`accessibility.action.${action}`)) are missing; add the keys
"accessibility.action.email", "accessibility.action.phone",
"accessibility.action.address", and "accessibility.action.website" with the
provided English strings to src/translations/en.json and add appropriate Spanish
and Arabic translations to src/translations/es.json and src/translations/ar.json
so t(...) resolves in each locale instead of relying on the defaultValue
fallback.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/services/app-reset.service.ts`:
- Around line 224-228: The logger call using logger.error({ message: 'Error
disconnecting from LiveKit room during reset', error }) fails because LogEntry
has no error property; update the call to wrap the error under the existing
context shape used elsewhere (e.g., change the object to logger.error({ message:
'Error disconnecting from LiveKit room during reset', context: { error } }) so
the error is passed as context consistent with other calls like the one using
context on line 278).
♻️ Duplicate comments (1)
src/services/app-reset.service.ts (1)
237-240: Fix TypeScript error:errorproperty doesn't exist inLogEntry.Same issue as above - the
errorproperty doesn't exist onLogEntrytype, causing the pipeline to fail.🐛 Proposed fix
} catch (error) { logger.error({ message: 'Error cleaning up audio stream during reset', - error, + context: { error: error instanceof Error ? error.message : String(error) }, }); }
🧹 Nitpick comments (2)
src/services/app-reset.service.ts (2)
51-57: Avoidas never[]type assertions.Using
as never[]bypasses TypeScript's type safety. Sincenever[]represents an array that can never contain elements, this assertion is semantically incorrect for arrays that will hold data at runtime. Consider using proper types or the actual array element types.♻️ Suggested approach
Define interfaces for your state shapes or use the actual types:
+import type { Call, CallPriority, CallType } from '@/types/calls'; export const INITIAL_CALLS_STATE = { - calls: [] as never[], - callPriorities: [] as never[], - callTypes: [] as never[], + calls: [] as Call[], + callPriorities: [] as CallPriority[], + callTypes: [] as CallType[], isLoading: false, error: null, };Alternatively, if you want to avoid importing all types, use generic empty arrays with explicit store state interfaces:
interface CallsState { calls: Call[]; callPriorities: CallPriority[]; callTypes: CallType[]; isLoading: boolean; error: Error | null; } export const INITIAL_CALLS_STATE: CallsState = { calls: [], callPriorities: [], callTypes: [], isLoading: false, error: null, };
285-305: Consider removing the default export in favor of named exports only.The module already provides named exports for all functions and constants. Having both named and default exports can lead to inconsistent import patterns across the codebase. Named exports are generally preferred as they enable better tree-shaking and more explicit imports.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/react-native-cicd.ymlsrc/services/app-reset.service.tssrc/translations/ar.jsonsrc/translations/en.jsonsrc/translations/es.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/translations/es.json
- src/translations/ar.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/translations/en.jsonsrc/services/app-reset.service.ts
src/translations/**
📄 CodeRabbit inference engine (.cursorrules)
Store translation dictionaries in src/translations
Files:
src/translations/en.json
src/translations/**/*.{json,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store translation dictionaries in src/translations
Files:
src/translations/en.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/services/app-reset.service.ts
🪛 GitHub Actions: React Native CI/CD
src/services/app-reset.service.ts
[error] 226-226: TS2353: Object literal may only specify known properties, and 'error' does not exist in type 'LogEntry'.
🪛 GitHub Check: test
src/services/app-reset.service.ts
[failure] 239-239:
Object literal may only specify known properties, and 'error' does not exist in type 'LogEntry'.
[failure] 226-226:
Object literal may only specify known properties, and 'error' does not exist in type 'LogEntry'.
🔇 Additional comments (6)
src/translations/en.json (2)
2-8: Accessibility action labels look good.Clear, concise activation prompts and consistent phrasing.
542-543: Settings translations for activation/logout confirmation are clear and consistent.Also applies to: 581-582
.github/workflows/react-native-cicd.yml (2)
322-328: LGTM! The|| truefix properly handles thegrep -vedge case.This correctly addresses the
set -eo pipefailissue wheregrep -vreturns exit code 1 when no lines match. The subshell pattern(grep -v ... || true)ensures the pipeline continues gracefully.
435-442: LGTM! Using jq's--argis the correct approach for safe JSON construction.The
$notesvariable from--arg notes "$RELEASE_NOTES"ensures proper JSON escaping of special characters, newlines, and quotes in the release notes content.src/services/app-reset.service.ts (2)
176-183: LGTM!The storage clearing logic correctly preserves specified keys while removing all others. The implementation is clean and straightforward.
257-283: LGTM!The orchestration function properly sequences the cleanup operations with appropriate logging and error propagation. Re-throwing after logging allows callers to handle failures if needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/services/app-reset.service.ts`:
- Line 230: Several store resets in app-reset.service.ts call setState with only
the new state (e.g., useLiveKitStore.setState(INITIAL_LIVEKIT_STATE)); update
these to use replace mode by passing true as the second argument
(useLiveKitStore.setState(INITIAL_LIVEKIT_STATE, true)) so the store is fully
replaced. Apply the same change to the other setState calls in this file that
reset stores (the three additional occurrences noted in the review) so they all
use setState(..., true).
🧹 Nitpick comments (1)
src/services/app-reset.service.ts (1)
224-227: Consider serializing error objects for better log output.Passing raw
Errorobjects to the logger'scontextmay not serialize cleanly in all logging backends. Converting to message strings ensures consistent output.♻️ Suggested improvement for error serialization
} catch (error) { logger.error({ message: 'Error disconnecting from LiveKit room during reset', - context: { error }, + context: { error: error instanceof Error ? error.message : String(error) }, }); }Apply the same pattern at line 239.
Also applies to: 237-240
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/services/app-reset.service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/services/app-reset.service.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/services/app-reset.service.ts
🧠 Learnings (1)
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{ts,tsx} : Handle errors gracefully and provide user feedback
Applied to files:
src/services/app-reset.service.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (6)
src/services/app-reset.service.ts (6)
35-49: LGTM - Well-structured initial state constants.The constants are properly typed and documented. The section header and export pattern make these easily reusable for testing.
176-183: LGTM - Simple and effective storage cleanup.The preserve list pattern allows for easy extension if more keys need to be preserved in the future.
218-242: Error handling for async operations is properly implemented.The try-catch blocks for LiveKit disconnect and audio stream cleanup ensure that failures in one cleanup operation don't prevent subsequent operations from running. This addresses the concern about isolated error handling.
257-283: LGTM - Well-structured main cleanup orchestration.The function properly sequences cleanup operations, provides observability through logging, and re-throws errors to allow callers to handle failures. This aligns with the coding guideline to handle errors gracefully and provide feedback. Based on learnings.
285-305: LGTM - Clean public API.The default export provides a convenient aggregation while named exports remain available for selective imports and tree-shaking.
198-208: No changes needed—allINITIAL_*_STATEconstants fully cover their respective stores' state properties. The proposed fix to addtrueas a second argument is unnecessary.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/calls/__tests__/call-files-modal.test.tsx (1)
504-512: MissingclearFilesin File Download test block's mock state.The
beforeEachin this test block doesn't includeclearFilesinmockStoreState, unlike other test blocks. This inconsistency could cause issues if the component accessesclearFiles.💚 Proposed fix
beforeEach(() => { // Reset to default state with files mockStoreState = { callFiles: defaultMockFiles, isLoadingFiles: false, errorFiles: null, fetchCallFiles: mockFetchCallFiles, + clearFiles: mockClearFiles, }; });
🤖 Fix all issues with AI agents
In `@src/components/calls/call-files-modal.tsx`:
- Around line 49-58: The effect's cleanup currently guards with if (!isOpen)
which uses a stale captured isOpen and prevents cleanup when the modal closes;
update the cleanup returned by the useEffect in call-files-modal to always run
(remove the if (!isOpen) check) so that setDownloadingFiles({}) and clearFiles()
execute whenever the effect is torn down or the component unmounts, keeping
references to the existing functions setDownloadingFiles and clearFiles.
In `@src/components/calls/call-images-modal.tsx`:
- Line 6: Remove the unused memo import from the React import list in
call-images-modal.tsx (the top-level import line) and fix import ordering;
update the import statement to omit "memo" and then run the ESLint autofix (npx
eslint --fix src/components/calls/call-images-modal.tsx) or your project's
import-sort command to resolve the import sort violation.
- Around line 85-96: The effect's cleanup is captured with the old isOpen value
so it never runs when the modal closes; instead, move the teardown into the
effect's else branch so it executes immediately when isOpen becomes false:
inside the useEffect that depends on isOpen, call setFullScreenImage(null),
setSelectedImageInfo(null), setImageErrors(new Set()), and clearImages() in the
else block (rather than in the returned cleanup function) and keep the returned
function only for any necessary unmount cleanup that should always run.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/calls/call-images-modal.tsx (2)
413-413: Unstable fallback key usingMath.random()can cause performance issues.Using
Math.random()as a fallback key will generate a new key on every render, causing React to unmount and remount items unnecessarily. This defeats the purpose of keys and may cause flickering.🐛 Proposed fix
Use the index as a fallback instead of random:
- keyExtractor={(item) => item?.Id || `image-${Math.random()}`} + keyExtractor={(item, index) => item?.Id || `image-fallback-${index}`}
91-101: Analytics event may fire multiple times while modal is open.The effect depends on
validImages.length, which changes after images load. This could result in multiplecall_images_modal_openedevents being tracked for a single modal open (once with 0 images, then again after fetch completes).🐛 Proposed fix
Consider tracking only on initial open by removing dynamic dependencies:
useEffect(() => { if (isOpen) { trackEvent('call_images_modal_opened', { callId: callId, - hasExistingImages: validImages.length > 0, - imagesCount: validImages.length, - isLoadingImages: isLoadingImages, - hasError: !!errorImages, }); } - }, [isOpen, trackEvent, callId, validImages.length, isLoadingImages, errorImages]); + }, [isOpen, trackEvent, callId]);Or use a ref to track if the event was already sent for this open session.
🤖 Fix all issues with AI agents
In `@src/components/calls/call-notes-modal.tsx`:
- Line 137: The ZeroState component is rendering hardcoded user text "No notes
found"; replace that literal with a translated string (e.g.,
t('callNotes.noNotesFound')) by importing/using your i18n helper (useTranslation
or t) inside the CallNotesModal component in call-notes-modal.tsx and pass the
translation result to ZeroState; also add the callNotes.noNotesFound key to your
translation files.
♻️ Duplicate comments (2)
src/components/calls/call-images-modal.tsx (1)
7-7: Unusedmemoimport.The
memoimport is still present but unused. This was flagged in a previous review.🧹 Proposed fix
-import React, { useCallback, useEffect, useMemo, useRef, useState, memo } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';src/services/app-reset.service.ts (1)
200-216: ThesesetStatecalls should use replace mode for consistency.Lines 200-208 and 216 call
setStatewithout the secondtrueargument, while lines 230, 242, 245, 248 use replace mode. Without replace mode,setStateperforms a shallow merge, which may leave stale properties if the initial state constants don't perfectly match the store shape.♻️ Proposed fix
- useCoreStore.setState(INITIAL_CORE_STATE); - useCallsStore.setState(INITIAL_CALLS_STATE); - useUnitsStore.setState(INITIAL_UNITS_STATE); - useContactsStore.setState(INITIAL_CONTACTS_STATE); - useNotesStore.setState(INITIAL_NOTES_STATE); - useRolesStore.setState(INITIAL_ROLES_STATE); - useProtocolsStore.setState(INITIAL_PROTOCOLS_STATE); - useDispatchStore.setState(INITIAL_DISPATCH_STATE); - securityStore.setState(INITIAL_SECURITY_STATE); + useCoreStore.setState(INITIAL_CORE_STATE, true); + useCallsStore.setState(INITIAL_CALLS_STATE, true); + useUnitsStore.setState(INITIAL_UNITS_STATE, true); + useContactsStore.setState(INITIAL_CONTACTS_STATE, true); + useNotesStore.setState(INITIAL_NOTES_STATE, true); + useRolesStore.setState(INITIAL_ROLES_STATE, true); + useProtocolsStore.setState(INITIAL_PROTOCOLS_STATE, true); + useDispatchStore.setState(INITIAL_DISPATCH_STATE, true); + securityStore.setState(INITIAL_SECURITY_STATE, true); // ... - useLocationStore.setState(INITIAL_LOCATION_STATE); + useLocationStore.setState(INITIAL_LOCATION_STATE, true);
🧹 Nitpick comments (10)
src/components/calls/call-notes-modal.tsx (4)
59-62: Consider removing the eslint-disable comment.The dependency array includes
callNoteswhich is correct sincesearchNoteslikely operates on the store'scallNotes. However, ifsearchNotesis already memoized and returns fresh results based oncallNotes, you may not need to includecallNotesexplicitly. Alternatively, restructure to avoid the lint suppression.♻️ Suggested refactor
const filteredNotes = useMemo(() => { return searchNotes(searchQuery); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [searchQuery, searchNotes, callNotes]); + }, [searchQuery, searchNotes, callNotes]);If
searchNotesis a stable function that internally readscallNotesfrom the store, the lint warning may be a false positive. Consider verifying whethersearchNotesreference changes whencallNotesupdates—if so, the current deps are correct and the disable can be removed.
128-135: Consider extractingkeyExtractorand adding FlatList optimizations.Per coding guidelines, avoid anonymous functions in FlatList. The
keyExtractorcan be extracted. Additionally, consider adding optimization props for better performance with larger lists.♻️ Suggested improvements
+ const keyExtractor = useCallback((item: (typeof filteredNotes)[0]) => item.CallNoteId, []); <FlatList data={filteredNotes} renderItem={renderNote} - keyExtractor={(item) => item.CallNoteId} + keyExtractor={keyExtractor} contentContainerStyle={styles.listContent} showsVerticalScrollIndicator={true} keyboardShouldPersistTaps="handled" + removeClippedSubviews={true} + maxToRenderPerBatch={10} />
67-77: Provide user feedback on error.When
addNotefails, the error is only logged to the console. Users should receive feedback that their note wasn't saved.💡 Suggested improvement
Consider using a toast or alert to notify the user:
const handleAddNote = useCallback(async () => { if (newNote.trim()) { try { await addNote(callId, newNote, currentUser, null, null); setNewNote(''); Keyboard.dismiss(); } catch (error) { console.error('Failed to add note:', error); + // Show user feedback - example using a toast library + // Toast.show({ type: 'error', text1: t('callNotes.addNoteError') }); } } }, [newNote, callId, currentUser, addNote]);
108-110: Add accessibility props to the close button.Per coding guidelines, ensure WCAG compliance for mobile. The close button should have accessibility attributes for screen reader users.
♿ Suggested accessibility improvement
- <TouchableOpacity onPress={handleClose} style={styles.closeButton} testID="close-button"> + <TouchableOpacity + onPress={handleClose} + style={styles.closeButton} + testID="close-button" + accessibilityLabel={t('common.close')} + accessibilityRole="button" + > <X size={24} color={isDark ? '#D1D5DB' : '#374151'} /> </TouchableOpacity>src/components/calls/call-images-modal.tsx (4)
53-53: Consider using a more precise type forflatListRef.The ref is typed as
any. Per coding guidelines, strive for precise types. Consider using a more specific type from the FlatList component.💡 Suggested improvement
- const flatListRef = useRef<any>(null); // FlashList ref type + const flatListRef = useRef<React.ElementRef<typeof FlatList>>(null);
249-257: Anonymous function inonPresshandler may cause unnecessary re-renders.Per coding guidelines, avoid anonymous functions in
renderItemor event handlers. The inline arrow function creates a new function instance on each render.💡 Suggested improvement
Consider extracting the press handler or using
useCallbackwith the item data:+ const handleImagePress = useCallback((source: { uri: string }, name?: string) => { + setFullScreenImage({ source, name }); + }, []); + const renderImageItem = ({ item, index }: { item: CallFileResultData; index: number }) => { // ... existing code ... return ( <TouchableOpacity - onPress={() => { - setFullScreenImage({ source: imageSource, name: item.Name }); - }} + onPress={() => handleImagePress(imageSource!, item.Name)}Note: A fully memoized approach would require restructuring with item-level components.
470-475: Use ternary operator instead of&&for conditional rendering.Per coding guidelines, use the conditional operator (
?:) for conditional rendering instead of&&.♻️ Proposed fix
- {!isAddingImage && !isLoadingImages && ( - <Button size="sm" variant="outline" onPress={() => setIsAddingImage(true)}> - <ButtonIcon as={PlusIcon} /> - <ButtonText>{t('callImages.add')}</ButtonText> - </Button> - )} + {!isAddingImage && !isLoadingImages ? ( + <Button size="sm" variant="outline" onPress={() => setIsAddingImage(true)}> + <ButtonIcon as={PlusIcon} /> + <ButtonText>{t('callImages.add')}</ButtonText> + </Button> + ) : null}
553-553: Consider wrapping component withReact.memoif parent re-renders frequently.Based on learnings,
React.memocan prevent unnecessary re-renders for components with static props. If the parent component re-renders frequently without changingisOpen,onClose, orcallId, wrapping withmemocould improve performance.Note: The
memoimport exists but is unused. If you decide to use it, keep the import; otherwise, remove it.src/services/app-reset.service.ts (2)
51-57: Consider using proper array types instead ofas never[].The
as never[]pattern bypasses TypeScript's type system. While functional for resetting stores, it reduces type safety and IDE support. Consider importing the actual element types from their respective stores or usingas unknown[]as a slightly safer alternative.This pattern is repeated across all initial state constants with arrays.
223-228: Consider serializing error to a string for consistent logging.The error is passed as a raw object within
context. For more reliable log output, especially if the logger doesn't handle Error object serialization, consider extracting the message:logger.error({ message: 'Error disconnecting from LiveKit room during reset', - context: { error }, + context: { error: error instanceof Error ? error.message : String(error) }, });This pattern would also apply to lines 237-240.
| keyboardShouldPersistTaps="handled" | ||
| /> | ||
| ) : ( | ||
| <ZeroState heading="No notes found" /> |
There was a problem hiding this comment.
Wrap user-facing text in t() for internationalization.
Per coding guidelines, all user-facing text should use the translation function. The hardcoded string should be replaced with a translation key.
🌐 Proposed fix
- <ZeroState heading="No notes found" />
+ <ZeroState heading={t('callNotes.noNotesFound')} />Ensure the translation key callNotes.noNotesFound is added to your translation files.
📝 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.
| <ZeroState heading="No notes found" /> | |
| <ZeroState heading={t('callNotes.noNotesFound')} /> |
🤖 Prompt for AI Agents
In `@src/components/calls/call-notes-modal.tsx` at line 137, The ZeroState
component is rendering hardcoded user text "No notes found"; replace that
literal with a translated string (e.g., t('callNotes.noNotesFound')) by
importing/using your i18n helper (useTranslation or t) inside the CallNotesModal
component in call-notes-modal.tsx and pass the translation result to ZeroState;
also add the callNotes.noNotesFound key to your translation files.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/calls/call-notes-modal.tsx (1)
47-57: Analytics event may fire excessively due to dependency array.The
trackEvent('call_notes_modal_opened', ...)will re-fire whenevercallNotes.lengthorsearchQuery.lengthchanges (e.g., as notes load or user types in search). This could produce many duplicate analytics events per modal open.Consider tracking only on initial open:
🔧 Proposed fix
useEffect(() => { if (isOpen) { trackEvent('call_notes_modal_opened', { callId: callId, - existingNotesCount: callNotes.length, - hasSearchQuery: searchQuery.length > 0, - isNotesLoading: isNotesLoading, }); } - }, [isOpen, trackEvent, callId, callNotes.length, searchQuery.length, isNotesLoading]); + }, [isOpen, trackEvent, callId]);If you need to capture notes count/search state, consider logging a separate event on close or using refs to avoid re-triggering.
src/components/calls/call-images-modal.tsx (1)
319-336: Add accessibility labels for icon-only buttons.The close and pagination buttons are icon-only, which leaves screen readers without a usable label. Add
accessibilityLabel(and ideallyaccessibilityRole) so users can identify the actions.🧩 Suggested a11y patch
-<TouchableOpacity testID="previous-button" onPress={handlePrevious} disabled={activeIndex === 0} className={`rounded-full bg-white/80 p-2 ${activeIndex === 0 ? 'opacity-50' : ''}`}> +<TouchableOpacity + testID="previous-button" + onPress={handlePrevious} + disabled={activeIndex === 0} + accessibilityRole="button" + accessibilityLabel={t('common.previous')} + className={`rounded-full bg-white/80 p-2 ${activeIndex === 0 ? 'opacity-50' : ''}`} +> -<TouchableOpacity +<TouchableOpacity testID="next-button" onPress={handleNext} disabled={activeIndex === validImages.length - 1} + accessibilityRole="button" + accessibilityLabel={t('common.next')} className={`rounded-full bg-white/80 p-2 ${activeIndex === validImages.length - 1 ? 'opacity-50' : ''}`} > -<TouchableOpacity onPress={handleClose} style={styles.closeButton} testID="close-button"> +<TouchableOpacity + onPress={handleClose} + style={styles.closeButton} + testID="close-button" + accessibilityRole="button" + accessibilityLabel={t('common.close')} +>As per coding guidelines, ensure accessibility for mobile UI elements.
Also applies to: 471-473
🤖 Fix all issues with AI agents
In `@src/components/calls/call-notes-modal.tsx`:
- Around line 116-118: The SearchIcon inside the InputSlot is using a React web
prop className which has no effect for lucide-react-native; update the JSX for
SearchIcon (the SearchIcon element) to remove className and pass a color prop
instead (e.g., color="gray" or the specific hex/Token used for text-gray-500)
while keeping size as-is so the icon renders with the intended color.
🧹 Nitpick comments (4)
src/components/calls/call-notes-modal.tsx (1)
128-135: Avoid anonymous function inkeyExtractor.Per coding guidelines, anonymous functions in FlatList props can cause unnecessary re-renders. Extract
keyExtractorto a stable callback.♻️ Proposed fix
Add a memoized keyExtractor callback:
+ const keyExtractor = useCallback((item: (typeof filteredNotes)[0]) => item.CallNoteId, []);Then use it in the FlatList:
<FlatList data={filteredNotes} renderItem={renderNote} - keyExtractor={(item) => item.CallNoteId} + keyExtractor={keyExtractor} contentContainerStyle={styles.listContent} showsVerticalScrollIndicator={true} keyboardShouldPersistTaps="handled" />src/components/calls/call-images-modal.tsx (3)
52-53: Avoidanyfor image sources, errors, and viewable items.Using
anyweakens type safety in several places. Please tighten types to keep the TS surface predictable.🔧 Suggested type-safe update
-import { Image } from 'expo-image'; +import { Image, type ImageSource } from 'expo-image'; -import { Dimensions, Keyboard, Modal, SafeAreaView, StyleSheet, TouchableOpacity, View } from 'react-native'; +import { Dimensions, Keyboard, Modal, SafeAreaView, StyleSheet, TouchableOpacity, View, type ViewToken } from 'react-native'; -const [fullScreenImage, setFullScreenImage] = useState<{ source: any; name?: string } | null>(null); +const [fullScreenImage, setFullScreenImage] = useState<{ source: ImageSource; name?: string } | null>(null); -const handleImageError = (itemId: string, error: any) => { +const handleImageError = (itemId: string, error: unknown) => { -const handleImagePress = useCallback((source: { uri: string }, name?: string) => { +const handleImagePress = useCallback((source: ImageSource, name?: string) => { -const handleViewableItemsChanged = useRef(({ viewableItems }: any) => { +const handleViewableItemsChanged = useRef(({ viewableItems }: { viewableItems: ViewToken[] }) => {As per coding guidelines, avoid
anyand prefer precise types.Also applies to: 204-206, 209-211, 282-286
253-274: Avoid inline handlers insiderenderImageItem.Inline
onPress,onError, andonLoadlambdas inrenderImageItemcreate new functions per render and violate the “no anonymous handlers in renderItem” guideline. Consider extracting a memoizedImageItemcomponent with stable callbacks.As per coding guidelines, avoid anonymous functions in renderItem/event handlers.
254-262: Confirm image component choice vs project standard.The guidelines for this repo prefer
react-native-fast-imagefor image optimization. You’re usingexpo-imagehere (including caching). If this is intentional for Expo compatibility, please confirm; otherwise consider migrating.Based on learnings, optimize images using react-native-fast-image.
Also applies to: 347-348
| <InputSlot> | ||
| <SearchIcon size={20} className="text-gray-500" /> | ||
| </InputSlot> |
There was a problem hiding this comment.
className prop on lucide icon has no effect.
Lucide-react-native icons don't support className. Use the color prop instead to style the icon.
🎨 Proposed fix
<InputSlot>
- <SearchIcon size={20} className="text-gray-500" />
+ <SearchIcon size={20} color={isDark ? '#9CA3AF' : '#6B7280'} />
</InputSlot>🤖 Prompt for AI Agents
In `@src/components/calls/call-notes-modal.tsx` around lines 116 - 118, The
SearchIcon inside the InputSlot is using a React web prop className which has no
effect for lucide-react-native; update the JSX for SearchIcon (the SearchIcon
element) to remove className and pass a color prop instead (e.g., color="gray"
or the specific hex/Token used for text-gray-500) while keeping size as-is so
the icon renders with the intended color.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.