Skip to content

Conversation

@ucswift
Copy link
Member

@ucswift ucswift commented Jan 22, 2026

Summary by CodeRabbit

  • New Features

    • Add Note bottom sheet, PTT channel selector and full PTT controls (hook), web full-screen & embedded location pickers, web-optimized sidebar/layout
  • Improvements

    • Web color-scheme theming, panels grow/shrink when collapsed, onboarding text internationalized, "Units On Scene" → "Personnel Available", selected theme now defaults to dark, Mapbox GL CSS updated
  • Chores

    • Removed Mapbox client key from client env; updated web start script
  • Tests

    • Expanded tests for PTT, Add Note sheet, side menu, and theme selection

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Warning

Rate limit exceeded

@ucswift has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 40 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Removed client MAPBOX_DLKEY; added Push‑To‑Talk hook, channel selector, and LiveKit/Android foreground service APIs; introduced AddNoteBottomSheet and wiring; added Mapbox web pickers and web color‑scheme layout (WebSidebar); renamed unitsOnScenepersonnelAvailable; expanded translations and tests.

Changes

Cohort / File(s) Summary
Environment & Scripts
env.js, src/lib/env.js, package.json
Removed MAPBOX_DLKEY from client env schema/population; web runtime env now merges with Expo env and forces IS_MOBILE_APP=false; web npm script changed to cross-env expo start --web --clear.
PTT Voice System
src/hooks/use-ptt.ts, src/hooks/__tests__/use-ptt.test.ts, src/stores/app/livekit-store.ts, src/components/dispatch-console/ptt-interface.tsx, src/components/dispatch-console/ptt-channel-selector.tsx
New usePTT hook (connect/disconnect, transmit, mute, channel selection, platform/CallKeep handling); LiveKit store adds async disconnect and Android foreground service helpers; PTT interface refactored to hook-driven flow; new channel selector and tests.
Add Note / Dispatch Notes
src/components/dispatch-console/add-note-bottom-sheet.tsx, src/components/dispatch-console/__tests__/add-note-bottom-sheet.test.tsx, src/components/dispatch-console/index.ts, src/app/(app)/home.tsx, src/app/(app)/home.web.tsx
New AddNoteBottomSheet component and export; home screens and NotesPanel wired with onNewNote/isAddNoteSheetOpen; categories fetched, notes saved via API, activity logging/refresh; tests added.
Web Layout & Theming
src/app/(app)/_layout.tsx, src/components/sidebar/web-sidebar.tsx, src/lib/hooks/use-selected-theme.tsx, src/lib/hooks/use-selected-theme.web.tsx, src/lib/hooks/__tests__/use-selected-theme.test.ts
Adds web color‑scheme detection and a web-specific layout path with a simplified drawer/sidebar; new WebSidebar; selected-theme default now falls back to 'dark' and web hook reads localStorage; tests updated.
Sidebar & Navigation
src/components/sidebar/side-menu.tsx, src/components/sidebar/__tests__/side-menu.test.tsx, src/components/sidebar/side-menu.web.tsx
Replaced prior web placeholder with full SideMenu implementation (hierarchical items, expand/collapse, routing); added colorScheme prop and MenuItem typing; removed old web file and added web-sidebar.tsx; tests expanded.
Dispatch Console Panels & Stats
src/components/dispatch-console/stats-header.tsx, src/components/dispatch-console/active-calls-panel.tsx, src/components/dispatch-console/activity-log-panel.tsx, src/components/dispatch-console/map-widget.tsx, src/components/dispatch-console/notes-panel.tsx, src/components/dispatch-console/personnel-panel.tsx, src/components/dispatch-console/units-panel.tsx
Replaced unitsOnScene with personnelAvailable prop/label; unified search rows into HStack; multiple panels now apply flex-1 only when expanded; minor theming/import adjustments.
Maps — Web & RN style normalization
src/components/maps/full-screen-location-picker.web.tsx, src/components/maps/location-picker.web.tsx, src/components/maps/location-picker.tsx, src/components/maps/full-screen-location-picker.tsx, src/components/maps/static-map.tsx, src/components/maps/unified-map-view*.tsx, src/components/maps/pin-marker.tsx, src/components/maps/static-map.web.tsx
Added Mapbox-based full-screen web picker and web LocationPicker; updated Mapbox CSS version on web; multiple RN components migrated to StyleSheet.flatten for style arrays; new web map components exported.
LiveKit Call & Store
src/features/livekit-call/components/LiveKitCallModal.tsx, src/stores/app/livekit-store.ts
handleLeaveRoom now awaits disconnectFromRoom(); LiveKit store exposes async disconnect and Android foreground service start/stop APIs.
Onboarding & Translations
src/app/onboarding.tsx, src/translations/{en,es,ar}.json
Internationalized onboarding into multi-screen content; renamed dispatch.units_on_scenedispatch.personnel_available; added voice/channel and note-related translation keys and other UI strings.
Tests & Test Updates
src/components/sidebar/__tests__/side-menu.test.tsx, src/components/dispatch-console/__tests__/add-note-bottom-sheet.test.tsx, src/hooks/__tests__/use-ptt.test.ts, src/lib/hooks/__tests__/use-selected-theme.test.ts
Added/expanded tests for sidebar, add-note sheet, usePTT, and selected-theme behavior with extensive mocking.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PTTInterface as PTT Interface
    participant usePTT as usePTT Hook
    participant LiveKitStore as LiveKit Store
    participant Platform as Platform Services

    User->>PTTInterface: Select channel / Connect
    PTTInterface->>usePTT: connect(channel)
    activate usePTT
    usePTT->>LiveKitStore: connectToRoom(selectedChannel)
    activate LiveKitStore
    LiveKitStore->>Platform: start foreground/session (Android/iOS)
    Platform-->>LiveKitStore: service started
    LiveKitStore-->>usePTT: connection established
    deactivate LiveKitStore
    usePTT-->>PTTInterface: isConnected, currentChannel
    deactivate usePTT

    User->>PTTInterface: Press PTT
    PTTInterface->>usePTT: startTransmitting()
    usePTT->>LiveKitStore: enable mic / publish audio
    usePTT-->>PTTInterface: isTransmitting = true

    User->>PTTInterface: Release PTT
    PTTInterface->>usePTT: stopTransmitting()
    usePTT->>LiveKitStore: disable mic / stop publish
    usePTT-->>PTTInterface: isTransmitting = false

    User->>PTTInterface: Disconnect
    PTTInterface->>usePTT: disconnect()
    usePTT->>LiveKitStore: disconnectFromRoom()
    LiveKitStore->>Platform: stop foreground/service
    LiveKitStore-->>usePTT: disconnected
    usePTT-->>PTTInterface: isConnected = false
Loading
sequenceDiagram
    participant User
    participant NotesPanel as Notes Panel
    participant AddNoteSheet as AddNoteBottomSheet
    participant API as Note API
    participant ActivityLog as Activity Log

    User->>NotesPanel: Click "Add Note"
    NotesPanel->>AddNoteSheet: open (isOpen = true)
    activate AddNoteSheet
    AddNoteSheet->>API: getNoteCategories()
    API-->>AddNoteSheet: categories
    AddNoteSheet->>User: show form

    User->>AddNoteSheet: Submit
    AddNoteSheet->>API: saveNote(data)
    API-->>AddNoteSheet: saved note
    AddNoteSheet->>ActivityLog: log note created
    AddNoteSheet-->>NotesPanel: onNoteAdded()
    NotesPanel->>API: refresh notes
    AddNoteSheet->>User: close sheet
    deactivate AddNoteSheet
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Poem

🐰
I hopped through channels, dark and bright,
Sidebars, notes, and maps took flight,
Web slipped on a cozy nightly cloak,
I pressed PTT — the voices spoke,
A little rabbit cheers: delight.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Develop' is vague and generic, failing to convey any meaningful information about the changeset's primary purpose. Replace with a descriptive title summarizing the main change, such as 'Add PTT voice integration, note management, and web layout improvements' or similar.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@package.json`:
- Line 12: The "web" npm script currently omits the EXPO_NO_DOTENV=1 prefix,
causing inconsistent dotenv behavior versus other scripts; update the "web"
script (named "web") to include EXPO_NO_DOTENV=1 like the other scripts, or if
the difference is intentional, add an inline comment near the "web" script
explaining why it should load .env (or not) to make the intent explicit and keep
behavior consistent across "start", "prebuild", "android", "ios", build scripts
and "web".

In `@src/app/`(app)/home.tsx:
- Around line 192-201: The availability logic is inconsistent: availableUnits
treats a falsy u.CurrentStatus as available while availablePersonnel requires an
explicit status match; decide and align them—either (A) if missing/null status
should count as available, change the personnel filter (referencing personnel
and p.Status in availablePersonnel) to also treat falsy as available and add a
clarifying comment, or (B) if missing status should not count as available,
remove the !u.CurrentStatus branch from the units filter (referencing units and
u.CurrentStatus in availableUnits) so both filters only count explicit
'available' or 'standing by' statuses and add a brief comment documenting the
chosen semantics.

In `@src/components/dispatch-console/add-note-bottom-sheet.tsx`:
- Around line 85-112: The submit handler onFormSubmit currently only logs errors
but gives no user feedback; add a saveError state (e.g., via useState<string |
null>) and clear it when submission starts (in onFormSubmit before calling
saveNote), then set a user-friendly message into saveError inside the catch
block (using the caught error to enrich the message) and trigger a visible UI
feedback (either render the saveError inline in the Add Note bottom sheet or
call a toast/alert helper) so users see the failure; ensure you still reset
setIsLoading(false) in finally and clear saveError on successful save before
calling onNoteAdded and onClose.

In `@src/components/sidebar/side-menu.tsx`:
- Around line 2-15: The imported symbol Map from lucide-react-native shadows the
global Map constructor; change the import to an aliased name (e.g., import { Map
as MapIcon } from 'lucide-react-native') and then replace all usages of the Map
component in this file (e.g., JSX where <Map ...> is used) with the new alias
(<MapIcon ...>) so the global Map remains unshadowed and icon references still
work.

In `@src/hooks/use-ptt.ts`:
- Around line 231-276: The hook currently defines startAndroidForegroundService
and stopAndroidForegroundService which duplicate foreground service lifecycle
already managed by connectToRoom and disconnectFromRoom in livekit-store.ts;
remove the duplicate management by deleting calls to
startAndroidForegroundService and stopAndroidForegroundService from the hook's
connect/disconnect flow (or remove these two functions entirely from use-ptt.ts)
so only the livekit-store.ts implementations handle Android foreground service
start/stop, ensuring you keep connectToRoom and disconnectFromRoom as the single
source of truth.

In `@src/stores/app/livekit-store.ts`:
- Around line 209-241: The store currently duplicates Android foreground service
logic and defines an unnecessary inner function; consolidate foreground service
management in livekit-store.ts by removing the duplicate start/stop logic from
use-ptt.ts and exposing a single startForegroundService/stopForegroundService
API on the store (or a clearly named methods like startAndroidForegroundService
and stopAndroidForegroundService) that the hook calls; simplify the
implementation in livekit-store.ts by eliminating the nested
startForegroundService function and directly call
notifee.registerForegroundService and notifee.displayNotification with a single
consistent android.channelId (pick one: 'ptt-channel' or 'notif') and keep
logger.debug/error handling around registration and display. Ensure
connectToRoom() no longer starts a separate foreground service and that stop
logic in the store stops the notification/service to avoid duplicate
notifications.
🧹 Nitpick comments (20)
src/components/sidebar/__tests__/side-menu.test.tsx (2)

115-128: Duplicate test assertions.

This test is named "should display menu items with lucide icons" but only verifies label text, making it identical to the "should render all menu items" test at lines 31-43. Consider either removing this test or adding actual icon verification (e.g., via testID on icon wrappers).


130-142: Theme tests only verify rendering, not styles.

These tests confirm the component renders with each colorScheme value but don't verify actual styling differences. This is acceptable as a smoke test given React Native styling is difficult to assert, but consider adding a comment noting the limitation.

src/components/sidebar/side-menu.tsx (1)

92-95: Consider typing routes to avoid as any.

The as any assertion bypasses expo-router's route type checking. If typed routes are configured, consider using the proper route type; otherwise, this is an acceptable workaround.

src/app/onboarding.tsx (1)

24-40: LGTM! Consider extracting to translation files.

The title update to "Resgrid Dispatch" aligns well with the app branding. However, note that these onboarding strings are hardcoded while the rest of the app uses i18n via useTranslation(). Consider moving these to translation files for consistency, especially since the AI summary indicates translation updates are being made across en.json, es.json, and ar.json in this PR.

,

src/components/dispatch-console/active-calls-panel.tsx (2)

3-3: Unused import.

useColorScheme is imported but not used in this component. Remove it to keep imports clean.

🔧 Suggested fix
-import { useColorScheme } from 'nativewind';

152-168: Remove debug logging before merging.

These console.log statements appear to be debug artifacts. Consider removing them or replacing with a conditional debug logger to avoid polluting production logs.

🔧 Suggested fix
-  // Log calls state for debugging
-  useEffect(() => {
-    console.log('[ActiveCallsPanel] Calls updated:', {
-      totalCalls: calls.length,
-      callStates: calls.map((c) => ({ id: c.CallId, name: c.Name, state: c.State })),
-    });
-  }, [calls]);
-
   const activeCalls = useMemo(() => {
     // Filter for active or open calls using utility function
     let filtered = calls.filter((c) => isCallActive(c.State));

-    console.log('[ActiveCallsPanel] Active calls filtered:', {
-      total: calls.length,
-      active: filtered.length,
-      filteredCalls: filtered.map((c) => ({ id: c.CallId, name: c.Name, state: c.State })),
-    });
-
     // Apply search filter
src/components/dispatch-console/units-panel.tsx (1)

2-2: Unused import: useColorScheme

The useColorScheme hook is imported but never used in this component. Remove to keep imports clean.

🧹 Suggested fix
-import { useColorScheme } from 'nativewind';
src/hooks/__tests__/use-ptt.test.ts (1)

465-489: Consider testing callback invocation in AppState change

The AppState tests verify listener setup/cleanup, but don't test the actual callback behavior (e.g., logging when coming to foreground, setting wasConnectedBeforeBackground when going to background). Consider adding a test that invokes the captured listener callback.

💡 Example test for callback behavior
it('should track connection state when going to background', () => {
  let capturedCallback: ((state: AppStateStatus) => void) | null = null;
  const mockRemove = jest.fn();
  const addEventListenerSpy = jest
    .spyOn(require('react-native').AppState, 'addEventListener')
    .mockImplementation((_, callback) => {
      capturedCallback = callback;
      return { remove: mockRemove };
    });

  mockUseLiveKitStore.mockReturnValue({
    isConnected: true,
    // ... other required fields
  });

  renderHook(() => usePTT());

  // Simulate going to background
  capturedCallback?.('background');
  
  // Can verify logging was called or internal state changed
  addEventListenerSpy.mockRestore();
});
src/hooks/use-ptt.ts (1)

151-155: Suppressed exhaustive-deps warning should include fetchVoiceSettings

The eslint-disable comment suppresses the warning for missing fetchVoiceSettings in the dependency array. Since fetchVoiceSettings comes from a Zustand store, it's stable and won't change, so this is acceptable. Consider adding a comment explaining why this is safe.

src/components/dispatch-console/activity-log-panel.tsx (1)

2-2: Unused import: useColorScheme

The useColorScheme hook is imported but never used in this component. Remove to keep imports clean.

🧹 Suggested fix
-import { useColorScheme } from 'nativewind';
src/components/dispatch-console/notes-panel.tsx (1)

2-2: Unused import.

useColorScheme is imported but never used in this file.

🧹 Remove unused import
-import { useColorScheme } from 'nativewind';
src/lib/hooks/use-selected-theme.tsx (1)

36-38: Web platform branch logs but doesn't apply the dark theme.

The log says "defaulting to dark mode" but no colorScheme.set('dark') call follows. Meanwhile, the native path (lines 46-47) explicitly sets colorScheme.set('dark') when no theme is found.

The web-specific file (use-selected-theme.web.tsx) handles this by calling colorScheme.set('dark') in its own loadSelectedTheme. Since the web build will use .web.tsx via bundler resolution, this native file's web branch is likely dead code—but the inconsistency is confusing.

Consider removing the web early-return here entirely (it's unreachable on web builds) or adding the colorScheme.set('dark') call to keep the code self-documenting.

src/components/dispatch-console/__tests__/add-note-bottom-sheet.test.tsx (1)

193-227: Test coverage is minimal.

The current tests verify basic rendering and category fetching. Consider adding tests for:

  • Form submission success and error paths
  • Close button behavior (especially when isLoading is true)
  • Form validation errors display

These can be added incrementally.

src/components/dispatch-console/add-note-bottom-sheet.tsx (2)

141-146: Redundant validation rule.

minLength: 1 is redundant when required is already specified—required ensures the field is non-empty.

🧹 Simplify validation
 rules={{
   required: t('form.required'),
-  minLength: {
-    value: 1,
-    message: t('form.required'),
-  },
 }}

199-204: Same redundant validation as the title field.

🧹 Simplify validation
 rules={{
   required: t('form.required'),
-  minLength: {
-    value: 1,
-    message: t('form.required'),
-  },
 }}
src/components/dispatch-console/stats-header.tsx (1)

88-88: Minor: Redundant static backgroundColor in styles.

The divider style at Line 107 sets a static backgroundColor: '#d1d5db', but it's always overridden inline at Line 88. Consider removing the static value or adding a comment for clarity.

♻️ Optional cleanup
 const styles = StyleSheet.create({
   iconContainer: {
     width: 32,
     height: 32,
     borderRadius: 8,
     alignItems: 'center',
     justifyContent: 'center',
   },
   divider: {
     width: 1,
     height: 24,
-    backgroundColor: '#d1d5db',
+    // backgroundColor set dynamically based on color scheme
   },
 });

Also applies to: 104-108

src/app/(app)/_layout.tsx (1)

399-408: Hardcoded string should use i18n translation.

The "Close Menu" text at Line 404 is hardcoded in English. Use the translation function for consistency with the rest of the app.

♻️ Proposed fix
-              <TouchableOpacity onPress={() => setIsOpen(false)} style={[layoutStyles.closeButton, webTheme.closeButton]}>
-                <RNText style={[layoutStyles.closeButtonText, webTheme.closeButtonText]}>Close Menu</RNText>
+              <TouchableOpacity onPress={() => setIsOpen(false)} style={[layoutStyles.closeButton, webTheme.closeButton]}>
+                <RNText style={[layoutStyles.closeButtonText, webTheme.closeButtonText]}>{t('common.close')}</RNText>
src/components/dispatch-console/ptt-interface.tsx (1)

85-88: Remove duplicate voice settings initialization.

The usePTT hook already calls refreshVoiceSettings() on mount via its internal useEffect with an empty dependency array (line 153 in use-ptt.ts). This additional useEffect in the component causes the same initialization to run twice, resulting in duplicate API calls.

♻️ Proposed fix
-  // Fetch voice settings on mount
-  useEffect(() => {
-    refreshVoiceSettings();
-  }, [refreshVoiceSettings]);
src/app/(app)/home.tsx (2)

291-299: Consider distinct translations for action and description.

Both action and description fields in the activity log entry use similar translation keys (note_created and note_added), which may result in redundant or confusing log entries like "Note Created - Note Added".

Suggested improvement
   const handleNoteAdded = () => {
     fetchNotes();
     addActivityLogEntry({
       type: 'system',
       action: t('dispatch.note_created'),
-      description: t('dispatch.note_added'),
+      description: t('dispatch.new_note_saved_successfully'),
     });
   };

604-607: Minor optimization: memoize containerStyle.

The containerStyle array is recreated on every render. While StyleSheet.flatten is cheap, memoizing this could avoid unnecessary allocations:

Suggested optimization
-  const containerStyle = colorScheme === 'dark' ? [styles.container, styles.containerDark] : [styles.container, styles.containerLight];
+  const containerStyle = useMemo(
+    () => (colorScheme === 'dark' ? [styles.container, styles.containerDark] : [styles.container, styles.containerLight]),
+    [colorScheme]
+  );

"android": "cross-env EXPO_NO_DOTENV=1 expo run:android",
"ios": "cross-env EXPO_NO_DOTENV=1 expo run:ios --device",
"web": "cross-env EXPO_NO_DOTENV=1 expo start --web",
"web": "cross-env expo start --web --clear",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Inconsistent EXPO_NO_DOTENV=1 usage across scripts.

The web script no longer sets EXPO_NO_DOTENV=1, while all other scripts (start, prebuild, android, ios, and build scripts on lines 8-11 and 28-35) consistently use it. This means .env files will be auto-loaded for web but not for other platforms, potentially causing different environment variable behavior between web and native development.

Was this intentional? If so, consider adding a comment explaining why web differs. Otherwise, restore consistency:

Suggested fix to restore consistency
-    "web": "cross-env expo start --web --clear",
+    "web": "cross-env EXPO_NO_DOTENV=1 expo start --web --clear",
📝 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.

Suggested change
"web": "cross-env expo start --web --clear",
"web": "cross-env EXPO_NO_DOTENV=1 expo start --web --clear",
🤖 Prompt for AI Agents
In `@package.json` at line 12, The "web" npm script currently omits the
EXPO_NO_DOTENV=1 prefix, causing inconsistent dotenv behavior versus other
scripts; update the "web" script (named "web") to include EXPO_NO_DOTENV=1 like
the other scripts, or if the difference is intentional, add an inline comment
near the "web" script explaining why it should load .env (or not) to make the
intent explicit and keep behavior consistent across "start", "prebuild",
"android", "ios", build scripts and "web".

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/stores/app/livekit-store.ts (1)

159-216: Stop the existing foreground service when reconnecting to avoid duplicates.

connectToRoom() disconnects an existing room but doesn’t stop the foreground service before starting a new one. This can leave multiple foreground notifications/services active when reconnecting.

🩹 Proposed fix
-      // Disconnect from current room if connected
-      if (currentRoom) {
-        currentRoom.disconnect();
-      }
+      // Disconnect from current room if connected
+      if (currentRoom) {
+        currentRoom.disconnect();
+        if (Platform.OS === 'android') {
+          await get().stopAndroidForegroundService();
+        }
+      }
🤖 Fix all issues with AI agents
In `@src/components/dispatch-console/add-note-bottom-sheet.tsx`:
- Around line 62-67: When the sheet closes the useEffect that currently calls
reset() should also clear the save error state so previous errors don't persist;
update the effect that depends on isOpen and reset to also call the save error
setter (e.g., setSaveError(null) or clearSaveError()) when !isOpen, ensuring
both reset() and the saveError clear happen together in the same block.

In `@src/hooks/use-ptt.ts`:
- Around line 187-191: The CallKeep mute callback currently only calls
setIsMuted and doesn't change the LiveKit microphone; update the callback passed
to callKeepService.setMuteStateCallback to call setMutedInternal(muted) (or
callMutedHandler that syncs mic state) instead of just setIsMuted so the actual
microphone is enabled/disabled; keep any user-interaction sync logic but ensure
setMutedInternal is invoked inside the async callback (e.g., replace
setIsMuted(muted) with await setMutedInternal(muted) or call it directly) to
guarantee the LiveKit mic state matches the UI mute state.

In `@src/stores/app/livekit-store.ts`:
- Around line 89-90: handleLeaveRoom in LiveKitCallModal currently calls
actions.disconnectFromRoom() without awaiting it, causing a race where onClose()
runs before the disconnect completes; make handleLeaveRoom an async function,
await actions.disconnectFromRoom() and only call onClose() after the await so
the disconnect finishes before closing the modal (update the handleLeaveRoom
declaration and any callers to handle the async function accordingly,
referencing handleLeaveRoom, actions.disconnectFromRoom, and onClose).
- Around line 306-342: The startAndroidForegroundService function uses
notifee.displayNotification with channelId 'ptt-channel' but never creates that
channel; before calling notifee.displayNotification (inside
startAndroidForegroundService or during initialization) call
notifee.createChannel with id 'ptt-channel' and appropriate settings so the
notification will be shown on Android 8+; update startAndroidForegroundService
to ensure notifee.createChannel(...) completes (await) prior to the
notifee.displayNotification(...) call.

In `@src/translations/en.json`:
- Around line 692-697: The translation keys for voice channels (voice_disabled,
select_channel, select_channel_description, change_channel_warning,
default_channel, no_channels_available) were added to en.json but are missing
from ar.json and es.json; update both src/translations/ar.json and
src/translations/es.json to include these exact keys with appropriate Arabic and
Spanish translations (or placeholders if needed) so the UI doesn't fall back to
English, ensuring the keys match exactly the identifiers from en.json and
preserving JSON formatting and ordering consistent with the surrounding entries.
🧹 Nitpick comments (4)
src/components/sidebar/side-menu.tsx (4)

32-50: Hardcoded labels should use i18n translations.

The menu labels are hardcoded English strings, but the codebase uses useTranslation for internationalization (as seen in settings.tsx). This inconsistency will cause localization issues.

♻️ Suggested approach
+import { useTranslation } from 'react-i18next';

// Inside the component:
+const { t } = useTranslation();

// Convert static menuItems to a function or move inside component:
-const menuItems: MenuItem[] = [
-  { id: 'home', label: 'Home', icon: Home, route: '/' },
+const getMenuItems = (t: TFunction): MenuItem[] => [
+  { id: 'home', label: t('menu.home'), icon: Home, route: '/' },
   // ... etc
];

65-67: Unused color definitions: buttonBg and buttonText.

These colors are defined but never used in this component. Consider removing them to reduce noise, or add a comment if they're reserved for future use.

♻️ Suggested fix
     childBg: '#ffffff', // white for child items
-    buttonBg: '#2563eb', // blue-600 (matching panel buttons)
-    buttonText: '#ffffff',
   },
   dark: {
     // ...
     childBg: '#111827', // gray-900 for child items
-    buttonBg: '#2563eb', // blue-600 (matching panel buttons)
-    buttonText: '#ffffff',
   },

Also applies to: 79-81


92-95: Consider typed routes instead of as any cast.

The route as any cast bypasses expo-router's type safety. If you have typed routes configured, consider using them; otherwise, this works but loses compile-time route validation.

♻️ Alternative using Href type
+import { useRouter, Href } from 'expo-router';

-  const handleNavigation = (route: string) => {
+  const handleNavigation = (route: Href) => {
     onNavigate?.();
-    router.push(route as any);
+    router.push(route);
   };

Then update MenuItem.route to be Href<string> or keep as string if routes aren't fully typed in your project.


152-156: testID placement may be misleading.

The testID="side-menu-scroll-view" is on the outer View, not the actual ScrollView. This could confuse tests expecting scroll behavior on this element.

♻️ Suggested fix
-    <View style={[styles.container, { backgroundColor: theme.background }]} testID="side-menu-scroll-view">
+    <View style={[styles.container, { backgroundColor: theme.background }]} testID="side-menu-container">
       <View style={[styles.header, { backgroundColor: theme.headerBg, borderBottomColor: theme.headerBorder }]}>
         <Text style={[styles.headerText, { color: theme.headerText }]}>Menu</Text>
       </View>
-      <ScrollView style={styles.scrollView} showsVerticalScrollIndicator={false}>
+      <ScrollView style={styles.scrollView} showsVerticalScrollIndicator={false} testID="side-menu-scroll-view">

Comment on lines +692 to +697
"voice_disabled": "Voice disabled",
"select_channel": "Select Channel",
"select_channel_description": "Choose a voice channel to connect to",
"change_channel_warning": "Selecting a new channel will disconnect from the current one",
"default_channel": "Default",
"no_channels_available": "No voice channels available",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing translations for voice channel keys in ar.json and es.json.

These new voice channel-related keys (voice_disabled, select_channel, select_channel_description, change_channel_warning, default_channel, no_channels_available) are only present in en.json. The Arabic and Spanish translation files are missing these keys, which will cause the UI to fall back to English for non-English users.

Run the following script to confirm the missing keys:

#!/bin/bash
# Description: Check if voice channel keys exist in ar.json and es.json

echo "Checking ar.json for voice channel keys:"
rg -c '"voice_disabled"|"select_channel"|"select_channel_description"|"change_channel_warning"|"default_channel"|"no_channels_available"' src/translations/ar.json || echo "No matches found in ar.json"

echo ""
echo "Checking es.json for voice channel keys:"
rg -c '"voice_disabled"|"select_channel"|"select_channel_description"|"change_channel_warning"|"default_channel"|"no_channels_available"' src/translations/es.json || echo "No matches found in es.json"
🤖 Prompt for AI Agents
In `@src/translations/en.json` around lines 692 - 697, The translation keys for
voice channels (voice_disabled, select_channel, select_channel_description,
change_channel_warning, default_channel, no_channels_available) were added to
en.json but are missing from ar.json and es.json; update both
src/translations/ar.json and src/translations/es.json to include these exact
keys with appropriate Arabic and Spanish translations (or placeholders if
needed) so the UI doesn't fall back to English, ensuring the keys match exactly
the identifiers from en.json and preserving JSON formatting and ordering
consistent with the surrounding entries.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In `@src/app/`(app)/_layout.tsx:
- Around line 369-415: The JSX block defining webTheme and the content variable
(including RNView, SideMenu, CreateDrawerMenuButton, and layoutStyles usage) has
Prettier formatting issues (indentation/trailing spaces); run the project
formatter (npx prettier --write) or your editor's format command on this file to
fix whitespace/indentation, ensure closing JSX tags and array style props (e.g.,
style={[layoutStyles.navBar, { paddingTop: insets.top }, webTheme.navBar]}) are
properly indented, and remove any trailing spaces so the web layout block
compiles cleanly.
- Around line 10-17: The import block in _layout.tsx is failing Prettier because
it spreads multiple named imports across lines; collapse the multi-line import
of ActivityIndicator, Platform, StyleSheet, Text as RNText, TouchableOpacity,
and View as RNView into a single-line import from 'react-native' (or run the
project's Prettier/format command) so the import statement matches the project's
formatting rules.

In `@src/app/`(app)/home.tsx:
- Around line 607-610: The expression assigning containerStyle via useMemo needs
Prettier-consistent formatting; reformat the useMemo callback that returns the
array based on colorScheme so it matches Prettier output (split the conditional
and array elements across lines as Prettier expects) and then run
Prettier/format on the file—update the code referencing containerStyle, useMemo,
colorScheme, and styles.container/styles.containerDark/styles.containerLight
accordingly so CI no longer flags formatting.

In `@src/components/sidebar/side-menu.tsx`:
- Around line 1-18: The import statements at the top of side-menu.tsx are not
sorted per the project's simple-import-sort rules, causing ESLint failures; run
the ESLint autofix (npx eslint --fix src/components/sidebar/side-menu.tsx) or
manually reorder imports into the project's groups (builtin/node, external
packages like react and react-native, scoped/external icons such as
lucide-react-native, hooks like useTranslation and useRouter, then
local/internal imports) so symbols like Href, useRouter, useTranslation, React,
useState, and the lucide icons are in the correct grouped and alphabetical order
to satisfy simple-import-sort.

In `@src/hooks/use-ptt.ts`:
- Around line 177-240: The hook uses setMutedInternal in startCallKeepSession's
dependency array before setMutedInternal is declared, causing a TDZ error; fix
by moving the setMutedInternal useCallback above startCallKeepSession (and
before any other CallKeep-related callbacks that reference it), ensuring
setMutedInternal is declared prior to startCallKeepSession and
endCallKeepSession, and keep existing references to callKeepService,
currentRoom, and setIsMuted unchanged.
♻️ Duplicate comments (1)
src/translations/en.json (1)

490-503: Add menu translations to other locales.

menu.* keys were added here, but the Spanish file in this PR doesn’t include them, so non‑English users will fall back to English. Please add matching keys in other locales.

🧹 Nitpick comments (1)
src/app/(app)/_layout.tsx (1)

580-587: Consider removing unused backdrop style.

The backdrop style is defined but doesn't appear to be used in the current implementation. If it's not needed, consider removing it to keep the codebase clean.

Comment on lines 607 to 610
const containerStyle = useMemo(
() => (colorScheme === 'dark' ? [styles.container, styles.containerDark] : [styles.container, styles.containerLight]),
[colorScheme]
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Prettier warning in containerStyle useMemo.

CI is flagging formatting here—reformat to match Prettier’s output.

🔧 Suggested Prettier-aligned change
-  const containerStyle = useMemo(
-    () => (colorScheme === 'dark' ? [styles.container, styles.containerDark] : [styles.container, styles.containerLight]),
-    [colorScheme]
-  );
+  const containerStyle = useMemo(() => (colorScheme === 'dark' ? [styles.container, styles.containerDark] : [styles.container, styles.containerLight]), [colorScheme]);
📝 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.

Suggested change
const containerStyle = useMemo(
() => (colorScheme === 'dark' ? [styles.container, styles.containerDark] : [styles.container, styles.containerLight]),
[colorScheme]
);
const containerStyle = useMemo(() => (colorScheme === 'dark' ? [styles.container, styles.containerDark] : [styles.container, styles.containerLight]), [colorScheme]);
🧰 Tools
🪛 GitHub Actions: React Native CI/CD

[warning] 607-607: Prettier: Replace ⏎····()·=>·(colorScheme·===·'dark'·?·[styles.container,·styles.containerDark]·:·[styles.container,·styles.containerLight]),⏎····[colorScheme]⎏·· with ()·=>·(colorScheme·===·'dark'·?·[styles.container,·styles.containerDark]·:·[styles.container,·styles.containerLight]),·[colorScheme]

🤖 Prompt for AI Agents
In `@src/app/`(app)/home.tsx around lines 607 - 610, The expression assigning
containerStyle via useMemo needs Prettier-consistent formatting; reformat the
useMemo callback that returns the array based on colorScheme so it matches
Prettier output (split the conditional and array elements across lines as
Prettier expects) and then run Prettier/format on the file—update the code
referencing containerStyle, useMemo, colorScheme, and
styles.container/styles.containerDark/styles.containerLight accordingly so CI no
longer flags formatting.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@src/app/`(app)/_layout.tsx:
- Around line 472-481: The web menu button in CreateDrawerMenuButton lacks an
accessibility label; update the TouchableOpacity returned when Platform.OS ===
'web' to include an accessibilityLabel (e.g., "Open navigation menu") and set
accessibilityRole="button" (and accessible={true}) so screen readers announce
its intent; locate the TouchableOpacity in CreateDrawerMenuButton (the element
with testID="drawer-menu-button" and onPress={() => setIsOpen(true)}) and add
these props while preserving existing styles and behavior.

In `@src/components/sidebar/side-menu.tsx`:
- Around line 20-38: The menu item defined in getMenuItems with id 'new-call'
currently uses the non-existent route '/new-call' causing 404s; update that
item's route property to '/call/new' so it matches the expo-router path for the
page (look for the object with id 'new-call' inside getMenuItems and replace
route: '/new-call' with route: '/call/new').

In `@src/hooks/use-ptt.ts`:
- Around line 248-261: The refreshVoiceSettings function leaves a stale error
when a subsequent refresh succeeds; after a successful await
fetchVoiceSettings() call, clear the error state and notify the error handler by
calling setError(null) and onErrorRef.current?.(null) (or the equivalent reset)
before logging success so previous error messages are removed; keep the catch
block unchanged to set errors on failure.

Comment on lines +472 to +481
const CreateDrawerMenuButton = ({ setIsOpen, colorScheme }: CreateDrawerMenuButtonProps) => {
// Use React Native primitives on web to avoid infinite render loops from gluestack-ui/lucide
if (Platform.OS === 'web') {
const isDark = colorScheme === 'dark';
return (
<TouchableOpacity onPress={() => setIsOpen(true)} testID="drawer-menu-button" style={layoutStyles.menuButton}>
<RNText style={[layoutStyles.menuIcon, { color: isDark ? '#f9fafb' : '#030712' }]}></RNText>
</TouchableOpacity>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add an accessibility label for the web menu button.

Right now the button is only an icon; screen readers won’t announce intent.

🛠️ Proposed fix
-    return (
-      <TouchableOpacity onPress={() => setIsOpen(true)} testID="drawer-menu-button" style={layoutStyles.menuButton}>
+    return (
+      <TouchableOpacity
+        onPress={() => setIsOpen(true)}
+        testID="drawer-menu-button"
+        accessibilityLabel="Open menu"
+        accessibilityRole="button"
+        style={layoutStyles.menuButton}
+      >
         <RNText style={[layoutStyles.menuIcon, { color: isDark ? '#f9fafb' : '#030712' }]}>☰</RNText>
       </TouchableOpacity>
     );
📝 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.

Suggested change
const CreateDrawerMenuButton = ({ setIsOpen, colorScheme }: CreateDrawerMenuButtonProps) => {
// Use React Native primitives on web to avoid infinite render loops from gluestack-ui/lucide
if (Platform.OS === 'web') {
const isDark = colorScheme === 'dark';
return (
<TouchableOpacity onPress={() => setIsOpen(true)} testID="drawer-menu-button" style={layoutStyles.menuButton}>
<RNText style={[layoutStyles.menuIcon, { color: isDark ? '#f9fafb' : '#030712' }]}></RNText>
</TouchableOpacity>
);
}
const CreateDrawerMenuButton = ({ setIsOpen, colorScheme }: CreateDrawerMenuButtonProps) => {
// Use React Native primitives on web to avoid infinite render loops from gluestack-ui/lucide
if (Platform.OS === 'web') {
const isDark = colorScheme === 'dark';
return (
<TouchableOpacity
onPress={() => setIsOpen(true)}
testID="drawer-menu-button"
accessibilityLabel="Open menu"
accessibilityRole="button"
style={layoutStyles.menuButton}
>
<RNText style={[layoutStyles.menuIcon, { color: isDark ? '#f9fafb' : '#030712' }]}></RNText>
</TouchableOpacity>
);
}
🤖 Prompt for AI Agents
In `@src/app/`(app)/_layout.tsx around lines 472 - 481, The web menu button in
CreateDrawerMenuButton lacks an accessibility label; update the TouchableOpacity
returned when Platform.OS === 'web' to include an accessibilityLabel (e.g.,
"Open navigation menu") and set accessibilityRole="button" (and
accessible={true}) so screen readers announce its intent; locate the
TouchableOpacity in CreateDrawerMenuButton (the element with
testID="drawer-menu-button" and onPress={() => setIsOpen(true)}) and add these
props while preserving existing styles and behavior.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Fix all issues with AI agents
In `@src/components/maps/full-screen-location-picker.web.tsx`:
- Around line 47-64: The reverseGeocode function currently parses
response.json() without checking HTTP status; update reverseGeocode to check
response.ok after the fetch (the call using Env.MAPBOX_PUBKEY) and handle
non-2xx responses by logging the status and body (or message), setting
setAddress(undefined) and skipping JSON parsing or throwing to be caught by the
existing catch block; ensure setIsReverseGeocoding(false) still runs in finally
and keep existing state updates (setAddress, setIsReverseGeocoding) in their
current places.
- Line 12: The MAPBOX_GL_CSS_URL constant currently pins the CSS to v3.1.2;
update MAPBOX_GL_CSS_URL to use
'https://api.mapbox.com/mapbox-gl-js/v3.15.0/mapbox-gl.css' so the CSS version
matches the installed mapbox-gl v3.15.0, and make the same change in the other
map components (Map, UnifiedMapView, LocationPicker, StaticMap) that hardcode
the CSS URL so all map controls/popups render correctly.

In `@src/components/maps/location-picker.web.tsx`:
- Around line 88-139: The map never initializes when currentLocation is
initially null because the map container div isn't rendered, causing the
useEffect that creates the Mapbox instance (useEffect that references
map.current and mapContainer.current) to return early; always render the DOM
node referenced by mapContainer (so mapContainer.current is non-null) regardless
of currentLocation and move any UI overlay logic (the "no location" UI) outside
or on top of that div; ensure the existing initializer in the useEffect creates
map.current and marker.current as before and keep getUserLocation() call, so the
effect can run and later updates to currentLocation just update marker.current
via the existing on('click') handler or setCurrentLocation logic.

In `@src/hooks/use-ptt.ts`:
- Around line 248-263: In refreshVoiceSettings (the useCallback defined as
refreshVoiceSettings), remove the success-path invocation
onErrorRef.current?.(null) since onError is typed as (error: string) => void;
instead only call onErrorRef.current when an actual error occurs; keep
setError(null) on success but do not pass null to onErrorRef.current and leave
the catch block unchanged (it should continue to call onErrorRef.current with
the error message).

In `@src/translations/en.json`:
- Around line 500-513: Add the missing "menu" translation block to both ar.json
and es.json by copying the "menu" object from en.json and translating (or at
minimum providing placeholders) for the keys: "calls", "calls_list", "contacts",
"home", "map", "menu", "messages", "new_call", "personnel", "protocols",
"settings", and "units"; ensure the JSON structure matches en.json (same key
names and nesting) so the UI no longer falls back to English.
♻️ Duplicate comments (1)
src/translations/ar.json (1)

725-730: Voice channel translations added - addresses previous review feedback.

The Arabic translations for voice channel keys are now properly included. The translations appear accurate and grammatically correct.

🧹 Nitpick comments (7)
src/components/sidebar/side-menu.tsx (2)

67-75: Consider memoizing menuItems to avoid recreation on each render.

getMenuItems(t) creates a new array and objects on every render. For a small menu this is negligible, but memoizing prevents unnecessary work.

♻️ Optional optimization
 function SideMenu({ onNavigate, colorScheme: propColorScheme }: SideMenuProps): React.JSX.Element {
   const router = useRouter();
   const { t } = useTranslation();
   const [expandedItems, setExpandedItems] = useState<Set<string>>(new Set());
-  const menuItems = getMenuItems(t);
+  const menuItems = React.useMemo(() => getMenuItems(t), [t]);

94-120: Consider adding accessibility labels for screen reader support.

The Pressable elements lack accessibilityLabel and accessibilityRole props, which would improve the experience for users relying on assistive technologies.

♿ Optional accessibility enhancement
       <Pressable
+        accessibilityRole="button"
+        accessibilityLabel={hasChildren ? `${item.label}, expandable` : item.label}
+        accessibilityState={{ expanded: hasChildren ? isExpanded : undefined }}
         onPress={() => {
           if (hasChildren) {
             toggleExpanded(item.id);
src/hooks/use-ptt.ts (1)

54-66: Unused variable voipServerWebsocketSslAddress.

The variable voipServerWebsocketSslAddress is destructured from the store but never used in this hook. The connection uses targetChannel.Token directly. Consider removing it to reduce noise.

♻️ Suggested fix
   const {
     isConnected: storeConnected,
     isConnecting: storeConnecting,
     currentRoom,
     currentRoomInfo,
     isVoiceEnabled,
-    voipServerWebsocketSslAddress,
     availableRooms,
     fetchVoiceSettings,
     connectToRoom,
     disconnectFromRoom,
   } = useLiveKitStore();
src/components/maps/full-screen-location-picker.web.tsx (2)

1-1: Unused import: MapPinIcon

MapPinIcon is imported but never used in this component.

Suggested fix
-import { MapPinIcon, XIcon } from 'lucide-react-native';
+import { XIcon } from 'lucide-react-native';

141-152: Duplicated dragend handler logic

The dragend handler logic is duplicated for the initial marker (lines 142-152) and the marker created on click (lines 172-182). Consider extracting to a shared helper.

Suggested refactor
+  const handleMarkerDragEnd = useCallback(() => {
+    const lngLat = marker.current?.getLngLat();
+    if (lngLat) {
+      const newLocation = {
+        latitude: lngLat.lat,
+        longitude: lngLat.lng,
+      };
+      setCurrentLocation(newLocation);
+      reverseGeocode(newLocation.latitude, newLocation.longitude);
+    }
+  }, [reverseGeocode]);

Then use marker.current.on('dragend', handleMarkerDragEnd); in both places.

Also applies to: 172-182

src/components/maps/location-picker.web.tsx (2)

11-11: CSS version mismatch (same as full-screen variant)

The CSS URL references v3.1.2 while the package is v3.15.0. Consider aligning versions or extracting the CSS URL to a shared constant.


1-217: Consider extracting shared Mapbox logic

This component shares significant code with full-screen-location-picker.web.tsx:

  • CSS injection (lines 34-42)
  • Geolocation logic (lines 44-86)
  • Map initialization pattern (lines 88-139)
  • Marker creation/update

Consider extracting into shared hooks like useMapboxCss(), useGeolocation(), or useMapboxMap() to reduce duplication and ensure consistent behavior.

import { Env } from '@/lib/env';

// Mapbox GL CSS needs to be injected for web
const MAPBOX_GL_CSS_URL = 'https://api.mapbox.com/mapbox-gl-js/v3.1.2/mapbox-gl.css';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find package.json and check mapbox-gl version
find . -name "package.json" -o -name "package-lock.json" -o -name "yarn.lock" | head -5

Repository: Resgrid/Dispatch

Length of output: 282


🏁 Script executed:

# Search for mapbox-gl version in package files
rg "mapbox-gl" --type json -A 2 -B 2 | head -50

Repository: Resgrid/Dispatch

Length of output: 275


🏁 Script executed:

# Check if there are other CSS URL references in the codebase
rg "mapbox-gl.*css" --type ts --type tsx -B 2 -A 2

Repository: Resgrid/Dispatch

Length of output: 87


🏁 Script executed:

# Search for all mapbox-gl version mentions
rg "v3\." -t ts -t tsx -t json | grep -i mapbox | head -20

Repository: Resgrid/Dispatch

Length of output: 87


🏁 Script executed:

# Search for mapbox-gl CSS references with proper syntax
rg "mapbox-gl.*css" --type-add 'web:*.{ts,tsx,jsx,js}' -t web -B 2 -A 2

Repository: Resgrid/Dispatch

Length of output: 4467


🏁 Script executed:

# Check the specific file mentioned in the review
cat -n src/components/maps/full-screen-location-picker.web.tsx | head -30

Repository: Resgrid/Dispatch

Length of output: 1473


🏁 Script executed:

# Search for all version references to mapbox-gl
rg "mapbox" src/ -i -B 1 -A 1

Repository: Resgrid/Dispatch

Length of output: 29765


🌐 Web query:

Mapbox GL JS CSS version compatibility matching package version best practice

💡 Result:

Short best-practice checklist for Mapbox GL JS + CSS version compatibility

  • Always use the CSS file that matches the exact Mapbox GL JS release you’re using (same version number). The Mapbox docs show JS+CSS are distributed together and the CSS is required for proper map controls/popups. [1][2].
  • Install or import the CSS from the same source as the JS (CDN pair or the same npm package path e.g. import "mapbox-gl/dist/mapbox-gl.css" or the CDN URL with the same vX.Y.Z). This avoids runtime layout/style mismatches. [1][2].
  • Lock versions in package.json (or your CDN URLs) and update both JS and CSS together in one change; use exact versions or a lockfile to prevent drifting. (See Mapbox migration guidance when upgrading between major versions.) [1][4].
  • When upgrading between major versions, read the Mapbox changelog/migration guide for any class/markup/style changes that affect plugins or custom CSS; test UI elements (controls, popups, markers) after upgrade. [1][4].
  • If you use forks/alternatives (MapLibre), import that package’s CSS (maplibre‑gl.css) rather than Mapbox’s — class names and CSS details may differ. MapLibre’s migration docs call out replacing the CSS link when switching. [3].
  • For frameworks (react-map-gl, plugins): ensure ancillary type packages or companion libs match the maplib version you use (e.g., @types/mapbox-gl should match mapbox-gl v1/v2 if used). Check the framework’s upgrade notes. [5].

References

  • Mapbox GL JS docs — CDN usage and CSS required. [1][2].
  • Mapbox GL JS migration/changelog guidance. [4].
  • MapLibre migration guide (CSS vs Mapbox differences). [3].
  • react-map-gl upgrade notes on matching maplib/mapbox versions. [5].

Update Mapbox GL CSS version to match installed package (v3.15.0)

The CSS URL hardcodes v3.1.2, but mapbox-gl v3.15.0 is installed. Official Mapbox documentation requires the CSS to match the JavaScript version exactly to ensure proper rendering of map controls and popups. Update the CSS URL and the same hardcoded version in the other map components (src/app/(app)/map.web.tsx, src/components/maps/unified-map-view.web.tsx, src/components/maps/location-picker.web.tsx, and src/components/maps/static-map.web.tsx).

🤖 Prompt for AI Agents
In `@src/components/maps/full-screen-location-picker.web.tsx` at line 12, The
MAPBOX_GL_CSS_URL constant currently pins the CSS to v3.1.2; update
MAPBOX_GL_CSS_URL to use
'https://api.mapbox.com/mapbox-gl-js/v3.15.0/mapbox-gl.css' so the CSS version
matches the installed mapbox-gl v3.15.0, and make the same change in the other
map components (Map, UnifiedMapView, LocationPicker, StaticMap) that hardcode
the CSS URL so all map controls/popups render correctly.

Comment on lines +500 to +513
"menu": {
"calls": "Calls",
"calls_list": "Calls List",
"contacts": "Contacts",
"home": "Home",
"map": "Map",
"menu": "Menu",
"messages": "Messages",
"new_call": "New Call",
"personnel": "Personnel",
"protocols": "Protocols",
"settings": "Settings",
"units": "Units"
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify if menu section exists in ar.json and es.json

echo "Checking for 'menu' section in ar.json:"
rg -n '"menu":\s*\{' src/translations/ar.json || echo "No 'menu' section found in ar.json"

echo ""
echo "Checking for 'menu' section in es.json:"
rg -n '"menu":\s*\{' src/translations/es.json || echo "No 'menu' section found in es.json"

echo ""
echo "Confirming 'menu' section exists in en.json:"
rg -n '"menu":\s*\{' src/translations/en.json || echo "No 'menu' section found in en.json"

Repository: Resgrid/Dispatch

Length of output: 272


Add missing menu section to ar.json and es.json.

The new menu section with navigation keys (calls, calls_list, contacts, home, map, menu, messages, new_call, personnel, protocols, settings, units) is only present in en.json (lines 500-513). Both ar.json and es.json are missing this entire section, which will cause the UI to fall back to English for non-English users.

🤖 Prompt for AI Agents
In `@src/translations/en.json` around lines 500 - 513, Add the missing "menu"
translation block to both ar.json and es.json by copying the "menu" object from
en.json and translating (or at minimum providing placeholders) for the keys:
"calls", "calls_list", "contacts", "home", "map", "menu", "messages",
"new_call", "personnel", "protocols", "settings", and "units"; ensure the JSON
structure matches en.json (same key names and nesting) so the UI no longer falls
back to English.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/components/maps/full-screen-location-picker.web.tsx`:
- Around line 218-259: The early return when isLoading removes the map <div> and
detaches the Mapbox element (mapContainer / map.current) so the useEffect
initialization (which guards with if (map.current) return) never reattaches;
instead always render the map container div (the ref mapContainer) so Mapbox
stays mounted and move the loading UI into an overlay that sits on top
(absolute/fixed within styles.container) when isLoading is true; remove the
early return branch that returns only the loading view and keep existing
functions/refs (mapContainer, map.current, useEffect) intact so the map instance
remains bound while the overlay shows loading.
- Around line 74-101: getUserLocation currently only updates an existing marker
(marker.current) so when initialLocation is null no marker is created; update
getUserLocation to mirror the map click handler by creating a new marker when
marker.current is falsy: instantiate marker.current = new mapboxgl.Marker({/*
same options as click handler */}).setLngLat([newLocation.longitude,
newLocation.latitude]).addTo(map.current), attach the same dragend handler (if
used), and ensure setCurrentLocation(newLocation),
reverseGeocode(newLocation.latitude, newLocation.longitude) and
setIsLoading(false) are called as before; keep all map movement logic using
map.current.flyTo unchanged.
♻️ Duplicate comments (1)
src/components/maps/location-picker.web.tsx (1)

147-174: Keep the map container mounted during loading to avoid a blank map

When isLoading is true, the component returns early and unmounts the map <div>. The Mapbox instance remains bound to the old DOM node and won’t reinitialize, so the map can stay blank after loading completes. Keep the map container always mounted and overlay the loading UI instead. Please verify by running the web picker, clicking “Get my location,” and confirming the map remains visible after loading.

🛠️ Suggested fix (keep map mounted + overlay loading)
-  if (isLoading) {
-    return (
-      <View style={StyleSheet.flatten([styles.container, { height }])}>
-        <View style={styles.loadingContainer}>
-          <Text style={styles.loadingText}>{t('common.loading')}</Text>
-        </View>
-      </View>
-    );
-  }
-
   return (
     <View style={StyleSheet.flatten([styles.container, { height }])}>
       <div ref={mapContainer} style={{ height: '100%', width: '100%' }} />
-      {!currentLocation ? (
+      {isLoading && (
+        <View style={[styles.loadingContainer, StyleSheet.absoluteFill]}>
+          <Text style={styles.loadingText}>{t('common.loading')}</Text>
+        </View>
+      )}
+      {!isLoading && !currentLocation ? (
         <View style={styles.noLocationContainer}>
           <Text style={styles.noLocationText}>{t('common.no_location')}</Text>
           <Pressable onPress={getUserLocation}>
             <Text style={styles.getLocationText}>{t('common.get_my_location')}</Text>
           </Pressable>
         </View>
       ) : null}
       <View style={styles.buttonContainer}>
-        <Button onPress={handleConfirmLocation} disabled={!currentLocation}>
+        <Button onPress={handleConfirmLocation} disabled={!currentLocation || isLoading}>
           <ButtonText>{t('common.confirm_location')}</ButtonText>
         </Button>
       </View>
     </View>
   );

ucswift and others added 3 commits January 22, 2026 12:18
@ucswift
Copy link
Member Author

ucswift commented Jan 22, 2026

Approve

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is approved.

@ucswift ucswift merged commit 31b86d4 into master Jan 22, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants