Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 37 minutes and 10 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds video-feed management, check-in timer systems (with offline queue and native live activity hooks), and weather-alerts (store, UI, SignalR events), plus numerous stores, API clients, models, screens, components, translations, build/config updates, and test/mocks to integrate these features. Changes
Sequence Diagram(s)sequenceDiagram
participant SignalR as SignalR Hub
participant SignalRStore as SignalR Store
participant WeatherStore as WeatherAlerts Store
participant API as Weather Alerts API
participant UI as Client UI
rect rgba(52, 211, 153, 0.5)
SignalR->>SignalRStore: emit weatherAlertReceived(alertId)
end
rect rgba(59, 130, 246, 0.5)
SignalRStore->>WeatherStore: handleAlertReceived(alertId)
WeatherStore->>API: fetchWeatherAlert(alertId)
API-->>WeatherStore: return alert data
WeatherStore->>WeatherStore: prepend/sort alerts, update timestamp
end
rect rgba(245, 158, 11, 0.5)
WeatherStore->>UI: notify subscribers / update state
UI->>UI: render banner/list/detail based on new alerts
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Member
Author
|
This PR will also resolve #92 |
![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: 17
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 (5)
src/components/settings/language-item.tsx (1)
23-35:⚠️ Potential issue | 🔴 CriticalRemove memoization and use
t()from react-i18next for translations.The
langsarray is memoized with an empty dependency array, causing labels to freeze at initial render. When language changes, the dropdown labels won't update. Additionally, the customtranslate()function should be replaced witht()fromreact-i18nextper project guidelines.Suggested fix
import { ChevronDownIcon } from 'lucide-react-native'; import * as React from 'react'; import { useTranslation } from 'react-i18next'; import { useSelectedLanguage } from '@/lib'; -import { translate } from '@/lib'; import type { Language } from '@/lib/i18n/resources'; import { Select, SelectBackdrop, SelectContent, SelectDragIndicator, SelectDragIndicatorWrapper, SelectIcon, SelectInput, SelectItem, SelectPortal, SelectTrigger } from '../ui/select'; import { Text } from '../ui/text'; import { View } from '../ui/view'; export const LanguageItem = () => { const { language, setLanguage } = useSelectedLanguage(); const { t } = useTranslation(); const onSelect = React.useCallback( (option: string) => { setLanguage(option as Language); }, [setLanguage] ); - const langs = React.useMemo( - () => [ - { label: translate('settings.english'), value: 'en' }, - { label: translate('settings.spanish'), value: 'es' }, - { label: translate('settings.swedish'), value: 'sv' }, - { label: translate('settings.german'), value: 'de' }, - { label: translate('settings.french'), value: 'fr' }, - { label: translate('settings.italian'), value: 'it' }, - { label: translate('settings.polish'), value: 'pl' }, - { label: translate('settings.ukrainian'), value: 'uk' }, - { label: translate('settings.arabic'), value: 'ar' }, - ], - [] - ); + const langs = [ + { label: t('settings.english'), value: 'en' }, + { label: t('settings.spanish'), value: 'es' }, + { label: t('settings.swedish'), value: 'sv' }, + { label: t('settings.german'), value: 'de' }, + { label: t('settings.french'), value: 'fr' }, + { label: t('settings.italian'), value: 'it' }, + { label: t('settings.polish'), value: 'pl' }, + { label: t('settings.ukrainian'), value: 'uk' }, + { label: t('settings.arabic'), value: 'ar' }, + ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/language-item.tsx` around lines 23 - 35, The langs array is memoized with React.useMemo and uses the custom translate() causing labels to remain stuck at initial render; remove the memoization and replace translate() with the react-i18next t function so labels update when language changes—specifically, replace the React.useMemo-created langs constant with a plain constant or computed value that calls t('settings.english'), t('settings.spanish'), etc., and ensure the component imports and uses the t function from useTranslation (or props) instead of translate().src/app/call/[id]/edit.tsx (1)
151-208:⚠️ Potential issue | 🟠 MajorDon't reset the whole form whenever async extras arrive.
This effect now reruns when
callExtraData, priorities, or types load, and each rerun callsreset(...). If the user starts editing before those requests finish, their changes get silently overwritten just to backfilldispatchSelection. Initialize once per call, or update only the dispatch-related fields when extra dispatch data arrives.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/`[id]/edit.tsx around lines 151 - 208, The effect resets the whole form every time callExtraData, callPriorities, or callTypes change which clobbers user edits; change it so the full reset happens only once per call and subsequent async arrivals only update dispatchSelection. Concretely: in the useEffect surrounding call handling, guard the reset(...) call with a one-time flag (e.g., a hasInitializedForm ref) so reset is invoked only when the call initially loads, and in later runs when callExtraData arrives call setDispatchSelection(...) (or use the form API setValue for the dispatchSelection field) instead of calling reset; keep references to reset, setDispatchSelection, DispatchSelection, callExtraData, and useRef/useEffect to implement the one-time initialization.src/app/call/[id].web.tsx (1)
436-438:⚠️ Potential issue | 🟡 MinorUpdate keyboard shortcuts hint to reflect new tab count.
The hint text says "Press 1-5 to switch tabs" but there are now 7 tabs available. Update the translation key or default text to reflect this.
Proposed fix
<Text style={StyleSheet.flatten([styles.shortcutText, isDark ? styles.shortcutTextDark : styles.shortcutTextLight])}> - {t('call_detail.keyboard_shortcuts', 'Tip: Press 1-5 to switch tabs, Ctrl+E to edit, Escape to go back')} + {t('call_detail.keyboard_shortcuts', 'Tip: Press 1-7 to switch tabs, Ctrl+E to edit, Escape to go back')} </Text>Note: The translation key
call_detail.keyboard_shortcutsshould also be updated in all translation files to say "1-7" instead of "1-5".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/`[id].web.tsx around lines 436 - 438, Update the keyboard shortcuts hint to reflect the new tab count: change the default text in the Text node that uses the translation key call_detail.keyboard_shortcuts (in src/app/call/[id].web.tsx where the Text component renders t('call_detail.keyboard_shortcuts', ...)) from "Press 1-5 to switch tabs" to "Press 1-7 to switch tabs", and then update the same translation string in all translation resource files so the localized messages also say "1-7" instead of "1-5".src/components/dispatch-console/activity-log-panel.tsx (1)
1-29:⚠️ Potential issue | 🟡 MinorFix import sorting as flagged by ESLint.
The static analysis indicates these imports need to be sorted. Run the autofix to resolve this.
#!/bin/bash # Quick fix: run eslint autofix on this file npx eslint --fix src/components/dispatch-console/activity-log-panel.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/activity-log-panel.tsx` around lines 1 - 29, ESLint flagged unsorted imports at the top of the file (things like React/useColorScheme, lucide-react-native icons, nativewind, expo-router, model/store imports and local components such as Badge, Box, HStack, Icon, Text, VStack, PanelHeader, CheckInBottomSheet, PersonnelActionsPanel, UnitActionsPanel); fix by reordering imports into the project’s configured groups (external libs first—react, expo-router, lucide-react-native—then react-native/nativewind/hooks, then typed models/stores, then local UI components) or simply run the linter autofix (npx eslint --fix) to apply the project’s import-sorting rules, then verify symbols like useColorScheme, Badge, PanelHeader, CheckInBottomSheet, PersonnelActionsPanel and UnitActionsPanel remain correctly imported.src/app/(app)/home.tsx (1)
1-32:⚠️ Potential issue | 🟡 MinorFix import sorting to resolve pipeline failure.
The pipeline is failing due to unsorted imports. Run the autofix to sort them.
npx eslint --fix src/app/\(app\)/home.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/home.tsx around lines 1 - 32, The top import block in home.tsx (imports such as useFocusEffect, router, useColorScheme, React/useCallback/useEffect, useTranslation, ScrollView, getCallNotes/getCallExtraData/getMapDataAndMarkers, and component imports like AudioStreamBottomSheet/CloseCallBottomSheet/ActiveCallsPanel/etc.) is not sorted; run the project's autofixer to reorder imports (e.g. npx eslint --fix <file>) or manually reorder imports to match the repo's import-sorting rules, then commit the updated file.
🟡 Minor comments (21)
src/components/dispatch-console/units-panel.tsx-93-102 (1)
93-102:⚠️ Potential issue | 🟡 MinorAdd accessibility metadata to the icon-only details button.
This control is not screen-reader friendly as-is. Please add an explicit role/label (and hint if useful), ideally localized.
As per coding guidelines "Ensure the app is accessible, following WCAG guidelines for mobile applications" and "Ensure all text is wrapped in `t()` from `react-i18next` for translations".♿ Suggested fix
<Pressable onPress={(e) => { e.stopPropagation(); router.push(`/units/${unit.UnitId}` as Href); }} + accessibilityRole="button" + accessibilityLabel={t('dispatch.view_unit_details')} + accessibilityHint={t('dispatch.open_unit_details_hint')} hitSlop={{ top: 8, bottom: 8, left: 8, right: 8 }} style={styles.detailsButton} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/units-panel.tsx` around lines 93 - 102, The icon-only details Pressable (the Pressable wrapping ExternalLink with style styles.detailsButton and onPress that calls router.push(`/units/${unit.UnitId}`)) needs screen-reader metadata: add accessibilityRole="button", a localized accessibilityLabel using t('units.detailsButton' or similar) from react-i18next, and an optional localized accessibilityHint (e.g., t('units.detailsButtonHint')) to explain the navigation action; ensure you import and use the t() hook or translate function and pass these props to the Pressable so the control is accessible to screen readers.src/app/call/new/index.tsx-280-281 (1)
280-281:⚠️ Potential issue | 🟡 MinorReplace the route after create instead of pushing it.
router.pushleaves the submitted form in history, so back-navigation can reopen a stale screen and make duplicate submits too easy.router.replacegives the same destination without that path.Safer navigation
- router.push('/(app)/home' as Href); + router.replace('/(app)/home' as Href);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/new/index.tsx` around lines 280 - 281, The navigation after creating a call uses router.push('/(app)/home' as Href which leaves the form submission in history; change this to router.replace('/(app)/home' as Href so the created-screen isn't kept in history and back-navigation won't reopen a stale submit. Locate the call to router.push in the new call flow (the router.push(...) call in src/app/call/new/index.tsx) and swap it to router.replace with the same Href value.src/translations/es.json-735-735 (1)
735-735:⚠️ Potential issue | 🟡 MinorRemove the duplicated
settings.arabickey.JSON keeps the last duplicate key silently, so the earlier entry is shadowed and later translation edits become brittle.
Minimal fix
- "arabic": "Árabe",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/es.json` at line 735, Remove the duplicated "arabic" translation entry in the Spanish translations JSON so there is only a single "arabic" key under the settings translations; locate the repeated "arabic" key (referenced as settings.arabic / the "arabic" JSON entry) and delete the earlier duplicate entry so edits only affect the remaining key.src/translations/fr.json-711-711 (1)
711-711:⚠️ Potential issue | 🟡 MinorRemove duplicate
arabickey.The key
"arabic"is declared twice within thesettingsobject (lines 711 and 730). In JSON, duplicate keys cause the second value to silently override the first. Remove the duplicate declaration.Proposed fix
Remove line 711 (or line 730) to eliminate the duplicate:
"app_name": "Nom de l'application", - "arabic": "Arabe", "audio_device_selection": {Keep only one
"arabic": "Arabe"entry within thesettingsobject.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/fr.json` at line 711, The translations JSON contains a duplicate "arabic" key in the settings object; remove one of the duplicate entries so only a single "arabic": "Arabe" remains. Locate the two "arabic" keys in the settings object (the duplicate entries named "arabic") and delete either occurrence so the settings map has a single unique "arabic" key, then validate the JSON to ensure no syntax errors remain.src/translations/de.json-711-711 (1)
711-711:⚠️ Potential issue | 🟡 MinorRemove duplicate
arabickey.The key
"arabic"is declared twice within thesettingsobject (lines 711 and 730). In JSON, duplicate keys cause the second value to silently override the first, which can lead to unexpected behavior. Remove the duplicate declaration.Proposed fix
Remove line 711 (or line 730) to eliminate the duplicate:
"app_name": "App-Name", - "arabic": "Arabisch", "audio_device_selection": {Keep only one
"arabic": "Arabisch"entry within thesettingsobject.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/de.json` at line 711, The translations file contains a duplicate "arabic" key inside the settings object; remove one of the duplicate entries so only a single "arabic": "Arabisch" remains (locate the duplicated key named "arabic" in src/translations/de.json and delete the redundant entry), ensuring the settings object has unique keys.src/lib/offline/checkInQueue.ts-21-25 (1)
21-25:⚠️ Potential issue | 🟡 MinorPotential race condition with concurrent queue operations.
The read-modify-write pattern in
enqueueCheckInanddequeueCheckInis not atomic. If multiple check-ins are queued rapidly (e.g., during connectivity restoration), data could be lost. For a critical offline queue, consider adding a mutex or using atomic storage operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/offline/checkInQueue.ts` around lines 21 - 25, The read-modify-write in enqueueCheckIn (and likewise dequeueCheckIn) can race and lose entries; wrap the queue access (getQueuedCheckIns, push/splice, and storage.set with QUEUE_KEY) in a mutual exclusion so only one operation runs at a time (e.g., add a small async mutex/lock or retry CAS-style loop) — acquire the lock at the start of enqueueCheckIn and dequeueCheckIn, re-read getQueuedCheckIns after acquiring it, perform the mutation, call storage.set(QUEUE_KEY, ...) and then release the lock to ensure atomic queue updates.src/app/units/[id].tsx-51-51 (1)
51-51:⚠️ Potential issue | 🟡 MinorLocalize the remaining hard-coded copy in this screen.
This detail view still ships a few user-facing strings outside
t()(Failed to load unit details,GPS,Unassigned,Note), so the new surface will be partially untranslated.As per coding guidelines, "Ensure all text is wrapped in
t()fromreact-i18nextfor translations with the dictionary files stored insrc/translations."Also applies to: 241-242, 279-279, 362-362
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/units/`[id].tsx at line 51, The string literals shown in the Unit detail component must be wrapped with the i18n t() function: replace the hard-coded 'Failed to load unit details', 'GPS', 'Unassigned', and 'Note' usages in the [id] component (the code that calls setError(...) and the JSX that renders the GPS/Unassigned/Note labels) to use t('units.failedToLoad') / t('units.gps') / t('units.unassigned') / t('units.note') (or similar keys), ensure the component imports useTranslation and calls const { t } = useTranslation(), and add the corresponding keys and translations into the src/translations dictionary files so the strings are translatable across locales.src/components/callVideoFeeds/video-feed-card.tsx-1-27 (1)
1-27:⚠️ Potential issue | 🟡 MinorFix the import order to unblock CI.
This file is already failing
simple-import-sort/imports, so the current ordering will keep the lint job red until autofixed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/callVideoFeeds/video-feed-card.tsx` around lines 1 - 27, The imports at the top of video-feed-card.tsx are not sorted per simple-import-sort rules; reorder them to fix the linter by grouping and alphabetizing: 1) external packages (e.g. 'lucide-react-native', 'nativewind', 'react', 'react-i18next', 'react-native') sorted alphabetically, 2) third-party hooks like React imports together (e.g. React, { useState }), 3) absolute/internal project imports (everything from '@/components/...' and '@/models/...') sorted alphabetically by module path; ensure grouped imports keep their named specifiers as-is (e.g., CameraIcon,... from 'lucide-react-native', Box, Button, ButtonIcon, ButtonText, HStack, VStack, Text, and the CallVideoFeed enums/types) so simple-import-sort/imports passes.src/components/checkIn/check-in-timer-card.tsx-49-50 (1)
49-50:⚠️ Potential issue | 🟡 MinorReuse
typeLabelin the fallback subtitle.The subtitle branch at Lines 74-76 bypasses the translated
typeLabelyou already computed and can fall back to raw/internalTargetTypevalues whenTargetTypeNameis missing.Suggested change
- <Text className="text-xs font-medium text-gray-500"> - {timer.TargetTypeName || String(timer.TargetType)}{timer.TargetEntityId ? ` #${timer.TargetEntityId}` : ''} - </Text> + <Text className="text-xs font-medium text-gray-500"> + {typeLabel}{timer.TargetEntityId ? ` #${timer.TargetEntityId}` : ''} + </Text>As per coding guidelines, "Ensure all text is wrapped in
t()fromreact-i18nextfor translations with the dictionary files stored insrc/translations."Also applies to: 74-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkIn/check-in-timer-card.tsx` around lines 49 - 50, The subtitle branch is bypassing the already-computed, translated typeLabel; update the subtitle logic to reuse typeLabel (instead of directly using timer.TargetType or timer.TargetTypeName) and ensure it's wrapped with the i18n translator t() where needed so all displayed text uses translation keys (reference symbols: typeLabel, timer.TargetTypeName, timer.TargetType, CHECK_IN_TYPE_KEYS, and t()).src/stores/callVideoFeeds/store.ts-1-12 (1)
1-12:⚠️ Potential issue | 🟡 MinorFix import sorting to resolve pipeline failure.
The pipeline is failing due to unsorted imports.
npx eslint --fix src/stores/callVideoFeeds/store.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/callVideoFeeds/store.ts` around lines 1 - 12, Imports in src/stores/callVideoFeeds/store.ts are not correctly sorted and fail the pipeline; reorder them to match the project's ESLint import-sort rules (or run npx eslint --fix src/stores/callVideoFeeds/store.ts) so that the external module import (create from 'zustand') appears first, followed by grouped and alphabetized internal imports such as the callVideoFeeds API functions/types (deleteCallVideoFeed, editCallVideoFeed, getCallVideoFeeds, saveCallVideoFeed, EditCallVideoFeedInput, SaveCallVideoFeedInput), then logger (logger), and finally the model type import (CallVideoFeedResultData); ensure types and values stay correctly imported and no lines are left duplicated.src/app/(app)/weather-alerts/index.tsx-153-158 (1)
153-158:⚠️ Potential issue | 🟡 MinorAdd
handleAlertPressto dependency array.The
renderItemcallback referenceshandleAlertPressbut it's not included in the dependency array, which could cause stale closure issues ifhandleAlertPresswere to change.Proposed fix
const renderItem = useCallback( ({ item }: { item: WeatherAlertResultData }) => ( <WeatherAlertCard alert={item} onPress={handleAlertPress} /> ), - [] + [handleAlertPress] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/weather-alerts/index.tsx around lines 153 - 158, The renderItem callback (created with useCallback) references handleAlertPress but omits it from the dependency array, so update the useCallback for renderItem to include handleAlertPress in its dependency array; if handleAlertPress is not already stable, wrap/define it with useCallback (or otherwise memoize it) so WeatherAlertCard receives a stable onPress prop and renderItem doesn't capture a stale closure.src/app/(app)/weather-alerts/index.tsx-1-22 (1)
1-22:⚠️ Potential issue | 🟡 MinorFix import sorting to resolve pipeline failure.
The pipeline is failing due to unsorted imports.
npx eslint --fix src/app/\(app\)/weather-alerts/index.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/weather-alerts/index.tsx around lines 1 - 22, Imports at the top of index.tsx are not sorted, causing CI to fail; reorder the import statements (group and sort them per the project's ESLint import/order rules) so library imports come first (react/react-native/external libs like '@react-navigation/native', 'expo-router', 'nativewind', 'react', 'react-i18next'), then absolute app imports (components like WeatherAlertCard, Text, VStack, models like WeatherAlertSeverity, WeatherAlertResultData, and stores like useWeatherAlertsStore), and finally styles/locals, or simply run the suggested fix command (npx eslint --fix src/app/\(app\)/weather-alerts/index.tsx) to auto-fix; ensure symbols such as useFocusEffect, router, useColorScheme, useTranslation, WeatherAlertCard, Text, VStack, WeatherAlertSeverity, WeatherAlertResultData, and useWeatherAlertsStore remain imported and in the corrected order.src/components/callVideoFeeds/video-feed-form-sheet.tsx-158-164 (1)
158-164:⚠️ Potential issue | 🟡 MinorWrap error messages in translation function.
The error messages concatenate translated field names with hardcoded English " is required". Use fully translated error messages for proper i18n support.
Proposed fix
const validate = (): boolean => { const newErrors: { name?: string; url?: string } = {}; - if (!name.trim()) newErrors.name = t('videoFeeds.form.name') + ' is required'; - if (!url.trim()) newErrors.url = t('videoFeeds.form.url') + ' is required'; + if (!name.trim()) newErrors.name = t('videoFeeds.form.nameRequired'); + if (!url.trim()) newErrors.url = t('videoFeeds.form.urlRequired'); setErrors(newErrors); return Object.keys(newErrors).length === 0; };As per coding guidelines: "Ensure all text is wrapped in
t()fromreact-i18nextfor translations."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/callVideoFeeds/video-feed-form-sheet.tsx` around lines 158 - 164, Validation error messages currently concatenate translated field labels with hardcoded English text; update the validate function to use fully translated messages via t() instead—e.g., replace newErrors.name = t('videoFeeds.form.name') + ' is required' and newErrors.url = ... with calls like t('videoFeeds.form.required', { field: t('videoFeeds.form.name') }) (or an equivalent per your i18n keys) so both the field label and the "required" message are localized; adjust the validate function (and add any new i18n keys such as videoFeeds.form.required if needed) and keep using setErrors(newErrors) as before.src/app/(app)/home.tsx-48-48 (1)
48-48:⚠️ Potential issue | 🟡 MinorRemove unused
lastUpdateTimestampvariable.
lastUpdateTimestampis destructured fromuseSignalRStorebut never used in the component. The code now uses the more granular timestamps (lastPersonnelUpdateTimestamp,lastUnitsUpdateTimestamp,lastCallsUpdateTimestamp).Proposed fix
- const { lastUpdateTimestamp, lastPersonnelUpdateTimestamp, lastUnitsUpdateTimestamp, lastCallsUpdateTimestamp, lastEventType } = useSignalRStore(); + const { lastPersonnelUpdateTimestamp, lastUnitsUpdateTimestamp, lastCallsUpdateTimestamp } = useSignalRStore();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/home.tsx at line 48, Remove the unused destructured variable by eliminating lastUpdateTimestamp from the useSignalRStore() destructure in the component; update the line that currently reads const { lastUpdateTimestamp, lastPersonnelUpdateTimestamp, lastUnitsUpdateTimestamp, lastCallsUpdateTimestamp, lastEventType } = useSignalRStore(); to only include the actually used identifiers (e.g., lastPersonnelUpdateTimestamp, lastUnitsUpdateTimestamp, lastCallsUpdateTimestamp, lastEventType) so lastUpdateTimestamp is no longer declared.src/components/callVideoFeeds/video-feed-form-sheet.tsx-329-335 (1)
329-335:⚠️ Potential issue | 🟡 MinorWrap placeholder text in translation function.
The "Latitude" and "Longitude" placeholder strings should be translated.
Proposed fix
- <InputField value={latitude} onChangeText={setLatitude} placeholder="Latitude" keyboardType="decimal-pad" /> + <InputField value={latitude} onChangeText={setLatitude} placeholder={t('videoFeeds.form.latitude')} keyboardType="decimal-pad" /> </Input> </Box> <Box className="flex-1"> <Input> - <InputField value={longitude} onChangeText={setLongitude} placeholder="Longitude" keyboardType="decimal-pad" /> + <InputField value={longitude} onChangeText={setLongitude} placeholder={t('videoFeeds.form.longitude')} keyboardType="decimal-pad" />As per coding guidelines: "Ensure all text is wrapped in
t()fromreact-i18nextfor translations."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/callVideoFeeds/video-feed-form-sheet.tsx` around lines 329 - 335, The placeholder strings for the latitude/longitude inputs are not localized; update the InputField usages (the components referencing latitude/setLatitude and longitude/setLongitude) to wrap the placeholder values with the t() translation function (e.g., placeholder={t('Latitude')} and placeholder={t('Longitude')}) and ensure t is imported from react-i18next (useReactI18next hook or useTranslation) at the top of the component so translations are available.src/stores/weatherAlerts/store.ts-102-115 (1)
102-115:⚠️ Potential issue | 🟡 MinorPotential duplicate alerts in
handleAlertReceived.When a new alert is received, it's prepended to the existing alerts array without checking if it already exists. If the same alert ID is received multiple times (e.g., due to network retries or SignalR reconnection), duplicates could appear in the list.
🛡️ Add duplicate check before inserting
handleAlertReceived: async (alertId: string) => { try { const result = await getWeatherAlert(alertId); if (result.Data) { const current = get().alerts; + // Avoid duplicates if alert already exists + if (current.some((a) => a.WeatherAlertId === alertId)) { + return; + } const updated = [result.Data, ...current].sort(sortAlerts); set({ alerts: updated, lastWeatherAlertTimestamp: Date.now() }); } } catch (error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/weatherAlerts/store.ts` around lines 102 - 115, handleAlertReceived currently prepends result.Data to get().alerts without deduplication, causing duplicate alerts if the same alert is received multiple times; update handleAlertReceived to first check the existing alerts (get().alerts) for an entry with the same unique id (e.g., result.Data.id or other identifier on result.Data) and either skip insertion or replace the existing entry, then sort with sortAlerts and set({ alerts: updated, lastWeatherAlertTimestamp: Date.now() }); keep the existing catch fallback to fetchActiveAlerts() and preserve logging in logger.error.src/translations/ar.json-1027-1029 (1)
1027-1029:⚠️ Potential issue | 🟡 MinorComplete the Arabic
unitsnamespace translations.The Arabic
unitsnamespace contains only"title", but the English version has 27 keys including search, loading, empty, details, status, group, type, custom_fields, destination, and others. Arabic users viewing unit-related screens will encounter untranslated text for all missing keys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/ar.json` around lines 1027 - 1029, The "units" translation namespace in ar.json currently only contains "title" and is missing the ~27 keys present in the English namespace (e.g., "search", "loading", "empty", "details", "status", "group", "type", "custom_fields", "destination", etc.); open the English translations to copy the full set of keys under "units" and add them to the Arabic file with proper Arabic translations for each key (or placeholder Arabic TODOs if a translator is needed), ensuring you preserve the exact key names from the English "units" namespace, keep valid JSON syntax, and maintain existing formatting/order in the "units" object.src/translations/en.json-730-731 (1)
730-731:⚠️ Potential issue | 🟡 MinorDuplicate key
arabicin settings object.The key
settings.arabicappears twice in this file - first on line 711 and again on line 730. JSON parsers will typically use the last occurrence, but this is likely unintentional and could cause confusion.🐛 Remove the duplicate key
"current_unit": "Current Unit", - "arabic": "Arabic", "english": "English", "french": "French",Keep only the first occurrence at line 711.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/en.json` around lines 730 - 731, The translations file contains a duplicated "arabic" key in the settings/translations object; remove the second occurrence (the later "arabic": "Arabic" entry) so only the original "arabic" key remains, ensuring settings.english and other keys remain unchanged and the JSON stays valid.src/stores/checkIn/store.ts-1-14 (1)
1-14:⚠️ Potential issue | 🟡 MinorFix import sorting as flagged by ESLint.
Static analysis indicates imports need to be sorted. Run autofix to resolve.
🔧 Run autofix or manually sort imports
import { create } from 'zustand'; import { getCheckInHistory, + getTimersForCall, getTimerStatuses, getTimerStatusesForCalls, - getTimersForCall, performCheckIn, type PerformCheckInInput, toggleCallTimers, } from '@/api/checkIn/checkInTimers'; + import { type CheckInRecordResultData } from '@/models/v4/checkIn/checkInRecordResultData'; import { type CheckInTimerStatusResultData } from '@/models/v4/checkIn/checkInTimerStatusResultData'; import { type ResolvedCheckInTimerResultData } from '@/models/v4/checkIn/resolvedCheckInTimerResultData';Or run:
npx eslint --fix src/stores/checkIn/store.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/checkIn/store.ts` around lines 1 - 14, The imports at the top of store.ts are not alphabetically/ESLint-sorted; reorder the import statements (e.g., grouped external packages first like create from 'zustand', then local API imports such as getCheckInHistory, getTimerStatuses, getTimerStatusesForCalls, getTimersForCall, performCheckIn, PerformCheckInInput, toggleCallTimers, followed by model types CheckInRecordResultData, CheckInTimerStatusResultData, ResolvedCheckInTimerResultData) to match your ESLint import/order rules or simply run npx eslint --fix src/stores/checkIn/store.ts to apply the autofix so the linter passes.src/translations/ar.json-731-732 (1)
731-732:⚠️ Potential issue | 🟡 MinorDuplicate key
arabicin settings object.Same issue as in
en.json- the keysettings.arabicappears twice (lines 712 and 731).🐛 Remove the duplicate key
"current_unit": "الوحدة الحالية", - "arabic": "عربي", "english": "إنجليزي",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/ar.json` around lines 731 - 732, Remove the duplicate settings key by deleting the repeated "arabic" entry so that settings.arabic appears only once; locate the duplicate "arabic" key in the translations object (settings.arabic) and remove the later occurrence (the one at the duplicate spot), ensuring the remaining entry has the correct Arabic string and that "english" and other sibling keys remain intact; also scan the other locale file (en.json) for the same duplicate and fix it there if present.src/components/checkIn/check-in-bottom-sheet.tsx-88-89 (1)
88-89:⚠️ Potential issue | 🟡 Minor
parseIntwithout radix and potential0value issue.Line 89 uses
parseInt(selected.TargetEntityId)without a radix parameter, and line 109 usesresolvedUnitId || undefinedwhich would convert a valid0toundefined.🐛 Fix parseInt and falsy value handling
// Resolve UnitId: prefer the explicit field, fall back to parsing TargetEntityId const resolvedUnitId = selected.UnitId - || parseInt(selected.TargetEntityId) || 0; + ?? (selected.TargetEntityId ? parseInt(selected.TargetEntityId, 10) : 0); ... const success = await performCheckIn({ CallId: effectiveCallId, CheckInType: selected.TargetType, - UnitId: resolvedUnitId || undefined, + UnitId: resolvedUnitId !== 0 ? resolvedUnitId : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkIn/check-in-bottom-sheet.tsx` around lines 88 - 89, The current assignment to resolvedUnitId uses parseInt without a radix and later uses falsy-coercion (resolvedUnitId || undefined) which turns a valid 0 into undefined; change parseInt(selected.TargetEntityId) to parseInt(selected.TargetEntityId, 10) and stop using || to normalize falsy numbers—use explicit checks or nullish coalescing (e.g., ?? or a ternary) so resolvedUnitId becomes either a number (including 0) or undefined, referencing resolvedUnitId, selected.UnitId and selected.TargetEntityId for the update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b3f5509c-7ff6-4e9e-a20c-7ead92e60d19
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (92)
.gitignoreCLAUDE.mdNOTICE__mocks__/@gorhom/bottom-sheet.web.jsapp.config.tsmetro.config.jspackage.jsonsrc/api/callVideoFeeds/callVideoFeeds.tssrc/api/calls/calls.tssrc/api/checkIn/checkInTimers.tssrc/api/weatherAlerts/weatherAlerts.tssrc/app/(app)/_layout.tsxsrc/app/(app)/calls.tsxsrc/app/(app)/home.tsxsrc/app/(app)/home.web.tsxsrc/app/(app)/personnel.tsxsrc/app/(app)/units.tsxsrc/app/(app)/weather-alerts/index.tsxsrc/app/_layout.tsxsrc/app/call/[id].tsxsrc/app/call/[id].web.tsxsrc/app/call/[id]/edit.tsxsrc/app/call/[id]/edit.web.tsxsrc/app/call/new/index.tsxsrc/app/call/new/index.web.tsxsrc/app/personnel/[id].tsxsrc/app/units/[id].tsxsrc/app/weather-alerts/[id].tsxsrc/components/callVideoFeeds/video-feed-card.tsxsrc/components/callVideoFeeds/video-feed-form-sheet.tsxsrc/components/callVideoFeeds/video-feeds-tab.tsxsrc/components/callVideoFeeds/video-player.tsxsrc/components/calls/__tests__/dispatch-selection-modal.test.tsxsrc/components/calls/call-card.tsxsrc/components/calls/dispatch-selection-modal.tsxsrc/components/checkIn/__tests__/check-in-bottom-sheet.test.tsxsrc/components/checkIn/__tests__/check-in-history-list.test.tsxsrc/components/checkIn/__tests__/check-in-tab.test.tsxsrc/components/checkIn/__tests__/check-in-timer-card.test.tsxsrc/components/checkIn/check-in-bottom-sheet.tsxsrc/components/checkIn/check-in-history-list.tsxsrc/components/checkIn/check-in-tab.tsxsrc/components/checkIn/check-in-timer-card.tsxsrc/components/dispatch-console/__tests__/active-calls-panel.test.tsxsrc/components/dispatch-console/activity-log-panel.tsxsrc/components/dispatch-console/personnel-panel.tsxsrc/components/dispatch-console/units-panel.tsxsrc/components/maps/unified-map-view.tsxsrc/components/settings/language-item.tsxsrc/components/sidebar/side-menu.tsxsrc/components/weatherAlerts/weather-alert-banner.tsxsrc/components/weatherAlerts/weather-alert-card.tsxsrc/lib/i18n/resources.tssrc/lib/native/checkInLiveActivity.tssrc/lib/offline/checkInQueue.tssrc/lib/offline/checkInQueueProcessor.tssrc/lib/storage/index.web.tsxsrc/models/v4/callVideoFeeds/callVideoFeedEnums.tssrc/models/v4/callVideoFeeds/callVideoFeedResultData.tssrc/models/v4/callVideoFeeds/callVideoFeedsResult.tssrc/models/v4/callVideoFeeds/deleteCallVideoFeedResult.tssrc/models/v4/callVideoFeeds/saveCallVideoFeedResult.tssrc/models/v4/calls/callResultData.tssrc/models/v4/checkIn/checkInRecordResult.tssrc/models/v4/checkIn/checkInRecordResultData.tssrc/models/v4/checkIn/checkInTimerStatusResult.tssrc/models/v4/checkIn/checkInTimerStatusResultData.tssrc/models/v4/checkIn/performCheckInResult.tssrc/models/v4/checkIn/resolvedCheckInTimerResult.tssrc/models/v4/checkIn/resolvedCheckInTimerResultData.tssrc/models/v4/weatherAlerts/activeWeatherAlertsResult.tssrc/models/v4/weatherAlerts/weatherAlertEnums.tssrc/models/v4/weatherAlerts/weatherAlertResult.tssrc/models/v4/weatherAlerts/weatherAlertResultData.tssrc/models/v4/weatherAlerts/weatherAlertSettingsData.tssrc/models/v4/weatherAlerts/weatherAlertSettingsResult.tssrc/models/v4/weatherAlerts/weatherAlertZoneResultData.tssrc/models/v4/weatherAlerts/weatherAlertZonesResult.tssrc/stores/callVideoFeeds/store.tssrc/stores/checkIn/store.tssrc/stores/dispatch/store.tssrc/stores/signalr/signalr-store.tssrc/stores/weatherAlerts/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 with no reviewable changes (1)
- src/app/(app)/calls.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/call/[id].web.tsx (1)
433-435:⚠️ Potential issue | 🟡 MinorKeyboard shortcuts hint is outdated.
The hint text mentions "Press 1-5 to switch tabs" but the actual keyboard handler (Line 143) now supports keys 1-7 for the new video and check-in tabs. The translation key should be updated or the hardcoded fallback corrected.
🔧 Suggested fix
<Text style={StyleSheet.flatten([styles.shortcutText, isDark ? styles.shortcutTextDark : styles.shortcutTextLight])}> - {t('call_detail.keyboard_shortcuts', 'Tip: Press 1-5 to switch tabs, Ctrl+E to edit, Escape to go back')} + {t('call_detail.keyboard_shortcuts', 'Tip: Press 1-7 to switch tabs, Ctrl+E to edit, Escape to go back')} </Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/`[id].web.tsx around lines 433 - 435, The tooltip string for 'call_detail.keyboard_shortcuts' is stale (mentions "Press 1-5") while the keyboard handler that processes numeric tab switches supports 1-7; update the translation fallback text (and any translation entry for call_detail.keyboard_shortcuts) to "Press 1-7 to switch tabs, Ctrl+E to edit, Escape to go back" so it matches the keyboard handler behavior, ensuring the Text component rendering that key ('call_detail.keyboard_shortcuts') and the keyboard handler logic remain consistent.
🧹 Nitpick comments (19)
src/app/(app)/units.tsx (2)
139-149: Consider adding FlatList optimization props.♻️ Suggested optimization props
<FlatList<UnitInfoResultData> testID="units-list" data={filteredUnits} renderItem={renderItem} keyExtractor={keyExtractor} refreshControl={<RefreshControl refreshing={isLoading} onRefresh={fetchUnits} />} ListEmptyComponent={<ZeroState heading={emptyHeading} description={emptyDescription} icon={Truck} />} contentContainerStyle={styles.listContent} + removeClippedSubviews={true} + maxToRenderPerBatch={10} + windowSize={5} />As per coding guidelines: "Optimize FlatLists with props like
removeClippedSubviews,maxToRenderPerBatch, andwindowSize".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/units.tsx around lines 139 - 149, The FlatList rendering in this component should be optimized: update the FlatList component (the instance that sets testID="units-list" and uses data={filteredUnits}, renderItem={renderItem}, keyExtractor={keyExtractor}) to add optimization props such as removeClippedSubviews={true}, maxToRenderPerBatch={10} (or an appropriate small number), and windowSize={5} (adjust as needed for UX) so off-screen items are clipped and rendering batches/window are reduced to improve performance.
122-122: Avoid anonymous function inrenderItemto prevent re-renders.Same issue as in
personnel.tsx— the inline() => handleUnitPress(item)creates a new function on each render, negatingReact.memobenefits.♻️ Proposed refactor
Update
UnitCardto acceptonPress: (unit: UnitInfoResultData) => voidand handle the callback internally:- const renderItem = useCallback(({ item }: { item: UnitInfoResultData }) => <UnitCard unit={item} onPress={() => handleUnitPress(item)} />, [handleUnitPress]); + const renderItem = useCallback(({ item }: { item: UnitInfoResultData }) => <UnitCard unit={item} onPress={handleUnitPress} />, [handleUnitPress]);As per coding guidelines: "Avoid anonymous functions in
renderItemor event handlers to prevent re-renders".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/units.tsx at line 122, The inline arrow in renderItem creates a new function each render; change UnitCard to accept onPress: (unit: UnitInfoResultData) => void and have UnitCard call onPress(unit) internally, then update renderItem to pass the stable handleUnitPress reference (e.g., const renderItem = useCallback(({ item }) => <UnitCard unit={item} onPress={handleUnitPress} />, [handleUnitPress])) so no per-item anonymous function is created; update UnitCard's prop type and its internal press handler to invoke onPress(unit).src/app/(app)/personnel.tsx (2)
110-110: Avoid anonymous function inrenderItemto prevent re-renders.The inline arrow function
() => handlePersonPress(item)creates a new function reference on each render, bypassing theReact.memooptimization onPersonnelCard. Consider passingitemas a prop and handling the press insidePersonnelCard.♻️ Proposed refactor
const PersonnelCard: React.FC<{ person: PersonnelInfoResultData; - onPress: () => void; -}> = React.memo(({ person, onPress }) => { + onPress: (person: PersonnelInfoResultData) => void; +}> = React.memo(({ person, onPress }) => { + const handlePress = useCallback(() => onPress(person), [onPress, person]); // ... rest of component return ( - <Pressable onPress={onPress} style={styles.cardPressable}> + <Pressable onPress={handlePress} style={styles.cardPressable}>Then update
renderItem:- const renderItem = useCallback(({ item }: { item: PersonnelInfoResultData }) => <PersonnelCard person={item} onPress={() => handlePersonPress(item)} />, [handlePersonPress]); + const renderItem = useCallback(({ item }: { item: PersonnelInfoResultData }) => <PersonnelCard person={item} onPress={handlePersonPress} />, [handlePersonPress]);As per coding guidelines: "Avoid anonymous functions in
renderItemor event handlers to prevent re-renders".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/personnel.tsx at line 110, The renderItem currently creates an inline arrow function for onPress which breaks memoization; update the pattern so PersonnelCard receives the raw handlers/data and invokes them internally: pass item as a prop (already passed as person) and pass the stable handler reference handlePersonPress (not a new arrow) to PersonnelCard (e.g., onPress={handlePersonPress}), then modify PersonnelCard to call onPress(person) when tapped. Change the renderItem callback signature to use the stable handlePersonPress reference and avoid creating () => handlePersonPress(item) so PersonnelCard can remain memoized.
128-136: Consider adding FlatList optimization props.For larger personnel lists, adding optimization props can improve scroll performance, especially on lower-end devices.
♻️ Suggested optimization props
<FlatList<PersonnelInfoResultData> testID="personnel-list" data={filteredPersonnel} renderItem={renderItem} keyExtractor={keyExtractor} refreshControl={<RefreshControl refreshing={isLoading} onRefresh={fetchPersonnel} />} ListEmptyComponent={<ZeroState heading={emptyHeading} description={emptyDescription} icon={Users} />} contentContainerStyle={styles.listContent} + removeClippedSubviews={true} + maxToRenderPerBatch={10} + windowSize={5} />As per coding guidelines: "Optimize FlatLists with props like
removeClippedSubviews,maxToRenderPerBatch, andwindowSize".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/personnel.tsx around lines 128 - 136, The FlatList rendering personnel lacks performance tuning for large lists; update the FlatList component by adding optimization props: set removeClippedSubviews={true}, adjust maxToRenderPerBatch (e.g., 10-20) and windowSize (e.g., 5-10) and consider initialNumToRender to a reasonable value; keep the existing props (data={filteredPersonnel}, renderItem={renderItem}, keyExtractor={keyExtractor}, refreshControl, ListEmptyComponent, contentContainerStyle={styles.listContent}) and tune the numeric values based on expected list size and device constraints to improve scroll performance.src/app/personnel/[id].tsx (1)
325-330: Consider using UDF field names instead of IDs for better user experience.The code currently displays
UdfFieldId(technical ID) rather than the human-readable field name. To implement this, fetch the UDF definitions using the existinggetUdfDefinition()API (for Personnel, EntityType=1), then map the field IDs to their display names from theNameproperty of the returnedUdfFieldResultData.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/personnel/`[id].tsx around lines 325 - 330, Replace displaying technical IDs with human-readable UDF names: call getUdfDefinition({ EntityType: 1 }) (or reuse existing cached definitions) in the component that renders person.UdfValues, build a map from UdfFieldResultData items keyed by UdfFieldId -> Name, then change the render logic inside the person.UdfValues.map (the block referencing UdfFieldId and Value) to use the mapped Name for the label (fallback to udf.UdfFieldId or '—' if no name found). Ensure fetching happens before render (use useEffect/useState or receive definitions as prop) and keep the existing Value display unchanged.src/components/calls/call-images-modal.tsx (1)
386-401: Add FlatList virtualization optimization props for fixed-width paging items.This horizontal paging FlatList should include virtualization hints to improve performance on lower-end devices.
♻️ Proposed patch
<FlatList ref={flatListRef} data={validImages} renderItem={renderImageItem} keyExtractor={(item) => item?.Id || `image-${Math.random()}`} + removeClippedSubviews + maxToRenderPerBatch={3} + windowSize={5} + getItemLayout={(_, index) => ({ + length: width, + offset: width * index, + index, + })} horizontal pagingEnabled showsHorizontalScrollIndicator={false}Per coding guidelines, FlatLists with consistent item sizing should use
getItemLayout, and virtualization props likeremoveClippedSubviews,maxToRenderPerBatch, andwindowSizeimprove rendering efficiency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/calls/call-images-modal.tsx` around lines 386 - 401, The horizontal paging FlatList (FlatList with ref={flatListRef}, data={validImages}, renderItem={renderImageItem}) lacks virtualization hints for fixed-width items; add a getItemLayout implementation that returns {length, offset, index} using the known item width to enable fast jumps and set removeClippedSubviews={true}, maxToRenderPerBatch (e.g. 3), and windowSize (e.g. 5) props to reduce work on low-end devices; keep initialScrollIndex and keyExtractor but ensure getItemLayout and these virtualization props are added to the FlatList props to improve performance.src/app/(app)/weather-alerts/index.tsx (1)
131-138: Consider adding FlatList optimization props.Per coding guidelines, FlatLists should be optimized with props like
removeClippedSubviews,maxToRenderPerBatch, andwindowSizefor better performance, especially on low-end devices.♻️ Suggested optimization
<FlatList data={filteredAlerts} keyExtractor={(item) => item.WeatherAlertId} renderItem={renderItem} contentContainerStyle={styles.listContent} ListEmptyComponent={renderEmpty} refreshControl={<RefreshControl refreshing={isLoading} onRefresh={fetchActiveAlerts} />} + removeClippedSubviews={true} + maxToRenderPerBatch={10} + windowSize={5} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/weather-alerts/index.tsx around lines 131 - 138, The FlatList rendering filteredAlerts lacks recommended performance props; update the FlatList (the component using data={filteredAlerts}, keyExtractor, renderItem, and refreshControl with fetchActiveAlerts) to include optimization props such as removeClippedSubviews={true}, maxToRenderPerBatch={10} (or a tuned value), and windowSize={21} (or tuned) and consider initialNumToRender if appropriate; add these props directly to the FlatList declaration to improve memory and scroll performance on low-end devices.src/components/dispatch-console/activity-log-panel.tsx (2)
689-691: Use ternary operator for conditional rendering.The weather tab button uses
&&for conditional rendering instead of the required ternary operator pattern.♻️ Suggested refactor
- {weatherSettings?.WeatherAlertsEnabled !== false && ( + {weatherSettings?.WeatherAlertsEnabled !== false ? ( <TabButton label={t('weatherAlerts.title')} icon={CloudLightning} isActive={activeTab === 'weather'} count={weatherAlertCount > 0 ? weatherAlertCount : undefined} onPress={() => setActiveTab('weather')} /> - )} + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/activity-log-panel.tsx` around lines 689 - 691, The conditional render of the weather tab uses && instead of the required ternary pattern; update the JSX around TabButton so it uses a ternary expression that checks weatherSettings?.WeatherAlertsEnabled !== false and returns the TabButton (with props: label={t('weatherAlerts.title')}, icon={CloudLightning}, isActive={activeTab === 'weather'}, count={weatherAlertCount > 0 ? weatherAlertCount : undefined}, onPress={() => setActiveTab('weather')}) when true and returns null when false, preserving the existing condition and prop logic.
560-574: Use ternary operator for conditional rendering.As per coding guidelines, use the ternary operator (
? :) instead of&&for conditional rendering. This applies to the summary badges section.♻️ Suggested refactor
{/* Summary badges */} - {urgentCheckInCount > 0 && ( + {urgentCheckInCount > 0 ? ( <HStack className="mb-2 gap-2"> - {criticalCheckInCount > 0 && ( + {criticalCheckInCount > 0 ? ( <Box className="rounded-full bg-red-200 px-2 py-0.5"> <Text className="text-xs font-bold text-red-700"> {criticalCheckInCount} {t('check_in.status_critical')} </Text> </Box> - )} - {overdueCheckInCount > 0 && ( + ) : null} + {overdueCheckInCount > 0 ? ( <Box className="rounded-full bg-red-100 px-2 py-0.5"> <Text className="text-xs font-medium text-red-600">{t('check_in.overdue_count', { count: overdueCheckInCount })}</Text> </Box> - )} + ) : null} </HStack> - )} + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/activity-log-panel.tsx` around lines 560 - 574, The JSX uses logical && for conditional rendering in the summary badges (inside the HStack) — replace those with ternary expressions so each conditional element uses the form condition ? <Box ...>...</Box> : null; specifically change the {criticalCheckInCount > 0 && (...)} and {overdueCheckInCount > 0 && (...)} (and the outer {urgentCheckInCount > 0 && (...)}) to use ternaries while preserving the same elements (HStack, Box, Text) and the i18n calls like t('check_in.status_critical') and t('check_in.overdue_count', { count: overdueCheckInCount }).src/app/(app)/home.tsx (2)
705-706: Use ternary operator for conditional rendering.The close call bottom sheet conditional rendering should use the ternary operator pattern.
♻️ Suggested refactor
{/* Close Call Bottom Sheet */} - {selectedCallId && <CloseCallBottomSheet isOpen={isCloseCallSheetOpen} onClose={() => setIsCloseCallSheetOpen(false)} callId={selectedCallId} />} + {selectedCallId ? ( + <CloseCallBottomSheet isOpen={isCloseCallSheetOpen} onClose={() => setIsCloseCallSheetOpen(false)} callId={selectedCallId} /> + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/home.tsx around lines 705 - 706, Replace the short-circuit conditional rendering of CloseCallBottomSheet with a ternary expression: where the JSX currently uses {selectedCallId && <CloseCallBottomSheet ... />}, change it to {selectedCallId ? <CloseCallBottomSheet isOpen={isCloseCallSheetOpen} onClose={() => setIsCloseCallSheetOpen(false)} callId={selectedCallId} /> : null} so the component mounts only when selectedCallId is truthy and otherwise renders null; update the usage around the CloseCallBottomSheet component and keep the same props (isCloseCallSheetOpen, setIsCloseCallSheetOpen, callId) intact.
690-692: Use ternary operator for conditional rendering.As per coding guidelines, replace the
&&operator with a ternary for the weather alert banner conditional rendering.♻️ Suggested refactor
{/* Weather Alert Banner */} - {weatherSettings?.WeatherAlertsEnabled !== false && weatherAlerts.length > 0 && <WeatherAlertBanner alerts={weatherAlerts} onPress={() => router.push('/(app)/weather-alerts' as Href)} />} + {weatherSettings?.WeatherAlertsEnabled !== false && weatherAlerts.length > 0 ? ( + <WeatherAlertBanner alerts={weatherAlerts} onPress={() => router.push('/(app)/weather-alerts' as Href)} /> + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/home.tsx around lines 690 - 692, The current JSX uses && for conditional rendering of the WeatherAlertBanner; update the expression to use a ternary so it follows the project guideline: replace the expression that checks weatherSettings?.WeatherAlertsEnabled !== false && weatherAlerts.length > 0 and renders <WeatherAlertBanner ... /> with a ternary that returns the banner when true and null (or an empty fragment) when false; keep the same props (alerts={weatherAlerts} and onPress={() => router.push('/(app)/weather-alerts' as Href)}) and ensure references to weatherSettings, WeatherAlertsEnabled, weatherAlerts, WeatherAlertBanner, router.push and Href are unchanged.src/components/checkIn/check-in-history-list.tsx (1)
57-57: Consider defensive date parsing for the timestamp.If
record.Timestampcontains an invalid date string,new Date(record.Timestamp)will create an "Invalid Date" object, andformat()will throw an error. Consider adding validation.🛡️ Suggested defensive parsing
- <Text className="text-xs text-gray-400">{record.Timestamp ? format(new Date(record.Timestamp), 'MMM d, h:mm a') : ''}</Text> + <Text className="text-xs text-gray-400"> + {record.Timestamp && !isNaN(new Date(record.Timestamp).getTime()) + ? format(new Date(record.Timestamp), 'MMM d, h:mm a') + : ''} + </Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkIn/check-in-history-list.tsx` at line 57, The Text line that formats record.Timestamp is not defensive against invalid dates; update the rendering in check-in-history-list (the Text that uses format(new Date(record.Timestamp), ...)) to validate the parsed date before calling format: parse record.Timestamp into a Date (or use date-fns parseISO), check validity (e.g., !isNaN(parsedDate.getTime())), and only call format when valid—otherwise render an empty string or a fallback like "Invalid date". Ensure you change the expression inside the Text component that references record.Timestamp so it performs the validation and conditional formatting.src/stores/checkIn/store.ts (2)
92-99: Silent failure infetchResolvedTimersloses debugging context.While the comment explains this is intentional for supplementary data, consider at least logging at debug level for troubleshooting:
fetchResolvedTimers: async (callId: number) => { try { const result = await getTimersForCall(callId); set({ resolvedTimers: result.Data || [] }); - } catch { + } catch (error) { + logger.debug({ message: 'Failed to fetch resolved timers (non-critical)', context: { callId, error } }); // silent fail — resolved timers are supplementary } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/checkIn/store.ts` around lines 92 - 99, The fetchResolvedTimers function currently swallows all errors silently; update its catch block to log the failure at debug (or trace) level including the error object and context (callId) so issues can be diagnosed without changing behavior—inside fetchResolvedTimers after calling getTimersForCall use your logger (or console.debug if no logger exists) to emit a message like "fetchResolvedTimers failed" with { callId, error } before returning, leaving set({ resolvedTimers: result.Data || [] }) unchanged.
33-34: Storing interval reference in Zustand state may cause issues with persistence/devtools.Storing
_pollingInterval(a timer ID) directly in Zustand state can cause issues:
- Timer IDs aren't serializable, breaking Redux DevTools and persistence middleware
- State snapshots will include stale timer references
Consider using a module-scoped variable instead (similar to
updateHubHandlersin signalr-store.ts):+// Module-scoped polling state to avoid serialization issues +let pollingInterval: ReturnType<typeof setInterval> | null = null; +let pollingCallIds: number[] = []; interface CheckInState { // ... other fields - _pollingInterval: ReturnType<typeof setInterval> | null; - _pollingCallIds: number[]; // ... }Then update the polling functions to use the module-scoped variables.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/checkIn/store.ts` around lines 33 - 34, The store currently keeps timer IDs in state via the _pollingInterval and _pollingCallIds fields which are non-serializable; move these to module-scoped variables (e.g., declare let modulePollingInterval: ReturnType<typeof setInterval> | null = null and let modulePollingCallIds: number[] = []) and remove _pollingInterval/_pollingCallIds from the Zustand state shape, then update all polling-related functions (the start/stop/poll handlers that reference _pollingInterval and _pollingCallIds) to read/update the new module-scoped variables instead of the state fields; ensure any state reads/writes that must persist only keep serializable data and that the module variables are cleared when stopping polling.src/app/weather-alerts/[id].tsx (2)
201-208: TheisDarkprop is unused inDetailRow.The
isDarkprop is passed toDetailRowbut the component relies solely on Tailwind dark mode classes (dark:text-gray-400, etc.) which auto-detect the color scheme. Consider removing the unused prop to avoid confusion.-const DetailRow: React.FC<{ label: string; value: string; isDark: boolean }> = ({ label, value, isDark }) => ( +const DetailRow: React.FC<{ label: string; value: string }> = ({ label, value }) => (Then update call sites (lines 156-159, 190-192) to remove the
isDarkprop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/weather-alerts/`[id].tsx around lines 201 - 208, Remove the unused isDark prop from the DetailRow component and all its call sites: update the DetailRow declaration (DetailRow: React.FC<{ label: string; value: string; isDark: boolean }>) to drop the isDark prop from the props type and signature, and then remove any passed isDark={...} arguments where DetailRow is instantiated so the component only takes label and value; ensure imports/uses still compile after the change.
105-113: Consider using StyleSheet for inline styles.The retry button and text use inline style objects. For consistency and potential performance benefits, consider moving these to the StyleSheet.
- <Text style={{ color: isDark ? '#fff' : '#000', marginBottom: 16 }}>{t('common.no_results_found')}</Text> + <Text style={StyleSheet.flatten([styles.noResultsText, isDark ? styles.textLight : styles.textDark])}> + {t('common.no_results_found')} + </Text> <Pressable onPress={() => { if (id) fetchAlertDetail(id); }} - style={{ paddingHorizontal: 16, paddingVertical: 8, backgroundColor: '#3b82f6', borderRadius: 8 }} + style={styles.retryButton} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/weather-alerts/`[id].tsx around lines 105 - 113, The inline style objects used on the no-results Text and the retry Pressable/Text should be moved into a StyleSheet to improve consistency and performance: create a StyleSheet entry (e.g., styles.noResultsText, styles.retryButton, styles.retryButtonText) and replace the inline style props on the Text and Pressable/Text elements inside the component that renders the retry button (the block that calls fetchAlertDetail(id) onPress). Ensure you preserve conditional color logic (isDark ? '#fff' : '#000') by applying it in the style prop or computing a textColor variable used with styles.noResultsText, and keep layout properties (paddingHorizontal, paddingVertical, backgroundColor, borderRadius, marginBottom) in the new StyleSheet entries.src/stores/weatherAlerts/store.ts (1)
5-5: Unused import:WeatherAlertSeverity.The
WeatherAlertSeverityenum is imported but not used in this file. ThesortAlertsfunction compares numeric severity values directly.-import { WeatherAlertSeverity } from '@/models/v4/weatherAlerts/weatherAlertEnums';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/weatherAlerts/store.ts` at line 5, The import WeatherAlertSeverity is unused in this file; remove the unused import statement to clean up the module. Locate the import line and delete the WeatherAlertSeverity token (or the whole import if it only contains that enum). Ensure the sortAlerts function continues to compare numeric severity values directly and no other code references WeatherAlertSeverity in this file.src/components/checkIn/check-in-bottom-sheet.tsx (2)
81-82:parseIntmay returnNaNfor non-numericTargetEntityId.If
TargetEntityIdis a UUID or non-numeric string,parseIntreturnsNaN, andNaN || 0evaluates to0. This is safe but consider adding explicit handling:- const resolvedUnitId = selected.UnitId || parseInt(selected.TargetEntityId) || 0; + const parsedEntityId = parseInt(selected.TargetEntityId, 10); + const resolvedUnitId = selected.UnitId || (Number.isNaN(parsedEntityId) ? 0 : parsedEntityId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkIn/check-in-bottom-sheet.tsx` around lines 81 - 82, The current resolvedUnitId computation uses parseInt(selected.TargetEntityId) which can produce NaN for UUIDs/non-numeric strings and then fall back to 0 implicitly; update the logic in the resolvedUnitId assignment to explicitly validate the parsed value: if selected.UnitId is truthy use it, otherwise attempt to parse selected.TargetEntityId, check with Number.isFinite or isNaN to ensure the parsed value is a valid number, and only then use it; if parsing fails, use a clear fallback (e.g., 0 or null) so resolvedUnitId never silently becomes 0 from an invalid parse.
84-90: Replaceconsole.logwithloggerfor consistency.The codebase uses a
loggerutility (seen in other files). Consider using it here for consistent logging behavior and potential production filtering.- console.log('[CheckIn] Performing check-in:', { + logger.debug({ message: 'Performing check-in', context: { effectiveCallId, TargetType: selected.TargetType, TargetEntityId: selected.TargetEntityId, UnitId: resolvedUnitId, TargetName: selected.TargetName, - }); + }});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkIn/check-in-bottom-sheet.tsx` around lines 84 - 90, Replace the console.log call in the CheckInBottomSheet component with the project's logger utility: import the shared logger and use logger.debug (or logger.info per convention) instead of console.log, passing the same structured object (effectiveCallId, selected.TargetType, selected.TargetEntityId, resolvedUnitId, selected.TargetName) so the log remains identical; ensure the import for logger is added at the top of the file if missing and remove the console.log call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(app)/weather-alerts/index.tsx:
- Line 104: renderItem currently has an empty dependency array but closes over
handleAlertPress; update the useCallback for renderItem to include
handleAlertPress in its dependency array so React (and exhaustive-deps linting)
knows it depends on that stable handler. Locate the renderItem definition and
add handleAlertPress to its dependencies (renderItem and handleAlertPress are
the identifiers to modify), ensuring you don't break other logic —
handleAlertPress itself remains unchanged (it depends on router.push).
In `@src/app/weather-alerts/`[id].tsx:
- Around line 136-142: The current markup wraps lucide-react-native icons
returned by getCategoryIcon() with the gluestack-ui Icon component (see Icon and
CategoryIcon usage inside the HStack), which violates the guideline to use
lucide-react-native icons directly; change the code to render the CategoryIcon
component directly (use the returned component from getCategoryIcon() as a JSX
element) and apply color/size by passing props or wrapping it in a View for
styling (replace <Icon as={CategoryIcon} .../> with direct <CategoryIcon .../>
or <View style={{...}}><CategoryIcon .../></View>), leaving the surrounding
HStack/VStack/Text unchanged.
In `@src/components/checkIn/check-in-bottom-sheet.tsx`:
- Around line 46-51: The typeLabel function is mismatching keys by using
String(timer.TargetType) for the CHECK_IN_TYPE_KEYS lookup; change it to use
timer.TargetTypeName for the mapping and translation: if timer.TargetTypeName
exists, return t(timer.TargetTypeName) (so the API name is translated),
otherwise look up CHECK_IN_TYPE_KEYS[timer.TargetTypeName] (or
CHECK_IN_TYPE_KEYS[String(timer.TargetTypeName)]) and if a key is found return
t(key); if nothing matches fall back to String(timer.TargetType) to preserve a
sensible output; update references in typeLabel and keep CHECK_IN_TYPE_KEYS,
timer.TargetTypeName, timer.TargetType and t() as the identifying symbols.
In `@src/components/dispatch-console/activity-log-panel.tsx`:
- Around line 393-403: The dependency array contains a complex expression
activeCallIds.join(','); extract this into a stable variable (e.g., const
activeCallIdsKey = useMemo(() => activeCallIds.join(','), [activeCallIds])) and
then use activeCallIdsKey in the useEffect dependencies instead of the inline
join; keep the other dependencies fetchTimerStatusesForCalls,
startCheckInPolling, stopCheckInPolling unchanged and ensure the effect still
uses activeCallIds for the actual calls when invoking fetchTimerStatusesForCalls
and startCheckInPolling.
In `@src/stores/callVideoFeeds/store.ts`:
- Around line 41-53: The addFeed implementation currently logs failures and
returns null but doesn't set the store's error state; update the addFeed (and
likewise updateFeed) to set({ error }) on catch so consumers get consistent UI
feedback, ensuring you still set({ isSaving: false }) before returning; include
a normalized error value (e.g., error.message or a wrapped object) so
store.error is predictable for components using getState().error.
- Around line 26-37: fetchFeeds can be overwritten by stale responses; add a
simple in-store fetch guard: before calling getCallVideoFeeds in fetchFeeds,
generate/increment a unique fetch token (e.g., currentFetchId or lastFetchId
stored in the same store state) and capture it in a local variable, then only
call set({ feeds, isLoading: false }) or set error if the captured token matches
the store's current fetch token (ignore the result when it does not match).
Apply the same token check for success and catch branches so older responses or
errors cannot overwrite newer callId results; reference fetchFeeds,
getCallVideoFeeds and the store keys feeds/isLoading/error/currentFetchId in
your changes.
- Around line 70-81: In removeFeed, don’t revert to the stale snapshot
(previousFeeds) on delete failure because that can clobber newer state; instead
catch the error from deleteCallVideoFeed(feedId), log it, and trigger a server
resync for the call (e.g., call the existing fetch/load function that populates
feeds for the given callId — reference callId parameter and the store’s
feed-loading method such as fetchCallVideoFeeds or loadFeedsForCall) to refresh
state from the server; remove the set({ feeds: previousFeeds }) rollback and
ensure the error path returns false after initiating the re-sync.
In `@src/stores/signalr/signalr-store.ts`:
- Around line 286-321: The three handlers updateHubHandlers.weatherAlertReceived
/ weatherAlertUpdated / weatherAlertExpired currently coerce message to a string
which can produce "[object Object]"; define a type-safe payload interface (e.g.,
WeatherAlertPayload with alertId: string) and add a type guard or runtime check
to extract alertId safely (handle string messages and objects with alertId),
then call useWeatherAlertsStore.getState().handleAlertReceived /
handleAlertUpdated / handleAlertExpired with the extracted alertId; update the
set(...) calls to store the raw message (e.g., JSON.stringify(message)) only
after validating/extracting alertId and log a warning or bail out if the payload
is invalid.
In `@src/stores/weatherAlerts/store.ts`:
- Around line 65-74: In fetchAlertDetail, when getWeatherAlert fails the
existing selectedAlert is left stale; in the catch block update the state via
set to clear selectedAlert (e.g., set selectedAlert to null or undefined) and
set isLoadingDetail: false so consumers don't see old data; modify the catch in
fetchAlertDetail to call set({ selectedAlert: null, isLoadingDetail: false })
and keep the existing logger.error call (references: fetchAlertDetail,
getWeatherAlert, selectedAlert, set).
- Around line 146-157: The reset() implementation in the WeatherAlerts store
currently omits the settings field from the set(...) call, leaving
WeatherAlertsState.settings unchanged; update reset() (the reset function in
src/stores/weatherAlerts/store.ts) to explicitly reset settings to its
initial/default value (e.g., the same default used when initializing
WeatherAlertsState or a named constant like defaultSettings) or to null if
that’s the intended default, ensuring settings is included alongside alerts,
isLoading, error, selectedAlert, isLoadingDetail, nearbyAlerts, isLoadingNearby,
and lastWeatherAlertTimestamp; if settings are intentionally persistent, add a
short comment above reset() documenting that behavior instead.
---
Outside diff comments:
In `@src/app/call/`[id].web.tsx:
- Around line 433-435: The tooltip string for 'call_detail.keyboard_shortcuts'
is stale (mentions "Press 1-5") while the keyboard handler that processes
numeric tab switches supports 1-7; update the translation fallback text (and any
translation entry for call_detail.keyboard_shortcuts) to "Press 1-7 to switch
tabs, Ctrl+E to edit, Escape to go back" so it matches the keyboard handler
behavior, ensuring the Text component rendering that key
('call_detail.keyboard_shortcuts') and the keyboard handler logic remain
consistent.
---
Nitpick comments:
In `@src/app/`(app)/home.tsx:
- Around line 705-706: Replace the short-circuit conditional rendering of
CloseCallBottomSheet with a ternary expression: where the JSX currently uses
{selectedCallId && <CloseCallBottomSheet ... />}, change it to {selectedCallId ?
<CloseCallBottomSheet isOpen={isCloseCallSheetOpen} onClose={() =>
setIsCloseCallSheetOpen(false)} callId={selectedCallId} /> : null} so the
component mounts only when selectedCallId is truthy and otherwise renders null;
update the usage around the CloseCallBottomSheet component and keep the same
props (isCloseCallSheetOpen, setIsCloseCallSheetOpen, callId) intact.
- Around line 690-692: The current JSX uses && for conditional rendering of the
WeatherAlertBanner; update the expression to use a ternary so it follows the
project guideline: replace the expression that checks
weatherSettings?.WeatherAlertsEnabled !== false && weatherAlerts.length > 0 and
renders <WeatherAlertBanner ... /> with a ternary that returns the banner when
true and null (or an empty fragment) when false; keep the same props
(alerts={weatherAlerts} and onPress={() => router.push('/(app)/weather-alerts'
as Href)}) and ensure references to weatherSettings, WeatherAlertsEnabled,
weatherAlerts, WeatherAlertBanner, router.push and Href are unchanged.
In `@src/app/`(app)/personnel.tsx:
- Line 110: The renderItem currently creates an inline arrow function for
onPress which breaks memoization; update the pattern so PersonnelCard receives
the raw handlers/data and invokes them internally: pass item as a prop (already
passed as person) and pass the stable handler reference handlePersonPress (not a
new arrow) to PersonnelCard (e.g., onPress={handlePersonPress}), then modify
PersonnelCard to call onPress(person) when tapped. Change the renderItem
callback signature to use the stable handlePersonPress reference and avoid
creating () => handlePersonPress(item) so PersonnelCard can remain memoized.
- Around line 128-136: The FlatList rendering personnel lacks performance tuning
for large lists; update the FlatList component by adding optimization props: set
removeClippedSubviews={true}, adjust maxToRenderPerBatch (e.g., 10-20) and
windowSize (e.g., 5-10) and consider initialNumToRender to a reasonable value;
keep the existing props (data={filteredPersonnel}, renderItem={renderItem},
keyExtractor={keyExtractor}, refreshControl, ListEmptyComponent,
contentContainerStyle={styles.listContent}) and tune the numeric values based on
expected list size and device constraints to improve scroll performance.
In `@src/app/`(app)/units.tsx:
- Around line 139-149: The FlatList rendering in this component should be
optimized: update the FlatList component (the instance that sets
testID="units-list" and uses data={filteredUnits}, renderItem={renderItem},
keyExtractor={keyExtractor}) to add optimization props such as
removeClippedSubviews={true}, maxToRenderPerBatch={10} (or an appropriate small
number), and windowSize={5} (adjust as needed for UX) so off-screen items are
clipped and rendering batches/window are reduced to improve performance.
- Line 122: The inline arrow in renderItem creates a new function each render;
change UnitCard to accept onPress: (unit: UnitInfoResultData) => void and have
UnitCard call onPress(unit) internally, then update renderItem to pass the
stable handleUnitPress reference (e.g., const renderItem = useCallback(({ item
}) => <UnitCard unit={item} onPress={handleUnitPress} />, [handleUnitPress])) so
no per-item anonymous function is created; update UnitCard's prop type and its
internal press handler to invoke onPress(unit).
In `@src/app/`(app)/weather-alerts/index.tsx:
- Around line 131-138: The FlatList rendering filteredAlerts lacks recommended
performance props; update the FlatList (the component using
data={filteredAlerts}, keyExtractor, renderItem, and refreshControl with
fetchActiveAlerts) to include optimization props such as
removeClippedSubviews={true}, maxToRenderPerBatch={10} (or a tuned value), and
windowSize={21} (or tuned) and consider initialNumToRender if appropriate; add
these props directly to the FlatList declaration to improve memory and scroll
performance on low-end devices.
In `@src/app/personnel/`[id].tsx:
- Around line 325-330: Replace displaying technical IDs with human-readable UDF
names: call getUdfDefinition({ EntityType: 1 }) (or reuse existing cached
definitions) in the component that renders person.UdfValues, build a map from
UdfFieldResultData items keyed by UdfFieldId -> Name, then change the render
logic inside the person.UdfValues.map (the block referencing UdfFieldId and
Value) to use the mapped Name for the label (fallback to udf.UdfFieldId or '—'
if no name found). Ensure fetching happens before render (use useEffect/useState
or receive definitions as prop) and keep the existing Value display unchanged.
In `@src/app/weather-alerts/`[id].tsx:
- Around line 201-208: Remove the unused isDark prop from the DetailRow
component and all its call sites: update the DetailRow declaration (DetailRow:
React.FC<{ label: string; value: string; isDark: boolean }>) to drop the isDark
prop from the props type and signature, and then remove any passed isDark={...}
arguments where DetailRow is instantiated so the component only takes label and
value; ensure imports/uses still compile after the change.
- Around line 105-113: The inline style objects used on the no-results Text and
the retry Pressable/Text should be moved into a StyleSheet to improve
consistency and performance: create a StyleSheet entry (e.g.,
styles.noResultsText, styles.retryButton, styles.retryButtonText) and replace
the inline style props on the Text and Pressable/Text elements inside the
component that renders the retry button (the block that calls
fetchAlertDetail(id) onPress). Ensure you preserve conditional color logic
(isDark ? '#fff' : '#000') by applying it in the style prop or computing a
textColor variable used with styles.noResultsText, and keep layout properties
(paddingHorizontal, paddingVertical, backgroundColor, borderRadius,
marginBottom) in the new StyleSheet entries.
In `@src/components/calls/call-images-modal.tsx`:
- Around line 386-401: The horizontal paging FlatList (FlatList with
ref={flatListRef}, data={validImages}, renderItem={renderImageItem}) lacks
virtualization hints for fixed-width items; add a getItemLayout implementation
that returns {length, offset, index} using the known item width to enable fast
jumps and set removeClippedSubviews={true}, maxToRenderPerBatch (e.g. 3), and
windowSize (e.g. 5) props to reduce work on low-end devices; keep
initialScrollIndex and keyExtractor but ensure getItemLayout and these
virtualization props are added to the FlatList props to improve performance.
In `@src/components/checkIn/check-in-bottom-sheet.tsx`:
- Around line 81-82: The current resolvedUnitId computation uses
parseInt(selected.TargetEntityId) which can produce NaN for UUIDs/non-numeric
strings and then fall back to 0 implicitly; update the logic in the
resolvedUnitId assignment to explicitly validate the parsed value: if
selected.UnitId is truthy use it, otherwise attempt to parse
selected.TargetEntityId, check with Number.isFinite or isNaN to ensure the
parsed value is a valid number, and only then use it; if parsing fails, use a
clear fallback (e.g., 0 or null) so resolvedUnitId never silently becomes 0 from
an invalid parse.
- Around line 84-90: Replace the console.log call in the CheckInBottomSheet
component with the project's logger utility: import the shared logger and use
logger.debug (or logger.info per convention) instead of console.log, passing the
same structured object (effectiveCallId, selected.TargetType,
selected.TargetEntityId, resolvedUnitId, selected.TargetName) so the log remains
identical; ensure the import for logger is added at the top of the file if
missing and remove the console.log call.
In `@src/components/checkIn/check-in-history-list.tsx`:
- Line 57: The Text line that formats record.Timestamp is not defensive against
invalid dates; update the rendering in check-in-history-list (the Text that uses
format(new Date(record.Timestamp), ...)) to validate the parsed date before
calling format: parse record.Timestamp into a Date (or use date-fns parseISO),
check validity (e.g., !isNaN(parsedDate.getTime())), and only call format when
valid—otherwise render an empty string or a fallback like "Invalid date". Ensure
you change the expression inside the Text component that references
record.Timestamp so it performs the validation and conditional formatting.
In `@src/components/dispatch-console/activity-log-panel.tsx`:
- Around line 689-691: The conditional render of the weather tab uses && instead
of the required ternary pattern; update the JSX around TabButton so it uses a
ternary expression that checks weatherSettings?.WeatherAlertsEnabled !== false
and returns the TabButton (with props: label={t('weatherAlerts.title')},
icon={CloudLightning}, isActive={activeTab === 'weather'},
count={weatherAlertCount > 0 ? weatherAlertCount : undefined}, onPress={() =>
setActiveTab('weather')}) when true and returns null when false, preserving the
existing condition and prop logic.
- Around line 560-574: The JSX uses logical && for conditional rendering in the
summary badges (inside the HStack) — replace those with ternary expressions so
each conditional element uses the form condition ? <Box ...>...</Box> : null;
specifically change the {criticalCheckInCount > 0 && (...)} and
{overdueCheckInCount > 0 && (...)} (and the outer {urgentCheckInCount > 0 &&
(...)}) to use ternaries while preserving the same elements (HStack, Box, Text)
and the i18n calls like t('check_in.status_critical') and
t('check_in.overdue_count', { count: overdueCheckInCount }).
In `@src/stores/checkIn/store.ts`:
- Around line 92-99: The fetchResolvedTimers function currently swallows all
errors silently; update its catch block to log the failure at debug (or trace)
level including the error object and context (callId) so issues can be diagnosed
without changing behavior—inside fetchResolvedTimers after calling
getTimersForCall use your logger (or console.debug if no logger exists) to emit
a message like "fetchResolvedTimers failed" with { callId, error } before
returning, leaving set({ resolvedTimers: result.Data || [] }) unchanged.
- Around line 33-34: The store currently keeps timer IDs in state via the
_pollingInterval and _pollingCallIds fields which are non-serializable; move
these to module-scoped variables (e.g., declare let modulePollingInterval:
ReturnType<typeof setInterval> | null = null and let modulePollingCallIds:
number[] = []) and remove _pollingInterval/_pollingCallIds from the Zustand
state shape, then update all polling-related functions (the start/stop/poll
handlers that reference _pollingInterval and _pollingCallIds) to read/update the
new module-scoped variables instead of the state fields; ensure any state
reads/writes that must persist only keep serializable data and that the module
variables are cleared when stopping polling.
In `@src/stores/weatherAlerts/store.ts`:
- Line 5: The import WeatherAlertSeverity is unused in this file; remove the
unused import statement to clean up the module. Locate the import line and
delete the WeatherAlertSeverity token (or the whole import if it only contains
that enum). Ensure the sortAlerts function continues to compare numeric severity
values directly and no other code references WeatherAlertSeverity in this file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31cb0436-1c2c-4775-ad52-da3dc1dc231a
📒 Files selected for processing (50)
metro.config.jssrc/__tests__/app/call/[id].security.test.tsxsrc/__tests__/app/call/[id].test.tsxsrc/api/weatherAlerts/weatherAlerts.tssrc/app/(app)/home.tsxsrc/app/(app)/home.web.tsxsrc/app/(app)/personnel.tsxsrc/app/(app)/units.tsxsrc/app/(app)/weather-alerts/index.tsxsrc/app/call/[id].tsxsrc/app/call/[id].web.tsxsrc/app/personnel/[id].tsxsrc/app/units/[id].tsxsrc/app/weather-alerts/[id].tsxsrc/components/callVideoFeeds/video-feed-card.tsxsrc/components/callVideoFeeds/video-feeds-tab.tsxsrc/components/callVideoFeeds/video-player.tsxsrc/components/calls/call-card.tsxsrc/components/calls/call-files-modal.tsxsrc/components/calls/call-images-modal.tsxsrc/components/checkIn/__tests__/check-in-bottom-sheet.test.tsxsrc/components/checkIn/__tests__/check-in-tab.test.tsxsrc/components/checkIn/__tests__/check-in-timer-card.test.tsxsrc/components/checkIn/check-in-bottom-sheet.tsxsrc/components/checkIn/check-in-history-list.tsxsrc/components/checkIn/check-in-tab.tsxsrc/components/checkIn/check-in-timer-card.tsxsrc/components/dispatch-console/activity-log-panel.tsxsrc/components/dispatch-console/personnel-panel.tsxsrc/components/maps/unified-map-view.tsxsrc/components/notifications/NotificationInbox.tsxsrc/components/ui/flat-list.tsxsrc/components/weatherAlerts/weather-alert-banner.tsxsrc/components/weatherAlerts/weather-alert-card.tsxsrc/lib/form-render/form-render-source.tssrc/lib/form-render/jquery-source.tssrc/lib/offline/checkInQueue.tssrc/lib/offline/checkInQueueProcessor.tssrc/models/v4/checkIn/checkInRecordResult.tssrc/models/v4/checkIn/checkInTimerStatusResult.tssrc/models/v4/checkIn/resolvedCheckInTimerResult.tssrc/stores/callVideoFeeds/store.tssrc/stores/checkIn/store.tssrc/stores/dispatch/store.tssrc/stores/signalr/signalr-store.tssrc/stores/weatherAlerts/store.tssrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
💤 Files with no reviewable changes (1)
- src/components/notifications/NotificationInbox.tsx
✅ Files skipped from review due to trivial changes (14)
- src/components/ui/flat-list.tsx
- src/components/calls/call-files-modal.tsx
- src/models/v4/checkIn/checkInTimerStatusResult.ts
- src/models/v4/checkIn/checkInRecordResult.ts
- src/models/v4/checkIn/resolvedCheckInTimerResult.ts
- src/components/checkIn/tests/check-in-tab.test.tsx
- src/components/calls/call-card.tsx
- src/translations/it.json
- src/components/checkIn/tests/check-in-bottom-sheet.test.tsx
- src/translations/sv.json
- src/components/checkIn/check-in-tab.tsx
- src/translations/pl.json
- src/translations/uk.json
- src/app/call/[id].tsx
🚧 Files skipped from review as they are similar to previous changes (13)
- metro.config.js
- src/components/checkIn/tests/check-in-timer-card.test.tsx
- src/components/weatherAlerts/weather-alert-card.tsx
- src/app/(app)/home.web.tsx
- src/lib/offline/checkInQueue.ts
- src/lib/offline/checkInQueueProcessor.ts
- src/components/callVideoFeeds/video-feed-card.tsx
- src/app/units/[id].tsx
- src/stores/dispatch/store.ts
- src/components/checkIn/check-in-timer-card.tsx
- src/components/callVideoFeeds/video-player.tsx
- src/components/weatherAlerts/weather-alert-banner.tsx
- src/components/callVideoFeeds/video-feeds-tab.tsx
| ); | ||
| }; | ||
|
|
||
| const renderItem = useCallback(({ item }: { item: WeatherAlertResultData }) => <WeatherAlertCard alert={item} onPress={handleAlertPress} />, []); |
Contributor
There was a problem hiding this comment.
Add handleAlertPress to the dependency array.
The renderItem callback references handleAlertPress but has an empty dependency array. While handleAlertPress is stable (only depends on router.push), including it in the dependencies makes the intent clearer and satisfies exhaustive-deps rules.
🔧 Suggested fix
- const renderItem = useCallback(({ item }: { item: WeatherAlertResultData }) => <WeatherAlertCard alert={item} onPress={handleAlertPress} />, []);
+ const renderItem = useCallback(({ item }: { item: WeatherAlertResultData }) => <WeatherAlertCard alert={item} onPress={handleAlertPress} />, [handleAlertPress]);📝 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 renderItem = useCallback(({ item }: { item: WeatherAlertResultData }) => <WeatherAlertCard alert={item} onPress={handleAlertPress} />, []); | |
| const renderItem = useCallback(({ item }: { item: WeatherAlertResultData }) => <WeatherAlertCard alert={item} onPress={handleAlertPress} />, [handleAlertPress]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(app)/weather-alerts/index.tsx at line 104, renderItem currently has
an empty dependency array but closes over handleAlertPress; update the
useCallback for renderItem to include handleAlertPress in its dependency array
so React (and exhaustive-deps linting) knows it depends on that stable handler.
Locate the renderItem definition and add handleAlertPress to its dependencies
(renderItem and handleAlertPress are the identifiers to modify), ensuring you
don't break other logic — handleAlertPress itself remains unchanged (it depends
on router.push).
Comment on lines
+46
to
+51
| const typeLabel = (timer: CheckInTimerStatusResultData) => { | ||
| // Use TargetTypeName from API, fall back to translation key lookup | ||
| if (timer.TargetTypeName) return timer.TargetTypeName; | ||
| const key = CHECK_IN_TYPE_KEYS[String(timer.TargetType)]; | ||
| return key ? t(key) : String(timer.TargetType); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what values TargetType and TargetTypeName contain in the model
ast-grep --pattern 'class CheckInTimerStatusResultData {
$$$
}'Repository: Resgrid/Dispatch
Length of output: 1341
🏁 Script executed:
rg "CHECK_IN_TYPE_KEYS" -A 10 -B 2Repository: Resgrid/Dispatch
Length of output: 4621
🏁 Script executed:
rg "enum.*TargetType|TargetType.*enum" -i -A 15Repository: Resgrid/Dispatch
Length of output: 42
🏁 Script executed:
rg "TargetType\s*[=:|<>]" -B 2 -A 2 | head -40Repository: Resgrid/Dispatch
Length of output: 2329
🏁 Script executed:
rg "TargetTypeName" -B 3 -A 3 src/components/checkIn/ | head -50Repository: Resgrid/Dispatch
Length of output: 3812
🏁 Script executed:
rg "TargetTypeName\s*[=:].*''|TargetTypeName.*empty" -iRepository: Resgrid/Dispatch
Length of output: 245
Fix type mismatch: TargetType is numeric but CHECK_IN_TYPE_KEYS has string keys.
timer.TargetType is a number, but CHECK_IN_TYPE_KEYS uses string keys like 'Personnel', 'Unit', etc. The current code String(timer.TargetType) converts numeric values (e.g., 1) to strings like '1', which will never match any keys in the mapping.
The lookup should use TargetTypeName instead:
const typeLabel = (timer: CheckInTimerStatusResultData) => {
// Use TargetTypeName from API, fall back to translation key lookup
- if (timer.TargetTypeName) return timer.TargetTypeName;
- const key = CHECK_IN_TYPE_KEYS[String(timer.TargetType)];
- return key ? t(key) : String(timer.TargetType);
+ if (timer.TargetTypeName) return t(CHECK_IN_TYPE_KEYS[timer.TargetTypeName] || timer.TargetTypeName);
+ return String(timer.TargetType);This also wraps TargetTypeName with t() for translation consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/checkIn/check-in-bottom-sheet.tsx` around lines 46 - 51, The
typeLabel function is mismatching keys by using String(timer.TargetType) for the
CHECK_IN_TYPE_KEYS lookup; change it to use timer.TargetTypeName for the mapping
and translation: if timer.TargetTypeName exists, return t(timer.TargetTypeName)
(so the API name is translated), otherwise look up
CHECK_IN_TYPE_KEYS[timer.TargetTypeName] (or
CHECK_IN_TYPE_KEYS[String(timer.TargetTypeName)]) and if a key is found return
t(key); if nothing matches fall back to String(timer.TargetType) to preserve a
sensible output; update references in typeLabel and keep CHECK_IN_TYPE_KEYS,
timer.TargetTypeName, timer.TargetType and t() as the identifying symbols.
Comment on lines
+146
to
+157
| reset: () => { | ||
| set({ | ||
| alerts: [], | ||
| isLoading: false, | ||
| error: null, | ||
| selectedAlert: null, | ||
| isLoadingDetail: false, | ||
| nearbyAlerts: [], | ||
| isLoadingNearby: false, | ||
| lastWeatherAlertTimestamp: 0, | ||
| }); | ||
| }, |
Contributor
There was a problem hiding this comment.
reset() does not clear settings field.
The settings field is part of WeatherAlertsState but is not reset. If this is intentional (settings persist across resets), consider documenting it. Otherwise:
reset: () => {
set({
alerts: [],
isLoading: false,
error: null,
selectedAlert: null,
isLoadingDetail: false,
nearbyAlerts: [],
isLoadingNearby: false,
lastWeatherAlertTimestamp: 0,
+ settings: null,
});
},📝 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
| reset: () => { | |
| set({ | |
| alerts: [], | |
| isLoading: false, | |
| error: null, | |
| selectedAlert: null, | |
| isLoadingDetail: false, | |
| nearbyAlerts: [], | |
| isLoadingNearby: false, | |
| lastWeatherAlertTimestamp: 0, | |
| }); | |
| }, | |
| reset: () => { | |
| set({ | |
| alerts: [], | |
| isLoading: false, | |
| error: null, | |
| selectedAlert: null, | |
| isLoadingDetail: false, | |
| nearbyAlerts: [], | |
| isLoadingNearby: false, | |
| lastWeatherAlertTimestamp: 0, | |
| settings: null, | |
| }); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/weatherAlerts/store.ts` around lines 146 - 157, The reset()
implementation in the WeatherAlerts store currently omits the settings field
from the set(...) call, leaving WeatherAlertsState.settings unchanged; update
reset() (the reset function in src/stores/weatherAlerts/store.ts) to explicitly
reset settings to its initial/default value (e.g., the same default used when
initializing WeatherAlertsState or a named constant like defaultSettings) or to
null if that’s the intended default, ensuring settings is included alongside
alerts, isLoading, error, selectedAlert, isLoadingDetail, nearbyAlerts,
isLoadingNearby, and lastWeatherAlertTimestamp; if settings are intentionally
persistent, add a short comment above reset() documenting that behavior instead.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
src/stores/callVideoFeeds/store.ts (1)
46-47:⚠️ Potential issue | 🟡 MinorClear stale
errorwhen startingaddFeed/updateFeed.At Line 47 and Line 62, starting a new save without resetting
errorcan keep old failure messages visible during retry.Suggested patch
addFeed: async (input: SaveCallVideoFeedInput) => { - set({ isSaving: true }); + set({ isSaving: true, error: null }); try { const result = await saveCallVideoFeed(input); // Re-fetch to get the full feed data await get().fetchFeeds(input.CallId); set({ isSaving: false }); return result.Id || null; @@ updateFeed: async (input: EditCallVideoFeedInput) => { - set({ isSaving: true }); + set({ isSaving: true, error: null }); try { await editCallVideoFeed(input); await get().fetchFeeds(input.CallId); set({ isSaving: false }); return true;As per coding guidelines: "Handle errors gracefully and provide user feedback".
Also applies to: 61-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/callVideoFeeds/store.ts` around lines 46 - 47, The addFeed and updateFeed save flows leave previous errors set when starting a new save; update the start of both functions (addFeed and updateFeed in src/stores/callVideoFeeds/store.ts) to clear stale error state by setting error to null when flipping isSaving to true (e.g., set({ isSaving: true, error: null })), so retries don't display old failure messages.
🧹 Nitpick comments (7)
src/app/weather-alerts/[id].tsx (1)
156-158: Use ternary operator for conditional rendering.These lines use the
&&operator. Per coding guidelines, prefer the ternary operator.♻️ Suggested fix
- {alert.OnsetUtc && <DetailRow label={t('weatherAlerts.detail.onset')} value={alert.OnsetUtc} isDark={isDark} />} - {alert.ExpiresUtc && <DetailRow label={t('weatherAlerts.detail.expires')} value={alert.ExpiresUtc} isDark={isDark} />} - {alert.SentUtc && <DetailRow label={t('weatherAlerts.detail.sent')} value={alert.SentUtc} isDark={isDark} />} + {alert.OnsetUtc ? <DetailRow label={t('weatherAlerts.detail.onset')} value={alert.OnsetUtc} isDark={isDark} /> : null} + {alert.ExpiresUtc ? <DetailRow label={t('weatherAlerts.detail.expires')} value={alert.ExpiresUtc} isDark={isDark} /> : null} + {alert.SentUtc ? <DetailRow label={t('weatherAlerts.detail.sent')} value={alert.SentUtc} isDark={isDark} /> : null}As per coding guidelines: "Use ternary operator (? :) for conditional rendering and not && operator".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/weather-alerts/`[id].tsx around lines 156 - 158, Replace the conditional renderings that use the && operator with ternary expressions so they explicitly render null when the value is falsy; specifically update the three occurrences where DetailRow is rendered for alert.OnsetUtc, alert.ExpiresUtc, and alert.SentUtc to use the ternary operator (e.g., alert.OnsetUtc ? <DetailRow ... /> : null) while preserving the same props (label from t('...') and isDark) and keeping DetailRow as the component being rendered.src/app/(app)/home.tsx (1)
709-710: Use ternary operator for conditional rendering.Per coding guidelines, use the ternary operator
(? :)instead of&&for conditional rendering.♻️ Suggested fix
{/* Close Call Bottom Sheet */} - {selectedCallId && <CloseCallBottomSheet isOpen={isCloseCallSheetOpen} onClose={() => setIsCloseCallSheetOpen(false)} callId={selectedCallId} />} + {selectedCallId ? <CloseCallBottomSheet isOpen={isCloseCallSheetOpen} onClose={() => setIsCloseCallSheetOpen(false)} callId={selectedCallId} /> : null}As per coding guidelines: "Use ternary operator (? :) for conditional rendering and not && operator".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/home.tsx around lines 709 - 710, The JSX currently uses && for conditional rendering: replace the expression that renders CloseCallBottomSheet when selectedCallId is truthy with a ternary expression (selectedCallId ? <CloseCallBottomSheet isOpen={isCloseCallSheetOpen} onClose={() => setIsCloseCallSheetOpen(false)} callId={selectedCallId} /> : null) so that rendering follows the project's guideline; update the usage of selectedCallId, CloseCallBottomSheet, isCloseCallSheetOpen and setIsCloseCallSheetOpen accordingly.src/components/dispatch-console/activity-log-panel.tsx (3)
627-627: Use lucide-react-native icons directly without the gluestack-ui Icon wrapper.Per coding guidelines, use
lucide-react-nativecomponents directly instead of wrapping them with theIconcomponent from gluestack-ui.♻️ Suggested fix
- <Icon as={ChevronRight} size="xs" className="text-gray-400 dark:text-gray-500" /> + <ChevronRight size={12} color="#9CA3AF" />As per coding guidelines: "Use
lucide-react-nativefor icons and use those components directly in the markup and don't use the gluestack-ui icon component".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/activity-log-panel.tsx` at line 627, The JSX uses the gluestack-ui Icon wrapper (Icon as={ChevronRight}) which violates the guideline—replace the wrapped usage with the lucide-react-native component directly: render ChevronRight instead of Icon, remove the Icon import from gluestack-ui, and import ChevronRight from 'lucide-react-native'; ensure you map/adjust props (size/color/className → size/color or style) on the ChevronRight component in activity-log-panel (reference Icon and ChevronRight symbols and the ActivityLogPanel component) and remove any unused Icon imports.
578-593: Use ternary operator for conditional rendering.Lines 578 and 587 use the
&&operator for conditional rendering. Per coding guidelines, prefer the ternary operator.♻️ Suggested fix
- {urgentCheckInCount > 0 && ( + {urgentCheckInCount > 0 ? ( <HStack className="mb-2 gap-2"> - {criticalCheckInCount > 0 && ( + {criticalCheckInCount > 0 ? ( <Box className="rounded-full bg-red-200 px-2 py-0.5"> <Text className="text-xs font-bold text-red-700"> {criticalCheckInCount} {t('check_in.status_critical')} </Text> </Box> - )} - {overdueCheckInCount > 0 && ( + ) : null} + {overdueCheckInCount > 0 ? ( <Box className="rounded-full bg-red-100 px-2 py-0.5"> <Text className="text-xs font-medium text-red-600">{t('check_in.overdue_count', { count: overdueCheckInCount })}</Text> </Box> - )} + ) : null} </HStack> - )} + ) : null}As per coding guidelines: "Use ternary operator (? :) for conditional rendering and not && operator".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/activity-log-panel.tsx` around lines 578 - 593, The conditional JSX uses logical && for rendering badges (checks using urgentCheckInCount, criticalCheckInCount, overdueCheckInCount inside the HStack/Box blocks) which violates the guideline; update each conditional expression to use a ternary operator instead (e.g., replace expressions like {urgentCheckInCount > 0 && (<HStack>...)} and the nested {criticalCheckInCount > 0 && (<Box>...)} / {overdueCheckInCount > 0 && (<Box>...)} with the equivalent ternary forms that render the JSX when true and null when false) so the component (activity-log-panel / ActivityLogPanel) follows the ? : pattern for conditional rendering.
705-711: Consider using ternary operator for consistency.The conditional tab rendering uses
&&operators. While functional, the coding guidelines recommend ternary operators for conditional rendering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/activity-log-panel.tsx` around lines 705 - 711, The JSX uses && short-circuiting for conditional TabButton rendering (e.g., the isCallFilterActive check and weatherSettings?.WeatherAlertsEnabled) which the style guide wants as ternary expressions; replace those expressions so each conditional renders either the TabButton or null using a ternary (for example update the isCallFilterActive && <TabButton .../> and weatherSettings?.WeatherAlertsEnabled !== false && <TabButton .../> to isCallFilterActive ? <TabButton .../> : null and weatherSettings?.WeatherAlertsEnabled !== false ? <TabButton .../> : null) while keeping existing props (label, icon, isActive, count, onPress) and the activeTab state handling intact.src/components/checkIn/check-in-timer-card.tsx (2)
56-61: Address formatting warnings from static analysis.The static analysis tool flagged formatting issues with the
||operator placement across multiple lines. These can be auto-fixed with Prettier.Run
npx prettier --write src/components/checkIn/check-in-timer-card.tsxto fix the formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkIn/check-in-timer-card.tsx` around lines 56 - 61, The expression computing isTargetNameActuallyTypeName has line-break/formatting issues causing static-analysis warnings; reformat that conditional (referencing isTargetNameActuallyTypeName, timer.TargetName, timer.TargetTypeName, and CHECK_IN_TYPE_KEYS) so the logical ORs are placed per the project's Prettier rules — the simplest fix is to run the project's formatter on the file (npx prettier --write src/components/checkIn/check-in-timer-card.tsx) or manually adjust line breaks so each || placement matches the repo style.
115-124: Type coercion for percentage width may cause issues.The
width: \${progress}%` as unknown as number` cast is a workaround for TypeScript's strict typing of React Native styles. While it works at runtime, it obscures the actual type and could mask issues.Consider using a more explicit approach:
♻️ Suggested alternative
<View style={{ height: 4, width: `${progress}%`, backgroundColor: statusColor, borderRadius: 2, } as ViewStyle} />Or calculate width as a fraction of container width using
onLayout.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/checkIn/check-in-timer-card.tsx` around lines 115 - 124, The inline cast width: `${progress}%` as unknown as number is unsafe; change the inner bar's style so width is typed as a string or computed number instead of coercing to number. Replace the cast with width: `${progress}%` and assert the style as ViewStyle (e.g., style={ { height:4, width: `${progress}%`, backgroundColor: statusColor, borderRadius:2 } as ViewStyle }) or compute an explicit pixel width in onLayout and pass that numeric value to width; update the component rendering the progress bar (the inner View using progress and statusColor) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(app)/home.tsx:
- Line 48: The destructured value lastUpdateTimestamp from useSignalRStore in
home.tsx is unused; remove lastUpdateTimestamp from the destructuring assignment
so it only pulls lastPersonnelUpdateTimestamp, lastUnitsUpdateTimestamp,
lastCallsUpdateTimestamp, and lastEventType, and run a quick search to ensure no
remaining references to lastUpdateTimestamp remain in this file (update the
destructuring near useSignalRStore accordingly).
In `@src/stores/callVideoFeeds/store.ts`:
- Line 84: The current line uses "await get().fetchFeeds(callId).catch(() =>
{});" which triggers formatter/linter warnings; replace it with an explicit
try/catch around the await call: call "await get().fetchFeeds(callId)" inside a
try block and handle errors in the catch block (e.g., empty or logged) so the
await isn't chained with .catch; update the code around the
get().fetchFeeds(callId) call accordingly.
- Around line 75-85: The removeFeed function currently logs delete failures and
returns false but does not set the store's error state, so consumers never see
actionable error info; update removeFeed (which calls deleteCallVideoFeed and
fetchFeeds) to set a descriptive error in the store (e.g., set({ error: {
action: 'deleteFeed', feedId, cause: error } })) when deleteCallVideoFeed
throws, before attempting the fetchFeeds resync, and clear or update that error
appropriately on successful delete or after resync so consumers can react to
failures via the store's error field.
In `@src/stores/checkIn/store.ts`:
- Around line 171-173: When callIds is empty the code resets timerStatuses and
isLoadingStatuses but leaves any previous statusError intact; update the
empty-call branch in the function handling callIds to also clear statusError by
calling set with statusError: null (or '' depending on app convention) alongside
timerStatuses: [] and isLoadingStatuses: false so prior fetch errors are not
shown after selection is cleared (refer to set, timerStatuses,
isLoadingStatuses, statusError, and callIds in the same block).
- Around line 256-263: The early return in startPollingForCalls leaves the
store's _pollingInterval state set even after clearInterval is called, causing
state/UI to think polling is active; after clearing the existing interval
(get()._pollingInterval) ensure you also update the store to clear the flag
(e.g., set _pollingInterval to undefined/null) before returning when
callIds.length === 0, and keep updating _pollingCallIds appropriately so
startPollingForCalls correctly reflects no active interval.
- Around line 170-184: fetchTimerStatusesForCalls is not updating the
resolvedTimers state like fetchTimerStatuses does; ensure resolvedTimers stays
in sync by capturing the timers fetched for each call (the resolvedResults ->
allResolved computed from getTimersForCall calls) and include them in the state
update (e.g., set({... , resolvedTimers: allResolved})) when you set
timerStatuses (also set resolvedTimers to [] in the early return when
callIds.length === 0); update the set calls inside fetchTimerStatusesForCalls to
write resolvedTimers accordingly so it stays fresh after multi-call polling.
---
Duplicate comments:
In `@src/stores/callVideoFeeds/store.ts`:
- Around line 46-47: The addFeed and updateFeed save flows leave previous errors
set when starting a new save; update the start of both functions (addFeed and
updateFeed in src/stores/callVideoFeeds/store.ts) to clear stale error state by
setting error to null when flipping isSaving to true (e.g., set({ isSaving:
true, error: null })), so retries don't display old failure messages.
---
Nitpick comments:
In `@src/app/`(app)/home.tsx:
- Around line 709-710: The JSX currently uses && for conditional rendering:
replace the expression that renders CloseCallBottomSheet when selectedCallId is
truthy with a ternary expression (selectedCallId ? <CloseCallBottomSheet
isOpen={isCloseCallSheetOpen} onClose={() => setIsCloseCallSheetOpen(false)}
callId={selectedCallId} /> : null) so that rendering follows the project's
guideline; update the usage of selectedCallId, CloseCallBottomSheet,
isCloseCallSheetOpen and setIsCloseCallSheetOpen accordingly.
In `@src/app/weather-alerts/`[id].tsx:
- Around line 156-158: Replace the conditional renderings that use the &&
operator with ternary expressions so they explicitly render null when the value
is falsy; specifically update the three occurrences where DetailRow is rendered
for alert.OnsetUtc, alert.ExpiresUtc, and alert.SentUtc to use the ternary
operator (e.g., alert.OnsetUtc ? <DetailRow ... /> : null) while preserving the
same props (label from t('...') and isDark) and keeping DetailRow as the
component being rendered.
In `@src/components/checkIn/check-in-timer-card.tsx`:
- Around line 56-61: The expression computing isTargetNameActuallyTypeName has
line-break/formatting issues causing static-analysis warnings; reformat that
conditional (referencing isTargetNameActuallyTypeName, timer.TargetName,
timer.TargetTypeName, and CHECK_IN_TYPE_KEYS) so the logical ORs are placed per
the project's Prettier rules — the simplest fix is to run the project's
formatter on the file (npx prettier --write
src/components/checkIn/check-in-timer-card.tsx) or manually adjust line breaks
so each || placement matches the repo style.
- Around line 115-124: The inline cast width: `${progress}%` as unknown as
number is unsafe; change the inner bar's style so width is typed as a string or
computed number instead of coercing to number. Replace the cast with width:
`${progress}%` and assert the style as ViewStyle (e.g., style={ { height:4,
width: `${progress}%`, backgroundColor: statusColor, borderRadius:2 } as
ViewStyle }) or compute an explicit pixel width in onLayout and pass that
numeric value to width; update the component rendering the progress bar (the
inner View using progress and statusColor) accordingly.
In `@src/components/dispatch-console/activity-log-panel.tsx`:
- Line 627: The JSX uses the gluestack-ui Icon wrapper (Icon as={ChevronRight})
which violates the guideline—replace the wrapped usage with the
lucide-react-native component directly: render ChevronRight instead of Icon,
remove the Icon import from gluestack-ui, and import ChevronRight from
'lucide-react-native'; ensure you map/adjust props (size/color/className →
size/color or style) on the ChevronRight component in activity-log-panel
(reference Icon and ChevronRight symbols and the ActivityLogPanel component) and
remove any unused Icon imports.
- Around line 578-593: The conditional JSX uses logical && for rendering badges
(checks using urgentCheckInCount, criticalCheckInCount, overdueCheckInCount
inside the HStack/Box blocks) which violates the guideline; update each
conditional expression to use a ternary operator instead (e.g., replace
expressions like {urgentCheckInCount > 0 && (<HStack>...)} and the nested
{criticalCheckInCount > 0 && (<Box>...)} / {overdueCheckInCount > 0 &&
(<Box>...)} with the equivalent ternary forms that render the JSX when true and
null when false) so the component (activity-log-panel / ActivityLogPanel)
follows the ? : pattern for conditional rendering.
- Around line 705-711: The JSX uses && short-circuiting for conditional
TabButton rendering (e.g., the isCallFilterActive check and
weatherSettings?.WeatherAlertsEnabled) which the style guide wants as ternary
expressions; replace those expressions so each conditional renders either the
TabButton or null using a ternary (for example update the isCallFilterActive &&
<TabButton .../> and weatherSettings?.WeatherAlertsEnabled !== false &&
<TabButton .../> to isCallFilterActive ? <TabButton .../> : null and
weatherSettings?.WeatherAlertsEnabled !== false ? <TabButton .../> : null) while
keeping existing props (label, icon, isActive, count, onPress) and the activeTab
state handling intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50f94b1e-2962-4f61-860b-dd0006e52e73
📒 Files selected for processing (11)
src/app/(app)/home.tsxsrc/app/weather-alerts/[id].tsxsrc/components/calls/__tests__/call-files-modal.test.tsxsrc/components/checkIn/check-in-bottom-sheet.tsxsrc/components/checkIn/check-in-timer-card.tsxsrc/components/dispatch-console/active-calls-panel.tsxsrc/components/dispatch-console/activity-log-panel.tsxsrc/stores/callVideoFeeds/store.tssrc/stores/checkIn/store.tssrc/stores/signalr/signalr-store.tssrc/stores/weatherAlerts/store.ts
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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
1 participant
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 by CodeRabbit
New Features
Improvements