Conversation
This comment has been minimized.
This comment has been minimized.
Contributor
📝 WalkthroughWalkthroughAdds Incident Command end-to-end, expands call detail with audio/reschedule/dispatch-more actions, introduces dispatch dashboard view toggles and combined resources, adds weather alert settings management, adds modern notification sounds, renders active custom map layers, and adds check-in PAR support. ChangesIncident Command Feature
Call Detail Enhancements
Dispatch Dashboard View Toggles and Resources Panel
Weather Alert Settings Management
Modern Notification Sounds
Active Custom Map Layer Rendering
Check-in Personnel Accountability (PAR)
Repository Configuration
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant IncidentCommandTab
participant IncidentCommandStore
participant IncidentCommandAPI
participant SignalRStore
User->>IncidentCommandTab: Open call detail Command tab
IncidentCommandTab->>IncidentCommandStore: loadForCall(callId)
IncidentCommandStore->>IncidentCommandAPI: getCommandBoard, getMyCapabilities, getTimeline
IncidentCommandAPI-->>IncidentCommandStore: board, capabilities, timeline
IncidentCommandStore-->>IncidentCommandTab: board state
User->>IncidentCommandTab: Establish command
IncidentCommandTab->>IncidentCommandStore: establish(input)
IncidentCommandStore->>IncidentCommandAPI: establishCommand(input)
IncidentCommandAPI-->>IncidentCommandStore: result
SignalRStore-->>IncidentCommandStore: incidentCommandUpdated(callId)
IncidentCommandStore->>IncidentCommandAPI: reload board data
sequenceDiagram
participant User
participant CallDetailWeb
participant DispatchSelectionModal
participant DispatchHelpers
participant CallDetailStore
participant CallsAPI
User->>CallDetailWeb: Tap Dispatch more
CallDetailWeb->>DispatchSelectionModal: open with existing dispatches
User->>DispatchSelectionModal: select additional resources
DispatchSelectionModal-->>CallDetailWeb: DispatchSelection
CallDetailWeb->>DispatchHelpers: buildAddResourcesUpdateRequest(call, dispatches, selection)
DispatchHelpers-->>CallDetailWeb: UpdateCallRequest
CallDetailWeb->>CallDetailStore: updateCall(request)
CallDetailStore->>CallsAPI: updateCall(request)
CallsAPI-->>CallDetailStore: response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/calls/reschedule-call-sheet.tsx (1)
1-103: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftMissing test coverage for new component.
No test file was provided alongside this new
RescheduleCallSheetcomponent. As per coding guidelines, "Generate tests for all components, services and logic generated. Ensure tests run without errors and fix any issues."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/calls/reschedule-call-sheet.tsx` around lines 1 - 103, Add test coverage for the new RescheduleCallSheet component. Create a test file that renders RescheduleCallSheet, mocks useCallDetailStore/useScheduledCallsStore/useCallsStore/useToastStore and useTranslation, and verifies the main flows: preset buttons update the input, invalid datetime blocks submit with an error toast, and a valid submit calls rescheduleDispatchTime, refreshes both stores, and closes the sheet. Reference the RescheduleCallSheet component and handleSubmit/applyPreset behavior so the tests stay resilient if layout changes.Source: Coding guidelines
src/components/calls/call-audio-modal.tsx (1)
1-217: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftMissing test coverage for new component.
No test file was provided alongside this new
CallAudioModalcomponent. As per coding guidelines, "Generate tests for all components, services and logic generated. Ensure tests run without errors and fix any issues."Want me to generate a Jest/RTL test suite for
CallAudioModal(open/close lifecycle, play/pause, error/empty states)?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/calls/call-audio-modal.tsx` around lines 1 - 217, The new CallAudioModal component needs test coverage. Add a Jest/RTL test suite for CallAudioModal that verifies the open/close lifecycle, fetchCallAudio behavior, playback controls in handlePlay/unloadSound, and the loading/error/empty states rendered by renderContent. Use the component’s key symbols such as CallAudioModal, handlePlay, unloadSound, and renderContent to locate behaviors, and mock the bottom sheet, expo-av Audio, useCallDetailStore, useAnalytics, and logger so the tests run reliably.Source: Coding guidelines
src/stores/weatherAlerts/store.ts (1)
272-290: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
reset()doesn't clearsettings.This hunk adds
history,sources,zones, and the loading/saving flags toreset(), butsettings(initialized tonullat line 79) is never reset here. Since Zustand'sset()merges partial state,settingswill retain its previous value across areset()call (e.g., logout/department switch), potentially showing stale weather-alert settings untilfetchSettings()runs again.🩹 Proposed fix
reset: () => { set({ alerts: [], isLoading: false, error: null, selectedAlert: null, isLoadingDetail: false, + settings: null, nearbyAlerts: [], isLoadingNearby: false, history: [],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/weatherAlerts/store.ts` around lines 272 - 290, The weather-alert store’s reset action is leaving stale state behind because it does not clear the settings field. Update the reset logic in the weatherAlerts store’s reset() method so it explicitly restores settings to null along with the other state fields already being reset. Make sure the change is applied in the same reset() function that clears alerts, history, sources, zones, and the loading flags.
🟡 Minor comments (14)
src/components/calls/call-audio-modal.tsx-184-214 (1)
184-214: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBottom sheet chrome ignores dark mode; duplicates
CustomBottomSheet.
backgroundStyle/handleIndicatorStyleare hardcoded light-theme colors, while the sheet's own content usesdark:classes extensively — the sheet chrome will look inconsistent in dark mode.reschedule-call-sheet.tsxreuses the sharedCustomBottomSheetcomponent instead of a raw@gorhom/bottom-sheetinstance; consider doing the same here for consistency and to inherit its theming. As per coding guidelines, "Ensure support for dark mode and light mode."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/calls/call-audio-modal.tsx` around lines 184 - 214, The call-audio modal’s BottomSheet styling is hardcoded for light mode, and it bypasses the shared sheet wrapper used elsewhere. Update the call-audio modal in call-audio-modal.tsx so the BottomSheet chrome (background and handle indicator) respects dark mode like the content does, and consider switching the modal to the shared CustomBottomSheet component used by reschedule-call-sheet.tsx to keep theming consistent. Use the existing call-audio-modal component and its BottomSheet props as the place to apply the dark/light mode handling.Source: Coding guidelines
src/components/settings/modern-notification-sounds-item.tsx-32-39 (1)
32-39: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSwitch lacks an accessible name for screen readers.
The
Switchhas noaccessibilityLabel, so assistive technology won't announce what it controls independent of the adjacentText.♿ Proposed fix
- <View className="flex-row items-center"> - <Switch size="md" value={isModernNotificationSoundsEnabled} onValueChange={handleToggle} /> - </View> + <View className="flex-row items-center"> + <Switch size="md" value={isModernNotificationSoundsEnabled} onValueChange={handleToggle} accessibilityLabel={t('settings.modern_notification_sounds')} /> + </View>As per coding guidelines, "Ensure the app is accessible, following WCAG guidelines for mobile applications."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/settings/modern-notification-sounds-item.tsx` around lines 32 - 39, The Switch in modern-notification-sounds-item.tsx is missing an accessible name for screen readers. Update the Switch element in the modern notification sounds item so it includes an accessibilityLabel that clearly describes what it controls, using the existing translation text or equivalent label near handleToggle and isModernNotificationSoundsEnabled.Source: Coding guidelines
src/app/(app)/weather-alerts/settings.tsx-193-217 (1)
193-217: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winIcon-only edit/delete buttons lack accessible labels.
The edit/delete
Buttons render only aButtonIconwith no text oraccessibilityLabel, making them unusable for screen-reader users. As per coding guidelines, "Ensure the app is accessible, following WCAG guidelines for mobile applications."♿ Proposed fix (sketch)
- <Button + <Button + accessibilityLabel={t('weatherAlerts.settings.edit_zone')} variant="link" size="xs" onPress={() => { setEditingZone(zone); setZoneSheetOpen(true); }} > <ButtonIcon as={PencilIcon} size="xs" /> </Button> <Button + accessibilityLabel={t('weatherAlerts.settings.delete_zone_confirm')} variant="link" size="xs" onPress={() => ... } > <ButtonIcon as={Trash2Icon} size="xs" className="text-red-500" /> </Button>(apply similarly to the source edit/delete buttons)
Also applies to: 255-279
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/weather-alerts/settings.tsx around lines 193 - 217, The icon-only edit/delete controls in the settings screen are missing accessible labels, so screen readers cannot identify them. Update the Button components that wrap PencilIcon and Trash2Icon to include an accessibilityLabel (or equivalent accessible text) that clearly describes each action, following the same pattern used by the other source edit/delete buttons in this file. Apply the same fix to the duplicate controls mentioned elsewhere in the component.Source: Coding guidelines
src/components/weatherAlerts/weather-alert-zone-sheet.tsx-85-93 (1)
85-93: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winHardcoded placeholder strings bypass i18n.
placeholder="e.g. TXZ211"(line 85) andplaceholder="lat,lng"(line 93) are hardcoded English text, not routed throught(). As per coding guidelines, "Ensure all text is wrapped int()fromreact-i18nextfor translations."🌐 Proposed fix
- <InputField value={zoneCode} onChangeText={setZoneCode} placeholder="e.g. TXZ211" autoCapitalize="characters" /> + <InputField value={zoneCode} onChangeText={setZoneCode} placeholder={t('weatherAlerts.settings.zone_code_placeholder')} autoCapitalize="characters" /> ... - <InputField value={center} onChangeText={setCenter} placeholder="lat,lng" autoCapitalize="none" /> + <InputField value={center} onChangeText={setCenter} placeholder={t('weatherAlerts.settings.center_geo_placeholder')} autoCapitalize="none" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/weatherAlerts/weather-alert-zone-sheet.tsx` around lines 85 - 93, The placeholders in weather-alert-zone-sheet.tsx are hardcoded and bypass i18n; update the InputField usages in the zoneCode and center inputs to pull placeholder text through t() like the existing FormControlLabelText does. Use the same translation pattern already present in WeatherAlertZoneSheet so the zone code and center field placeholders are localized instead of inline English strings.Source: Coding guidelines
src/app/(app)/weather-alerts/settings.tsx-41-51 (1)
41-51: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlert.alert buttons use hardcoded 'Cancel'/'OK' text.
Should route through
t()like the rest of the screen's strings, per coding guidelines' translation requirement.🌐 Proposed fix
- Alert.alert('', message, [ - { text: 'Cancel', style: 'cancel' }, - { text: 'OK', style: 'destructive', onPress: onConfirm }, - ]); + Alert.alert('', message, [ + { text: t('common.cancel'), style: 'cancel' }, + { text: t('common.ok'), style: 'destructive', onPress: onConfirm }, + ]);Note:
confirmDeletewould needtpassed in or converted to a hook, since it's currently defined outside the component.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/weather-alerts/settings.tsx around lines 41 - 51, The confirmDelete helper in settings.tsx uses hardcoded Alert.alert button labels, which should be translated like the rest of the screen. Update confirmDelete so the non-web Alert.alert actions use t()-driven strings instead of literal Cancel/OK, and since confirmDelete is outside the component, pass t into it or move it into the component/hook scope where t is available.Source: Coding guidelines
src/app/(app)/weather-alerts/settings.tsx-26-39 (1)
26-39: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSeverity/source-type labels are hardcoded, not translated.
SEVERITY_OPTIONSlabels andsourceTypeLabel()return hardcoded English strings instead of routing throught(). As per coding guidelines, "Ensure all text is wrapped int()fromreact-i18nextfor translations."🌐 Proposed fix (sketch)
-const SEVERITY_OPTIONS = [ - { value: WeatherAlertSeverity.Extreme, label: 'Extreme' }, - { value: WeatherAlertSeverity.Severe, label: 'Severe' }, - { value: WeatherAlertSeverity.Moderate, label: 'Moderate' }, - { value: WeatherAlertSeverity.Minor, label: 'Minor' }, - { value: WeatherAlertSeverity.Unknown, label: 'Unknown' }, -]; +const useSeverityOptions = () => { + const { t } = useTranslation(); + return [ + { value: WeatherAlertSeverity.Extreme, label: t('weatherAlerts.severity.extreme') }, + { value: WeatherAlertSeverity.Severe, label: t('weatherAlerts.severity.severe') }, + { value: WeatherAlertSeverity.Moderate, label: t('weatherAlerts.severity.moderate') }, + { value: WeatherAlertSeverity.Minor, label: t('weatherAlerts.severity.minor') }, + { value: WeatherAlertSeverity.Unknown, label: t('weatherAlerts.severity.unknown') }, + ]; +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/weather-alerts/settings.tsx around lines 26 - 39, The severity and source-type display strings in settings.tsx are hardcoded English and should be translated via react-i18next. Update SEVERITY_OPTIONS and sourceTypeLabel() to use t() for each label, and ensure the component has access to the translation hook or function so the labels are resolved through the existing i18n keys rather than inline text.Source: Coding guidelines
src/components/weatherAlerts/weather-alert-source-sheet.tsx-67-67 (1)
67-67: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNo validation prevents a negative poll interval.
parseInt(pollInterval, 10) || 15only falls back to 15 when the parsed value is falsy (e.g.0orNaN); a negative string like"-5"parses to-5(truthy) and is sent as-is.💚 Suggested fix
- PollIntervalMinutes: parseInt(pollInterval, 10) || 15, + PollIntervalMinutes: Math.max(1, parseInt(pollInterval, 10) || 15),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/weatherAlerts/weather-alert-source-sheet.tsx` at line 67, The PollIntervalMinutes value in weather-alert-source-sheet.tsx can still accept negative input because parseInt(pollInterval, 10) only falls back on falsy values, not invalid ranges. Update the logic around the PollIntervalMinutes assignment in the weather alert source sheet flow to validate that the parsed poll interval is a positive number, and fall back to the default 15 when the input is NaN, zero, or negative.src/app/(app)/map.web.tsx-52-54 (1)
52-54: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winActive custom layers aren't refreshed on screen focus.
Same gap as the native screen:
fetchLayers()refreshes legacy vector layers on focus, butrefetchActiveLayersis never called, so newly toggled custom-map layers won't appear until a full remount.🔄 Proposed fix
- const { activeLayers } = useActiveMapLayers(); + const { activeLayers, refetchActiveLayers } = useActiveMapLayers();useFocusEffect( useCallback(() => { fetchLayers(); - }, [fetchLayers]) + refetchActiveLayers(); + }, [fetchLayers, refetchActiveLayers]) );Also applies to: 161-166
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/map.web.tsx around lines 52 - 54, The custom-map layers are not being refreshed when the screen regains focus because only the legacy vector layer refresh path is wired up. Update the map focus handling in the web screen logic around useActiveMapLayers, fetchLayers, and refetchActiveLayers so both legacy layers and active custom layers are reloaded on focus. Make sure the focus effect invokes refetchActiveLayers alongside the existing fetchLayers call, matching the native screen behavior so newly toggled layers appear without a remount.src/app/(app)/map.tsx-59-61 (1)
59-61: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winActive custom layers aren't refreshed on screen focus.
fetchLayers()is called in the focus effect to refresh legacy vector layers, butrefetchActiveLayers(exposed by the hook) is never invoked, so newly added/removed on-by-default custom-map layers won't appear until the app fully restarts/remounts this screen.🔄 Proposed fix
- const { activeLayers } = useActiveMapLayers(); + const { activeLayers, refetchActiveLayers } = useActiveMapLayers();// Refresh layers when map is focused fetchLayers(); + refetchActiveLayers();- }, [isMapReady, location.latitude, location.longitude, location.isMapLocked, location.heading, fetchLayers]) + }, [isMapReady, location.latitude, location.longitude, location.isMapLocked, location.heading, fetchLayers, refetchActiveLayers])Also applies to: 128-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/map.tsx around lines 59 - 61, Active custom map layers are not being refreshed when the screen regains focus. In the focus effect inside map.tsx, alongside fetchLayers() for legacy vector layers, invoke refetchActiveLayers from useActiveMapLayers so newly added/removed on-by-default layers are reloaded on focus; update the effect dependencies accordingly and keep the hook call near activeLayers/refetchActiveLayers so the screen reflects changes without a full remount.src/components/dispatch-console/dashboard-view-toggles.tsx-44-65 (1)
44-65: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace
gapwith margins for web compatibility.
rowandpillstyles use thegapproperty, which the project guideline explicitly disallows due to inconsistent web support.🩹 Proposed fix using margins
const styles = StyleSheet.create({ row: { flexDirection: 'row', - gap: 8, marginBottom: 8, flexWrap: 'wrap', }, pill: { flexDirection: 'row', alignItems: 'center', - gap: 6, paddingHorizontal: 12, paddingVertical: 6, borderRadius: 999, },Then apply spacing on the pill's children (e.g., margin on the
Icon/Textwrapper, or margin between the twoPressables) instead of relying ongap.As per coding guidelines, "Avoid using the
gapproperty in StyleSheet styles as it has inconsistent support on web. Use margin properties instead."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/dispatch-console/dashboard-view-toggles.tsx` around lines 44 - 65, Remove the unsupported gap usage from the dashboard view toggles styles by updating the StyleSheet in dashboard-view-toggles.tsx: the row and pill style entries should use margin-based spacing instead of gap. Keep the existing layout intent in the DashboardViewToggles component by adding spacing between the pill children and between the pill buttons with margins on the relevant Icon/Text wrappers or Pressable elements, so the styling remains web-compatible.Source: Coding guidelines
src/components/incident-command/command-voice.tsx-66-75 (1)
66-75: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSilent failure on mic toggle gives no user feedback.
setTalkingswallows mic errors without surfacing anything to the user. For a PTT control in a safety-critical incident-command flow, a failedsetMicrophoneEnabled(e.g., permission revoked) should notify the user rather than fail silently, since they may believe they are transmitting when they are not.🔔 Proposed fix
const setTalking = async (on: boolean) => { const room = useLiveKitStore.getState().currentRoom; if (!room) return; try { await room.localParticipant.setMicrophoneEnabled(on); useLiveKitStore.getState().setIsTalking(on); } catch { - // ignore transient mic errors + showToast('error', t('incident_command.voice_join_error')); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/incident-command/command-voice.tsx` around lines 66 - 75, The `setTalking` handler in `command-voice.tsx` is swallowing `setMicrophoneEnabled` failures, so the PTT control can fail silently. Update the `catch` path to surface the error to the user with a visible notification or error state, and keep `useLiveKitStore.setIsTalking` unchanged unless the mic toggle succeeds. Make sure the message clearly tells the user the microphone could not be enabled/disabled, and reference `room.localParticipant.setMicrophoneEnabled` and `setTalking` when wiring the fix.src/components/incident-command/command-map.tsx-118-137 (1)
118-137: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDelete-annotation confirmation button is mislabeled "Release" instead of a delete/remove action.
The destructive button in the native delete-annotation
Alertusest('incident_command.release')("Release"), which is the label already used elsewhere for releasing a resource assignment from a lane — not for deleting a map annotation. This mismatched wording on a destructive confirmation could confuse users about what the action actually does.🐛 Proposed fix — use a delete-specific translation key
Alert.alert('', t('incident_command.delete_annotation_confirm'), [ { text: t('common.cancel'), style: 'cancel' }, - { text: t('incident_command.release'), style: 'destructive', onPress: () => void doDelete() }, + { text: t('common.delete'), style: 'destructive', onPress: () => void doDelete() }, ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/incident-command/command-map.tsx` around lines 118 - 137, The native delete-annotation confirmation in confirmDelete uses the wrong destructive label, showing the release action instead of a delete/remove action. Update the Alert button in command-map.tsx to use a delete-specific translation key from incident_command rather than t('incident_command.release'), keeping the existing cancel option and doDelete flow unchanged. Use confirmDelete and the Alert.alert invocation as the place to fix this label.src/components/incident-command/command-map.web.tsx-17-17 (1)
17-17: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the Mapbox GL CSS URL to v3.15.0
src/components/incident-command/command-map.web.tsx:17still loadsv3.1.2even though the repo pinsmapbox-glto^3.15.0and the other web map components already usev3.15.0. Keep the stylesheet version aligned with the installed package to avoid UI drift in controls and markers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/incident-command/command-map.web.tsx` at line 17, The Mapbox GL stylesheet version in command-map.web.tsx is still pinned to an older release, causing it to drift from the installed mapbox-gl package and other web map components. Update the MAPBOX_GL_CSS_URL constant in command-map.web.tsx to match the repo’s v3.15.0 Mapbox GL CSS version, keeping it aligned with the rest of the map components and the pinned dependency.src/app/call/[id].tsx-617-623 (1)
617-623: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse explicit null checks for map coordinates.
DestinationLatitude/DestinationLongitudeare alreadynumber | null, so the parsing concern doesn’t apply;mapLatitude && mapLongitudewill still drop valid0coordinates. UsemapLatitude != null && mapLongitude != nullinstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/call/`[id].tsx around lines 617 - 623, The map coordinate check in the call page should use explicit nullability checks instead of truthiness so valid 0 values are preserved. Update the logic around the destination map selection in the call detail component (the `showDestinationMap`/`mapLatitude`/`mapLongitude` block in `call/[id].tsx`) to rely on `!= null` checks for both latitude and longitude, and ensure any downstream condition that gates rendering or map switching uses the same explicit null check pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(app)/map.web.tsx:
- Around line 36-37: The active layer re-add flow in map.web.tsx is only driven
by the activeLayers/isMapReady effect, so layers disappear after
map.current.setStyle(getMapStyle()) and are not restored on theme changes.
Update the Map component logic around activeSourceIdsRef, activeLayerIdsRef, and
the active-layers effect to re-run the addSource/addLayer path when the new
style finishes loading, ideally by handling style.load (or an equivalent
post-setStyle callback) so runtime-added layers are restored automatically.
In `@src/app/weather-alerts/`[id].tsx:
- Around line 166-168: The conditional rendering in the weather alert detail
rows uses && instead of the required ternary operator. Update the
alert.OnsetUtc, alert.ExpiresUtc, and alert.SentUtc render conditions in the
weather-alerts detail component to use ? : so the DetailRow rendering follows
the coding guideline consistently.
- Around line 15-22: The timestamp formatting in formatTimestamp uses
unsupported tokens in formatDateForDisplay, so the date renders with literal d,
h, and a text. Update the pattern passed to formatDateForDisplay in
formatTimestamp to a supported format such as MMM dd, yyyy hh:mm t, keeping the
rest of the parseDateISOString and fallback logic unchanged.
In `@src/components/calls/call-audio-modal.tsx`:
- Around line 94-117: The current handlePlay flow in call-audio-modal.tsx can
overlap concurrent Audio.Sound.createAsync calls and orphan a previously loaded
sound. Add a request-generation or in-flight guard inside handlePlay (and
coordinate with unloadSound/soundRef.current) so only the latest tap can set
soundRef.current and start playback, and earlier async completions are ignored
or cleaned up before they can overwrite state.
In `@src/components/dispatch-console/active-calls-panel.tsx`:
- Around line 472-493: The handleDispatchAdditional flow is using a potentially
stale callDispatchesMap[call.CallId] when building the updateCall request, which
can overwrite the server state with an incomplete DispatchList. Update
handleDispatchAdditional to ensure the latest snapshot is loaded via
getCallExtraData(call.CallId) immediately before calling
buildAddResourcesUpdateRequest, or prevent dispatch-more until that per-call
dispatch data is available. Keep the fix localized to handleDispatchAdditional,
updateCall, and buildAddResourcesUpdateRequest usage.
In `@src/components/dispatch-console/resources-panel.tsx`:
- Around line 35-46: ResourcesPanel is missing the call-context props that
UnitsPanel and PersonnelPanel already support, so the singleList path loses
call-aware filtering, highlighting, and selection/status actions. Update
ResourcesPanel to accept and use the same call-related props from the dashboard
flow, including isCallFilterActive, selectedCallId, callDispatches,
selectedUnitId/selectedPersonnelId, onSelect* handlers, and onSet*StatusForCall
callbacks, then wire them through the combined units/personnel rendering so
singleList remains a drop-in replacement.
In `@src/components/incident-command/command-board-view.tsx`:
- Around line 42-52: The confirmAction helper uses hardcoded Alert button labels
instead of translated text. Update confirmAction to receive t from react-i18next
(or otherwise access it from the calling component) and wrap the dialog button
labels with t() for both Cancel and OK. Keep the web confirm behavior unchanged,
and make sure the Alert.alert call in command-board-view.tsx uses translated
strings only.
- Around line 298-320: The objective completion check in command-board-view.tsx
is using a repeated store lookup inside the Objectives map, which is unnecessary
and less clear. Update the logic in the objectives rendering loop to use the
current `objective`’s own status directly, ideally via `objective.Status` with
`TacticalObjectiveStatus.Complete` or the existing `isObjectiveComplete(...)`
helper, and remove the extra `find(...)` call from the `board.Objectives.map`
block.
In `@src/components/status/__tests__/status-bottom-sheet.test.tsx`:
- Around line 302-313: The mocked fixture updates are fine, but this test suite
still never exercises the real
`useStatusBottomSheetStore`/`fetchDestinationData` flow, so the primary open
path for the `select-status` step can remain broken. Add a non-mocked
integration or store-level test that uses the real store, calls
`setIsOpen(true)` on the `StatusBottomSheet`, and verifies `availableStatuses`
is populated on open even when no status is pre-selected. Keep the existing
mocked component tests, but add coverage around the real store wiring to catch
regressions in the gating logic.
In `@src/components/status/status-bottom-sheet.tsx`:
- Line 62: The status picker data load is still tied to the selected-status
guard, so the initial no-status flow never triggers the fetch and
availableStatuses stays empty. Update the StatusBottomSheet flow so
fetchDestinationData() runs when the sheet opens or whenever the picker needs
data, not only when selectedStatus is already set. Keep the fix localized to
StatusBottomSheet and the effect that currently depends on selectedStatus, and
make sure the select-status step can render from a preloaded availableStatuses
list.
In `@src/components/weatherAlerts/weather-alert-source-sheet.tsx`:
- Around line 24-28: The SOURCE_TYPE_OPTIONS labels in
weather-alert-source-sheet.tsx are hardcoded strings instead of translated text.
Update the SOURCE_TYPE_OPTIONS definition to use t() from react-i18next for each
label, and make sure the component has access to the translation hook so the
WeatherAlertSourceType values still map to localized display names.
In `@src/lib/dispatch-helpers.ts`:
- Around line 63-102: The additive update payload built by
buildAddResourcesUpdateRequest is still omitting PlusCode and CallFormData,
which can cause existing server-side values to be cleared when updateCall()
normalizes missing fields. Update the buildAddResourcesUpdateRequest flow to
accept and thread through these fields from the source data alongside
CallResultData, and include them in the returned UpdateCallRequest so existing
values are preserved.
In `@src/stores/incident-command/store.ts`:
- Around line 250-271: The moveNode path in store.ts can create a cycle because
it only checks the selected node and does not prevent reparenting under a
descendant. Update the moveNode flow, and if needed the MoveNodeSheet picker, to
reject any parentNodeId that is the target node itself or any of its descendants
before calling saveNode. Use the existing reorderNode and moveNode actions plus
the command-board-view.tsx parent-chain behavior to locate the affected logic
and apply the ancestor check before persisting.
In `@src/translations/fr.json`:
- Around line 188-206: The French locale file contains several newly added
translation keys that are still in English, so update all affected entries to
proper French text. Review the new call delete/reschedule/dispatch-more keys
(such as delete_call, reschedule_tomorrow_morning, dispatch_more), the dispatch
resources and weather alert settings strings, and the entire incident_command.*
block, then replace the English values with French translations while keeping
the existing key names and JSON structure unchanged.
In `@src/translations/it.json`:
- Around line 188-206: The Italian locale file still contains newly added
untranslated English strings, including the call delete/reschedule/dispatch-more
entries and the `incident_command.*` block. Update the affected translation
entries in `it.json` to proper Italian equivalents, using the existing keys such
as `delete_call`, `reschedule_*`, `dispatch_more_*`, and the `incident_command`
namespace, and make sure the other flagged sections are translated consistently
as well.
In `@src/translations/pl.json`:
- Around line 188-206: The Polish locale file contains many newly added strings
still left in English, creating a mixed-language UI for the new call, dispatch,
settings, weather alerts, and incident command features. Update the relevant
translation entries in pl.json for the affected sections (including the
delete/reschedule/dispatch-more keys, calls.audio and notify_cancelled_entities,
menu.incident_command, settings.modern_notification_sounds*, dispatch
resources-panel strings, weatherAlerts.settings, and the incident_command block)
with proper Polish text instead of copying the English source strings. Use the
existing translation structure and keep the keys aligned with the current locale
sections.
In `@src/translations/sv.json`:
- Around line 189-206: Several newly added keys in sv.json are still English
placeholders, so update the untranslated sections with proper Swedish text to
match the rest of the locale file. Focus on the call audio/delete/reschedule
strings, the notify-cancelled-entities and dispatch resource list entries, the
weather alert settings labels, and the full incident_command block, keeping the
existing key names and translating the values consistently with nearby Swedish
entries like update_call_error and menu.weatherAlerts.
In `@src/translations/uk.json`:
- Around line 189-206: The Ukrainian locale file still contains several newly
added English strings, so localize the missing entries in the call details
section, calls.audio, dispatch resources, weatherAlerts.settings, and the entire
incident_command block to match the rest of uk.json. Update the affected
translation keys in src/translations/uk.json so they use Ukrainian text instead
of the current English placeholders, using the existing keys like delete_call,
reschedule, dispatch_more, and incident_command as anchors.
---
Outside diff comments:
In `@src/components/calls/call-audio-modal.tsx`:
- Around line 1-217: The new CallAudioModal component needs test coverage. Add a
Jest/RTL test suite for CallAudioModal that verifies the open/close lifecycle,
fetchCallAudio behavior, playback controls in handlePlay/unloadSound, and the
loading/error/empty states rendered by renderContent. Use the component’s key
symbols such as CallAudioModal, handlePlay, unloadSound, and renderContent to
locate behaviors, and mock the bottom sheet, expo-av Audio, useCallDetailStore,
useAnalytics, and logger so the tests run reliably.
In `@src/components/calls/reschedule-call-sheet.tsx`:
- Around line 1-103: Add test coverage for the new RescheduleCallSheet
component. Create a test file that renders RescheduleCallSheet, mocks
useCallDetailStore/useScheduledCallsStore/useCallsStore/useToastStore and
useTranslation, and verifies the main flows: preset buttons update the input,
invalid datetime blocks submit with an error toast, and a valid submit calls
rescheduleDispatchTime, refreshes both stores, and closes the sheet. Reference
the RescheduleCallSheet component and handleSubmit/applyPreset behavior so the
tests stay resilient if layout changes.
In `@src/stores/weatherAlerts/store.ts`:
- Around line 272-290: The weather-alert store’s reset action is leaving stale
state behind because it does not clear the settings field. Update the reset
logic in the weatherAlerts store’s reset() method so it explicitly restores
settings to null along with the other state fields already being reset. Make
sure the change is applied in the same reset() function that clears alerts,
history, sources, zones, and the loading flags.
---
Minor comments:
In `@src/app/`(app)/map.tsx:
- Around line 59-61: Active custom map layers are not being refreshed when the
screen regains focus. In the focus effect inside map.tsx, alongside
fetchLayers() for legacy vector layers, invoke refetchActiveLayers from
useActiveMapLayers so newly added/removed on-by-default layers are reloaded on
focus; update the effect dependencies accordingly and keep the hook call near
activeLayers/refetchActiveLayers so the screen reflects changes without a full
remount.
In `@src/app/`(app)/map.web.tsx:
- Around line 52-54: The custom-map layers are not being refreshed when the
screen regains focus because only the legacy vector layer refresh path is wired
up. Update the map focus handling in the web screen logic around
useActiveMapLayers, fetchLayers, and refetchActiveLayers so both legacy layers
and active custom layers are reloaded on focus. Make sure the focus effect
invokes refetchActiveLayers alongside the existing fetchLayers call, matching
the native screen behavior so newly toggled layers appear without a remount.
In `@src/app/`(app)/weather-alerts/settings.tsx:
- Around line 193-217: The icon-only edit/delete controls in the settings screen
are missing accessible labels, so screen readers cannot identify them. Update
the Button components that wrap PencilIcon and Trash2Icon to include an
accessibilityLabel (or equivalent accessible text) that clearly describes each
action, following the same pattern used by the other source edit/delete buttons
in this file. Apply the same fix to the duplicate controls mentioned elsewhere
in the component.
- Around line 41-51: The confirmDelete helper in settings.tsx uses hardcoded
Alert.alert button labels, which should be translated like the rest of the
screen. Update confirmDelete so the non-web Alert.alert actions use t()-driven
strings instead of literal Cancel/OK, and since confirmDelete is outside the
component, pass t into it or move it into the component/hook scope where t is
available.
- Around line 26-39: The severity and source-type display strings in
settings.tsx are hardcoded English and should be translated via react-i18next.
Update SEVERITY_OPTIONS and sourceTypeLabel() to use t() for each label, and
ensure the component has access to the translation hook or function so the
labels are resolved through the existing i18n keys rather than inline text.
In `@src/app/call/`[id].tsx:
- Around line 617-623: The map coordinate check in the call page should use
explicit nullability checks instead of truthiness so valid 0 values are
preserved. Update the logic around the destination map selection in the call
detail component (the `showDestinationMap`/`mapLatitude`/`mapLongitude` block in
`call/[id].tsx`) to rely on `!= null` checks for both latitude and longitude,
and ensure any downstream condition that gates rendering or map switching uses
the same explicit null check pattern.
In `@src/components/calls/call-audio-modal.tsx`:
- Around line 184-214: The call-audio modal’s BottomSheet styling is hardcoded
for light mode, and it bypasses the shared sheet wrapper used elsewhere. Update
the call-audio modal in call-audio-modal.tsx so the BottomSheet chrome
(background and handle indicator) respects dark mode like the content does, and
consider switching the modal to the shared CustomBottomSheet component used by
reschedule-call-sheet.tsx to keep theming consistent. Use the existing
call-audio-modal component and its BottomSheet props as the place to apply the
dark/light mode handling.
In `@src/components/dispatch-console/dashboard-view-toggles.tsx`:
- Around line 44-65: Remove the unsupported gap usage from the dashboard view
toggles styles by updating the StyleSheet in dashboard-view-toggles.tsx: the row
and pill style entries should use margin-based spacing instead of gap. Keep the
existing layout intent in the DashboardViewToggles component by adding spacing
between the pill children and between the pill buttons with margins on the
relevant Icon/Text wrappers or Pressable elements, so the styling remains
web-compatible.
In `@src/components/incident-command/command-map.tsx`:
- Around line 118-137: The native delete-annotation confirmation in
confirmDelete uses the wrong destructive label, showing the release action
instead of a delete/remove action. Update the Alert button in command-map.tsx to
use a delete-specific translation key from incident_command rather than
t('incident_command.release'), keeping the existing cancel option and doDelete
flow unchanged. Use confirmDelete and the Alert.alert invocation as the place to
fix this label.
In `@src/components/incident-command/command-map.web.tsx`:
- Line 17: The Mapbox GL stylesheet version in command-map.web.tsx is still
pinned to an older release, causing it to drift from the installed mapbox-gl
package and other web map components. Update the MAPBOX_GL_CSS_URL constant in
command-map.web.tsx to match the repo’s v3.15.0 Mapbox GL CSS version, keeping
it aligned with the rest of the map components and the pinned dependency.
In `@src/components/incident-command/command-voice.tsx`:
- Around line 66-75: The `setTalking` handler in `command-voice.tsx` is
swallowing `setMicrophoneEnabled` failures, so the PTT control can fail
silently. Update the `catch` path to surface the error to the user with a
visible notification or error state, and keep `useLiveKitStore.setIsTalking`
unchanged unless the mic toggle succeeds. Make sure the message clearly tells
the user the microphone could not be enabled/disabled, and reference
`room.localParticipant.setMicrophoneEnabled` and `setTalking` when wiring the
fix.
In `@src/components/settings/modern-notification-sounds-item.tsx`:
- Around line 32-39: The Switch in modern-notification-sounds-item.tsx is
missing an accessible name for screen readers. Update the Switch element in the
modern notification sounds item so it includes an accessibilityLabel that
clearly describes what it controls, using the existing translation text or
equivalent label near handleToggle and isModernNotificationSoundsEnabled.
In `@src/components/weatherAlerts/weather-alert-source-sheet.tsx`:
- Line 67: The PollIntervalMinutes value in weather-alert-source-sheet.tsx can
still accept negative input because parseInt(pollInterval, 10) only falls back
on falsy values, not invalid ranges. Update the logic around the
PollIntervalMinutes assignment in the weather alert source sheet flow to
validate that the parsed poll interval is a positive number, and fall back to
the default 15 when the input is NaN, zero, or negative.
In `@src/components/weatherAlerts/weather-alert-zone-sheet.tsx`:
- Around line 85-93: The placeholders in weather-alert-zone-sheet.tsx are
hardcoded and bypass i18n; update the InputField usages in the zoneCode and
center inputs to pull placeholder text through t() like the existing
FormControlLabelText does. Use the same translation pattern already present in
WeatherAlertZoneSheet so the zone code and center field placeholders are
localized instead of inline English strings.
---
Nitpick comments:
In `@src/api/incidentCommand/incidentCommand.ts`:
- Line 19: The URI segment helper is duplicated across incident API modules, so
move the shared seg encoding logic out of incidentCommand.ts,
incidentReporting.ts, and incidentRoles.ts into a common utility near
createApiEndpoint in `@/api/common/client`. Update the affected modules to import
and use that shared helper instead of redefining the same encodeURIComponent
wrapper locally, keeping a single source of truth for segment encoding.
- Line 132: The API module is re-exporting CommandLogEntry alongside the request
helpers, which blurs the boundary between wrapper functions and model types.
Remove the type re-export from incidentCommand.ts and keep CommandLogEntry
exported from its owning model module instead, leaving the API module focused on
request functions like the incident command helpers.
In `@src/components/checkIn/check-in-tab.tsx`:
- Around line 152-169: Add Jest coverage in check-in-tab.test.tsx for the new
PAR roster behavior in CheckInTab: verify the roster renders when
callPersonnelStatuses has items, does not render when checkInTimersEnabled is
false or the list is empty, and assert parColorClass-driven status styling for
the supported personnel statuses. Also cover the fetchCallPersonnelStatuses
wiring so the new UI path is exercised end-to-end in the existing CheckInTab
test setup.
- Around line 152-169: The new PAR roster block in check-in-tab.tsx should
follow the component’s translation and conditional-rendering patterns: replace
the raw person.Status label in the map output with the appropriate t() lookup
used elsewhere in check-in-tabbased status rendering, and change the outer
checkInTimersEnabled / callPersonnelStatuses.length display guard from &&
chaining to a ternary expression. Use the existing symbols checkInTimersEnabled,
callPersonnelStatuses, person.Status, and parColorClass to locate and update
this block.
In `@src/components/dispatch-console/dashboard-view-toggles.tsx`:
- Around line 11-42: `DashboardViewToggles` is a prop-free component that is
being re-rendered unnecessarily when its parent updates, so wrap the component
definition in `React.memo()` to prevent extra renders. Update the
`DashboardViewToggles` export itself (the component returning the two
`Pressable` toggles) and keep its current behavior unchanged, since it only
depends on store selectors and translation.
- Around line 1-65: The new DashboardViewToggles component is missing Jest
coverage, so add tests for its render and behavior. Create a test for
DashboardViewToggles that mocks useDashboardViewStore and useTranslation,
verifies both toggles render with the expected labels/icons/testIDs, and
confirms onPress calls toggleAvailableOnly and toggleSingleList while
accessibilityState reflects the current availableOnly and singleList values.
In `@src/components/dispatch-console/personnel-panel.tsx`:
- Around line 236-241: The `singleList` behavior is split inconsistently between
`PersonnelPanel` and `UnitsPanel`, which makes the pair tightly coupled and
fragile. Remove the hidden fallback from `UnitsPanel` and the `return null`
branch in `PersonnelPanel`, and instead handle the `singleList` switch at each
layout call site (as done in `home.tsx`/`home.web.tsx` phone layout) so
`ResourcesPanel` is rendered explicitly when needed. Keep the panel components
focused on their own props and render paths, and use the `singleList`,
`PersonnelPanel`, `UnitsPanel`, and `ResourcesPanel` symbols to find the
affected branches.
In `@src/components/incident-command/command-board-view.tsx`:
- Around line 96-109: The ordered lane hierarchy in command-board-view.tsx is
being rebuilt on every render via the orderedLanes IIFE, childrenOf, and walk
recursion. Move this computation into a useMemo so it only recalculates when the
underlying board.Nodes/activeNodes data changes, and keep the recursive
traversal logic inside that memoized block to avoid repeated heavy work during
render.
In `@src/components/incident-command/command-map.tsx`:
- Line 57: The canManage bitmask check in command-map.tsx is reimplementing
shared capability logic instead of using hasIncidentCapability, which can
diverge from incidentCommandEnums semantics; update the command-map logic to
import and call hasIncidentCapability from
`@/models/v4/incidentCommand/incidentCommandEnums` for the ManageAnnotations
check, and make the same change in command-map.web.tsx so both paths use the
shared helper consistently.
- Line 51: The `useLocationStore` selector in `command-map.tsx` is creating a
new object on every update, which defeats zustand’s reference-equality bailout
and causes extra re-renders. Update the `useLocationStore` usage in `CommandMap`
to select `latitude` and `longitude` as primitives (or apply a shallow
comparator) and then reference those values directly instead of
`userLocation.latitude`/`userLocation.longitude`; make the same adjustment in
`command-map.web.tsx` where the pattern repeats.
- Around line 25-37: The GeoJSON helpers in command-map are duplicated in both
the native and web tactical map components, so move the shared logic from
toFeatureCollection and firstPoint into a common utility module and import it
from both places. Create a shared GeoJSON helper (for example in a lib utility)
and update command-map.tsx and command-map.web.tsx to use the same exported
functions so their behavior stays in sync as the implementation evolves.
- Around line 128-132: The `Platform.OS === 'web'` branch in `CommandMap` is
dead code because web resolves to `command-map.web.tsx` via platform-specific
module resolution. Remove the web-only confirmation/delete block from
`command-map.tsx` and keep the native-only `doDelete` flow there, letting the
web implementation handle its own behavior in the web-specific file.
- Around line 76-85: The fallback center in the `useMemo` for `center` is
inconsistent with the web implementation, since `command-map.tsx` returns `[0,
0]` when no command post, annotation, or user location is available. Update the
final fallback in `center` to match the native/web shared behavior used by
`command-map.web.tsx` so both variants default to the same continental US center
when no location data exists.
In `@src/components/incident-command/command-map.web.tsx`:
- Around line 144-146: The marker label in command-map.web.tsx is still
assembled with innerHTML in the marker creation logic, which relies on a fragile
partial escape of annotation.Label. Update the marker-building code around the
DOM creation in the same block to construct the label and icon with
document.createElement and textContent instead of string concatenation, so the
label from annotation.Label and the fallback t('incident_command.marker') are
inserted safely without manual sanitization.
In `@src/components/incident-command/incident-command-sheets.tsx`:
- Around line 416-430: The personnel Select markup is duplicated in
AssignRoleSheet and TransferCommandSheet, so extract the repeated
Select/SelectItem block into a shared PersonSelect presentational component and
reuse it in both places. Use the existing Select, SelectTrigger, SelectInput,
SelectIcon, SelectPortal, SelectBackdrop, SelectContent, and SelectItem
structure as the basis, and keep the personnel mapping logic centralized in the
new component so future filtering or sorting changes happen in one place.
In `@src/hooks/use-active-map-layers.ts`:
- Around line 21-61: Add a companion test file for useActiveMapLayers to cover
its async behavior, including auto-fetch on mount, abort handling via
AbortController, filtering of non-custom or non-default layers, and swallowing
per-layer fetch errors while still returning successful layers. Mock
getAllActiveLayers, getCustomMapRegionsGeoJSON, logger.error, and isAbort so you
can verify refetchActiveLayers and the effect cleanup paths without real network
calls. Also assert isLoadingActiveLayers toggles correctly and that logger.error
is called only for non-abort failures.
In `@src/lib/hooks/use-modern-notification-sounds.tsx`:
- Around line 35-37: The error handling in useModernNotificationSounds currently
logs through console.error instead of the app’s structured logger. Update the
catch block in useModernNotificationSounds to use logger.error like the
push-notification.ts pattern, preserving the same error context so logging stays
consistent and Sentry-integrated.
In `@src/lib/resource-availability.ts`:
- Around line 1-21: Add direct unit tests for the pure helpers isUnitAvailable
and isPersonnelAvailable, covering the branch cases in resource-availability.ts:
matching CurrentStatus, matching CurrentStatusId, empty/unknown status fallback,
and non-available values. Place the tests in a dedicated spec file that imports
these helpers directly so the availability logic used by resources-panel.tsx,
units-panel.tsx, and personnel-panel.tsx is verified independently of UI code.
In `@src/lib/unit-status-helpers.ts`:
- Around line 18-43: Add direct unit coverage for resolveUnitStatusOptions in
unit-status-helpers to exercise each of the four branches independently. Create
tests that verify the exact custom-set id match, the unit Type match, the
server-provided fallback array, and the default UnitType '0' group, including
cases where a matched group exists but Statuses is empty. Keep the tests focused
on resolveUnitStatusOptions rather than indirect consumer behavior.
In `@src/models/v4/checkIn/callPersonnelCheckInStatusResultData.ts`:
- Around line 9-10: The Status field in CallPersonnelCheckInStatusResultData is
typed too broadly as string even though the doc comment defines a closed set of
values. Update this model to use a literal union for Status with the documented
values, and keep the default initialization compatible with that type. Make sure
any consumers that branch on Status, such as parColorClass and the
STATUS_SEVERITY lookup, continue to type-check against the narrower union.
In `@src/models/v4/incidentCommand/accountability.ts`:
- Around line 11-12: Update the Status field in the accountability model to use
a literal union instead of a plain string. In the Accountability class, replace
the current Status typing with the documented set of values ("Green" | "Warning"
| "Critical") so consumers like the board renderItem filtering logic can’t pass
invalid statuses. Keep the existing Status property and adjust any related
assignments or comparisons in this model to match the new union type.
In `@src/models/v4/incidentCommand/incidentCommand.ts`:
- Around line 12-16: The `IncidentCommand` model’s `IcsLevel` and `Status`
fields are still plain numbers, so update them in `incidentCommand.ts` to use
the corresponding enum types from `IncidentCommandEnums` (or add explicit inline
references/comments if typing can’t be changed). Keep the change localized to
the `IncidentCommand` class so consumers can discover the intended backing enums
and get stronger type safety.
In `@src/models/v4/incidentCommand/incidentCommandEnums.ts`:
- Around line 143-225: The capability mapping in IncidentCapabilities and
getIncidentRoleCapabilities is a manual mirror of backend authorization logic
and can drift from the server. Update this file so the client no longer relies
on a hand-maintained source of truth, and add a parity/contract test or
generated binding that verifies IncidentCapabilities and
getIncidentRoleCapabilities stay synchronized with
Resgrid.Model.IncidentCapabilities and IncidentRoleCapabilityMap used by
command-board-view.tsx and the GetMyCapabilities flow.
In `@src/models/v4/incidentCommand/incidentMapAnnotation.ts`:
- Line 9: `IncidentMapAnnotation.AnnotationType` is currently a bare number
without any indication of valid values, which makes it easy to misuse in
rendering paths like `command-map.tsx`. Update the `IncidentMapAnnotation` model
to reference a shared enum or add a clear doc comment on `AnnotationType`
describing the allowed values and meaning, and use that named type consistently
where the field is read or assigned.
In `@src/services/push-notification.ts`:
- Around line 131-160: The refreshNotificationChannels() method is swallowing
failures by catching and only logging errors, so callers like
useModernNotificationSounds cannot detect when channel recreation fails. Update
refreshNotificationChannels() to preserve the error for the caller by rethrowing
after logger.error (or removing the local catch and letting the existing
try/catch in useModernNotificationSounds handle it), while keeping the existing
Android-only guard and channel refresh logic in
src/services/push-notification.ts.
In `@src/stores/dispatch/dashboard-view-store.ts`:
- Around line 1-29: The new zustand store useDashboardViewStore in
dashboard-view-store.ts has no accompanying test coverage for its toggle/set
logic. Add a test file for DashboardViewState that verifies toggleAvailableOnly,
toggleSingleList, setAvailableOnly, and setSingleList update state correctly,
and ensure the store can be instantiated/reset between assertions.
- Around line 1-29: Persist the dispatcher view flags in useDashboardViewStore
so availableOnly and singleList survive app restarts. Update the Zustand store
to use the persist middleware with the project’s react-native-mmkv storage
convention, and keep the existing toggle/set actions intact so only the storage
layer changes. Make sure the persisted state keys match DashboardViewState and
that the store initialization still behaves correctly when no saved preferences
exist.
In `@src/stores/incident-command/__tests__/store.test.ts`:
- Around line 59-124: The store tests in useIncidentCommandStore only cover
loadForCall, can, establish, and assignResource, so add cases for the remaining
mutation paths and error handling. Extend the existing setup in store.test.ts to
exercise closeCommand, transfer, moveNode/reorderNode, saveObjective,
startTimer, saveAnnotation, assignRole, evaluateAccountability, and the reload
error-swallowing branch, verifying each mutation calls the expected API and
updates state via the shared mutate/requireCommand flow. Keep the new assertions
aligned with the existing test style and use the same mock helpers and store
methods to locate the behavior.
In `@src/stores/incident-command/store.ts`:
- Around line 330-345: `evaluateAccountability` is duplicating the same
mutate/error/reload/finally flow already handled by the shared `mutate` helper.
Refactor `mutate` into a generic `mutateWithResult<T>` (or equivalent) that
accepts the async action and returns the action result while preserving the
existing `isMutating`, `error`, `reload`, and `logger.error` behavior. Update
`evaluateAccountability` and the existing void-returning callers to use the
shared helper so the result-preserving case no longer reimplements the mutation
pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+15
to
+22
| const formatTimestamp = (value?: string | null): string => { | ||
| if (!value) return '—'; | ||
| try { | ||
| return formatDateForDisplay(parseDateISOString(value), 'MMM d, yyyy h:mm a'); | ||
| } catch { | ||
| return value; | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm formatDateForDisplay's supported tokens and am/pm handling
rg -n "format.indexOf" src/lib/utils.tsRepository: Resgrid/Dispatch
Length of output: 607
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the formatter implementation and the local timestamp helper
sed -n '250,335p' src/lib/utils.ts
printf '\n---\n'
sed -n '1,40p' src/app/weather-alerts/[id].tsxRepository: Resgrid/Dispatch
Length of output: 4098
Use supported date tokens here formatDateForDisplay doesn’t replace d, h, or a, so this timestamp renders with literal tokens. Switch to a supported pattern like MMM dd, yyyy hh:mm t.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/weather-alerts/`[id].tsx around lines 15 - 22, The timestamp
formatting in formatTimestamp uses unsupported tokens in formatDateForDisplay,
so the date renders with literal d, h, and a text. Update the pattern passed to
formatDateForDisplay in formatTimestamp to a supported format such as MMM dd,
yyyy hh:mm t, keeping the rest of the parseDateISOString and fallback logic
unchanged.
Comment on lines
+188
to
+206
| "update_call_success": "Intervento aggiornato con successo", | ||
| "audio": "Audio", | ||
| "delete_call": "Delete Call", | ||
| "delete_call_confirm": "Delete this call? This cannot be undone.", | ||
| "delete_call_success": "Call deleted", | ||
| "delete_call_error": "Failed to delete call", | ||
| "reschedule": "Reschedule", | ||
| "reschedule_datetime": "Dispatch date & time", | ||
| "reschedule_invalid": "Enter a valid date and time (YYYY-MM-DD HH:mm)", | ||
| "reschedule_success": "Dispatch time updated", | ||
| "reschedule_error": "Failed to reschedule call", | ||
| "reschedule_in_1_hour": "In 1 hour", | ||
| "reschedule_in_1_day": "In 1 day", | ||
| "reschedule_tomorrow_morning": "Tomorrow 8 AM", | ||
| "map_call": "Call", | ||
| "map_destination": "Destination", | ||
| "dispatch_more": "Dispatch additional resources", | ||
| "dispatch_more_success": "Resources dispatched and notified", | ||
| "dispatch_more_error": "Failed to dispatch resources" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
New keys remain untranslated (English text in the Italian locale file).
Same issue as flagged in fr.json: the newly added call delete/reschedule/dispatch-more, call audio, dispatch resources list, weather alert settings, and incident_command strings are left in English rather than translated to Italian (e.g. "delete_call": "Delete Call", full incident_command.* block).
Also applies to: 353-361, 1017-1022, 1141-1178, 1238-1347
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/translations/it.json` around lines 188 - 206, The Italian locale file
still contains newly added untranslated English strings, including the call
delete/reschedule/dispatch-more entries and the `incident_command.*` block.
Update the affected translation entries in `it.json` to proper Italian
equivalents, using the existing keys such as `delete_call`, `reschedule_*`,
`dispatch_more_*`, and the `incident_command` namespace, and make sure the other
flagged sections are translated consistently as well.
Comment on lines
+188
to
+206
| "update_call_success": "Zgłoszenie zaktualizowane pomyślnie", | ||
| "audio": "Audio", | ||
| "delete_call": "Delete Call", | ||
| "delete_call_confirm": "Delete this call? This cannot be undone.", | ||
| "delete_call_success": "Call deleted", | ||
| "delete_call_error": "Failed to delete call", | ||
| "reschedule": "Reschedule", | ||
| "reschedule_datetime": "Dispatch date & time", | ||
| "reschedule_invalid": "Enter a valid date and time (YYYY-MM-DD HH:mm)", | ||
| "reschedule_success": "Dispatch time updated", | ||
| "reschedule_error": "Failed to reschedule call", | ||
| "reschedule_in_1_hour": "In 1 hour", | ||
| "reschedule_in_1_day": "In 1 day", | ||
| "reschedule_tomorrow_morning": "Tomorrow 8 AM", | ||
| "map_call": "Call", | ||
| "map_destination": "Destination", | ||
| "dispatch_more": "Dispatch additional resources", | ||
| "dispatch_more_success": "Resources dispatched and notified", | ||
| "dispatch_more_error": "Failed to dispatch resources" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Newly added strings are left untranslated (English) in the Polish locale file.
Nearly all new keys added by this PR (call_detail delete/reschedule/dispatch-more, calls.audio/notify_cancelled_entities, menu.incident_command, settings.modern_notification_sounds*, dispatch resources-panel strings, weatherAlerts.settings, and the entire incident_command block) are copied verbatim in English rather than translated into Polish (e.g. "delete_call": "Delete Call", "title": "Incident Command"). This leaves Polish-locale users with a mixed-language UI for all of this PR's new features.
Since I can't verify Polish translations, please have a native speaker (or translation pipeline) fill these in before release, or confirm this is an intentional placeholder pending a follow-up localization pass.
Also applies to: 353-362, 610-611, 791-792, 1017-1022, 1141-1178, 1238-1347
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/translations/pl.json` around lines 188 - 206, The Polish locale file
contains many newly added strings still left in English, creating a
mixed-language UI for the new call, dispatch, settings, weather alerts, and
incident command features. Update the relevant translation entries in pl.json
for the affected sections (including the delete/reschedule/dispatch-more keys,
calls.audio and notify_cancelled_entities, menu.incident_command,
settings.modern_notification_sounds*, dispatch resources-panel strings,
weatherAlerts.settings, and the incident_command block) with proper Polish text
instead of copying the English source strings. Use the existing translation
structure and keep the keys aligned with the current locale sections.
Comment on lines
+189
to
+206
| "audio": "Audio", | ||
| "delete_call": "Delete Call", | ||
| "delete_call_confirm": "Delete this call? This cannot be undone.", | ||
| "delete_call_success": "Call deleted", | ||
| "delete_call_error": "Failed to delete call", | ||
| "reschedule": "Reschedule", | ||
| "reschedule_datetime": "Dispatch date & time", | ||
| "reschedule_invalid": "Enter a valid date and time (YYYY-MM-DD HH:mm)", | ||
| "reschedule_success": "Dispatch time updated", | ||
| "reschedule_error": "Failed to reschedule call", | ||
| "reschedule_in_1_hour": "In 1 hour", | ||
| "reschedule_in_1_day": "In 1 day", | ||
| "reschedule_tomorrow_morning": "Tomorrow 8 AM", | ||
| "map_call": "Call", | ||
| "map_destination": "Destination", | ||
| "dispatch_more": "Dispatch additional resources", | ||
| "dispatch_more_success": "Resources dispatched and notified", | ||
| "dispatch_more_error": "Failed to dispatch resources" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
New strings left untranslated (English) in the Swedish locale file.
All newly added keys for call audio/delete/reschedule, notify-cancelled-entities, dispatch resource list, weather alert settings, and the entire incident_command block are English placeholders rather than Swedish translations, inconsistent with the fully localized strings around them (e.g., update_call_error, menu.weatherAlerts).
Also applies to: 354-361, 1018-1022, 1142-1179, 1239-1347
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/translations/sv.json` around lines 189 - 206, Several newly added keys in
sv.json are still English placeholders, so update the untranslated sections with
proper Swedish text to match the rest of the locale file. Focus on the call
audio/delete/reschedule strings, the notify-cancelled-entities and dispatch
resource list entries, the weather alert settings labels, and the full
incident_command block, keeping the existing key names and translating the
values consistently with nearby Swedish entries like update_call_error and
menu.weatherAlerts.
Comment on lines
+189
to
+206
| "audio": "Audio", | ||
| "delete_call": "Delete Call", | ||
| "delete_call_confirm": "Delete this call? This cannot be undone.", | ||
| "delete_call_success": "Call deleted", | ||
| "delete_call_error": "Failed to delete call", | ||
| "reschedule": "Reschedule", | ||
| "reschedule_datetime": "Dispatch date & time", | ||
| "reschedule_invalid": "Enter a valid date and time (YYYY-MM-DD HH:mm)", | ||
| "reschedule_success": "Dispatch time updated", | ||
| "reschedule_error": "Failed to reschedule call", | ||
| "reschedule_in_1_hour": "In 1 hour", | ||
| "reschedule_in_1_day": "In 1 day", | ||
| "reschedule_tomorrow_morning": "Tomorrow 8 AM", | ||
| "map_call": "Call", | ||
| "map_destination": "Destination", | ||
| "dispatch_more": "Dispatch additional resources", | ||
| "dispatch_more_success": "Resources dispatched and notified", | ||
| "dispatch_more_error": "Failed to dispatch resources" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
New strings left untranslated (English) in the Ukrainian locale file.
Same issue as in sv.json: newly added call_detail, calls.audio, dispatch resources, weatherAlerts.settings, and the full incident_command block are English rather than Ukrainian, unlike the rest of the file which is fully localized.
Also applies to: 354-361, 1018-1022, 1142-1179, 1239-1347
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/translations/uk.json` around lines 189 - 206, The Ukrainian locale file
still contains several newly added English strings, so localize the missing
entries in the call details section, calls.audio, dispatch resources,
weatherAlerts.settings, and the entire incident_command block to match the rest
of uk.json. Update the affected translation keys in src/translations/uk.json so
they use Ukrainian text instead of the current English placeholders, using the
existing keys like delete_call, reschedule, dispatch_more, and incident_command
as anchors.
Resgrid-Bot
reviewed
Jul 4, 2026
| }, | ||
| }); | ||
|
|
||
| export default CommandMap; |
There was a problem hiding this comment.
Redundant default export detected in CommandMap, call-audio-modal.tsx, command-voice.tsx, and command-map.web.tsx. The export default lines create dead code that causes refactoring issues since these files already utilize named exports.
Kody rule violation: Avoid default exports
// Remove the default export; the named export `export const CommandMap` is the only one used.Prompt for LLM
File src/components/incident-command/command-map.tsx:
Line 221:
Violates rule 'Avoid default exports': the file already exports `CommandVoice`/`CommandMap` as a named export, and a grep confirms it is only ever imported via that named export (`import { CommandMap } from '@/components/incident-command/command-map'`). The `export default CommandMap;` line is redundant dead code. Named exports provide better clarity and prevent refactoring issues.
Suggested Code:
// Remove the default export; the named export `export const CommandMap` is the only one used.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Jul 4, 2026
| _setEnabled(value); | ||
| await pushNotificationService.refreshNotificationChannels(); | ||
| } catch (error) { | ||
| console.error('Failed to update modern notification sounds state:', error); |
There was a problem hiding this comment.
Unstructured error logging identified in use-modern-notification-sounds.tsx and unit-actions-panel.tsx. The console.error calls lack the structured context required by the rest of the PR, preventing proper tracking of operation names and identifiers.
Kody rule violation: Include error context in structured logs
logger.error({ message: 'Failed to update modern notification sounds state', context: { error, value } });Prompt for LLM
File src/lib/hooks/use-modern-notification-sounds.tsx:
Line 36:
Violates rule 'Include error context in structured logs': the catch block uses `console.error('Failed to update modern notification sounds state:', error)` — a bare console.error with just a message string and no structured fields. Every other new file in this PR (call-audio-modal.tsx, active-calls-panel.tsx, use-active-map-layers.ts) correctly uses the structured `logger.error({ message, context: { error, ...ids } })` pattern. Error logs must include the operation name and relevant identifiers as structured fields.
Suggested Code:
logger.error({ message: 'Failed to update modern notification sounds state', context: { error, value } });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Jul 4, 2026
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ✅ |
Resgrid-Bot
reviewed
Jul 4, 2026
Comment on lines
+120
to
+122
| if (requestId !== playRequestRef.current) { | ||
| await sound.unloadAsync().catch(() => {}); | ||
| return; |
There was a problem hiding this comment.
Violates rule 'Avoid empty catch blocks': the newly-added .catch(() => {}) at line 121 silently swallows any exception from sound.unloadAsync(). The rule requires logging with context or explicit handling rather than discarding the error. Even for best-effort cleanup, log with context so a failed dispose is observable.
if (requestId !== playRequestRef.current) {
await sound.unloadAsync().catch((err) => {
logger.error({ message: 'Failed to unload superseded call audio', context: { error: err, callId } });
});
return;
}Prompt for LLM
File src/components/calls/call-audio-modal.tsx:
Line 120 to 122:
Violates rule 'Avoid empty catch blocks': the newly-added `.catch(() => {})` at line 121 silently swallows any exception from `sound.unloadAsync()`. The rule requires logging with context or explicit handling rather than discarding the error. Even for best-effort cleanup, log with context so a failed dispose is observable.
Suggested Code:
if (requestId !== playRequestRef.current) {
await sound.unloadAsync().catch((err) => {
logger.error({ message: 'Failed to unload superseded call audio', context: { error: err, callId } });
});
return;
}
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Resgrid-Bot
reviewed
Jul 4, 2026
| // Unit names dispatched to the call — used for on-call highlighting (mirrors UnitsPanel). | ||
| const dispatchedUnitNames = useMemo(() => { | ||
| if (!callDispatches) return new Set<string>(); | ||
| return new Set(callDispatches.filter((d) => d.Type === 'Unit' || d.Type === 'u').map((d) => d.Name.toLowerCase())); |
There was a problem hiding this comment.
Violates rule 'Extract duplicated logic into functions': the newly-added personnel/type-matching code repeats the same new Set(['Personnel', 'personnel', 'p', 'P', 'User', 'user']) literal three times (lines 89, 100, 119) and the d.Type === 'Unit' || d.Type === 'u' filter predicate twice (lines 83, 112). These duplicate sequences should be hoisted into a single module-level constant (and ideally a shared helper, since personnel-panel.tsx defines the identical set) so the dispatch-type set has one source of truth.
// Module-level (defined once, shared by all three useMemo hooks):
const PERSONNEL_DISPATCH_TYPES = new Set(['Personnel', 'personnel', 'p', 'P', 'User', 'user']);
const isUnitDispatch = (d: DispatchedEventResultData) => d.Type === 'Unit' || d.Type === 'u';
const isPersonnelDispatch = (d: DispatchedEventResultData) => PERSONNEL_DISPATCH_TYPES.has(d.Type);
// Then everywhere:
callDispatches.filter(isUnitDispatch)
callDispatches.filter(isPersonnelDispatch)Prompt for LLM
File src/components/dispatch-console/resources-panel.tsx:
Line 83:
Violates rule 'Extract duplicated logic into functions': the newly-added personnel/type-matching code repeats the same `new Set(['Personnel', 'personnel', 'p', 'P', 'User', 'user'])` literal three times (lines 89, 100, 119) and the `d.Type === 'Unit' || d.Type === 'u'` filter predicate twice (lines 83, 112). These duplicate sequences should be hoisted into a single module-level constant (and ideally a shared helper, since personnel-panel.tsx defines the identical set) so the dispatch-type set has one source of truth.
Suggested Code:
// Module-level (defined once, shared by all three useMemo hooks):
const PERSONNEL_DISPATCH_TYPES = new Set(['Personnel', 'personnel', 'p', 'P', 'User', 'user']);
const isUnitDispatch = (d: DispatchedEventResultData) => d.Type === 'Unit' || d.Type === 'u';
const isPersonnelDispatch = (d: DispatchedEventResultData) => PERSONNEL_DISPATCH_TYPES.has(d.Type);
// Then everywhere:
callDispatches.filter(isUnitDispatch)
callDispatches.filter(isPersonnelDispatch)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/(app)/map.web.tsx (1)
155-173: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winKeep theme swaps in one place The map is recreated on color-scheme changes because
getMapStyleis part of the init effect deps, so the separatesetStyleeffect does an extra style swap on the fresh instance. DropgetMapStylefrom the init effect deps (or otherwise keep the map alive across theme toggles) so only one effect owns style changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/map.web.tsx around lines 155 - 173, The theme swap logic is split between the map initialization effect and the separate style-update effect in map.web.tsx, causing an extra style reset when colorScheme changes. Update the initialization effect that depends on getMapStyle so it no longer recreates the map on theme toggles, or otherwise ensure the map instance stays alive across color-scheme changes. Keep the style change responsibility in the useEffect that calls setStyle and reattaches layers via addCustomMapLayersRef.current and addActiveMapLayersRef.current so only one place owns style swaps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/app/`(app)/map.web.tsx:
- Around line 155-173: The theme swap logic is split between the map
initialization effect and the separate style-update effect in map.web.tsx,
causing an extra style reset when colorScheme changes. Update the initialization
effect that depends on getMapStyle so it no longer recreates the map on theme
toggles, or otherwise ensure the map instance stays alive across color-scheme
changes. Keep the style change responsibility in the useEffect that calls
setStyle and reattaches layers via addCustomMapLayersRef.current and
addActiveMapLayersRef.current so only one place owns style swaps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5026f29-b1dd-43b0-a0f9-b22b51cdc69c
📒 Files selected for processing (32)
src/app/(app)/home.tsxsrc/app/(app)/home.web.tsxsrc/app/(app)/incident-command.tsxsrc/app/(app)/map.web.tsxsrc/app/call/[id].tsxsrc/app/call/[id].web.tsxsrc/app/weather-alerts/[id].tsxsrc/components/calls/call-audio-modal.tsxsrc/components/checkIn/check-in-tab.tsxsrc/components/dispatch-console/active-calls-panel.tsxsrc/components/dispatch-console/activity-log-panel.tsxsrc/components/dispatch-console/resources-panel.tsxsrc/components/dispatch-console/units-panel.tsxsrc/components/incident-command/command-board-view.tsxsrc/components/incident-command/ic-labels.tssrc/components/incident-command/incident-command-sheets.tsxsrc/components/status/__tests__/status-bottom-sheet-store.test.tsxsrc/components/status/status-bottom-sheet.tsxsrc/components/weatherAlerts/weather-alert-source-sheet.tsxsrc/lib/dispatch-helpers.tssrc/models/v4/checkIn/checkInEnums.tssrc/models/v4/incidentCommand/incidentCommandEnums.tssrc/stores/incident-command/store.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
✅ Files skipped from review due to trivial changes (4)
- src/models/v4/checkIn/checkInEnums.ts
- src/components/status/tests/status-bottom-sheet-store.test.tsx
- src/translations/pl.json
- src/translations/fr.json
🚧 Files skipped from review as they are similar to previous changes (22)
- src/app/(app)/incident-command.tsx
- src/models/v4/incidentCommand/incidentCommandEnums.ts
- src/components/dispatch-console/units-panel.tsx
- src/components/status/status-bottom-sheet.tsx
- src/components/checkIn/check-in-tab.tsx
- src/components/weatherAlerts/weather-alert-source-sheet.tsx
- src/components/incident-command/ic-labels.ts
- src/lib/dispatch-helpers.ts
- src/app/weather-alerts/[id].tsx
- src/components/incident-command/command-board-view.tsx
- src/translations/ar.json
- src/translations/it.json
- src/components/incident-command/incident-command-sheets.tsx
- src/translations/sv.json
- src/translations/es.json
- src/translations/uk.json
- src/app/(app)/home.tsx
- src/translations/en.json
- src/app/call/[id].web.tsx
- src/app/call/[id].tsx
- src/translations/de.json
- src/components/dispatch-console/active-calls-panel.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a comprehensive Incident Command (ICS) system along with significant enhancements to call management, weather alerts, dashboard views, map rendering, and notification sounds.
Major Features
Incident Command System
A full ICS/NIMS-compliant incident command board, accessible from the call detail screen, a dedicated full-screen board, and a department-wide active-commands list:
incidentCommandUpdatedevent triggers automatic board refreshCall Management Enhancements
Dashboard View Toggles
ResourcesPanelWeather Alert Configuration
Modern Notification Sounds
Map Layer Improvements
Check-in / Accountability
Unit Status Resolution
CustomStatusSetId), with reliable fallback through type name, server-provided list, and department defaults — applied in both the status bottom sheet and the unit actions panelSummary by CodeRabbit
New Features
Bug Fixes
Settings