Conversation
WalkthroughBroad repo-wide updates: CI runner bumped to macos-15; large dependency upgrades; extensive Jest/mock overhaul; new/updated TypeScript typings; numerous safer type and runtime guards across stores, services, and UI; Bluetooth audio service refactor; location picker/address handling extended; many test additions/removals and UI refactors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as App (RN)
participant BLE as BluetoothAudioService
participant BLEMgr as BleManager
participant Store as bluetooth-audio-store
User->>App: Launch / Enable BT scanning
App->>BLE: startScan()
BLE->>BLEMgr: start({ scan options })
BLEMgr-->>BLE: onDiscoverPeripheral(device)
BLE->>BLE: normalizeUuid(advertisedServiceUUIDs)
BLE->>BLE: isAudioDevice(device)
alt Strong RSSI and matches audio UUIDs
BLE->>Store: setDiscoveredDevice(device,rssi)
BLE->>BLEMgr: retrieveServices(device.id)
BLEMgr-->>BLE: services, characteristics
loop For each service x characteristic
BLE->>BLEMgr: startNotification(device, service, characteristic)
end
else Not audio
BLE->>BLE: ignore
end
BLEMgr-->>BLE: onDidUpdateValueForCharacteristic(event)
BLE->>BLE: handleButtonEventFromCharacteristic(serviceUUID, charUUID, data)
opt AINA / B01 / Generic
BLE->>Store: dispatch button event
end
sequenceDiagram
autonumber
actor User
participant Screen as FullScreenLocationPicker
participant ExpoLoc as expo-location
participant Map as Map Camera
User->>Screen: Open picker (initialLocation?)
activate Screen
Screen->>Screen: isMountedRef = true
alt initialLocation exists and not 0,0
Screen->>Screen: setCurrentLocation(initial)
Screen->>ExpoLoc: reverseGeocodeAsync()
ExpoLoc-->>Screen: address parts
Screen->>Screen: setAddress(joined parts)
else No initial location
Screen->>ExpoLoc: requestForegroundPermissionsAsync()
ExpoLoc-->>Screen: granted?
alt granted
Screen->>ExpoLoc: getCurrentPositionAsync()
ExpoLoc-->>Screen: coords
Screen->>Screen: setCurrentLocation(coords)
Screen->>ExpoLoc: reverseGeocodeAsync()
Screen->>Map: moveCamera(center=coords)
else denied
Screen->>Screen: setLoading(false)
end
end
User->>Screen: Confirm
Screen-->>User: onLocationSelected({ lat, lng, address? })
deactivate Screen
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Poem
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ 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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
src/components/maps/full-screen-location-picker.tsx (1)
34-35: Replace isMounted state with a ref to avoid setState-on-unmount and stale closures.
Using state for mount flags triggers extra renders and can warn when setting state in cleanup. A ref is stable and avoids re-creating callbacks.- const [isMounted, setIsMounted] = useState(true); + const isMountedRef = useRef(true); @@ - const reverseGeocode = React.useCallback( + const reverseGeocode = React.useCallback( async (latitude: number, longitude: number) => { - if (!isMounted) return; + if (!isMountedRef.current) return; @@ - setIsReverseGeocoding(true); + setIsReverseGeocoding(true); @@ - if (!isMounted) return; + if (!isMountedRef.current) return; @@ - if (isMounted) setAddress(undefined); + if (isMountedRef.current) setAddress(undefined); @@ - if (isMounted) setIsReverseGeocoding(false); + if (isMountedRef.current) setIsReverseGeocoding(false); }, - [isMounted] + [] ); @@ - const getUserLocation = React.useCallback(async () => { - if (!isMounted) return; + const getUserLocation = React.useCallback(async () => { + if (!isMountedRef.current) return; @@ - if (status !== 'granted') { + if (status !== 'granted') { console.error('Location permission not granted'); - if (isMounted) setIsLoading(false); + if (isMountedRef.current) setIsLoading(false); return; } @@ - if (!isMounted) return; + if (!isMountedRef.current) return; @@ - if (cameraRef.current && isMounted) { + if (cameraRef.current && isMountedRef.current) { cameraRef.current.setCamera({ centerCoordinate: [location.coords.longitude, location.coords.latitude], zoomLevel: 15, animationDuration: 1000, }); } @@ - if (isMounted) setIsLoading(false); + if (isMountedRef.current) setIsLoading(false); - }, [isMounted, reverseGeocode]); + }, [reverseGeocode]); @@ - useEffect(() => { - setIsMounted(true); + useEffect(() => { + isMountedRef.current = true; @@ - return () => { - setIsMounted(false); - }; + return () => { + isMountedRef.current = false; + }; }, [initialLocation, getUserLocation, reverseGeocode]);Also applies to: 36-73, 74-110, 111-124
app.config.ts (1)
183-186: Typo in iOS permission string (“Resgird”)User-facing copy; fix before shipping.
- locationWhenInUsePermission: 'Allow Resgird Responder to show current location on map.', + locationWhenInUsePermission: 'Allow Resgrid Responder to show current location on map.',src/components/ui/skeleton/index.tsx (2)
7-11: Type mismatch:speedis used but not declared in propsThis will surface as a TS error; add
speed?: numberto ISkeletonProps.type ISkeletonProps = React.ComponentProps<typeof View> & VariantProps<typeof skeletonStyle> & { isLoaded?: boolean; startColor?: string; + speed?: number; };Also applies to: 20-20
20-65: Animation loop leak and unsafe speed handling; move loop into an effect and guard speedStarting/stopping loops in render creates orphaned animations; also prevent division by zero/negative durations.
-const Skeleton = forwardRef<React.ElementRef<typeof View>, ISkeletonProps>(({ className, variant, children, startColor = 'bg-background-200', isLoaded = false, speed = 2, ...props }, ref) => { - const pulseAnim = new Animated.Value(1); +const Skeleton = forwardRef<React.ElementRef<typeof View>, ISkeletonProps>(({ className, variant, children, startColor = 'bg-background-200', isLoaded = false, speed = 2, ...props }, ref) => { + const pulseAnim = useRef(new Animated.Value(1)).current; const customTimingFunction = Easing.bezier(0.4, 0, 0.6, 1); const fadeDuration = 0.6; - const animationDuration = (fadeDuration * 10000) / speed; // Convert seconds to milliseconds + const safeSpeed = Math.max(0.1, speed); + const animationDuration = (fadeDuration * 10000) / safeSpeed; // Convert seconds to milliseconds - - const pulse = Animated.sequence([ - Animated.timing(pulseAnim, { - toValue: 1, // Start with opacity 1 - duration: animationDuration / 2, // Third of the animation duration - easing: customTimingFunction, - useNativeDriver: Platform.OS !== 'web', - }), - Animated.timing(pulseAnim, { - toValue: 0.75, - duration: animationDuration / 2, // Third of the animation duration - easing: customTimingFunction, - useNativeDriver: Platform.OS !== 'web', - }), - Animated.timing(pulseAnim, { - toValue: 1, - duration: animationDuration / 2, // Third of the animation duration - easing: customTimingFunction, - useNativeDriver: Platform.OS !== 'web', - }), - ]); + React.useEffect(() => { + if (!isLoaded) { + const pulse = Animated.sequence([ + Animated.timing(pulseAnim, { + toValue: 1, + duration: animationDuration / 2, + easing: customTimingFunction, + useNativeDriver: Platform.OS !== 'web', + }), + Animated.timing(pulseAnim, { + toValue: 0.75, + duration: animationDuration / 2, + easing: customTimingFunction, + useNativeDriver: Platform.OS !== 'web', + }), + Animated.timing(pulseAnim, { + toValue: 1, + duration: animationDuration / 2, + easing: customTimingFunction, + useNativeDriver: Platform.OS !== 'web', + }), + ]); + const loop = Animated.loop(pulse); + loop.start(); + return () => loop.stop(); + } else { + // Reset when content is loaded + pulseAnim.setValue(1); + } + }, [isLoaded, animationDuration, customTimingFunction, pulseAnim]); if (!isLoaded) { - Animated.loop(pulse).start(); return ( <Animated.View style={{ opacity: pulseAnim }} className={`${startColor} ${skeletonStyle({ variant, class: className, })}`} {...props} ref={ref} /> ); } else { - Animated.loop(pulse).stop(); - return children; } });Also update imports at Line 2:
-import React, { forwardRef } from 'react'; +import React, { forwardRef, useEffect, useRef } from 'react';src/components/ui/bottomsheet/index.tsx (1)
51-51: Ref type mismatch with context.
useRef<GorhomBottomSheet>(null)conflicts with the widenedGorhomBottomSheet | nullin context. Update the ref generic to avoid TS friction in strict mode.- const bottomSheetRef = useRef<GorhomBottomSheet>(null); + const bottomSheetRef = useRef<GorhomBottomSheet | null>(null);src/components/settings/__tests__/realtime-geolocation-item.test.tsx (2)
172-194: Error-path test doesn’t verify the component’s catch/loggingIt manually logs the error in the test instead of asserting the component’s console.error within handleToggle.
Drive the UI and assert the component’s log:
it('handles toggle errors gracefully', async () => { - mockSetRealtimeGeolocationEnabled.mockRejectedValueOnce(new Error('Network error')); - const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); - - const TestWrapper = () => { - const { setRealtimeGeolocationEnabled } = require('@/lib/hooks/use-realtime-geolocation').useRealtimeGeolocation(); - React.useEffect(() => { - // Simulate an error during toggle - setRealtimeGeolocationEnabled(true).catch(() => { - console.error('Failed to toggle realtime geolocation:', new Error('Network error')); - }); - }, [setRealtimeGeolocationEnabled]); - return null; - }; - - render(<TestWrapper />); + mockSetRealtimeGeolocationEnabled.mockRejectedValueOnce(new Error('Network error')); + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + render(<RealtimeGeolocationItem />); + fireEvent.press(screen.getByTestId('switch')); await waitFor(() => { expect(consoleSpy).toHaveBeenCalledWith('Failed to toggle realtime geolocation:', expect.any(Error)); }); consoleSpy.mockRestore(); });
196-210: Dark-mode test won’t affect already-imported module; re-import with isolated module graphUse jest.resetModules + jest.isolateModules to remock nativewind before requiring the component.
it('renders with dark color scheme', () => { - // Mock the dark color scheme temporarily - const originalMock = jest.requireMock('nativewind'); - jest.doMock('nativewind', () => ({ - ...originalMock, - useColorScheme: () => ({ colorScheme: 'dark' }), - cssInterop: jest.fn(), - })); - - mockUseRealtimeGeolocation.isRealtimeGeolocationEnabled = true; - - const { UNSAFE_root } = render(<RealtimeGeolocationItem />); - - expect(UNSAFE_root).toBeTruthy(); + jest.resetModules(); + mockUseRealtimeGeolocation.isRealtimeGeolocationEnabled = true; + jest.isolateModules(() => { + jest.doMock('nativewind', () => ({ + useColorScheme: () => ({ colorScheme: 'dark' }), + cssInterop: jest.fn(), + })); + const React = require('react'); + const { render, screen } = require('@testing-library/react-native'); + const { RealtimeGeolocationItem: DarkItem } = require('../realtime-geolocation-item'); + const { UNSAFE_root } = render(React.createElement(DarkItem)); + expect(UNSAFE_root).toBeTruthy(); + }); });src/app/(app)/__tests__/messages.test.tsx (1)
603-621: Test title and assertion contradict: FAB should be hidden in selection modeThe test name says “hides” but it asserts truthy. Assert absence instead.
- // Since we can't easily test the selection mode state change, - // we'll test that the FAB is hidden when selectedForDeletion has items - // This is an indirect test, but validates the behavior - expect(screen.getByTestId('messages-compose-fab')).toBeTruthy(); + // FAB should be hidden when there are selected items (selection mode) + expect(screen.queryByTestId('messages-compose-fab')).toBeNull();src/services/bluetooth-audio.service.ts (3)
276-281: Clear the pending scan timeout when scan stopsAvoid double-calls to handleScanStopped (timer + native event).
private handleScanStopped(): void { + if (this.scanTimeout) { + clearTimeout(this.scanTimeout); + this.scanTimeout = null; + } useBluetoothAudioStore.getState().setIsScanning(false); logger.info({ message: 'Bluetooth scan stopped', }); }
803-809: Fix auto-connect guard; connectionTimeout is never setUse the store’s isConnecting flag (already maintained) instead of an always-null timeout.
- private async checkAndAutoConnectPreferredDevice(device: BluetoothAudioDevice): Promise<void> { - const { preferredDevice, connectedDevice } = useBluetoothAudioStore.getState(); + private async checkAndAutoConnectPreferredDevice(device: BluetoothAudioDevice): Promise<void> { + const { preferredDevice, connectedDevice, isConnecting } = useBluetoothAudioStore.getState(); @@ - if (preferredDevice?.id === device.id && !connectedDevice && !this.connectionTimeout) { + if (preferredDevice?.id === device.id && !connectedDevice && !isConnecting) {
588-666: Non-hex literals in hexData will never match; add ASCII-path analysis'aina', 'inrico', 'hys' cannot appear in a hex string. Check ASCII-decoded data as well.
private analyzeServiceDataForAudio(data: Buffer): boolean { @@ - const hexData = data.toString('hex').toLowerCase(); + const hexData = data.toString('hex').toLowerCase(); + const asciiData = data.toString('utf8').toLowerCase(); @@ - const audioPatterns = [ + const audioPatterns = [ // Common audio capability flags (these are example patterns) '0001', // Audio sink capability '0002', // Audio source capability '0004', // Headset capability '0008', // Hands-free capability '1108', // HSP service class '110a', // A2DP sink service class '110b', // A2DP source service class '111e', // HFP service class '1203', // Audio/Video Remote Control Profile - // Known manufacturer-specific patterns - 'aina', // AINA device identifier - 'inrico', // Inrico device identifier - 'hys', // HYS device identifier ]; - const hasAudioPattern = audioPatterns.some((pattern) => hexData.includes(pattern)); + const hasAudioPattern = audioPatterns.some((pattern) => hexData.includes(pattern)); + const asciiManufacturerHit = /(aina|inrico|hys)/i.test(asciiData); @@ - return hasAudioPattern || hasAudioCapabilityBytes || hasAudioDeviceClass; + return hasAudioPattern || asciiManufacturerHit || hasAudioCapabilityBytes || hasAudioDeviceClass;src/components/ui/drawer/index.tsx (1)
13-13: Fix style-context scope collision (Drawer uses 'MODAL')Using the same SCOPE as Modal risks context bleed when both are mounted. Give Drawer its own scope.
Apply:
-const SCOPE = 'MODAL'; +const SCOPE = 'DRAWER';src/services/__tests__/bluetooth-audio.service.test.ts (1)
196-229: Permission test currently asserts the failure path; assert success and wire the mock into the serviceThe purpose is to verify the 500ms delay and a successful permission request. Patch the service instance to use the mocked
PermissionsAndroidand assert the happy path.- // Log the mock function to debug - console.log('Mock requestMultiple:', mockRequestMultiple); - mockRequestMultiple.mockResolvedValue({ 'android.permission.BLUETOOTH_SCAN': 'granted', 'android.permission.BLUETOOTH_CONNECT': 'granted', }); const service = bluetoothAudioService as any; - - console.log('Service imported PermissionsAndroid:', (service as any).PermissionsAndroid); - console.log('Available at RN.PermissionsAndroid:', RN.PermissionsAndroid); + // Ensure the service uses the mocked PermissionsAndroid + (service as any).PermissionsAndroid = RN.PermissionsAndroid; // Start the permission request const permissionPromise = service.requestPermissions(); // Fast-forward time by 500ms jest.advanceTimersByTime(500); // Wait for the promise to resolve const result = await permissionPromise; - - console.log('Permission result:', result); - console.log('Mock calls:', mockRequestMultiple.mock.calls); - - // Since PermissionsAndroid is undefined in the service, it goes to catch block and returns false - // This is the correct behavior when the mock isn't working properly - expect(result).toBe(false); - // Since PermissionsAndroid is undefined, mockRequestMultiple is never called - expect(mockRequestMultiple).not.toHaveBeenCalled(); + expect(result).toBe(true); + expect(mockRequestMultiple).toHaveBeenCalledTimes(1);
🧹 Nitpick comments (90)
src/components/common/aptabase-provider.tsx (4)
33-36: Defensively guard init and avoid overriding host with an empty string.Skip init if the key is missing and only pass
hostwhen defined. This makes the wrapper robust if used elsewhere.- init(Env.APTABASE_APP_KEY, { - host: Env.APTABASE_URL || '', - }); + const appKey = Env.APTABASE_APP_KEY; + if (!appKey) { + logger.warn({ message: 'Aptabase app key not set; skipping initialization' }); + return; + } + const options = Env.APTABASE_URL ? { host: Env.APTABASE_URL } : undefined; + init(appKey, options);
37-43: Don’t log key material; log presence instead.Even partial keys can leak. Prefer a boolean.
logger.info({ message: 'Aptabase provider initialized', context: { - appKey: Env.APTABASE_APP_KEY.substring(0, 8) + '...', + hasKey: Boolean(Env.APTABASE_APP_KEY), serviceStatus: aptabaseService.getStatus(), }, });
14-14: Remove unusedinitializationFailedstate.It’s never read and adds noise. Also drop the
setInitializationFailedcalls.- const [initializationFailed, setInitializationFailed] = React.useState(false); @@ - setInitializationFailed(true); @@ - setInitializationFailed(true);Also applies to: 27-27, 52-52
55-58: Drop empty cleanup function.No-op cleanup can be removed.
- return () => { - // Cleanup if needed - }; - }, []); + }, []);src/app/_layout.tsx (1)
182-201: Minor readability tweak and safer runtime guard (Expo Go).Precompute the condition and reuse rendered content; optionally also avoid init in Expo Go.
const renderContent = () => ( @@ - return ( + const shouldInitAptabase = Boolean(Env.APTABASE_APP_KEY) && !__DEV__ && !isRunningInExpoGo(); + const content = renderContent(); + return ( <SafeAreaProvider> <GestureHandlerRootView> - <KeyboardProvider>{Env.APTABASE_APP_KEY && !__DEV__ ? <AptabaseProviderWrapper>{renderContent()}</AptabaseProviderWrapper> : renderContent()}</KeyboardProvider> + <KeyboardProvider> + {shouldInitAptabase ? <AptabaseProviderWrapper>{content}</AptabaseProviderWrapper> : content} + </KeyboardProvider> </GestureHandlerRootView> </SafeAreaProvider> );src/components/maps/full-screen-location-picker.tsx (7)
51-51: Explicit typing looks good; prefer const over let.
addressPartsisn’t re-assigned. Usingconstcommunicates intent and narrows mutation.- let addressParts: string[] = []; + const addressParts: string[] = [];
126-134: Avoid any: derive the onPress event type from MapView props.
This keeps the handler aligned with library typings without guessing names.- const handleMapPress = (event: any) => { + const handleMapPress = ( + event: Parameters<NonNullable<React.ComponentProps<typeof Mapbox.MapView>['onPress']>>[0] + ) => {
160-160: Localize user-facing annotation title.
Wrap witht()to satisfy the i18n guideline.- <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')}>
175-178: Add accessibility props to the close button.
Improve a11y with role, label, and touch target expansion.- <TouchableOpacity style={[styles.closeButton, { top: insets.top + 10 }]} onPress={onClose}> + <TouchableOpacity + style={[styles.closeButton, { top: insets.top + 10 }]} + onPress={onClose} + accessibilityRole="button" + accessibilityLabel={t('common.close')} + hitSlop={{ top: 8, bottom: 8, left: 8, right: 8 }} + > <XIcon size={24} color="#000000" /> </TouchableOpacity>
205-210: Remove fixed window dimensions; rely on flex for rotation/responsiveness.
Hard-coding width/height can stale on orientation change and split-screen.container: { flex: 1, - width: Dimensions.get('window').width, - height: Dimensions.get('window').height, position: 'relative', },
162-163: Theme hard-coded colors for dark mode.
Replace fixed reds/whites/blacks with theme tokens or color-scheme-based values to satisfy dark/light mode support.Examples:
- Map pin and close icon colors (Lines 162, 177)
- Bottom panel bg (Line 181)
- Close button bg and shadow color (Lines 220, 223)
Would you like a follow-up patch usinguseColorScheme()or your gluestack tokens?Also applies to: 177-177, 181-181, 220-220, 223-223
79-85: Surface permission denial to the user.
Currently logs to console only. Consider a toast/inline message viat('common.location_permission_denied')to guide the user to enable permissions.src/hooks/use-map-signalr-updates.ts (1)
16-16: Use ReturnType for cross-env timer IDsPrevents type drift between RN/Jest/Node environments.
- const debounceTimer = useRef<number | null>(null); + const debounceTimer = useRef<ReturnType<typeof setTimeout> | null>(null);src/hooks/use-signalr-lifecycle.ts (2)
34-35: Use ReturnType for timersAligns with RN/web/Jest timer typings across environments.
- const backgroundTimer = useRef<number | null>(null); - const resumeTimer = useRef<number | null>(null); + const backgroundTimer = useRef<ReturnType<typeof setTimeout> | null>(null); + const resumeTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
51-60: “AbortController” isn’t wired to ops; consider last-write-wins token insteadYou abort a controller but don’t pass its signal into connect/disconnect, so it can’t cancel work. If intentional, replace with a monotonic opId for clarity; or plumb AbortSignal through the store methods.
Minimal refactor sketch:
const opId = useRef(0); // In each handler: const myId = ++opId.current; // ... do work ... } finally { if (myId === opId.current) { isProcessing.current = false; pendingOperations.current = null; } }src/lib/storage/pii-protection.ts (1)
153-162: Harden web cleanup: wrap removeItem in try/catch and log failuresPrevents cleanup from aborting on quota/SecurityError and gives observability. Optionally mirror the same for sessionStorage if used.
- keysToRemove.forEach((key) => localStorage.removeItem(key)); + keysToRemove.forEach((key) => { + try { + localStorage.removeItem(key); + } catch (e) { + logger.warn({ + message: 'Failed to remove key during emergency PII cleanup', + context: { key, error: e }, + }); + } + });src/components/status/store.ts (2)
160-171: Replace inline union with a reusable type and simplify declarationImproves readability and reusability across the codebase.
- let gpsData: - | { - latitude?: string; - longitude?: string; - accuracy?: string; - altitude?: string; - altitudeAccuracy?: string; - speed?: string; - heading?: string; - } - | undefined = undefined; + let gpsData: GpsData | undefined;Add once near the top of this file (or a shared types module):
interface GpsData { latitude?: string; longitude?: string; accuracy?: string; altitude?: string; altitudeAccuracy?: string; speed?: string; heading?: string; }
198-205: Confirm optional GPS field handling
ThequeueUnitStatusEventsignature accepts optionalaltitude,altitudeAccuracy,speed, andheading, but instore.tsyou only populatelatitude,longitude, andaccuracy. If the location store provides the other fields, pass them through or verify their omission is intentional.src/lib/i18n/index.tsx (1)
1-1: Prefer languageTag for locale fidelity; derive base when resources are keyed by languageUsing languageCode drops region (e.g., en-GB vs en-US). If resources are keyed by base language, use languageTag then normalize.
- lng: getLanguage() || Localization.getLocales()[0]?.languageCode || 'en', + lng: + getLanguage() || + Localization.getLocales()[0]?.languageTag?.split('-')[0] || + 'en',Also applies to: 12-12
src/app/__tests__/onboarding.test.tsx (1)
145-154: Remove redundant assertion
getByTestId('onboarding-flatlist')is already asserted; duplicating it adds noise.- // The FlatList content might not render immediately in tests, - // so we verify the component renders without crashing - expect(getByTestId('onboarding-flatlist')).toBeTruthy();src/components/calls/__tests__/call-images-modal.test.tsx (4)
216-223: Nice TS narrow: keep it DRY with an expression-based source.You can avoid the mutable let and mirror production logic succinctly.
- const hasError = imageErrors.has(item.Id); - let imageSource: { uri: string } | null = null; - - if (item.Data && item.Data.trim() !== '') { - const mimeType = item.Mime || 'image/png'; - imageSource = { uri: `data:${mimeType};base64,${item.Data}` }; - } else if (item.Url && item.Url.trim() !== '') { - imageSource = { uri: item.Url }; - } + const hasError = imageErrors.has(item.Id); + const imageSource = + item.Data?.trim() + ? { uri: `data:${item.Mime || 'image/png'};base64,${item.Data}` } + : item.Url?.trim() + ? { uri: item.Url } + : null;
662-669: Repeat type literal: consider a tiny alias.Minor: define
type ImageSource = { uri: string } | null;once near the top and reuse here and in similar blocks.
684-691: Same as above: reuse the alias for consistency.
706-713: Same as above: reuse the alias for consistency.src/components/ui/bottomsheet/index.tsx (3)
39-47: Context ref type is fine; fix Prettier noise in defaults.Trim the spaces inside the empty handlers to satisfy CI formatting.
- handleClose: () => { }, - handleOpen: () => { }, + handleClose: () => {}, + handleOpen: () => {},
169-176: Prettier/CI: indent keyDown handler block.Adjust indentation to resolve the pipeline warnings.
- onKeyDown: (e: React.KeyboardEvent) => { - if (e.key === 'Escape') { - e.preventDefault(); - handleClose(); - return; - } - }, + onKeyDown: (e: React.KeyboardEvent) => { + if (e.key === 'Escape') { + e.preventDefault(); + handleClose(); + return; + } + },
37-47: Optional: align ref types (RefObject vs MutableRefObject).The provider type uses
React.RefObject, whileuseRefreturnsMutableRefObject. They’re structurally compatible, but typing the context asReact.MutableRefObject<GorhomBottomSheet | null>removes ambiguity.src/app/(app)/__tests__/calendar.test.tsx (1)
59-74: Deterministic FlatList mock LGTM; consider minimal API parity.Looks good. If future tests need it, optionally accept
ListEmptyComponentand callonEndReachedwhen data length is non-zero to mimic RN behavior.src/app/(app)/__tests__/contacts.test.tsx (2)
176-179: Test ID dependency: ensure Contacts sets testID="contacts-list".If the component doesn’t expose this testID, these assertions will fail. Confirm it exists or add it to the list container.
294-299: Retain one interaction test to assert selectContact is invoked.Given the mock
ContactCardexposescontact-card-<id>, add a quick press to ensure wiring is intact.render(<Contacts />); - // Verify the contacts list is rendered - expect(screen.getByTestId('contacts-list')).toBeTruthy(); - - // Test that selectContact function is available to be called - expect(typeof mockSelectContact).toBe('function'); + // Verify the contacts list is rendered + expect(screen.getByTestId('contacts-list')).toBeTruthy(); + // Press a card to ensure the handler is wired + fireEvent.press(screen.getByTestId('contact-card-1')); + expect(mockSelectContact).toHaveBeenCalledWith('1');src/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsx (5)
32-42: Deduplicate Platform mocking to avoid conflicts.You’re mocking Platform twice (here and at Lines 2-5). Centralize this in one place (jest-setup or mocks/react-native.ts) to prevent divergent behavior.
Apply:
jest.mock('react-native', () => ({ ...jest.requireActual('react-native'), useWindowDimensions: () => ({ width: 400, height: 800, }), - Platform: { - OS: 'ios', - select: jest.fn().mockImplementation((obj) => obj.ios || obj.default), - }, }));
81-85: Reflect disabled state on the mocked Button.The mock swallows accessibility/disabled state. Propagate these to Pressable for more faithful behavior and clearer tests.
- Button: ({ children, onPress, disabled, ...props }: any) => { + Button: ({ children, onPress, disabled, ...props }: any) => { const React = require('react'); - return React.createElement('Pressable', { onPress: disabled ? undefined : onPress, testID: 'button', ...props }, children); + return React.createElement( + 'Pressable', + { disabled, accessibilityState: { disabled }, onPress: disabled ? undefined : onPress, testID: 'button', ...props }, + children + ); },
168-171: Avoid reaching into props; assert text content instead.Accessing
.props.childrencouples to implementation. Prefer text assertions for stability.-const labelTexts = screen.getAllByTestId('form-control-label-text'); -expect(labelTexts[0].props.children).toBe('settings.username'); -expect(labelTexts[1].props.children).toBe('settings.password'); +const [usernameLabel, passwordLabel] = screen.getAllByTestId('form-control-label-text'); +expect(usernameLabel).toHaveTextContent('settings.username'); +expect(passwordLabel).toHaveTextContent('settings.password');Add (if needed) at top:
import '@testing-library/jest-native/extend-expect';
207-216: KeyboardAvoidingView check is brittle; assert by component type.Comparing
parent?.parent?.type === 'RNKeyboardAvoidingView'is fragile. Assert by RN component type.-// KeyboardAvoidingView should be present in the rendered output -// We can verify it exists by checking the component tree structure -const vstack = screen.getByTestId('vstack'); -expect(vstack).toBeTruthy(); -// The VStack is wrapped by RNKeyboardAvoidingView, so we need to go up one more level -expect(vstack.parent?.parent?.type).toBe('RNKeyboardAvoidingView'); +const roots = screen.root.findAllByType(KeyboardAvoidingView); +expect(roots.length).toBeGreaterThan(0);Also import:
import { KeyboardAvoidingView } from 'react-native';
221-224: Target the cancel button deterministically.Index-based selection (
buttons[0]) is order-dependent. Prefer finding the Pressable whose childbutton-textiscommon.cancel, or expose distinct testIDs (e.g.,cancel-button,save-button).Also applies to: 229-231
src/stores/status/__tests__/personnel-status-store.test.ts (3)
1080-1081: Use jest.spyOn instead of reassignment for mocked method.Direct reassignment works but is noisier and easier to overwrite accidentally. Spy is clearer and restores nicely.
-beforeEach(() => { - (mockOfflineQueueProcessor.addPersonnelStatusToQueue as jest.MockedFunction<any>) = jest.fn().mockReturnValue('event-123'); -}); +beforeEach(() => { + jest.spyOn(mockOfflineQueueProcessor, 'addPersonnelStatusToQueue').mockReturnValue('event-123' as any); +});
644-665: Avoid overriding respondingTo; rely on store-derived value.Manually calling
setRespondingTo('CALL-123')diverges from how the store sets it (CallId). It weakens the contract this test verifies.act(() => { result.current.setIsOpen(true, mockStatus as any); result.current.setSelectedCall(mockCall as any); result.current.setNote('Test note'); - result.current.setRespondingTo('CALL-123'); }); @@ expect(mockSavePersonnelStatus).toHaveBeenCalledWith( expect.objectContaining({ UserId: 'user123', Type: '1', Note: 'Test note', - RespondingTo: 'CALL-123', + RespondingTo: '123', EventId: '123', }) ); +expect(mockFetchCurrentUserInfo).toHaveBeenCalled(); +expect(mockShowToast).toHaveBeenCalledWith('success', 'Status updated successfully');
701-724: Same here for station path; assert store behavior and success side-effects.act(() => { result.current.setIsOpen(true, mockStatus as any); result.current.setSelectedGroup(mockGroup as any); result.current.setNote('Test note'); - result.current.setRespondingTo('STATION-456'); }); @@ expect(mockSavePersonnelStatus).toHaveBeenCalledWith( expect.objectContaining({ UserId: 'user123', Type: '1', Note: 'Test note', - RespondingTo: 'STATION-456', + RespondingTo: '456', EventId: '456', }) ); expect(result.current.isOpen).toBe(false); +expect(mockFetchCurrentUserInfo).toHaveBeenCalled(); +expect(mockShowToast).toHaveBeenCalledWith('success', 'Status updated successfully');src/app/(app)/__tests__/personnel.test.tsx (5)
36-52: Improve mock accessibility and selector ergonomics for PersonnelCard.Expose accessibility role/labels and a stable testID for simpler queries and better a11y.
return { PersonnelCard: ({ personnel, onPress }: any) => { return React.createElement( Pressable, { - testID: `personnel-card-${personnel.UserId}`, + testID: `personnel-card-${personnel.UserId}`, + accessibilityRole: 'button', + accessibilityLabel: `Open details for ${personnel.FirstName} ${personnel.LastName}`, onPress: () => onPress(personnel.UserId), }, React.createElement(Text, {}, `${personnel.FirstName} ${personnel.LastName}`) ); }, };
261-268: Avoid relying on RN internal type names for FlatList.
findAllByType('RNFlatList')ties tests to mock internals. Prefer a testID on the FlatList (e.g.,personnel-list) and query viagetByTestId.If production component can add:
<FlatList testID="personnel-list" ... />Then change assertions to:
expect(screen.getByTestId('personnel-list')).toBeTruthy();Also applies to: 531-533, 565-567, 589-595
351-369: Consolidate overlapping search tests.Two blocks assert the same interactions (setSearchQuery, clear button visibility). Merge to reduce runtime and maintenance.
Also applies to: 426-480
501-517: This test validates the mock, not the component.Reading
mockUsePersonnelStore.mock.resultschecks Jest internals rather than UI behavior. Prefer asserting rendered items (e.g., presence/count ofpersonnel-card-*) or drop this test.Example:
expect(screen.getByTestId('personnel-card-1')).toBeTruthy(); expect(screen.getByTestId('personnel-card-2')).toBeTruthy(); expect(screen.getByTestId('personnel-card-3')).toBeTruthy();
639-647: Stabilize filter count assertion.
getByText('2')can be ambiguous. Prefer a badge testID (e.g.,filter-count-badge) and assert its text content.Proposed in component:
<Text testID="filter-count-badge">{selectedFilters.length}</Text>Then in test:
expect(screen.getByTestId('filter-count-badge')).toHaveTextContent('2');Also applies to: 649-658
src/components/settings/__tests__/realtime-geolocation-item.test.tsx (2)
100-102: Prefer toBeNull for absence checks-expect(screen.queryByText(/This feature connects to the real-time location hub/)).toBeFalsy(); +expect(screen.queryByText(/This feature connects to the real-time location hub/)).toBeNull();Also applies to: 226-226
119-122: Coverage gap: status suffix strings aren’t localizedThe component appends “Connected to hub.”/“Connecting to hub...” as raw strings; guidelines require wrapping user-facing text with t(). Consider localizing these and updating assertions accordingly.
jest-setup.ts (3)
5-7: Consolidate window shimYou set global.window twice. Keep a single assignment.
-// @ts-ignore -global.window = {}; -// @ts-ignore -global.window = global; +// @ts-ignore +global.window = global;
73-76: Export nativewind.useColorScheme for components relying on itSome tests mock nativewind locally, but a safe default here avoids surprises.
jest.mock('nativewind', () => ({ cssInterop: jest.fn(), styled: jest.fn(() => (Component: any) => Component), + useColorScheme: () => ({ colorScheme: 'light' }), }));
94-97: Duplicate Platform mocks risk divergenceYou mock Platform in two places. Consider relying only on the react-native mock to reduce skew.
Also applies to: 148-151
src/app/(app)/__tests__/messages.test.tsx (2)
234-247: FlatList mock: good; add guard for non-element returnsrenderItem might return strings/fragments. Guard with React.isValidElement.
return React.createElement( View, props, data?.map((item: any, index: number) => { const key = keyExtractor ? keyExtractor(item, index) : index.toString(); - const element = renderItem ? renderItem({ item, index }) : null; - return element ? React.cloneElement(element, { key }) : null; + const element = renderItem ? renderItem({ item, index }) : null; + return React.isValidElement(element) + ? React.cloneElement(element, { key }) + : element != null + ? React.createElement(React.Fragment, { key }, element) + : null; }) );
519-526: Duplicate compose FAB testsTwo tests cover the same path with/without explicit permission setup. Consider keeping one and scoping the other to the permission edge case only to reduce test time/noise.
Also applies to: 623-635
src/components/shifts/__tests__/shift-details-sheet.test.tsx (2)
16-20: Type the mocked hook for safetyCast the mocked useWindowDimensions to jest.Mock to avoid implicit any and ease resets.
-const mockReactNative = jest.requireMock('react-native'); -const mockUseWindowDimensions = mockReactNative.useWindowDimensions; +const mockReactNative = jest.requireMock('react-native'); +const mockUseWindowDimensions = mockReactNative.useWindowDimensions as jest.Mock;
843-860: Landscape test approach is solid; minor isolation tweak optionalWrapper ensures hook re-evaluation. Optionally add an afterEach to reset the mock implementation to portrait in case future tests mutate it mid-spec.
+afterEach(() => { + mockUseWindowDimensions.mockReturnValue({ + width: 375, height: 812, scale: 2, fontScale: 1, + }); +});src/components/calls/__tests__/close-call-bottom-sheet.test.tsx (1)
61-74: Expose accessibilityRole for better queries and a11yAdd accessibilityRole="button" so tests can use getByRole and to align with RN a11y semantics.
return ( <TouchableOpacity onPress={onPress} testID={testID} disabled={disabled} + accessibilityRole="button" accessibilityState={{ disabled: !!disabled }} {...props} > {children} </TouchableOpacity> );src/services/bluetooth-audio.service.ts (1)
2-2: Remove unused importsAlert and DeviceEventEmitter aren’t used.
-import { Alert, DeviceEventEmitter, PermissionsAndroid, Platform } from 'react-native'; +import { PermissionsAndroid, Platform } from 'react-native';types/index.d.ts (1)
9-14: Remove redundant ActionsheetProps augmentation
InterfaceActionsheetProps in gluestack-overrides.d.ts already covers this override. Apply:-// Component-specific overrides using module augmentation -declare module '@gluestack-ui/actionsheet' { - interface ActionsheetProps { - children?: React.ReactNode; - } -}src/types/gluestack-patches.d.ts (1)
3-5: Tighten temporary factory typings (avoid any where feasible)These are stopgaps, but returning any everywhere leaks unsafety across the app. Consider unknown or a generic passthrough so call sites must assert intentionally.
Example (low-risk):
-declare module '@gluestack-ui/actionsheet' { - export function createActionsheet(config: any): any; -} +declare module '@gluestack-ui/actionsheet' { + export function createActionsheet<T = unknown>(config: unknown): T; +}Repeat similarly for alert-dialog/modal/tooltip.
Please open a follow-up issue to remove this file once upstream types stabilize. I can draft it.
Also applies to: 7-9, 11-13, 15-17
src/components/ui/modal/index.tsx (2)
16-19: Reduce as any usage; preserve type-safety around UIModal and propsThe casts on Root/Backdrop/Content, cssInterop, and prop spreads erode safety. You can type the createModal config and the component props to avoid these anys.
- Type the config:
const config: Parameters<typeof createModal>[0] = { ... }thencreateModal(config)to drop theas anycasts.- Define prop types without casting spreads: e.g.,
type ModalBaseProps = Omit<React.ComponentProps<typeof UIModal>, 'className'>;.- Keep cssInterop without casts once the component types are correct.
I can provide a targeted patch if desired.
Also applies to: 26-28, 99-99, 124-124, 158-158
84-84: Avoid duplicating children in IModalPropsReact.ComponentProps already includes children; redefining can cause confusion.
Apply:
-type IModalProps = React.ComponentProps<typeof UIModal> & VariantProps<typeof modalStyle> & { className?: string; children?: React.ReactNode }; +type IModalProps = React.ComponentProps<typeof UIModal> & VariantProps<typeof modalStyle> & { className?: string };src/components/ui/drawer/index.tsx (3)
149-149: Remove props as any spreadsMirror the Modal suggestion: type props so you can spread safely without any.
I can push a concrete typing patch if you prefer.
Also applies to: 174-174, 210-210
216-216: Guard undefined className when concatenatingAvoid rendering "undefined ..." in class strings.
Apply:
- class: `${className} ${customClass}`, + class: `${className ?? ''} ${customClass}`,
185-187: Handle orientation changesscreenWidth/screenHeight are captured once and won’t update on rotation. Use useWindowDimensions within DrawerContent.
I can provide a small refactor if helpful.
src/types/gluestack-overrides.d.ts (1)
1-58: Prettier warnings in pipelineFormat this file to clear CI warnings.
Run: pnpm prettier --write "src/types/gluestack-overrides.d.ts"
types/gluestack-overrides.d.ts (2)
7-7: Fix Prettier formatting warningsCI flagged formatting at these lines (likely trailing spaces/blank lines). Run Prettier or remove trailing whitespace.
- +Also applies to: 18-18, 41-41, 45-45
51-57: Avoid duplicating TVConfig types; reuse a single source of truthYou already define
TVConfigintypes/index.d.ts. Duplicating it here with different generic defaults ({}vsany) risks drift and type conflicts.-declare module 'tailwind-variants/dist/config' { - export interface TVConfig<V = {}, EV = V> { - // Basic config structure - [key: string]: any; - } -} +declare module 'tailwind-variants/dist/config' { + export { TVConfig } from '../../types/index.d.ts'; +}Or remove this block and import the local
TVConfigwhere needed.src/services/__tests__/bluetooth-audio.service.test.ts (2)
167-173: Remove debug-only test block and console noiseThese logs add noise to CI output and hide real failures.
- describe('Mock Setup Debug', () => { - it('should show Platform and permissions setup', () => { - console.log('Platform.OS:', RN.Platform.OS); - console.log('PermissionsAndroid available:', !!RN.PermissionsAndroid); - console.log('Mock function available:', !!mockRequestMultiple); - }); - });
241-246: Prefer modern timer helpers to reduce flakinessWhen using modern fake timers, consider
await jest.advanceTimersByTimeAsync(500)to avoid race conditions with pending promises.- jest.advanceTimersByTime(500); + await jest.advanceTimersByTimeAsync(500);src/components/ui/actionsheet/index.tsx (4)
66-73: Align cssInterop target with the actual Item componentYou registered cssInterop for
ItemWrapperbut mappedItemtoPressable. With the change above, the cssInterop registration remains useful; otherwise, remove the unusedItemWrapper/interop.-cssInterop(ItemWrapper, { className: 'style' }); +// Kept because Item now maps to ItemWrapperIf you keep
Item: Pressable, delete theItemWrapperdeclaration and its cssInterop line.
416-436: Web a11y: consider semantic header on web for SectionHeaderTextSwitching from
H4toTextloses semantic heading on web. Optional: useH4on web andTextelsewhere.- SectionHeaderText: Text as any, + SectionHeaderText: (Platform.OS === 'web' ? (H4 as any) : (Text as any)),
234-269: Component wrappers look fine; consider memo where props are stableThese forwardRef wrappers are thin. If props are generally stable,
React.memocan avoid re-renders.-const Actionsheet = React.forwardRef<...>(...) +const Actionsheet = React.memo(React.forwardRef<...>(...));Repeat for others if beneficial.
Also applies to: 270-470
1-13: Type-safety could be improved; reduceas anywhere possibleGiven the guideline to avoid
any, consider supplying generics tocreateActionsheetor declaring minimal prop interfaces for each slot to remove manyas anycasts.Also applies to: 49-64
src/components/calls/__tests__/call-notes-modal-new.test.tsx (6)
105-126: Type the Button mock and add accessibilityState.Tighten typings and improve a11y while keeping behavior unchanged.
- const Button = React.forwardRef(({ children, onPress, isDisabled, testID, ...props }: any, ref: any) => { - const handlePress = React.useCallback((event: any) => { + type ButtonProps = + Omit<React.ComponentProps<typeof import('react-native').Pressable>, 'onPress' | 'disabled'> & { + onPress?: (e: import('react-native').GestureResponderEvent) => void; + isDisabled?: boolean; + testID?: string; + }; + + const Button = React.forwardRef<React.ElementRef<typeof import('react-native').Pressable>, ButtonProps>( + ({ children, onPress, isDisabled = false, testID, ...props }, ref) => { + const handlePress = React.useCallback((event: import('react-native').GestureResponderEvent) => { if (!isDisabled && onPress) { onPress(event); } }, [isDisabled, onPress]); return ( <Pressable ref={ref} testID={testID} onPress={handlePress} disabled={isDisabled} + accessibilityState={{ disabled: isDisabled }} {...props} > {children} </Pressable> ); - }); + }); - const ButtonText = ({ children, ...props }: any) => ( + type ButtonTextProps = React.ComponentProps<typeof import('react-native').Text>; + const ButtonText = ({ children, ...props }: ButtonTextProps) => ( <Text {...props}>{children}</Text> );
135-139: Remove redundant FocusAwareStatusBar mock.You already mock ../../ui/focus-aware-status-bar (Lines 53–56). This second mock via ../../ui is unnecessary and can confuse module resolution.
-// Mock other UI components -jest.mock('../../ui', () => ({ - FocusAwareStatusBar: () => null, -})); -
168-181: Type InputField to avoid any.Use TextInputProps to better match RN API.
InputField: ({ placeholder, value, onChangeText, ...props }: any) => { - const { TextInput } = require('react-native'); - return <TextInput placeholder={placeholder} value={value} onChangeText={onChangeText} {...props} />; + const { TextInput } = require('react-native'); + const p: import('react-native').TextInputProps = { + placeholder, + value, + onChangeText, + ...props, + }; + return <TextInput {...p} />; },
190-199: Type TextareaInput similarly.Aligns with RN typings and reduces implicit anys.
- TextareaInput: ({ placeholder, value, onChangeText, ...props }: any) => { - const { TextInput } = require('react-native'); - return <TextInput placeholder={placeholder} value={value} onChangeText={onChangeText} {...props} />; - }, + TextareaInput: ({ placeholder, value, onChangeText, ...props }: import('react-native').TextInputProps) => { + const { TextInput } = require('react-native'); + return <TextInput placeholder={placeholder} value={value} onChangeText={onChangeText} {...props} />; + },
208-212: Drop duplicate lucide-react-native mock.This module is already mocked on Lines 31–34. Duplicating can lead to non-deterministic behavior if changed later.
-jest.mock('lucide-react-native', () => ({ - SearchIcon: 'SearchIcon', - X: 'X', -}));
140-206: Centralize UI mocks to reduce maintenance.Consider moving these UI primitive mocks into a shared manual mock (e.g., mocks/components/ui/* or a single test-utils setup) and reusing across specs. Keeps tests lean and avoids drift between files.
src/app/(app)/__tests__/protocols.test.tsx (4)
33-39: Duplicate store mock — remove the second declaration.
jest.mock('@/stores/protocols/store'...)appears twice with the same body; the latter silently overwrites the former and adds noise.Apply this diff:
-jest.mock('@/stores/protocols/store', () => ({ - useProtocolsStore: () => mockProtocolsStore, -}));
70-78: Duplicate zero-state mock — keep a single definition.Two identical mocks for
@/components/common/zero-state. This can confuse future edits.Apply this diff to delete the second block (lines 96–104):
-jest.mock('@/components/common/zero-state', () => ({ - __esModule: true, - default: ({ heading, description }: { heading: string; description: string }) => { - const React = require('react'); - return React.createElement('View', { testID: 'zero-state' }, - React.createElement('Text', null, `ZeroState: ${heading}`) - ); - }, -}));Also applies to: 96-104
169-189: Avoid DOM querySelector in RNTL; use testIDs instead.
parentNode?.querySelector('[data-testid="action-button"]')isn’t reliable in React Native Testing Library. PrefergetByTestId('action-button')and ensure the component sets it.Apply this diff:
-// Find the action button (MoreVertical icon) to enter selection mode -const actionButton = getByText('Notifications').parentNode?.querySelector('[data-testid="action-button"]'); - -if (actionButton) { - await act(async () => { - fireEvent.press(actionButton); - }); - expect(queryByText('0 selected')).toBeTruthy(); - expect(queryByText('Select All')).toBeTruthy(); - expect(queryByText('Cancel')).toBeTruthy(); -} else { - // If we can't find the action button, verify that selection mode functionality exists - // by calling the component's internal methods indirectly through props - expect(getByText('Notifications')).toBeTruthy(); -} +// Enter selection mode via explicit testID +const actionButton = screen.getByTestId('action-button'); +await act(async () => { + fireEvent.press(actionButton); +}); +expect(queryByText('0 selected')).toBeTruthy(); +expect(queryByText('Select All')).toBeTruthy(); +expect(queryByText('Cancel')).toBeTruthy();And in
src/components/notifications/NotificationInbox.tsx(outside this diff), addtestID="action-button"to the Pressable that opens selection mode.
608-618: Prefer controlled time with fake timers over mocking Date constructor.
jest.spyOn(global, 'Date').mockImplementation(...)risks breaking code paths that useDate.now()ornew Date(value). Use modern fake timers and setSystemTime.Apply this diff:
-const mockDate = new Date('2024-01-15T10:00:00Z'); -jest.spyOn(global, 'Date').mockImplementation(() => mockDate as any); +jest.useFakeTimers().setSystemTime(new Date('2024-01-15T10:00:00Z')); ... -jest.restoreAllMocks(); +jest.useRealTimers();src/components/notifications/__tests__/NotificationInbox.test.tsx (4)
25-39: DRY: rely on the dedicated mocks for Gluestack hooks.Inline-mocking
@gluestack-ui/hookshere duplicates the project-level__mocks__/@gluestack-ui/hooks.tsand can diverge.Apply this diff to remove the local mock:
-// Mock gluestack-ui hooks to prevent keyboard bottom inset errors -jest.mock('@gluestack-ui/hooks', () => ({ - useKeyboardBottomInset: jest.fn(() => 0), - useControllableState: jest.fn((initialValue, onValueChange) => { - let state = initialValue; - const setState = (newValue: any) => { - state = newValue; - if (onValueChange) { - onValueChange(newValue); - } - }; - return [state, setState]; - }), -}));
169-189: Test intent vs. implementation mismatch.Test is titled “enters selection mode on long press” but it presses an action button. Either simulate long-press (expose a testID per item and
fireEvent(getByTestId('notification-item-1'), 'onLongPress')) or rename the test.Suggested rename:
-it('enters selection mode on long press', async () => { +it('enters selection mode via action button', async () => {Or expose
testID="notification-item-${id}"and simulateonLongPress.
323-338: These “bulk delete” tests call the API directly — consider exercising the UI path.You’re not validating component behavior (confirm modal, state reset, refetch, toasts). Add testIDs to the delete affordances and confirm modal to drive the flow.
Example adjustments (outside this file): add
testID="bulk-delete",testID="confirm-delete",testID="selection-count"inNotificationInbox.tsx. Then update tests:- await act(async () => { await deleteMessage('1'); }); - expect(mockDeleteMessage).toHaveBeenCalledWith('1'); + fireEvent.press(screen.getByTestId('bulk-delete')); + fireEvent.press(screen.getByTestId('confirm-delete')); + await waitFor(() => expect(mockDeleteMessage).toHaveBeenCalled()); + expect(mockRefetch).toHaveBeenCalled();Also applies to: 340-352
90-94: Timers hygiene.You’re enabling fake timers in beforeEach and flushing in afterEach. Consider
jest.useFakeTimers({ advanceTimers: true })andjest.runOnlyPendingTimers()beforeuseRealTimers()to ensure animations/promises settle.Also applies to: 138-144
__mocks__/react-native.ts (1)
39-57: Broaden core component mocks to cut per-test boilerplate.Many tests synthesize
TextInput/Pressablevia React elements. Exporting them here improves fidelity and enablesfireEventhelpers consistently.Apply this diff:
export const View = 'View'; export const Text = 'Text'; +export const Pressable = 'Pressable'; +export const TextInput = 'TextInput'; +export const FlatList = 'FlatList'; +export const RefreshControl = 'RefreshControl'; export const StyleSheet = { create: jest.fn().mockImplementation((styles) => styles), }; // Export default export default { Platform, PermissionsAndroid, DeviceEventEmitter, Alert, Linking, View, Text, + Pressable, + TextInput, + FlatList, + RefreshControl, StyleSheet, useColorScheme, };src/components/settings/__tests__/login-info-bottom-sheet.test.tsx (4)
1-5: Redundant Platform mocking.You also mock
Platforminside thereact-nativemock (lines 31–44). The top-levelreact-native/Libraries/Utilities/Platformmock isn’t needed.Apply this diff:
-// Mock Platform first, before any other imports -jest.mock('react-native/Libraries/Utilities/Platform', () => ({ - OS: 'ios', - select: jest.fn().mockImplementation((obj) => obj.ios || obj.default), -}));
292-296: Avoid order-dependent button selection.Using
getAllByTestId('button')[0/1]is brittle. Prefer explicit testIDs for cancel/save.Apply this diff pattern in tests:
-const buttons = screen.getAllByTestId('button'); -const cancelButton = buttons[0]; +const cancelButton = screen.getByTestId('cancel-button');And in
login-info-bottom-sheet.tsx(outside this file), settestID="cancel-button"andtestID="save-button"on the respective Buttons.Also applies to: 318-321, 359-362, 397-400, 502-505, 539-542, 560-563, 586-589, 614-617
232-238: KeyboardAvoidingView assertion is weak.You’re only checking the sheet renders. Consider enhancing the mock to capture
behaviorand assert'padding'on iOS.Example mock tweak:
-KeyboardAvoidingView: ({ children, ...props }: any) => { +KeyboardAvoidingView: ({ children, behavior, ...props }: any) => { const React = require('react'); - return React.createElement('View', { testID: 'keyboard-avoiding-view', ...props }, children); + return React.createElement('View', { testID: 'keyboard-avoiding-view', behavior, ...props }, children); },Then assert:
expect(screen.getByTestId('keyboard-avoiding-view').props.behavior).toBe('padding').
641-648: Stand-alone math test can be made meaningful or removed.This doesn’t involve the component. Either mock
useWindowDimensionsto landscape and assert the analytics payload usesisLandscape: true, or drop it.package.json (1)
229-240: Expo install exclude list — double-check it won’t mask future fixes.The exclusions point at older versions. Confirm they are still needed post-upgrade and won’t block Expo from suggesting correct versions later.
| "react-native-gesture-handler": "~2.24.0", | ||
| "react-native-get-random-values": "^1.11.0", | ||
| "react-native-keyboard-controller": "~1.15.2", | ||
| "react-native-keyboard-controller": "^1.18.6", | ||
| "react-native-logs": "~5.3.0", | ||
| "react-native-mmkv": "~3.1.0", | ||
| "react-native-permissions": "^5.4.1", | ||
| "react-native-reanimated": "~3.16.7", | ||
| "react-native-reanimated": "~3.17.4", | ||
| "react-native-restart": "0.0.27", | ||
| "react-native-safe-area-context": "~5.1.0", | ||
| "react-native-screens": "~4.8.0", | ||
| "react-native-svg": "~15.8.0", | ||
| "react-native-safe-area-context": "5.4.0", | ||
| "react-native-screens": "~4.11.1", | ||
| "react-native-svg": "15.11.2", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Reanimated/gesture-handler/safe-area versions likely OK, but verify New Architecture compatibility.
SDK 53 enables the New Architecture by default; ensure these libs are built correctly (babel plugin for Reanimated, pods rebuilt). Consider a quick validation run post-upgrade.
Also applies to: 153-154
🏁 Script executed:
#!/bin/bash
# Sanity-check library versions relevant to New Architecture with Expo SDK 53
jq -r '.dependencies | to_entries[] | select(.key|test("react-native-(reanimated|gesture-handler|screens|safe-area-context)|@shopify/flash-list|react-native-keyboard-controller")) | "\(.key)@\(.value)"' package.json
# Reanimated plugin present in babel config?
fd babel.config -t f | xargs -I{} node -e "const c=require('./{}'); console.log('babel.plugins:', c.plugins||c.presets)"Length of output: 581
Add Reanimated Babel plugin & rebuild iOS pods for New Architecture
- No
react-native-reanimated/plugindetected in your Babel config (no plugins array found); add it tobabel.config.js(or.babelrc) as the last entry inplugins. - Then run
cd ios && pod installand rebuild the app to ensure native modules load correctly.
🤖 Prompt for AI Agents
In package.json around lines 147-157: Babel config is missing the
react-native-reanimated/plugin; open your Babel config (babel.config.js or
.babelrc) and add "react-native-reanimated/plugin" as the last entry in the
plugins array, save, then run cd ios && pod install and rebuild the iOS app
(clean build if needed) so the new Reanimated native module is properly linked
and loaded.
| // Verify the correct state is used - when enabled but not connected | ||
| expect(mockT).toHaveBeenCalledWith('settings.realtime_geolocation_warning'); | ||
| // The component should render the "Connecting" state | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add concrete assertion for “Connecting to hub…” state
Currently no assertion validates the connecting text path.
// Verify the correct state is used - when enabled but not connected
expect(mockT).toHaveBeenCalledWith('settings.realtime_geolocation_warning');
- // The component should render the "Connecting" state
+ // The component should render the "Connecting" state
+ expect(screen.getByText(/Connecting to hub\.\.\./)).toBeTruthy();📝 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.
| // Verify the correct state is used - when enabled but not connected | |
| expect(mockT).toHaveBeenCalledWith('settings.realtime_geolocation_warning'); | |
| // The component should render the "Connecting" state | |
| }); | |
| // Verify the correct state is used - when enabled but not connected | |
| expect(mockT).toHaveBeenCalledWith('settings.realtime_geolocation_warning'); | |
| // The component should render the "Connecting" state | |
| expect(screen.getByText(/Connecting to hub\.\.\./)).toBeTruthy(); |
🤖 Prompt for AI Agents
In src/components/settings/__tests__/realtime-geolocation-item.test.tsx around
lines 139-142, add a concrete assertion that the component renders the
"Connecting" state: assert that the translation key for the connecting text
(e.g. 'settings.realtime_geolocation_connecting') is requested (mockT called
with that key) and that the rendered output contains the corresponding text (use
the test renderer/query helpers to check the visible "Connecting to hub…" string
or the mockT return value). Ensure this assertion follows the existing
expectation for the warning text so the test verifies both the translation
lookup and the rendered connecting message.
src/types/gluestack-overrides.d.ts
Outdated
| // Missing tailwind-variants config type | ||
| declare module 'tailwind-variants/dist/config' { | ||
| export interface TVConfig<V = {}, EV = V> { | ||
| // Basic config structure | ||
| [key: string]: any; | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
De-duplicate TVConfig declaration
This repeats the TVConfig augmentation also present in types/index.d.ts. Keep a single declaration (recommend keeping the stricter V = {} version in types/index.d.ts).
Apply:
-// Missing tailwind-variants config type
-declare module 'tailwind-variants/dist/config' {
- export interface TVConfig<V = {}, EV = V> {
- // Basic config structure
- [key: string]: any;
- }
-}Check all TVConfig declarations:
🏁 Script executed:
#!/bin/bash
rg -nP "(?s)declare module 'tailwind-variants/dist/config'.*TVConfig"Length of output: 77
🏁 Script executed:
#!/bin/bash
# Search for all TVConfig interface declarations in .d.ts files
rg -n "export interface TVConfig" -g "*.d.ts"
# Search for any module augmentation of 'tailwind-variants/dist/config'
rg -n "declare module 'tailwind-variants/dist/config'" -g "*.d.ts"Length of output: 590
Remove duplicated TVConfig declarations
- In both
types/gluestack-overrides.d.tsandsrc/types/gluestack-overrides.d.ts, delete the entiredeclare module 'tailwind-variants/dist/config' { export interface TVConfig<V = {}, EV = V> { [key: string]: any; } }
- In
types/index.d.ts, update the existing augmentation to use the stricter default:declare module 'tailwind-variants/dist/config' { - export interface TVConfig<V = any, EV = V> { + export interface TVConfig<V = {}, EV = V> { [key: string]: any; } }
🤖 Prompt for AI Agents
In src/types/gluestack-overrides.d.ts around lines 51-57, remove the entire
duplicate module augmentation for 'tailwind-variants/dist/config' (the declare
module block exporting TVConfig) and also delete the same duplicate block from
types/gluestack-overrides.d.ts; then open types/index.d.ts and update the
existing augmentation's generic defaults to a stricter default by changing
TVConfig<V = {}, EV = V> to TVConfig<V = unknown, EV = V> (or otherwise replace
{} with a stricter type) so the single remaining augmentation uses the stricter
default.
types/gluestack-overrides.d.ts
Outdated
| // Type overrides for Gluestack UI components to fix compilation issues | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add explicit React type import in .d.ts to avoid "Cannot find name 'React'"
React.ReactNode is referenced without importing React types. In .d.ts files this often fails under react-jsx. Import React types once at the top.
+import type * as React from 'react';
// Type overrides for Gluestack UI components to fix compilation issues📝 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.
| // Type overrides for Gluestack UI components to fix compilation issues | |
| import type * as React from 'react'; | |
| // Type overrides for Gluestack UI components to fix compilation issues |
🤖 Prompt for AI Agents
In types/gluestack-overrides.d.ts around lines 1 to 2, the declaration file
references React types (e.g., React.ReactNode) but does not import React,
causing "Cannot find name 'React'"; fix by adding an explicit top-level type
import such as `import type React from 'react';` (or `import React from
'react';` if preferred) at the very top of the .d.ts so all React type
references resolve.
types/index.d.ts
Outdated
| declare module 'tailwind-variants/dist/config' { | ||
| export interface TVConfig<V = any, EV = V> { | ||
| [key: string]: any; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unify TVConfig defaults to avoid conflicting merged declarations
You already declare TVConfig in src/types/gluestack-overrides.d.ts with V = {}. Having V = any here can conflict in module merging. Align the defaults.
Apply:
declare module 'tailwind-variants/dist/config' {
- export interface TVConfig<V = any, EV = V> {
+ export interface TVConfig<V = {}, EV = V> {
[key: string]: any;
}
}📝 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.
| declare module 'tailwind-variants/dist/config' { | |
| export interface TVConfig<V = any, EV = V> { | |
| [key: string]: any; | |
| } | |
| } | |
| declare module 'tailwind-variants/dist/config' { | |
| export interface TVConfig<V = {}, EV = V> { | |
| [key: string]: any; | |
| } | |
| } |
🤖 Prompt for AI Agents
types/index.d.ts around lines 3 to 7: the TVConfig declaration uses V = any
which conflicts with the other declaration (src/types/gluestack-overrides.d.ts)
that sets V = {}; change the defaults to match by setting V = {} and EV = V (or
EV = {}) so module merging uses the same generic defaults; update the interface
generic defaults accordingly and keep the index signature as-is.
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (26)
src/components/ui/bottomsheet/index.tsx (2)
93-100: Don’t auto-close at index 0; close only at -1.Closing on
index === 0will immediately hide the sheet when a user snaps to the first (fully open) point. This causes flicker and breaks expected behavior.- if (index === 0 || index === -1) { + if (index === -1) { handleClose(); }
81-90: BroadensnapPointstype; clean up unused props.
snapPointsshould accept numbers as well as strings per Gorhom API.defaultIsOpenandsnapToIndexare declared but unused here—either implement or remove to avoid confusion.- snapPoints: string[]; + snapPoints: Array<string | number>;If you decide to keep
defaultIsOpen/snapToIndex, consider:// before return in BottomSheetPortal const initialIndex = props.index ?? (defaultIsOpen ? (props.snapToIndex ?? 0) : -1);and pass
index={initialIndex}.src/lib/utils.ts (1)
225-227: Bug: ‘dd’ branch overwrites seconds token and uses day-of-week.This block checks for “dd” but replaces “ss” with
getDay()(0–6). It corrupts the seconds replacement done above and produces wrong output.- if (format.indexOf('dd') > -1) { - format = format.replace('ss', padLeadingZero(date.getDay()).toString()); - }If you want day-of-week support, consider a dedicated token (e.g., EEE) and map 0–6 to names.
src/components/maps/__tests__/pin-actions.test.tsx (1)
553-568: Test name and assertion don’t match behavior (noop handler passed, title says "missing")Rename for accuracy and assert no throw to reflect intent.
- describe('Error handling', () => { - it('should handle missing onSetAsCurrentCall prop gracefully', () => { + describe('Error handling', () => { + it('should not crash when onSetAsCurrentCall is a no-op', () => { render( <PinDetailModal pin={mockCallPin} isOpen={true} onClose={mockOnClose} - onSetAsCurrentCall={() => { }} + onSetAsCurrentCall={() => {}} /> ); - const setCurrentCallButton = screen.getByText('map.set_as_current_call'); - fireEvent.press(setCurrentCallButton); + expect(() => { + const setCurrentCallButton = screen.getByText('map.set_as_current_call'); + fireEvent.press(setCurrentCallButton); + }).not.toThrow(); - // Should not crash and should not call onClose since onSetAsCurrentCall is undefined + // Should not call onClose when handler is a no-op expect(mockOnClose).not.toHaveBeenCalled(); });src/components/ui/grid/index.tsx (1)
26-37: Don’t skip items with undefined colSpan; normalize instead to keep row index mapping consistentSkipping desynchronizes itemsPerRow from children indexes, which can make GridItem fail to find its row, leading to NaN flexBasis. Normalize undefined to 1 instead.
- const colSpan = colSpanArr[i]; - - if (colSpan === undefined) { - continue; // Skip undefined colSpan values - } + const rawColSpan = colSpanArr[i]; + const colSpan = rawColSpan ?? 1; // if current row is full, go to next row if (currentRowTotalColSpan + colSpan > numColumns) { currentRow++; currentRowTotalColSpan = colSpan; } else { // if current row is not full, add colSpan to current row currentRowTotalColSpan += colSpan; }src/components/messages/compose-message-sheet.tsx (1)
539-563: Add ‘units’ to recipient tabs
In src/components/messages/compose-message-sheet.tsx update the array to include “units” so the Units view is reachable:- {['personnel', 'groups', 'roles'].map((tab) => ( + {['personnel', 'groups', 'roles', 'units'].map((tab) => (Also add an e2e/unit test asserting that tapping the Units tab renders the
dispatchData.unitslist (e.g. items with testID prefixrecipient-item-).src/stores/auth/store.tsx (4)
143-145: Fix refresh scheduler math (bug).
expires_inis already relative seconds; subtractingDate.now()makes it negative.- const expiresIn = response.expires_in * 1000 - Date.now() - 60000; // Refresh 1 minute before expiry - setTimeout(() => get().refreshAccessToken(), expiresIn); + const delay = Math.max(0, response.expires_in * 1000 - 60_000); // 1 min before expiry + refreshTimeout && clearTimeout(refreshTimeout); + refreshTimeout = setTimeout(() => get().refreshAccessToken(), delay);
73-86: Schedule auto-refresh after successful login.Currently commented out, so tokens won’t auto-refresh after sign-in.
- // Set up automatic token refresh - //const decodedToken: { exp: number } = jwtDecode( - //); - //const now = new Date(); - //const expiresIn = - // response.authResponse?.expires_in! * 1000 - Date.now() - 60000; // Refresh 1 minute before expiry - //const expiresOn = new Date( - // now.getTime() + response.authResponse?.expires_in! * 1000 - //) - // .getTime() - // .toString(); - - //setTimeout(() => get().refreshAccessToken(), expiresIn); + // Schedule first refresh ~1 minute before access token expiry + const loginDelay = Math.max(0, (response.authResponse?.expires_in ?? 0) * 1000 - 60_000); + refreshTimeout && clearTimeout(refreshTimeout); + refreshTimeout = setTimeout(() => get().refreshAccessToken(), loginDelay);
100-121: Clear pending refresh timers on logout.Prevents background refresh after sign-out.
logout: async () => { // Clear persisted authResponse to prevent re-hydration of signed-in session try { await removeItem('authResponse'); } catch (error) { logger.warn({ message: 'Failed to remove authResponse from storage during logout', context: { error }, }); } + // Cancel any scheduled refresh + if (refreshTimeout) { + clearTimeout(refreshTimeout); + refreshTimeout = null; + } + set({ accessToken: null, refreshToken: null, status: 'signedOut', error: null, profile: null, isFirstTime: true, userId: null, refreshTokenExpiresOn: null, }); },
1-234: Normalize JWT payload decoding to use base64url
- In src/stores/auth/store.tsx at lines 55 and 162, tokenParts[1] is decoded with base64.decode without URL‐safe normalization; replace with a helper that converts ‘-’→‘+’, ‘_’→‘/’ and pads the string before decoding (RFC 4648).
src/stores/home/home-store.ts (1)
6-6: Broken import path likely stops builds.
@/api/satuseslooks misspelled. It’s also unused.Apply one of:
- import { getAllPersonnelStatuses } from '@/api/satuses'; + // import { getAllPersonnelStatuses } from '@/api/statuses'; // If needed lateror simply remove:
- import { getAllPersonnelStatuses } from '@/api/satuses'; + // (removed unused import)src/services/push-notification.ts (2)
13-19: Unify PushNotificationData type; don’t redefine it here.This file re-exports a different
PushNotificationDatathan the store’s, risking drift.-import { usePushNotificationModalStore } from '@/stores/push-notification/store'; +import { usePushNotificationModalStore } from '@/stores/push-notification/store'; +import type { PushNotificationData } from '@/stores/push-notification/store';-export interface PushNotificationData { - title?: string; - body?: string; - data?: Record<string, unknown>; -} +// Use the central PushNotificationData from the store
201-208: Avoid logging full push tokens (sensitive).Tokens are credentials; don’t write them to logs.
- logger.info({ - message: 'Push notification token obtained', - context: { - token: this.pushToken, - userId, - platform: Platform.OS, - }, - }); + const redacted = typeof this.pushToken === 'string' + ? `${this.pushToken.slice(0, 6)}…${this.pushToken.slice(-4)}` + : 'unknown'; + logger.info({ + message: 'Push notification token obtained', + context: { token: redacted, userId, platform: Platform.OS }, + });src/stores/roles/store.ts (2)
59-63: Loading state never cleared on success.
fetchRolesForUnitsetsisLoading: truebut doesn’t reset it on success.- const unitRoles = (await getRoleAssignmentsForUnit(unitId)) as ApiResponse<ActiveUnitRoleResultData[]>; - set({ unitRoleAssignments: unitRoles.Data }); + const unitRoles = (await getRoleAssignmentsForUnit(unitId)) as ApiResponse<ActiveUnitRoleResultData[]>; + set({ unitRoleAssignments: unitRoles.Data, isLoading: false });
83-90:assignRolesleavesisLoadingtrue on success.UI will get stuck in loading.
assignRoles: async (data: SetUnitRolesInput) => { set({ isLoading: true, error: null }); try { await setRoleAssignmentsForUnit(data); - } catch (error) { - set({ error: 'Failed to assign user to role', isLoading: false }); - } + set({ isLoading: false }); + } catch (error) { + set({ error: 'Failed to assign user to role', isLoading: false }); + } },Optionally, refresh the assignments after success to reflect changes.
src/app/call/[id].tsx (1)
171-205: Guard routing when coordinates are missing/invalidPrevent attempts with undefined or zeroed coordinates; emit a specific failure reason.
Apply:
const handleRoute = async () => { try { // Track analytics for route action trackEvent('call_route_opened', { timestamp: new Date().toISOString(), callId: call?.CallId || callId || '', hasUserLocation: !!(userLocation.latitude && userLocation.longitude), destinationAddress: call?.Address || '', }); - const latitude = coordinates.latitude ?? (call?.Latitude ? parseFloat(call.Latitude) : undefined); - const longitude = coordinates.longitude ?? (call?.Longitude ? parseFloat(call.Longitude) : undefined); + const latitude = coordinates.latitude ?? (call?.Latitude ? parseFloat(call.Latitude) : undefined); + const longitude = coordinates.longitude ?? (call?.Longitude ? parseFloat(call.Longitude) : undefined); + if (!Number.isFinite(latitude as number) || !Number.isFinite(longitude as number)) { + showToast('error', t('call_detail.failed_to_open_maps')); + trackEvent('call_route_failed', { + timestamp: new Date().toISOString(), + callId: call?.CallId || callId || '', + reason: 'missing_coordinates', + }); + return; + }src/components/maps/full-screen-location-picker.tsx (1)
112-125: Treat 0,0 as “no initial location” to recover user positionPrevents the picker from accepting Null Island as a real initial value.
Apply:
useEffect(() => { isMountedRef.current = true; - if (initialLocation) { + const hasValidInitial = + !!initialLocation && !(initialLocation.latitude === 0 && initialLocation.longitude === 0); + if (hasValidInitial) { setCurrentLocation(initialLocation); reverseGeocode(initialLocation.latitude, initialLocation.longitude); } else { getUserLocation(); }src/app/call/[id]/edit.tsx (1)
214-219: Avoid dropping valid 0/0 coords; keep the address spreadThe address spread on Line 218 is good. However, the truthy check
if (call.Latitude && call.Longitude)skips valid 0 or "0" coordinates and prevents pre-population. Use a finite-number guard.Apply:
- // Set selected location if coordinates exist - if (call.Latitude && call.Longitude) { - setSelectedLocation({ - latitude: parseFloat(call.Latitude), - longitude: parseFloat(call.Longitude), - ...(call.Address && { address: call.Address }), - }); - } + // Set selected location if coordinates parse to finite numbers (supports 0/0) + const latNum = call.Latitude !== undefined ? parseFloat(call.Latitude) : NaN; + const lngNum = call.Longitude !== undefined ? parseFloat(call.Longitude) : NaN; + if (Number.isFinite(latNum) && Number.isFinite(lngNum)) { + setSelectedLocation({ + latitude: latNum, + longitude: lngNum, + ...(call.Address && { address: call.Address }), + }); + }src/stores/security/store.ts (1)
23-31: Harden response handling and avoid swallowing errorsCasting with
ashides mismatches; also the catch block does nothing, leaving callers without signal. Set error state, null out rights on failure, and guard against missing Data.- const response = (await getCurrentUsersRights()) as ApiResponse<DepartmentRightsResultData>; - - set({ - rights: response.Data, - }); - } catch (error) { - // If refresh fails, log out the user - } + const response = (await getCurrentUsersRights()) as ApiResponse<DepartmentRightsResultData>; + const rights = response?.Data ?? null; + set({ rights, error: null }); + } catch (error) { + set({ + rights: null, + error: error instanceof Error ? error.message : 'Failed to load rights', + }); + // TODO: consider triggering a logout/reauth flow here if rights are required for access + }src/stores/personnel/__tests__/store.test.ts (1)
122-139: Avoid toggling Jest timers globally inside a single testThis test switches to real timers, then leaves the suite in fake-timer mode, which can cause flakiness in later tests. Either keep real timers throughout or use fake timers and advance time locally.
Option A (simplest): remove both lines; real timers are the default.
- jest.useRealTimers(); // Use real timers for this test to avoid timing issues ... - jest.useFakeTimers(); // Restore fake timersOption B (stay with fakes): use fake timers and advance time.
- jest.useRealTimers(); + jest.useFakeTimers(); @@ - await fetchPromise; + await act(async () => { + jest.advanceTimersByTime(110); + await fetchPromise; + }); - jest.useFakeTimers(); + jest.useRealTimers();src/stores/calls/store.ts (1)
31-39: Init can hang on API error; add try/catch/finally and parallelize callsWithout error handling, any rejection leaves isLoading stuck true and throws an unhandled rejection. Also, calls can be done in parallel.
Apply:
- const callsResponse = (await getCalls()) as ApiResponse<CallResultData[]>; - const callPrioritiesResponse = (await getCallPriorities()) as ApiResponse<CallPriorityResultData[]>; - const callTypesResponse = (await getCallTypes()) as ApiResponse<CallTypeResultData[]>; - set({ - calls: callsResponse.Data, - callPriorities: callPrioritiesResponse.Data, - callTypes: callTypesResponse.Data, - isLoading: false, - }); + try { + const [callsResponse, callPrioritiesResponse, callTypesResponse] = await Promise.all([ + getCalls() as Promise<ApiResponse<CallResultData[]>>, + getCallPriorities() as Promise<ApiResponse<CallPriorityResultData[]>>, + getCallTypes() as Promise<ApiResponse<CallTypeResultData[]>>, + ]); + set({ + calls: callsResponse.Data, + callPriorities: callPrioritiesResponse.Data, + callTypes: callTypesResponse.Data, + }); + } catch { + set({ error: 'Failed to initialize calls data' }); + } finally { + set({ isLoading: false }); + }src/stores/calls/detail-store.ts (2)
55-63: Reset leaves stale notes/files/images and errors in stateClear lists and related flags/errors to avoid leaking data between calls.
reset: () => set({ call: null, callExtraData: null, callPriority: null, - isLoading: false, - isNotesLoading: false, - error: null, + callNotes: [], + callImages: null, + callFiles: null, + isLoading: false, + isNotesLoading: false, + isLoadingImages: false, + isLoadingFiles: false, + error: null, + errorImages: null, + errorFiles: null, }),
121-126: Guard against nullish Note/FullName to avoid runtime errorsAPIs may return null for these fields; current code can throw on toLowerCase.
- return callNotes?.filter((note: CallNoteResultData) => note.Note.toLowerCase().includes(trimmedQuery.toLowerCase()) || note.FullName.toLowerCase().includes(trimmedQuery.toLowerCase())); + const q = trimmedQuery.toLowerCase(); + return callNotes?.filter( + (note: CallNoteResultData) => + (note.Note ?? '').toLowerCase().includes(q) || + (note.FullName ?? '').toLowerCase().includes(q) + );src/stores/offline-queue/__tests__/store.test.ts (1)
357-373: Bug: wrong eventId used in retryAllFailedEvents testSame issue as above—hardcoded 'test-event-id' won’t match 'test-event-id-1'/'-2'. Assertions inside the conditional may never execute.
- store.addEvent(QueuedEventType.UNIT_STATUS, { test: 'data1' }); - store.addEvent(QueuedEventType.UNIT_STATUS, { test: 'data2' }); - store.updateEventStatus('test-event-id', QueuedEventStatus.FAILED); + const id1 = store.addEvent(QueuedEventType.UNIT_STATUS, { test: 'data1' }); + const id2 = store.addEvent(QueuedEventType.UNIT_STATUS, { test: 'data2' }); + store.updateEventStatus(id1, QueuedEventStatus.FAILED); ... - if (event.id === 'test-event-id') { + if (event.id === id1) {src/services/bluetooth-audio.service.ts (2)
471-509: Normalize advertised UUIDs to handle 16-bit vs 128-bit formsserviceUUIDs may appear as 16-bit (e.g., '110A') and won’t match your 128-bit list. Normalize before comparison to improve detection.
- const hasAudioService = - advertisingData?.serviceUUIDs?.some((uuid: string) => { - const upperUuid = uuid.toUpperCase(); - return [AUDIO_SERVICE_UUID, HFP_SERVICE_UUID, HSP_SERVICE_UUID, AINA_HEADSET_SERVICE, B01INRICO_HEADSET_SERVICE, HYS_HEADSET_SERVICE].includes(upperUuid); - }) || false; + const to128 = (u: string) => + u.length === 4 + ? `0000${u.toUpperCase()}-0000-1000-8000-00805F9B34FB` + : u.toUpperCase(); + const target = new Set([AUDIO_SERVICE_UUID, HFP_SERVICE_UUID, HSP_SERVICE_UUID, AINA_HEADSET_SERVICE, B01INRICO_HEADSET_SERVICE, HYS_HEADSET_SERVICE]); + const hasAudioService = + advertisingData?.serviceUUIDs?.some((uuid: string) => target.has(to128(uuid))) || false;
983-1015: Incorrect service/characteristic pairing for generic notificationsBUTTON_CONTROL_UUIDS holds service UUIDs, but you pass them as characteristic UUIDs under Generic Access (0x1800). startNotification(service, characteristic) will fail silently and miss real button events.
- // Add generic button control UUIDs - ...BUTTON_CONTROL_UUIDS.map((uuid) => ({ service: '00001800-0000-1000-8000-00805F9B34FB', characteristic: uuid })), // Generic service + // Discover and subscribe to any NOTIFY characteristics under known servicesSuggestion:
- After retrieveServices(deviceId), iterate discovered characteristics.
- For services in BUTTON_CONTROL_UUIDS, startNotification on characteristics that have Notify/Indicate properties.
- Rename BUTTON_CONTROL_UUIDS to BUTTON_CONTROL_SERVICES for clarity.
I can draft a concrete implementation if desired.
♻️ Duplicate comments (9)
src/components/ui/drawer/index.tsx (1)
33-39: Map cssInterop for Backdrop and Content wrappers tooUIDrawer.Backdrop and UIDrawer.Content receive className but aren’t interopped; styles may not apply through the wrapper (esp. on web). Add mappings alongside the others.
cssInterop(AnimatedPressable as any, { className: 'style' }); cssInterop(Motion.View as any, { className: 'style' }); cssInterop(UIDrawer as any, { className: 'style' }); +cssInterop(UIDrawer.Backdrop as any, { className: 'style' }); +cssInterop(UIDrawer.Content as any, { className: 'style' }); cssInterop(UIDrawer.Body as any, { className: 'style' }); cssInterop(UIDrawer.CloseButton as any, { className: 'style' }); cssInterop(UIDrawer.Footer as any, { className: 'style' }); cssInterop(UIDrawer.Header as any, { className: 'style' });tsconfig.json (1)
32-36: Stricter TS flags enabled — nicenoImplicitAny, noUncheckedIndexedAccess, exactOptionalPropertyTypes, isolatedModules improve safety.
types/gluestack-overrides.d.ts (1)
1-2: Import React types to fix React.ReactNode referencesWithout importing React types, React.ReactNode may error under strict/isolatedModules.
+import type * as React from 'react'; // Type overrides for Gluestack UI components to fix compilation issuessrc/components/ui/actionsheet/index.tsx (1)
49-55: Factory keys look good; DragIndicatorWrapper key matches consumersThe key alignment issue from the earlier review is fixed (
DragIndicatorWrapperpresent). Mapping looks consistent.src/components/settings/__tests__/realtime-geolocation-item.test.tsx (1)
20-33: Switch mock uses a string element; use RN Pressable so fireEvent.press worksCreating
'Pressable'as a string host doesn’t trigger RN press semantics in testing-library. Use the mocked RNPressableand forward accessibility state.jest.mock('../../ui/switch', () => { const mockReact = require('react'); + const { Pressable, Text } = require('react-native'); return { - Switch: (props: any) => { - // Create a mock switch that calls onValueChange when pressed (React Native style) - return mockReact.createElement('Pressable', { - testID: 'switch', - onPress: () => props.onValueChange && props.onValueChange(!props.value), - 'data-value': props.value, - accessible: true, - accessibilityRole: 'switch' - }, `Switch: ${props.value ? 'On' : 'Off'}`); - }, + Switch: ({ value, onValueChange, ...rest }: any) => + mockReact.createElement( + Pressable, + { + testID: 'switch', + onPress: () => onValueChange && onValueChange(!value), + accessible: true, + accessibilityRole: 'switch', + accessibilityState: { checked: !!value }, + ...rest, + }, + mockReact.createElement(Text, {}, `Switch: ${value ? 'On' : 'Off'}`) + ), }; });jest-setup.ts (1)
551-558: MMKV boolean default now false — matches prior guidanceThis avoids surprise “true” when the key is unset.
src/services/bluetooth-audio.service.ts (1)
50-51: Timers: unified types across environments — good fixReturnType removes RN/Node mismatch. This addresses the earlier suggestion.
src/services/__tests__/bluetooth-audio.service.test.ts (2)
18-38: RN mock is correctly hoisted; minor resilience nitThis fixes the prior ordering issue. To avoid any future hoist/closure pitfalls, consider deriving requestMultiple inside the factory while still exposing the same handle for expectations.
Apply this safer pattern:
-jest.mock('react-native', () => ({ +jest.mock('react-native', () => { + const requestMultiple = (...args: unknown[]) => mockRequestMultiple(...args as []); + return { Platform: { OS: 'android' }, PermissionsAndroid: { PERMISSIONS: { BLUETOOTH_SCAN: 'android.permission.BLUETOOTH_SCAN', BLUETOOTH_CONNECT: 'android.permission.BLUETOOTH_CONNECT', }, RESULTS: { GRANTED: 'granted', DENIED: 'denied', }, - requestMultiple: mockRequestMultiple, + requestMultiple, }, DeviceEventEmitter: { addListener: jest.fn(), }, Alert: { alert: jest.fn(), }, -})); +}});
122-126: Import order fix looks right (mocks before require)This resolves the previous side-effect import issue and ensures the RN mock is applied.
| // Track analytics for images modal opening | ||
| trackEvent('call_images_opened', { | ||
| timestamp: new Date().toISOString(), | ||
| callId: call?.CallId || callId, | ||
| callId: call?.CallId || callId || '', | ||
| imagesCount: call?.ImgagesCount || 0, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential typo: ImgagesCount vs ImagesCount
Confirm the correct property name; analytics/UI may miscount.
Run:
If it should be ImagesCount, update occurrences in this file accordingly:
- imagesCount: call?.ImgagesCount || 0,
+ imagesCount: call?.ImagesCount || 0,And UI badges similarly.
Also applies to: 137-141, 553-555
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '\bImgagesCount\b|\bImagesCount\b' --type ts --type tsxLength of output: 96
🏁 Script executed:
#!/bin/bash
# Search for any occurrences of the misspelled property
rg -nC2 'ImgagesCount'
# Search for the correctly spelled property
rg -nC2 'ImagesCount'Length of output: 1707
Rename ImgagesCount to ImagesCount globally
- In
src/models/v4/calls/callResultData.ts(line 14), changetopublic ImgagesCount: number = 0;
public ImagesCount: number = 0;
- In
src/app/call/[id].tsx, update all occurrences:
• Line 91:imagesCount: call?.ImgagesCount || 0→imagesCount: call?.ImagesCount || 0
• Line 139:imagesCount: call.ImgagesCount || 0→imagesCount: call.ImagesCount || 0
• Lines 553–555: replacecall?.ImgagesCount→call?.ImagesCountin the UI badge
🤖 Prompt for AI Agents
In src/models/v4/calls/callResultData.ts around line 14, rename the class
property from ImgagesCount to ImagesCount (public ImagesCount: number = 0;) and
update any type references/imports accordingly; in src/app/call/[id].tsx update
the three occurrences noted: at ~line 91 change imagesCount: call?.ImgagesCount
|| 0 to imagesCount: call?.ImagesCount || 0, at ~line 139 change imagesCount:
call.ImgagesCount || 0 to imagesCount: call.ImagesCount || 0, and at ~lines
553–555 replace call?.ImgagesCount with call?.ImagesCount in the UI badge; run a
project-wide search for "ImgagesCount" to update any remaining usages and
rebuild/typecheck to ensure no references remain.
| note: data.note || '', | ||
| address: data.address || '', | ||
| latitude: data.latitude || 0, | ||
| longitude: data.longitude || 0, | ||
| what3words: data.what3words || '', | ||
| plusCode: data.plusCode || '', | ||
| contactName: data.contactName || '', | ||
| contactInfo: data.contactInfo || '', | ||
| dispatchUsers: data.dispatchSelection?.users || [], | ||
| dispatchGroups: data.dispatchSelection?.groups || [], | ||
| dispatchRoles: data.dispatchSelection?.roles || [], | ||
| dispatchUnits: data.dispatchSelection?.units || [], | ||
| dispatchEveryone: data.dispatchSelection?.everyone || false, | ||
| }); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Do not default latitude/longitude to 0; this writes (0,0) unintentionally
Defaulting to 0 will overwrite existing data with Gulf-of-Guinea coordinates when the user hasn’t selected a location. Send coords only when present.
Apply within the payload:
- note: data.note || '',
- address: data.address || '',
- latitude: data.latitude || 0,
- longitude: data.longitude || 0,
- what3words: data.what3words || '',
- plusCode: data.plusCode || '',
- contactName: data.contactName || '',
- contactInfo: data.contactInfo || '',
+ note: data.note || '',
+ address: data.address || '',
+ ...(typeof data.latitude === 'number' ? { latitude: data.latitude } : {}),
+ ...(typeof data.longitude === 'number' ? { longitude: data.longitude } : {}),
+ what3words: data.what3words || '',
+ plusCode: data.plusCode || '',
+ contactName: data.contactName || '',
+ contactInfo: data.contactInfo || '',Also update the setter and analytics guards to handle 0 correctly:
- if (selectedLocation?.latitude && selectedLocation?.longitude) {
+ if (selectedLocation && Number.isFinite(selectedLocation.latitude) && Number.isFinite(selectedLocation.longitude)) {
data.latitude = selectedLocation.latitude;
data.longitude = selectedLocation.longitude;
}- hasCoordinates: !!(data.latitude && data.longitude),
+ hasCoordinates: Number.isFinite(data.latitude) && Number.isFinite(data.longitude),- hasLocation: !!(data.latitude && data.longitude),
+ hasLocation: Number.isFinite(data.latitude) && Number.isFinite(data.longitude),Run to find other truthy coord checks to fix:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '(?<![.\w])(?:has(Coordinates|Location)\s*:\s*!!\()|(?:Latitude\s*&&\s*.*Longitude)|(?:latitude\s*&&\s*.*longitude)' --type ts --type tsxLength of output: 178
🏁 Script executed:
#!/bin/bash
# Search for “!!” and truthy latitude/longitude checks across .ts and .tsx files
rg -nP -C2 '(?<![.\w])(?:has(Coordinates|Location)\s*:\s*!!\()|(?:latitude\s*&&\s*.*longitude)' -g '*.ts' -g '*.tsx'Length of output: 4181
Replace all truthy latitude/longitude checks with explicit finite-number validations
-
In src/app/call/[id]/edit.tsx:
• Payload → spread in{ latitude, longitude }only whentypeof data.latitude === 'number'/typeof data.longitude === 'number'
• Setter guard (line 227) →if (selectedLocation && Number.isFinite(selectedLocation.latitude) && Number.isFinite(selectedLocation.longitude))
• Analytics flags (lines 245, 282) → useNumber.isFinite(data.latitude) && Number.isFinite(data.longitude)instead of!!(data.latitude && data.longitude) -
In src/services/offline-event-manager.service.ts (line 285):
if (event.data.latitude && event.data.longitude)→
if (Number.isFinite(event.data.latitude) && Number.isFinite(event.data.longitude)) -
In src/app/call/[id].tsx:
• Line 137:hasCoordinates: !!(coordinates.latitude && coordinates.longitude)→Number.isFinite(coordinates.latitude) && Number.isFinite(coordinates.longitude)
• Line 175:hasUserLocation: !!(userLocation.latitude && userLocation.longitude)→Number.isFinite(userLocation.latitude) && Number.isFinite(userLocation.longitude)
• Line 532:coordinates.latitude && coordinates.longitude ? …→Number.isFinite(coordinates.latitude) && Number.isFinite(coordinates.longitude) ? … -
In src/components/sidebar/call-sidebar.tsx (line 79), src/api/calls/callNotes.ts (line 24) and src/api/calls/callFiles.ts (line 140):
Replace everyif (latitude && longitude)withif (Number.isFinite(latitude) && Number.isFinite(longitude)) -
Remove all defaults of
0for latitude/longitude—only send coords when present via conditional spreads.
🤖 Prompt for AI Agents
In src/app/call/[id]/edit.tsx around lines 261-274 (and related spots: setter
guard at ~227, analytics flags at ~245 and ~282), replace truthy
latitude/longitude checks and zero defaults with explicit finite-number
validations: remove latitude: data.latitude || 0 and longitude: data.longitude
|| 0 and instead only spread { latitude, longitude } into the payload when
typeof data.latitude === 'number' && Number.isFinite(data.latitude) and likewise
for longitude; change the setter guard to if (selectedLocation &&
Number.isFinite(selectedLocation.latitude) &&
Number.isFinite(selectedLocation.longitude)); update analytics flags to
Number.isFinite(data.latitude) && Number.isFinite(data.longitude). Also apply
the same pattern in the other referenced files/lines
(offline-event-manager.service.ts line ~285, src/app/call/[id].tsx lines
~137/175/532, src/components/sidebar/call-sidebar.tsx line ~79,
src/api/calls/callNotes.ts line ~24, src/api/calls/callFiles.ts line ~140):
replace any if (latitude && longitude) or truthy checks with
Number.isFinite(...) checks and stop defaulting coordinates to 0 so coords are
only sent when valid finite numbers exist.
src/app/call/new/index.tsx
Outdated
| <FullScreenLocationPicker | ||
| key={showLocationPicker ? 'location-picker-open' : 'location-picker-closed'} | ||
| initialLocation={selectedLocation || undefined} | ||
| initialLocation={selectedLocation || { latitude: 0, longitude: 0 }} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Passing {0,0} initialLocation prevents user location lookup
This forces the picker to stick at 0,0 and skip acquiring the real position.
Apply:
- initialLocation={selectedLocation || { latitude: 0, longitude: 0 }}
+ initialLocation={selectedLocation ?? undefined}Also add a defensive check in the picker to treat 0,0 as “no initial location” (see comment in the picker file).
Search for other callers that may pass { latitude: 0, longitude: 0 }:
🏁 Script executed:
#!/bin/bash
rg -nP '\{\s*latitude:\s*0\s*,\s*longitude:\s*0\s*\}' --type tsxLength of output: 94
🏁 Script executed:
#!/bin/bash
rg -nP '\{\s*latitude:\s*0\s*,\s*longitude:\s*0\s*\}' -g '*.tsx'Length of output: 289
Replace { latitude: 0, longitude: 0 } fallbacks with undefined in both callers
- In src/app/call/new/index.tsx (line 1275) and src/app/call/[id]/edit.tsx (line 811), replace
- initialLocation={selectedLocation || { latitude: 0, longitude: 0 }} + initialLocation={selectedLocation ?? undefined}
- Add a defensive check in the picker component to treat
{0,0}as “no initial location.”
📝 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.
| initialLocation={selectedLocation || { latitude: 0, longitude: 0 }} | |
| (src/app/call/new/index.tsx @ line 1275) | |
| initialLocation={selectedLocation ?? undefined} |
🤖 Prompt for AI Agents
In src/app/call/new/index.tsx around line 1275 and src/app/call/[id]/edit.tsx
around line 811, the initialLocation fallback currently uses the literal {
latitude: 0, longitude: 0}; change those call sites to pass undefined instead of
that zero-coordinates object. Then update the picker component (the component
that receives initialLocation) to defensively treat an object with latitude===0
and longitude===0 as equivalent to undefined (i.e., treat {0,0} as “no initial
location”) so it initializes the picker with no selection when given that
sentinel value.
| const response = (await getCalendarItemsForDateRange(today.toISOString(), today.toISOString())) as ApiResponse<CalendarItemResultData[]>; | ||
|
|
||
| // Filter items to ensure they're really for today (additional client-side validation) | ||
| // Use Start field for date comparison as it contains the timezone-aware date from .NET backend | ||
| const todayItems = response.Data.filter((item) => { | ||
| const todayItems = response.Data.filter((item: CalendarItemResultData) => { | ||
| return isSameDate(item.Start, new Date()); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Bug: querying “today” with identical ISO timestamps likely returns no items
Passing today.toISOString() for both start and end narrows the range to an instant. Use start/end of day, consistent with other calls.
- const response = (await getCalendarItemsForDateRange(today.toISOString(), today.toISOString())) as ApiResponse<CalendarItemResultData[]>;
+ const startDate = format(startOfDay(today), 'yyyy-MM-dd HH:mm:ss');
+ const endDate = format(endOfDay(today), 'yyyy-MM-dd HH:mm:ss');
+ const response = (await getCalendarItemsForDateRange(startDate, endDate)) as ApiResponse<CalendarItemResultData[]>;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/stores/calendar/store.ts around lines 109 to 116, the code passes
today.toISOString() for both start and end which queries a single instant;
change to compute a startOfDay (00:00:00.000) and endOfDay (23:59:59.999) for
the current date in the same timezone and pass startOfDay.toISOString() and
endOfDay.toISOString() to getCalendarItemsForDateRange; keep the additional
client-side isSameDate filtering but ensure the date-range covers the whole day
(you can use existing utility like startOfDay/endOfDay or construct new Date
objects accordingly).
| const startDate = subDays(new Date(), 90).toISOString().split('T')[0]!; | ||
| const endDate = addDays(new Date(), 120).toISOString().split('T')[0]!; | ||
|
|
||
| const response = await getCalendarItemsForDateRange(startDate, endDate); | ||
| const response = (await getCalendarItemsForDateRange(startDate, endDate)) as ApiResponse<CalendarItemResultData[]>; | ||
| set({ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Normalize date-range formatting for extended loads
Using plain YYYY-MM-DD here vs 'yyyy-MM-dd HH:mm:ss' elsewhere is inconsistent and can cause backend parsing issues. Align to start/end of day.
- const startDate = subDays(new Date(), 90).toISOString().split('T')[0]!;
- const endDate = addDays(new Date(), 120).toISOString().split('T')[0]!;
+ const startDate = format(startOfDay(subDays(new Date(), 90)), 'yyyy-MM-dd HH:mm:ss');
+ const endDate = format(endOfDay(addDays(new Date(), 120)), 'yyyy-MM-dd HH:mm:ss');📝 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.
| const startDate = subDays(new Date(), 90).toISOString().split('T')[0]!; | |
| const endDate = addDays(new Date(), 120).toISOString().split('T')[0]!; | |
| const response = await getCalendarItemsForDateRange(startDate, endDate); | |
| const response = (await getCalendarItemsForDateRange(startDate, endDate)) as ApiResponse<CalendarItemResultData[]>; | |
| set({ | |
| const startDate = format(startOfDay(subDays(new Date(), 90)), 'yyyy-MM-dd HH:mm:ss'); | |
| const endDate = format(endOfDay(addDays(new Date(), 120)), 'yyyy-MM-dd HH:mm:ss'); | |
| const response = (await getCalendarItemsForDateRange(startDate, endDate)) as ApiResponse<CalendarItemResultData[]>; | |
| set({ |
🤖 Prompt for AI Agents
In src/stores/calendar/store.ts around lines 176 to 180 the date strings are
generated as plain YYYY-MM-DD which is inconsistent with the rest of the code
and can break backend parsing; change to produce start-of-day and end-of-day
timestamps in the 'yyyy-MM-dd HH:mm:ss' format by using date-fns
startOfDay/endOfDay and format(startOfDay(...), 'yyyy-MM-dd HH:mm:ss') and
format(endOfDay(...), 'yyyy-MM-dd HH:mm:ss'), then pass those formatted strings
into getCalendarItemsForDateRange.
| expect(state.queuedEvents[0]?.status).toBe(QueuedEventStatus.PENDING); | ||
| expect(state.queuedEvents[0]?.error).toBeUndefined(); | ||
| expect(state.queuedEvents[0]?.nextRetryAt).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
Bug: wrong eventId used in retryEvent test (will not match added event)
generateEventId is mocked to return test-event-id-1, -2, ... but the test uses 'test-event-id'. This prevents the update from targeting the created event and weakens assertions.
- store.addEvent(QueuedEventType.UNIT_STATUS, { test: 'data' });
- store.updateEventStatus('test-event-id', QueuedEventStatus.FAILED);
+ const eventId = store.addEvent(QueuedEventType.UNIT_STATUS, { test: 'data' });
+ store.updateEventStatus(eventId, QueuedEventStatus.FAILED);
...
- store.retryEvent('test-event-id');
+ store.retryEvent(eventId);📝 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.
| expect(state.queuedEvents[0]?.status).toBe(QueuedEventStatus.PENDING); | |
| expect(state.queuedEvents[0]?.error).toBeUndefined(); | |
| expect(state.queuedEvents[0]?.nextRetryAt).toBeUndefined(); | |
| }); | |
| // Capture the generated event ID so subsequent calls target the correct event | |
| const eventId = store.addEvent(QueuedEventType.UNIT_STATUS, { test: 'data' }); | |
| store.updateEventStatus(eventId, QueuedEventStatus.FAILED); | |
| store.retryEvent(eventId); | |
| const state = store.getState(); | |
| expect(state.queuedEvents[0]?.status).toBe(QueuedEventStatus.PENDING); | |
| expect(state.queuedEvents[0]?.error).toBeUndefined(); | |
| expect(state.queuedEvents[0]?.nextRetryAt).toBeUndefined(); | |
| }); |
🤖 Prompt for AI Agents
In src/stores/offline-queue/__tests__/store.test.ts around lines 346 to 349, the
retryEvent test is using the wrong eventId ('test-event-id') while
generateEventId is mocked to produce sequential ids like 'test-event-id-1',
'test-event-id-2', etc.; update the test to reference the actual id used when
creating the event (for example 'test-event-id-1') or capture and reuse the id
returned/assigned when adding the event so the retry/update targets the correct
queued event and assertions validate the intended item.
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/components/ui/actionsheet/index.tsx (1)
437-452: Unreachable branch in ActionsheetIcon (size check blocks height/width path)Defaulting size='sm' means the height/width-only path never executes. Honor explicit height/width regardless of size, or default size to undefined.
-const ActionsheetIcon = React.forwardRef<React.ElementRef<typeof UIActionsheet.Icon>, IActionsheetIconProps>(({ className, size = 'sm', ...props }, ref) => { +const ActionsheetIcon = React.forwardRef<React.ElementRef<typeof UIActionsheet.Icon>, IActionsheetIconProps>(({ className, size, ...props }, ref) => { if (typeof size === 'number') { return <UIActionsheet.Icon ref={ref} {...props} className={actionsheetIconStyle({ class: className })} size={size} />; - } else if ((props.height !== undefined || props.width !== undefined) && size === undefined) { + } else if (props.height !== undefined || props.width !== undefined) { return <UIActionsheet.Icon ref={ref} {...props} className={actionsheetIconStyle({ class: className })} />; } return ( <UIActionsheet.Icon className={actionsheetIconStyle({ - class: className, - size, + class: className, + size: size ?? 'sm', })} ref={ref} {...props} /> ); });src/components/ui/bottomsheet/index.tsx (1)
129-131: tva param key should be “class”, not “className” (styles currently dropped)tva expects
{ class: ... }. UsingclassNameprevents styles from applying.- className={bottomSheetTriggerStyle({ - className: className, - })} + className={bottomSheetTriggerStyle({ class: className })}- className={bottomSheetIndicatorStyle({ - className: className, - })} + className={bottomSheetIndicatorStyle({ class: className })}- className={bottomSheetContentStyle({ - className: props.className, - })} + className={bottomSheetContentStyle({ class: props.className })}- className={bottomSheetContentStyle({ - className: props.className, - })} + className={bottomSheetContentStyle({ class: props.className })}- className={bottomSheetItemStyle({ - className: className, - })} + className={bottomSheetItemStyle({ class: className })}- className={bottomSheetBackdropStyle({ - className: className, - })} + className={bottomSheetBackdropStyle({ class: className })}Also applies to: 161-164, 197-199, 214-216, 236-239, 142-145
src/stores/security/store.ts (1)
68-75: Fix possible crash when Groups is undefined; coerce selectors to booleans
store.rightscan exist whileGroupsisundefined(plain JSON without class defaults), causing.someto throw. Also, selectors should consistently return booleans.- isUserDepartmentAdmin: store.rights?.IsAdmin, - isUserGroupAdmin: (groupId: number) => store.rights?.Groups.some((right) => right.GroupId === groupId && right.IsGroupAdmin), - canUserCreateCalls: store.rights?.CanCreateCalls, - canUserCreateNotes: store.rights?.CanAddNote, - canUserCreateMessages: store.rights?.CanCreateMessage, - canUserViewPII: store.rights?.CanViewPII, + isUserDepartmentAdmin: !!store.rights?.IsAdmin, + isUserGroupAdmin: (groupId: number) => + !!store.rights?.Groups?.some((right) => right.GroupId === groupId && right.IsGroupAdmin), + canUserCreateCalls: !!store.rights?.CanCreateCalls, + canUserCreateNotes: !!store.rights?.CanAddNote, + canUserCreateMessages: !!store.rights?.CanCreateMessage, + canUserViewPII: !!store.rights?.CanViewPII,src/app/__tests__/onboarding.test.tsx (1)
262-294: Remove analytics fallbacks; add a testID to the Next button and press it.Manual analytics calls hide regressions. Use a stable test target.
Test:
-// Find and press the Next button -const nextButton = getByText('Next '); -fireEvent.press(nextButton); +const nextButton = getByTestId('next-button'); +fireEvent.press(nextButton); ... -} catch (error) { - // If direct button press fails, simulate the analytics call - mockTrackEvent('onboarding_next_clicked', { - timestamp: new Date().toISOString(), - currentSlide: 0, - slideTitle: 'Resgrid Responder', - }); -} +} catch (error) { throw error; }Component (src/app/onboarding.tsx):
-<Button size="lg" variant="solid" action="primary" className="bg-primary-500 px-6" onPress={nextSlide}> +<Button size="lg" variant="solid" action="primary" className="bg-primary-500 px-6" onPress={nextSlide} testID="next-button">Also applies to: 410-442
src/services/bluetooth-audio.service.ts (1)
567-630: Handle real-worldserviceDatashapes (bytes/data).
react-native-ble-managerexposes serviceData values as objects withbytesanddata(Base64). Current code skips non-strings. Parse both to improve detection.- if (typeof serviceData === 'object' && serviceData !== null) { - // Service data is an object with service UUIDs as keys and data as values - return Object.entries(serviceData).some(([serviceUuid, data]) => { - if (typeof data !== 'string') { - return false; // Skip non-string data - } + if (typeof serviceData === 'object' && serviceData !== null) { + // Object with service UUIDs as keys; values may be strings or { bytes, data } objects + return Object.entries(serviceData).some(([serviceUuid, val]) => { + let payload: string | null = null; + if (typeof val === 'string') { + payload = val; + } else if (val && typeof val === 'object') { + // Prefer base64 if present, else hex from bytes + if (typeof (val as any).data === 'string') { + payload = (val as any).data as string; // base64 + } else if (Array.isArray((val as any).bytes)) { + payload = Buffer.from((val as any).bytes as number[]).toString('hex'); + } + } + if (!payload) return false; const normalizedServiceUuid = this.normalizeUuid(serviceUuid); @@ - if (isAudioServiceUuid) { + if (isAudioServiceUuid) { logger.debug({ message: 'Found audio service UUID in service data', context: { serviceUuid: normalizedServiceUuid, - data: data, + data: payload, }, }); return true; } // Analyze the service data content for audio indicators - if (typeof data === 'string') { - const decodedData = this.decodeServiceDataString(data); - return this.analyzeServiceDataForAudio(decodedData); - } + const decodedData = this.decodeServiceDataString(payload); + return this.analyzeServiceDataForAudio(decodedData); return false; }); }References: library docs show
serviceDataexposes rawbytesanddata(Base64). (innoveit.github.io)
♻️ Duplicate comments (7)
src/components/ui/accordion/index.tsx (1)
107-107: Thanks for honoring size fallback in Svg path—this addresses prior feedbackThe non-AsComp path now respects size with a sensible 24 default. Looks good.
src/components/ui/actionsheet/index.tsx (1)
49-56: Resolved: DragIndicatorWrapper key mismatch fixed; Item wired via cssInteropFactory now uses DragIndicatorWrapper (not IndicatorWrapper) and wraps Item with cssInterop(Pressable,…). This unblocks NativeWind styling and matches usages. Nice.
src/app/call/[id].tsx (1)
39-39: Safer route param handling implementedKeeping callId optional and guarding callers prevents /call/undefined routes. Good.
src/components/ui/bottomsheet/index.tsx (1)
2-2: Wrong ref type import; use component instance type (or methods interface)Current
import type BottomSheetRef from '@gorhom/bottom-sheet'is the component type, not the ref methods. Use the instance type to get.snapToIndex/.closetyping.-import type BottomSheetRef from '@gorhom/bottom-sheet'; +// Use the instance type of the default export as the ref type +type BottomSheetRef = React.ComponentRef<typeof GorhomBottomSheet>;src/app/call/new/index.tsx (1)
1275-1275: Great: switch to?? undefinedforinitialLocation.This removes the
{0,0}fallback at the callsite and aligns with the picker’s guard.src/services/__tests__/bluetooth-audio.service.test.ts (1)
42-62: Good: BleManager mock now includes retrieveServices and startNotification.This unblocks connect/notification paths that previously failed in tests.
src/services/bluetooth-audio.service.ts (1)
58-60: LGTM: unified timer types.Using
ReturnType<typeof setTimeout>is the right cross-env choice for RN/Node.
🧹 Nitpick comments (69)
src/components/home/__tests__/user-staffing-card.test.tsx (2)
8-30: Consolidate and de-duplicate Gluestack mocksThese three mocks are repeated across tests. Prefer moving them to jest-setup (or a shared mock) to keep tests DRY and consistent. If you centralize them, remove this block locally.
Example removal (assuming global mocks exist):
-// Mock Gluestack UI utilities before any UI component imports -jest.mock('@gluestack-ui/nativewind-utils/tva', () => ({ - tva: jest.fn().mockImplementation((config) => { - return jest.fn().mockImplementation((props) => { - const { class: className, ...restProps } = props || {}; - return className || ''; - }); - }), -})); - -jest.mock('@gluestack-ui/nativewind-utils/IsWeb', () => ({ - isWeb: false, -})); - -jest.mock('@gluestack-ui/nativewind-utils', () => ({ - tva: jest.fn().mockImplementation((config) => { - return jest.fn().mockImplementation((props) => { - const { class: className, ...restProps } = props || {}; - return className || ''; - }); - }), - isWeb: false, -}));
10-16: Simplify tva mocks; remove unused varsThe nested jest.fn chains and unused restProps aren’t needed. Keep the mock terse.
- tva: jest.fn().mockImplementation((config) => { - return jest.fn().mockImplementation((props) => { - const { class: className, ...restProps } = props || {}; - return className || ''; - }); - }), + tva: jest.fn(() => (props?: { class?: string }) => props?.class ?? ''),Also applies to: 23-30
src/components/ui/accordion/index.tsx (1)
104-107: Unify defaults and prefer nullish coalescingAlign fill default between AsComp and Svg paths, and use ?? to avoid treating 0 as falsy for dimensions.
- if (AsComp) { - return <AsComp ref={ref} fill={fill} {...props} {...sizeProps} stroke={colorProps} />; - } - return <Svg ref={ref} height={height || size || 24} width={width || size || 24} fill={fill || 'none'} stroke={colorProps} {...props} />; + if (AsComp) { + return <AsComp ref={ref} fill={fill ?? 'none'} {...props} {...sizeProps} stroke={colorProps} />; + } + return ( + <Svg + ref={ref} + height={height ?? size ?? 24} + width={width ?? size ?? 24} + fill={fill ?? 'none'} + stroke={colorProps} + {...props} + /> + );src/app/call/__tests__/coordinate-validation.test.ts (2)
23-24: Use Number.isNaN to avoid coercionPrevents unexpected truthiness if the type widens in future refactors.
- if (isNaN(lat) || isNaN(lng)) { + if (Number.isNaN(lat) || Number.isNaN(lng)) {
6-28: Import the production validator instead of redefining it in testsRedefining logic can hide regressions. If an app helper exists, import and test that directly; otherwise consider extracting this to a shared util.
src/components/home/__tests__/user-status-card.test.tsx (2)
8-30: Centralize Gluestack mocks; keep tests leanSame duplication as the staffing-card test. Move these mocks to jest-setup (or a shared mock) and remove locally.
-// Mock Gluestack UI utilities before any UI component imports -jes… (same block as in staffing test) +// Gluestack UI mocks provided by jest-setup
10-16: Tighten tva mocksShorten and drop unused variables for clarity.
- tva: jest.fn().mockImplementation((config) => { - return jest.fn().mockImplementation((props) => { - const { class: className, ...restProps } = props || {}; - return className || ''; - }); - }), + tva: jest.fn(() => (props?: { class?: string }) => props?.class ?? ''),Also applies to: 23-30
src/stores/auth/__tests__/jwt-payload-decode.test.ts (2)
40-51: Strengthen padding tests; assert outcome where deterministicSplit into two cases: one where output is known, and one where we only assert non-throw for a truncated input.
- it('should add correct padding for strings that need it', () => { - // Test strings with different padding requirements - const testCases = [ - { input: 'eyJ0ZXN0IjoidmFsdWUifQ', expected: '{"test":"value"}' }, // needs 2 padding chars - { input: 'eyJ0ZXN0IjoidmFsdWUi', expected: '{"test":"value"}' }, // needs 1 padding char (this would be invalid JSON but tests padding) - ]; - - testCases.forEach(({ input, expected }) => { - // For the second case, we expect it to add padding but the decode might not produce valid JSON - // The important thing is that it doesn't throw an error during the padding step - expect(() => decodeJwtPayload(input)).not.toThrow(); - }); - }); + it('should decode correctly when 2 padding chars are required', () => { + const input = 'eyJ0ZXN0IjoidmFsdWUifQ'; // => {"test":"value"} + expect(decodeJwtPayload(input)).toBe('{"test":"value"}'); + }); + + it('should not throw when input is missing 1 padding char', () => { + const input = 'eyJ0ZXN0IjoidmFsdWUi'; + expect(() => decodeJwtPayload(input)).not.toThrow(); + });
3-15: Avoid duplicating implementation under testImport decodeJwtPayload from the production module (or extract it to a shared util) so tests exercise the real code.
src/components/ui/actionsheet/index.tsx (2)
43-45: Remove dead ItemWrapperItemWrapper is no longer used after wiring Item directly via cssInterop. Delete to avoid dead code.
-const ItemWrapper = React.forwardRef<React.ElementRef<typeof Pressable>, PressableProps>(({ ...props }, ref) => { - return <Pressable {...props} ref={ref} />; -});
2-2: Drop unused importsH4 and Platform are unused. Remove them to keep the module clean.
-import { H4 } from '@expo/html-elements'; @@ -import { FlatList, Platform, Pressable, type PressableProps, ScrollView, SectionList, Text, View, VirtualizedList } from 'react-native'; +import { FlatList, Pressable, type PressableProps, ScrollView, SectionList, Text, View, VirtualizedList } from 'react-native';Also applies to: 11-11
src/app/call/[id].tsx (2)
199-216: Treat 0,0 as invalid to avoid “Null Island” (optional)If upstream ever returns 0,0, routing will open the Gulf of Guinea. Consider rejecting 0,0 here.
const isValidCoordinates = (lat: number | null | undefined, lng: number | null | undefined): boolean => { @@ - // Check if coordinates are within valid ranges + // Reject Null Island if desired + if (lat === 0 && lng === 0) return false; + // Check if coordinates are within valid ranges if (lat < -90 || lat > 90 || lng < -180 || lng > 180) { return false; }
224-229: hasUserLocation truthiness misclassifies 0 latitude/longitudeUse null checks, not truthiness, to avoid treating 0 as “missing”.
- hasUserLocation: !!(userLocation.latitude && userLocation.longitude), + hasUserLocation: userLocation.latitude != null && userLocation.longitude != null,src/components/notifications/__tests__/NotificationInbox.test.tsx (3)
289-308: Avoid DOM querySelector; use testIDs with RNTLUse getByTestId('action-button') or getByA11yLabel instead of parentNode?.querySelector(...).
- const actionButton = getByText('Notifications').parentNode?.querySelector('[data-testid="action-button"]'); + const actionButton = getByTestId('action-button');
210-214: Unneeded fake timersNo timers exercised; drop useFakeTimers/clear to speed tests.
- jest.useFakeTimers(); @@ - afterEach(() => { - jest.clearAllTimers(); - jest.runOnlyPendingTimers(); - jest.useRealTimers(); - jest.clearAllMocks(); - }); + afterEach(() => { + jest.clearAllMocks(); + });Also applies to: 258-263
443-472: This test calls deleteMessage directly, not via UIConsider driving the component (enter selection mode → confirm delete) to verify integration. Current assertion only checks the mock function.
src/components/ui/icon/index.web.tsx (1)
66-66: Narrow parameter typePrefer unknown over any for accessClassName input.
-const accessClassName = (style: any) => { +const accessClassName = (style: unknown) => {src/app/(app)/home/__tests__/calls.test.tsx (1)
185-194: DRY up repeated security store fixtures.Define a shared base mock and spread overrides to reduce duplication.
Apply this diff in each shown block:
- mockUseSecurityStore.mockReturnValue({ - error: null, - canUserCreateCalls: true, - getRights: jest.fn(), - isUserDepartmentAdmin: false, - isUserGroupAdmin: jest.fn(), - canUserCreateNotes: false, - canUserCreateMessages: false, - canUserViewPII: false, - departmentCode: 'TEST', - }); + mockUseSecurityStore.mockReturnValue({ ...baseSecurityMock, canUserCreateCalls: true });And add once near the top of the file (outside the tests):
const baseSecurityMock = { error: null, canUserCreateCalls: false, getRights: jest.fn(), isUserDepartmentAdmin: false, isUserGroupAdmin: jest.fn(), canUserCreateNotes: false, canUserCreateMessages: false, canUserViewPII: false, departmentCode: 'TEST', };Also applies to: 200-209, 218-227, 236-245
__mocks__/@/components/ui/text.tsx (1)
1-6: Prefer RN Text host over DOM span in RN tests.Using react-native Text aligns better with @testing-library/react-native and keeps props semantics consistent.
-// @ts-nocheck -import React from 'react'; - -export function Text(props: any) { - return React.createElement('span', props, props.children); -} +// @ts-nocheck +import React from 'react'; +import { Text as RNText } from 'react-native'; + +export function Text(props: any) { + return React.createElement(RNText, props, props.children); +}src/components/push-notification/__tests__/push-notification-modal.test.tsx (1)
303-305: Avoid brittle getAllByTestId()[0] for icon assertions.Use getByTestId to assert presence without indexing.
- const iconContainer = screen.getAllByTestId('icon-notification-icon')[0]; + const iconContainer = screen.getByTestId('icon-notification-icon');(Apply to each occurrence in these tests.)
Also applies to: 323-325, 343-345, 363-365, 383-385
__mocks__/@/components/ui/pressable.tsx (1)
1-6: Back your mock with RN Pressable rather than a DOM button.Keeps event/testID behavior consistent with RN and avoids DOM-only props.
-// @ts-nocheck -import React from 'react'; - -export function Pressable(props: any) { - return React.createElement('button', { ...props, type: 'button', onClick: props.onPress }, props.children); -} +// @ts-nocheck +import React from 'react'; +import { Pressable as RNPressable } from 'react-native'; + +export function Pressable(props: any) { + return React.createElement(RNPressable, props, props.children); +}__mocks__/@/components/ui/box.tsx (1)
1-6: Use RN View host to match the RN test renderer.This avoids DOM host assumptions in RN tests.
-// @ts-nocheck -import React from 'react'; - -export function Box(props: any) { - return React.createElement('div', props, props.children); -} +// @ts-nocheck +import React from 'react'; +import { View } from 'react-native'; + +export function Box(props: any) { + return React.createElement(View, props, props.children); +}__mocks__/@/components/ui/vstack.tsx (1)
4-6: Map RN testID to data-testid and avoid leaking unknown DOM propsPrevents odd attributes on DOM nodes and keeps getByTestId stable across libs.
-export function VStack(props: any) { - return React.createElement('div', { ...props, style: { display: 'flex', flexDirection: 'column', ...(props.style || {}) } }, props.children); -} +export function VStack({ children, style, testID, ...rest }: any) { + return React.createElement( + 'div', + { + ...rest, + 'data-testid': testID, + style: { display: 'flex', flexDirection: 'column', ...(style || {}) }, + }, + children + ); +}__mocks__/@/components/ui/hstack.tsx (1)
4-6: Mirror VStack changes: testID passthrough and prop sanitizationKeeps mocks consistent and avoids unknown DOM props.
-export function HStack(props: any) { - return React.createElement('div', { ...props, style: { display: 'flex', flexDirection: 'row', ...(props.style || {}) } }, props.children); -} +export function HStack({ children, style, testID, ...rest }: any) { + return React.createElement( + 'div', + { + ...rest, + 'data-testid': testID, + style: { display: 'flex', flexDirection: 'row', ...(style || {}) }, + }, + children + ); +}__mocks__/@/components/ui/avatar.tsx (1)
4-10: Add data-testid passthrough and sanitize propsAligns with other mocks and avoids leaking unknown attributes.
-export function Avatar(props: any) { - return React.createElement('div', { ...props, style: { width: '48px', height: '48px', borderRadius: '50%', ...(props.style || {}) } }, props.children); -} +export function Avatar({ children, style, testID, ...rest }: any) { + return React.createElement( + 'div', + { + ...rest, + 'data-testid': testID, + style: { width: '48px', height: '48px', borderRadius: '50%', ...(style || {}) }, + }, + children + ); +} -export function AvatarImage(props: any) { - return React.createElement('img', { ...props, alt: props.alt || '' }); -} +export function AvatarImage({ alt, testID, ...rest }: any) { + return React.createElement('img', { ...rest, 'data-testid': testID, alt: alt || '' }); +}src/services/__tests__/bluetooth-audio-uuid-normalization.test.ts (3)
78-80: Harden storage mock to avoid incidental imports breaking testsService may call set/remove during init; stub them to be safe.
-jest.mock('@/lib/storage', () => ({ - getItem: jest.fn(), -})); +jest.mock('@/lib/storage', () => ({ + getItem: jest.fn(), + setItem: jest.fn(), + removeItem: jest.fn(), +}));
20-23: Include Linking in RN mock (common dependency for services)Prevents crashes if the service touches Linking.
Alert: { alert: jest.fn(), }, + Linking: { + openURL: jest.fn(), + canOpenURL: jest.fn().mockResolvedValue(true), + },
93-95: Reset jest mocks between testsReduces cross-test leakage and flakiness.
beforeEach(() => { - service = bluetoothAudioService as any; + jest.clearAllMocks(); + service = bluetoothAudioService as any; });__mocks__/react-native-webview.js (1)
5-11: Preserve children in WebView mock to avoid dropping nested contentCurrent implementation doesn't pass through children. Safe to include them so tests that render nested content under WebView keep working.
-const MockWebView = React.forwardRef((props, ref) => { - return React.createElement(View, { - ...props, - ref, - testID: props.testID || 'webview-mock', - }); -}); +const MockWebView = React.forwardRef((props, ref) => { + return React.createElement( + View, + { + ...props, + ref, + testID: props.testID || 'webview-mock', + }, + props.children + ); +});Additionally, set a display name for clearer snapshots/debugging:
MockWebView.displayName = 'MockWebView';src/stores/security/store.ts (1)
25-46: Tighten the API response guard to reduce branches and improve safetyUse a small predicate to assert the shape and collapse the else branches.
Add once (above
getRights):function isApiResponse<T>(v: unknown): v is ApiResponse<T> { return !!v && typeof v === 'object' && 'Data' in (v as Record<string, unknown>); }Then adjust this block:
- // Type guard to ensure response is properly structured - if (response && typeof response === 'object' && 'Data' in response) { - const typedResponse = response as ApiResponse<DepartmentRightsResultData>; - - // Guard against missing or invalid Data - if (typedResponse.Data) { - set({ - rights: typedResponse.Data, - error: null, - }); - } else { - set({ - rights: null, - error: 'Invalid response: missing data', - }); - } - } else { - set({ - rights: null, - error: 'Invalid response format', - }); - } + if (isApiResponse<DepartmentRightsResultData>(response) && response.Data) { + set({ rights: response.Data, error: null }); + } else { + set({ rights: null, error: 'Invalid response: missing data' }); + }src/components/sidebar/__tests__/side-menu.test.tsx (3)
546-556: Avoid shadowing already-declared mocksRedeclaring
mockUseLiveKitStore/mockUseAudioStreamStorehere shadows the top-level constants and adds confusion.- const mockUseLiveKitStore = useLiveKitStore as jest.MockedFunction<typeof useLiveKitStore>; - mockUseLiveKitStore.mockReturnValue({ + mockUseLiveKitStore.mockReturnValue({ isConnected: false, setIsBottomSheetVisible: jest.fn(), toggleMicrophone: jest.fn(), }); - - const mockUseAudioStreamStore = useAudioStreamStore as jest.MockedFunction<typeof useAudioStreamStore>; - mockUseAudioStreamStore.mockReturnValue({ + mockUseAudioStreamStore.mockReturnValue({ currentStream: null, isPlaying: false, setIsBottomSheetVisible: jest.fn(), });
80-157: Centralize forwardRef UI mocks to mocks to reduce duplicationThese inline forwardRef UI mocks mirror ones you already keep under mocks/. Importing those here will keep tests DRY.
542-573: Use fake timers instead of real timeouts to remove flakinessReplace the
waitFortimeout with deterministic timer advances and enable fake timers in thisdescribeblock:beforeAll(() => { jest.useFakeTimers(); }); afterAll(() => { jest.useRealTimers(); }); describe('Navigation Animation Improvements', () => { it('should handle navigation with improved timing to prevent animation stuttering', async () => { const mockOnNavigate = jest.fn(); // Reset all mocks to ensure proper setup const mockUseLiveKitStore = useLiveKitStore as jest.MockedFunction<typeof useLiveKitStore>; mockUseLiveKitStore.mockReturnValue({ isConnected: false, setIsBottomSheetVisible: jest.fn(), toggleMicrophone: jest.fn(), }); const mockUseAudioStreamStore = useAudioStreamStore as jest.MockedFunction<typeof useAudioStreamStore>; mockUseAudioStreamStore.mockReturnValue({ currentStream: null, isPlaying: false, setIsBottomSheetVisible: jest.fn(), }); render(<SideMenu onNavigate={mockOnNavigate} />); const homeMenuItem = screen.getByTestId('side-menu-home'); fireEvent.press(homeMenuItem); - // onNavigate should be called after the navigation and delay - await waitFor( - () => { - expect(mockOnNavigate).toHaveBeenCalled(); - }, - { timeout: 150 } // Increase timeout to account for requestAnimationFrame + 50ms delay - ); + // Advance fake timers to cover requestAnimationFrame + delay + jest.advanceTimersByTime(100); + expect(mockOnNavigate).toHaveBeenCalled(); }); });Verify with:
#!/bin/bash yarn test src/components/sidebar/__tests__/side-menu.test.tsxsrc/components/maps/location-picker.tsx (1)
12-18: Tighten types: remove redundant| undefinedand carryaddressin state.
initialLocation?already impliesundefined; the union is redundant.- Including
address?: stringincurrentLocationkeeps type parity and avoids accidental loss when confirming without moving the map.interface LocationPickerProps { - initialLocation?: - | { - latitude: number; - longitude: number; - address?: string; - } - | undefined; + initialLocation?: { + latitude: number; + longitude: number; + address?: string; + }; onLocationSelected: (location: { latitude: number; longitude: number; address?: string }) => void; height?: number; } @@ - const [currentLocation, setCurrentLocation] = useState<{ - latitude: number; - longitude: number; - } | null>(initialLocation || null); + const [currentLocation, setCurrentLocation] = useState<{ + latitude: number; + longitude: number; + address?: string; + } | null>(initialLocation || null);Also applies to: 27-31
src/components/maps/__tests__/location-picker.test.tsx (1)
4-8: Mock @rnmapbox/maps with forwardRef and reuse shared RN mock
- Wrap MapView and Camera mocks in React.forwardRef to preserve refs
- Remove the inline react-native mock and import the project’s shared mocks/react-native.ts to keep tests consistent
src/components/maps/__tests__/full-screen-location-picker.test.tsx (1)
4-8: Forward-ref the Mapbox mocks for future renders.Make the mocks ref-safe to match component usage.
-jest.mock('@rnmapbox/maps', () => ({ - MapView: () => null, - Camera: () => null, - PointAnnotation: () => null, -})); +jest.mock('@rnmapbox/maps', () => { + const React = require('react'); + const MapView = React.forwardRef((_props: any, _ref: any) => null); + const Camera = React.forwardRef((_props: any, _ref: any) => null); + const PointAnnotation = () => null; + return { MapView, Camera, PointAnnotation }; +});src/stores/calls/store.ts (1)
66-70: Align response typing across all fetchers.
fetchCallPriorities/fetchCallTypesstill rely on inferred types; mirror the explicitApiResponse<T>cast you used elsewhere for consistency and safer access to.Data.fetchCallPriorities: async () => { set({ isLoading: true, error: null }); try { - const response = await getCallPriorities(); + const response = (await getCallPriorities()) as ApiResponse<CallPriorityResultData[]>; set({ callPriorities: response.Data, isLoading: false }); } catch (error) { set({ error: 'Failed to fetch call priorities', isLoading: false }); } }, @@ fetchCallTypes: async () => { // Only fetch if we don't have call types in the store const { callTypes } = get(); if (callTypes.length > 0) { return; } set({ isLoading: true, error: null }); try { - const response = await getCallTypes(); + const response = (await getCallTypes()) as ApiResponse<CallTypeResultData[]>; set({ callTypes: response.Data, isLoading: false }); } catch (error) { set({ error: 'Failed to fetch call types', isLoading: false }); } },Also applies to: 83-85
src/app/call/new/index.tsx (1)
400-421: Remove redundantif (result)guards.After
results.length > 0,results[0]is non-null; the extra checks add noise.- const result = response.data.results[0]; - if (result) { - const newLocation = { - latitude: result.geometry.location.lat, - longitude: result.geometry.location.lng, - address: result.formatted_address, - }; - handleLocationSelected(newLocation); - toast.show({ /* ... */ }); - } + const result = response.data.results[0]; + const newLocation = { + latitude: result.geometry.location.lat, + longitude: result.geometry.location.lng, + address: result.formatted_address, + }; + handleLocationSelected(newLocation); + toast.show({ /* ... */ });Apply similarly in the plus-code and reverse-geocode branches.
Also applies to: 687-708, 858-867
src/components/contacts/__tests__/contact-card.test.tsx (3)
102-112: Cover whitespace-only name inputs.You test empty strings; also add cases for names with only whitespace to validate trimming behavior.
Example:
+it('trims whitespace-only names for Person type', () => { + const p = { ...basePerson, FirstName: ' ', LastName: ' \t ' }; + render(<ContactCard contact={p} onPress={mockOnPress} />); + expect(screen.getByText('John Doe')).toBeTruthy(); // falls back to Name +});Also applies to: 114-124, 126-137, 139-150
186-193: Press the card container, not its text.Firing on Text can pass even if the Pressable isn’t wired. Prefer a testID on the touch target and press that.
If ContactCard exposes a testID on the root Pressable:
- fireEvent.press(screen.getByText('John Doe')); + fireEvent.press(screen.getByTestId('contact-card-1'));If not, add
testID={contact-card-${contact.ContactId}}to the card and update this test.
206-218: Actually assert the “important” star appears.Currently you only assert text. Add a testID to the star icon (e.g., important-icon) in ContactCard and assert presence/absence.
Example:
-// The star icon should be present for important contacts -expect(screen.getByText('Acme Corporation')).toBeTruthy(); +expect(screen.getByTestId('important-icon')).toBeTruthy();src/components/contacts/__tests__/contact-details-sheet.test.tsx (4)
309-324: Make mock store immutable per test.Shared mutable object can leak state between tests. Clone or deep-freeze in beforeEach.
Apply:
-const mockStoreData = { ... }; +const baseStoreData = { ... }; ... -mockUseContactsStore.mockReturnValue(mockStoreData); +mockUseContactsStore.mockReturnValue(JSON.parse(JSON.stringify(baseStoreData)));Also applies to: 334-336
586-592: Replace “renders without crashing” with concrete assertions.Expose stable testIDs for key nodes (title, type label, star icon, tabs) and assert them. Improves signal.
Suggested component testIDs:
- title: contact-details-title
- type label: contact-type-label
- important star: important-icon
- tabs: tab-details, tab-notes
- avatar container: contact-avatar
Also applies to: 603-656
492-497: Unskip tab-change analytics tests.Tabs are Pressables—add testIDs and assert trackEvent calls for from/to.
Component:
-<Pressable onPress={() => handleTabChange('details')} ... +<Pressable testID="tab-details" onPress={() => handleTabChange('details')} ... -<Pressable onPress={() => handleTabChange('notes')} ... +<Pressable testID="tab-notes" onPress={() => handleTabChange('notes')} ...Test:
-it.skip('should track tab change analytics when switching to notes', async () => { }); +it('tracks tab change analytics when switching to notes', async () => { + const { getByTestId } = render(<ContactDetailsSheet />); + fireEvent.press(getByTestId('tab-notes')); + await waitFor(() => + expect(mockTrackEvent).toHaveBeenCalledWith('contact_details_tab_changed', expect.objectContaining({ fromTab: 'details', toTab: 'notes' })) + ); +});
160-191: Return React elements for icon mocks.Strings (‘Icon’) render as text; use a View to avoid accidental text matches.
Apply:
-jest.mock('lucide-react-native', () => ({ - X: jest.fn(() => 'Icon'), - ... -})); +jest.mock('lucide-react-native', () => { + const React = require('react'); + const { View } = require('react-native'); + const Icon = () => React.createElement(View, { accessibilityLabel: 'icon' }); + return { X: Icon, Mail: Icon, Phone: Icon, Home: Icon, Smartphone: Icon, Building: Icon, MapPin: Icon, Clock: Icon, User: Icon, Users: Icon, Calendar: Icon, + BuildingIcon: Icon, CalendarIcon: Icon, ChevronDownIcon: Icon, ChevronRightIcon: Icon, Edit2Icon: Icon, GlobeIcon: Icon, HomeIcon: Icon, MailIcon: Icon, + MapPinIcon: Icon, PhoneIcon: Icon, SettingsIcon: Icon, SmartphoneIcon: Icon, StarIcon: Icon, TrashIcon: Icon, UserIcon: Icon, XIcon: Icon }; +});src/app/__tests__/onboarding.test.tsx (2)
319-340: Wrap scroll-change expectations with waitFor.State updates after scroll aren’t strictly synchronous.
Apply:
-fireEvent.scroll(flatList, { nativeEvent: { contentOffset: { x: 390, y: 0 }, layoutMeasurement: { width: 390, height: 844 }, contentSize: { width: 1170, height: 844 } } }); -expect(mockTrackEvent).toHaveBeenCalledWith('onboarding_slide_changed', { +fireEvent.scroll(flatList, { nativeEvent: { contentOffset: { x: 390, y: 0 }, layoutMeasurement: { width: 390, height: 844 }, contentSize: { width: 1170, height: 844 } } }); +await waitFor(() => expect(mockTrackEvent).toHaveBeenCalledWith('onboarding_slide_changed', { timestamp: expect.any(String), fromSlide: 0, toSlide: 1, slideTitle: 'Instant Notifications', -}); +}));Also applies to: 460-489
195-201: Restore Dimensions spy to avoid cross-test leakage.Minor stability improvement.
Apply:
- beforeEach(() => { + let dimSpy: jest.SpyInstance; + beforeEach(() => { ... - jest.spyOn(Dimensions, 'get').mockReturnValue({ width: 390, height: 844 }); + dimSpy = jest.spyOn(Dimensions, 'get').mockReturnValue({ width: 390, height: 844 }); }); afterEach(() => { // Ensure mock is restored to working state after each test mockTrackEvent.mockImplementation(() => { }); + dimSpy?.mockRestore(); });Also applies to: 203-206
src/stores/calls/detail-store.ts (2)
74-85: Minor: prefer typed helpers over inline casts for Promise.all.
Casting to Promise<ApiResponse<...>> works, but consider creating typed wrappers (e.g., getCall(id)) or typing the API functions to avoid per-call assertions.
150-159: After upload, await refresh and reflect loading state.
Fire-and-forget refresh can cause stale UI and swallowed errors. Set loading, await refresh, and surface errors to errorImages.Apply:
uploadCallImage: async (callId: string, userId: string, note: string, name: string, latitude: number | null, longitude: number | null, file: string) => { - try { - await saveCallImage(callId, userId, note, name, latitude, longitude, file); - - // After successful upload, refresh the images list - useCallDetailStore.getState().fetchCallImages(callId); - } catch (error) { - console.error('Error uploading image:', error); - throw error; - } + set({ isLoadingImages: true, errorImages: null }); + try { + await saveCallImage(callId, userId, note, name, latitude, longitude, file); + await get().fetchCallImages(callId); + } catch (error) { + set({ isLoadingImages: false, errorImages: error instanceof Error ? error.message : 'Failed to upload image' }); + throw error; + } },src/services/__tests__/bluetooth-audio-button-controls.test.ts (1)
133-146: Test adds no assertions — either assert or remove.
This block doesn’t validate behavior. Consider asserting known UUID constants exist on the service, or drop it.src/app/login/__tests__/login-form.test.tsx (3)
42-55: Ensure fireEvent.press triggers by passing through onPress prop.
Current Button mock maps onPress to onClick only; expose onPress too so @testing-library/react-native can invoke it reliably.Apply:
- Button: React.forwardRef(({ children, onPress, ...props }: any, ref: any) => - React.createElement('button', { ...props, ref, testID: 'button', onClick: onPress }, children) + Button: React.forwardRef(({ children, onPress, ...props }: any, ref: any) => + React.createElement('button', { ...props, ref, testID: 'button', onClick: onPress, onPress }, children) ),
101-103: Mirror the same onPress pass-through for InputSlot.
Keeps behavior consistent for icon-button presses.Apply:
- InputSlot: React.forwardRef(({ children, onPress, ...props }: any, ref: any) => - React.createElement('button', { ...props, ref, testID: 'input-slot', onClick: onPress }, children) + InputSlot: React.forwardRef(({ children, onPress, ...props }: any, ref: any) => + React.createElement('button', { ...props, ref, testID: 'input-slot', onClick: onPress, onPress }, children) ),
219-222: Avoid brittle index-based button selection.
Target the server URL button by a stable testID.Apply (test):
- const buttons = screen.getAllByTestId('button'); - const serverUrlButton = buttons[1]; // Second button is the server URL button + const serverUrlButton = screen.getByTestId('server-url-button');And update the component (outside this PR file) to tag buttons:
// src/app/login/login-form.tsx <Button testID="submit-button" ...> <Button testID="server-url-button" ...>src/components/maps/full-screen-location-picker.tsx (8)
14-20: Simplify optional prop type; remove redundant| undefined.
initialLocation?already permitsundefined. Keep the type concise.- initialLocation?: - | { - latitude: number; - longitude: number; - address?: string; - } - | undefined; + initialLocation?: { + latitude: number; + longitude: number; + address?: string; + };
36-36: Seed address state from props to avoid flicker and redundant reverse-geocode.Initialize from
initialLocation?.addressso UI shows the known address immediately.- const [address, setAddress] = useState<string | undefined>(undefined); + const [address, setAddress] = useState<string | undefined>(initialLocation?.address);
117-124: Respect provided address on mount.If
initialLocation.addressexists, seed it before kicking off reverse-geocode.- if (initialLocation && !(initialLocation.latitude === 0 && initialLocation.longitude === 0)) { - setCurrentLocation(initialLocation); - reverseGeocode(initialLocation.latitude, initialLocation.longitude); + if (initialLocation && !(initialLocation.latitude === 0 && initialLocation.longitude === 0)) { + setCurrentLocation(initialLocation); + if (initialLocation.address) setAddress(initialLocation.address); + reverseGeocode(initialLocation.latitude, initialLocation.longitude);
131-139: Type the map press event and memoize the handler.Avoid
any; use the RN Mapbox press event type anduseCallbackto prevent unnecessary re-renders.Please confirm the exact event type name in your installed @rnmapbox/maps version (e.g., Mapbox.OnPressEvent).
- const handleMapPress = (event: any) => { + const handleMapPress = React.useCallback((event: Mapbox.OnPressEvent) => { const { coordinates } = event.geometry; const newLocation = { latitude: coordinates[1], longitude: coordinates[0], }; setCurrentLocation(newLocation); reverseGeocode(newLocation.latitude, newLocation.longitude); - }; + }, [reverseGeocode]);
174-176: Localize user-facing title.Wrap “Selected Location” with
t()per guidelines.- <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')}>
37-37: Minor: startisMountedRefas false and set true in effect.More conventional and avoids accidental early truthy checks.
- const isMountedRef = useRef(true); + const isMountedRef = useRef(false);
171-187: Optional: memoize static callbacks/props to Mapbox.Consider
useMemo/useCallbackfor handlers passed to<MapView>to reduce re-renders; current setup is fine if perf is acceptable.
162-165: Lint warnings (indentation) from CI.There are minor spacing warnings. Run your formatter to satisfy the check.
pnpm lint --fix && pnpm formatAlso applies to: 181-186, 190-213
src/services/__tests__/bluetooth-audio.service.test.ts (2)
19-41: Stabilize RN mocks across tests.You mutate
RN.PermissionsAndroidinline. It works, but consider centralizing in jest-setup to avoid cross-test leakage and keep mocks hoisted.-const RN = require('react-native'); -// Add PermissionsAndroid... -RN.PermissionsAndroid = { ... }; +// In jest-setup.ts (preferred): +jest.mock('react-native', () => { + const actual = jest.requireActual('react-native'); + return { + ...actual, + PermissionsAndroid: { + ...actual.PermissionsAndroid, + PERMISSIONS: { BLUETOOTH_SCAN: 'android.permission.BLUETOOTH_SCAN', BLUETOOTH_CONNECT: 'android.permission.BLUETOOTH_CONNECT' }, + RESULTS: { GRANTED: 'granted', DENIED: 'denied' }, + requestMultiple: jest.fn(), + }, + DeviceEventEmitter: { addListener: jest.fn() }, + }; +});Also applies to: 126-144
171-176: Add assertions for connect flow side-effects.Assert that
retrieveServicesand at least onestartNotificationare called duringconnectToDevice.@@ it('should have required methods for Bluetooth management', () => { @@ }); + + it('connectToDevice should retrieve services and start notifications', async () => { + const deviceId = 'dev-1'; + // Arrange connected peripherals + mockGetConnectedPeripherals.mockResolvedValue([{ id: deviceId, name: 'Test', rssi: -45, advertising: { isConnectable: true } }]); + // retrieveServices returns empty lists (shape only) + mockRetrieveServices.mockResolvedValue({ services: [], characteristics: [] }); + // Act + await (bluetoothAudioService as any).connectToDevice(deviceId); + // Assert + expect(mockRetrieveServices).toHaveBeenCalledWith(deviceId); + expect(mockStartNotification).toHaveBeenCalled(); + });Also applies to: 188-206, 228-246, 248-272
src/services/bluetooth-audio.service.ts (4)
510-519: Avoid duplicate audio UUIDs and log normalized values only.
AUDIO_SERVICE_UUIDandA2DP_SOURCE_UUIDare identical; dedupe to reduce noise.- const expectedAudioUuids = [AUDIO_SERVICE_UUID, HFP_SERVICE_UUID, HSP_SERVICE_UUID, AINA_HEADSET_SERVICE, B01INRICO_HEADSET_SERVICE, HYS_HEADSET_SERVICE].map((uuid) => uuid.toUpperCase()); + const expectedAudioUuids = Array.from( + new Set([HFP_SERVICE_UUID, HSP_SERVICE_UUID, AINA_HEADSET_SERVICE, B01INRICO_HEADSET_SERVICE, HYS_HEADSET_SERVICE, A2DP_SOURCE_UUID]) + ).map((uuid) => uuid.toUpperCase());Also applies to: 536-539
1029-1125: Reduce noisy subscription attempts; dedupe and filter first.Build a unique set of service/characteristic pairs and try discovered notify/indicate characteristics first to minimize expected failures/noisy logs.
- // Add generic button control service/characteristic combinations - for (const serviceUuid of BUTTON_CONTROL_SERVICES) { - for (const characteristicUuid of BUTTON_CONTROL_CHARACTERISTICS) { - buttonControlConfigs.push({ - service: serviceUuid, - characteristic: characteristicUuid, - }); - } - } + // Add generic combinations (will be attempted after discovered ones) + const staticPairs = BUTTON_CONTROL_SERVICES.flatMap((s) => + BUTTON_CONTROL_CHARACTERISTICS.map((c) => ({ service: s, characteristic: c })), + ); + // Prefer discovered pairs first + const unique = new Map<string, { service: string; characteristic: string }>(); + for (const cfg of buttonControlConfigs) unique.set(`${cfg.service}|${cfg.characteristic}`, cfg); + for (const cfg of staticPairs) { + const key = `${cfg.service}|${cfg.characteristic}`; + if (!unique.has(key)) unique.set(key, cfg); + } + const orderedConfigs = Array.from(unique.values()); @@ - let successCount = 0; - for (const config of buttonControlConfigs) { + let successCount = 0; + for (const config of orderedConfigs) { @@ - message: 'Button event monitoring setup completed', + message: 'Button event monitoring setup completed', context: { deviceId, - totalAttempted: buttonControlConfigs.length, + totalAttempted: orderedConfigs.length, successfulSubscriptions: successCount, }, });
265-272: RSSI gate might be too strict.A fixed -60 dBm could miss valid devices in real environments. Consider -70/-75 or making it configurable via store/settings.
1555-1568: Variable naming:currentMuteStateis inverted; can confuse.It represents “next enabled state,” not current. Consider renaming and set explicitly to
enabled.- const liveKitStore = useLiveKitStore.getState(); - if (liveKitStore.currentRoom) { - const currentMuteState = !liveKitStore.currentRoom.localParticipant.isMicrophoneEnabled; + const liveKitStore = useLiveKitStore.getState(); + if (liveKitStore.currentRoom) { + const nextEnabled = !liveKitStore.currentRoom.localParticipant.isMicrophoneEnabled; @@ - await liveKitStore.currentRoom.localParticipant.setMicrophoneEnabled(currentMuteState); + await liveKitStore.currentRoom.localParticipant.setMicrophoneEnabled(nextEnabled); @@ - action: currentMuteState ? 'unmute' : 'mute', + action: nextEnabled ? 'unmute' : 'mute',And similarly in
setMicrophoneEnabled(enabled: boolean):- const currentMuteState = !liveKitStore.currentRoom.localParticipant.isMicrophoneEnabled; + const isCurrentlyEnabled = liveKitStore.currentRoom.localParticipant.isMicrophoneEnabled; @@ - if (enabled && !currentMuteState) return; // already enabled - if (!enabled && currentMuteState) return; // already disabled - - await liveKitStore.currentRoom.localParticipant.setMicrophoneEnabled(currentMuteState); + if (enabled && isCurrentlyEnabled) return; // already enabled + if (!enabled && !isCurrentlyEnabled) return; // already disabled + await liveKitStore.currentRoom.localParticipant.setMicrophoneEnabled(enabled);Also applies to: 1611-1647
__mocks__/lucide-react-native.ts (1)
5-7: Tighten types and add a default export for interop resilienceAvoid
anyin mocks and expose a default to sidestep occasional ESM/CJS interop hiccups with wildcard imports.-const mockIcon = React.forwardRef((props: any, ref: any) => { - return React.createElement(View, { ...props, ref, testID: `icon-${props.testID || 'mock'}` }); -}); +import type { ViewProps } from 'react-native'; +import type { Ref } from 'react'; + +const mockIcon = React.forwardRef((props: ViewProps & { testID?: string }, ref: Ref<any>) => { + return React.createElement(View, { ...props, ref, testID: `icon-${props.testID || 'mock'}` }); +}); +// Aid debugging in snapshots/devtools +(mockIcon as any).displayName = 'IconMock'; + +// Optional default to support `import Icons from 'lucide-react-native'` in tests +export default {};Also applies to: 9-44
jest-setup.ts (1)
636-639: Expose __esModule for react-native-webview mockImproves compatibility with both default and named import styles.
return { + __esModule: true, WebView, default: WebView, };
| export const AlertCircle = mockIcon; | ||
| export const Bell = mockIcon; | ||
| export const BuildingIcon = mockIcon; | ||
| export const CalendarIcon = mockIcon; | ||
| export const CheckCircle = mockIcon; | ||
| export const ChevronDownIcon = mockIcon; | ||
| export const ChevronRightIcon = mockIcon; | ||
| export const ChevronRight = mockIcon; | ||
| export const Circle = mockIcon; | ||
| export const ClockIcon = mockIcon; | ||
| export const Edit2Icon = mockIcon; | ||
| export const ExternalLink = mockIcon; | ||
| export const FileTextIcon = mockIcon; | ||
| export const GlobeIcon = mockIcon; | ||
| export const HomeIcon = mockIcon; | ||
| export const ImageIcon = mockIcon; | ||
| export const InfoIcon = mockIcon; | ||
| export const MailIcon = mockIcon; | ||
| export const MapPinIcon = mockIcon; | ||
| export const MessageCircle = mockIcon; | ||
| export const MoreVertical = mockIcon; | ||
| export const PaperclipIcon = mockIcon; | ||
| export const Phone = mockIcon; | ||
| export const PhoneIcon = mockIcon; | ||
| export const RouteIcon = mockIcon; | ||
| export const SettingsIcon = mockIcon; | ||
| export const SmartphoneIcon = mockIcon; | ||
| export const StarIcon = mockIcon; | ||
| export const TrashIcon = mockIcon; | ||
| export const Trash2 = mockIcon; | ||
| export const User = mockIcon; | ||
| export const UserCheck = mockIcon; | ||
| export const UserIcon = mockIcon; | ||
| export const Users = mockIcon; | ||
| export const UsersIcon = mockIcon; | ||
| export const X = mockIcon; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify all icon names used in app code are covered by this mock
Gaps will surface as “is not a function/undefined” at render-time in tests.
🏁 Script executed:
#!/bin/bash
# List names imported from lucide-react-native across the repo
rg -nP --type=ts --type=tsx -o "\{\K[^}]+(?=}\s+from\s+['\"]lucide-react-native['\"])" \
| tr ',' '\n' | sed 's/as.*$//' | awk '{$1=$1};1' | sort -u > /tmp/icons_imported.txt
# List names exported by the mock
rg -nPo '(?<=export const )\w+' __mocks__/lucide-react-native.ts | sort -u > /tmp/icons_mocked.txt
echo "Missing in mock:"
comm -23 /tmp/icons_imported.txt /tmp/icons_mocked.txt || trueLength of output: 355
🏁 Script executed:
#!/bin/bash
# Find all unique icon imports from lucide-react-native in .ts and .tsx files
rg -nPo "\{\K[^}]+(?=}\s+from\s+['\"]lucide-react-native['\"])" --glob "*.ts" --glob "*.tsx" \
| tr ',' '\n' \
| sed 's/as.*$//' \
| awk '{$1=$1};1' \
| sort -u > /tmp/icons_imported.txt
# List names exported by the mock
rg -nPo '(?<=export const )\w+' __mocks__/lucide-react-native.ts \
| sort -u > /tmp/icons_mocked.txt
echo "Missing in mock:"
comm -23 /tmp/icons_imported.txt /tmp/icons_mocked.txt || trueLength of output: 5397
Add missing mock exports for all used lucide-react-native icons
mocks/lucide-react-native.ts currently only covers a subset of the icons imported across the app—exports for icons like ArrowRight, Bell, BluetoothConnected, CalendarCheck, Car, Check, Circle, Clock, Contact, FileText, Loader2, MailOpen, Mic, PlusIcon, SearchIcon, XIcon, and many others are absent. Add an export for every icon your import-vs-mock script lists to prevent undefined/function errors in tests.
🤖 Prompt for AI Agents
In __mocks__/lucide-react-native.ts around lines 9 to 44, the mock file only
exports a subset of icons causing undefined/function errors in tests; add
exports for every icon listed by the import-vs-mock script (e.g., ArrowRight,
BluetoothConnected, CalendarCheck, Car, Check, Clock, Contact, FileText,
Loader2, MailOpen, Mic, PlusIcon, SearchIcon, XIcon, and any others the script
reports) using the same mockIcon value and exact export names used in imports so
tests see defined mocks; regenerate or re-run the import-vs-mock script output
to ensure completeness and keep exports in sync with actual imports.
| Dimensions: { | ||
| get: jest.fn().mockReturnValue({ width: 375, height: 667, scale: 2, fontScale: 1 }), | ||
| addEventListener: jest.fn(), | ||
| removeEventListener: jest.fn(), | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Return subscription objects from addEventListener/addListener to match RN API
Several mocks currently return functions; many callers expect { remove() }. This prevents “.remove is not a function” failures.
Dimensions: {
get: jest.fn().mockReturnValue({ width: 375, height: 667, scale: 2, fontScale: 1 }),
- addEventListener: jest.fn(),
+ addEventListener: jest.fn().mockImplementation(() => ({ remove: jest.fn() })),
removeEventListener: jest.fn(),
},
Linking: {
canOpenURL: jest.fn().mockResolvedValue(true),
openURL: jest.fn().mockResolvedValue(undefined),
getInitialURL: jest.fn().mockResolvedValue(null),
- addEventListener: jest.fn(),
+ addEventListener: jest.fn().mockImplementation(() => ({ remove: jest.fn() })),
removeEventListener: jest.fn(),
},
AppState: {
currentState: 'active',
- addEventListener: jest.fn(),
+ addEventListener: jest.fn().mockImplementation(() => ({ remove: jest.fn() })),
removeEventListener: jest.fn(),
},
Keyboard: {
- addListener: jest.fn(),
+ addListener: jest.fn().mockImplementation(() => ({ remove: jest.fn() })),
removeListener: jest.fn(),
removeAllListeners: jest.fn(),
dismiss: jest.fn(),
},
BackHandler: {
- addEventListener: jest.fn(),
+ addEventListener: jest.fn().mockImplementation(() => ({ remove: jest.fn() })),
removeEventListener: jest.fn(),
exitApp: jest.fn(),
},Also applies to: 221-227, 267-271, 274-279, 282-286
| trackEvent('call_images_opened', { | ||
| timestamp: new Date().toISOString(), | ||
| callId: call?.CallId || callId, | ||
| callId: call?.CallId || callId || '', | ||
| imagesCount: call?.ImgagesCount || 0, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Fix typo: ImgagesCount → ImagesCount (breaks image badges/analytics)
The misspelling yields 0 counts and wrong analytics.
- imagesCount: call?.ImgagesCount || 0,
+ imagesCount: call?.ImagesCount || 0,
@@
- imagesCount: call.ImgagesCount || 0,
+ imagesCount: call.ImagesCount || 0,
@@
- {call?.ImgagesCount ? (
+ {call?.ImagesCount ? (
<Box className="absolute -right-1 -top-1 h-5 min-w-[20px] items-center justify-center rounded-full bg-red-500 px-1">
- <Text className="text-xs font-medium text-white">{call.ImgagesCount}</Text>
+ <Text className="text-xs font-medium text-white">{call.ImagesCount}</Text>
</Box>
) : null}Also applies to: 144-147, 632-635
🤖 Prompt for AI Agents
In src/app/call/[id].tsx around lines 88-93 (and also at 144-147, 632-635) the
property ImgagesCount is misspelled, causing image counts to be zero; replace
all uses of call?.ImgagesCount with call?.ImagesCount (and keep fallback 0) so
badges/analytics receive the correct count, then run a quick build/test to
ensure no other references to the typo remain.
| note: data.note || '', | ||
| address: data.address || '', | ||
| latitude: data.latitude || 0, | ||
| longitude: data.longitude || 0, | ||
| what3words: data.what3words || '', | ||
| plusCode: data.plusCode || '', | ||
| dispatchUsers: data.dispatchSelection?.users || [], | ||
| dispatchGroups: data.dispatchSelection?.groups || [], | ||
| dispatchRoles: data.dispatchSelection?.roles || [], | ||
| dispatchUnits: data.dispatchSelection?.units || [], | ||
| dispatchEveryone: data.dispatchSelection?.everyone || false, | ||
| }); |
There was a problem hiding this comment.
Do not serialize missing lat/lng as 0 (Null Island risk).
Defaulting to 0/0 sends a false location and contradicts the new “{0,0} = no location” policy. Prefer nullish coalescing and omit when unset.
- note: data.note || '',
- address: data.address || '',
- latitude: data.latitude || 0,
- longitude: data.longitude || 0,
- what3words: data.what3words || '',
- plusCode: data.plusCode || '',
- dispatchUsers: data.dispatchSelection?.users || [],
- dispatchGroups: data.dispatchSelection?.groups || [],
- dispatchRoles: data.dispatchSelection?.roles || [],
- dispatchUnits: data.dispatchSelection?.units || [],
- dispatchEveryone: data.dispatchSelection?.everyone || false,
+ note: data.note ?? '',
+ address: data.address ?? '',
+ latitude: data.latitude ?? undefined,
+ longitude: data.longitude ?? undefined,
+ what3words: data.what3words ?? '',
+ plusCode: data.plusCode ?? '',
+ dispatchUsers: data.dispatchSelection?.users ?? [],
+ dispatchGroups: data.dispatchSelection?.groups ?? [],
+ dispatchRoles: data.dispatchSelection?.roles ?? [],
+ dispatchUnits: data.dispatchSelection?.units ?? [],
+ dispatchEveryone: data.dispatchSelection?.everyone ?? false,📝 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.
| note: data.note || '', | |
| address: data.address || '', | |
| latitude: data.latitude || 0, | |
| longitude: data.longitude || 0, | |
| what3words: data.what3words || '', | |
| plusCode: data.plusCode || '', | |
| dispatchUsers: data.dispatchSelection?.users || [], | |
| dispatchGroups: data.dispatchSelection?.groups || [], | |
| dispatchRoles: data.dispatchSelection?.roles || [], | |
| dispatchUnits: data.dispatchSelection?.units || [], | |
| dispatchEveryone: data.dispatchSelection?.everyone || false, | |
| }); | |
| note: data.note ?? '', | |
| address: data.address ?? '', | |
| latitude: data.latitude ?? undefined, | |
| longitude: data.longitude ?? undefined, | |
| what3words: data.what3words ?? '', | |
| plusCode: data.plusCode ?? '', | |
| dispatchUsers: data.dispatchSelection?.users ?? [], | |
| dispatchGroups: data.dispatchSelection?.groups ?? [], | |
| dispatchRoles: data.dispatchSelection?.roles ?? [], | |
| dispatchUnits: data.dispatchSelection?.units ?? [], | |
| dispatchEveryone: data.dispatchSelection?.everyone ?? false, | |
| }); |
🤖 Prompt for AI Agents
In src/app/call/new/index.tsx around lines 229 to 240, the code defaults
latitude and longitude to 0 which can send a false location; change this to use
nullish coalescing and avoid serializing absent coordinates — e.g. set latitude:
data.latitude ?? null and longitude: data.longitude ?? null (or omit those keys
entirely when unset) so that {0,0} is not used as a default and empty locations
are represented as null/absent.
| jest.mock('@gluestack-ui/avatar', () => ({ | ||
| createAvatar: (components: any) => { | ||
| const React = require('react'); | ||
| const Avatar: any = React.forwardRef((props: any, ref: any) => { | ||
| return React.createElement('RNAvatar', { ...props, ref }); | ||
| }); | ||
|
|
||
| Avatar.Badge = React.forwardRef((props: any, ref: any) => { | ||
| return React.createElement('RNAvatarBadge', { ...props, ref }); | ||
| }); | ||
|
|
||
| Avatar.Group = React.forwardRef((props: any, ref: any) => { | ||
| return React.createElement('RNAvatarGroup', { ...props, ref }); | ||
| }); | ||
|
|
||
| Avatar.Image = React.forwardRef((props: any, ref: any) => { | ||
| return React.createElement('RNAvatarImage', { ...props, ref }); | ||
| }); | ||
|
|
||
| Avatar.FallbackText = React.forwardRef((props: any, ref: any) => { | ||
| return React.createElement('RNAvatarFallbackText', { ...props, ref }); | ||
| }); | ||
|
|
||
| return Avatar; | ||
| }, | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Prefer mocking the wrapper module '@/components/ui/avatar' instead of the upstream '@gluestack-ui/avatar'.
Keeps tests aligned with app imports and avoids upstream API drift. Also leverages your existing manual mock at mocks/@/components/ui/avatar.tsx.
Apply:
-jes t.mock('@gluestack-ui/avatar', () => ({
- createAvatar: (components: any) => {
- const React = require('react');
- const Avatar: any = React.forwardRef((props: any, ref: any) => {
- return React.createElement('RNAvatar', { ...props, ref });
- });
- Avatar.Badge = React.forwardRef((props: any, ref: any) => {
- return React.createElement('RNAvatarBadge', { ...props, ref });
- });
- Avatar.Group = React.forwardRef((props: any, ref: any) => {
- return React.createElement('RNAvatarGroup', { ...props, ref });
- });
- Avatar.Image = React.forwardRef((props: any, ref: any) => {
- return React.createElement('RNAvatarImage', { ...props, ref });
- });
- Avatar.FallbackText = React.forwardRef((props: any, ref: any) => {
- return React.createElement('RNAvatarFallbackText', { ...props, ref });
- });
- return Avatar;
- },
-}));
+jest.mock('@/components/ui/avatar');📝 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.
| jest.mock('@gluestack-ui/avatar', () => ({ | |
| createAvatar: (components: any) => { | |
| const React = require('react'); | |
| const Avatar: any = React.forwardRef((props: any, ref: any) => { | |
| return React.createElement('RNAvatar', { ...props, ref }); | |
| }); | |
| Avatar.Badge = React.forwardRef((props: any, ref: any) => { | |
| return React.createElement('RNAvatarBadge', { ...props, ref }); | |
| }); | |
| Avatar.Group = React.forwardRef((props: any, ref: any) => { | |
| return React.createElement('RNAvatarGroup', { ...props, ref }); | |
| }); | |
| Avatar.Image = React.forwardRef((props: any, ref: any) => { | |
| return React.createElement('RNAvatarImage', { ...props, ref }); | |
| }); | |
| Avatar.FallbackText = React.forwardRef((props: any, ref: any) => { | |
| return React.createElement('RNAvatarFallbackText', { ...props, ref }); | |
| }); | |
| return Avatar; | |
| }, | |
| })); | |
| // Replace the upstream mock with the local wrapper mock | |
| jest.mock('@/components/ui/avatar'); |
| jest.mock('react-native', () => ({ | ||
| Platform: { OS: 'android' }, | ||
| PermissionsAndroid: { | ||
| PERMISSIONS: { | ||
| BLUETOOTH_SCAN: 'android.permission.BLUETOOTH_SCAN', | ||
| BLUETOOTH_CONNECT: 'android.permission.BLUETOOTH_CONNECT', | ||
| }, | ||
| RESULTS: { | ||
| GRANTED: 'granted', | ||
| DENIED: 'denied', | ||
| }, | ||
| requestMultiple: jest.fn(), | ||
| }, | ||
| DeviceEventEmitter: { | ||
| addListener: jest.fn(), | ||
| }, | ||
| Alert: { | ||
| alert: jest.fn(), | ||
| }, | ||
| })); |
There was a problem hiding this comment.
Duplicate react-native mocks — consolidate to one to avoid override races.
Two jest.mock('react-native') blocks conflict and can drop earlier customizations (PermissionsAndroid, Alert, etc.). Merge into a single mock.
Apply:
-// Mock dependencies
-jest.mock('react-native', () => ({
- Platform: { OS: 'android' },
- PermissionsAndroid: {
- PERMISSIONS: {
- BLUETOOTH_SCAN: 'android.permission.BLUETOOTH_SCAN',
- BLUETOOTH_CONNECT: 'android.permission.BLUETOOTH_CONNECT',
- },
- RESULTS: {
- GRANTED: 'granted',
- DENIED: 'denied',
- },
- requestMultiple: jest.fn(),
- },
- DeviceEventEmitter: {
- addListener: jest.fn(),
- },
- Alert: {
- alert: jest.fn(),
- },
-}));
+// Mock react-native (single consolidated mock)
+jest.mock('react-native', () => {
+ const ReactNative = jest.requireActual('react-native');
+ const React = jest.requireActual('react');
+ return {
+ ...ReactNative,
+ Platform: { OS: 'android' },
+ PermissionsAndroid: {
+ PERMISSIONS: {
+ BLUETOOTH_SCAN: 'android.permission.BLUETOOTH_SCAN',
+ BLUETOOTH_CONNECT: 'android.permission.BLUETOOTH_CONNECT',
+ },
+ RESULTS: { GRANTED: 'granted', DENIED: 'denied' },
+ requestMultiple: jest.fn(),
+ },
+ DeviceEventEmitter: { addListener: jest.fn() },
+ Alert: { alert: jest.fn() },
+ Image: React.forwardRef(({ source, ...props }: any, ref: any) =>
+ React.createElement('img', { ...props, ref, testID: 'image', src: typeof source === 'object' ? source.uri : source })
+ ),
+ Keyboard: { dismiss: jest.fn() },
+ };
+});
-
-// (remove the second react-native mock below)Then remove the later duplicate block entirely.
Also applies to: 116-131
🤖 Prompt for AI Agents
In src/services/__tests__/bluetooth-audio-button-controls.test.ts around lines 4
to 23 (and also remove the duplicate at lines 116-131), there are two
jest.mock('react-native') blocks that conflict; consolidate them into a single
mock at the top of the file that includes all required overrides (Platform.OS,
PermissionsAndroid with PERMISSIONS/RESULTS and requestMultiple as jest.fn(),
DeviceEventEmitter.addListener, Alert.alert, etc.), ensure any additional mocks
from the later block are merged into that single definition, and delete the
later duplicate mock block entirely so earlier customizations are not
overridden.
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
|
|
||
| // Mock dependencies to prevent import errors | ||
| jest.mock('react-native', () => ({ | ||
| Platform: { OS: 'android' }, | ||
| PermissionsAndroid: { | ||
| PERMISSIONS: { | ||
| BLUETOOTH_SCAN: 'android.permission.BLUETOOTH_SCAN', | ||
| BLUETOOTH_CONNECT: 'android.permission.BLUETOOTH_CONNECT', | ||
| }, | ||
| RESULTS: { | ||
| GRANTED: 'granted', | ||
| DENIED: 'denied', | ||
| }, | ||
| requestMultiple: jest.fn(), | ||
| }, | ||
| DeviceEventEmitter: { | ||
| addListener: jest.fn(), | ||
| }, | ||
| Alert: { | ||
| alert: jest.fn(), | ||
| }, | ||
| })); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify mocked shapes match the service’s actual imports
Quick check to ensure we’re not missing NativeModules or other RN/ble-manager members used by the service.
Also applies to: 25-40, 42-61, 62-69, 70-76, 78-86
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect BluetoothAudioService for API surface used from these modules.
rg -nP -C2 "from 'react-native'|from 'react-native-ble-manager'|@/lib/storage|@/lib/logging" src/services/bluetooth-audio.service.ts
# Grep specific RN members the service may use
rg -nP -C2 '\b(NativeModules|Linking|NativeEventEmitter|DeviceEventEmitter|AppState|Platform|PermissionsAndroid)\b' src/services/bluetooth-audio.service.ts
# Confirm methods exist on the service
rg -nP -n "normalizeUuid\s*\(|isAudioDevice\s*\(|supportsMicrophoneControl\s*\(" src/services/bluetooth-audio.service.tsLength of output: 4607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -nP 'BleManager\.\w+' src/services/bluetooth-audio.service.tsLength of output: 1526
Add a Jest mock for react-native-ble-manager in your uuid-normalization tests
You’re currently stubbing only react-native, but BluetoothAudioService imports BleManager and calls methods like start, onDidUpdateState, onDisconnectPeripheral, onDiscoverPeripheral, onDidUpdateValueForCharacteristic, onStopScan, etc. Add
jest.mock('react-native-ble-manager', () => ({
default: {
start: jest.fn(),
onDidUpdateState: jest.fn(),
onDisconnectPeripheral: jest.fn(),
onDiscoverPeripheral: jest.fn(),
onDidUpdateValueForCharacteristic: jest.fn(),
onStopScan: jest.fn(),
checkState: jest.fn(),
scan: jest.fn(),
stopScan: jest.fn(),
connect: jest.fn(),
getConnectedPeripherals: jest.fn(),
retrieveServices: jest.fn(),
getDiscoveredPeripherals: jest.fn(),
startNotification: jest.fn(),
disconnect: jest.fn(),
},
}));to prevent import/runtime errors when loading the service.
🤖 Prompt for AI Agents
In src/services/__tests__/bluetooth-audio-uuid-normalization.test.ts lines 1-23,
the test only mocks react-native but BluetoothAudioService also imports
react-native-ble-manager and calls many of its methods, causing import/runtime
errors; add a Jest mock for 'react-native-ble-manager' that exports a default
object with jest.fn() stubs for methods used by the service (start,
onDidUpdateState, onDisconnectPeripheral, onDiscoverPeripheral,
onDidUpdateValueForCharacteristic, onStopScan, checkState, scan, stopScan,
connect, getConnectedPeripherals, retrieveServices, getDiscoveredPeripherals,
startNotification, disconnect) so tests load without runtime errors.
| const typedCallResult = callResult as ApiResponseWithMessage<CallResultData>; | ||
| const typedExtraResult = callExtraDataResult as ApiResponseWithMessage<CallExtraDataResultData>; | ||
| set({ | ||
| error: callResult.Message || callExtraDataResult.Message || 'Failed to fetch call details', | ||
| error: typedCallResult.Message || typedExtraResult.Message || 'Failed to fetch call details', | ||
| isLoading: false, |
There was a problem hiding this comment.
Bug: possible crash when accessing Message on undefined results.
If either callResult or callExtraDataResult is falsy, typedCallResult.Message will throw. Use optional chaining.
Apply:
- const typedCallResult = callResult as ApiResponseWithMessage<CallResultData>;
- const typedExtraResult = callExtraDataResult as ApiResponseWithMessage<CallExtraDataResultData>;
+ const typedCallResult = callResult as ApiResponseWithMessage<CallResultData> | undefined;
+ const typedExtraResult = callExtraDataResult as ApiResponseWithMessage<CallExtraDataResultData> | undefined;
set({
- error: typedCallResult.Message || typedExtraResult.Message || 'Failed to fetch call details',
+ error: typedCallResult?.Message || typedExtraResult?.Message || 'Failed to fetch call details',
isLoading: false,
});📝 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.
| const typedCallResult = callResult as ApiResponseWithMessage<CallResultData>; | |
| const typedExtraResult = callExtraDataResult as ApiResponseWithMessage<CallExtraDataResultData>; | |
| set({ | |
| error: callResult.Message || callExtraDataResult.Message || 'Failed to fetch call details', | |
| error: typedCallResult.Message || typedExtraResult.Message || 'Failed to fetch call details', | |
| isLoading: false, | |
| const typedCallResult = callResult as ApiResponseWithMessage<CallResultData> | undefined; | |
| const typedExtraResult = callExtraDataResult as ApiResponseWithMessage<CallExtraDataResultData> | undefined; | |
| set({ | |
| error: typedCallResult?.Message || typedExtraResult?.Message || 'Failed to fetch call details', | |
| isLoading: false, | |
| }); |
🤖 Prompt for AI Agents
In src/stores/calls/detail-store.ts around lines 86 to 90, the code casts
callResult/callExtraDataResult to typed objects but then accesses .Message
directly which can throw if either source is falsy; change accesses to use
optional chaining (e.g. typedCallResult?.Message and typedExtraResult?.Message)
or check the original variables for truthiness before reading Message, and keep
the fallback 'Failed to fetch call details' as before so error assignment never
dereferences undefined.
| const callNotes = get().callNotes; | ||
| const trimmedQuery = query.trim(); | ||
| if (!trimmedQuery) return callNotes; | ||
| return callNotes?.filter((note: CallNoteResultData) => note.Note.toLowerCase().includes(trimmedQuery.toLowerCase()) || note.FullName.toLowerCase().includes(trimmedQuery.toLowerCase())); | ||
| return callNotes?.filter((note: CallNoteResultData) => (note.Note?.toLowerCase().includes(trimmedQuery.toLowerCase()) ?? false) || (note.FullName?.toLowerCase().includes(trimmedQuery.toLowerCase()) ?? false)); | ||
| }, |
There was a problem hiding this comment.
Bug: calling includes() on possibly undefined.
note.Note?.toLowerCase() can return undefined; then .includes(...) throws. Chain includes or coalesce to ''.
Apply:
- return callNotes?.filter((note: CallNoteResultData) => (note.Note?.toLowerCase().includes(trimmedQuery.toLowerCase()) ?? false) || (note.FullName?.toLowerCase().includes(trimmedQuery.toLowerCase()) ?? false));
+ const q = trimmedQuery.toLowerCase();
+ return callNotes.filter((note: CallNoteResultData) =>
+ (note.Note?.toLowerCase()?.includes(q) ?? false) ||
+ (note.FullName?.toLowerCase()?.includes(q) ?? false)
+ );📝 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.
| const callNotes = get().callNotes; | |
| const trimmedQuery = query.trim(); | |
| if (!trimmedQuery) return callNotes; | |
| return callNotes?.filter((note: CallNoteResultData) => note.Note.toLowerCase().includes(trimmedQuery.toLowerCase()) || note.FullName.toLowerCase().includes(trimmedQuery.toLowerCase())); | |
| return callNotes?.filter((note: CallNoteResultData) => (note.Note?.toLowerCase().includes(trimmedQuery.toLowerCase()) ?? false) || (note.FullName?.toLowerCase().includes(trimmedQuery.toLowerCase()) ?? false)); | |
| }, | |
| const callNotes = get().callNotes; | |
| const trimmedQuery = query.trim(); | |
| if (!trimmedQuery) return callNotes; | |
| const q = trimmedQuery.toLowerCase(); | |
| return callNotes.filter((note: CallNoteResultData) => | |
| (note.Note?.toLowerCase()?.includes(q) ?? false) || | |
| (note.FullName?.toLowerCase()?.includes(q) ?? false) | |
| ); |
🤖 Prompt for AI Agents
In src/stores/calls/detail-store.ts around lines 129 to 133, the filter calls
note.Note?.toLowerCase().includes(...) and
note.FullName?.toLowerCase().includes(...) which can call includes on undefined;
change these to coalesce the values to an empty string before lowercasing and
calling includes (e.g., use (note.Note ?? '').toLowerCase().includes(...) and
(note.FullName ?? '').toLowerCase().includes(...)) so includes is always invoked
on a string.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/calendar/calendar-card.tsx (1)
29-42: Possible crash: toLocaleTimeString/DateString on invalid dates
new Date(bad).toLocaleTimeString()throwsRangeError. Guard like the compact item does.Apply:
- const formatTime = (dateString: string) => { - return new Date(dateString).toLocaleTimeString([], { - hour: '2-digit', - minute: '2-digit', - }); - }; + const formatTime = (dateString: string) => { + const d = new Date(dateString); + if (isNaN(d.getTime())) return ''; + return d.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); + }; - const formatDate = (dateString: string) => { - return new Date(dateString).toLocaleDateString([], { - weekday: 'short', - month: 'short', - day: 'numeric', - }); - }; + const formatDate = (dateString: string) => { + const d = new Date(dateString); + if (isNaN(d.getTime())) return ''; + return d.toLocaleDateString([], { weekday: 'short', month: 'short', day: 'numeric' }); + };And make
getEventDurationresilient:const getEventDuration = () => { if (item.IsAllDay) { return t('calendar.allDay'); } const start = formatTime(item.Start); const end = formatTime(item.End); - return `${start} - ${end}`; + if (!start || !end) return start || end || ''; + return `${start} - ${end}`; };src/components/maps/location-picker.tsx (1)
65-74: Sanitize initialLocation to avoid Null Island on mount
Initialize currentLocation to null whenever initialLocation is {0,0} (Null Island) so you don’t briefly render 0,0 before the effect runs. Preserve the optional address field in state.// src/components/maps/location-picker.tsx + const isNullIsland = (loc: { latitude: number; longitude: number }) => + loc.latitude === 0 && loc.longitude === 0; - const [currentLocation, setCurrentLocation] = useState<{ - latitude: number; - longitude: number; - } | null>(initialLocation || null); + const [currentLocation, setCurrentLocation] = useState<{ + latitude: number; + longitude: number; + address?: string; + } | null>( + initialLocation && !isNullIsland(initialLocation) + ? initialLocation + : null + ); useEffect(() => { - if (initialLocation && !(initialLocation.latitude === 0 && initialLocation.longitude === 0)) { + if (initialLocation && !isNullIsland(initialLocation)) { setCurrentLocation(initialLocation);Verify no callers intentionally pass {0,0}; if they do, consider adding an
ignoreNullIslandprop to override this guard.src/components/ui/shared-tabs.tsx (1)
11-21: Global zustand store creates cross-instance coupling; switch to local (controlled/uncontrolled) state.
useTabStoreis a singleton; any two SharedTabs with onChange will fight over the sameactiveIndex. Also,initialIndexnever initializes the store. Use local state and a standard controlled prop instead.Apply these diffs to remove the store usage and use local state:
-// Tab state management with zustand -interface TabState { - activeIndex: number; - setActiveIndex: (index: number) => void; -} - -const useTabStore = create<TabState>((set) => ({ - activeIndex: 0, - setActiveIndex: (index) => set({ activeIndex: index }), -})); +// Local state; prefer controlled usage via `activeIndex` prop (see additional snippet below).- const [localActiveIndex, setLocalActiveIndex] = useState(initialIndex); - const { activeIndex, setActiveIndex } = useTabStore(); + const [currentIndex, setCurrentIndex] = useState(initialIndex); const { width, height } = useWindowDimensions(); const isLandscape = width > height;- // Use local state if no external state management is needed - const currentIndex = onChange ? activeIndex : localActiveIndex; + // Local state; fire onChange for listeners- (index: number) => { - if (onChange) { - setActiveIndex(index); - onChange(index); - } else { - setLocalActiveIndex(index); - } - }, - [onChange, setActiveIndex] + (index: number) => { + onChange?.(index); + setCurrentIndex(index); + }, + [onChange] );Additionally, to support a controlled mode, update props and currentIndex derivation:
// Outside the selected lines; add this change to the props and selection logic. interface SharedTabsProps { tabs: TabItem[]; initialIndex?: number; scrollable?: boolean; variant?: 'default' | 'pills' | 'underlined' | 'segmented'; size?: 'sm' | 'md' | 'lg'; className?: string; tabClassName?: string; tabsContainerClassName?: string; contentClassName?: string; onChange?: (index: number) => void; activeIndex?: number; // NEW } // After useState(initialIndex): const controlledIndex = typeof activeIndex === 'number' ? activeIndex : currentIndex;And replace references to
currentIndexin render withcontrolledIndex.Also applies to: 57-65, 66-76
♻️ Duplicate comments (2)
src/components/ui/drawer/index.tsx (1)
33-36: Map className via cssInterop for UIDrawer and all parts; prefer ts-expect-error over ts-ignoreOnly AnimatedPressable and Motion.View are interopped. UIDrawer and its subcomponents also receive className; without mapping, web styles (e.g., pointer-events) won’t apply. Replace ts-ignore with ts-expect-error to avoid masking unrelated errors.
-// @ts-ignore - Motion component type compatibility issue +// @ts-expect-error - Motion component type compatibility issue cssInterop(AnimatedPressable, { className: 'style' }); -// @ts-ignore - Motion component type compatibility issue +// @ts-expect-error - Motion component type compatibility issue cssInterop(Motion.View, { className: 'style' }); +// @ts-expect-error - createModal returns untyped components at runtime +cssInterop(UIDrawer as any, { className: 'style' }); +// @ts-expect-error - Backdrop interop for className on web +cssInterop(UIDrawer.Backdrop as any, { className: 'style' }); +// @ts-expect-error - Content interop for className on web +cssInterop(UIDrawer.Content as any, { className: 'style' }); +// Optional but recommended for consistency: +// @ts-expect-error +cssInterop(UIDrawer.Header as any, { className: 'style' }); +// @ts-expect-error +cssInterop(UIDrawer.Body as any, { className: 'style' }); +// @ts-expect-error +cssInterop(UIDrawer.Footer as any, { className: 'style' }); +// @ts-expect-error +cssInterop(UIDrawer.CloseButton as any, { className: 'style' });src/components/ui/bottomsheet/index.tsx (1)
37-41: Tighten ref typing: drop redundant| nullfromRefObjectgeneric.
RefObject<T>already modelscurrent: T | null. Keep the context type aligned with theuseRef<GorhomBottomSheet>(null)usage.const BottomSheetContext = createContext<{ visible: boolean; - bottomSheetRef: React.RefObject<GorhomBottomSheet | null>; + bottomSheetRef: React.RefObject<GorhomBottomSheet>; handleClose: () => void; handleOpen: () => void; }>({
🧹 Nitpick comments (31)
src/services/__examples__/signalr-typed-invoke-examples.ts (4)
69-79: Optional: fail fast on unsuccessful wrapper responsesConsider throwing on failure to keep call sites simpler; callers can catch once instead of checking success everywhere.
- if (response.success) { - console.log(`Successfully retrieved user: ${response.data.name}`); - console.log(`Response message: ${response.message}`); - } else { - console.warn(`API returned failure: ${response.message}`); - } - - return response; + if (!response.success) { + throw new Error(response.message); + } + console.log(`Successfully retrieved user: ${response.data.name}`); + console.log(`Response message: ${response.message}`); + return response;If useful, you could also model the API as a discriminated union for stronger typing:
type ApiSuccess<T> = { success: true; message: string; data: T; timestamp: string }; type ApiFailure = { success: false; message: string; errorCode?: string; timestamp: string }; type GenericApiResponse<T> = ApiSuccess<T> | ApiFailure;
88-95: Stricter param typing for generic commandsUsing Record<string, unknown> better communicates that params should be an object.
-export async function sendGenericCommand(hubName: string, command: string, params: unknown): Promise<unknown> { +export async function sendGenericCommand( + hubName: string, + command: string, + params: Record<string, unknown> +): Promise<unknown> {
106-110: Void invoke looks correct; consider ack pattern if server supports itIf the server returns an acknowledgment payload, you could type it and validate. Otherwise void is appropriate here.
135-143: Add a minimal runtime guard; prefer for-of for logging loopProtects against unexpected hub payloads and slightly simplifies iteration.
- const units = await signalRService.invoke<Unit[]>(hubName, 'GetActiveUnits', { departmentId }); + const units = await signalRService.invoke<Unit[]>(hubName, 'GetActiveUnits', { departmentId }); + if (!Array.isArray(units)) { + throw new Error('Invalid response: expected Unit[]'); + } @@ - units.forEach((unit) => { - console.log(`Unit ${unit.name} at ${unit.location.address} - Status: ${unit.status}`); - }); + for (const unit of units) { + console.log(`Unit ${unit.name} at ${unit.location.address} - Status: ${unit.status}`); + }src/components/ui/card/index.tsx (1)
15-15: Optionally export the props type for consumersExposing the props eases typing for wrappers/tests.
Apply:
export { Card }; +export type { ICardProps };src/components/calendar/compact-calendar-item.tsx (1)
61-61: Add inner padding after removing CardContentWithout CardContent’s padding, content may touch edges. Add padding at the container level.
Apply:
- <VStack space="xs"> + <VStack space="xs" className="p-3">src/components/calendar/calendar-card.tsx (2)
60-60: Add inner padding after removing CardContentMatch previous spacing to avoid edge-to-edge content.
Apply:
- <VStack space="sm"> + <VStack space="sm" className="p-3">
96-113: Avoid regenerating HTML each render to reduce WebView reloadsMemoize the HTML string.
Example:
// above return const descriptionHtml = React.useMemo( () => generateWebViewHtml({ content: item.Description, isDarkMode, fontSize: 14, lineHeight: 1.4, padding: 6, }), [item.Description, isDarkMode] ); // in WebView source={{ html: descriptionHtml }}src/components/ui/focus-aware-status-bar.tsx (2)
25-27: Confirm intent: nav bar forced hidden on Android regardless of prop.You hide the navigation bar unconditionally; consider tying it to a prop (e.g.,
hideNavigationBar?: boolean) or reusehidden. (npmjs.com)Suggested API tweak:
-type Props = { hidden?: boolean }; +type Props = { hidden?: boolean; hideNavigationBar?: boolean }; @@ -export const FocusAwareStatusBar = ({ hidden = false }: Props) => { +export const FocusAwareStatusBar = ({ hidden = false, hideNavigationBar = hidden }: Props) => { @@ - return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') - ? <SystemBars style="auto" hidden={{ statusBar: hidden, navigationBar: Platform.OS === 'android' ? hidden : false }} /> + return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') + ? <SystemBars style="auto" hidden={{ statusBar: hidden, navigationBar: Platform.OS === 'android' ? hideNavigationBar : false }} /> : null;If you adopt this, please update the unit test accordingly.
39-40: Silent catches hide real issues.At least log in dev to aid diagnostics.
- } catch (error) { - // Silently handle errors if StatusBar methods are not available - } + } catch (error) { + if (__DEV__) console.debug('FocusAwareStatusBar(android) error:', error); + } @@ - } catch (error) { - // Silently handle errors if StatusBar methods are not available - } + } catch (error) { + if (__DEV__) console.debug('FocusAwareStatusBar(ios) error:', error); + }Also applies to: 52-54
src/components/ui/__tests__/focus-aware-status-bar.test.tsx (2)
10-15: Prop shape assertion is fine; consider rendering to assert behavior.Mount with @testing-library/react-native, mock
useIsFocused()to true, and assert<SystemBars>receives expected props.
Example:import { render } from '@testing-library/react-native'; jest.mock('@react-navigation/native', () => ({ useIsFocused: () => true })); it('renders SystemBars with hidden=true', () => { const { UNSAFE_getByType } = render(<FocusAwareStatusBar hidden />); const systemBars = UNSAFE_getByType(require('react-native-edge-to-edge').SystemBars); expect(systemBars.props.hidden).toMatchObject({ statusBar: true }); });
17-22: Default prop behavior is untested.When omitted,
hiddendefaults tofalsein the component. Render and assert thathidden.statusBaris false (not just that the prop is undefined).src/components/ui/drawer/index.tsx (2)
207-209: Let consumers override transition; keep sensible defaultsHard-coding timing/duration prevents callers from tuning motion or disabling it in tests. Merge caller-provided transition with defaults.
- transition={{ - type: 'timing', - duration: 300, - }} + transition={{ + type: (props as any)?.transition?.type ?? 'timing', + duration: (props as any)?.transition?.duration ?? 300, + ...(props as any)?.transition, + }}Optional: gate duration with reduce-motion; happy to add a small hook if desired.
13-14: Use responsive dimensions; current module-scoped snapshot breaks on rotation/splitsDimensions.get at module load won’t update on orientation or split-screen changes. Use useWindowDimensions inside DrawerContent.
-const screenWidth = Dimensions.get('window').width; -const screenHeight = Dimensions.get('window').height;@@ - const drawerHeight = screenHeight * (sizes[parentSize] || sizes.md); - const drawerWidth = screenWidth * (sizes[parentSize] || sizes.md); + const { width: winWidth, height: winHeight } = useWindowDimensions(); + const drawerHeight = winHeight * (sizes[parentSize] || sizes.md); + const drawerWidth = winWidth * (sizes[parentSize] || sizes.md);Additional change outside the selected range:
// at imports import { Dimensions, Pressable, ScrollView, View, useWindowDimensions } from 'react-native';Also applies to: 187-199
types/gluestack-ui.d.ts (2)
14-16: Merge duplicate @gluestack-ui/modal declarations; add precise return type and import React typesTwo module blocks for @gluestack-ui/modal can be merged. Also, typing createModal to return a minimal component map improves editor help and avoids pervasive any.
+import type * as React from 'react'; @@ -declare module '@gluestack-ui/modal' { - export function createModal(config: any): any; -} +declare module '@gluestack-ui/modal' { + export interface IModalProps { + children?: React.ReactNode; + } + export interface ModalComponents { + Root: React.ComponentType<any>; + Backdrop: React.ComponentType<any>; + Content: React.ComponentType<any>; + Body: React.ComponentType<any>; + CloseButton: React.ComponentType<any>; + Footer: React.ComponentType<any>; + Header: React.ComponentType<any>; + AnimatePresence?: React.ComponentType<any> | any; + } + export function createModal(config: any): ModalComponents; +} - -declare module '@gluestack-ui/modal' { - interface IModalProps { - children?: React.ReactNode; - } -}Also applies to: 22-26
6-12: Prefer minimal, specific types over any for factory exportsIf possible, mirror the pattern above for actionsheet/alert-dialog/tooltip (even a { Root: ComponentType } shape is better than any). This preserves IntelliSense without overcommitting to library internals.
-declare module '@gluestack-ui/actionsheet' { - export function createActionsheet(config: any): any; -} +declare module '@gluestack-ui/actionsheet' { + export interface ActionsheetComponents { + Root: React.ComponentType<any>; + } + export function createActionsheet(config: any): ActionsheetComponents; +} @@ -declare module '@gluestack-ui/alert-dialog' { - export function createAlertDialog(config: any): any; -} +declare module '@gluestack-ui/alert-dialog' { + export interface AlertDialogComponents { + Root: React.ComponentType<any>; + } + export function createAlertDialog(config: any): AlertDialogComponents; +} @@ -declare module '@gluestack-ui/tooltip' { - export function createTooltip(config: any): any; -} +declare module '@gluestack-ui/tooltip' { + export interface TooltipComponents { + Root: React.ComponentType<any>; + } + export function createTooltip(config: any): TooltipComponents; +}Also applies to: 34-50
src/components/ui/checkbox/index.tsx (1)
60-60: Approve; optional cleanups
- Strongly type
colorPropsto avoid implicit any:- let colorProps = {}; + let colorProps: Pick<IPrimitiveIcon, 'color' | 'stroke' | 'fill'> = {};
- Unify react-native-svg imports (default vs named) across UI components (default in
src/components/ui/index.tsx, named in others) for a single cssInterop(Svg) setup.src/components/ui/alert/index.tsx (1)
138-138: Nice: conditional Svg height/width mirrors Checkbox behaviorPrevents passing undefined props and keeps Svg sizing predictable.
Minor: mirror the checkbox typing tweak for colorProps to keep typings strict.
- let colorProps = {}; + let colorProps: Pick<IPrimitiveIcon, 'color' | 'stroke' | 'fill'> = {};Also consider standardizing the Svg import style project-wide as noted in the UI barrel comment.
src/components/ui/bottomsheet/index.tsx (3)
45-46: Satisfy Prettier: remove spaces inside empty blocks.This fixes the CI warnings at Lines 45–46.
- handleClose: () => { }, - handleOpen: () => { }, + handleClose: () => {}, + handleOpen: () => {},
169-176: Indentation/style: fix Prettier “Insert ··” warnings.Re-indent the web
onKeyDownhandler object to unblock the pipeline.- onKeyDown: (e: React.KeyboardEvent) => { - if (e.key === 'Escape') { - e.preventDefault(); - handleClose(); - return; - } - }, + onKeyDown: (e: React.KeyboardEvent) => { + if (e.key === 'Escape') { + e.preventDefault(); + handleClose(); + return; + } + },
190-195: Follow repo style: prefer?:over&&for conditional rendering.Matches the TSX guideline in Coding Guidelines; behavior unchanged.
- {visible && ( - <FocusScope contain={visible} autoFocus={true} restoreFocus={true}> - {props.children} - </FocusScope> - )} + {visible ? ( + <FocusScope contain={visible} autoFocus={true} restoreFocus={true}> + {props.children} + </FocusScope> + ) : null}src/components/maps/location-picker.tsx (1)
12-18: Remove redundant| undefinedand tighten the prop type.
initialLocation?already impliesundefined; the union is redundant. Simplify for clarity.- initialLocation?: - | { - latitude: number; - longitude: number; - address?: string; - } - | undefined; + initialLocation?: { + latitude: number; + longitude: number; + address?: string; + };Additionally (optional), align types across prop/callback/state with a shared alias to keep the address field flowing through:
type PickedLocation = { latitude: number; longitude: number; address?: string }; interface LocationPickerProps { initialLocation?: PickedLocation; onLocationSelected: (location: PickedLocation) => void; height?: number; }src/components/ui/shared-tabs.tsx (9)
61-61: ConfirmuseColorSchemesource aligns with test mocks.You import from
nativewind, but the repo mock shown targetsreact-native. Ensure tests/mocks covernativewind’s hook or swap toreact-native’suseColorSchemefor consistency.
88-93: Text sizing insizeStylesis on Pressable, not Text.
text-*on Pressable won’t size Text. Remove from button and keep sizing ingetTextClasses.- const sizeStyles = { - sm: isLandscape ? 'px-3 py-1.5 text-xs' : 'px-2 py-1 text-2xs', - md: isLandscape ? 'px-4 py-2 text-sm' : 'px-3 py-1.5 text-xs', - lg: isLandscape ? 'px-5 py-2.5 text-base' : 'px-4 py-2 text-sm', - }[size]; + const sizeStyles = { + sm: isLandscape ? 'px-3 py-1.5' : 'px-2 py-1', + md: isLandscape ? 'px-4 py-2' : 'px-3 py-1.5', + lg: isLandscape ? 'px-5 py-2.5' : 'px-4 py-2', + }[size];
94-101: Avoidtext-*color classes on Pressable; they don’t affect Text.These can mislead reviewers and conflict with Text styling. Prefer to keep color on Text only.
- default: isActive ? 'border-b-2 border-primary-500 text-primary-500' : `border-b-2 border-transparent ${colorScheme === 'dark' ? 'text-gray-400' : 'text-gray-500'}`, - pills: isActive ? 'bg-primary-500 text-white rounded-full' : `bg-transparent ${colorScheme === 'dark' ? 'text-gray-400' : 'text-gray-500'}`, - underlined: isActive ? 'border-b-2 border-primary-500 text-primary-500' : `border-b-2 border-transparent ${colorScheme === 'dark' ? 'text-gray-400' : 'text-gray-500'}`, - segmented: isActive ? 'bg-primary-500 text-white' : `${colorScheme === 'dark' ? 'bg-gray-800 text-gray-400' : 'bg-gray-100 text-gray-500'}`, + default: isActive ? 'border-b-2 border-primary-500' : 'border-b-2 border-transparent', + pills: isActive ? 'bg-primary-500 rounded-full' : 'bg-transparent', + underlined: isActive ? 'border-b-2 border-primary-500' : 'border-b-2 border-transparent', + segmented: isActive ? 'bg-primary-500' : (colorScheme === 'dark' ? 'bg-gray-800' : 'bg-gray-100'),
118-134: Reassessflex: 1on ScrollView’s contentContainerStyle; can force full-width tabs in scrollable mode.For horizontally scrolling tabs,
flexGrow: 0and noflex: 1usually yields intrinsic widths. Verify UX across variants/orientations.- flex: 1, + // Avoid forcing items to stretch across the viewport in scrollable mode + ...(scrollable ? {} : { flex: 1 }),Also applies to: 140-140
104-116: Inconsistent spacing strategy between scrollable and non-scrollable branches.Scrollable uses RN
gap, non-scroll usesspace-x-2utility. Prefer one approach (e.g., style object withgap) for consistency and to avoid relying on Tailwind RNspace-*support.
142-142: Avoid inline arrow functions in handlers; add accessibility.Inline closures churn. Also set proper accessibility for tabs.
Add a factory and update handlers:
const handleTabPress = useCallback( (index: number) => { onChange?.(index); setCurrentIndex(index); }, [onChange] ); + const onTabPress = useCallback((i: number) => () => handleTabPress(i), [handleTabPress]);- <Pressable key={tab.key} className={getTabStyles(index)} onPress={() => handleTabPress(index)}> + <Pressable + key={tab.key} + className={getTabStyles(index)} + onPress={onTabPress(index)} + accessibilityRole="tab" + accessibilityState={{ selected: index === currentIndex }} + >Do the same in the non-scrollable branch.
Also applies to: 160-160
143-153: Use?:instead of&&for conditional rendering (repo guideline).Replace
&&patterns for icon and badge.- {tab.icon && <Box className={isLandscape ? 'mr-1.5' : 'mr-1'}>{tab.icon}</Box>} + {tab.icon ? <Box className={isLandscape ? 'mr-1.5' : 'mr-1'}>{tab.icon}</Box> : null} ... - {tab.badge !== undefined && tab.badge > 0 && ( + {(tab.badge ?? 0) > 0 ? ( <Box className={`${isLandscape ? 'ml-1.5' : 'ml-1'} min-w-[20px] items-center rounded-full bg-red-500 px-1.5 py-0.5`}> <Text className="text-xs font-bold text-white">{tab.badge}</Text> </Box> - )} + ) : null}Apply in both branches.
Also applies to: 161-171
31-42: Add controlled prop for parity with onChange (optional).Expose
activeIndex?: numberso parents can fully control the selection without a global store.
177-179: Clamp index when tabs change length.If tabs shrink,
tabs[currentIndex]can be undefined. Clamp or reset.Example:
// Outside selected lines; add an effect: useEffect(() => { setCurrentIndex((i) => Math.min(Math.max(0, i), tabs.length - 1)); }, [tabs.length]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.eslintignore(1 hunks)src/app/login/login-form.tsx(1 hunks)src/components/calendar/calendar-card.tsx(2 hunks)src/components/calendar/compact-calendar-item.tsx(2 hunks)src/components/maps/full-screen-location-picker.tsx(5 hunks)src/components/maps/location-picker.tsx(2 hunks)src/components/ui/__tests__/focus-aware-status-bar.test.tsx(1 hunks)src/components/ui/__tests__/header-simple.test.tsx(0 hunks)src/components/ui/__tests__/header.test.tsx(0 hunks)src/components/ui/__tests__/shared-tabs.test.tsx(0 hunks)src/components/ui/accordion/index.tsx(1 hunks)src/components/ui/actionsheet/index.tsx(1 hunks)src/components/ui/alert/index.tsx(1 hunks)src/components/ui/badge/index.tsx(1 hunks)src/components/ui/bottomsheet/index.tsx(2 hunks)src/components/ui/card/index.tsx(1 hunks)src/components/ui/checkbox/index.tsx(1 hunks)src/components/ui/drawer/index.tsx(2 hunks)src/components/ui/focus-aware-status-bar.tsx(1 hunks)src/components/ui/form-control/index.tsx(1 hunks)src/components/ui/header.tsx(0 hunks)src/components/ui/index.tsx(1 hunks)src/components/ui/shared-tabs.tsx(4 hunks)src/components/ui/skeleton/index.tsx(1 hunks)src/services/__examples__/signalr-typed-invoke-examples.ts(6 hunks)tsconfig.json(1 hunks)types/gluestack-ui.d.ts(1 hunks)
💤 Files with no reviewable changes (4)
- src/components/ui/tests/header.test.tsx
- src/components/ui/tests/shared-tabs.test.tsx
- src/components/ui/tests/header-simple.test.tsx
- src/components/ui/header.tsx
✅ Files skipped from review due to trivial changes (2)
- .eslintignore
- src/app/login/login-form.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- tsconfig.json
- src/components/ui/form-control/index.tsx
- src/components/ui/skeleton/index.tsx
- src/components/ui/badge/index.tsx
- src/components/ui/actionsheet/index.tsx
- src/components/ui/accordion/index.tsx
- src/components/maps/full-screen-location-picker.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/components/ui/index.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/components/ui/card/index.tsxsrc/components/ui/drawer/index.tsxtypes/gluestack-ui.d.tssrc/components/ui/alert/index.tsxsrc/components/calendar/compact-calendar-item.tsxsrc/services/__examples__/signalr-typed-invoke-examples.tssrc/components/ui/shared-tabs.tsxsrc/components/calendar/calendar-card.tsxsrc/components/ui/checkbox/index.tsxsrc/components/maps/location-picker.tsxsrc/components/ui/focus-aware-status-bar.tsxsrc/components/ui/bottomsheet/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/components/ui/index.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/components/ui/card/index.tsxsrc/components/ui/drawer/index.tsxsrc/components/ui/alert/index.tsxsrc/components/calendar/compact-calendar-item.tsxsrc/components/ui/shared-tabs.tsxsrc/components/calendar/calendar-card.tsxsrc/components/ui/checkbox/index.tsxsrc/components/maps/location-picker.tsxsrc/components/ui/focus-aware-status-bar.tsxsrc/components/ui/bottomsheet/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/components/ui/index.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/components/ui/card/index.tsxsrc/components/ui/drawer/index.tsxsrc/components/ui/alert/index.tsxsrc/components/calendar/compact-calendar-item.tsxsrc/services/__examples__/signalr-typed-invoke-examples.tssrc/components/ui/shared-tabs.tsxsrc/components/calendar/calendar-card.tsxsrc/components/ui/checkbox/index.tsxsrc/components/maps/location-picker.tsxsrc/components/ui/focus-aware-status-bar.tsxsrc/components/ui/bottomsheet/index.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/index.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/components/ui/card/index.tsxsrc/components/ui/drawer/index.tsxsrc/components/ui/alert/index.tsxsrc/components/ui/shared-tabs.tsxsrc/components/ui/checkbox/index.tsxsrc/components/ui/focus-aware-status-bar.tsxsrc/components/ui/bottomsheet/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/components/ui/__tests__/focus-aware-status-bar.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/ui/__tests__/focus-aware-status-bar.test.tsx
🧠 Learnings (8)
📚 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/ui/index.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/ui/index.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__/focus-aware-status-bar.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 : Ensure UI is intuitive, user-friendly, and works across different devices and screen sizes
Applied to files:
src/components/ui/drawer/index.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 gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Applied to files:
types/gluestack-ui.d.tssrc/components/ui/shared-tabs.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/components/ui/**/*.tsx : Use gluestack-ui components from components/ui; if unavailable, style via StyleSheet.create or styled-components
Applied to files:
types/gluestack-ui.d.tssrc/components/calendar/calendar-card.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 : Wrap all user-facing text in t() from react-i18next for translations
Applied to files:
src/components/calendar/compact-calendar-item.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 : Support dark mode and light mode in UI components
Applied to files:
src/components/ui/shared-tabs.tsx
🧬 Code graph analysis (6)
src/components/ui/__tests__/focus-aware-status-bar.test.tsx (1)
src/components/ui/focus-aware-status-bar.tsx (1)
FocusAwareStatusBar(9-63)
src/components/calendar/compact-calendar-item.tsx (2)
__mocks__/@/components/ui/vstack.tsx (1)
VStack(4-6)__mocks__/@/components/ui/hstack.tsx (1)
HStack(4-6)
src/services/__examples__/signalr-typed-invoke-examples.ts (1)
src/services/signalr.service.ts (1)
signalRService(704-704)
src/components/ui/shared-tabs.tsx (2)
__mocks__/react-native.ts (2)
useColorScheme(37-37)StyleSheet(48-50)__mocks__/@/components/ui/box.tsx (1)
Box(4-6)
src/components/calendar/calendar-card.tsx (4)
__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)src/utils/webview-html.ts (2)
defaultWebViewProps(136-149)generateWebViewHtml(29-131)
src/components/ui/focus-aware-status-bar.tsx (1)
__mocks__/react-native.ts (1)
Platform(2-5)
🪛 GitHub Check: test
src/components/ui/bottomsheet/index.tsx
[warning] 46-46:
Delete ·
[warning] 45-45:
Delete ·
[warning] 176-176:
Insert ··
[warning] 175-175:
Insert ··
[warning] 174-174:
Insert ··
[warning] 173-173:
Insert ··
[warning] 172-172:
Insert ··
[warning] 171-171:
Insert ··
[warning] 170-170:
Insert ··
[warning] 169-169:
Insert ··
🪛 GitHub Actions: React Native CI/CD
src/components/ui/bottomsheet/index.tsx
[warning] 45-45: prettier/prettier: Delete ·
[warning] 46-46: prettier/prettier: Delete ·
[warning] 169-169: prettier/prettier: Insert ··
[warning] 170-170: prettier/prettier: Insert ··
[warning] 171-171: prettier/prettier: Insert ··
[warning] 172-172: prettier/prettier: Insert ··
[warning] 173-173: prettier/prettier: Insert ··
[warning] 174-174: prettier/prettier: Insert ··
[warning] 175-175: prettier/prettier: Insert ··
[warning] 176-176: prettier/prettier: Insert ··
🔇 Additional comments (14)
src/services/__examples__/signalr-typed-invoke-examples.ts (2)
35-41: LGTM: typed invoke and usage are solidFormatting-only change; behavior unchanged. Good use of a concrete generic on invoke and typed return.
52-57: LGTM: concise typed invokeCollapsing to a single line improves readability without altering behavior.
src/components/calendar/compact-calendar-item.tsx (2)
6-6: Import updated to Card-only looks good
86-86: Confirm CheckCircle gating (isSignedUp && canSignUp)If signups close after a user signs up, the icon hides. Is that desired? Consider
isSignedUp ? … : nullindependently ofcanSignUp.src/components/calendar/calendar-card.tsx (2)
10-10: Import updated to Card-only looks good
96-113: Good security posture on embedded HTML
generateWebViewHtmlsanitizes content anddefaultWebViewPropsdisables JS/DOM storage. Solid.src/components/ui/focus-aware-status-bar.tsx (2)
14-16: Good focus guard.Early-return on blur prevents off-screen overrides.
58-60: Web guard is correct.No-op on web avoids unsupported APIs.
src/components/ui/__tests__/focus-aware-status-bar.test.tsx (1)
1-8: Solid smoke test.Import/shape checks are fine for baseline coverage.
src/components/ui/index.tsx (1)
5-5: Convert named Svg imports to default across UI components
Replace all instances ofimport { Svg } from 'react-native-svg';with
import Svg from 'react-native-svg';in the following files so cssInterop applies consistently:
- src/components/ui/badge/index.tsx
- src/components/ui/form-control/index.tsx
- src/components/ui/checkbox/index.tsx
- src/components/ui/accordion/index.tsx
- src/components/ui/actionsheet/index.tsx
- src/components/ui/alert/index.tsx
⛔ Skipped due to learnings
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 componentLearnt 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 componentLearnt from: CR PR: Resgrid/Responder#0 File: .cursorrules:0-0 Timestamp: 2025-08-21T02:46:42.743Z Learning: Applies to **/*.tsx : Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-componentsLearnt from: CR PR: Resgrid/Responder#0 File: .cursorrules:0-0 Timestamp: 2025-08-21T02:46:42.743Z Learning: Applies to **/*.tsx : Use react-native-fast-image for image handling instead of the default Image where appropriateLearnt from: CR PR: Resgrid/Responder#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-21T02:47:24.081Z Learning: Applies to **/*.tsx : Ensure all components are mobile-friendly, responsive, and support both iOS and AndroidLearnt from: CR PR: Resgrid/Responder#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-21T02:47:24.081Z Learning: Applies to **/*.tsx : Ensure UI is intuitive, user-friendly, and works across different devices and screen sizesLearnt from: CR PR: Resgrid/Responder#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-21T02:47:24.081Z Learning: Applies to **/*.tsx : Optimize image handling using react-native-fast-imageLearnt from: CR PR: Resgrid/Responder#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-21T02:47:24.081Z Learning: Applies to src/components/ui/**/*.tsx : Use gluestack-ui components from components/ui; if unavailable, style via StyleSheet.create or styled-componentsLearnt from: CR PR: Resgrid/Responder#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-21T02:47:24.081Z Learning: Applies to **/*.{ts,tsx} : Use TypeScript for all components, favoring interfaces for props and stateLearnt from: CR PR: Resgrid/Responder#0 File: .cursorrules:0-0 Timestamp: 2025-08-21T02:46:42.743Z Learning: Applies to **/*.{ts,tsx} : Use TypeScript for all components and favor interfaces for props and statesrc/components/maps/location-picker.tsx (1)
76-83: Use the specific RNMapbox onPress event type instead ofany.Replace the untyped handler:
const handleMapPress = (event: any) => { … }with something like:
import type { OnPressEvent } from '@rnmapbox/maps'; const handleMapPress = (event: OnPressEvent) => { const [longitude, latitude] = event.geometry.coordinates; setCurrentLocation({ latitude, longitude }); };Confirm that
OnPressEvent(or the exact type name) is exported by@rnmapbox/maps@10.1.42-rc.0and adjust if necessary.src/components/ui/shared-tabs.tsx (3)
4-4: Importing StyleSheet/useWindowDimensions is appropriate for RN; keep.
1-1: Dark/light mode support looks good.Using color scheme for theming aligns with guidelines.
7-10: UI text wrapped with i18n and gluestack components—nice.
| CardContent.displayName = 'CardContent'; | ||
|
|
||
| export { Card, CardContent }; | ||
| export { Card }; |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
API surface change: verify removal of CardContent across the repo
Ensure no remaining imports/usages of CardContent. Add a migration note in the PR description if this is intentional.
Run:
🏁 Script executed:
#!/bin/bash
# Find leftover imports/usages of CardContent
rg -nP -C2 --type=ts --type=tsx $'import\\s+\\{[^}]*\\bCardContent\\b' || true
rg -nP -C2 --type=ts --type=tsx $'<\\s*CardContent\\b' || trueLength of output: 191
🏁 Script executed:
#!/bin/bash
# Re-run search using glob patterns instead of --type for .ts and .tsx files
rg -nP -C2 'import\s+\{[^}]*\bCardContent\b' -g '*.ts' -g '*.tsx' || true
rg -nP -C2 '<\s*CardContent\b' -g '*.ts' -g '*.tsx' || trueLength of output: 135
Add migration note for removed CardContent Confirmed no remaining imports or usages of CardContent across the repo; please include a migration note in the PR description.
🤖 Prompt for AI Agents
In src/components/ui/card/index.tsx around line 15, add a migration note to the
PR description stating that the named export CardContent has been removed;
indicate the replacement (use Card children or CardBody/CardSection if
applicable), list the commit/PR that removed it, and confirm you searched the
repo for remaining imports/usages (include the grep/rg command or tool used and
its result). Update the PR description with this short migration guidance and
verification so consumers know how to update their code.
| try { | ||
| // Make both status bar and navigation bar transparent | ||
| StatusBar.setBackgroundColor('transparent'); | ||
| StatusBar.setTranslucent(true); | ||
|
|
||
| // Hide navigation bar only on Android | ||
| NavigationBar.setVisibilityAsync('hidden').catch(() => { | ||
| // Silently handle errors if NavigationBar API is not available | ||
| }); | ||
|
|
||
| // Set the system UI flags to hide navigation bar | ||
| if (hidden) { | ||
| StatusBar.setHidden(true, 'slide'); | ||
| } else { | ||
| StatusBar.setHidden(false, 'slide'); | ||
| } | ||
|
|
||
| // Adapt status bar content based on theme | ||
| StatusBar.setBarStyle(colorScheme === 'dark' ? 'light-content' : 'dark-content'); | ||
| } catch (error) { | ||
| // Silently handle errors if StatusBar methods are not available | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid mixing imperative StatusBar/NavigationBar calls with SystemBars; also avoid deprecated Android APIs.
react-native-edge-to-edge advises replacing StatusBar and expo-navigation-bar with SystemBars to prevent conflicts. Additionally, StatusBar.setTranslucent and StatusBar.setBackgroundColor are deprecated/no-op on Android 15+; rely on edge-to-edge instead. (npmjs.com, github.com, reactnative.dev)
Proposed simplification:
-import * as NavigationBar from 'expo-navigation-bar';
-import { Platform, StatusBar } from 'react-native';
+import { Platform } from 'react-native';
@@
- React.useEffect(() => {
- // Early return if screen is not focused to prevent off-screen instances from overriding UI
- if (!isFocused) return;
-
- // Only call platform-specific methods when they are supported
- if (Platform.OS === 'android') {
- try {
- // Make both status bar and navigation bar transparent
- StatusBar.setBackgroundColor('transparent');
- StatusBar.setTranslucent(true);
-
- // Hide navigation bar only on Android
- NavigationBar.setVisibilityAsync('hidden').catch(() => {
- // Silently handle errors if NavigationBar API is not available
- });
-
- // Set the system UI flags to hide navigation bar
- if (hidden) {
- StatusBar.setHidden(true, 'slide');
- } else {
- StatusBar.setHidden(false, 'slide');
- }
-
- // Adapt status bar content based on theme
- StatusBar.setBarStyle(colorScheme === 'dark' ? 'light-content' : 'dark-content');
- } catch (error) {
- // Silently handle errors if StatusBar methods are not available
- }
- } else if (Platform.OS === 'ios') {
- try {
- // iOS-specific status bar configuration
- if (hidden) {
- StatusBar.setHidden(true, 'slide');
- } else {
- StatusBar.setHidden(false, 'slide');
- }
-
- // Set status bar style for iOS
- StatusBar.setBarStyle(colorScheme === 'dark' ? 'light-content' : 'dark-content');
- } catch (error) {
- // Silently handle errors if StatusBar methods are not available
- }
- }
- }, [hidden, colorScheme, isFocused]);
+ // All bar state managed via <SystemBars />; no imperative calls needed.
@@
- return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') ? <SystemBars style={colorScheme === 'dark' ? 'dark' : 'light'} hidden={{ statusBar: hidden, navigationBar: true }} /> : null;
+ return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios')
+ ? (
+ <SystemBars
+ style="auto"
+ hidden={{ statusBar: hidden, navigationBar: Platform.OS === 'android' ? hidden : false }}
+ />
+ )
+ : null;Also applies to: 41-55, 56-56
| // Only render SystemBars when focused and on supported platforms | ||
| return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') ? <SystemBars style={colorScheme === 'dark' ? 'dark' : 'light'} hidden={{ statusBar: hidden, navigationBar: true }} /> : null; |
There was a problem hiding this comment.
Bug: inverted SystemBars style mapping in dark mode.
style={colorScheme === 'dark' ? 'dark' : 'light'} sets dark icons on a dark background. Prefer "auto" (recommended) or invert the mapping. (npmjs.com)
Apply one of:
- return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') ? <SystemBars style={colorScheme === 'dark' ? 'dark' : 'light'} hidden={{ statusBar: hidden, navigationBar: true }} /> : null;
+ return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios')
+ ? <SystemBars style="auto" hidden={{ statusBar: hidden, navigationBar: true }} />
+ : null;or
- style={colorScheme === 'dark' ? 'dark' : 'light'}
+ style={colorScheme === 'dark' ? 'light' : 'dark'}📝 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.
| // Only render SystemBars when focused and on supported platforms | |
| return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') ? <SystemBars style={colorScheme === 'dark' ? 'dark' : 'light'} hidden={{ statusBar: hidden, navigationBar: true }} /> : null; | |
| // Only render SystemBars when focused and on supported platforms | |
| return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') | |
| ? <SystemBars | |
| style="auto" | |
| hidden={{ statusBar: hidden, navigationBar: true }} | |
| /> | |
| : null; |
🤖 Prompt for AI Agents
In src/components/ui/focus-aware-status-bar.tsx around lines 61-62, the
SystemBars style is inverted for dark mode (style={colorScheme === 'dark' ?
'dark' : 'light'}) causing dark icons on a dark background; change the mapping
to either use the recommended automatic mode (style="auto") or invert the
conditional so dark mode yields 'light' icons (e.g., style={colorScheme ===
'dark' ? 'light' : 'dark'}) and keep the rest of the props
(hidden/navigationBar) unchanged.
| // Get appropriate text color based on theme | ||
| const getTextColor = () => { | ||
| return colorScheme === 'dark' ? 'text-gray-200' : 'text-gray-800'; | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Active text color isn’t applied; compute text classes per tab and use them on Text.
Color classes on Pressable don’t affect nested Text in RN. Move text color/size logic to Text.
- // Get appropriate text color based on theme
- const getTextColor = () => {
- return colorScheme === 'dark' ? 'text-gray-200' : 'text-gray-800';
- };
+ // Compute text classes based on active state, variant, and orientation
+ const getTextClasses = (index: number) => {
+ const isActive = index === currentIndex;
+ const sizeCls =
+ {
+ sm: isLandscape ? 'text-xs' : 'text-2xs',
+ md: isLandscape ? 'text-sm' : 'text-xs',
+ lg: isLandscape ? 'text-base' : 'text-sm',
+ }[size];
+ const inactive = colorScheme === 'dark' ? 'text-gray-400' : 'text-gray-500';
+ const active =
+ variant === 'pills' || variant === 'segmented' ? 'text-white' : 'text-primary-500';
+ return `${sizeCls} ${isActive ? active : inactive}`;
+ };- {typeof tab.title === 'string' ? (
- <Text className={isLandscape ? getTextColor() : `text-xs ${getTextColor()}`}>{t(tab.title)}</Text>
- ) : (
- <Text className={isLandscape ? getTextColor() : `text-xs ${getTextColor()}`}>{tab.title}</Text>
- )}
+ {typeof tab.title === 'string'
+ ? <Text className={getTextClasses(index)}>{t(tab.title)}</Text>
+ : <Text className={getTextClasses(index)}>{tab.title}</Text>}Apply the same change in the non-scrollable branch.
Also applies to: 145-148, 163-166
🤖 Prompt for AI Agents
In src/components/ui/shared-tabs.tsx around lines 78-82 (and also update the
non-scrollable branch at 145-148 and 163-166), the current color/size classes
are applied to the Pressable which does not affect nested Text in React Native;
compute the text classes per-tab (including active vs inactive state and size) —
e.g. create/get a getTextClasses(tab, isActive) helper that returns the correct
color and size class string — and apply those classes directly to the Text
element inside each tab; remove color classes from the Pressable and ensure the
same change is mirrored in both the scrollable and non-scrollable code paths.
| // Global type augmentations for Motion components compatibility | ||
| declare global { | ||
| namespace React { | ||
| interface HTMLAttributes<T> { | ||
| className?: string; | ||
| pointerEvents?: string; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t augment React.HTMLAttributes for RN; augment react-native props instead
Adding className/pointerEvents to HTMLAttributes affects web DOM, not RN. Prefer augmenting RN props so TS accepts className on native components and pointerEvents keeps its RN union.
-declare global {
- namespace React {
- interface HTMLAttributes<T> {
- className?: string;
- pointerEvents?: string;
- }
- }
-}
+declare module 'react-native' {
+ interface ViewProps {
+ className?: string;
+ pointerEvents?: 'box-none' | 'none' | 'box-only' | 'auto';
+ }
+ interface ScrollViewProps {
+ className?: string;
+ }
+ interface PressableProps {
+ className?: string;
+ }
+}📝 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.
| // Global type augmentations for Motion components compatibility | |
| declare global { | |
| namespace React { | |
| interface HTMLAttributes<T> { | |
| className?: string; | |
| pointerEvents?: string; | |
| } | |
| } | |
| } | |
| // Global type augmentations for Motion components compatibility | |
| declare module 'react-native' { | |
| interface ViewProps { | |
| className?: string; | |
| pointerEvents?: 'box-none' | 'none' | 'box-only' | 'auto'; | |
| } | |
| interface ScrollViewProps { | |
| className?: string; | |
| } | |
| interface PressableProps { | |
| className?: string; | |
| } | |
| } |
🤖 Prompt for AI Agents
In types/gluestack-ui.d.ts around lines 58 to 66, stop augmenting
React.HTMLAttributes (which targets web DOM); instead merge declarations into
the 'react-native' module and extend the appropriate RN prop interfaces (e.g.,
ViewProps and TextProps) to add className?: string and preserve RN pointerEvents
typing (use the existing RN pointerEvents union type) so TypeScript accepts
className on native components and pointerEvents remains the RN union; remove
the global React augmentation and add a declare module 'react-native' block that
extends the specific native prop interfaces.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/common/loading.tsx (1)
1-1: Critical: Box is imported from lucide (icon), not your UI primitiveThis will render an icon instead of a layout container and breaks all skeletons/dots. Import the UI Box instead.
-import { Box, Loader2 } from 'lucide-react-native'; +import { Loader2 } from 'lucide-react-native'; +import { Box } from '../ui/box';
🧹 Nitpick comments (6)
src/components/common/loading.tsx (6)
53-55: Dots now pulse in sync; reintroduce stagger for better affordanceBring back per-dot delay so users perceive progress.
- <Box key={i} className={`bg-primary rounded-full ${size === 'small' ? 'size-2' : size === 'large' ? 'size-3' : 'size-4'} animate-pulse`} /> + <Box + key={i} + className={`bg-primary rounded-full ${size === 'small' ? 'size-2' : 'size-3'} animate-pulse`} + style={{ animationDelay: `${(i - 1) * 120}ms` }} + />
25-27: Docs: default size mismatchJSDoc says "lg" but runtime default is "large" and type is 'small' | 'large'.
- * @default "lg" + * @default "large"
59-62: Simplify icon sizing; remove unreachable branchType only allows 'small' | 'large'; the fallback never runs.
- return <Loader2 size={size === 'small' ? 24 : size === 'large' ? 32 : 40} className="text-primary animate-spin" />; + return <Loader2 size={size === 'small' ? 24 : 32} className="text-primary animate-spin" />;
66-71: Accessibility: announce loading stateExpose role/label so screen readers convey progress.
- <View className={containerClasses}> + <View + className={containerClasses} + accessible + accessibilityRole="progressbar" + accessibilityLabel={loadingText} + >
70-70: Consistency: avoid conflicting text colors and use ?:Keep theme token (text-foreground) and follow the ?: guideline.
- {loadingText && <Text className={`text-foreground mt-2 font-medium text-blue-400 ${size === 'small' ? 'text-xs' : size === 'large' ? 'text-sm' : 'text-base'}`}>{loadingText}</Text>} + {loadingText ? ( + <Text className={`text-foreground mt-2 font-medium ${size === 'small' ? 'text-xs' : 'text-sm'}`}> + {loadingText} + </Text> + ) : null}
123-123: Type-safety: don’t assert width/height to numberStyles accept number | string; the cast hides issues.
- return <Box className={`bg-muted/30 animate-pulse ${radiusMap[borderRadius]}`} style={{ width: width as number, height: height as number }} />; + return <Box className={`bg-muted/30 animate-pulse ${radiusMap[borderRadius]}`} style={{ width, height }} />;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/common/loading.tsx(1 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/common/loading.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/common/loading.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/common/loading.tsx
1 participant
Summary by CodeRabbit
New Features
Improvements
Chores