Develop #80
Develop #80
Conversation
WalkthroughApplies Android safe-area insets to the New Call screen and overlays, migrates FlashMessage -> toast system (store, container, hook, utils, tests), changes createCall DispatchList to raw IDs, implements timestamped token refresh with API interceptor changes, and centralizes RefreshControl imports across screens. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant NewCall as NewCallScreen
participant API as createCall()
participant Backend as SaveCall
participant Hook as useToast
participant Store as useToastStore
participant UI as ToastContainer
participant Client as APIClient
participant Auth as AuthStore
User->>NewCall: Submit new call
NewCall->>API: send payload (DispatchList with raw IDs)
API->>Backend: POST SaveCall
alt Success
Backend-->>API: 200 OK
API-->>NewCall: response
NewCall->>Hook: toast.success(message, title?, position?, duration?)
Hook->>Store: showToast(type='success', message, title, position, duration)
Store-->>UI: enqueue toast (position/duration)
UI->>User: render toast in correct stack
NewCall->>NewCall: navigate('/home/calls')
else Error
Backend-->>API: Error
API-->>NewCall: error
NewCall->>Hook: toast.error(...)
end
Note over Client,Auth: Token refresh during API requests
Client->>Auth: isAccessTokenExpiringSoon?
alt ExpiringSoon
Client->>Auth: refreshAccessToken()
Auth-->Client: new tokens (access/refresh + obtained_at timestamps)
end
Client->>Backend: request with Bearer (latest token)
Backend-->>Client: 401
Client->>Auth: attempt refresh (response path)
alt Refresh success
Auth-->Client: new access token
Client->>Backend: retry original request
else Permanent failure
Auth->>Auth: logout(reason)
Client-->>User: surface error via toast.error(...)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/android-system-bar-fix-new-call.md(1 hunks)src/api/calls/calls.ts(1 hunks)src/app/_layout.tsx(2 hunks)src/app/call/new/__tests__/what3words.test.tsx(1 hunks)src/app/call/new/index.tsx(25 hunks)src/components/toast/toast-container.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
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
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
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/_layout.tsxsrc/app/call/new/__tests__/what3words.test.tsxsrc/components/toast/toast-container.tsxsrc/api/calls/calls.tssrc/app/call/new/index.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
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 to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/app/_layout.tsxsrc/app/call/new/__tests__/what3words.test.tsxsrc/components/toast/toast-container.tsxsrc/app/call/new/index.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/app/_layout.tsxsrc/app/call/new/__tests__/what3words.test.tsxsrc/components/toast/toast-container.tsxsrc/api/calls/calls.tssrc/app/call/new/index.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/app/call/new/__tests__/what3words.test.tsx
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/app/call/new/__tests__/what3words.test.tsx
🧬 Code graph analysis (3)
src/app/_layout.tsx (1)
src/components/toast/toast-container.tsx (1)
ToastContainer(7-17)
src/components/toast/toast-container.tsx (1)
__mocks__/@/components/ui/vstack.tsx (1)
VStack(4-6)
src/app/call/new/index.tsx (2)
__mocks__/react-native.ts (2)
ScrollView(47-47)Platform(2-5)__mocks__/@/components/ui/box.tsx (1)
Box(4-6)
🔇 Additional comments (11)
src/components/toast/toast-container.tsx (1)
11-11: Verify toast positioning with increased top margin.The top margin increased from
mt-10(40px) tomt-20(80px). Combined withpt-safe-top(which adds the safe area inset), this may create excessive spacing on devices without notches or on Android devices.Please verify the visual appearance of toasts on:
- iOS devices with and without notches
- Android devices with different status bar configurations
- Tablets
If the spacing is too large, consider a smaller increment or conditional spacing based on platform.
src/app/call/new/__tests__/what3words.test.tsx (1)
53-61: LGTM! Test mock updated to match new toast API.The toast mock now includes
success,error,warning, andinfomethods alongside the existingshowmethod, aligning with the standardized toast API used throughout the application.docs/android-system-bar-fix-new-call.md (1)
1-128: LGTM! Comprehensive documentation of Android safe-area handling.The documentation clearly explains the issue, root cause, solution implementation, and testing approach for Android system navigation bar handling. The code examples are helpful and the structure is easy to follow.
Note: This documentation references changes to
src/app/call/new/index.tsx, which is not included in the current review batch. Ensure that file implements the changes described here.src/api/calls/calls.ts (1)
90-104: Unify dispatch list formatting and remove commented mapping codeThe
createCallimplementation pushes raw IDs (commenting out theU:,G:,R:,U:prefixes), whereasupdateCallstill prefixes entries (U:,G:,R:, etc.). Confirm the API’s expected format and update both functions to use the same scheme. Also remove the obsolete commented-out mapping code increateCall.src/app/call/new/index.tsx (7)
12-13: LGTM! Safe-area imports added correctly.The addition of
PlatformanduseSafeAreaInsetsproperly supports the Android safe-area handling implemented throughout this component.
124-125: LGTM! Hooks initialized correctly.Both
useToastanduseSafeAreaInsetsare properly initialized at the component's top level.
255-744: LGTM! Toast API refactored consistently.The migration from custom toast rendering to the standardized toast API (
toast.success,toast.error,toast.warning,toast.info) is clean and consistent across all 22 toast invocations in the component.
774-774: LGTM! ScrollView safe-area handling implemented correctly.The
contentContainerStyleproperly applies bottom padding using insets with a sensible minimum (Math.max(insets.bottom + 20, 40)), ensuring content is never obscured by system UI elements.
1052-1052: LGTM! Platform-specific margin handling for action bar.The conditional margin using
Platform.OS === 'android'appropriately adjusts bottom spacing for Android's gesture navigation while maintaining fixed spacing on iOS.
1065-1085: LGTM! Full-screen overlay safe-area handling.The location picker overlay correctly applies Android-specific bottom padding using insets to prevent content from being obscured by system gesture areas, while iOS doesn't require it.
258-258: Confirmed: route/home/callsexists atsrc/app/(app)/home/calls.tsx. No changes required.
src/app/_layout.tsx
Outdated
| <ToastContainer /> | ||
| <FlashMessage position="top" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify the need for both toast systems.
Both ToastContainer and FlashMessage are now rendered in the provider tree. These components serve similar purposes (displaying toast notifications).
Verify whether both are needed or if FlashMessage can be removed:
If FlashMessage is no longer used, remove it to avoid confusion and reduce bundle size:
-import FlashMessage from 'react-native-flash-message';
// ... in Providers
<LiveKitBottomSheet />
<PushNotificationModal />
<ToastContainer />
- <FlashMessage position="top" />
</BottomSheetModalProvider>🏁 Script executed:
#!/bin/bash
# Description: Search for usages of FlashMessage to determine if it's still needed
echo "=== Searching for FlashMessage usage ==="
rg -n 'showMessage|FlashMessage' --type=tsx --type=ts -g '!node_modules' -g '!_layout.tsx'
echo ""
echo "=== Searching for new toast API usage ==="
rg -n 'useToast|toast\.(success|error|warning|info|show)' --type=tsx --type=ts -g '!node_modules' -g '!__tests__'Length of output: 139
🏁 Script executed:
#!/bin/bash
echo "=== FlashMessage imports ==="
rg -n "import FlashMessage" -g "*.tsx" -g "*.ts" -g "!node_modules"
echo "=== FlashMessage usage ==="
rg -n "FlashMessage" -g "*.tsx" -g "*.ts" -g "!node_modules"
echo "=== FlashMessage showMessage usage ==="
rg -n "showMessage" -g "*.tsx" -g "*.ts" -g "!node_modules"
echo "=== Toast API usage ==="
rg -n "toast(" -g "*.tsx" -g "*.ts" -g "!node_modules"
echo "=== useToast usage ==="
rg -n "useToast" -g "*.tsx" -g "*.ts" -g "!node_modules"Length of output: 7500
Migrate FlashMessage calls before removal
FlashMessage is still relied on by showMessage in src/components/ui/utils.tsx. Migrate these to the new toast API (useToast/useToastStore) and ensure <ToastContainer/> covers all use cases before deleting <FlashMessage> from src/app/_layout.tsx.
🤖 Prompt for AI Agents
In src/app/_layout.tsx around lines 190-191, FlashMessage is still rendered next
to ToastContainer; update codebase before removing FlashMessage by migrating all
callers of showMessage in src/components/ui/utils.tsx to the new toast API
(useToast or useToastStore). Modify utils.tsx to replace showMessage usages with
calls to the toast hooks/store (preserving severity, title, description, and
position semantics), ensure ToastContainer supports those positions and
variants, and run the app to confirm parity; only after verifying ToastContainer
covers every migrated case remove the <FlashMessage /> render and any related
imports.
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/app/(app)/home/units.tsx (1)
44-49: Guard refreshing state with try/finally.If
fetchUnits()throws,refreshingstays true. Wrap with try/finally.- const handleRefresh = React.useCallback(async () => { - setRefreshing(true); - await fetchUnits(); - setRefreshing(false); - }, [fetchUnits]); + const handleRefresh = React.useCallback(async () => { + setRefreshing(true); + try { + await fetchUnits(); + } finally { + setRefreshing(false); + } + }, [fetchUnits]);src/app/(app)/protocols.tsx (1)
38-42: Always clear refreshing via try/finally.Protect the state in case
fetchProtocols()rejects.- const handleRefresh = React.useCallback(async () => { - setRefreshing(true); - await fetchProtocols(); - setRefreshing(false); - }, [fetchProtocols]); + const handleRefresh = React.useCallback(async () => { + setRefreshing(true); + try { + await fetchProtocols(); + } finally { + setRefreshing(false); + } + }, [fetchProtocols]);src/app/(app)/home/calls.tsx (1)
45-49: Refreshing UX: add state + try/finally; avoid hardcodedrefreshing={false}.Currently the spinner never shows. Track
refreshingand clear it reliably.-import React, { useCallback, useState } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; ... - const handleRefresh = () => { - fetchCalls(); - fetchCallPriorities(); - }; + const [refreshing, setRefreshing] = useState(false); + const handleRefresh = useCallback(async () => { + setRefreshing(true); + try { + await Promise.all([fetchCalls(), fetchCallPriorities()]); + } finally { + setRefreshing(false); + } + }, [fetchCalls, fetchCallPriorities]); ... - refreshControl={<RefreshControl refreshing={false} onRefresh={handleRefresh} />} + refreshControl={<RefreshControl refreshing={refreshing} onRefresh={handleRefresh} />}Also applies to: 76-77
src/app/(app)/home/personnel.tsx (1)
48-59: Harden filtering against non-string roles to avoid runtime errorsRoles may contain null/undefined or non-strings; calling toLowerCase would throw. Guard the type in the predicate.
Apply this diff:
- person.Roles?.some((role) => role.toLowerCase().includes(query)) + person.Roles?.some((role) => typeof role === 'string' && role.toLowerCase().includes(query))src/components/calls/dispatch-selection-modal.tsx (3)
229-241: Respect safe‑area insets for the full‑screen overlay.The absolute-positioned root container will clip under status bars/notches. Wrap with SafeAreaView (react-native-safe-area-context) to pad top/bottom on iOS/Android.
-import { ScrollView, TouchableOpacity, View } from 'react-native'; +import { ScrollView, TouchableOpacity, View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context' @@ - return ( - <View + return ( + <SafeAreaView + edges={['top','bottom']} style={{ position: 'absolute', top: 0, left: 0, right: 0, bottom: 0, zIndex: 1000, backgroundColor: colorScheme === 'dark' ? '#000000' : '#ffffff', }} > @@ - </View> + </SafeAreaView>Also applies to: 424-425
265-269: Render a safe, translated error string.Error may be an object; avoid “[object Object]” and leaking raw messages.
- <Text className="text-center text-red-500">{error}</Text> + <Text className="text-center text-red-500"> + {typeof error === 'string' ? error : (error as any)?.message ?? t('common.generic_error', { defaultValue: 'Something went wrong.' })} + </Text>
392-396: Trim the query in the “No results” guard to match filtering logic.Prevents mismatch when query is only whitespace.
- {!isLoading && !error && searchQuery && filteredData.users.length === 0 && filteredData.groups.length === 0 && filteredData.roles.length === 0 && filteredData.units.length === 0 && ( + {!isLoading && !error && searchQuery.trim() && filteredData.users.length === 0 && filteredData.groups.length === 0 && filteredData.roles.length === 0 && filteredData.units.length === 0 && (
🧹 Nitpick comments (34)
src/api/messaging/messages.ts (2)
62-73: Avoid sending empty strings for optional fields (omit ExpireOn when undefined).Sending '' can be ambiguous server-side. Prefer omitting ExpireOn unless provided.
As per coding guidelines
export const sendMessage = async (messageData: SendMessageRequest) => { const data = { Title: messageData.subject, Body: messageData.body, Type: messageData.type, Recipients: messageData.recipients.map((recipient) => ({ Id: recipient.id, Type: recipient.type, Name: recipient.name, })), - ExpireOn: messageData.expireOn || '', + ...(messageData.expireOn ? { ExpireOn: messageData.expireOn } : {}), };
50-60: Tighten request typing (enum for type; consider DTO type for payload).
- Replace type: number with a MessageType enum (or union) for clarity and safety.
- Optionally define a SendMessageDto with PascalCase keys to make the mapping explicit and catch future API renames at compile time.
As per coding guidelines
// Example (adjust values to match API) export enum MessageType { General = 0, Urgent = 1, // ... } export interface SendMessageRequest { subject: string; body: string; type: MessageType; recipients: { id: string; type: number; name: string }[]; expireOn?: string; } // Optional: transport DTO for server payload interface SendMessageDto { Title: string; Body: string; Type: MessageType; Recipients: { Id: string; Type: number; Name: string }[]; ExpireOn?: string; }src/hooks/use-toast.ts (1)
7-21: Prefer an options object to avoid ambiguous positional args (non‑blocking).Positional (title, position, duration) is error‑prone and forces callers to pass
undefinedto reach later params. Consider{ title?, position?, duration? }for all helpers.Example diff (local to this file):
- show: (type: ToastType, message: string, title?: string, position?: ToastPosition, duration?: number) => { - showToast(type, message, title, position, duration); + show: (type: ToastType, message: string, opts?: { title?: string; position?: ToastPosition; duration?: number }) => { + showToast(type, message, opts?.title, opts?.position, opts?.duration); }, - success: (message: string, title?: string, position?: ToastPosition, duration?: number) => { - showToast('success', message, title, position, duration); + success: (message: string, opts?: { title?: string; position?: ToastPosition; duration?: number }) => { + showToast('success', message, opts?.title, opts?.position, opts?.duration); }, - error: (message: string, title?: string, position?: ToastPosition, duration?: number) => { - showToast('error', message, title, position, duration); + error: (message: string, opts?: { title?: string; position?: ToastPosition; duration?: number }) => { + showToast('error', message, opts?.title, opts?.position, opts?.duration); }, - warning: (message: string, title?: string, position?: ToastPosition, duration?: number) => { - showToast('warning', message, title, position, duration); + warning: (message: string, opts?: { title?: string; position?: ToastPosition; duration?: number }) => { + showToast('warning', message, opts?.title, opts?.position, opts?.duration); }, - info: (message: string, title?: string, position?: ToastPosition, duration?: number) => { - showToast('info', message, title, position, duration); + info: (message: string, opts?: { title?: string; position?: ToastPosition; duration?: number }) => { + showToast('info', message, opts?.title, opts?.position, opts?.duration); },src/app/(app)/home/units.tsx (2)
96-103: Avoid inline renderItem to reduce re‑renders; add useCallback.Define
renderItemwithuseCallbackand pass a typed prop toUnitCardto eliminateany.- return units.filter( + const renderItem = React.useCallback(({ item }: { item: Unit /* import your type */ }) => ( + <UnitCard unit={item} onPress={selectUnit} /> + ), [selectUnit]); + return units.filter( ... - <FlashList + <FlashList data={filteredUnits} keyExtractor={(item, index) => item.UnitId || `unit-${index}`} - renderItem={({ item }) => <UnitCard unit={item as any} onPress={selectUnit} />} + renderItem={renderItem}
15-16: Consolidate duplicate imports from the same module.Merge the two
@/components/ui/inputimports to a single line to avoid duplication.src/app/(app)/protocols.tsx (1)
76-85: Avoid inline renderItem; memoize for FlashList.Define
renderItemwithuseCallbackfor better list perf.src/lib/auth/types.tsx (1)
1-20: Nit: file extension can be .ts (types only).Since the file exports types/interfaces only, consider renaming to
.tsfor clarity.src/app/(app)/home/calls.tsx (2)
55-55: Memoize filtered list to avoid needless recompute.Wrap the filter in
useMemokeyed bycallsandsearchQuery.
68-76: Avoid inline renderItem; memoize for list perf.Define
renderItemwithuseCallbackand reuse it.src/app/(app)/home/personnel.tsx (1)
94-101: FlashList: add estimatedItemSize and keyboard handling for smoother UXTo reduce layout passes and improve scroll perf while typing, supply an estimatedItemSize and persist taps during search.
Example:
- <FlashList + <FlashList data={filteredPersonnel} keyExtractor={(item, index) => item.UserId || `personnel-${index}`} renderItem={({ item }) => <PersonnelCard personnel={item} onPress={selectPersonnel} />} showsVerticalScrollIndicator={false} contentContainerStyle={{ paddingBottom: 100 }} + estimatedItemSize={88} + keyboardShouldPersistTaps="handled" refreshControl={<RefreshControl refreshing={refreshing} onRefresh={handleRefresh} />} />As per coding guidelines (optimize FlatList).
src/app/(app)/contacts.tsx (1)
84-93: FlashList: supply estimatedItemSize and keyboard tap behaviorAdds predictability and reduces re-layout during filtering.
Example:
- <FlashList + <FlashList testID="contacts-list" data={filteredContacts} keyExtractor={(item) => item.ContactId} renderItem={({ item }) => <ContactCard contact={item} onPress={selectContact} />} showsVerticalScrollIndicator={false} contentContainerStyle={{ paddingBottom: 100 }} + estimatedItemSize={80} + keyboardShouldPersistTaps="handled" refreshControl={<RefreshControl refreshing={refreshing} onRefresh={handleRefresh} />} />As per coding guidelines (optimize FlatList).
src/app/(app)/notes.tsx (2)
49-55: Avoid double-filtering notes during search analyticsYou filter once in filteredNotes and again in handleSearchChange to compute resultsCount. This doubles O(n) work per keystroke.
Track search analytics in an effect keyed by searchQuery and reuse filteredNotes.length:
- const handleSearchChange = React.useCallback( - (query: string) => { - setSearchQuery(query); - if (query.trim() && query !== searchQuery) { - const resultsCount = query.trim() - ? notes.filter((note) => note.Title.toLowerCase().includes(query.toLowerCase()) || note.Body.toLowerCase().includes(query.toLowerCase()) || note.Category?.toLowerCase().includes(query.toLowerCase())).length - : notes.length; - trackEvent('notes_searched', { timestamp: new Date().toISOString(), searchQuery: query, resultsCount }); - } - }, - [setSearchQuery, trackEvent, searchQuery, notes] - ); + const handleSearchChange = React.useCallback((query: string) => setSearchQuery(query), [setSearchQuery]); + + React.useEffect(() => { + const q = searchQuery.trim(); + if (!q) return; + trackEvent('notes_searched', { + timestamp: new Date().toISOString(), + searchQuery, + resultsCount: filteredNotes.length, + }); + }, [searchQuery, filteredNotes.length, trackEvent]);As per coding guidelines (minimize heavy work during render).
Also applies to: 56-73
112-119: FlashList: add estimatedItemSize and keyboard tap behaviorImproves scroll perf while typing.
Example:
<FlashList data={filteredNotes} keyExtractor={(item) => item.NoteId} renderItem={({ item }) => <NoteCard note={item} onPress={handleNoteSelect} />} showsVerticalScrollIndicator={false} contentContainerStyle={{ paddingBottom: 100 }} + estimatedItemSize={84} + keyboardShouldPersistTaps="handled" refreshControl={<RefreshControl refreshing={refreshing} onRefresh={handleRefresh} />} />As per coding guidelines (optimize FlatList).
src/components/ui/__tests__/utils-toast-migration.test.tsx (1)
12-21: Stabilize tests with fake timers and flush pending timeoutsshowToast auto-removes toasts after duration via setTimeout. Using real timers risks flakiness and leaking timers across tests.
Apply:
describe('Utils Toast Migration', () => { - beforeEach(() => { + beforeAll(() => { + jest.useFakeTimers(); + }); + afterAll(() => { + jest.useRealTimers(); + }); + beforeEach(() => { // Clear console log mock jest.clearAllMocks(); // Clear the store before each test - useToastStore.getState().toasts.forEach((toast) => { - useToastStore.getState().removeToast(toast.id); - }); + useToastStore.setState({ toasts: [] }); // Mock console.log jest.spyOn(console, 'log').mockImplementation(() => { }); }); afterEach(() => { jest.restoreAllMocks(); + // Ensure timers from auto-dismiss are flushed + jest.runOnlyPendingTimers(); });As per testing best practices.
Also applies to: 23-25
src/components/calendar/calendar-item-details-sheet.tsx (3)
305-310: onNavigationStateChange cannot block navigation; rely on onShouldStartLoadWithRequestReturning false here has no effect; the blocking is already correctly handled by onShouldStartLoadWithRequest.
Remove this handler to avoid confusion:
- onNavigationStateChange={(navState) => { - if (navState.url !== 'about:blank' && !navState.url.startsWith('data:')) { - return false; - } - }}Behavior remains unchanged and clearer.
317-317: Avoid 'unsafe-inline' by using injectedJavaScript; keeps CSP stricterAllowing inline scripts weakens CSP. Move the height script to injectedJavaScript and remove the script tag.
Sketch:
- <WebView + <WebView ... - source={{ html: `...<meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src 'unsafe-inline'; script-src 'unsafe-inline';">...<script>/* height logic */</script>...` }} + injectedJavaScript={` + (function(){ + function updateHeight(){ + var h = document.body.scrollHeight || 0; + window.ReactNativeWebView && window.ReactNativeWebView.postMessage(String(h)); + } + document.addEventListener('DOMContentLoaded', updateHeight); + window.addEventListener('resize', updateHeight); + if (typeof MutationObserver !== 'undefined') { + new MutationObserver(updateHeight).observe(document.body, { childList: true, subtree: true, attributes: true, characterData: true }); + } + updateHeight(); + })(); + true; + `} + source={{ html: `...<meta http-equiv="Content-Security-Policy" content="default-src 'none'; style-src 'unsafe-inline';">...` }} />This retains functionality without inline scripts. Based on learnings.
Also applies to: 334-361
294-301: Throttle height updates to reduce state churnMultiple rapid messages (resize/mutations) can cause excessive re-renders. Throttle updates.
Example:
- onMessage={(event) => { - const height = parseInt(event.nativeEvent.data, 10); - if (height && height > 0) { - setWebViewHeight(Math.max(height + 20, 120)); - } - }} + onMessage={(() => { + let last = 0; + return (event) => { + const now = Date.now(); + if (now - last < 100) return; + last = now; + const height = parseInt(event.nativeEvent.data, 10); + if (height && height > 0) { + setWebViewHeight(Math.max(height + 20, 120)); + } + }; + })()}src/components/toast/__tests__/toast-container-integration.test.tsx (2)
15-21: Stabilize tests: fake timers and direct store resetAuto-removal uses setTimeout; adopt fake timers and hard-reset the store to avoid flakiness and act warnings.
Apply:
@@ describe('ToastContainer Integration', () => { - beforeEach(() => { - // Clear the store before each test - useToastStore.getState().toasts.forEach((toast) => { - useToastStore.getState().removeToast(toast.id); - }); - }); + beforeAll(() => { + jest.useFakeTimers(); + }); + afterAll(() => { + jest.useRealTimers(); + }); + beforeEach(() => { + // Hard reset store + useToastStore.setState({ toasts: [] }); + });
23-35: Consider longer durations or advancing timers explicitlyWhen asserting presence, either enqueue with very long durations (e.g., Number.MAX_SAFE_INTEGER) or advance timers inside act() if you later test auto-dismissal. Current values are fine but brittle on slow CI.
src/components/toast/toast-container.tsx (1)
16-43: Use ternary instead of && for conditional rendering (project guideline)Replace && blocks with ?: null per repo TSX conventions. Also consider pointerEvents="box-none" on each VStack to avoid intercepting taps outside the toast.
Apply:
@@ - {/* Top positioned toasts */} - {topToasts.length > 0 && ( - <VStack className="pt-safe-top absolute inset-x-0 top-0 z-50 mt-20" space="sm"> - {topToasts.map((toast) => ( - <ToastMessage key={toast.id} {...toast} /> - ))} - </VStack> - )} + {/* Top positioned toasts */} + {topToasts.length > 0 ? ( + <VStack + pointerEvents="box-none" + className="pt-safe-top absolute inset-x-0 top-0 z-50 mt-20" + space="sm" + > + {topToasts.map((toast) => ( + <ToastMessage key={toast.id} {...toast} /> + ))} + </VStack> + ) : null} @@ - {/* Center positioned toasts */} - {centerToasts.length > 0 && ( - <VStack className="absolute inset-x-0 top-1/2 z-50 -translate-y-1/2" space="sm"> - {centerToasts.map((toast) => ( - <ToastMessage key={toast.id} {...toast} /> - ))} - </VStack> - )} + {/* Center positioned toasts */} + {centerToasts.length > 0 ? ( + <VStack + pointerEvents="box-none" + className="absolute inset-x-0 top-1/2 z-50 -translate-y-1/2" + space="sm" + > + {centerToasts.map((toast) => ( + <ToastMessage key={toast.id} {...toast} /> + ))} + </VStack> + ) : null} @@ - {/* Bottom positioned toasts */} - {bottomToasts.length > 0 && ( - <VStack className="pb-safe-bottom absolute inset-x-0 bottom-0 z-50 mb-20" space="sm"> - {bottomToasts.map((toast) => ( - <ToastMessage key={toast.id} {...toast} /> - ))} - </VStack> - )} + {/* Bottom positioned toasts */} + {bottomToasts.length > 0 ? ( + <VStack + pointerEvents="box-none" + className="pb-safe-bottom absolute inset-x-0 bottom-0 z-50 mb-20" + space="sm" + > + {bottomToasts.map((toast) => ( + <ToastMessage key={toast.id} {...toast} /> + ))} + </VStack> + ) : null}Optional: unify import path to
@/stores/toast/storefor consistency with alias usage elsewhere.src/components/toast/toast.tsx (2)
6-8: Remove unused import
useToastStoreis imported but unused. Clean it up to satisfy lint.-import { type ToastType, useToastStore } from '../../stores/toast/store'; +import { type ToastType, type ToastPosition } from '../../stores/toast/store';
9-16: Type position precisely; keep API aligned with storeUse ToastPosition instead of string. Future-proofing and better type safety.
export const ToastMessage: React.FC<{ //id: string; type: ToastType; title?: string; message: string; - position?: string; + position?: ToastPosition; duration?: number; }> = ({ /*id,*/ type, title, message /*, position, duration*/ }) => {src/app/(app)/__tests__/calendar.test.tsx (1)
400-426: Refine test naming to match actual assertions.The test names suggest validation of "proper scrolling configuration" including
showsVerticalScrollIndicatorandestimatedItemSize, but the tests only assert FlatList presence viatestID. The comments acknowledge that prop validation "would be tested through integration tests," which creates a mismatch between the test name and what it actually verifies.Consider either:
- Renaming the tests to reflect their actual scope (e.g., "renders FlatList for multiple today/upcoming items"), or
- Enhancing the mock to expose props for validation if you want to test configuration in unit tests.
Example rename:
- it('renders FlatList with proper scrolling configuration for today items', () => { + it('renders FlatList for multiple today items', () => {- it('renders FlatList with proper scrolling configuration for upcoming items', () => { + it('renders FlatList for multiple upcoming items', () => {Also applies to: 574-601
src/components/toast/__tests__/toast-migration.test.tsx (1)
7-12: Simplify store cleanup with a direct state reset.The current cleanup iterates over toasts and removes them individually. Consider resetting the store state directly for cleaner, more efficient test setup.
Apply this diff to simplify the cleanup:
beforeEach(() => { - // Clear the store before each test - useToastStore.getState().toasts.forEach((toast) => { - useToastStore.getState().removeToast(toast.id); - }); + // Reset the store to initial state + useToastStore.setState({ toasts: [] }); });src/components/toast/__tests__/feature-parity.test.tsx (1)
14-18: Simplify store cleanup with a direct state reset.Same pattern as in
toast-migration.test.tsx: iterating and removing toasts individually is less efficient than resetting state directly.Apply this diff:
beforeEach(() => { - // Clear the store before each test - useToastStore.getState().toasts.forEach((toast) => { - useToastStore.getState().removeToast(toast.id); - }); + // Reset the store to initial state + useToastStore.setState({ toasts: [] }); jest.clearAllMocks(); jest.spyOn(console, 'log').mockImplementation(() => { }); });docs/auth-token-refresh-implementation.md (1)
1-327: Excellent comprehensive documentation for token refresh implementation.This documentation provides thorough coverage of the authentication token refresh enhancement, including problem statement, solution architecture, implementation details, testing strategy, monitoring approach, and deployment considerations. The code examples are clear, the logging strategy is well-defined, and the operational guidance (monitoring, alerts) will help maintain the system in production.
Consider adding cross-references to the actual implementation files for easier navigation. For example:
### 1. Enhanced Authentication State Management + +*Implementation: `src/stores/auth/store.tsx`* **New State Fields:**### Comprehensive Test Coverage + +*Test files: `src/stores/auth/__tests__/`* **Test Files Created:**This would help developers quickly locate the corresponding code while reading the documentation.
src/stores/auth/__tests__/store-token-refresh.test.ts (1)
201-224: Test validates token update but not automatic refresh scheduling.The test verifies that tokens are successfully refreshed but only comments that "we can't easily test the exact setTimeout call." Since the store implementation schedules automatic refresh after a successful refresh (lines 260-273 in store.tsx), consider whether this critical behavior should be validated.
Consider enhancing this test to verify the automatic refresh scheduling behavior more explicitly. You could:
- Spy on
setTimeoutto verify it was called with expected timing- Advance fake timers and verify the refresh is triggered again
- Or at minimum, add a comment explaining why this behavior isn't tested
Example enhancement:
it('should set up automatic token refresh after successful refresh', async () => { + const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); const mockResponse = { access_token: 'new-access-token', refresh_token: 'new-refresh-token', id_token: 'new-id-token', expires_in: 3600, // 1 hour token_type: 'Bearer', expiration_date: new Date(Date.now() + 3600 * 1000).toISOString(), }; mockedRefreshTokenRequest.mockResolvedValueOnce(mockResponse); await useAuthStore.getState().refreshAccessToken(); // Check that the state shows the tokens were refreshed const state = useAuthStore.getState(); expect(state.accessToken).toBe('new-access-token'); expect(state.refreshToken).toBe('new-refresh-token'); - // Verify that a timeout was set up (we can't easily test the exact setTimeout call, - // but we can verify the tokens were updated which happens before setTimeout) + // Verify automatic refresh was scheduled (3600s - 5min = 3300s) + expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 3300 * 1000); expect(state.accessTokenObtainedAt).toBeGreaterThan(Date.now() - 1000); });src/stores/auth/store.tsx (2)
305-377: Hydration logic is solid with one edge case consideration.The hydration flow properly:
- Validates token format
- Computes token ages and expiry states
- Handles expired refresh tokens by forcing logout
- Schedules refresh for expired access tokens
However, the setTimeout on line 371 (100ms delay) could race with the automatic refresh scheduling that happens after setting state. Consider whether both timers are needed or if they could conflict.
The 100ms setTimeout (line 371) to trigger refresh after hydration might conflict with the automatic refresh scheduling in the login flow. Consider:
- Calling
refreshAccessToken()directly instead of using setTimeout if you want immediate refresh- Or documenting why the 100ms delay is necessary
- Ensuring that the refresh timer management (from the previous comment) prevents duplicate timers
// If access token is expired but refresh token is valid, attempt refresh if (isAccessExpired) { logger.info({ message: 'Access token expired during hydration, attempting refresh', context: { userId: profileData.sub }, }); - // Use setTimeout to avoid blocking hydration - setTimeout(() => { - const state = get(); - if (state.isAuthenticated()) { - state.refreshAccessToken(); - } - }, 100); + // Trigger immediate refresh for expired access token + const state = get(); + if (state.isAuthenticated()) { + state.refreshAccessToken(); + } }
435-435: Consider making token expiry configurable.The access token expiry time is hardcoded to 3600 seconds (1 hour). The
AuthResponseincludes anexpires_infield that could be used for more accurate expiry checking. Consider storing and using the actualexpires_invalue from the token response.Consider storing
expires_inin the state and using it for expiry calculation instead of the hardcoded 3600 seconds:export interface AuthState { accessToken: string | null; refreshToken: string | null; refreshTokenExpiresOn: string | null; accessTokenObtainedAt: number | null; refreshTokenObtainedAt: number | null; + accessTokenExpiresIn: number | null; // seconds status: AuthStatus; // ... rest of state } isAccessTokenExpired: (): boolean => { const state = get(); if (!state.accessTokenObtainedAt || !state.accessToken) { return true; } const now = Date.now(); const tokenAge = now - state.accessTokenObtainedAt; - const expiryTime = 3600 * 1000; // 1 hour in milliseconds (default) + const expiryTime = (state.accessTokenExpiresIn || 3600) * 1000; return tokenAge >= expiryTime; },Then update login and refresh flows to store the actual
expires_invalue from the response.src/components/calls/dispatch-selection-modal.tsx (5)
291-292: Use i18n pluralization for section headers with counts.Enables proper grammar in other locales and avoids manual "(N)".
- {t('calls.groups')} ({filteredData.groups.length}) + {t('calls.groups_count', { count: filteredData.groups.length, defaultValue: '{{count}} Groups' })} @@ - {t('calls.units')} ({filteredData.units.length}) + {t('calls.units_count', { count: filteredData.units.length, defaultValue: '{{count}} Units' })} @@ - {t('calls.roles')} ({filteredData.roles.length}) + {t('calls.roles_count', { count: filteredData.roles.length, defaultValue: '{{count}} Roles' })} @@ - {t('calls.users')} ({filteredData.users.length}) + {t('calls.users_count', { count: filteredData.users.length, defaultValue: '{{count}} Users' })}Also applies to: 317-318, 343-344, 369-370
295-296: Avoid inline arrow handlers in render; memoize per-id factories.Reduces re-creation on every render and aligns with repo guidelines.
- <TouchableOpacity onPress={() => handleToggleGroup(group.Id)}> + <TouchableOpacity onPress={onPressGroup(group.Id)}> @@ - <TouchableOpacity onPress={() => handleToggleUnit(unit.Id)}> + <TouchableOpacity onPress={onPressUnit(unit.Id)}> @@ - <TouchableOpacity onPress={() => handleToggleRole(role.Id)}> + <TouchableOpacity onPress={onPressRole(role.Id)}> @@ - <TouchableOpacity onPress={() => handleToggleUser(user.Id)}> + <TouchableOpacity onPress={onPressUser(user.Id)}>Add these inside the component (outside JSX):
const onPressGroup = useCallback((id: string) => () => handleToggleGroup(id), [handleToggleGroup]); const onPressUnit = useCallback((id: string) => () => handleToggleUnit(id), [handleToggleUnit]); const onPressRole = useCallback((id: string) => () => handleToggleRole(id), [handleToggleRole]); const onPressUser = useCallback((id: string) => () => handleToggleUser(id), [handleToggleUser]);Also applies to: 321-322, 347-348, 373-374
247-249: Add accessibility roles/labels for buttons and toggles.Improves WCAG/mobile accessibility.
- <TouchableOpacity onPress={handleCancel}> + <TouchableOpacity onPress={handleCancel} accessibilityRole="button" accessibilityLabel={t('common.close', { defaultValue: 'Close' })}> @@ - <TouchableOpacity onPress={handleToggleEveryone}> + <TouchableOpacity + onPress={handleToggleEveryone} + accessibilityRole="switch" + accessibilityState={{ checked: selection.everyone }} + accessibilityLabel={t('calls.everyone')} + > @@ - <TouchableOpacity onPress={onPressGroup(group.Id)}> + <TouchableOpacity + onPress={onPressGroup(group.Id)} + accessibilityRole="checkbox" + accessibilityState={{ checked: selection.groups.includes(group.Id) }} + accessibilityLabel={group.Name} + > @@ - <TouchableOpacity onPress={onPressUnit(unit.Id)}> + <TouchableOpacity + onPress={onPressUnit(unit.Id)} + accessibilityRole="checkbox" + accessibilityState={{ checked: selection.units.includes(unit.Id) }} + accessibilityLabel={unit.Name} + > @@ - <TouchableOpacity onPress={onPressRole(role.Id)}> + <TouchableOpacity + onPress={onPressRole(role.Id)} + accessibilityRole="checkbox" + accessibilityState={{ checked: selection.roles.includes(role.Id) }} + accessibilityLabel={role.Name} + > @@ - <TouchableOpacity onPress={onPressUser(user.Id)}> + <TouchableOpacity + onPress={onPressUser(user.Id)} + accessibilityRole="checkbox" + accessibilityState={{ checked: selection.users.includes(user.Id) }} + accessibilityLabel={user.Name} + >Also applies to: 275-285, 295-308, 321-334, 347-360, 373-386
271-271: Consider SectionList with 4 sections for virtualization and simpler markup.For larger datasets, ScrollView + multiple maps can drop frames. SectionList also gives sticky headers for Groups/Units/Roles/Users.
Outline:
const sections = useMemo(() => ([ { title: t('calls.groups'), key: 'groups', data: filteredData.groups, onToggle: onPressGroup, isSelected: (id:string)=>selection.groups.includes(id) }, { title: t('calls.units'), key: 'units', data: filteredData.units, onToggle: onPressUnit, isSelected: (id:string)=>selection.units.includes(id) }, { title: t('calls.roles'), key: 'roles', data: filteredData.roles, onToggle: onPressRole, isSelected: (id:string)=>selection.roles.includes(id) }, { title: t('calls.users'), key: 'users', data: filteredData.users, onToggle: onPressUser, isSelected: (id:string)=>selection.users.includes(id) }, ].filter(s => s.data.length)), [filteredData, selection]);Then render a single SectionList with a memoized Item component.
Also applies to: 287-337, 339-389
412-414: Pluralize the “selected” footer text.Improves localization.
- <Text className="mb-3 text-sm text-neutral-500"> - {getSelectionCount()} {t('calls.selected')} - </Text> + <Text className="mb-3 text-sm text-neutral-500"> + {t('calls.selected_count', { count: getSelectionCount(), defaultValue: '{{count}} selected' })} + </Text>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
docs/auth-token-refresh-implementation.md(1 hunks)src/api/messaging/messages.ts(1 hunks)src/app/(app)/__tests__/calendar.test.tsx(2 hunks)src/app/(app)/calendar.tsx(4 hunks)src/app/(app)/contacts.tsx(1 hunks)src/app/(app)/home/calls.tsx(2 hunks)src/app/(app)/home/personnel.tsx(2 hunks)src/app/(app)/home/units.tsx(2 hunks)src/app/(app)/notes.tsx(1 hunks)src/app/(app)/protocols.tsx(1 hunks)src/app/(app)/shifts.tsx(2 hunks)src/app/_layout.tsx(2 hunks)src/components/calendar/calendar-item-details-sheet.tsx(5 hunks)src/components/calls/dispatch-selection-modal.tsx(3 hunks)src/components/toast/__tests__/feature-parity.test.tsx(1 hunks)src/components/toast/__tests__/toast-container-integration.test.tsx(1 hunks)src/components/toast/__tests__/toast-migration.test.tsx(1 hunks)src/components/toast/toast-container.tsx(1 hunks)src/components/toast/toast.tsx(1 hunks)src/components/ui/__tests__/utils-toast-migration.test.tsx(1 hunks)src/components/ui/utils.tsx(2 hunks)src/hooks/use-toast.ts(1 hunks)src/lib/auth/types.tsx(1 hunks)src/stores/auth/__tests__/store-login-hydration.test.ts(1 hunks)src/stores/auth/__tests__/store-logout.test.ts(4 hunks)src/stores/auth/__tests__/store-token-refresh.test.ts(1 hunks)src/stores/auth/store.tsx(5 hunks)src/stores/toast/store.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/(app)/shifts.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
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
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
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/api/messaging/messages.tssrc/components/ui/utils.tsxsrc/app/(app)/home/personnel.tsxsrc/app/(app)/contacts.tsxsrc/stores/toast/store.tssrc/app/(app)/home/calls.tsxsrc/lib/auth/types.tsxsrc/stores/auth/store.tsxsrc/app/(app)/notes.tsxsrc/components/toast/__tests__/feature-parity.test.tsxsrc/hooks/use-toast.tssrc/app/_layout.tsxsrc/components/toast/toast.tsxsrc/components/calendar/calendar-item-details-sheet.tsxsrc/stores/auth/__tests__/store-token-refresh.test.tssrc/app/(app)/__tests__/calendar.test.tsxsrc/components/ui/__tests__/utils-toast-migration.test.tsxsrc/components/toast/toast-container.tsxsrc/app/(app)/protocols.tsxsrc/stores/auth/__tests__/store-login-hydration.test.tssrc/app/(app)/home/units.tsxsrc/components/toast/__tests__/toast-migration.test.tsxsrc/stores/auth/__tests__/store-logout.test.tssrc/components/toast/__tests__/toast-container-integration.test.tsxsrc/components/calls/dispatch-selection-modal.tsxsrc/app/(app)/calendar.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/api/messaging/messages.tssrc/components/ui/utils.tsxsrc/app/(app)/home/personnel.tsxsrc/app/(app)/contacts.tsxsrc/stores/toast/store.tssrc/app/(app)/home/calls.tsxsrc/lib/auth/types.tsxsrc/stores/auth/store.tsxsrc/app/(app)/notes.tsxsrc/components/toast/__tests__/feature-parity.test.tsxsrc/hooks/use-toast.tssrc/app/_layout.tsxsrc/components/toast/toast.tsxsrc/components/calendar/calendar-item-details-sheet.tsxsrc/stores/auth/__tests__/store-token-refresh.test.tssrc/app/(app)/__tests__/calendar.test.tsxsrc/components/ui/__tests__/utils-toast-migration.test.tsxsrc/components/toast/toast-container.tsxsrc/app/(app)/protocols.tsxsrc/stores/auth/__tests__/store-login-hydration.test.tssrc/app/(app)/home/units.tsxsrc/components/toast/__tests__/toast-migration.test.tsxsrc/stores/auth/__tests__/store-logout.test.tssrc/components/toast/__tests__/toast-container-integration.test.tsxsrc/components/calls/dispatch-selection-modal.tsxsrc/app/(app)/calendar.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
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 to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/components/ui/utils.tsxsrc/app/(app)/home/personnel.tsxsrc/app/(app)/contacts.tsxsrc/app/(app)/home/calls.tsxsrc/lib/auth/types.tsxsrc/stores/auth/store.tsxsrc/app/(app)/notes.tsxsrc/components/toast/__tests__/feature-parity.test.tsxsrc/app/_layout.tsxsrc/components/toast/toast.tsxsrc/components/calendar/calendar-item-details-sheet.tsxsrc/app/(app)/__tests__/calendar.test.tsxsrc/components/ui/__tests__/utils-toast-migration.test.tsxsrc/components/toast/toast-container.tsxsrc/app/(app)/protocols.tsxsrc/app/(app)/home/units.tsxsrc/components/toast/__tests__/toast-migration.test.tsxsrc/components/toast/__tests__/toast-container-integration.test.tsxsrc/components/calls/dispatch-selection-modal.tsxsrc/app/(app)/calendar.tsx
src/components/ui/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui components from components/ui; if unavailable, style via StyleSheet.create or styled-components
Files:
src/components/ui/utils.tsxsrc/components/ui/__tests__/utils-toast-migration.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/components/toast/__tests__/feature-parity.test.tsxsrc/stores/auth/__tests__/store-token-refresh.test.tssrc/app/(app)/__tests__/calendar.test.tsxsrc/components/ui/__tests__/utils-toast-migration.test.tsxsrc/stores/auth/__tests__/store-login-hydration.test.tssrc/components/toast/__tests__/toast-migration.test.tsxsrc/stores/auth/__tests__/store-logout.test.tssrc/components/toast/__tests__/toast-container-integration.test.tsx
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/toast/__tests__/feature-parity.test.tsxsrc/stores/auth/__tests__/store-token-refresh.test.tssrc/app/(app)/__tests__/calendar.test.tsxsrc/components/ui/__tests__/utils-toast-migration.test.tsxsrc/stores/auth/__tests__/store-login-hydration.test.tssrc/components/toast/__tests__/toast-migration.test.tsxsrc/stores/auth/__tests__/store-logout.test.tssrc/components/toast/__tests__/toast-container-integration.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Applied to files:
src/app/(app)/contacts.tsxsrc/app/(app)/home/calls.tsx
🧬 Code graph analysis (15)
src/components/ui/utils.tsx (1)
src/stores/toast/store.ts (1)
useToastStore(21-44)
src/stores/toast/store.ts (1)
src/components/toast/toast.tsx (1)
ToastMessage(9-28)
src/stores/auth/store.tsx (3)
src/lib/auth/types.tsx (1)
AuthResponse(11-19)src/lib/storage/index.tsx (2)
setItem(23-25)removeItem(27-29)src/lib/storage/secure-storage.ts (1)
set(118-130)
src/components/toast/__tests__/feature-parity.test.tsx (3)
src/stores/toast/store.ts (1)
useToastStore(21-44)src/hooks/use-toast.ts (1)
useToast(3-23)src/components/ui/utils.tsx (2)
showError(13-18)showErrorMessage(20-22)
src/hooks/use-toast.ts (1)
src/stores/toast/store.ts (3)
useToastStore(21-44)ToastType(3-3)ToastPosition(4-4)
src/app/_layout.tsx (1)
src/components/toast/toast-container.tsx (1)
ToastContainer(8-45)
src/components/calendar/calendar-item-details-sheet.tsx (2)
__mocks__/@/components/ui/vstack.tsx (1)
VStack(4-6)src/utils/webview-html.ts (1)
sanitizeHtmlContent(20-22)
src/stores/auth/__tests__/store-token-refresh.test.ts (1)
src/lib/storage/index.tsx (1)
setItem(23-25)
src/app/(app)/__tests__/calendar.test.tsx (2)
src/stores/calendar/store.ts (1)
useCalendarStore(62-338)src/app/(app)/calendar.tsx (1)
CalendarScreen(28-281)
src/components/ui/__tests__/utils-toast-migration.test.tsx (2)
src/stores/toast/store.ts (1)
useToastStore(21-44)src/components/ui/utils.tsx (2)
showError(13-18)showErrorMessage(20-22)
src/components/toast/toast-container.tsx (2)
src/stores/toast/store.ts (1)
useToastStore(21-44)src/components/toast/toast.tsx (1)
ToastMessage(9-28)
src/stores/auth/__tests__/store-login-hydration.test.ts (1)
src/lib/storage/index.tsx (1)
setItem(23-25)
src/components/toast/__tests__/toast-migration.test.tsx (2)
src/stores/toast/store.ts (1)
useToastStore(21-44)src/hooks/use-toast.ts (1)
useToast(3-23)
src/components/toast/__tests__/toast-container-integration.test.tsx (2)
src/stores/toast/store.ts (1)
useToastStore(21-44)src/components/toast/toast-container.tsx (1)
ToastContainer(8-45)
src/components/calls/dispatch-selection-modal.tsx (3)
__mocks__/@/components/ui/vstack.tsx (1)
VStack(4-6)__mocks__/@/components/ui/hstack.tsx (1)
HStack(4-6)__mocks__/@/components/ui/box.tsx (1)
Box(4-6)
🪛 GitHub Check: test
src/components/calls/dispatch-selection-modal.tsx
[warning] 299-299:
Delete ··
[warning] 298-298:
Insert ⏎··························
[warning] 325-325:
Delete ··
[warning] 324-324:
Insert ⏎··························
[warning] 377-377:
Delete ··
[warning] 376-376:
Insert ⏎··························
🪛 Gitleaks (8.28.0)
src/stores/auth/__tests__/store-login-hydration.test.ts
[high] 89-89: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 278-278: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
| export const sendMessage = async (messageData: SendMessageRequest) => { | ||
| const data = { | ||
| Subject: messageData.subject, | ||
| Title: messageData.subject, |
There was a problem hiding this comment.
🧩 Analysis chain
Subject -> Title rename looks good; verify backend and callers.
Change aligns with a server-side rename. Please confirm:
- Server now requires Title everywhere, and no remaining payloads still send Subject.
- Tests/fixtures reflect Title.
Run this to spot any lingering PascalCase Subject keys:
🏁 Script executed:
#!/bin/bash
# Find any payload objects still using PascalCase `Subject:`
rg -nP --type=ts --type=tsx -C2 '\bSubject\s*:'
# Also inventory where `Title:` is used now
rg -nP --type=ts --type=tsx -C2 '\bTitle\s*:'Length of output: 55
Please re-run verification with this updated script to ensure no Subject: keys remain and inventory Title: usage:
🏁 Script executed:
#!/bin/bash
# Search all .ts and .tsx files for lingering "Subject:" keys
rg -nP -g '*.ts' -g '*.tsx' -C2 '\bSubject\s*:'
# Inventory all occurrences of "Title:" keys
rg -nP -g '*.ts' -g '*.tsx' -C2 '\bTitle\s*:'Length of output: 33383
Rename remaining Subject keys to Title in tests and models
Tests (e.g. src/stores/messages/__tests__/store.test.ts) and the model src/models/v4/messages/messageResultData.ts still declare and reference PascalCase Subject; update them to Title to match the payload mapping change.
🤖 Prompt for AI Agents
In src/api/messaging/messages.ts around line 64, rename the remaining PascalCase
key "Subject" to "Title" so the payload maps consistently (e.g., change Title:
messageData.subject to Title: messageData.title and ensure any reference to
messageData.subject is updated). Then update the model
src/models/v4/messages/messageResultData.ts and any tests (for example
src/stores/messages/__tests__/store.test.ts) to replace properties and
assertions named Subject with Title, update any TypeScript interfaces/types and
fixtures to use Title, and run the test suite to catch any remaining references
to Subject.
| className={`size-5 items-center justify-center rounded border-2 ${selection.groups.includes(group.Id) ? 'border-blue-500 bg-blue-500' : colorScheme === 'dark' ? 'border-neutral-600' : 'border-neutral-300' | ||
| }`} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Fix minor formatting warnings flagged by CI.
Run the formatter to address whitespace/newline hints around these lines.
Also applies to: 324-325, 376-377
🏁 Script executed:
#!/bin/bash
# Try project scripts first; fall back to Prettier
npm run lint --silent -- --fix || npx prettier --write "src/**/*.ts" "src/**/*.tsx"Length of output: 89988
🏁 Script executed:
#!/bin/bash
sed -n '280,315p' src/components/calls/dispatch-selection-modal.tsx
sed -n '320,355p' src/components/calls/dispatch-selection-modal.tsx
sed -n '360,400p' src/components/calls/dispatch-selection-modal.tsxLength of output: 5964
Align template literal braces and apply Prettier: The closing } in the className template literal is on its own line at 298–299 (and similarly at 324–325, 376–377), triggering CI formatting warnings. Run
npx prettier --write src/components/calls/dispatch-selection-modal.tsx
to auto-fix.
🧰 Tools
🪛 GitHub Check: test
[warning] 299-299:
Delete ··
[warning] 298-298:
Insert ⏎··························
🤖 Prompt for AI Agents
In src/components/calls/dispatch-selection-modal.tsx around lines 298-299 (and
also fix the similar occurrences at 324-325 and 376-377): the closing brace for
the template literal used in the className is on its own line causing
Prettier/CI formatting errors; move the closing `}` back to the same line as the
template expression (so the ternary/template literal is properly aligned) and
then run `npx prettier --write
src/components/calls/dispatch-selection-modal.tsx` to apply consistent
formatting across the file.
| console.log(JSON.stringify(error?.response?.data)); | ||
| const description = extractError(error?.response?.data).trimEnd(); | ||
|
|
||
| showMessage({ | ||
| message: 'Error', | ||
| description, | ||
| type: 'danger', | ||
| duration: 4000, | ||
| icon: 'danger', | ||
| }); | ||
| useToastStore.getState().showToast('error', description, 'Error', 'top', 4000); | ||
| }; |
There was a problem hiding this comment.
Avoid logging raw error payloads; localize user text
- Do not log raw response bodies; risk of PII leakage. Gate logs to dev and log metadata only.
- Wrap user strings in i18n. Replace literal 'Error' and default 'Something went wrong ' with t()-backed keys.
Apply:
@@
-import { Dimensions, Platform } from 'react-native';
+import { Dimensions, Platform } from 'react-native';
+import { t } from 'i18next';
+import { logger } from '@/lib/logging';
@@
-export const showError = (error: AxiosError) => {
- console.log(JSON.stringify(error?.response?.data));
+export const showError = (error: AxiosError) => {
+ if (__DEV__) {
+ logger.warn({
+ message: 'API error',
+ context: { status: error?.response?.status, url: error?.config?.url, method: error?.config?.method },
+ });
+ }
const description = extractError(error?.response?.data).trimEnd();
-
- useToastStore.getState().showToast('error', description, 'Error', 'top', 4000);
+ useToastStore.getState().showToast(
+ 'error',
+ description || t('errors.genericMessage'),
+ t('errors.title'),
+ 'top',
+ 4000
+ );
};
@@
-export const showErrorMessage = (message: string = 'Something went wrong ') => {
- useToastStore.getState().showToast('error', message, undefined, 'top', 4000);
+export const showErrorMessage = (message: string = t('errors.genericMessage')) => {
+ useToastStore.getState().showToast('error', message, undefined, 'top', 4000);
};As per coding guidelines (i18n, privacy).
📝 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.
| console.log(JSON.stringify(error?.response?.data)); | |
| const description = extractError(error?.response?.data).trimEnd(); | |
| showMessage({ | |
| message: 'Error', | |
| description, | |
| type: 'danger', | |
| duration: 4000, | |
| icon: 'danger', | |
| }); | |
| useToastStore.getState().showToast('error', description, 'Error', 'top', 4000); | |
| }; | |
| import { Dimensions, Platform } from 'react-native'; | |
| import { t } from 'i18next'; | |
| import { logger } from '@/lib/logging'; | |
| import type { AxiosError } from 'axios'; | |
| import { extractError } from './error-utils'; | |
| import { useToastStore } from '../stores/toast-store'; | |
| export const showError = (error: AxiosError) => { | |
| if (__DEV__) { | |
| logger.warn({ | |
| message: 'API error', | |
| context: { | |
| status: error?.response?.status, | |
| url: error?.config?.url, | |
| method: error?.config?.method, | |
| }, | |
| }); | |
| } | |
| const description = extractError(error?.response?.data).trimEnd(); | |
| useToastStore.getState().showToast( | |
| 'error', | |
| description || t('errors.genericMessage'), | |
| t('errors.title'), | |
| 'top', | |
| 4000 | |
| ); | |
| }; | |
| export const showErrorMessage = (message: string = t('errors.genericMessage')) => { | |
| useToastStore.getState().showToast( | |
| 'error', | |
| message, | |
| undefined, | |
| 'top', | |
| 4000 | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In src/components/ui/utils.tsx around lines 14 to 18, avoid printing raw
response bodies and hard-coded user-facing strings: remove or wrap the
console.log so it only runs in non-production (e.g., process.env.NODE_ENV ===
'development') and log only metadata (status, code, message) instead of
error?.response?.data; sanitize/trim the extracted description and replace
literal strings with i18n keys (use t('toast.errorTitle') instead of 'Error' and
t('toast.defaultMessage') as the fallback when description is empty), then pass
the sanitized, localized strings into useToastStore.getState().showToast.
| obtained_at?: number; // Unix timestamp when token was obtained | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Clarify epoch units for obtained_at and ensure consistency project‑wide.
Is obtained_at seconds or milliseconds? Stores often use Date.now() (ms) while OAuth expires_in is seconds. Mismatched units will break expiry math.
Run to verify usages and unit math:
If stores/tests assume ms, either:
- Change to
obtained_at_ms(server/client mapping), or - Normalize on parse:
obtainedAtMs = (resp.obtained_at ?? Date.now()/1000) * 1000.
🏁 Script executed:
#!/bin/bash
# Find all references to obtained_at and nearby math with expires_in or Date.now
rg -n -C3 '\bobtained_at\b|expires_in|Date\.now\(' --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Search for obtained_at occurrences
rg -n -C2 'obtained_at' -g '*.ts' -g '*.tsx'
# Search for expires_in usage
rg -n -C2 'expires_in' -g '*.ts' -g '*.tsx'
# Search for Date.now() usage
rg -n -C2 'Date\.now\(' -g '*.ts' -g '*.tsx'Length of output: 46294
Document obtained_at as milliseconds
Update the comment on obtained_at in src/lib/auth/types.tsx to “milliseconds since Unix epoch when token was obtained.” Consider renaming it to obtained_at_ms for clarity.
🤖 Prompt for AI Agents
In src/lib/auth/types.tsx around lines 18 to 19, the comment for obtained_at is
ambiguous; update the comment to read "milliseconds since Unix epoch when token
was obtained" and, if you want clearer semantics, rename the field to
obtained_at_ms across the codebase (types, usages, serializers/deserializers and
any storage/transport logic) ensuring backward compatibility or migration where
necessary.
src/stores/auth/store.tsx
Outdated
| // Set up automatic token refresh 5 minutes before expiry | ||
| const expiresIn = (response.authResponse?.expires_in ?? 3600) * 1000 - 5 * 60 * 1000; | ||
| if (expiresIn > 0) { | ||
| setTimeout(() => { | ||
| const state = get(); | ||
| if (state.isAuthenticated() && state.shouldRefreshToken()) { | ||
| logger.info({ | ||
| message: 'Auto-refreshing token before expiry', | ||
| context: { userId: state.userId }, | ||
| }); | ||
| state.refreshAccessToken(); | ||
| } | ||
| }, expiresIn); | ||
| } |
There was a problem hiding this comment.
Potential race condition and memory leak with setTimeout.
The automatic token refresh scheduling has several issues:
- No cleanup mechanism: If the user logs out or the component unmounts before the timeout fires, the callback will still execute with stale state
- Multiple timers: If login is called multiple times (e.g., user logs in, logs out, logs in again quickly), multiple timers could be active
- Lost timer reference: There's no way to cancel the timer if needed
Apply this pattern to properly manage the refresh timer:
+let refreshTimerId: NodeJS.Timeout | null = null;
+
+const clearRefreshTimer = () => {
+ if (refreshTimerId) {
+ clearTimeout(refreshTimerId);
+ refreshTimerId = null;
+ }
+};
login: async (credentials: LoginCredentials) => {
// ... existing code ...
// Set up automatic token refresh 5 minutes before expiry
const expiresIn = (response.authResponse?.expires_in ?? 3600) * 1000 - 5 * 60 * 1000;
if (expiresIn > 0) {
- setTimeout(() => {
+ clearRefreshTimer(); // Cancel any existing timer
+ refreshTimerId = setTimeout(() => {
const state = get();
if (state.isAuthenticated() && state.shouldRefreshToken()) {
logger.info({
message: 'Auto-refreshing token before expiry',
context: { userId: state.userId },
});
state.refreshAccessToken();
}
}, expiresIn);
}
},
logout: async (reason?: string) => {
+ clearRefreshTimer(); // Cancel refresh timer on logout
// ... existing logout code ...
},Apply the same pattern to the setTimeout in refreshAccessToken (lines 260-273).
📝 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.
| // Set up automatic token refresh 5 minutes before expiry | |
| const expiresIn = (response.authResponse?.expires_in ?? 3600) * 1000 - 5 * 60 * 1000; | |
| if (expiresIn > 0) { | |
| setTimeout(() => { | |
| const state = get(); | |
| if (state.isAuthenticated() && state.shouldRefreshToken()) { | |
| logger.info({ | |
| message: 'Auto-refreshing token before expiry', | |
| context: { userId: state.userId }, | |
| }); | |
| state.refreshAccessToken(); | |
| } | |
| }, expiresIn); | |
| } | |
| // At the top of src/stores/auth/store.tsx (module scope) | |
| let refreshTimerId: ReturnType<typeof setTimeout> | null = null; | |
| const clearRefreshTimer = () => { | |
| if (refreshTimerId !== null) { | |
| clearTimeout(refreshTimerId); | |
| refreshTimerId = null; | |
| } | |
| }; | |
| // ... inside your zustand store definition ... | |
| login: async (credentials: LoginCredentials) => { | |
| // ... existing login logic ... | |
| // Set up automatic token refresh 5 minutes before expiry | |
| const expiresIn = | |
| (response.authResponse?.expires_in ?? 3600) * 1000 - 5 * 60 * 1000; | |
| if (expiresIn > 0) { | |
| clearRefreshTimer(); // Cancel any existing timer | |
| refreshTimerId = setTimeout(() => { | |
| const state = get(); | |
| if (state.isAuthenticated() && state.shouldRefreshToken()) { | |
| logger.info({ | |
| message: 'Auto-refreshing token before expiry', | |
| context: { userId: state.userId }, | |
| }); | |
| state.refreshAccessToken(); | |
| } | |
| }, expiresIn); | |
| } | |
| }, | |
| logout: async (reason?: string) => { | |
| clearRefreshTimer(); // Cancel refresh timer on logout | |
| // ... existing logout logic ... | |
| }, | |
| // ... later in refreshAccessToken, apply the same pattern around its setTimeout ... |
src/stores/auth/store.tsx
Outdated
| // Set up next token refresh 5 minutes before expiry | ||
| const expiresIn = response.expires_in * 1000 - 5 * 60 * 1000; | ||
| if (expiresIn > 0) { | ||
| setTimeout(() => { | ||
| const state = get(); | ||
| if (state.isAuthenticated() && state.shouldRefreshToken()) { | ||
| logger.info({ | ||
| message: 'Auto-refreshing token before expiry (from previous refresh)', | ||
| context: { userId: state.userId }, | ||
| }); | ||
| state.refreshAccessToken(); | ||
| } | ||
| }, expiresIn); | ||
| } |
There was a problem hiding this comment.
Same timer management issue as in login.
This setTimeout has the same race condition and memory leak issues identified in the login flow (lines 105-118). The timer should be properly managed with cleanup.
See the fix suggested for lines 105-118. Apply the same timer management pattern here to prevent multiple concurrent timers and ensure proper cleanup.
🤖 Prompt for AI Agents
In src/stores/auth/store.tsx around lines 260 to 273, the setTimeout for
auto-refreshing the token creates the same race condition and memory leak as the
login flow; replace this ad-hoc timer with the shared timer-management pattern
used at lines 105-118: ensure you clear any existing refresh timer before
creating a new one, store the returned timer id in state so it can be cleared
later, and make sure timers are cleared on logout/cleanup to avoid multiple
concurrent timers and memory leaks.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
src/components/calendar/calendar-item-details-sheet.tsx (5)
37-37: Extract a shared MIN_WEBVIEW_HEIGHT constant (avoid magic number).
Use a named constant and reuse it where clamping occurs.+const MIN_WEBVIEW_HEIGHT = 120; ... -const [webViewHeight, setWebViewHeight] = useState(120); +const [webViewHeight, setWebViewHeight] = useState(MIN_WEBVIEW_HEIGHT);
237-238: Good fix for the prior 'vh' issue; prefer flex + maxHeight for robustness.
Percent heights require a bounded parent; flex avoids layout surprises in varied sheet sizes.- <VStack style={{ height: '80%' }}> + <VStack style={{ flex: 1, maxHeight: '80%' }}>
286-301: De‑noise height updates to prevent re-render thrash.
Only update when height meaningfully changes and reuse the min-height constant.- style={[styles.container, { height: webViewHeight }]} + style={[styles.container, { height: webViewHeight }]} ... - onMessage={(event) => { - const height = parseInt(event.nativeEvent.data, 10); - if (height && height > 0) { - // Add some padding to ensure all content is visible - setWebViewHeight(Math.max(height + 20, 120)); - } - }} + onMessage={(event) => { + const raw = Number.parseInt(event.nativeEvent.data, 10); + if (!Number.isNaN(raw) && raw > 0) { + const next = Math.max(raw + 20, MIN_WEBVIEW_HEIGHT); + if (Math.abs(next - webViewHeight) > 2) { + setWebViewHeight(next); + } + } + }}
301-310: onNavigationStateChange return value is ignored; rely on onShouldStartLoadWithRequest.
Returning false here doesn’t block navigation; your onShouldStartLoadWithRequest already enforces it. Remove this handler (or keep it for analytics only).- onNavigationStateChange={(navState) => { - // Prevent any navigation away from the initial HTML - if (navState.url !== 'about:blank' && !navState.url.startsWith('data:')) { - return false; - } - }}
368-368: androidLayerType='software' may impact perf; keep only if required.
If this is a workaround for known GPU artifacts, OK; otherwise consider default layering for smoother scrolling.docs/full-screen-location-picker-integration.md (1)
28-68: Usage example looks good, but consider TypeScript typing for consistency.The usage example demonstrates the component integration well. However,
selectedLocationand the parameter inhandleLocationSelectedlack explicit typing.Consider adding explicit types for better type safety:
- const [selectedLocation, setSelectedLocation] = useState(null); + const [selectedLocation, setSelectedLocation] = useState<{ latitude: number; longitude: number } | null>(null); - const handleLocationSelected = (location) => { + const handleLocationSelected = (location: { latitude: number; longitude: number }) => {src/stores/auth/__tests__/store-login-hydration.test.ts (1)
143-167: Test comment suggests verification of auto-refresh setup, but test doesn't verify it.The comment on line 143 says "should set up automatic token refresh after successful login," but the test only verifies that login succeeded and tokens were set. It doesn't verify that any automatic refresh mechanism was initialized (e.g., checking for a scheduled refresh, verifying a refresh timer was started, etc.).
If automatic token refresh is truly set up during login, consider adding assertions to verify that setup. Otherwise, update the test description to reflect what it actually validates (e.g., "should successfully login and set token timestamps").
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (23)
docs/full-screen-location-picker-integration.md(1 hunks)docs/token-refresh-race-condition-fix.md(1 hunks)package.json(1 hunks)src/api/common/__tests__/client-token-refresh.test.ts(1 hunks)src/api/common/client.tsx(5 hunks)src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx(1 hunks)src/components/calendar/calendar-item-details-sheet.tsx(5 hunks)src/components/calls/__tests__/call-images-modal.test.tsx(1 hunks)src/components/maps/__tests__/full-screen-location-picker.test.tsx(5 hunks)src/components/maps/__tests__/location-picker.test.tsx(3 hunks)src/components/maps/full-screen-location-picker.tsx(7 hunks)src/components/maps/location-picker.tsx(5 hunks)src/components/ui/__tests__/shared-tabs.test.tsx(1 hunks)src/components/ui/shared-tabs-usage-example.tsx(1 hunks)src/components/ui/shared-tabs.tsx(5 hunks)src/stores/app/__tests__/core-store.test.ts(1 hunks)src/stores/auth/__tests__/store-login-hydration.test.ts(1 hunks)src/stores/auth/__tests__/store-logout.test.ts(4 hunks)src/stores/auth/__tests__/store-token-refresh.test.ts(1 hunks)src/stores/auth/store.tsx(4 hunks)src/translations/ar.json(3 hunks)src/translations/en.json(3 hunks)src/translations/es.json(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/ui/shared-tabs-usage-example.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
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
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
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/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/maps/__tests__/location-picker.test.tsxsrc/components/maps/__tests__/full-screen-location-picker.test.tsxsrc/stores/auth/__tests__/store-logout.test.tssrc/api/common/__tests__/client-token-refresh.test.tssrc/components/calls/__tests__/call-images-modal.test.tsxsrc/api/common/client.tsxsrc/stores/auth/__tests__/store-login-hydration.test.tssrc/components/ui/__tests__/shared-tabs.test.tsxsrc/components/calendar/calendar-item-details-sheet.tsxsrc/components/ui/shared-tabs.tsxsrc/stores/auth/__tests__/store-token-refresh.test.tssrc/stores/auth/store.tsxsrc/components/maps/full-screen-location-picker.tsxsrc/stores/app/__tests__/core-store.test.tssrc/components/maps/location-picker.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
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 to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/maps/__tests__/location-picker.test.tsxsrc/components/maps/__tests__/full-screen-location-picker.test.tsxsrc/components/calls/__tests__/call-images-modal.test.tsxsrc/api/common/client.tsxsrc/components/ui/__tests__/shared-tabs.test.tsxsrc/components/calendar/calendar-item-details-sheet.tsxsrc/components/ui/shared-tabs.tsxsrc/stores/auth/store.tsxsrc/components/maps/full-screen-location-picker.tsxsrc/components/maps/location-picker.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/maps/__tests__/location-picker.test.tsxsrc/components/maps/__tests__/full-screen-location-picker.test.tsxsrc/stores/auth/__tests__/store-logout.test.tssrc/api/common/__tests__/client-token-refresh.test.tssrc/components/calls/__tests__/call-images-modal.test.tsxsrc/api/common/client.tsxsrc/stores/auth/__tests__/store-login-hydration.test.tssrc/components/ui/__tests__/shared-tabs.test.tsxsrc/translations/es.jsonsrc/components/calendar/calendar-item-details-sheet.tsxsrc/translations/en.jsonsrc/components/ui/shared-tabs.tsxsrc/stores/auth/__tests__/store-token-refresh.test.tssrc/stores/auth/store.tsxsrc/components/maps/full-screen-location-picker.tsxsrc/stores/app/__tests__/core-store.test.tssrc/translations/ar.jsonsrc/components/maps/location-picker.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/maps/__tests__/location-picker.test.tsxsrc/components/maps/__tests__/full-screen-location-picker.test.tsxsrc/stores/auth/__tests__/store-logout.test.tssrc/api/common/__tests__/client-token-refresh.test.tssrc/components/calls/__tests__/call-images-modal.test.tsxsrc/stores/auth/__tests__/store-login-hydration.test.tssrc/components/ui/__tests__/shared-tabs.test.tsxsrc/stores/auth/__tests__/store-token-refresh.test.tssrc/stores/app/__tests__/core-store.test.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsxsrc/components/maps/__tests__/location-picker.test.tsxsrc/components/maps/__tests__/full-screen-location-picker.test.tsxsrc/stores/auth/__tests__/store-logout.test.tssrc/api/common/__tests__/client-token-refresh.test.tssrc/components/calls/__tests__/call-images-modal.test.tsxsrc/stores/auth/__tests__/store-login-hydration.test.tssrc/components/ui/__tests__/shared-tabs.test.tsxsrc/stores/auth/__tests__/store-token-refresh.test.tssrc/stores/app/__tests__/core-store.test.ts
src/components/ui/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui components from components/ui; if unavailable, style via StyleSheet.create or styled-components
Files:
src/components/ui/__tests__/shared-tabs.test.tsxsrc/components/ui/shared-tabs.tsx
src/translations/**/*.json
📄 CodeRabbit inference engine (.cursorrules)
Store translation dictionary files under src/translations as JSON resources
Files:
src/translations/es.jsonsrc/translations/en.jsonsrc/translations/ar.json
src/translations/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store translation dictionary files in src/translations
Files:
src/translations/es.jsonsrc/translations/en.jsonsrc/translations/ar.json
🧠 Learnings (5)
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Applied to files:
src/components/maps/__tests__/full-screen-location-picker.test.tsx
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
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/maps/__tests__/full-screen-location-picker.test.tsx
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to src/**/*.test.{ts,tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/components/ui/__tests__/shared-tabs.test.tsx
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps, mapping, or vehicle navigation
Applied to files:
src/components/maps/full-screen-location-picker.tsxsrc/components/maps/location-picker.tsx
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or navigation features
Applied to files:
src/components/maps/full-screen-location-picker.tsxsrc/components/maps/location-picker.tsx
🧬 Code graph analysis (8)
src/api/common/__tests__/client-token-refresh.test.ts (1)
src/api/common/client.tsx (1)
api(201-201)
src/stores/auth/__tests__/store-login-hydration.test.ts (1)
src/lib/storage/index.tsx (1)
setItem(23-25)
src/components/ui/__tests__/shared-tabs.test.tsx (1)
src/components/ui/shared-tabs.tsx (2)
TabItem(23-29)SharedTabs(45-191)
src/components/calendar/calendar-item-details-sheet.tsx (2)
__mocks__/@/components/ui/vstack.tsx (1)
VStack(4-6)src/utils/webview-html.ts (1)
sanitizeHtmlContent(20-22)
src/stores/auth/__tests__/store-token-refresh.test.ts (1)
src/lib/storage/index.tsx (1)
setItem(23-25)
src/stores/auth/store.tsx (3)
src/lib/auth/types.tsx (4)
AuthStatus(40-40)ProfileModel(26-38)LoginCredentials(6-9)AuthResponse(11-19)src/lib/storage/secure-storage.ts (1)
set(118-130)src/lib/storage/index.tsx (2)
setItem(23-25)removeItem(27-29)
src/components/maps/full-screen-location-picker.tsx (2)
env.js (1)
Env(170-170)src/services/location.ts (1)
locationService(349-349)
src/components/maps/location-picker.tsx (1)
env.js (1)
Env(170-170)
🪛 Gitleaks (8.28.0)
src/stores/auth/__tests__/store-login-hydration.test.ts
[high] 88-88: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
[high] 277-277: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
⏰ 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 (38)
package.json (1)
186-186: Scoped axios-mock-adapter to devDependenciesGood call keeping the mock adapter confined to the tooling side so it doesn’t bloat the runtime install set.
src/components/calendar/calendar-item-details-sheet.tsx (2)
334-334: No action required: strictSanitizeConfig already discards script and iframe tags, removes inline event handlers, and only allows safe URL schemes.
317-361: Harden CSP and move resize logic into injectedJavaScript
Tighten your<meta http-equiv="Content-Security-Policy">to disallow inline scripts (script-src 'none'), remove the inline<script>block, and inject your updateHeight routine via the WebView’s injectedJavaScript prop. Verify on both iOS and Android that height updates still occur and images render as expected.src/translations/en.json (2)
350-370: LGTM! New common translation keys added.The new keys under "common" (get_my_location, loading_address, no_address_found, no_location, set_location) are well-named and support the location-related features in this PR.
561-565: LGTM! New maps section added.The new "maps" section with keys for Mapbox configuration and error states is appropriately scoped and provides clear user-facing messages for map-related scenarios.
src/translations/ar.json (2)
350-370: LGTM! Arabic translations for common keys.The Arabic translations for the new common keys (get_my_location, loading_address, etc.) are consistent with the English translations and follow the existing translation patterns.
561-565: LGTM! Arabic translations for maps section.The new "maps" section with Arabic translations provides proper localization for map-related messages.
src/translations/es.json (2)
350-370: LGTM! Spanish translations for common keys.The Spanish translations for the new common keys are consistent with the English translations and properly localized.
561-565: LGTM! Spanish translations for maps section.The new "maps" section with Spanish translations provides proper localization for map-related error and configuration messages.
src/components/ui/__tests__/shared-tabs.test.tsx (1)
1-146: LGTM! Comprehensive test suite for SharedTabs.The test suite provides excellent coverage of the SharedTabs component, including:
- Rendering with various props (size, variant, titleFontSize)
- User interactions (tab press, onChange callback)
- Badge display
- ReactNode titles and icons
- Scrollable and non-scrollable modes
The tests properly mock dependencies (react-i18next, nativewind) and validate the new titleFontSize prop introduced in this PR.
src/components/ui/shared-tabs.tsx (3)
37-37: LGTM! New titleFontSize prop added.The new optional titleFontSize prop provides explicit control over tab title font sizing, with a well-defined type union covering common text size classes.
85-99: LGTM! Smart font sizing logic.The getTitleFontSize helper provides:
- Explicit override when titleFontSize prop is provided
- Smart defaults based on size and orientation (smaller fonts in portrait, larger in landscape)
- Centralized font sizing logic for maintainability
The orientation-aware defaults improve responsive design across device orientations.
162-162: LGTM! Consistent title rendering.Both scrollable and non-scrollable tab rendering paths now consistently apply:
- getTitleFontSize() for centralized font sizing
- getTextColor() for theme-aware text color
This ensures uniform styling across all tab variants.
Also applies to: 176-176
docs/token-refresh-race-condition-fix.md (1)
1-75: LGTM! Well-structured technical documentation.The documentation clearly describes the token refresh race condition fix, migration from timer-based to timestamp-based strategy, error handling approach, and benefits. The flow description and migration notes provide good context for developers.
src/stores/app/__tests__/core-store.test.ts (1)
339-339: LGTM! Mock correctly updated to reflect new token timestamp model.The change from
refreshTokenExpiresOntorefreshTokenObtainedAtaligns with the PR's shift to timestamp-based token tracking. Setting it tonullis appropriate for the "no user ID" test scenario.src/components/calls/__tests__/call-images-modal.test.tsx (1)
358-358: LGTM! Mock updated to use timestamp-based token tracking.The change to
refreshTokenObtainedAtwith a 30-day-old timestamp is appropriate for testing authenticated scenarios. This aligns with the PR's timestamp-based token refresh strategy.docs/full-screen-location-picker-integration.md (1)
1-117: LGTM! Comprehensive integration documentation.The documentation clearly describes the location picker integration with the location store, initialization priorities, error handling, and usage patterns. The benefits and testing sections provide good context for developers.
src/components/maps/__tests__/location-picker.test.tsx (4)
3-8: LGTM! Mapbox environment mock added.The Env mock provides a test key for Mapbox configuration tests, aligning with the component's need to check for Mapbox setup.
24-24: LGTM! i18n mock enhanced to support fallback pattern.The updated mock now accepts an optional fallback parameter, making tests more flexible when components provide default text.
28-30: LGTM! AlertTriangle icon mocked for UI tests.The lucide-react-native mock covers the AlertTriangle icon used in error states.
60-72: LGTM! Mapbox configuration detection tests added.The tests verify the boolean logic for detecting configured vs. unconfigured Mapbox keys, covering both empty and valid key scenarios.
src/api/common/__tests__/client-token-refresh.test.ts (4)
1-43: LGTM! Well-structured test setup.The test suite setup is comprehensive with proper mocking of dependencies, auth store initialization with timestamp fields, and cleanup in
afterEach. The mock configuration for storage, logging, and API endpoints provides good isolation for testing token refresh flows.
164-337: LGTM! Comprehensive response interceptor test coverage.The response interceptor tests thoroughly cover all the critical scenarios:
- Transient failures (5xx) that preserve login state with warning logs
- Permanent failures (4xx) that force logout with error logs
- Expired refresh token detection that skips refresh and logs out
- Successful token refresh followed by request retry
The assertions verify the correct state transitions, logging calls, and retry behavior for each scenario.
339-378: LGTM! Helper method tests validate token expiry logic.The tests correctly validate that
isAccessTokenExpiringSoon()andshouldRefreshToken()accurately identify tokens that need refresh (4 minutes until expiry) versus tokens that are still valid (30 minutes remaining).
125-161: Approve “fail open” behavior in request interceptor.On refresh failure,
refreshAccessTokenlogs out the user and the interceptor catches the error—allowing the API call to proceed (per the test’s expectations).src/stores/auth/__tests__/store-login-hydration.test.ts (6)
1-80: LGTM! Comprehensive test setup with proper mocking.The test setup properly mocks all external dependencies (storage, logging, auth API/utils) and initializes the auth store with the complete state including new timestamp fields. The use of fake timers is appropriate for testing time-dependent token expiry logic.
88-88: Static analysis false positive: test JWT token is safe.The JWT token on this line is a well-known example token used for testing purposes with a public payload (
{"sub":"test-user","name":"Test User"}). It poses no security risk in test code and is appropriate for validating JWT parsing logic.
90-141: LGTM! Thorough test of successful login flow.The test comprehensively validates the successful login path including:
- API call with correct credentials
- Storage persistence with timestamp
- State updates (tokens, status, userId, profile, timestamps)
- Structured logging with context
169-273: LGTM! Comprehensive error handling test coverage.The login failure tests thoroughly cover all error scenarios:
- Unsuccessful API response
- Missing ID token
- Invalid ID token format
- Network exceptions
Each test correctly verifies error state, null tokens, and appropriate error logging.
277-277: Static analysis false positive: test JWT token is safe.Same as line 88, this is a well-known example JWT token used for testing JWT parsing. It poses no security risk in test code.
279-444: LGTM! Comprehensive hydration test coverage.The hydration tests thoroughly validate all scenarios:
- Successful restoration of valid auth state with detailed logging
- Detection (but not automatic refresh) of expired access tokens during hydration, leaving refresh to the API interceptor
- Forced logout when refresh token is expired
- Graceful handling of missing stored auth
- Error handling for invalid ID tokens and storage failures
The tests correctly validate the passive hydration approach where expired access tokens are detected but not immediately refreshed, deferring refresh to the API interceptor on first request.
src/components/maps/location-picker.tsx (7)
1-24: LGTM! Imports and props properly updated.The additions align with project guidelines:
- Using
lucide-react-nativefor icons (AlertTriangle) as per coding guidelines- Env import for runtime configuration checks
- Default
height = 200parameter improves API ergonomics
34-45: LGTM! Proper runtime configuration validation.The
isMapboxConfiguredcheck and error state initialization provide good defensive programming. The configuration check validates both existence and non-empty value after trimming.Note: The
mapErrorstate set in the useEffect (lines 40-45) is somewhat redundant with the direct!isMapboxConfiguredcheck at line 117, but having both ensures consistent error messaging and could support future scenarios wheremapErroris set from other sources (like map load failures).
47-79: LGTM! Clean location fetch implementation.The addition of
hasAttemptedLocationFetchon line 49 prevents redundant location fetches, and the use of an intermediatenewLocationvariable (lines 59-64) improves code clarity.
81-98: LGTM! Improved location initialization logic.The enhanced useEffect properly guards against Mapbox misconfiguration (lines 82-85) and uses
hasAttemptedLocationFetchto prevent redundant location fetches. The updated dependency array correctly includes the new dependencies.
100-114: LGTM! Proper guard clause prevents interaction in error states.The addition of the guard clause at line 101 prevents map interactions when Mapbox is misconfigured or the map has failed to load, avoiding potential crashes or unexpected behavior.
116-134: LGTM! Clear error and loading state handling.The error UI (lines 117-125) provides excellent user feedback with clear messaging and admin contact instructions. The loading condition on line 128 (
isLoading && !currentLocation) prevents loading flicker during location updates by only showing loading when there's no location yet.
136-194: LGTM! Comprehensive map rendering with good fallback UX.The map rendering properly handles both scenarios:
- With current location: renders map with marker and camera centered on location
- Without location: renders fallback map (US center) with overlay prompting user to fetch location
The addition of
onDidFailLoadingMaphandlers (lines 148-151, 170-173) provides robust error handling for network or map loading failures.Minor note: There's some duplication between the two
MapViewcomponents (lines 139-158 and 161-185). While refactoring to reduce duplication is possible, the current approach is clear and maintainable given the different camera configurations and overlay requirements.
| // Note: JavaScript is enabled but restricted by CSP and sanitization | ||
| expect(webview.props['data-js-enabled']).toBe(true); | ||
| expect(webview.props['data-dom-storage']).toBe(false); |
There was a problem hiding this comment.
Re-enable the WebView hardening—keep JavaScript disabled
Switching the test to expect javaScriptEnabled to be true drops the last line of defense in this sheet. Our sanitizer (sanitizeHtmlContent) only strips <script> tags; it does not scrub inline handlers like <img onerror="...">. With JS now allowed, any residual on* attribute executes, so the CSP/sanitizer combination no longer prevents XSS. Please keep the WebView’s javaScriptEnabled flag false (and retain the existing assertion) unless the sanitizer is upgraded to strip all executable attributes and protocols.
🤖 Prompt for AI Agents
In
src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx
around lines 191 to 193, the test was changed to expect the WebView's JavaScript
to be enabled; revert that change so the WebView hardening remains in place by
asserting data-js-enabled (or javaScriptEnabled) is false and keep the existing
data-dom-storage assertion; ensure the test checks the property that the
component actually sets (match the prop name used in the WebView) and do not
enable JavaScript in the fixture unless the sanitizer is enhanced to remove
inline event handlers and dangerous protocols.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/maps/full-screen-location-picker.tsx (1)
388-391: Localize the annotation title.
title="Selected Location"is visible to users, but it's hard-coded in English. Please pass it throught(...)with a default so translations work.- <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title="Selected Location"> + <Mapbox.PointAnnotation + id="selectedLocation" + coordinate={[currentLocation.longitude, currentLocation.latitude]} + title={t('maps.selected_location', 'Selected location')} + >As per coding guidelines
🧹 Nitpick comments (1)
src/components/maps/full-screen-location-picker.tsx (1)
55-276: Drop the unusedhasAttemptedLocationFetchstate.
hasAttemptedLocationFetchis written in several places but never read, so it just adds noise. Removing it will simplify the component.- const [mapError, setMapError] = useState<string | null>(null); - const [hasAttemptedLocationFetch, setHasAttemptedLocationFetch] = useState(false); + const [mapError, setMapError] = useState<string | null>(null); @@ - setHasAttemptedLocationFetch(true); @@ - setHasAttemptedLocationFetch(true); @@ - setHasAttemptedLocationFetch(false); @@ - setHasAttemptedLocationFetch(true);As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/api/common/client.tsx(5 hunks)src/components/maps/full-screen-location-picker.tsx(7 hunks)src/stores/auth/store.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
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
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
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/maps/full-screen-location-picker.tsxsrc/api/common/client.tsxsrc/stores/auth/store.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
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 to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/components/maps/full-screen-location-picker.tsxsrc/api/common/client.tsxsrc/stores/auth/store.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/components/maps/full-screen-location-picker.tsxsrc/api/common/client.tsxsrc/stores/auth/store.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps, mapping, or vehicle navigation
Applied to files:
src/components/maps/full-screen-location-picker.tsx
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or navigation features
Applied to files:
src/components/maps/full-screen-location-picker.tsx
🧬 Code graph analysis (2)
src/components/maps/full-screen-location-picker.tsx (2)
env.js (1)
Env(170-170)src/services/location.ts (1)
locationService(349-349)
src/stores/auth/store.tsx (3)
src/lib/auth/types.tsx (4)
AuthStatus(40-40)ProfileModel(26-38)LoginCredentials(6-9)AuthResponse(11-19)src/lib/storage/secure-storage.ts (1)
set(118-130)src/lib/storage/index.tsx (2)
setItem(23-25)removeItem(27-29)
🪛 GitHub Check: test
src/components/maps/full-screen-location-picker.tsx
[warning] 32-32:
Insert ····
[warning] 31-31:
Insert ··
| console.error('Map failed to load'); | ||
| setMapError('Map failed to load'); | ||
| }} |
There was a problem hiding this comment.
Localize map load failure message.
The setMapError('Map failed to load') string is rendered to users but bypasses i18n, violating our localization requirement. Please wrap it with t(...) so non-English locales see a translated message.
- onDidFailLoadingMap={() => {
- console.error('Map failed to load');
- setMapError('Map failed to load');
+ onDidFailLoadingMap={() => {
+ console.error('Map failed to load');
+ setMapError(t('maps.map_failed_to_load', 'Map failed to load'));
}}
@@
- onDidFailLoadingMap={() => {
- console.error('Map failed to load');
- setMapError('Map failed to load');
+ onDidFailLoadingMap={() => {
+ console.error('Map failed to load');
+ setMapError(t('maps.map_failed_to_load', 'Map failed to load'));
}}As per coding guidelines
Also applies to: 407-409
🤖 Prompt for AI Agents
In src/components/maps/full-screen-location-picker.tsx around lines 382-384 (and
also apply the same change at lines 407-409), the user-facing error string
passed to setMapError is not localized; replace setMapError('Map failed to
load') with setMapError(t('Map failed to load')) (or the appropriate i18n key
usage in this codebase) and ensure the t import/context is available in the
component so the message is translated for all locales.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/stores/auth/store.tsx (1)
390-414: Consider using the actual token expiry fromAuthResponse.Both
isAccessTokenExpiredandisAccessTokenExpiringSoonhardcodeexpiryTime = 3600 * 1000(1 hour). While this is a common default, the actualexpires_invalue from the server is stored inAuthResponsebut not currently persisted in state. If the server issues tokens with different lifetimes, these methods will misreport expiry.Consider either:
- Adding
accessTokenExpiresIn: numbertoAuthStateand populating it during login/refresh, or- Reading
expires_infrom the storedauthResponsewithin these methodsExample for option 1:
interface AuthState { // Tokens accessToken: string | null; refreshToken: string | null; accessTokenObtainedAt: number | null; refreshTokenObtainedAt: number | null; + accessTokenExpiresIn: number; // in secondsThen update login, refresh, and hydration flows to set this value, and use it in the expiry calculations:
isAccessTokenExpired: (): boolean => { const state = get(); if (!state.accessTokenObtainedAt || !state.accessToken) { return true; } const now = Date.now(); const tokenAge = now - state.accessTokenObtainedAt; - const expiryTime = 3600 * 1000; // 1 hour in milliseconds (default) + const expiryTime = state.accessTokenExpiresIn * 1000; return tokenAge >= expiryTime; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/stores/auth/store.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
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
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
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/stores/auth/store.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
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 to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/stores/auth/store.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/stores/auth/store.tsx
🧬 Code graph analysis (1)
src/stores/auth/store.tsx (2)
src/lib/auth/types.tsx (4)
AuthStatus(40-40)ProfileModel(26-38)LoginCredentials(6-9)AuthResponse(11-19)src/lib/storage/index.tsx (3)
setItem(23-25)removeItem(27-29)getItem(18-21)
⏰ 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). (3)
- GitHub Check: test
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
src/stores/auth/store.tsx (6)
13-38: LGTM! Interface changes are well-structured.The addition of timestamp fields and computed token expiry methods provides a solid foundation for timestamp-based authentication management. The optional
reasonparameter onlogoutenables better observability for forced vs. voluntary logouts.
132-179: LGTM! Logout flow is well-implemented.The enhanced logging with reason-based distinction between forced and voluntary logouts provides excellent observability. All state fields, including the new timestamp fields, are properly reset.
181-265: Token refresh flow properly addresses past review concerns.The refresh token preservation logic (lines 221-222) correctly:
- Uses the new refresh token if the server provides one
- Falls back to the existing refresh token if not provided
- Only updates the timestamp when a new token is received
This properly resolves the critical issue from the past review about overwriting tokens with
undefined.
266-376: LGTM! Hydration flow is comprehensive and robust.The hydration logic properly:
- Validates token format and calculates token ages
- Checks refresh token expiry and forces logout if expired
- Sets
isFirstTimetofalseon success andtrueon failure- Provides detailed logging with contextual information
The comment at line 342 clarifies that the API interceptor handles token refresh, which aligns with the removal of setTimeout-based auto-refresh logic.
377-435: Computed properties are well-implemented.The token expiry and authentication checks provide a clean API for the rest of the application. The logic is clear and correct.
52-130: Confirm interceptor-handled token refresh; legacy timer logic removedVerified that the API interceptor in src/api/common/client.tsx handles all token refresh logic (using isAccessTokenExpiringSoon and shouldRefreshToken checks with try/catch), and the auth store no longer relies on any setTimeout for refresh.
| // Read existing stored auth response to preserve refresh token if not provided | ||
| const existingAuthResponse = getItem<AuthResponse>('authResponse'); |
There was a problem hiding this comment.
Remove unused variable.
The existingAuthResponse variable is read from storage but never used. The token preservation logic correctly uses currentState instead.
- // Read existing stored auth response to preserve refresh token if not provided
- const existingAuthResponse = getItem<AuthResponse>('authResponse');
-
// Determine which refresh token and timestamp to use📝 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.
| // Read existing stored auth response to preserve refresh token if not provided | |
| const existingAuthResponse = getItem<AuthResponse>('authResponse'); | |
| // Determine which refresh token and timestamp to use |
🤖 Prompt for AI Agents
In src/stores/auth/store.tsx around lines 217 to 218, the variable
existingAuthResponse is read from storage but never used; remove the const
existingAuthResponse = getItem<AuthResponse>('authResponse'); line and update or
remove the preceding comment if needed so the code no longer declares an unused
variable (ensure no other code relies on that variable and run TypeScript checks
after removal).
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Summary by CodeRabbit