-
Notifications
You must be signed in to change notification settings - Fork 4
Develop #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds an app initialization singleton, integrates iOS CallKeep mute-state callbacks and audio session options, updates push-permission request to include iOS options, refactors FocusAwareStatusBar to runtime effects with tests, removes LiveKit token fetch/connect, updates mocks/tests, and adds iOS Expo entitlements. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant RootLayout
participant AppInit as AppInitializationService
participant CallKeep as CallKeepService (iOS)
participant Logger
App->>RootLayout: mount
RootLayout->>AppInit: initialize()
AppInit->>Logger: debug "Initializing..."
alt Platform is iOS
AppInit->>CallKeep: setup(config)
CallKeep-->>AppInit: resolved
AppInit->>Logger: info "CallKeep initialized"
else other platforms
AppInit->>Logger: debug "skipping CallKeep"
end
AppInit-->>RootLayout: initialized
sequenceDiagram
autonumber
participant LiveKitStore
participant Room
participant CallKeep as CallKeepService (iOS)
participant Logger
LiveKitStore->>Room: connectToRoom(...) (connect call commented/out)
Note right of LiveKitStore: on Connected event handlers run
Room-->>LiveKitStore: ConnectionStateChanged: Connected
LiveKitStore->>CallKeep: setMuteStateCallback(fn)
LiveKitStore->>CallKeep: startCall(...)
CallKeep-->>LiveKitStore: didPerformSetMutedCallAction (muted)
CallKeep->>LiveKitStore: invoke registered fn(muted)
LiveKitStore->>Logger: logs
Room-->>LiveKitStore: ConnectionStateChanged: Disconnected
LiveKitStore->>CallKeep: setMuteStateCallback(null)
LiveKitStore->>CallKeep: endCall(...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/services/push-notification.ts (2)
185-187: Redundant ternary when assigning tokenThe ternary returns the same value on both branches. Simplify for clarity.
- const token = Platform.OS === 'ios' ? devicePushToken.data : devicePushToken.data; + const token = devicePushToken.data;
189-196: Avoid logging raw device push tokensDevice push tokens can be considered sensitive. Logging them at info level risks leakage in logs and analytics. Redact or omit.
[security]
- logger.info({ - message: 'Push notification token obtained', - context: { - token: this.pushToken, - unitId, - platform: Platform.OS, - }, - }); + logger.info({ + message: 'Push notification token obtained', + context: { + tokenSuffix: this.pushToken?.slice(-6), // optional minimal traceability + unitId, + platform: Platform.OS, + }, + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/services/push-notification.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/services/push-notification.ts
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/services/push-notification.ts
| ios: { | ||
| allowAlert: true, | ||
| allowBadge: true, | ||
| allowSound: true, | ||
| allowCriticalAlerts: true, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify iOS critical alerts entitlement before requesting allowCriticalAlerts
Requesting allowCriticalAlerts (Line 163) requires the com.apple.developer.usernotifications.critical-alerts entitlement and App Store approval. Without it, the request may fail or be ignored. Consider gating it behind a feature flag or config check.
Run this script to verify the entitlement is configured:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for iOS critical alerts entitlement/config..."
# Look through common Expo/React Native config files
fd -a -t f 'app.json' || true
fd -a -t f 'app.config.*' || true
fd -a -t f 'Info.plist' || true
fd -a -t f '*.entitlements' || true
# Grep for entitlement keys and related config flags
rg -n -S -C2 'com\.apple\.developer\.usernotifications\.critical-alerts|criticalAlerts|allowCriticalAlerts|providesAppNotificationSettings' || trueLength of output: 1043
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for .entitlements files..."
fd -t f --glob '*.entitlements' || true
echo
echo "Displaying contents of any .entitlements files (first 200 lines each)..."
fd -t f --glob '*.entitlements' | xargs -r -n1 sh -c 'echo "== $0 =="; sed -n "1,200p" "$0"'
echo
echo "Searching app.config.ts for entitlement keys..."
rg -n 'entitl' app.config.ts || true
rg -n 'critical-alerts' app.config.ts || true
rg -n 'usernotifications' app.config.ts || true
echo
echo "Displaying first 200 lines of app.config.ts..."
sed -n '1,200p' app.config.tsLength of output: 6743
Missing iOS Critical Alerts Entitlement
I searched the repo and didn’t find any .entitlements files or Expo config entries for com.apple.developer.usernotifications.critical-alerts. As a result, setting allowCriticalAlerts: true will either be ignored or fail at runtime without App Store approval.
Action items:
- Add the
com.apple.developer.usernotifications.critical-alertsentitlement to your iOS build (via an.entitlementsfile orInfo.plist, or inapp.config.tsunderios.entitlements). - Gate the
allowCriticalAlertsflag behind a feature flag or configuration check until the entitlement is fully set up.
🤖 Prompt for AI Agents
In src/services/push-notification.ts around lines 159 to 164, the iOS push
options enable allowCriticalAlerts:true but the repo lacks the required
com.apple.developer.usernotifications.critical-alerts entitlement, which causes
the setting to be ignored or fail. Add the entitlement to the iOS build
(create/update an .entitlements file or add ios.entitlements in app.config.ts /
Expo config or appropriate Info.plist entry) and, until the entitlement is
present and approved, wrap or gate allowCriticalAlerts behind a runtime/config
feature flag check so the code only sets it when the entitlement is configured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (1)
502-537: Deduplicate and fix nested “Error Handling” describe block.There is a nested duplicate “Error Handling” describe, and the “basic error state management” test appears twice. This increases test noise and can mislead future readers.
Apply this diff to remove the duplicated/nested suite:
- describe('Error Handling', () => { - describe('Error Handling', () => { - it('should handle room initialization errors', async () => { + describe('Error Handling', () => { + it('should handle room initialization errors', async () => { // Make the Room constructor throw an error MockedRoom.mockImplementationOnce(() => { throw new Error('Failed to initialize room'); }); @@ expect(result.current.isConnected).toBe(false); }); - it('should handle basic error state management', async () => { - const { result } = renderHook(() => useLiveKitCallStore()); - - // Test basic error clearing functionality since token fetching isn't implemented - act(() => { - // Set an error state and then clear it - result.current.actions._clearError(); - }); - - expect(result.current.error).toBeNull(); - }); - }); - - it('should handle basic error state management', async () => { - const { result } = renderHook(() => useLiveKitCallStore()); - - // Test basic error clearing functionality since token fetching isn't implemented - act(() => { - // Set an error state and then clear it - result.current.actions._clearError(); - }); - - expect(result.current.error).toBeNull(); - }); + it('should handle basic error state management', async () => { + const { result } = renderHook(() => useLiveKitCallStore()); + act(() => { + result.current.actions._clearError(); + }); + expect(result.current.error).toBeNull(); + }); });Also applies to: 538-548
src/features/livekit-call/store/useLiveKitCallStore.ts (2)
5-7: Fix platform-specific import: use RN resolver instead of hardcoding .ios.This store is used on both platforms, but it imports
callkeep.service.iosdirectly. On Android builds, this can fail at bundle-time. Prefer importingcallkeep.serviceand let React Native resolve.ios/.android.Apply this diff:
-import { callKeepService } from '../../../services/callkeep.service.ios'; +import { callKeepService } from '../../../services/callkeep.service';
220-226: Fix broken connection flow:isConnectingis never clearedInspect
connectToRoomin
src/features/livekit-call/store/useLiveKitCallStore.ts:
- Line 70: you call
set({ isConnecting: true, … })- Lines 114–116: the actual connect is commented out:
// await newRoom.connect(LIVEKIT_URL, token, connectOptions);- No normal‐path
set({ isConnecting: false }), so without firing ConnectionStateChanged, the store remains stuck “connecting” and UI blocks further attempts.Potential fixes:
- Option A (recommended): Re-enable the connect call (or guard it behind a production feature flag) so events fire and clear the flag:
const connectOptions: RoomConnectOptions = { autoSubscribe: true }; if (process.env.NODE_ENV !== 'test') { await newRoom.connect(LIVEKIT_URL, token, connectOptions); }- Option B: If disabling connect is intentional in this PR, explicitly clear the flag and log a warning:
- // await newRoom.connect(LIVEKIT_URL, token, connectOptions); + logger.warn('LiveKit connection disabled; skipping connect'); + set({ isConnecting: false });Please address to avoid leaving the store in a “connecting” limbo.
🧹 Nitpick comments (17)
src/components/ui/focus-aware-status-bar.tsx (4)
21-32: Align navigation bar visibility with the hidden propAndroid nav bar is forced hidden even when hidden=false. This makes the component’s behavior asymmetric and can be surprising. Toggle nav bar visibility consistently with the prop.
Apply this diff:
- // Hide navigation bar only on Android - NavigationBar.setVisibilityAsync('hidden').catch(() => { + // Toggle navigation bar visibility on Android based on `hidden` + NavigationBar.setVisibilityAsync(hidden ? 'hidden' : 'visible').catch(() => { // Silently handle errors if NavigationBar API is not available });
35-37: Consider logging caught errors in dev buildsSilently swallowing errors can make debugging harder in development. Suggest logging behind a dev guard.
Apply this diff pattern to both catch blocks:
} catch (error) { - // Silently handle errors if StatusBar methods are not available + // Silently handle in production; log in dev for visibility + if (__DEV__) { + // eslint-disable-next-line no-console + console.warn('FocusAwareStatusBar (android) error:', error); + } }and
} catch (error) { - // Silently handle errors if StatusBar methods are not available + if (__DEV__) { + // eslint-disable-next-line no-console + console.warn('FocusAwareStatusBar (ios) error:', error); + } }Also applies to: 49-51
55-59: Constrain SystemBars style to 'light' | 'dark' regardless of providerIf nativewind’s colorScheme ever yields an unexpected value (e.g., 'system'), passing it through could break SystemBars typing/behavior. Map to the accepted set.
Apply this diff:
- // Only render SystemBars when focused and on supported platforms - return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') ? <SystemBars style={colorScheme} hidden={{ statusBar: hidden, navigationBar: true }} /> : null; + // Only render SystemBars when focused and on supported platforms + const systemBarsStyle = colorScheme === 'dark' ? 'dark' : 'light'; + return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') + ? <SystemBars style={systemBarsStyle} hidden={{ statusBar: hidden, navigationBar: true }} /> + : null;
8-10: Use React.FC for consistency with project guidelinesKeeps component typing consistent across the codebase.
Apply this diff:
-type Props = { hidden?: boolean }; -export const FocusAwareStatusBar = ({ hidden = false }: Props) => { +type Props = { hidden?: boolean }; +export const FocusAwareStatusBar: React.FC<Props> = ({ hidden = false }) => {src/components/ui/__tests__/focus-aware-status-bar.test.tsx (4)
112-119: Make SystemBars call assertions resilient to React internalsAsserting the second argument as {} is brittle across React versions. Prefer not asserting it or use expect.anything().
Apply this diff in the three places:
- expect(mockSystemBars).toHaveBeenCalledWith( + expect(mockSystemBars).toHaveBeenCalledWith( { style: 'light', hidden: { statusBar: false, navigationBar: true }, }, - {} + expect.anything() );Also applies to: 170-177, 287-294
54-55: Avoidanyin tests; provide a small helper for typed color scheme mocksThis keeps tests type-safe without scattered casts.
One option is to add a helper near the top and replace the scattered casts:
@@ - mockUseColorScheme.mockReturnValue({ colorScheme: 'light' } as any); + const setColorScheme = (scheme: 'light' | 'dark') => + mockUseColorScheme.mockReturnValue({ colorScheme: scheme } as unknown as ReturnType<typeof useColorScheme>); + setColorScheme('light'); @@ - mockUseColorScheme.mockReturnValue({ colorScheme: 'dark' } as any); + setColorScheme('dark'); @@ - mockUseColorScheme.mockReturnValue({ colorScheme: 'dark' } as any); + setColorScheme('dark'); @@ - mockUseColorScheme.mockReturnValue({ colorScheme: 'dark' } as any); + setColorScheme('dark'); @@ - mockUseColorScheme.mockReturnValue({ colorScheme: 'dark' } as any); + setColorScheme('dark'); @@ - mockUseColorScheme.mockReturnValue({ colorScheme: 'dark' } as any); + setColorScheme('dark');Also applies to: 93-94, 158-159, 276-277, 284-285, 345-346
22-47: Restore original StatusBar statics after tests to prevent cross-file leakageYou replace StatusBar methods but never restore them. Jest’s module isolation usually mitigates this, but restoring improves test hygiene and future-proofs.
Apply this diff:
@@ -// Mock StatusBar methods -const mockStatusBar = { +// Mock StatusBar methods +const originalStatusBar = { + setBackgroundColor: StatusBar.setBackgroundColor, + setTranslucent: StatusBar.setTranslucent, + setHidden: StatusBar.setHidden, + setBarStyle: StatusBar.setBarStyle, +}; +const mockStatusBar = { setBackgroundColor: jest.fn(), setTranslucent: jest.fn(), setHidden: jest.fn(), setBarStyle: jest.fn(), }; @@ afterEach(() => { // Reset Platform.OS to original value Object.defineProperty(Platform, 'OS', { value: originalPlatform, writable: true, }); }); + + afterAll(() => { + // Restore original StatusBar statics + Object.defineProperty(StatusBar, 'setBackgroundColor', { value: originalStatusBar.setBackgroundColor, writable: true }); + Object.defineProperty(StatusBar, 'setTranslucent', { value: originalStatusBar.setTranslucent, writable: true }); + Object.defineProperty(StatusBar, 'setHidden', { value: originalStatusBar.setHidden, writable: true }); + Object.defineProperty(StatusBar, 'setBarStyle', { value: originalStatusBar.setBarStyle, writable: true }); + });Also applies to: 59-65
210-233: Unknown platform OS type castCasting to any to assign 'unknown' is fine in practice, but consider a typed helper to reduce casts and centralize platform mutation.
For example:
const setPlatform = (os: 'android' | 'ios' | 'web' | (string & {})) => { Object.defineProperty(Platform, 'OS', { value: os, writable: true }); };Then use setPlatform('android'), setPlatform('ios'), setPlatform('web'), setPlatform('someCustomOS').
src/app/_layout.tsx (1)
145-158: Initialize global services: looks correct; optionally consider parallelization/log wrappingThe initialization pattern and logging are consistent with the existing startup steps. If desired, you could launch all startup tasks in parallel and wrap them in a single
Promise.allSettledto reduce cold-start time and to unify success/failure logging, but this is optional since each call already has its own logging and they already run without awaiting one another.src/services/callkeep.service.ios.ts (3)
63-66: Use bitwise OR for categoryOptions instead of additionThese flags are bitmasks; while addition works with disjoint power-of-two values, bitwise OR communicates intent and avoids edge cases if flags change in the future.
Apply this diff:
- audioSession: { - categoryOptions: AudioSessionCategoryOption.allowAirPlay + AudioSessionCategoryOption.allowBluetooth + AudioSessionCategoryOption.allowBluetoothA2DP + AudioSessionCategoryOption.defaultToSpeaker, - mode: AudioSessionMode.voiceChat, - }, + audioSession: { + categoryOptions: + AudioSessionCategoryOption.allowAirPlay | + AudioSessionCategoryOption.allowBluetooth | + AudioSessionCategoryOption.allowBluetoothA2DP | + AudioSessionCategoryOption.defaultToSpeaker, + mode: AudioSessionMode.voiceChat, + },
207-214: Public API for mute-state callback: solid and minimalSimple setter with nullable unregistration keeps lifecycle management easy. Consider also nulling this in
cleanup()to avoid holding stale references across re-initializations.To complement this change outside the selected range, add the following in
cleanup()after removing event listeners:this.muteStateCallback = null;
271-288: Gate mute callback by active call UUID to avoid cross-call leaksIf multiple calls occur back-to-back, a late event for a previous call could incorrectly trigger the current mute handler. Guarding by
currentCallUUIDavoids that.Apply this diff:
RNCallKeep.addEventListener('didPerformSetMutedCallAction', ({ muted, callUUID }) => { logger.debug({ message: 'CallKeep mute state changed', context: { muted, callUUID }, }); - // Call the registered callback if available - if (this.muteStateCallback) { + // Only act on the current call and call the registered callback if available + if (currentCallUUID && callUUID === currentCallUUID && this.muteStateCallback) { try { this.muteStateCallback(muted); } catch (error) { logger.warn({ message: 'Failed to execute mute state callback', context: { error, muted, callUUID }, }); } } });src/services/__tests__/callkeep.service.ios.test.ts (1)
108-118: Broaden setup assertions to cover audioSession optionsSince audio session configuration is critical for iOS behavior, consider asserting the presence and composition of
ios.audioSessionfields to catch regressions early.You can extend this test with:
const setupCall = mockCallKeep.setup.mock.calls[0][0] as any; expect(setupCall.ios.audioSession).toBeDefined(); expect(setupCall.ios.audioSession.mode).toBe(1); // AudioSessionMode.voiceChat // For bitmask: allowAirPlay|allowBluetooth|allowBluetoothA2DP|defaultToSpeaker = 1|2|4|8 = 15 expect(setupCall.ios.audioSession.categoryOptions).toBe(15);src/services/__tests__/app-initialization.service.test.ts (1)
17-23: Future-proof mocks: avoid hardcoding the iOS-specific module path.Importing/mocking
../callkeep.service.iosties the tests to a platform-specific filename. If the implementation switches to the React Native platform resolver (callkeep.servicewith.ios/.androidsuffixes), the mocks and imports should follow suit to prevent brittle test failures.Apply this diff if you switch the implementation to platform-agnostic import:
-jest.mock('../callkeep.service.ios', () => ({ +jest.mock('../callkeep.service', () => ({ callKeepService: { setup: jest.fn(), cleanup: jest.fn(), }, })); -import { callKeepService } from '../callkeep.service.ios'; +import { callKeepService } from '../callkeep.service';Also applies to: 29-33
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (1)
118-126: Optional: Assert the mute-state callback actually toggles the microphone.You verify registration, but not behavior. A small addition can ensure the callback effect is wired correctly.
Example to add:
it('should toggle microphone via mute callback when connected', async () => { const { result } = renderHook(() => useLiveKitCallStore()); // Establish connected state act(() => { result.current.actions._setRoomInstance(mockRoom); result.current.actions._setIsConnected(true); }); await act(async () => { await result.current.actions.connectToRoom('test-room', 'test-participant'); }); // Find the registered callback and invoke it const cb = mockCallKeepService.setMuteStateCallback.mock.calls[0][0] as (muted: boolean) => void; act(() => cb(true)); // muted => disable mic expect(mockRoom.localParticipant.setMicrophoneEnabled).toHaveBeenCalledWith(false); act(() => cb(false)); // unmuted => enable mic expect(mockRoom.localParticipant.setMicrophoneEnabled).toHaveBeenCalledWith(true); });src/features/livekit-call/store/useLiveKitCallStore.ts (1)
95-107: Handle errors from mic/camera enable calls to avoid silent failures.These return Promises and may reject. Add
.catchwith logging to avoid unhandled failures in the connection flow.Apply this diff:
- newRoom.localParticipant.setMicrophoneEnabled(true); - newRoom.localParticipant.setCameraEnabled(false); // No video + newRoom.localParticipant.setMicrophoneEnabled(true).catch((error) => + logger.warn({ message: 'Failed to enable microphone on connect', context: { error, roomId } }) + ); + newRoom.localParticipant.setCameraEnabled(false) // No video + .catch((error) => + logger.warn({ message: 'Failed to disable camera on connect', context: { error, roomId } }) + );src/services/app-initialization.service.ts (1)
135-152: Optional: Guard cleanup on non-iOS or rely on a no-op Android implementation.If the Android module is a no-op, current code is fine. Otherwise, you may early-return on non-iOS to avoid unnecessary calls.
Minimal guard example:
async cleanup(): Promise<void> { try { - // Cleanup CallKeep - await callKeepService.cleanup(); + // Cleanup CallKeep (Android implementation may be a no-op) + await callKeepService.cleanup();Alternatively, if you do not provide an Android stub, guard with
if (Platform.OS === 'ios').
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
__mocks__/react-native-callkeep.ts(2 hunks)src/app/_layout.tsx(2 hunks)src/app/login/login-form.tsx(1 hunks)src/components/ui/__tests__/focus-aware-status-bar.test.tsx(1 hunks)src/components/ui/focus-aware-status-bar.tsx(1 hunks)src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts(6 hunks)src/features/livekit-call/store/useLiveKitCallStore.ts(5 hunks)src/services/__tests__/app-initialization.service.test.ts(1 hunks)src/services/__tests__/callkeep.service.ios.test.ts(3 hunks)src/services/app-initialization.service.ts(1 hunks)src/services/callkeep.service.ios.ts(4 hunks)src/stores/app/__tests__/livekit-store.test.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/login/login-form.tsx
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/components/ui/focus-aware-status-bar.tsxsrc/services/__tests__/app-initialization.service.test.tssrc/app/_layout.tsxsrc/services/callkeep.service.ios.tssrc/services/app-initialization.service.tssrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/features/livekit-call/store/useLiveKitCallStore.tssrc/services/__tests__/callkeep.service.ios.test.tssrc/stores/app/__tests__/livekit-store.test.ts__mocks__/react-native-callkeep.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for React component names (e.g., UserProfile, ChatScreen)
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo() for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Optimize images using react-native-fast-image
Use React Navigation for navigation and deep linking with best practices
Wrap all user-facing text in t() from react-i18next
Use react-hook-form for form handling
Use @rnmapbox/maps for maps or vehicle navigation
Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Use the conditional operator (?:) for conditional rendering instead of &&
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect/useState and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers
Optimize image handling using react-native-fast-image
Wrap all user-facing text in t() from react-i18next
Ensure support for dark mode and light mode
Ensure the app is accessible following WCAG for mobile
Use react-hook-form for form handling
Use @rnmapbox/maps for maps and navigation
Use lucide-react-native for icons directly in markup; do not us...
Files:
src/components/ui/focus-aware-status-bar.tsxsrc/app/_layout.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/components/ui/focus-aware-status-bar.tsxsrc/services/__tests__/app-initialization.service.test.tssrc/app/_layout.tsxsrc/services/callkeep.service.ios.tssrc/services/app-initialization.service.tssrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/features/livekit-call/store/useLiveKitCallStore.tssrc/services/__tests__/callkeep.service.ios.test.tssrc/stores/app/__tests__/livekit-store.test.ts__mocks__/react-native-callkeep.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Build and maintain shared Gluestack UI wrappers in src/components/ui
Files:
src/components/ui/focus-aware-status-bar.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/components/ui/focus-aware-status-bar.tsxsrc/app/_layout.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/services/__tests__/app-initialization.service.test.tssrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/services/__tests__/callkeep.service.ios.test.tssrc/stores/app/__tests__/livekit-store.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/services/__tests__/app-initialization.service.test.tssrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/services/__tests__/callkeep.service.ios.test.tssrc/stores/app/__tests__/livekit-store.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/services/__tests__/app-initialization.service.test.tssrc/components/ui/__tests__/focus-aware-status-bar.test.tsxsrc/services/__tests__/callkeep.service.ios.test.tssrc/stores/app/__tests__/livekit-store.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
🧠 Learnings (1)
📚 Learning: 2025-08-12T03:33:40.227Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.227Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Generate tests for all components, services, and logic; ensure tests run without errors
Applied to files:
src/services/__tests__/app-initialization.service.test.ts
🧬 Code Graph Analysis (7)
src/services/__tests__/app-initialization.service.test.ts (3)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)src/services/callkeep.service.ios.ts (1)
callKeepService(341-341)src/services/app-initialization.service.ts (1)
appInitializationService(156-156)
src/app/_layout.tsx (2)
src/services/app-initialization.service.ts (1)
appInitializationService(156-156)src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)
src/services/callkeep.service.ios.ts (2)
__mocks__/react-native-callkeep.ts (2)
AudioSessionCategoryOption(17-22)AudioSessionMode(24-26)src/lib/logging/index.tsx (2)
error(74-76)logger(80-80)
src/services/app-initialization.service.ts (2)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)src/services/callkeep.service.ios.ts (1)
callKeepService(341-341)
src/components/ui/__tests__/focus-aware-status-bar.test.tsx (1)
src/components/ui/focus-aware-status-bar.tsx (1)
FocusAwareStatusBar(9-60)
src/features/livekit-call/store/useLiveKitCallStore.ts (1)
src/services/callkeep.service.ios.ts (1)
callKeepService(341-341)
src/services/__tests__/callkeep.service.ios.test.ts (2)
src/lib/logging/index.tsx (1)
logger(80-80)src/services/callkeep.service.ios.ts (1)
CallKeepService(19-338)
🔇 Additional comments (18)
src/app/_layout.tsx (2)
29-29: Consolidated storage imports look goodMerging
getDeviceUuidandsetDeviceUuidinto a single named import is cleaner and avoids duplicate import paths.
32-32: App initialization service importImporting the singleton here is appropriate and keeps startup concerns centralized.
src/services/callkeep.service.ios.ts (1)
23-23: Mute-state callback storage is straightforwardStoring the callback as a private field with a nullable type is clear and keeps the surface area small.
src/stores/app/__tests__/livekit-store.test.ts (4)
41-41: Mock surface extended with setMuteStateCallbackIncluding
setMuteStateCallbackin the mock aligns tests with the new public API and avoids undefined reference issues.
396-396: Resetting setMuteStateCallback per testResetting to a no-op ensures isolation between tests and avoids incidental assertion coupling.
406-406: Presence assertion for mute callback methodVerifies the new API is available to the store; good guard against regressions in the mocked surface.
410-420: Setup call verification is appropriate given global initAcknowledging that setup is now global while still ensuring the method is callable in tests keeps this suite robust without duplicating initialization logic.
__mocks__/react-native-callkeep.ts (3)
1-13: Stable default export and richer mock methodsExporting a named
mockMethodsobject improves readability and reusability across tests. The method coverage looks sufficient for current usage.
15-16: Default export updateSwitching to
export default mockMethodsis fine and consistent with typical Jest manual mocks.
17-27: Expose numeric enum values to support assertionsProviding explicit numeric values for
AudioSessionCategoryOptionandAudioSessionModeenables precise assertions on bitmask composition in tests.src/services/__tests__/callkeep.service.ios.test.ts (4)
20-21: Logger.warn added to mocksIncluding
warnsupports verifying error handling in the mute-callback path.
142-173: Mute-callback registration and execution tests are solidGreat coverage for both muted and unmuted paths, and for capturing the handler off the RNCallKeep event wiring.
174-207: Mute-callback error handling test is preciseValidates logging shape and that callback exceptions do not break the handler chain.
209-235: Clearing mute-callback is verifiedGood to see explicit coverage for unregistration semantics.
src/services/__tests__/app-initialization.service.test.ts (1)
43-68: Solid coverage of happy paths, idempotency, concurrent calls, failures, cleanup, reset, and singleton behavior.The scenarios exercised here mirror the service’s contract well. The iOS vs Android branching, idempotency, and in-flight promise handling are verified thoroughly. No issues found with the test logic in these blocks.
Also applies to: 70-85, 87-101, 103-118, 120-136, 158-175, 176-192, 194-214, 216-224
src/features/livekit-call/store/useLiveKitCallStore.ts (1)
72-80: Mute-callback wiring looks good.Callback respects current connection state and delegates to
setMicrophoneEnabled, which already guards and logs. No changes needed.src/services/app-initialization.service.ts (2)
37-63: Idempotent + in-flight dedup logic is correct.The combination of
isInitializedandinitializationPromiseavoids duplicate work across repeated and concurrent calls, with proper error reset for retries.
3-5: Platform-agnostic import unavailable (no callkeep.service.ts found)There is only
callkeep.service.ios.tsinsrc/services, so changing the import toimport { callKeepService } from './callkeep.service';would break (module not found). You can either:
- Create a new
src/services/callkeep.service.tsthat conditionally re-exports the iOS or Android implementation, then switch imports to the generic path.- Or continue using the explicit
./callkeep.service.iosimport and leave platform-guards in place.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app.config.ts (2)
52-55: Confirm TypeScript support for ios.entitlements in ExpoConfigIn a typed app.config.ts, ensure
ios.entitlementsis recognized by the ExpoConfig types you’re importing. If the TS type doesn’t include it in your SDK version, you may hit “Object literal may only specify known properties” onentitlements. If that happens, either upgrade the types, switch the config to app.json(app.config.js), or inject via a config plugin. I can help generate a minimal plugin if needed.
52-55: Ensure gating for restricted critical-alert entitlement
com.apple.developer.usernotifications.critical-alertsis a restricted entitlement that requires explicit Apple approval—enabling it without approval will cause EAS build/signing failures or App Store rejections. By contrast,time-sensitivenotifications don’t require special approval. To avoid surprises, gate the critical-alerts entitlement by environment or a feature flag:entitlements: { 'com.apple.developer.usernotifications.time-sensitive': true, - 'com.apple.developer.usernotifications.critical-alerts': true, + // Only include critical alerts in approved environments + ...(Env.APP_ENV === 'production' && { + 'com.apple.developer.usernotifications.critical-alerts': true, + }), },Or using an explicit flag:
+// env.ts +export const Env = { + …, + ENABLE_CRITICAL_ALERTS: process.env.ENABLE_CRITICAL_ALERTS === 'true', +}; entitlements: { 'com.apple.developer.usernotifications.time-sensitive': true, - 'com.apple.developer.usernotifications.critical-alerts': true, + ...(Env.ENABLE_CRITICAL_ALERTS && { + 'com.apple.developer.usernotifications.critical-alerts': true, + }), },[optional_refactors_recommended]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (1)
app.config.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
app.config.ts
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
app.config.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/ui/focus-aware-status-bar.tsx (1)
14-16: Side-effects now gated by focus — nice fix.Early-returning when not focused prevents off-screen instances from overriding the active screen’s system UI. This addresses the prior review feedback.
🧹 Nitpick comments (9)
src/components/ui/focus-aware-status-bar.tsx (5)
19-40: Avoid double-hiding the navigation bar; rely on SystemBars to manage it.You’re hiding the nav bar imperatively via expo-navigation-bar and also via SystemBars’ hidden property. This duplication can persist a hidden nav bar after this component unmounts. Prefer a single source of truth — SystemBars — to avoid flicker and state leaks.
Apply this diff to remove the imperative nav-bar hiding:
- // Hide navigation bar only on Android - NavigationBar.setVisibilityAsync('hidden').catch(() => { - // Silently handle errors if NavigationBar API is not available - });
56-56: Add a cleanup/blur handler to restore bars when the screen loses focus.Right now, if another screen doesn’t explicitly reset system UI, the nav/status bars may stay hidden. Add a small effect that reverts on blur/unmount.
}, [hidden, colorScheme, isFocused]); + + // Revert UI when screen blurs/unmounts to avoid persistent hidden bars + React.useEffect(() => { + if (isFocused) return; + try { + if (Platform.OS === 'android') { + StatusBar.setHidden(false, 'slide'); + // If you keep the imperative API, ensure visibility is restored on blur + NavigationBar.setVisibilityAsync('visible').catch(() => {}); + } else if (Platform.OS === 'ios') { + StatusBar.setHidden(false, 'slide'); + } + } catch { + // no-op + } + }, [isFocused]);
38-40: Don’t swallow errors entirely — log in development for debuggability.Silent catches make platform issues hard to diagnose. Log in dev without affecting production UX.
- } catch (error) { - // Silently handle errors if StatusBar methods are not available - } + } catch (error) { + if (__DEV__) { + // eslint-disable-next-line no-console + console.warn('FocusAwareStatusBar(android) failed to update system UI:', error); + } + } @@ - } catch (error) { - // Silently handle errors if StatusBar methods are not available - } + } catch (error) { + if (__DEV__) { + // eslint-disable-next-line no-console + console.warn('FocusAwareStatusBar(iOS) failed to update system UI:', error); + } + }Also applies to: 52-54
9-9: Type the component as React.FC to align with repo guidelines.-export const FocusAwareStatusBar = ({ hidden = false }: Props) => { +export const FocusAwareStatusBar: React.FC<Props> = ({ hidden = false }) => {
36-37: Deduplicate style control — prefer SystemBars.style over StatusBar.setBarStyle.You set bar style via both StatusBar.setBarStyle and SystemBars.style. Consolidate to reduce conflicting updates and imperative calls.
- StatusBar.setBarStyle(colorScheme === 'dark' ? 'light-content' : 'dark-content'); + // SystemBars receives `style={colorScheme}` below; rely on that for consistency @@ - StatusBar.setBarStyle(colorScheme === 'dark' ? 'light-content' : 'dark-content'); + // SystemBars receives `style={colorScheme}` below; rely on that for consistencyAlso applies to: 50-51
src/services/__tests__/callkeep.service.ios.test.ts (4)
36-36: Optional: tighten mock typing with jest.mockedYour current cast is fine. If your Jest/TS versions support it and the module has types, you could use jest.mocked for better inference and to avoid a manual cast.
Apply this localized change if desired:
-const mockCallKeep = RNCallKeep as jest.Mocked<typeof RNCallKeep>; +const mockCallKeep = jest.mocked(RNCallKeep);Note: If RNCallKeep lacks type definitions (i.e., is any), keeping your current assertion is safer.
144-175: Remove conditional guard after asserting handler existenceYou already assert the handler is defined. Dropping the if prevents silent passes if the handler is unexpectedly undefined and keeps the test strict.
- // Trigger mute event - if (muteEventHandler) { - muteEventHandler({ muted: true, callUUID: 'test-uuid' }); - expect(mockMuteCallback).toHaveBeenCalledWith(true); - - muteEventHandler({ muted: false, callUUID: 'test-uuid' }); - expect(mockMuteCallback).toHaveBeenCalledWith(false); - } + // Trigger mute event + muteEventHandler!({ muted: true, callUUID: 'test-uuid' }); + expect(mockMuteCallback).toHaveBeenCalledWith(true); + + muteEventHandler!({ muted: false, callUUID: 'test-uuid' }); + expect(mockMuteCallback).toHaveBeenCalledWith(false);
177-210: Same here: remove the if-guard around the handlerLean on the previous assertion and use a non-null assertion to avoid false negatives.
- if (muteEventHandler) { - muteEventHandler({ muted: true, callUUID: 'test-uuid' }); - - expect(errorCallback).toHaveBeenCalledWith(true); - expect(mockLogger.warn).toHaveBeenCalledWith({ - message: 'Failed to execute mute state callback', - context: { - error: expect.any(Error), - muted: true, - callUUID: 'test-uuid' - }, - }); - } + muteEventHandler!({ muted: true, callUUID: 'test-uuid' }); + expect(errorCallback).toHaveBeenCalledWith(true); + expect(mockLogger.warn).toHaveBeenCalledWith({ + message: 'Failed to execute mute state callback', + context: { + error: expect.any(Error), + muted: true, + callUUID: 'test-uuid' + }, + });
212-238: And here: drop the conditional to keep the test strictThis ensures we won't miss a regression where the listener isn't registered.
- if (muteEventHandler) { - muteEventHandler({ muted: true, callUUID: 'test-uuid' }); - - // Callback should not be called after being cleared - expect(mockMuteCallback).not.toHaveBeenCalled(); - } + muteEventHandler!({ muted: true, callUUID: 'test-uuid' }); + // Callback should not be called after being cleared + expect(mockMuteCallback).not.toHaveBeenCalled();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
app.config.ts(1 hunks)src/components/ui/focus-aware-status-bar.tsx(1 hunks)src/services/__tests__/callkeep.service.ios.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app.config.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/services/__tests__/callkeep.service.ios.test.tssrc/components/ui/focus-aware-status-bar.tsx
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/services/__tests__/callkeep.service.ios.test.tssrc/components/ui/focus-aware-status-bar.tsx
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/services/__tests__/callkeep.service.ios.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/services/__tests__/callkeep.service.ios.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/services/__tests__/callkeep.service.ios.test.ts
**/*.tsx
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for React component names (e.g., UserProfile, ChatScreen)
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo() for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Optimize images using react-native-fast-image
Use React Navigation for navigation and deep linking with best practices
Wrap all user-facing text in t() from react-i18next
Use react-hook-form for form handling
Use @rnmapbox/maps for maps or vehicle navigation
Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Use the conditional operator (?:) for conditional rendering instead of &&
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect/useState and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers
Optimize image handling using react-native-fast-image
Wrap all user-facing text in t() from react-i18next
Ensure support for dark mode and light mode
Ensure the app is accessible following WCAG for mobile
Use react-hook-form for form handling
Use @rnmapbox/maps for maps and navigation
Use lucide-react-native for icons directly in markup; do not us...
Files:
src/components/ui/focus-aware-status-bar.tsx
src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Build and maintain shared Gluestack UI wrappers in src/components/ui
Files:
src/components/ui/focus-aware-status-bar.tsx
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/components/ui/focus-aware-status-bar.tsx
🧬 Code Graph Analysis (1)
src/services/__tests__/callkeep.service.ios.test.ts (1)
src/lib/logging/index.tsx (1)
logger(80-80)
🔇 Additional comments (5)
src/components/ui/focus-aware-status-bar.tsx (1)
61-62: Render guard is concise and uses ?: as per guidelines — LGTM.The platform check plus focus guard keep SystemBars rendering scoped to supported, focused screens.
src/services/__tests__/callkeep.service.ios.test.ts (4)
20-21: Good addition: logger.warn mock enables negative-path assertionsThis unblocks validation of error handling in the mute-state callback tests.
25-27: Explicit jest.mock('react-native-callkeep') resolves the native module mocking pitfallImporting after this ensures the manual mock is used during tests. This addresses the earlier review concern about relying on mocks without an explicit mock.
28-30: Importing RNCallKeep after mocks is correctThis guarantees the manual mock is applied and avoids loading the native module in tests.
111-121: Stabilize setup payload assertions (use a typed SetupArg and stringify numeric fields)Add the following
SetupArgtype near the top ofsrc/services/__tests__/callkeep.service.ios.test.ts(e.g. after your mocks):type SetupArg = { ios: { appName: string; maximumCallGroups: string | number; maximumCallsPerCallGroup: string | number; includesCallsInRecents: boolean; supportsVideo: boolean; }; android: { alertTitle: string; }; };Then update your assertions around line 111–121 as follows:
- const setupCall = mockCallKeep.setup.mock.calls[0][0] as any; + const setupCall = mockCallKeep.setup.mock.calls[0][0] as SetupArg; expect(setupCall.ios.appName).toBe('Test App'); - expect(setupCall.ios.maximumCallGroups).toBe('1'); - expect(setupCall.ios.maximumCallsPerCallGroup).toBe('1'); + expect(String(setupCall.ios.maximumCallGroups)).toBe('1'); + expect(String(setupCall.ios.maximumCallsPerCallGroup)).toBe('1'); expect(setupCall.ios.includesCallsInRecents).toBe(false); expect(setupCall.ios.supportsVideo).toBe(false); expect(setupCall.android.alertTitle).toBe('Permissions required');This removes the brittle
anycast, ensures proper type coverage, and guards against future shifts between string vs. number for the iOS numeric fields.
| } 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 | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
iOS: verify Info.plist allows programmatic status-bar control.
StatusBar.setHidden can be ignored if UIViewControllerBasedStatusBarAppearance is true. Confirm your app config (Info.plist or app config) permits programmatic control.
Run:
Expected: The key is present and set appropriately (often false) or your view-controller-based approach matches this imperative usage.
🏁 Script executed:
#!/bin/bash
# Search for the setting across native and Expo config
rg -n -C3 'UIViewControllerBasedStatusBarAppearance|infoPlist' || true
fd -a -t f 'Info.plist' | xargs -r rg -n -C3 'UIViewControllerBasedStatusBarAppearance' || trueLength of output: 978
Enable programmatic status‐bar control by setting UIViewControllerBasedStatusBarAppearance to false
The search shows no UIViewControllerBasedStatusBarAppearance key in your app.config.ts or any native Info.plist, so iOS will default to true and ignore StatusBar.setHidden(...). You need to explicitly disable view-controller–based appearance.
• In app.config.ts, under the existing infoPlist object, add:
infoPlist: {
UIBackgroundModes: ['remote-notification', 'audio', 'bluetooth-central', 'voip'],
ITSAppUsesNonExemptEncryption: false,
NSBluetoothAlwaysUsageDescription: 'Allow Resgrid Unit to connect to bluetooth devices for PTT.',
+ UIViewControllerBasedStatusBarAppearance: false,
},• If you maintain a native Info.plist (in an ios/ folder), add the same key there:
<key>UIViewControllerBasedStatusBarAppearance</key>
<false/>This will ensure StatusBar.setHidden and .setBarStyle calls take effect on iOS.
🤖 Prompt for AI Agents
In src/components/ui/focus-aware-status-bar.tsx around lines 41 to 55: iOS
StatusBar.setHidden/.setBarStyle calls are ignored unless
UIViewControllerBasedStatusBarAppearance is set to false; update your app
configuration and native plist(s) to disable view-controller–based appearance so
the programmatic calls take effect: add the
UIViewControllerBasedStatusBarAppearance key with value false under the existing
infoPlist object in app.config.ts, and if you have an ios/Info.plist in the repo
add the same key/value there; after changing config/plists, rebuild the native
app (or re-run the iOS build) so the new plist setting is applied.
|
Approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is approved.
Summary by CodeRabbit
New Features
Bug Fixes / UI
Tests
Chores