Conversation
📝 WalkthroughWalkthroughAdds a headset button (PTT) feature with a new service and hook, integrates monitoring and microphone controls into LiveKit/Bluetooth stores, adds UI/layout/tests, image and contact rendering improvements, translations, a microphone-toggle utility, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant Device as Device/Media Button
participant HBS as HeadsetButton<br/>Service
participant LKS as LiveKit<br/>Store
participant BAS as Bluetooth Audio<br/>Store
participant Audio as Audio Service
Device->>HBS: media button event (type, timestamp)
HBS->>HBS: map type -> event (single/double/long)
HBS->>HBS: evaluate config (pttMode, mappings)
alt mapped to mic control
HBS->>LKS: toggleMicrophone()/setMicrophoneEnabled()
LKS->>LKS: update participant mic state
LKS->>BAS: set lastAction / monitoring state
LKS->>Audio: play feedback (if enabled)
Audio-->>LKS: feedback played
LKS-->>HBS: mic state confirmed
else no action
HBS->>HBS: ignore or await further events
end
HBS->>BAS: record button event/timestamp
BAS-->>HBS: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/app/livekit-store.ts (1)
89-101: Interface mismatch:disconnectFromRoomsignature.The interface declares
disconnectFromRoom: () => void;but the implementation at line 280 isdisconnectFromRoom: async () => {...}. This creates a type inconsistency.🔧 Fix interface signature
// Room operations connectToRoom: (roomInfo: DepartmentVoiceChannelResultData, token: string) => Promise<void>; - disconnectFromRoom: () => void; + disconnectFromRoom: () => Promise<void>;
🤖 Fix all issues with AI agents
In `@src/app/call/_layout.tsx`:
- Around line 7-14: The TypeScript error comes from passing
headerBackTitleVisible inside the Stack's screenOptions where expo-router's
types don't include it; update the screenOptions to use a type assertion so TS
accepts the extra property—e.g. cast the object to any or to the library's
screen options type with an intersection that includes headerBackTitleVisible—so
modify the Stack call (symbol: Stack, prop: screenOptions) in _layout.tsx to
assert the options object (including the Platform.OS === 'android' animation
branch) as any/unknown as the expected type, preserving the
headerBackTitleVisible usage without changing runtime behavior.
In `@src/app/call/`[id]/_layout.tsx:
- Around line 7-14: The TypeScript error comes from headerBackTitleVisible not
matching the expected screenOptions type on the Stack component; update the
screenOptions object to a correctly-typed options object by importing the
NativeStackNavigationOptions type (or appropriate navigation options type used
by your Stack) and asserting the screenOptions as that type, e.g. add an import
for NativeStackNavigationOptions and change screenOptions to: screenOptions={ ({
headerBackTitleVisible: false, ...(Platform.OS === 'android' && { animation:
'slide_from_right' }) } as NativeStackNavigationOptions) }; this keeps
headerBackTitleVisible while satisfying the Stack's type checker (alternatively
cast to any if you prefer a quicker bypass).
In `@src/components/calls/call-images-modal.tsx`:
- Around line 175-176: The filename extension is hardcoded to .jpg while native
images are encoded as PNG; update the logic around setSelectedImageInfo and the
uploadCallImage invocation to derive the extension from the actual encoding
format (use .png when ImageManipulator.SaveFormat.PNG is used on native, .jpg
for JPEG/web), or switch native encoding to JPEG so names match bytes; ensure
the computed filename passed to setSelectedImageInfo({ uri, filename }) and
uploadCallImage(...) uses that derived extension so uploaded bytes and file
extension/MIME are consistent.
In `@src/components/contacts/contact-details-sheet.tsx`:
- Around line 139-187: HtmlContentField currently injects raw user HTML into the
WebView, so replace the direct interpolation of value with a sanitized string
produced by sanitizeHtmlContent(value) imported from src/utils/webview-html.ts,
and harden the WebView props to follow the defaultWebViewProps pattern (use
originWhitelist: ['about:'], set javaScriptEnabled: false and domStorageEnabled:
false) while keeping androidLayerType as needed; see
calendar-item-details-sheet.tsx for the exact sanitization + prop pattern and
update the WebView usage in HtmlContentField accordingly.
In `@src/services/__tests__/headset-button.service.test.ts`:
- Around line 224-258: The Button Type Mapping describe block lacks isolated
setup/teardown causing leaked mock calls; add a beforeEach that calls
jest.clearAllMocks() (or mockAddButtonEvent.mockClear()), re-initializes
headsetButtonService and calls headsetButtonService.startMonitoring() as needed
for each test, and add an afterEach that calls
headsetButtonService.stopMonitoring() to clean up; reference the describe block
and the methods startMonitoring, stopMonitoring, simulateButtonPress and the
mock mockAddButtonEvent when making these changes so each test runs with a fresh
mock/state.
In `@src/stores/app/livekit-store.ts`:
- Around line 445-461: The try/catch in startHeadsetButtonMonitoring won't catch
rejections from headsetButtonService.initialize() because the code uses .then();
change the function to handle async errors by making
startHeadsetButtonMonitoring async and using await for
headsetButtonService.initialize() (then call
headsetButtonService.startMonitoring() and update useBluetoothAudioStore) inside
the try block, so any thrown/rejected error is caught by the catch;
alternatively, if you prefer promises, add a .catch() on
headsetButtonService.initialize() to log the error via logger.error with the
same context.
🧹 Nitpick comments (17)
src/app/call/__tests__/_layout.test.tsx (3)
3-3: Unused import.
Viewis imported but not used in the test file body—it's re-imported inside the mock factory on line 8.🔧 Remove unused import
import { render } from '@testing-library/react-native'; import React from 'react'; -import { View } from 'react-native';
28-32: Test doesn't verify screenOptions configuration.The second test has the same assertions as the first and doesn't actually verify that screenOptions are configured correctly. Consider inspecting the props passed to the mock Stack.
🧪 Suggested improvement to actually test screenOptions
it('should configure Stack with proper screenOptions', () => { - const { getByTestId } = render(<CallLayout />); - const stack = getByTestId('mock-stack'); - expect(stack).toBeDefined(); + const { getByTestId } = render(<CallLayout />); + const stack = getByTestId('mock-stack'); + expect(stack.props.screenOptions).toBeDefined(); + expect(stack.props.screenOptions.headerBackTitleVisible).toBe(false); });
9-9: Avoid usinganytype.The mock uses
anyfor props and ref parameters. Per coding guidelines, avoidanyand use precise types.🔧 Use proper types
- const MockStack = React.forwardRef((props: any, ref: any) => ( + const MockStack = React.forwardRef((props: Record<string, unknown>, ref: React.Ref<View>) => (src/app/call/_layout.tsx (1)
5-16: Code duplication withCallIdLayout.This component is nearly identical to
src/app/call/[id]/_layout.tsx. Consider extracting the sharedscreenOptionsto a constant or helper to reduce duplication.♻️ Extract shared screenOptions
Create a shared configuration file, e.g.,
src/app/call/shared-options.ts:import { Platform } from 'react-native'; import type { NativeStackNavigationOptions } from '@react-navigation/native-stack'; export const callStackScreenOptions: NativeStackNavigationOptions = { headerBackTitleVisible: false, ...(Platform.OS === 'android' && { animation: 'slide_from_right', }), };Then import and use in both layouts:
import { callStackScreenOptions } from './shared-options'; export default function CallLayout() { return <Stack screenOptions={callStackScreenOptions} />; }src/app/call/[id]/__tests__/_layout.test.tsx (3)
3-3: Unused import.
Viewis imported but not used—it's re-imported inside the mock factory.🔧 Remove unused import
import { render } from '@testing-library/react-native'; import React from 'react'; -import { View } from 'react-native';
6-18: Duplicate mock setup across test files.This mock is identical to the one in
src/app/call/__tests__/_layout.test.tsx. Consider extracting a shared mock to a test utility file.♻️ Extract shared mock to test utility
Create
src/test-utils/mocks/expo-router.ts:import React from 'react'; import { View } from 'react-native'; export const createMockStack = () => { const MockStack = React.forwardRef((props: Record<string, unknown>, ref: React.Ref<typeof View>) => ( <View ref={ref as React.Ref<View>} testID="mock-stack" {...props}> {props.children as React.ReactNode} </View> )); MockStack.Screen = jest.fn(() => null); return { Stack: MockStack }; };Then in test files:
jest.mock('expo-router', () => require('@/test-utils/mocks/expo-router').createMockStack());
28-32: Test doesn't verify screenOptions configuration.Same issue as the other test file—the second test doesn't actually verify screenOptions are configured.
🧪 Suggested improvement
it('should configure Stack with proper screenOptions', () => { - const { getByTestId } = render(<CallIdLayout />); - const stack = getByTestId('mock-stack'); - expect(stack).toBeDefined(); + const { getByTestId } = render(<CallIdLayout />); + const stack = getByTestId('mock-stack'); + expect(stack.props.screenOptions).toBeDefined(); + expect(stack.props.screenOptions.headerBackTitleVisible).toBe(false); });src/app/call/[id]/_layout.tsx (1)
5-16: Implementation identical toCallLayout.This component is a duplicate of
src/app/call/_layout.tsx. If both need identical configuration, extract to a shared constant as mentioned in the earlier review comment.src/stores/app/bluetooth-audio-store.ts (1)
50-60: Duplicate type definitions with headset-button.service.ts.
PttModeandHeadsetButtonConfigare defined identically in both this file andsrc/services/headset-button.service.ts(lines 47-55). This violates DRY and can lead to drift if one is updated without the other.Consider importing these types from a single source of truth.
♻️ Suggested approach
Either:
- Export types from the service and import them here:
-// PTT mode for headset button control -export type PttMode = 'toggle' | 'push_to_talk' | 'disabled'; - -// Headset button configuration -export interface HeadsetButtonConfig { - pttMode: PttMode; - playPauseAction: 'toggle_mute' | 'none'; - doubleClickAction: 'toggle_mute' | 'none'; - longPressAction: 'toggle_mute' | 'none'; - soundFeedback: boolean; -} +export { type PttMode, type HeadsetButtonConfig } from '@/services/headset-button.service';
- Or create a shared types file (e.g.,
src/types/headset-button.types.ts) and import from there in both files.src/services/headset-button.service.ts (5)
14-14: Unused imports:NativeEventEmitterandNativeModules.These are imported but never used in the file.
🧹 Remove unused imports
-import { AppState, DeviceEventEmitter, NativeEventEmitter, NativeModules, Platform } from 'react-native'; +import { AppState, DeviceEventEmitter, Platform } from 'react-native';
19-37: Lazy loading and dynamic imports introduce runtime fragility.The lazy loading pattern for
liveKitStoreModuleand try/catch foraudioServiceare pragmatic for avoiding circular dependencies and test issues, but:
- Lines 20-21, 31: Uses
anytype which violates coding guidelines ("Avoid using any; use precise types")- Line 35: Empty catch block silently swallows errors - at minimum, log a warning
♻️ Suggested improvements
// Import livekit store lazily to avoid circular dependency -// eslint-disable-next-line `@typescript-eslint/no-explicit-any` -let liveKitStoreModule: any = null; +import type { useLiveKitStore as LiveKitStoreType } from '@/stores/app/livekit-store'; +let liveKitStoreModule: { useLiveKitStore: typeof LiveKitStoreType } | null = null; const getLiveKitStore = () => { if (!liveKitStoreModule) { // eslint-disable-next-line `@typescript-eslint/no-var-requires` liveKitStoreModule = require('@/stores/app/livekit-store'); } return liveKitStoreModule.useLiveKitStore; }; // Import audioService dynamically to avoid expo module import errors in tests -let audioService: any; +import type { audioService as AudioServiceType } from '@/services/audio.service'; +let audioService: Partial<typeof AudioServiceType> = {}; try { // eslint-disable-next-line `@typescript-eslint/no-var-requires` audioService = require('@/services/audio.service').audioService; } catch { - audioService = {}; + logger.warn({ message: 'audioService could not be loaded, sound feedback will be unavailable' }); }
72-77: Unused class properties:longPressTimerandisButtonHeld.These properties are declared and managed in
stopMonitoring()but the long press detection logic is never implemented. The documentation (line 154) also notes "push_to_talk (not yet implemented)".Consider either implementing the feature or removing dead code to avoid confusion.
198-213: Use precise types instead ofanyfor event handlers.Multiple handler methods use
anyfor event parameters, which violates coding guidelines.♻️ Define event interfaces
+interface RawHeadsetButtonEvent { + type?: string; + keyCode?: number; + source?: string; + deviceName?: string; +} + /** * Handle headset button events */ -private handleHeadsetButtonEvent(event: any): void { +private handleHeadsetButtonEvent(event: RawHeadsetButtonEvent): void { if (!this.isMonitoring) return; // ... }Apply similar typing to
handleRemoteControlEvent,handleMediaButtonEvent,handleAudioRouteChange, andhandleHeadsetConnectionChange.
493-535: Good implementation oftoggleMicrophonewith proper error handling.The method correctly:
- Checks for active room before proceeding
- Uses try/catch for error handling
- Updates the Bluetooth audio store
- Conditionally plays sound feedback
However, there's a minor logic concern: the variable
currentMuteStateis confusingly named - it actually represents the new enabled state, not the current mute state.♻️ Improve variable naming for clarity
- const currentMuteState = !liveKitStore.currentRoom.localParticipant.isMicrophoneEnabled; - await liveKitStore.currentRoom.localParticipant.setMicrophoneEnabled(currentMuteState); + const newEnabledState = !liveKitStore.currentRoom.localParticipant.isMicrophoneEnabled; + await liveKitStore.currentRoom.localParticipant.setMicrophoneEnabled(newEnabledState);src/stores/app/livekit-store.ts (1)
371-405: Duplicate microphone toggle logic.This
toggleMicrophoneimplementation duplicates nearly identical logic fromheadset-button.service.ts(lines 493-535). Both:
- Check for active room
- Toggle microphone state
- Update Bluetooth audio store
- Play sound feedback
Consider extracting shared logic to avoid drift and maintenance burden.
src/lib/hooks/use-headset-button-ptt.ts (2)
122-132: Effect dependencies won't trigger re-configuration after mount.The
pttModeandsoundFeedbackare in the dependency array, but the effect is guarded byinitialSetupDone.current, so changing these props after mount won't update the service configuration. This could lead to unexpected behavior.Either:
- Remove them from deps if one-time init is intentional (add eslint-disable comment)
- Or separate the initialization from configuration updates
♻️ Separate initialization from configuration
// Initialize service on mount useEffect(() => { if (!initialSetupDone.current) { headsetButtonService.initialize(); - headsetButtonService.setConfig({ - pttMode, - soundFeedback, - }); initialSetupDone.current = true; } - }, [pttMode, soundFeedback]); + }, []); + + // Update config when options change + useEffect(() => { + if (initialSetupDone.current) { + headsetButtonService.setConfig({ + pttMode, + soundFeedback, + }); + } + }, [pttMode, soundFeedback]);
134-141: Potential race condition with auto-start on connect.If
autoStartOnConnectis true and the user connects to LiveKit, this effect will callstartHeadsetButtonMonitoring(). However,connectToRoomin the LiveKit store (lines 248-263) also starts monitoring. This could result in duplicate initialization attempts.The service's
startMonitoring()guards against this with an early return, but the redundancy is worth noting.
src/app/call/_layout.tsx
Outdated
| <Stack | ||
| screenOptions={{ | ||
| headerBackTitleVisible: false, | ||
| ...(Platform.OS === 'android' && { | ||
| animation: 'slide_from_right', | ||
| }), | ||
| }} | ||
| /> |
There was a problem hiding this comment.
TypeScript error on headerBackTitleVisible.
Same TypeScript error as in the root layout—headerBackTitleVisible is not recognized in expo-router's types.
🐛 Proposed fix using type assertion
+import type { NativeStackNavigationOptions } from '@react-navigation/native-stack';
+
export default function CallLayout() {
return (
<Stack
screenOptions={{
headerBackTitleVisible: false,
...(Platform.OS === 'android' && {
animation: 'slide_from_right',
}),
- }}
+ } as NativeStackNavigationOptions}
/>
);
}🧰 Tools
🪛 GitHub Check: test
[failure] 9-9:
Object literal may only specify known properties, and 'headerBackTitleVisible' does not exist in type 'NativeStackNavigationOptions | ((props: { route: RouteProp<ParamListBase, string>; navigation: NativeStackNavigationProp<ParamListBase, string, undefined>; theme: Theme; }) => NativeStackNavigationOptions)'.
🤖 Prompt for AI Agents
In `@src/app/call/_layout.tsx` around lines 7 - 14, The TypeScript error comes
from passing headerBackTitleVisible inside the Stack's screenOptions where
expo-router's types don't include it; update the screenOptions to use a type
assertion so TS accepts the extra property—e.g. cast the object to any or to the
library's screen options type with an intersection that includes
headerBackTitleVisible—so modify the Stack call (symbol: Stack, prop:
screenOptions) in _layout.tsx to assert the options object (including the
Platform.OS === 'android' animation branch) as any/unknown as the expected type,
preserving the headerBackTitleVisible usage without changing runtime behavior.
| describe('Button Type Mapping', () => { | ||
| // Test the internal button type mapping through public interface | ||
| it('should handle play_pause button type', () => { | ||
| headsetButtonService.startMonitoring(); | ||
| headsetButtonService.simulateButtonPress('play_pause'); | ||
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| button: 'mute', | ||
| }) | ||
| ); | ||
| headsetButtonService.stopMonitoring(); | ||
| }); | ||
|
|
||
| it('should handle hook button type', () => { | ||
| headsetButtonService.startMonitoring(); | ||
| headsetButtonService.simulateButtonPress('hook'); | ||
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| button: 'mute', | ||
| }) | ||
| ); | ||
| headsetButtonService.stopMonitoring(); | ||
| }); | ||
|
|
||
| it('should handle unknown button type', () => { | ||
| headsetButtonService.startMonitoring(); | ||
| headsetButtonService.simulateButtonPress('next'); | ||
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| button: 'unknown', | ||
| }) | ||
| ); | ||
| headsetButtonService.stopMonitoring(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "headset-button.service.test.ts" -type fRepository: Resgrid/Responder
Length of output: 116
🏁 Script executed:
cat -n src/services/__tests__/headset-button.service.test.tsRepository: Resgrid/Responder
Length of output: 9882
Add setup/teardown to the Button Type Mapping describe block to ensure mock isolation.
This describe block is outside the main HeadsetButtonService scope, so the main beforeEach (which clears mocks) does not apply. Tests can accumulate mock calls from previous tests, causing false positives. Add a beforeEach to clear mocks and initialize the service, plus an afterEach to clean up.
Suggested setup/teardown
describe('Button Type Mapping', () => {
+ beforeEach(async () => {
+ jest.clearAllMocks();
+ await headsetButtonService.initialize();
+ });
+
+ afterEach(() => {
+ headsetButtonService.stopMonitoring();
+ });
// Test the internal button type mapping through public interface
it('should handle play_pause button type', () => {📝 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.
| describe('Button Type Mapping', () => { | |
| // Test the internal button type mapping through public interface | |
| it('should handle play_pause button type', () => { | |
| headsetButtonService.startMonitoring(); | |
| headsetButtonService.simulateButtonPress('play_pause'); | |
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| button: 'mute', | |
| }) | |
| ); | |
| headsetButtonService.stopMonitoring(); | |
| }); | |
| it('should handle hook button type', () => { | |
| headsetButtonService.startMonitoring(); | |
| headsetButtonService.simulateButtonPress('hook'); | |
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| button: 'mute', | |
| }) | |
| ); | |
| headsetButtonService.stopMonitoring(); | |
| }); | |
| it('should handle unknown button type', () => { | |
| headsetButtonService.startMonitoring(); | |
| headsetButtonService.simulateButtonPress('next'); | |
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| button: 'unknown', | |
| }) | |
| ); | |
| headsetButtonService.stopMonitoring(); | |
| }); | |
| }); | |
| describe('Button Type Mapping', () => { | |
| beforeEach(async () => { | |
| jest.clearAllMocks(); | |
| await headsetButtonService.initialize(); | |
| }); | |
| afterEach(() => { | |
| headsetButtonService.stopMonitoring(); | |
| }); | |
| // Test the internal button type mapping through public interface | |
| it('should handle play_pause button type', () => { | |
| headsetButtonService.startMonitoring(); | |
| headsetButtonService.simulateButtonPress('play_pause'); | |
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| button: 'mute', | |
| }) | |
| ); | |
| headsetButtonService.stopMonitoring(); | |
| }); | |
| it('should handle hook button type', () => { | |
| headsetButtonService.startMonitoring(); | |
| headsetButtonService.simulateButtonPress('hook'); | |
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| button: 'mute', | |
| }) | |
| ); | |
| headsetButtonService.stopMonitoring(); | |
| }); | |
| it('should handle unknown button type', () => { | |
| headsetButtonService.startMonitoring(); | |
| headsetButtonService.simulateButtonPress('next'); | |
| expect(mockAddButtonEvent).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| button: 'unknown', | |
| }) | |
| ); | |
| headsetButtonService.stopMonitoring(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In `@src/services/__tests__/headset-button.service.test.ts` around lines 224 -
258, The Button Type Mapping describe block lacks isolated setup/teardown
causing leaked mock calls; add a beforeEach that calls jest.clearAllMocks() (or
mockAddButtonEvent.mockClear()), re-initializes headsetButtonService and calls
headsetButtonService.startMonitoring() as needed for each test, and add an
afterEach that calls headsetButtonService.stopMonitoring() to clean up;
reference the describe block and the methods startMonitoring, stopMonitoring,
simulateButtonPress and the mock mockAddButtonEvent when making these changes so
each test runs with a fresh mock/state.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/calls/call-images-modal.tsx (1)
344-356: Fallback encoding format creates filename/content mismatch.When the primary PNG manipulation fails on native platforms, the fallback uses
SaveFormat.JPEG(line 352) while the filename retains the.pngextension set earlier (lines 176, 195, 253, 272). This causes the same filename/encoding mismatch that was previously identified - JPEG bytes uploaded under a.pngfilename.Consider using
SaveFormat.PNGin the fallback to maintain consistency, or update the filename when falling back to JPEG encoding.🛠️ Suggested fix - use PNG in fallback for consistency
} catch (manipulateError) { console.error('Error manipulating image:', manipulateError); // Try without resize as fallback for problematic images manipulatedImage = await ImageManipulator.manipulateAsync( selectedImageInfo.uri, [], // No transformations { compress: 0.8, - format: ImageManipulator.SaveFormat.JPEG, + format: ImageManipulator.SaveFormat.PNG, base64: true, } ); }
🤖 Fix all issues with AI agents
In @.github/copilot-instructions.md:
- Line 51: Update the sentence "This is an expo managed project that uses
prebuild, do not make native code changes outside of expo prebuild
capabilities." to use the hyphenated and capitalized form "Expo‑managed" and
adjust capitalization as needed so it reads naturally (e.g., "This is an
Expo‑managed project that uses prebuild; do not make native code changes outside
of Expo prebuild capabilities.")—locate and replace the exact string in the
document.
In `@src/services/headset-button.service.ts`:
- Around line 431-440: The setTimeout callback calls the async method
executeButtonAction(event, this.buttonPressCount) but does not handle promise
rejections; wrap the call so any thrown errors are caught (e.g., use an async
IIFE or call executeButtonAction(...).catch(...)) and ensure buttonPressCount is
reset after handling; modify the block using the existing symbols
buttonPressTimer, executeButtonAction, buttonPressCount and
DOUBLE_CLICK_THRESHOLD to call executeButtonAction safely and log or handle
errors to avoid unhandled promise rejections.
In `@src/stores/app/livekit-store.ts`:
- Around line 100-101: The startHeadsetButtonMonitoring signature is typed as ()
=> void but its implementation is async (await
headsetButtonService.initialize()), so change the interface/type to () =>
Promise<void> for startHeadsetButtonMonitoring in livekit-store (and any
exported types referencing it), then update/verify call sites such as
use-headset-button-ptt.ts to await or handle the returned promise (e.g., await
store.startHeadsetButtonMonitoring() or attach .catch) to properly propagate
rejections and avoid unhandled promise errors.
🧹 Nitpick comments (5)
src/components/calls/call-images-modal.tsx (1)
440-470: Consider extracting event handlers to avoid anonymous functions.Per coding guidelines, anonymous functions in event handlers can cause unnecessary re-renders. The
onPress,onError, andonLoadhandlers create new function instances on each render.Since these handlers need access to
itemcontext, consider using a memoized callback with item ID or extracting to a separate component.♻️ Example approach - extract to memoized callbacks
const handleImagePress = useCallback((imageSource: { uri: string }, itemName?: string) => { setFullScreenImage({ source: imageSource, name: itemName }); }, []); const handleImageLoad = useCallback((itemId: string) => { setImageErrors((prev) => { const newSet = new Set(prev); newSet.delete(itemId); return newSet; }); }, []); // Then in renderImageItem: onPress={() => handleImagePress(imageSource, item.Name)} onLoad={() => handleImageLoad(item.Id)} onError={() => handleImageError(item.Id, 'expo-image load error')}src/components/contacts/contact-details-sheet.tsx (1)
497-505: Guard against invalid date parsing before formatting.If
parseDateISOStringreturns an invalid date,formatDateForDisplaymay throw or display “Invalid Date.” Consider parsing once and guarding the result.♻️ Proposed refactor
+ const addedOnDate = selectedContact.AddedOn ? parseDateISOString(selectedContact.AddedOn) : null; + const editedOnDate = selectedContact.EditedOn ? parseDateISOString(selectedContact.EditedOn) : null; ... - value={selectedContact.AddedOn ? formatDateForDisplay(parseDateISOString(selectedContact.AddedOn), 'yyyy-MM-dd HH:mm') : undefined} + value={addedOnDate ? formatDateForDisplay(addedOnDate, 'yyyy-MM-dd HH:mm') : undefined} ... - value={selectedContact.EditedOn ? formatDateForDisplay(parseDateISOString(selectedContact.EditedOn), 'yyyy-MM-dd HH:mm') : undefined} + value={editedOnDate ? formatDateForDisplay(editedOnDate, 'yyyy-MM-dd HH:mm') : undefined}src/utils/microphone-toggle.ts (1)
35-67: Rename the toggle state variable for clarity.
currentMuteStateactually represents the next “enabled” state; a clearer name reduces confusion in logs and actions.♻️ Suggested refactor
- const currentMuteState = !room.localParticipant.isMicrophoneEnabled; - await room.localParticipant.setMicrophoneEnabled(currentMuteState); + const nextMicrophoneEnabled = !room.localParticipant.isMicrophoneEnabled; + await room.localParticipant.setMicrophoneEnabled(nextMicrophoneEnabled); - context: { enabled: currentMuteState }, + context: { enabled: nextMicrophoneEnabled }, - action: currentMuteState ? 'unmute' : 'mute', + action: nextMicrophoneEnabled ? 'unmute' : 'mute', - if (currentMuteState) { + if (nextMicrophoneEnabled) {src/lib/hooks/__tests__/use-headset-button-ptt.test.ts (1)
59-81: Avoidas anyin mocked store return values.
Use typed assertions to keep tests aligned with store contracts (apply similarly to other mock returns). As per coding guidelines, avoidany.♻️ Example adjustment
- mockedUseLiveKitStore.mockReturnValue({ + mockedUseLiveKitStore.mockReturnValue({ isConnected: false, currentRoom: null, toggleMicrophone: mockToggleMicrophone, setMicrophoneEnabled: mockSetMicrophoneEnabled, startHeadsetButtonMonitoring: mockStartHeadsetButtonMonitoring, stopHeadsetButtonMonitoring: mockStopHeadsetButtonMonitoring, - } as any); + } as ReturnType<typeof useLiveKitStore>); - mockedUseBluetoothAudioStore.mockReturnValue({ + mockedUseBluetoothAudioStore.mockReturnValue({ isHeadsetButtonMonitoring: false, headsetButtonConfig: { pttMode: 'toggle', soundFeedback: true, }, lastButtonAction: null, setHeadsetButtonConfig: mockSetHeadsetButtonConfig, - } as any); + } as ReturnType<typeof useBluetoothAudioStore>);src/services/headset-button.service.ts (1)
19-37: Reduceanyusage for dynamic modules and event payloads.
anymasks invalid fields and weakens editor help; a small event interface + typed dynamic imports keeps flexibility while improving safety.♻️ Possible typing cleanup
-// eslint-disable-next-line `@typescript-eslint/no-explicit-any` -let liveKitStoreModule: any = null; +type LiveKitStoreModule = typeof import('@/stores/app/livekit-store'); +type AudioService = typeof import('@/services/audio.service')['audioService']; +type HeadsetButtonNativeEvent = { + type?: string | number; + keyCode?: number; + command?: string; + source?: string; + deviceName?: string; + newRoute?: string; +}; +let liveKitStoreModule: LiveKitStoreModule | null = null; @@ -let audioService: any; +let audioService: Partial<AudioService> = {}; @@ - private handleHeadsetButtonEvent(event: any): void { + private handleHeadsetButtonEvent(event: HeadsetButtonNativeEvent): void { @@ - private handleRemoteControlEvent(event: any): void { + private handleRemoteControlEvent(event: HeadsetButtonNativeEvent): void { @@ - private handleMediaButtonEvent(event: any): void { + private handleMediaButtonEvent(event: HeadsetButtonNativeEvent): void { @@ - private handleAudioRouteChange(event: any): void { + private handleAudioRouteChange(event: HeadsetButtonNativeEvent): void { @@ - private handleHeadsetConnectionChange(event: any): void { + private handleHeadsetConnectionChange(event: HeadsetButtonNativeEvent): void {As per coding guidelines, avoid
anyin TypeScript.Also applies to: 198-283
| - Handle errors gracefully and provide user feedback. | ||
| - Implement proper offline support. | ||
| - Ensure the user interface is intuitive and user-friendly and works seamlessly across different devices and screen sizes. | ||
| - This is an expo managed project that uses prebuild, do not make native code changes outside of expo prebuild capabilities. |
There was a problem hiding this comment.
Hyphenate and capitalize “Expo‑managed.”
Minor wording polish for readability.
✏️ Suggested edit
- This is an expo managed project that uses prebuild, do not make native code changes outside of expo prebuild capabilities.
+ This is an Expo-managed project that uses prebuild; do not make native code changes outside of Expo prebuild capabilities.📝 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.
| - This is an expo managed project that uses prebuild, do not make native code changes outside of expo prebuild capabilities. | |
| - This is an Expo-managed project that uses prebuild; do not make native code changes outside of Expo prebuild capabilities. |
🧰 Tools
🪛 LanguageTool
[grammar] ~51-~51: Use a hyphen to join words.
Context: ...ices and screen sizes. - This is an expo managed project that uses prebuild, do n...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In @.github/copilot-instructions.md at line 51, Update the sentence "This is an
expo managed project that uses prebuild, do not make native code changes outside
of expo prebuild capabilities." to use the hyphenated and capitalized form
"Expo‑managed" and adjust capitalization as needed so it reads naturally (e.g.,
"This is an Expo‑managed project that uses prebuild; do not make native code
changes outside of Expo prebuild capabilities.")—locate and replace the exact
string in the document.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/lib/hooks/__tests__/use-headset-button-ptt.test.ts`:
- Around line 498-503: The test uses an invalid PttMode value ('push') in the
config object; update the test in use-headset-button-ptt.test.ts so the
config.pttMode uses a valid PttMode string (one of 'toggle', 'push_to_talk', or
'disabled') so TypeScript typing aligns with the PttMode union and the test
continues to assert expected behavior for the hook under test (replace the
pttMode value in the config object used in the 'should expose current config'
spec).
In `@src/services/headset-button.service.ts`:
- Around line 262-273: handleAudioRouteChange currently only sets bluetooth
routing to true (via bluetoothStore.setAudioRoutingActive(true)) when
event.newRoute contains "Bluetooth" or "AirPods", leaving the store stuck true
when the route changes away; update the method to setAudioRoutingActive(false)
when event?.newRoute does not include those strings or is missing, i.e.,
determine a boolean isBluetoothRoute = !!event?.newRoute &&
(event.newRoute.includes('Bluetooth') || event.newRoute.includes('AirPods')) and
call bluetoothStore.setAudioRoutingActive(isBluetoothRoute) so the store is
correctly toggled both on and off.
- Around line 19-37: The lazy imports use `any`, losing type-safety; replace
them with type-only imports and narrow types: add a type-only import for the
exported hook (e.g., useLiveKitStore's type) and declare liveKitStoreModule so
getLiveKitStore returns that hook's typed signature (reference: getLiveKitStore,
liveKitStoreModule, useLiveKitStore); similarly import the AudioService type
only and type the dynamic audioService as AudioService | Record<string, never>,
ensuring it exposes playStartTransmittingSound and playStopTransmittingSound
(reference: audioService, AudioService, playStartTransmittingSound,
playStopTransmittingSound) while keeping the runtime require/try-catch to avoid
circular imports.
♻️ Duplicate comments (2)
src/services/headset-button.service.ts (1)
413-447: CaptureclickCountbefore awaiting to avoid lost presses.
buttonPressCountis reset only after the async action completes. If a new press comes in whileexecuteButtonActionis in flight, thefinallyblock can wipe the new sequence. Capture and clear before the await.🐛 Suggested fix
// Set timer to process the final action this.buttonPressTimer = setTimeout(() => { - (async () => { - try { - await this.executeButtonAction(event, this.buttonPressCount); - } catch (error) { - console.error('Error executing button action:', error); - } finally { - this.buttonPressCount = 0; - } - })(); + const clickCount = this.buttonPressCount; + this.buttonPressCount = 0; + void this.executeButtonAction(event, clickCount).catch((error) => { + console.error('Error executing button action:', error); + }); }, this.DOUBLE_CLICK_THRESHOLD);src/components/calls/call-images-modal.tsx (1)
156-205: Normalize uploaded filenames to the actual encoded format.
selectedImageInfo.filenamecan retain the original asset extension (e.g., .heic/.jpg), while native manipulation re-encodes to PNG and the fallback path switches to JPEG. That can reintroduce extension/MIME mismatches during upload. Consider deriving the upload filename from the finalImageManipulatorformat (and, if using a data-URI fallback on web, the URI’s mime type) right beforeuploadCallImage.🛠️ Suggested fix
@@ - try { - let base64Image: string; + try { + let base64Image: string; + let outputFormat = Platform.OS === 'web' ? ImageManipulator.SaveFormat.JPEG : ImageManipulator.SaveFormat.PNG; @@ - const manipulatedImage = await ImageManipulator.manipulateAsync(selectedImageInfo.uri, [{ resize: { width: 1024 } }], { - compress: 0.8, - format: ImageManipulator.SaveFormat.JPEG, - base64: true, // Get base64 directly to avoid FileSystem issues on web - }); + const manipulatedImage = await ImageManipulator.manipulateAsync(selectedImageInfo.uri, [{ resize: { width: 1024 } }], { + compress: 0.8, + format: outputFormat, + base64: true, // Get base64 directly to avoid FileSystem issues on web + }); @@ - manipulatedImage = await ImageManipulator.manipulateAsync(selectedImageInfo.uri, [{ resize: { width: 1024 } }], { - compress: 0.8, - format: ImageManipulator.SaveFormat.PNG, - base64: true, // Get base64 directly - }); + manipulatedImage = await ImageManipulator.manipulateAsync(selectedImageInfo.uri, [{ resize: { width: 1024 } }], { + compress: 0.8, + format: outputFormat, + base64: true, // Get base64 directly + }); } catch (manipulateError) { console.error('Error manipulating image:', manipulateError); // Try without resize as fallback for problematic images + outputFormat = ImageManipulator.SaveFormat.JPEG; manipulatedImage = await ImageManipulator.manipulateAsync( selectedImageInfo.uri, [], // No transformations { compress: 0.8, - format: ImageManipulator.SaveFormat.JPEG, + format: outputFormat, base64: true, } ); } @@ - await uploadCallImage( + const normalizeFilename = (name: string, extension: string) => + name.includes('.') ? name.replace(/\.[^.]+$/, `.${extension}`) : `${name}.${extension}`; + const uploadFilename = normalizeFilename( + selectedImageInfo.filename, + outputFormat === ImageManipulator.SaveFormat.PNG ? 'png' : 'jpg' + ); + + await uploadCallImage( callId, useAuthStore.getState().userId!, newImageNote || '', // Use note for the note field - selectedImageInfo.filename, // Use filename for the name field + uploadFilename, // Match filename to encoded format currentLatitude, // Current latitude currentLongitude, // Current longitude base64Image );Also applies to: 308-379
🧹 Nitpick comments (3)
src/components/contacts/contact-details-sheet.tsx (1)
145-194: Consider dynamic height for variable HTML content.The
HtmlContentFieldcomponent uses a fixedminHeightof 60px withscrollEnabled={false}. For longer HTML content (e.g., detailed descriptions or notes), this may clip the content without any way for users to view the rest.Consider either:
- Enabling scroll for the WebView when content exceeds the container
- Implementing dynamic height measurement using
onContentSizeChangeorinjectedJavaScriptto measure content height♻️ Option 1: Enable scrolling for overflow
<View className="min-h-[60px] overflow-hidden rounded-lg"> <WebView style={[styles.htmlContent, { backgroundColor: bgColor }]} originWhitelist={['about:']} - scrollEnabled={false} + scrollEnabled={true} showsVerticalScrollIndicator={false}♻️ Option 2: Dynamic height with content measurement
const HtmlContentField: React.FC<HtmlContentFieldProps> = ({ label, value }) => { const { colorScheme } = useColorScheme(); const [webViewHeight, setWebViewHeight] = useState(60); // ... existing code ... return ( <VStack space="xs" className="py-2"> <Text className="text-sm text-gray-500 dark:text-gray-400">{label}</Text> <View style={{ minHeight: webViewHeight }} className="overflow-hidden rounded-lg"> <WebView style={[styles.htmlContent, { backgroundColor: bgColor, height: webViewHeight }]} // ... other props ... onMessage={(event) => { const height = parseInt(event.nativeEvent.data, 10); if (height > 0) setWebViewHeight(height); }} injectedJavaScript={` window.ReactNativeWebView.postMessage( document.body.scrollHeight.toString() ); true; `} javaScriptEnabled={true} // Required for height measurement // ... rest of props ... /> </View> </VStack> ); };src/components/calls/call-images-modal.tsx (2)
395-414: Replaceanyin the error callback.Use
unknown(or a specific error type) to keep type safety and avoid propagatingany.As per coding guidelines, avoid `any`.♻️ Suggested typing
- const handleImageError = useCallback((itemId: string, error: any) => { - console.error(`Image failed to load for ${itemId}:`, error); + const handleImageError = useCallback((itemId: string, error: unknown) => { + const message = error instanceof Error ? error.message : String(error); + console.error(`Image failed to load for ${itemId}:`, message); setImageErrors((prev) => new Set([...prev, itemId])); }, []);
456-475: Avoid inline handlers inrenderItemto reduce re-renders.Inline lambdas for
onPress/onLoad/onErrorallocate per item; extracting a memoized child with stable callbacks keeps FlashList perf predictable.♻️ Suggested refactor (usage)
- <TouchableOpacity - onPress={() => handleImagePress(imageSource, item.Name)} - testID={`image-${item.Id}-touchable`} - activeOpacity={0.7} - style={{ width: '100%' }} - delayPressIn={0} - delayPressOut={0} - > - <Image - key={`${item.Id}-${index}`} - source={imageSource} - style={styles.galleryImage} - contentFit="contain" - transition={200} - pointerEvents="none" - cachePolicy="memory-disk" - recyclingKey={item.Id} - onError={() => handleImageLoadError(item.Id)} - onLoad={() => handleImageLoadSuccess(item.Id)} - /> - </TouchableOpacity> + <CallImagePressable + itemId={item.Id} + itemName={item.Name} + imageSource={imageSource} + testID={`image-${item.Id}-touchable`} + onPressImage={handleImagePress} + onErrorImage={handleImageLoadError} + onLoadImage={handleImageLoadSuccess} + />Example component (place near the other helpers):
interface CallImagePressableProps { itemId: string; itemName?: string; imageSource: { uri: string }; testID?: string; onPressImage: (source: { uri: string }, name?: string) => void; onLoadImage: (itemId: string) => void; onErrorImage: (itemId: string) => void; } const CallImagePressable: React.FC<CallImagePressableProps> = React.memo( ({ itemId, itemName, imageSource, testID, onPressImage, onLoadImage, onErrorImage }) => { const handlePress = useCallback(() => onPressImage(imageSource, itemName), [onPressImage, imageSource, itemName]); const handleLoad = useCallback(() => onLoadImage(itemId), [onLoadImage, itemId]); const handleError = useCallback(() => onErrorImage(itemId), [onErrorImage, itemId]); return ( <TouchableOpacity onPress={handlePress} testID={testID} activeOpacity={0.7} style={{ width: '100%' }} delayPressIn={0} delayPressOut={0} > <Image source={imageSource} style={styles.galleryImage} contentFit="contain" transition={200} pointerEvents="none" cachePolicy="memory-disk" recyclingKey={itemId} onError={handleError} onLoad={handleLoad} /> </TouchableOpacity> ); } );Based on learnings, avoid anonymous handlers in renderItem/event callbacks.
| it('should expose current config', () => { | ||
| const config = { | ||
| pttMode: 'push' as const, | ||
| soundFeedback: false, | ||
| }; | ||
|
|
There was a problem hiding this comment.
Use a valid PttMode value in the test config.
'push' is not part of the PttMode union ('toggle' | 'push_to_talk' | 'disabled'). This weakens the type-signal of the test.
✅ Suggested fix
const config = {
- pttMode: 'push' as const,
+ pttMode: 'push_to_talk' as const,
soundFeedback: false,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should expose current config', () => { | |
| const config = { | |
| pttMode: 'push' as const, | |
| soundFeedback: false, | |
| }; | |
| it('should expose current config', () => { | |
| const config = { | |
| pttMode: 'push_to_talk' as const, | |
| soundFeedback: false, | |
| }; |
🤖 Prompt for AI Agents
In `@src/lib/hooks/__tests__/use-headset-button-ptt.test.ts` around lines 498 -
503, The test uses an invalid PttMode value ('push') in the config object;
update the test in use-headset-button-ptt.test.ts so the config.pttMode uses a
valid PttMode string (one of 'toggle', 'push_to_talk', or 'disabled') so
TypeScript typing aligns with the PttMode union and the test continues to assert
expected behavior for the hook under test (replace the pttMode value in the
config object used in the 'should expose current config' spec).
| // Import livekit store lazily to avoid circular dependency | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| let liveKitStoreModule: any = null; | ||
| const getLiveKitStore = () => { | ||
| if (!liveKitStoreModule) { | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| liveKitStoreModule = require('@/stores/app/livekit-store'); | ||
| } | ||
| return liveKitStoreModule.useLiveKitStore; | ||
| }; | ||
|
|
||
| // Import audioService dynamically to avoid expo module import errors in tests | ||
| let audioService: any; | ||
| try { | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| audioService = require('@/services/audio.service').audioService; | ||
| } catch { | ||
| audioService = {}; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and display the headset-button.service.ts file
fd -t f "headset-button.service.ts"Repository: Resgrid/Responder
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Search for any explicit `any` types in the file
rg -n "\bany\b" src/services/headset-button.service.tsRepository: Resgrid/Responder
Length of output: 631
🏁 Script executed:
#!/bin/bash
# Read the full file to understand context and all `any` usages
cat -n src/services/headset-button.service.ts | head -60Repository: Resgrid/Responder
Length of output: 2600
🏁 Script executed:
#!/bin/bash
# Check if livekit-store exists and what it exports
fd -t f "livekit-store.ts"Repository: Resgrid/Responder
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Read livekit-store to verify useLiveKitStore export
cat -n src/stores/app/livekit-store.ts | head -30Repository: Resgrid/Responder
Length of output: 1941
🏁 Script executed:
#!/bin/bash
# Check if audio.service exists and what it exports
fd -t f "audio.service.ts"Repository: Resgrid/Responder
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Read audio.service to verify audioService export structure
cat -n src/services/audio.service.ts | head -40Repository: Resgrid/Responder
Length of output: 1554
🏁 Script executed:
#!/bin/bash
# Search for useLiveKitStore export in livekit-store
rg -n "useLiveKitStore\|export.*create" src/stores/app/livekit-store.ts | head -20Repository: Resgrid/Responder
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check the end of livekit-store to see exports
tail -50 src/stores/app/livekit-store.ts | cat -nRepository: Resgrid/Responder
Length of output: 1691
🏁 Script executed:
#!/bin/bash
# Check audio.service exports at the end of file
tail -20 src/services/audio.service.ts | cat -nRepository: Resgrid/Responder
Length of output: 690
🏁 Script executed:
#!/bin/bash
# Search for the export statement in livekit-store more thoroughly
rg -A 5 "export" src/stores/app/livekit-store.ts | tail -40Repository: Resgrid/Responder
Length of output: 240
🏁 Script executed:
#!/bin/bash
# Get line count and check the very end of livekit-store.ts
wc -l src/stores/app/livekit-store.ts
tail -10 src/stores/app/livekit-store.ts | cat -nRepository: Resgrid/Responder
Length of output: 388
🏁 Script executed:
#!/antml/bin/bash
# Search for playStartTransmittingSound and playStopTransmittingSound in audio.service
rg -n "playStartTransmittingSound|playStopTransmittingSound" src/services/audio.service.tsRepository: Resgrid/Responder
Length of output: 175
🏁 Script executed:
#!/bin/bash
# Get the full picture of what getLiveKitStore returns (verify it's being used correctly in the file)
rg -A 3 "getLiveKitStore()" src/services/headset-button.service.ts | head -20Repository: Resgrid/Responder
Length of output: 895
🏁 Script executed:
#!/bin/bash
# Verify audioService method usage in headset-button.service.ts
rg -n "audioService\." src/services/headset-button.service.tsRepository: Resgrid/Responder
Length of output: 185
Avoid any in lazy imports; add minimal typings.
any erodes type-safety for getLiveKitStore and audioService. Since livekit-store.ts exports useLiveKitStore from zustand and audio.service.ts exports an AudioService instance with playStartTransmittingSound() and playStopTransmittingSound() methods, you can safely add type-only imports and interfaces without triggering circular dependencies.
♻️ Suggested update
import { AppState, DeviceEventEmitter, NativeEventEmitter, NativeModules, Platform } from 'react-native';
+import type { useLiveKitStore } from '@/stores/app/livekit-store';
// Import livekit store lazily to avoid circular dependency
-// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
-let liveKitStoreModule: any = null;
+type LiveKitStoreModule = { useLiveKitStore: typeof useLiveKitStore };
+let liveKitStoreModule: LiveKitStoreModule | null = null;
const getLiveKitStore = () => {
if (!liveKitStoreModule) {
// eslint-disable-next-line `@typescript-eslint/no-var-requires`
- liveKitStoreModule = require('@/stores/app/livekit-store');
+ liveKitStoreModule = require('@/stores/app/livekit-store') as LiveKitStoreModule;
}
return liveKitStoreModule.useLiveKitStore;
};
// Import audioService dynamically to avoid expo module import errors in tests
-let audioService: any;
+interface AudioService {
+ playStartTransmittingSound?: () => Promise<void>;
+ playStopTransmittingSound?: () => Promise<void>;
+}
+let audioService: AudioService = {};
try {
// eslint-disable-next-line `@typescript-eslint/no-var-requires`
- audioService = require('@/services/audio.service').audioService;
+ audioService = require('@/services/audio.service').audioService as AudioService;
} catch {
audioService = {};
}Type-only imports are erased at runtime and do not create circular dependencies. Align with coding guidelines to avoid any and use precise types.
🤖 Prompt for AI Agents
In `@src/services/headset-button.service.ts` around lines 19 - 37, The lazy
imports use `any`, losing type-safety; replace them with type-only imports and
narrow types: add a type-only import for the exported hook (e.g.,
useLiveKitStore's type) and declare liveKitStoreModule so getLiveKitStore
returns that hook's typed signature (reference: getLiveKitStore,
liveKitStoreModule, useLiveKitStore); similarly import the AudioService type
only and type the dynamic audioService as AudioService | Record<string, never>,
ensuring it exposes playStartTransmittingSound and playStopTransmittingSound
(reference: audioService, AudioService, playStartTransmittingSound,
playStopTransmittingSound) while keeping the runtime require/try-catch to avoid
circular imports.
| private handleAudioRouteChange(event: any): void { | ||
| logger.debug({ | ||
| message: 'Audio route changed', | ||
| context: { event }, | ||
| }); | ||
|
|
||
| // Update bluetooth audio store with new route info if needed | ||
| const bluetoothStore = useBluetoothAudioStore.getState(); | ||
| if (event?.newRoute?.includes('Bluetooth') || event?.newRoute?.includes('AirPods')) { | ||
| bluetoothStore.setAudioRoutingActive(true); | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid leaving bluetooth routing state stuck on true.
setAudioRoutingActive(true) is called for Bluetooth/AirPods, but there’s no path to set it back to false when the route changes away. This can leave stale state in the store.
💡 Suggested tweak
- if (event?.newRoute?.includes('Bluetooth') || event?.newRoute?.includes('AirPods')) {
- bluetoothStore.setAudioRoutingActive(true);
- }
+ const isBluetoothRoute =
+ event?.newRoute?.includes('Bluetooth') || event?.newRoute?.includes('AirPods');
+ bluetoothStore.setAudioRoutingActive(Boolean(isBluetoothRoute));📝 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.
| private handleAudioRouteChange(event: any): void { | |
| logger.debug({ | |
| message: 'Audio route changed', | |
| context: { event }, | |
| }); | |
| // Update bluetooth audio store with new route info if needed | |
| const bluetoothStore = useBluetoothAudioStore.getState(); | |
| if (event?.newRoute?.includes('Bluetooth') || event?.newRoute?.includes('AirPods')) { | |
| bluetoothStore.setAudioRoutingActive(true); | |
| } | |
| } | |
| private handleAudioRouteChange(event: any): void { | |
| logger.debug({ | |
| message: 'Audio route changed', | |
| context: { event }, | |
| }); | |
| // Update bluetooth audio store with new route info if needed | |
| const bluetoothStore = useBluetoothAudioStore.getState(); | |
| const isBluetoothRoute = | |
| event?.newRoute?.includes('Bluetooth') || event?.newRoute?.includes('AirPods'); | |
| bluetoothStore.setAudioRoutingActive(Boolean(isBluetoothRoute)); | |
| } |
🤖 Prompt for AI Agents
In `@src/services/headset-button.service.ts` around lines 262 - 273,
handleAudioRouteChange currently only sets bluetooth routing to true (via
bluetoothStore.setAudioRoutingActive(true)) when event.newRoute contains
"Bluetooth" or "AirPods", leaving the store stuck true when the route changes
away; update the method to setAudioRoutingActive(false) when event?.newRoute
does not include those strings or is missing, i.e., determine a boolean
isBluetoothRoute = !!event?.newRoute && (event.newRoute.includes('Bluetooth') ||
event.newRoute.includes('AirPods')) and call
bluetoothStore.setAudioRoutingActive(isBluetoothRoute) so the store is correctly
toggled both on and off.
| /** | ||
| * Execute the appropriate action based on button event and click count | ||
| */ | ||
| private async executeButtonAction(event: HeadsetButtonEvent, clickCount: number): Promise<void> { | ||
| const liveKitStore = getLiveKitStore().getState(); | ||
|
|
||
| // Only process if we have an active LiveKit connection | ||
| if (!liveKitStore.currentRoom || !liveKitStore.isConnected) { | ||
| logger.debug({ | ||
| message: 'No active LiveKit room, ignoring headset button', | ||
| context: { event, clickCount }, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Handle based on button type and click count | ||
| if ((event.type === 'play_pause' || event.type === 'hook') && clickCount === 1) { | ||
| // Single click - toggle mute based on config | ||
| if (this.config.playPauseAction === 'toggle_mute') { | ||
| await this.toggleMicrophone(); | ||
| } | ||
| } else if ((event.type === 'play_pause' || event.type === 'hook') && clickCount === 2) { | ||
| // Double click | ||
| if (this.config.doubleClickAction === 'toggle_mute') { | ||
| await this.toggleMicrophone(); | ||
| } | ||
| } | ||
|
|
||
| logger.info({ | ||
| message: 'Headset button action executed', | ||
| context: { event, clickCount, action: 'toggle_mute' }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
longPressAction and PTT modes are currently ignored.
HeadsetButtonConfig.longPressAction and pttMode are exposed, but executeButtonAction only handles single/double click toggles. Also, LONG_PRESS_THRESHOLD, longPressTimer, and isButtonHeld are unused. If the UI exposes long-press or push‑to‑talk, this will never fire.
Please either implement the long‑press / PTT behavior or remove those options from the config to avoid misleading behavior.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.