Conversation
|
Task linked: CU-868f4q2qr Fix issues with Call Images |
WalkthroughAdds iOS CallKit integration and mocks, LiveKit store CallKeep wiring and tests, KeyboardAvoidingView and input hints in login settings, a localized audio device selection UI with tests and translations, a bluetooth-audio-store type fix, a small login layout tweak, and a runtime dependency on react-native-callkeep. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI (Join/Leave)
participant Store as useLiveKitCallStore
participant LK as LiveKit Room
participant CK as CallKeepService (iOS)
participant RN as RN CallKeep
UI->>Store: setupCallKeep()
alt iOS
Store->>CK: setup(config)
CK->>RN: setup(ios/android options)
RN-->>CK: resolved
else non-iOS
Store-->>UI: no-op
end
UI->>Store: connectToRoom(roomId)
Store->>LK: connect(token)
LK-->>Store: connected
alt iOS
Store->>CK: startCall(roomId)
CK->>RN: startCall(uuid, handle, ...)
RN-->>CK: ok
end
UI->>Store: disconnectFromRoom()
Store->>LK: disconnect()
LK-->>Store: disconnected
alt iOS
Store->>CK: endCall()
CK->>RN: endCall(uuid)
RN-->>CK: ok
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (28)
src/app/login/login-form.tsx (2)
176-180: Use ternary instead of && for conditional rendering (project guideline)The codebase guideline prefers the conditional operator over &&. Replace the && block with a ternary.
- {onServerUrlPress && ( + {onServerUrlPress ? ( <Button className="mt-14 w-full" variant="outline" action="secondary" onPress={onServerUrlPress}> <ButtonText>{t('settings.server_url')}</ButtonText> </Button> - )} + ) : null}
177-177: Spacing change to mt-14: double-check small-screen layoutBumping to mt-14 significantly increases vertical spacing. Ensure this doesn’t push the button below the fold on smaller devices.
src/components/settings/login-info-bottom-sheet.tsx (3)
56-58: Good call adding KeyboardAvoidingView; consider a keyboardVerticalOffsetOn iOS, fixed headers/sheet handles often require an offset to prevent input occlusion. Use safe area inset to compute a reliable offset.
- <KeyboardAvoidingView behavior={Platform.OS === 'ios' ? 'padding' : 'height'} className="w-full"> + <KeyboardAvoidingView behavior={Platform.OS === 'ios' ? 'padding' : 'height'} keyboardVerticalOffset={top + 64} className="w-full">Additionally, add this import and hook:
// At top-level imports import { useSafeAreaInsets } from 'react-native-safe-area-context'; // Inside component (near other hooks) const { top } = useSafeAreaInsets();
68-71: Improve UX: submit on Return and hint IMELet users submit from the username field via the keyboard. This also helps tests and accessibility.
- <InputField value={value} onChangeText={onChange} placeholder={t('settings.enter_username')} autoCapitalize="none" autoCorrect={false} textContentType="username" autoComplete="username" /> + <InputField + value={value} + onChangeText={onChange} + placeholder={t('settings.enter_username')} + autoCapitalize="none" + autoCorrect={false} + textContentType="username" + autoComplete="username" + returnKeyType="next" + onSubmitEditing={() => {/* optionally focus password field here */}} + />
85-93: Password field: ensure submit on Return and secure entry across platformsType="password" is great; adding secureTextEntry guards Android/older RN implementations. Also enable submitting on Return.
<InputField value={value} onChangeText={onChange} placeholder={t('settings.enter_password')} type="password" autoCapitalize="none" autoCorrect={false} textContentType="password" autoComplete="password" + secureTextEntry + returnKeyType="done" + onSubmitEditing={handleSubmit(onFormSubmit)} />__mocks__/react-native-callkeep.ts (1)
2-6: Align mock method signatures with library (sync vs async)In the real API, several methods (e.g., setup, startCall, reportConnecting..., reportConnected..., endCall) are synchronous. Using mockResolvedValue makes them async in tests unnecessarily.
- setup: jest.fn().mockResolvedValue(undefined), - startCall: jest.fn().mockResolvedValue(undefined), - reportConnectingOutgoingCallWithUUID: jest.fn().mockResolvedValue(undefined), - reportConnectedOutgoingCallWithUUID: jest.fn().mockResolvedValue(undefined), - endCall: jest.fn().mockResolvedValue(undefined), + setup: jest.fn(), + startCall: jest.fn(), + reportConnectingOutgoingCallWithUUID: jest.fn(), + reportConnectedOutgoingCallWithUUID: jest.fn(), + endCall: jest.fn(),src/services/callkeep.service.ios.ts (5)
53-75: Consider awaiting setup to guarantee readiness before useIf RNCallKeep.setup resolves asynchronously on some platforms/versions, awaiting it ensures listeners/state are initialized deterministically before startCall is invoked.
- RNCallKeep.setup(options); + await RNCallKeep.setup(options as any); this.setupEventListeners(); this.isSetup = true;
106-113: Preemptively end any existing call if a UUID exists (covers in-progress state)The current check only ends when isCallActive is true. If startCall is called twice quickly, the first call may not have flipped isCallActive yet, leaving an orphaned system call.
- if (this.isCallActive && currentCallUUID) { + if (currentCallUUID) { logger.debug({ - message: 'Call already active, ending existing call first', + message: 'Existing call UUID found, ending before starting a new one', context: { currentCallUUID }, }); await this.endCall(); }
270-276: Use a stronger UUID source when availablePrefer crypto.randomUUID() when available; fall back to the current impl.
private generateUUID(): string { - return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) { - const r = (Math.random() * 16) | 0; - const v = c === 'x' ? r : (r & 0x3) | 0x8; - return v.toString(16); - }); + // RN 0.76 typically provides global crypto.randomUUID via Hermes/JSI + const rndUUID = (global as any)?.crypto?.randomUUID?.(); + if (typeof rndUUID === 'string') return rndUUID; + // Fallback + return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, (c) => { + const r = (Math.random() * 16) | 0; + const v = c === 'x' ? r : (r & 0x3) | 0x8; + return v.toString(16); + }); }
7-9: Optional: make currentCallUUID an instance fieldThe singleton already scopes state; moving currentCallUUID into the class improves encapsulation and testability.
If you’d like, I can provide a patch to convert currentCallUUID into a private instance property and adjust references accordingly.
3-3: Nit: remove unused importsAudioSessionCategoryOption, AudioSessionMode, and CK_CONSTANTS are imported but not used.
-import RNCallKeep, { AudioSessionCategoryOption, AudioSessionMode, CONSTANTS as CK_CONSTANTS } from 'react-native-callkeep'; +import RNCallKeep from 'react-native-callkeep';src/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsx (3)
2-5: Consolidate Platform mocking and preserve React Native primitivesYou’re mocking Platform twice (once via the internal RN path and again via a full
react-nativemock), and the latter replaces the entire RN module without preserving core components like View/Text/TextInput. This can introduce brittle behavior and break host components. Prefer a singlereact-nativemock that spreads the real module and overrides only what’s needed.Apply this diff to remove the internal Platform mock and switch to a safe, consolidated mock that preserves RN primitives:
-// 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), -})); +// Consolidated RN mock that preserves actual components and overrides only what's needed @@ -jest.mock('react-native', () => ({ - useWindowDimensions: () => ({ - width: 400, - height: 800, - }), - Platform: { - OS: 'ios', - select: jest.fn().mockImplementation((obj) => obj.ios || obj.default), - }, - KeyboardAvoidingView: ({ children, ...props }: any) => { - const React = require('react'); - return React.createElement('View', { testID: 'keyboard-avoiding-view', ...props }, children); - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + useWindowDimensions: () => ({ + width: 400, + height: 800, + }), + Platform: { + ...RN.Platform, + OS: 'ios', + select: jest.fn().mockImplementation((obj) => obj.ios ?? obj.default), + }, + KeyboardAvoidingView: ({ children, ...props }: any) => { + const React = require('react'); + return React.createElement(RN.View, { testID: 'keyboard-avoiding-view', ...props }, children); + }, + }; +});Also applies to: 25-38
214-221: Make the Cancel press assertion less brittleAccessing
.parentof the text node relies on test-renderer internals. Prefer selecting the interactive element directly. Since the Button mock already forwards props, expose a testID on the cancel button for a stable query.For example, in the component, set
testID="cancel-button"on the Cancel Button. Then in the test:-const cancelButton = screen.getByText('common.cancel').parent; -fireEvent.press(cancelButton); +fireEvent.press(screen.getByTestId('cancel-button'));If you don’t want to touch the component, at least query the nearest “button” testID instance instead of relying on
.parent.
1-223: Avoid test duplication with the sibling test fileThis test file appears effectively identical to src/components/settings/tests/login-info-bottom-sheet.test.tsx. Duplicated suites increase runtime and maintenance load without adding coverage. Consider keeping only one suite and moving the shared mocking to a small test utility for reuse.
I can help factor the shared mocks into a reusable test utils module if you want.
src/components/settings/__tests__/login-info-bottom-sheet.test.tsx (3)
2-5: Unify RN mocking to avoid brittle behaviorSame concern as the “simple” variant: double-mocking Platform and fully replacing the RN module risks breaking primitives. Spread the actual RN module and override only what’s needed.
Apply the same consolidated mock approach suggested in the sibling file to this one as well.
Also applies to: 25-38
214-221: Press handler selection could be more robustUsing
.parentto find the pressable is brittle. Prefer attaching a testID to the Cancel button and firing the press on it directly, or enhance the Button mock to accept and forward an accessibilityRole="button" and query by role.
1-223: Test duplication with the “simple” suiteThis file largely duplicates src/components/settings/tests/login-info-bottom-sheet-simple.test.tsx. Please consolidate to a single suite and extract the common mock scaffolding.
src/features/livekit-call/store/useLiveKitCallStore.ts (4)
5-6: Don’t import an iOS-specific file from cross-platform store codeImporting
callkeep.service.iosdirectly in a shared store can complicate Android builds and bundling. Prefer a platform-agnostic import path with.ios.tsand.android.tssiblings (the latter being a no-op stub), or lazy-load inside an iOS guard.Apply this diff to move to a platform-agnostic import:
-import { callKeepService } from '../../../services/callkeep.service.ios'; +import { callKeepService } from '../../../services/callkeep.service';And add a stub at src/services/callkeep.service.android.ts:
// src/services/callkeep.service.android.ts export const callKeepService = { setup: async (_config?: unknown) => {}, startCall: async (_roomId: string, _handle?: string) => '', endCall: async () => {}, isCallActiveNow: () => false, getCurrentCallUUID: () => null as string | null, cleanup: async () => {}, };Alternatively, dynamically import under the iOS guard:
if (Platform.OS === 'ios') { const { callKeepService } = await import('../../../services/callkeep.service.ios'); await callKeepService.setup({...}); }
138-148: Start CallKeep call on connect is reasonable, but consider display handleUsing
roomIdis fine for now. If you have a human-readable room name, consider passing it as the call handle to improve iOS Call UI clarity.
160-170: Potential double endCall on disconnectYou end the CallKeep call both in the ConnectionState.Disconnected handler and again in disconnectFromRoom(). The service should no-op safely when no active call exists, but this can add log noise.
If you prefer to avoid duplicate calls, you can rely solely on one path (e.g., keep it in the ConnectionState handler) or guard with
callKeepService.isCallActiveNow():-// End CallKeep call for iOS -if (Platform.OS === 'ios') { - try { - await callKeepService.endCall(); - console.log('CallKeep call ended successfully'); - } catch (error) { - console.warn('Failed to end CallKeep call:', error); - } -} +// End CallKeep call for iOS (only if still active to avoid duplicate logs) +if (Platform.OS === 'ios' && callKeepService.isCallActiveNow()) { + try { + await callKeepService.endCall(); + console.log('CallKeep call ended successfully'); + } catch (error) { + console.warn('Failed to end CallKeep call:', error); + } +}Also applies to: 248-256
103-129: Prefer project logger over console for consistencyElsewhere you’re using a structured logger; using console.* here diverges from that pattern and can reduce observability in production.
If you have a logger at
src/lib/logging, import and replace console.*:+import { logger } from '../../../lib/logging'; - console.warn('Connection attempt while already connecting or connected.'); + logger.warn?.({ message: 'Connection attempt while already connecting or connected.' }); - console.log('LiveKit Connection State Changed:', state); + logger.debug?.({ message: 'LiveKit Connection State Changed', context: { state } }); - console.error('Failed to connect to LiveKit room:', err); + logger.error?.({ message: 'Failed to connect to LiveKit room', context: { error: err } });Also applies to: 200-206, 215-228
src/stores/app/__tests__/livekit-store.test.ts (2)
122-127: Preserve RN module and override only PlatformMocking the entire
react-nativemodule with only Platform can break other imports if this test grows. Safer to spread the actual module and override Platform.OS.Apply this diff:
-jest.mock('react-native', () => ({ - Platform: { - OS: 'android', - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + Platform: { ...RN.Platform, OS: 'android' }, + }; +});
382-451: “CallKeep Integration” block isn’t exercising store behaviorThese tests confirm the mock’s shape and basic calls, but they don’t exercise any behavior of the store under test. Consider moving this block to the CallKeep service tests (where it’s more relevant) or reworking it to invoke store actions that actually use CallKeep (e.g.,
setupCallKeep()or connect/disconnect flows in the LiveKit call store).To make this meaningful here, assert platform gating by calling the relevant store action and verifying the mock wasn’t invoked on Android:
-it('should skip CallKeep operations on non-iOS platforms', async () => { - (Platform as any).OS = 'android'; - // Verify that platform checks work as expected - expect(Platform.OS).toBe('android'); - // CallKeep operations would be skipped on Android - // This test confirms the platform detection works properly -}); +it('should skip setupCallKeep on Android', async () => { + (Platform as any).OS = 'android'; + const { setupCallKeep } = require('../../../features/livekit-call/store/useLiveKitCallStore').useLiveKitCallStore.getState().actions; + await setupCallKeep(); + expect(mockCallKeepService.setup).not.toHaveBeenCalled(); +});src/services/__tests__/callkeep.service.ios.test.ts (1)
244-255: Singleton tests look good; consider a cleanup testAs an optional enhancement, add a test for
cleanup()to ensure event listeners are removed and internal flags reset.I can draft this if useful.
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (4)
56-57: Inconsistent variable naming.The variable
originalEnvis declared outside thebeforeAllhook but only used within test lifecycle hooks. This same pattern is repeated in the error handling test (line 513).Move the declaration inside
beforeAll:describe('useLiveKitCallStore with CallKeep Integration', () => { - // Mock environment variable for successful token fetching - const originalEnv = process.env.STORYBOOK_LIVEKIT_TOKEN; beforeAll(() => { + const originalEnv = process.env.STORYBOOK_LIVEKIT_TOKEN; process.env.STORYBOOK_LIVEKIT_TOKEN = 'mock-test-token'; - }); - - afterAll(() => { - if (originalEnv) { - process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv; - } else { - delete process.env.STORYBOOK_LIVEKIT_TOKEN; - } + + return () => { + if (originalEnv) { + process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv; + } else { + delete process.env.STORYBOOK_LIVEKIT_TOKEN; + } + }; });
147-155: Mock implementation doesn't cover all event types.The mock only handles
connectionStateChangedevents, but the store implementation subscribes to multiple RoomEvent types. This could lead to incomplete test coverage or missed edge cases.Consider implementing a more comprehensive mock that handles all event types:
beforeEach(() => { // Mock successful connection flow + const eventListeners = new Map<string, Function>(); mockRoom.on.mockImplementation((event: any, callback: any) => { - if (event === 'connectionStateChanged') { - // Simulate connected state - setTimeout(() => callback('connected'), 0); - } + eventListeners.set(event, callback); + // Simulate connected state for connectionStateChanged + if (event === 'connectionStateChanged') { + setTimeout(() => callback('connected'), 0); + } return mockRoom; }); });
184-200: Test doesn't verify that the error is properly set in the store.The test handles CallKeep start call errors but only verifies console output. It should also check if the error is reflected in the store state since the implementation seems to continue despite the error.
Add assertion to verify the store's error state remains unaffected (since this is a non-critical error):
expect(consoleSpy).toHaveBeenCalledWith( 'Failed to start CallKeep call (background audio may not work):', error ); + // Verify that the error doesn't affect the store state (non-critical error) + expect(result.current.error).toBeNull(); + expect(result.current.isConnected).toBe(true); consoleSpy.mockRestore();
300-307: Test relies on timing assumptions that could cause flakiness.The test checks if
connectionStateListeneris defined and then calls it, but there's a potential race condition if the mock implementation changes or if the async operations take longer than expected.Add a more robust wait mechanism:
expect(connectionStateListener).toBeDefined(); // Simulate disconnection - if (connectionStateListener) { - act(() => { - connectionStateListener!('disconnected'); - }); - } + await act(async () => { + if (!connectionStateListener) { + throw new Error('Connection state listener was not set up'); + } + connectionStateListener('disconnected'); + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + }); expect(mockCallKeepService.endCall).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 ignored due to path filters (47)
.DS_Storeis excluded by!**/.DS_Storeassets/audio/callclosed.wavis excluded by!**/*.wavassets/audio/callemergency.wavis excluded by!**/*.wavassets/audio/callhigh.wavis excluded by!**/*.wavassets/audio/calllow.wavis excluded by!**/*.wavassets/audio/callmedium.wavis excluded by!**/*.wavassets/audio/callupdated.wavis excluded by!**/*.wavassets/audio/custom/c1.wavis excluded by!**/*.wavassets/audio/custom/c10.wavis excluded by!**/*.wavassets/audio/custom/c11.wavis excluded by!**/*.wavassets/audio/custom/c12.wavis excluded by!**/*.wavassets/audio/custom/c13.wavis excluded by!**/*.wavassets/audio/custom/c14.wavis excluded by!**/*.wavassets/audio/custom/c15.wavis excluded by!**/*.wavassets/audio/custom/c16.wavis excluded by!**/*.wavassets/audio/custom/c17.wavis excluded by!**/*.wavassets/audio/custom/c18.wavis excluded by!**/*.wavassets/audio/custom/c19.wavis excluded by!**/*.wavassets/audio/custom/c2.wavis excluded by!**/*.wavassets/audio/custom/c20.wavis excluded by!**/*.wavassets/audio/custom/c21.wavis excluded by!**/*.wavassets/audio/custom/c22.wavis excluded by!**/*.wavassets/audio/custom/c23.wavis excluded by!**/*.wavassets/audio/custom/c24.wavis excluded by!**/*.wavassets/audio/custom/c25.wavis excluded by!**/*.wavassets/audio/custom/c3.wavis excluded by!**/*.wavassets/audio/custom/c4.wavis excluded by!**/*.wavassets/audio/custom/c5.wavis excluded by!**/*.wavassets/audio/custom/c6.wavis excluded by!**/*.wavassets/audio/custom/c7.wavis excluded by!**/*.wavassets/audio/custom/c8.wavis excluded by!**/*.wavassets/audio/custom/c9.wavis excluded by!**/*.wavassets/audio/newcall.wavis excluded by!**/*.wavassets/audio/newchat.wavis excluded by!**/*.wavassets/audio/newmessage.wavis excluded by!**/*.wavassets/audio/newshift.wavis excluded by!**/*.wavassets/audio/newtraining.wavis excluded by!**/*.wavassets/audio/notification.wavis excluded by!**/*.wavassets/audio/personnelstaffingupdated.wavis excluded by!**/*.wavassets/audio/personnelstatusupdated.wavis excluded by!**/*.wavassets/audio/troublealert.wavis excluded by!**/*.wavassets/audio/unitnotice.wavis excluded by!**/*.wavassets/audio/unitstatusupdated.wavis excluded by!**/*.wavassets/audio/upcomingshift.wavis excluded by!**/*.wavassets/audio/upcomingtraining.wavis excluded by!**/*.wavassets/icon.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (11)
__mocks__/react-native-callkeep.ts(1 hunks)package.json(1 hunks)src/app/login/login-form.tsx(1 hunks)src/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsx(1 hunks)src/components/settings/__tests__/login-info-bottom-sheet.test.tsx(1 hunks)src/components/settings/login-info-bottom-sheet.tsx(2 hunks)src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts(1 hunks)src/features/livekit-call/store/useLiveKitCallStore.ts(6 hunks)src/services/__tests__/callkeep.service.ios.test.ts(1 hunks)src/services/callkeep.service.ios.ts(1 hunks)src/stores/app/__tests__/livekit-store.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/components/settings/__tests__/login-info-bottom-sheet.test.tsxsrc/app/login/login-form.tsxsrc/services/__tests__/callkeep.service.ios.test.tssrc/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsxsrc/components/settings/login-info-bottom-sheet.tsx__mocks__/react-native-callkeep.tssrc/services/callkeep.service.ios.tssrc/stores/app/__tests__/livekit-store.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.tssrc/features/livekit-call/store/useLiveKitCallStore.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/settings/__tests__/login-info-bottom-sheet.test.tsxsrc/app/login/login-form.tsxsrc/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsxsrc/components/settings/login-info-bottom-sheet.tsx
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/components/settings/__tests__/login-info-bottom-sheet.test.tsxsrc/app/login/login-form.tsxpackage.jsonsrc/services/__tests__/callkeep.service.ios.test.tssrc/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsxsrc/components/settings/login-info-bottom-sheet.tsx__mocks__/react-native-callkeep.tssrc/services/callkeep.service.ios.tssrc/stores/app/__tests__/livekit-store.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.tssrc/features/livekit-call/store/useLiveKitCallStore.ts
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/components/settings/__tests__/login-info-bottom-sheet.test.tsxsrc/services/__tests__/callkeep.service.ios.test.tssrc/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsxsrc/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/components/settings/__tests__/login-info-bottom-sheet.test.tsxsrc/services/__tests__/callkeep.service.ios.test.tssrc/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsxsrc/stores/app/__tests__/livekit-store.test.tssrc/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/components/settings/__tests__/login-info-bottom-sheet.test.tsxsrc/app/login/login-form.tsxsrc/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsxsrc/components/settings/login-info-bottom-sheet.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/components/settings/__tests__/login-info-bottom-sheet.test.tsxsrc/services/__tests__/callkeep.service.ios.test.tssrc/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsxsrc/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 **/*.tsx : Use react-hook-form for form handling
Applied to files:
src/components/settings/login-info-bottom-sheet.tsx
🧬 Code Graph Analysis (8)
src/components/settings/__tests__/login-info-bottom-sheet.test.tsx (1)
src/components/settings/login-info-bottom-sheet.tsx (1)
LoginInfoBottomSheet(25-112)
src/app/login/login-form.tsx (1)
src/components/ui/button/index.tsx (1)
Button(334-334)
src/services/__tests__/callkeep.service.ios.test.ts (2)
src/lib/logging/index.tsx (1)
logger(80-80)src/services/callkeep.service.ios.ts (2)
CallKeepService(19-310)callKeepService(313-313)
src/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsx (1)
src/components/settings/login-info-bottom-sheet.tsx (1)
LoginInfoBottomSheet(25-112)
src/components/settings/login-info-bottom-sheet.tsx (2)
src/components/ui/input/index.tsx (2)
Input(174-174)InputField(174-174)src/components/ui/button/index.tsx (3)
Button(334-334)ButtonText(334-334)ButtonSpinner(334-334)
src/services/callkeep.service.ios.ts (1)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (1)
src/features/livekit-call/store/useLiveKitCallStore.ts (1)
useLiveKitCallStore(70-303)
src/features/livekit-call/store/useLiveKitCallStore.ts (1)
src/services/callkeep.service.ios.ts (1)
callKeepService(313-313)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (14)
package.json (1)
147-147: Ensure iOS infoPlist includes microphone usage and verify VoIP modeYour app.config.ts already defines UIBackgroundModes (
remote-notification,audio,bluetooth-central,voip) and NSBluetoothAlwaysUsageDescription, but you’re missing the required microphone permission string and should confirm whether you really need “voip” background mode.• File: app.config.ts (around lines 47–51)
– Add
"NSMicrophoneUsageDescription": "Explain why your app needs microphone access for calls."
– If you’re not using PushKit/PKPushRegistry for incoming VoIP pushes, remove"voip"from UIBackgroundModes and keep only["audio"]to avoid App Store issues.Suggested diff:
infoPlist: { - UIBackgroundModes: ['remote-notification','audio','bluetooth-central','voip'], + UIBackgroundModes: ['remote-notification','audio','bluetooth-central'], + NSMicrophoneUsageDescription: 'Your app needs microphone access to make and receive calls.', ITSAppUsesNonExemptEncryption: false, NSBluetoothAlwaysUsageDescription: 'Allow Resgrid Unit to connect to bluetooth devices for PTT.', },Likely an incorrect or invalid review comment.
src/components/settings/login-info-bottom-sheet.tsx (1)
56-109: LGTM: Nice keyboard handling and input hintsThe KeyboardAvoidingView and text input hints (autoCapitalize/autoComplete/textContentType) materially improve usability. Button sizing by orientation is a nice touch.
__mocks__/react-native-callkeep.ts (1)
1-13: LGTM: Comprehensive CallKeep mockThe mock covers the surface needed by the service and tests and exports named constants to satisfy imports.
src/components/settings/__tests__/login-info-bottom-sheet-simple.test.tsx (2)
143-166: Solid coverage of rendering and propsGood assertions on Actionsheet presence, KeyboardAvoidingView, and i18n label rendering under the iOS mock. This meaningfully guards the regression surface around recent KAV and input prop changes.
173-184: Nice verification of input field ergonomicsYou thoroughly assert the input props for username and password (autoCapitalize, autoCorrect, autoComplete, textContentType, secureTextEntry, placeholders). This is pragmatic coverage for mobile UX concerns.
Also applies to: 186-198
src/components/settings/__tests__/login-info-bottom-sheet.test.tsx (3)
157-165: Good: iOS rendering behavior is assertedThe checks for Actionsheet/Content and KeyboardAvoidingView presence under iOS are on point.
173-184: Good: Input props assertions cover key UX contractsVerifying username/password props here mirrors the component’s intended ergonomics.
Also applies to: 186-198
200-205: iOS-specific KAV behavior coveredAsserting behavior="padding" under iOS mock helps prevent regressions when platform logic is refactored.
src/features/livekit-call/store/useLiveKitCallStore.ts (1)
82-98: setupCallKeep is correctly gated and error-handledThe iOS guard and try/catch with user-facing error state are appropriate. This makes the action idempotent per the service’s own semantics.
src/stores/app/__tests__/livekit-store.test.ts (1)
335-344: Good: unsupported platform handling verifiedAsserting that nothing is called and no logs are emitted on an unsupported platform is pragmatic and keeps the permission logic focused.
src/services/__tests__/callkeep.service.ios.test.ts (4)
97-132: Setup configuration and logging are well-verifiedAsserting the full RNCallKeep.setup payload and structured logs (info) provides solid guarantees around initialization behavior.
134-149: Event listener wiring coveredGood checks for all expected listeners, including audio session activation/deactivation and mute state actions.
167-197: Start call flow assertions are thoroughYou validate UUID format, RNCallKeep.startCall arguments, and the “connecting” report. The custom handle case is a nice addition.
215-241: End call behaviors are well-exercisedVerifying proper endCall arguments, state reset, and the “No active call to end” path increases confidence in teardown logic.
| const originalEnv = process.env.STORYBOOK_LIVEKIT_TOKEN; | ||
| delete process.env.STORYBOOK_LIVEKIT_TOKEN; | ||
|
|
||
| const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); | ||
|
|
||
| const { result } = renderHook(() => useLiveKitCallStore()); | ||
|
|
||
| await act(async () => { | ||
| await result.current.actions.connectToRoom('test-room', 'test-participant'); | ||
| }); | ||
|
|
||
| expect(result.current.error).toBe('Failed to fetch a valid connection token.'); | ||
| expect(result.current.isConnecting).toBe(false); | ||
| expect(result.current.isConnected).toBe(false); | ||
| expect(result.current.roomInstance).toBeNull(); | ||
| expect(result.current.currentRoomId).toBeNull(); | ||
|
|
||
| // Restore original environment | ||
| if (originalEnv) { | ||
| process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv; | ||
| } | ||
|
|
||
| consoleSpy.mockRestore(); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Test has cleanup issues with environment variables.
The test manipulates process.env.STORYBOOK_LIVEKIT_TOKEN but could interfere with other tests if it fails before restoration. This is particularly problematic since the describe block's afterAll won't run if this specific test fails.
Use a try-finally block or afterEach hook to ensure cleanup:
it('should handle token fetch errors', async () => {
// Mock environment to simulate missing token
const originalEnv = process.env.STORYBOOK_LIVEKIT_TOKEN;
- delete process.env.STORYBOOK_LIVEKIT_TOKEN;
-
- const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
-
- const { result } = renderHook(() => useLiveKitCallStore());
-
- await act(async () => {
- await result.current.actions.connectToRoom('test-room', 'test-participant');
- });
-
- expect(result.current.error).toBe('Failed to fetch a valid connection token.');
- expect(result.current.isConnecting).toBe(false);
- expect(result.current.isConnected).toBe(false);
- expect(result.current.roomInstance).toBeNull();
- expect(result.current.currentRoomId).toBeNull();
-
- // Restore original environment
- if (originalEnv) {
- process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv;
- }
-
- consoleSpy.mockRestore();
+ try {
+ delete process.env.STORYBOOK_LIVEKIT_TOKEN;
+
+ const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+
+ const { result } = renderHook(() => useLiveKitCallStore());
+
+ await act(async () => {
+ await result.current.actions.connectToRoom('test-room', 'test-participant');
+ });
+
+ expect(result.current.error).toBe('Failed to fetch a valid connection token.');
+ expect(result.current.isConnecting).toBe(false);
+ expect(result.current.isConnected).toBe(false);
+ expect(result.current.roomInstance).toBeNull();
+ expect(result.current.currentRoomId).toBeNull();
+
+ consoleSpy.mockRestore();
+ } finally {
+ // Always restore original environment
+ if (originalEnv) {
+ process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv;
+ }
+ }
});📝 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 originalEnv = process.env.STORYBOOK_LIVEKIT_TOKEN; | |
| delete process.env.STORYBOOK_LIVEKIT_TOKEN; | |
| const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); | |
| const { result } = renderHook(() => useLiveKitCallStore()); | |
| await act(async () => { | |
| await result.current.actions.connectToRoom('test-room', 'test-participant'); | |
| }); | |
| expect(result.current.error).toBe('Failed to fetch a valid connection token.'); | |
| expect(result.current.isConnecting).toBe(false); | |
| expect(result.current.isConnected).toBe(false); | |
| expect(result.current.roomInstance).toBeNull(); | |
| expect(result.current.currentRoomId).toBeNull(); | |
| // Restore original environment | |
| if (originalEnv) { | |
| process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv; | |
| } | |
| consoleSpy.mockRestore(); | |
| }); | |
| it('should handle token fetch errors', async () => { | |
| // Mock environment to simulate missing token | |
| const originalEnv = process.env.STORYBOOK_LIVEKIT_TOKEN; | |
| try { | |
| delete process.env.STORYBOOK_LIVEKIT_TOKEN; | |
| const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); | |
| const { result } = renderHook(() => useLiveKitCallStore()); | |
| await act(async () => { | |
| await result.current.actions.connectToRoom('test-room', 'test-participant'); | |
| }); | |
| expect(result.current.error).toBe('Failed to fetch a valid connection token.'); | |
| expect(result.current.isConnecting).toBe(false); | |
| expect(result.current.isConnected).toBe(false); | |
| expect(result.current.roomInstance).toBeNull(); | |
| expect(result.current.currentRoomId).toBeNull(); | |
| consoleSpy.mockRestore(); | |
| } finally { | |
| // Always restore original environment | |
| if (originalEnv) { | |
| process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv; | |
| } | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts around
lines 513 to 536, the test removes process.env.STORYBOOK_LIVEKIT_TOKEN and sets
up a console spy but only restores them at the end of the test body, which can
leak state if the test throws; wrap the test actions in a try-finally (or move
cleanup to an afterEach) so that in the finally block you always restore
process.env.STORYBOOK_LIVEKIT_TOKEN to the previously saved originalEnv (or
delete it if originalEnv was undefined) and call consoleSpy.mockRestore(),
ensuring no environment or spy leakage even on failures.
| export interface CallKeepConfig { | ||
| appName: string; | ||
| maximumCallGroups: string; | ||
| maximumCallsPerCallGroup: string; | ||
| includesCallsInRecents: boolean; | ||
| supportsVideo: boolean; | ||
| ringtoneSound?: string; | ||
| } |
There was a problem hiding this comment.
Type mismatch: maximumCallGroups and maximumCallsPerCallGroup should be numbers
RNCallKeep iOS options expect numeric values. Using strings can cause native type mismatches at runtime.
export interface CallKeepConfig {
appName: string;
- maximumCallGroups: string;
- maximumCallsPerCallGroup: string;
+ maximumCallGroups: number;
+ maximumCallsPerCallGroup: number;
includesCallsInRecents: boolean;
supportsVideo: boolean;
ringtoneSound?: 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.
| export interface CallKeepConfig { | |
| appName: string; | |
| maximumCallGroups: string; | |
| maximumCallsPerCallGroup: string; | |
| includesCallsInRecents: boolean; | |
| supportsVideo: boolean; | |
| ringtoneSound?: string; | |
| } | |
| export interface CallKeepConfig { | |
| appName: string; | |
| maximumCallGroups: number; | |
| maximumCallsPerCallGroup: number; | |
| includesCallsInRecents: boolean; | |
| supportsVideo: boolean; | |
| ringtoneSound?: string; | |
| } |
🤖 Prompt for AI Agents
In src/services/callkeep.service.ios.ts around lines 10 to 17, the
CallKeepConfig interface declares maximumCallGroups and maximumCallsPerCallGroup
as strings but RNCallKeep iOS expects numbers; change both properties to type
number and update any places that construct or pass this config to ensure
numeric values (parse or convert strings where necessary) so the native bridge
receives numbers, and add/update tests or TypeScript callers to reflect the new
types.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
src/components/livekit/livekit-bottom-sheet.tsx (2)
219-219: Prefer a localized fallback instead of an empty string for room nameUsing an empty fallback hides useful context and can be confusing. Align with other UI fallbacks and i18n:
- <Text className="text-center text-lg font-medium">{currentRoomInfo?.Name || ''}</Text> + <Text className="text-center text-lg font-medium"> + {currentRoomInfo?.Name ?? t('common.unknown')} + </Text>
267-268: Avoid fixed-width spacer to keep layout robust across locales and font scalesA hard-coded width can break with long translations or dynamic font sizes. Use an invisible placeholder that matches the actual back label width so the title stays centered consistently.
Apply this minimal change:
- <View style={styles.spacer} /> + <Text style={styles.invisibleBackPlaceholder}>{t('common.back')}</Text>And update the style:
- spacer: { - width: 50, - }, + invisibleBackPlaceholder: { + opacity: 0, + },This reserves the exact space of the back label in the current locale without assuming a fixed width.
Also applies to: 354-356
src/components/settings/__tests__/audio-device-selection.test.tsx (4)
148-160: Avoid brittle DOM traversal by using stable testIDs on pressable itemsRelying on parent?.parent?.parent is fragile and may break with minor layout changes. Prefer pressing the list item via a stable testID exposed by the component.
Apply this test change after adding a testID to the Pressable (see component review):
- const { getAllByText } = render(<AudioDeviceSelection />); - - // Find the first device card (should be in microphone section) - const deviceCards = getAllByText('Bluetooth Headset'); - fireEvent.press(deviceCards[0].parent?.parent?.parent as any); + render(<AudioDeviceSelection />); + const pressable = screen.getByTestId(`audio-device-item-microphone-${bluetoothMic.id}`); + fireEvent.press(pressable);If you want, I can update all affected tests after we add the testID in the component.
162-174: Same here: use testIDs for speaker item pressMirror the microphone test to avoid layout-coupled traversal.
- const { getAllByText } = render(<AudioDeviceSelection />); - - // Find the second device card (should be in speaker section) - const deviceCards = getAllByText('Bluetooth Speaker'); - fireEvent.press(deviceCards[1].parent?.parent?.parent as any); + render(<AudioDeviceSelection />); + const pressable = screen.getByTestId(`audio-device-item-speaker-${bluetoothSpeaker.id}`); + fireEvent.press(pressable);
194-205: Add a test to assert the “Unavailable” suffix for non-Bluetooth devicesYou cover filtering for unavailable bluetooth devices, but not that non-bluetooth unavailable devices still render with the “(Unavailable)” suffix, as the component currently does.
Consider adding:
+ it('shows "(Unavailable)" suffix for unavailable non-bluetooth devices in microphone list', () => { + const wiredUnavailable = createMockDevice('wired-u', 'Wired Mic', 'wired', false); + mockStore.availableAudioDevices = [wiredUnavailable]; + render(<AudioDeviceSelection />); + expect(screen.getByText('Wired Mic')).toBeTruthy(); + expect(screen.getByText(/Unavailable$/)).toBeTruthy(); + });
271-281: Clarify the unknown type fallback assertionRight now you assert twice against 'Unknown Device', which coincidentally matches the device name. If the fallback label remains “Unknown Device Device”, assert it explicitly to prove the label path too, or switch the component fallback to an i18n key and assert that.
Example tweak if keeping the current fallback logic:
- expect(screen.getAllByText('Unknown Device').length).toBeGreaterThan(0); - expect(screen.getAllByText('Unknown Device').length).toBeGreaterThan(0); + expect(screen.getAllByText('Unknown Device').length).toBeGreaterThan(0); // name + expect(screen.getAllByText('Unknown Device Device').length).toBeGreaterThan(0); // type label fallbacksrc/components/settings/audio-device-selection.tsx (6)
35-46: Tighten the device type typing and handle the 'default' type explicitlyType the parameter precisely and add a mapping for 'default'. Also consider localizing the fallback instead of concatenating " Device".
- const getDeviceTypeLabel = (deviceType: string) => { + const getDeviceTypeLabel = (deviceType: AudioDeviceInfo['type'] | (string & {})) => { switch (deviceType) { case 'bluetooth': return t('settings.audio_device_selection.bluetooth_device'); case 'wired': return t('settings.audio_device_selection.wired_device'); case 'speaker': return t('settings.audio_device_selection.speaker_device'); + case 'default': + // Add a translation key for this across locales; fallback keeps current UX stable if missing. + return 'Default Device'; default: - return deviceType.charAt(0).toUpperCase() + deviceType.slice(1) + ' Device'; + // Consider introducing a key like `settings.audio_device_selection.unknown_device_type` + // and/or avoiding hard-coded English suffixes here. + return deviceType.charAt(0).toUpperCase() + deviceType.slice(1) + ' Device'; } };I can add i18n keys for default/unknown device types in en/es/ar to remove the hard-coded English.
81-84: Follow guideline: use ?: instead of && for conditional renderingCoding guidelines request the conditional operator over &&.
- {showTitle && ( - <Heading size="lg" className="text-gray-900 dark:text-gray-100"> - <Text>{t('settings.audio_device_selection.title')}</Text> - </Heading> - )} + {showTitle ? ( + <Heading size="lg" className="text-gray-900 dark:text-gray-100"> + <Text>{t('settings.audio_device_selection.title')}</Text> + </Heading> + ) : null}
94-101: Localize punctuation: include the colon in the translation or use a dedicated keyAppending ":" outside t() can be awkward for some locales. Prefer including punctuation/formatting in the translation.
- <Text className="text-blue-800 dark:text-blue-200">{t('settings.audio_device_selection.microphone')}:</Text> + <Text className="text-blue-800 dark:text-blue-200">{t('settings.audio_device_selection.microphone_label')}</Text> ... - <Text className="text-blue-800 dark:text-blue-200">{t('settings.audio_device_selection.speaker')}:</Text> + <Text className="text-blue-800 dark:text-blue-200">{t('settings.audio_device_selection.speaker_label')}</Text>I can add microphone_label/speaker_label keys to en/es/ar with a colon.
53-71: Expose stable testIDs and improve accessibility for device itemsAdd testIDs to Pressable for reliable e2e/unit tests and set proper accessibility props. This avoids brittle DOM traversal in tests and improves screen reader support.
- <Pressable key={`${deviceType}-${device.id}`} onPress={onSelect}> + <Pressable + key={`${deviceType}-${device.id}`} + testID={`audio-device-item-${deviceType}-${device.id}`} + accessibilityRole="button" + accessibilityLabel={`${device.name}, ${deviceTypeLabel}${unavailableText ? ` ${t('settings.audio_device_selection.unavailable')}` : ''}`} + onPress={onSelect} + > <Card className={`mb-2 p-4 ${isSelected ? 'border-blue-600 bg-blue-50 dark:border-blue-400 dark:bg-blue-950/30' : 'border-gray-200 dark:border-gray-700'}`}> <HStack className="items-center justify-between"> <HStack className="flex-1 items-center" space="md"> {renderDeviceIcon(device)} <VStack className="flex-1"> <Text className={`font-medium ${isSelected ? 'text-blue-900 dark:text-blue-100' : 'text-gray-900 dark:text-gray-100'}`}>{device.name}</Text> <Text className={`text-sm ${isSelected ? 'text-blue-700 dark:text-blue-300' : 'text-gray-500 dark:text-gray-400'}`}> {deviceTypeLabel} {unavailableText} </Text> </VStack> </HStack> - {isSelected && <CheckCircle size={20} className="text-blue-600 dark:text-blue-400" />} + {isSelected ? <CheckCircle size={20} className="text-blue-600 dark:text-blue-400" /> : null} </HStack> </Card> </Pressable>
22-32: Confirm className support on lucide-react-native iconsOn React Native, lucide icons typically accept color/size props; className styling depends on your NativeWind/tailwind setup. If className isn’t applied to SVGs in your config, these icons won’t get the intended colors.
If needed, switch to explicit color props:
- return <Headphones size={20} className="text-gray-600 dark:text-gray-400" />; + return <Headphones size={20} color={isDarkMode ? '#9CA3AF' : '#4B5563'} />;I can check your NativeWind config and provide a safe theme-aware helper for icon colors.
114-116: Avoid inline arrow functions in render for onPress handlersInline lambdas within map cause re-creation each render. Not critical here, but you can stabilize handlers with useCallback and pass the device id.
For example:
const onSelectMic = React.useCallback((device: AudioDeviceInfo) => () => setSelectedMicrophone(device), [setSelectedMicrophone]); const onSelectSpk = React.useCallback((device: AudioDeviceInfo) => () => setSelectedSpeaker(device), [setSelectedSpeaker]); // ... availableMicrophones.map((device) => renderDeviceItem(device, selectedAudioDevices.microphone?.id === device.id, onSelectMic(device), 'microphone')) availableSpeakers.map((device) => renderDeviceItem(device, selectedAudioDevices.speaker?.id === device.id, onSelectSpk(device), 'speaker'))Also applies to: 132-134
📜 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 (7)
src/components/livekit/livekit-bottom-sheet.tsx(3 hunks)src/components/settings/__tests__/audio-device-selection.test.tsx(1 hunks)src/components/settings/audio-device-selection.tsx(3 hunks)src/stores/app/bluetooth-audio-store.ts(1 hunks)src/translations/ar.json(1 hunks)src/translations/en.json(1 hunks)src/translations/es.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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/stores/app/bluetooth-audio-store.tssrc/components/livekit/livekit-bottom-sheet.tsxsrc/components/settings/audio-device-selection.tsxsrc/components/settings/__tests__/audio-device-selection.test.tsx
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/stores/app/bluetooth-audio-store.tssrc/translations/ar.jsonsrc/components/livekit/livekit-bottom-sheet.tsxsrc/components/settings/audio-device-selection.tsxsrc/translations/es.jsonsrc/components/settings/__tests__/audio-device-selection.test.tsxsrc/translations/en.json
src/translations/**
📄 CodeRabbit Inference Engine (.cursorrules)
Store translation dictionaries in src/translations
Files:
src/translations/ar.jsonsrc/translations/es.jsonsrc/translations/en.json
src/translations/**/*.{json,ts}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Store translation dictionaries in src/translations
Files:
src/translations/ar.jsonsrc/translations/es.jsonsrc/translations/en.json
**/*.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/livekit/livekit-bottom-sheet.tsxsrc/components/settings/audio-device-selection.tsxsrc/components/settings/__tests__/audio-device-selection.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/livekit/livekit-bottom-sheet.tsxsrc/components/settings/audio-device-selection.tsxsrc/components/settings/__tests__/audio-device-selection.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/components/settings/__tests__/audio-device-selection.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/settings/__tests__/audio-device-selection.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/components/settings/__tests__/audio-device-selection.test.tsx
🧠 Learnings (4)
📚 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 **/*.{ts,tsx} : Use react-i18next for internationalization
Applied to files:
src/components/settings/audio-device-selection.tsx
📚 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 **/*.tsx : Wrap all user-facing text in t() from react-i18next
Applied to files:
src/components/settings/audio-device-selection.tsx
📚 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 **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/components/settings/__tests__/audio-device-selection.test.tsx
📚 Learning: 2025-08-12T03:34:25.963Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.963Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated components
Applied to files:
src/components/settings/__tests__/audio-device-selection.test.tsx
🧬 Code Graph Analysis (2)
src/components/settings/audio-device-selection.tsx (1)
src/stores/app/bluetooth-audio-store.ts (2)
useBluetoothAudioStore(105-228)AudioDeviceInfo(38-43)
src/components/settings/__tests__/audio-device-selection.test.tsx (2)
src/stores/app/bluetooth-audio-store.ts (2)
AudioDeviceInfo(38-43)AudioDeviceSelection(45-48)src/components/settings/audio-device-selection.tsx (1)
AudioDeviceSelection(18-143)
🔇 Additional comments (6)
src/stores/app/bluetooth-audio-store.ts (1)
114-116: No lingering ‘default’ speaker type checks; change is safe
I searched for anyselectedAudioDevices.speaker?.type === 'default'comparisons and fixtures/mocks using adefault-speakerwithtype: 'default'and found none. The switch totype: 'speaker'in bothavailableAudioDevices(lines 114–116) andselectedAudioDevices(lines 118–120) is safe.Optional refactor: centralize your defaults as constants in
src/stores/app/bluetooth-audio-store.tsfor consistency and to avoid drift:const DEFAULT_MIC: AudioDeviceInfo = { id: 'default-mic', name: 'Default Microphone', type: 'default', isAvailable: true, }; const DEFAULT_SPEAKER: AudioDeviceInfo = { id: 'default-speaker', name: 'Default Speaker', type: 'speaker', isAvailable: true, };Then reuse
DEFAULT_MIC/DEFAULT_SPEAKERin bothavailableAudioDevicesandselectedAudioDevices.src/translations/en.json (1)
534-546: Good addition: complete audio device selection keys present and consistentKeys are organized under settings.audio_device_selection, consistent with other locales and referenced UI. No issues found.
src/translations/es.json (1)
534-546: Spanish translations added correctly and align with the English keysThe 11 keys match the English set and read naturally. Looks good.
src/translations/ar.json (1)
534-546: Arabic translations added correctly and mirror other localesThe keys are complete and appropriate for RTL contexts. Approved.
src/components/settings/__tests__/audio-device-selection.test.tsx (1)
45-47: Mocking the store looks good and keeps tests deterministicThe mock store shape matches the interface used by the component and isolates tests from Zustand. Good call.
src/components/settings/audio-device-selection.tsx (1)
73-76: Verify filtering semantics: speaker devices appear under “Microphone”current logic shows all non-bluetooth devices (including type 'speaker') in the Microphone section, and only available devices in the Speaker section. If the intent was to show device “roles” rather than connection “types”, this may be confusing to users.
Would you like me to adjust filtering to separate input vs output devices (if the store exposes that) or at least hide speaker-type devices from the microphone list?
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/services/callkeep.service.ios.ts (1)
57-61: Bug: Pass numeric options to RNCallKeep instead of stringsRNCallKeep iOS options expect numbers for maximumCallGroups and maximumCallsPerCallGroup. Converting to strings can cause native type mismatches at runtime.
Apply this diff:
const options = { ios: { appName: config.appName, - maximumCallGroups: config.maximumCallGroups.toString(), - maximumCallsPerCallGroup: config.maximumCallsPerCallGroup.toString(), + maximumCallGroups: config.maximumCallGroups, + maximumCallsPerCallGroup: config.maximumCallsPerCallGroup, includesCallsInRecents: config.includesCallsInRecents, supportsVideo: config.supportsVideo, ringtoneSound: config.ringtoneSound, },src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (1)
527-548: Ensure env var cleanup even on test failure (use try/finally)If this test throws before restoration, STORYBOOK_LIVEKIT_TOKEN may leak into other tests. Wrap with try/finally to guarantee cleanup.
- it('should handle token fetch errors', async () => { - // Mock environment to simulate missing token - const originalEnv = process.env.STORYBOOK_LIVEKIT_TOKEN; - delete process.env.STORYBOOK_LIVEKIT_TOKEN; - - const { result } = renderHook(() => useLiveKitCallStore()); - - await act(async () => { - await result.current.actions.connectToRoom('test-room', 'test-participant'); - }); - - expect(result.current.error).toBe('Failed to fetch a valid connection token.'); - expect(result.current.isConnecting).toBe(false); - expect(result.current.isConnected).toBe(false); - expect(result.current.roomInstance).toBeNull(); - expect(result.current.currentRoomId).toBeNull(); - - // Restore original environment - if (originalEnv) { - process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv; - } - }); + it('should handle token fetch errors', async () => { + // Mock environment to simulate missing token + const originalEnv = process.env.STORYBOOK_LIVEKIT_TOKEN; + try { + delete process.env.STORYBOOK_LIVEKIT_TOKEN; + const { result } = renderHook(() => useLiveKitCallStore()); + await act(async () => { + await result.current.actions.connectToRoom('test-room', 'test-participant'); + }); + expect(result.current.error).toBe('Failed to fetch a valid connection token.'); + expect(result.current.isConnecting).toBe(false); + expect(result.current.isConnected).toBe(false); + expect(result.current.roomInstance).toBeNull(); + expect(result.current.currentRoomId).toBeNull(); + } finally { + if (originalEnv) { + process.env.STORYBOOK_LIVEKIT_TOKEN = originalEnv; + } else { + delete process.env.STORYBOOK_LIVEKIT_TOKEN; + } + } + });
🧹 Nitpick comments (6)
src/features/livekit-call/store/useLiveKitCallStore.ts (3)
6-6: Decouple from iOS-specific service path to enable platform resolutionImporting the .ios file directly ties this store to iOS at build time. Prefer importing from a platform-agnostic module so Metro can resolve the correct platform file (ios/android/native) and you can provide a no-op or Android implementation later without touching callers.
-import { callKeepService } from '../../../services/callkeep.service.ios'; +// Let RN resolve the platform-specific service (provide .ios/.android/.native files) +import { callKeepService } from '../../../services/callkeep.service';If you want, I can scaffold a no-op Android variant to keep the API consistent.
141-176: Start CallKeep on connect: looks good; also guard against unhandled mic/camera promise rejectionsThe CallKeep start flow is clean and well-logged. One nit: setMicrophoneEnabled/setCameraEnabled return Promises; if they reject, they can raise unhandled rejections since they aren’t awaited or caught here.
You can defensively attach catch handlers without blocking the connect path:
// immediately after setting connection state, before/around CallKeep start void newRoom.localParticipant.setMicrophoneEnabled(true).catch((e) => { logger.warn({ message: 'Failed to enable microphone after connect', context: { error: e, roomId } }); }); void newRoom.localParticipant.setCameraEnabled(false).catch((e) => { logger.debug({ message: 'Failed to disable camera after connect (video unsupported)', context: { error: e, roomId } }); });
318-331: Avoid duplicating CallKeep end across multiple pathsYou end the CallKeep call here and also in the ConnectionStateChanged → Disconnected handler. While harmless (the service no-ops if there’s no active call), consider consolidating to a single place to reduce duplicate calls and logs.
src/services/callkeep.service.ios.ts (1)
38-45: Clarify platform intent (Android options present but never used)setup() returns early for non-iOS, so Android options are currently dead code. Either:
- support Android here (remove the early return and let RNCallKeep.setup run cross-platform with appropriate options), or
- remove the Android options to avoid confusion and keep this service strictly iOS.
Given the filename is .ios.ts, the latter is consistent; otherwise consider creating platform-specific files with a shared import path.
Also applies to: 64-70
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (2)
463-471: Fix misleading test comment about connection preventionThe test sets isConnecting to true, so the first attempt is prevented by design. Update the comment to reflect the behavior under test.
- // First connection attempt should succeed + // First connection attempt should be prevented due to isConnecting = true
139-140: Prefer spying on Platform.OS getter over property re-assignmentDirectly mutating Platform.OS can be brittle. Using a getter spy avoids tampering with a potentially read-only property across RN versions.
Example:
const osSpy = jest.spyOn(Platform, 'OS', 'get'); osSpy.mockReturnValue('android'); // ... osSpy.mockRestore();Also applies to: 236-237
📜 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 (5)
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts(1 hunks)src/features/livekit-call/store/useLiveKitCallStore.ts(11 hunks)src/services/__tests__/callkeep.service.ios.test.ts(1 hunks)src/services/callkeep.service.ios.ts(1 hunks)src/stores/app/__tests__/livekit-store.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/stores/app/tests/livekit-store.test.ts
- src/services/tests/callkeep.service.ios.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (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/features/livekit-call/store/__tests__/useLiveKitCallStore.test.tssrc/services/callkeep.service.ios.tssrc/features/livekit-call/store/useLiveKitCallStore.ts
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.tssrc/services/callkeep.service.ios.tssrc/features/livekit-call/store/useLiveKitCallStore.ts
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/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/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/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts
🧬 Code Graph Analysis (3)
src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (2)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)src/features/livekit-call/store/useLiveKitCallStore.ts (1)
useLiveKitCallStore(81-385)
src/services/callkeep.service.ios.ts (1)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)
src/features/livekit-call/store/useLiveKitCallStore.ts (3)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)__mocks__/livekit-client.ts (4)
Room(1-12)ConnectionState(25-30)RoomEvent(14-23)RemoteParticipant(32-32)src/services/callkeep.service.ios.ts (1)
callKeepService(317-317)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (7)
src/features/livekit-call/store/useLiveKitCallStore.ts (4)
93-114: LGTM: iOS CallKeep setup is correctly gated and error-mappedGood platform gating, structured logging, and user-facing error mapping on setup failures. The numeric constraints look reasonable for single-call usage.
188-204: LGTM: Ending CallKeep on disconnectCleanly ends the CallKeep call on iOS with appropriate logging on both success and failure paths.
14-17: LGTM: Improved observability for token fetch and misconfigurationThe structured debug/warn logging around token fetching and placeholder URL is helpful for diagnosing connection issues in dev/test.
Also applies to: 26-34
120-123: LGTM: Safe-guard against duplicate connection attemptsThe early-exit with a clear warn log prevents racey multi-connects and keeps the state machine simpler.
src/services/callkeep.service.ios.ts (2)
114-147: LGTM: Start call lifecycle and audio-session activationThe startCall/reportConnecting/reportConnected sequence and audio-session event handling are consistent with CallKit patterns. Good defensive cleanup of existing calls before starting a new one.
221-227: Confirmed bulk removal of RNCallKeep event listenersVerified against react-native-callkeep v4.x documentation:
• CallingRNCallKeep.removeEventListener('eventName')without a callback removes all listeners for that event.
• Supplying both the event name and callback removes only that specific listener.Your current pattern—adding anonymous callbacks and later calling
removeEventListener('…')—will correctly clean up all handlers for those events. No changes are required here.If you ever need to remove only a single listener, store the callback reference and use:
RNCallKeep.removeEventListener('eventName', yourCallback);src/features/livekit-call/store/__tests__/useLiveKitCallStore.test.ts (1)
121-136: Solid coverage and realistic integration flowGood use of mocks, platform gating scenarios, and lifecycle assertions (connect, disconnect, start/end CallKeep, mic control, prevention, and error paths). The structured logger assertions align with the store’s behavior.
Also applies to: 175-186, 217-233, 288-347, 387-451, 455-506, 510-525
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit
New Features
Style
Tests