Conversation
📝 WalkthroughWalkthroughAdds comprehensive maps and routes features: new mapping and routing APIs, typed models, Zustand stores, many Expo Router screens/components, Mapbox source/layer implementations (raster/image), i18n expansions, Sentry/logging integrations, UI components, and a .gitignore entry for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as App UI
participant Store as Zustand Store
participant API as Backend API
participant Map as Mapbox
User->>UI: open maps or routes screen
UI->>Store: dispatch fetch (maps/routes)
Store->>API: GET mapping/routes endpoints (cached where applicable)
API-->>Store: return typed Data
Store-->>UI: update state (activeLayers, routeInstance, stops, geojson)
UI->>Map: add ShapeSource/RasterSource and Layers (GeoJSON/tiles)
Map-->>UI: render overlays (polylines, geofence, markers)
User->>UI: perform action (start/check-in/toggle layer)
UI->>Store: invoke action
Store->>API: POST action (start/check-in/acknowledge)
API-->>Store: result -> Store updates state -> UI updates Map/UI
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 19
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 (2)
src/components/settings/language-item.tsx (1)
23-35:⚠️ Potential issue | 🟡 MinorRecompute language labels when locale changes.
langsis memoized with[], so labels stay in the old language after switching locale during runtime.💡 Suggested fix
- 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 = 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' }, + ], + [language] + );🤖 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 created in the React.useMemo block (langs, translate) is using an empty dependency array so labels won't update when the app locale changes; update the memo dependencies to include the current locale/source of translations (e.g., pass the i18n locale or translation function reference) so translate(...) runs again on locale change, or remove useMemo if unnecessary—specifically modify the React.useMemo call that builds langs to depend on the locale (or translate) to ensure labels re-render in the new language.src/app/login/login-form.tsx (1)
1-21:⚠️ Potential issue | 🟡 MinorFix import sorting and merge duplicated
lucide-react-nativeimports.This section currently fails
simple-import-sort/imports, andChevronDownIconis imported from the same package in a separate statement.💡 Suggested fix
-import { AlertTriangle, EyeIcon, EyeOffIcon, Globe, LogIn, ShieldCheck } from 'lucide-react-native'; +import { AlertTriangle, ChevronDownIcon, EyeIcon, EyeOffIcon, Globe, LogIn, ShieldCheck } from 'lucide-react-native'; ... -import { ChevronDownIcon } from 'lucide-react-native';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/login/login-form.tsx` around lines 1 - 21, Combine the two separate lucide-react-native imports into a single import that lists AlertTriangle, EyeIcon, EyeOffIcon, Globe, LogIn, ShieldCheck, and ChevronDownIcon, and then reorder imports to satisfy simple-import-sort (group external packages first, then internal aliases) so the import block in login-form.tsx is de-duplicated and properly sorted; update the existing import statement that currently declares AlertTriangle, EyeIcon, EyeOffIcon, Globe, LogIn, ShieldCheck to also include ChevronDownIcon and ensure overall import ordering follows the project's import-sorting rules.
🟡 Minor comments (16)
src/app/routes/stop/contact.tsx-95-99 (1)
95-99:⚠️ Potential issue | 🟡 MinorDon't leave the screen in a permanent loading state when
stopIdis missing.This early return keeps
isLoadingattrue, so a bad deep link or missing param renders the spinner forever. Set an error/empty state before exiting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/stop/contact.tsx` around lines 95 - 99, The early return inside the useEffect when stopId is falsy leaves the component stuck in loading; update the block so that when !stopId you call setIsLoading(false) and setError(...) (or set a suitable empty state) before returning. Locate the useEffect that references stopId, cancelled, setIsLoading and setError in contact.tsx and add the two state updates (setIsLoading(false) and setError with a descriptive message or null->empty-state marker) immediately prior to the return to ensure the spinner is cleared and an error/empty UI can render.src/app/routes/history/[planId].tsx-1-21 (1)
1-21:⚠️ Potential issue | 🟡 MinorRun the import sorter here.
CI is already failing on this block with
simple-import-sort/imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/history/`[planId].tsx around lines 1 - 21, The import block at the top of src/app/routes/history/[planId].tsx is failing simple-import-sort; reorder the imports (or run the import sorter/ESLint --fix) to satisfy simple-import-sort rules: group and sort external packages (e.g., useFocusEffect, useLocalSearchParams, Clock, MapPin, Navigation), then React and hooks (React, useCallback, useTranslation), then react-native imports (Pressable, RefreshControl, View), then internal app aliases (Loading, ZeroState, Badge, Box, FlatList, HStack, Icon, Text, VStack, RouteInstanceResultData, useRoutesStore), ensuring each group is alphabetized and separated as required so eslint simple-import-sort/imports passes.src/app/routes/stop/contact.tsx-1-29 (1)
1-29:⚠️ Potential issue | 🟡 MinorRun the import sorter here too.
ESLint is failing on this import block (
simple-import-sort/imports).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/stop/contact.tsx` around lines 1 - 29, The import block at the top of contact.tsx violates simple-import-sort/imports; run the import sorter or ESLint --fix to reorder imports into canonical groups (builtin, external like 'react', 'react-native', 'expo-router', 'lucide-react-native', then internal aliases like '@/components/...' and '@/api/...'), alphabetize within each group and ensure single blank lines between groups so the imports that include symbols like Stack, useLocalSearchParams, BuildingIcon, Camera, MapView, getStopContact, and ContactResultData are correctly grouped and sorted to satisfy the rule.src/app/routes/history/[planId].tsx-31-37 (1)
31-37:⚠️ Potential issue | 🟡 MinorLocalize the status labels and stop count.
Lines 31-37: The
STATUS_LABELSobject uses hardcoded English strings. Create translation keys for each status (e.g.,routes.status.pending,routes.status.active, etc.) or move the labels into the component to uset().Line 137: Replace the hardcoded
"stops"suffix witht('routes.stop_count', { count: item.CurrentStopIndex ?? 0 })— the translation key already exists in all language dictionaries.Per coding guidelines, all user-facing text must be wrapped in
t()fromreact-i18next.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/history/`[planId].tsx around lines 31 - 37, STATUS_LABELS currently contains hardcoded English strings; replace these with translated values by either moving the label mapping into the component where the i18n hook is available or by turning STATUS_LABELS into a function that accepts t and returns { [RouteInstanceStatus.Pending]: t('routes.status.pending'), ... } so each status uses translation keys like routes.status.pending/active/paused/completed/cancelled; also change the hardcoded "stops" suffix usage (around the CurrentStopIndex display) to use t('routes.stop_count', { count: item.CurrentStopIndex ?? 0 }) so pluralization/localization is applied consistently.src/app/routes/stop/contact.tsx-217-222 (1)
217-222:⚠️ Potential issue | 🟡 MinorWrap phone labels and map legend text in
t()for translation support.The following hardcoded English strings prevent the screen from translating fully in new locales:
Lines 217-222:
"Phone","Mobile","Home","Cell","Office","Fax"(PhoneRow labels)
Lines 296-308:"Location","Entrance","Exit"(map legend)All user-facing text must be wrapped in
t()fromreact-i18nextper the coding guidelines. The file already demonstrates the correct pattern witht('routes.contact'),t('routes.address'), etc., suggesting these labels were missed during implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/stop/contact.tsx` around lines 217 - 222, Wrap all hardcoded phone label and map legend strings in the i18n translator function t(): replace literal labels passed to PhoneRow (e.g., "Phone", "Mobile", "Home", "Cell", "Office", "Fax") with t('routes.phone.<key>') or similar keys and also wrap the map legend strings ("Location", "Entrance", "Exit") with t() in the map/legend rendering; use the same translation namespace pattern already used in this file (see existing t('routes.contact') and t('routes.address')) and update translation keys accordingly so all user-facing labels are internationalized.src/translations/en.json-705-705 (1)
705-705:⚠️ Potential issue | 🟡 MinorConfigure i18next pluralization for count-based strings.
"{{count}} floors"and"{{count}} stops"are grammatically incorrect forcount: 1(produces "1 floors"), and this approach cannot scale to languages with complex plural rules like Polish, Russian, or Ukrainian, which require multiple plural forms (_one,_other,_few, etc.).Update
src/lib/i18n/index.tsxto enable pluralization with a plural suffix configuration, then refactor both strings to use i18next plural keys (e.g.,floor_count_one,floor_count_other) in all 9 locale files. The component code callingt('routes.stop_count', { count: ... })will work unchanged once i18next is configured to handle the suffixes automatically.Also applies to: 772-772
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/en.json` at line 705, The current singular/plural strings use a single key ("floor_count", "stop_count") which yields incorrect text for count=1 and fails for languages with complex plural rules; update src/lib/i18n/index.tsx to enable i18next plural suffix handling (set pluralSeparator to "_" in the i18next init options and ensure pluralization is enabled), then refactor the translation keys in all 9 locale JSON files to provide plural variants (e.g., "floor_count_one", "floor_count_other", and similarly for "stop_count" using the appropriate plural forms per locale); no component changes are required because calls like t('routes.stop_count', { count }) will automatically resolve the correct plural form once i18next is configured and the new keys are present.src/app/routes/directions.tsx-1-32 (1)
1-32:⚠️ Potential issue | 🟡 MinorFix import sorting to pass CI.
The ESLint/CI check is failing due to unsorted imports. Run the autofix command to resolve this.
npx eslint --fix src/app/routes/directions.tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/directions.tsx` around lines 1 - 32, The import block in directions.tsx is unsorted (imports like useLocalSearchParams, lucide icons, nativewind, React hooks, i18n, React Native, map components Camera/MapView/PointAnnotation/ShapeSource/etc., Loading, UI components Box/Button/Heading/HStack/Text/VStack, useRoutesStore, RouteStopStatus are out of order); run the project's auto-fixer (npx eslint --fix src/app/routes/directions.tsx) or manually reorder imports to match the repository's import-sort/ESLint rules (group external packages, then local aliases, and keep named imports alphabetized) so the file passes CI.src/translations/sv.json-525-527 (1)
525-527:⚠️ Potential issue | 🟡 MinorPlaceholder text "obytes app site" should be updated.
Similar to the French translation, this contains placeholder text that should be replaced with the actual app name.
✏️ Suggested fix
"onboarding": { - "message": "Välkommen till obytes app site" + "message": "Välkommen till Resgrid Unit" },Also update line 803:
-"welcome": "Välkommen till obytes app site" +"welcome": "Välkommen till Resgrid Unit"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/sv.json` around lines 525 - 527, Replace the placeholder text "obytes app site" in the Swedish translations with the actual app name wherever it appears (for example the onboarding.message key in src/translations/sv.json) and also update the other occurrence noted in the file (the second placeholder instance near the end of the file) so both entries use the correct app name.src/app/maps/custom/[id].tsx-88-92 (1)
88-92:⚠️ Potential issue | 🟡 MinorPotential undefined multiplication with
layer.Opacity.If
layer.Opacityis undefined or null, the expressionlayer.Opacity * 0.3will produceNaN. Add a fallback default.🛡️ Proposed fix
<Mapbox.FillLayer id={`fill-${layer.CustomMapLayerId}`} style={{ fillColor: layer.Color || '#3B82F6', - fillOpacity: layer.Opacity * 0.3, + fillOpacity: (layer.Opacity ?? 1) * 0.3, }} />Apply similar fix to lines 98 and 134, 154.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/maps/custom/`[id].tsx around lines 88 - 92, The style object uses layer.Opacity in arithmetic which can be undefined and yield NaN; update the calculations to use a safe numeric default such as (Number(layer.Opacity ?? 1) * 0.3) for the fillOpacity (and analogous places) so undefined/null becomes 1 (or another sane default) before multiplying; apply the same pattern to the other style usages referenced (the occurrences near the current fillOpacity, and the similar expressions at the locations you noted around lines 98, 134, and 154) to ensure all opacity multiplications use Number(layer.Opacity ?? <default>) instead of raw layer.Opacity.src/translations/fr.json-525-527 (1)
525-527:⚠️ Potential issue | 🟡 MinorPlaceholder text "obytes app" should be updated.
The onboarding message contains "obytes app" which appears to be template/placeholder text rather than the actual app name "Resgrid Unit".
✏️ Suggested fix
"onboarding": { - "message": "Bienvenue sur l'application obytes" + "message": "Bienvenue sur Resgrid Unit" },Also update line 803:
-"welcome": "Bienvenue sur l'application obytes" +"welcome": "Bienvenue sur Resgrid Unit"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/fr.json` around lines 525 - 527, Update the placeholder app name in the French translations by replacing the "onboarding.message" value that currently reads "Bienvenue sur l'application obytes" with the correct app name "Resgrid Unit"; also locate the other occurrence mentioned around the second instance (the key referenced near line 803) and replace any "obytes" occurrences with "Resgrid Unit" to ensure consistency across the fr.json translation keys.src/app/maps/index.tsx-170-170 (1)
170-170:⚠️ Potential issue | 🟡 Minor
RefreshControlalways showsrefreshing={false}regardless of actual loading state.The
refreshingprop is hardcoded tofalse, so users won't see the refresh indicator while data is loading. Use the actual loading state.🐛 Proposed fix
-refreshControl={<RefreshControl refreshing={false} onRefresh={handleRefresh} />} +refreshControl={<RefreshControl refreshing={isLoading} onRefresh={handleRefresh} />}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/maps/index.tsx` at line 170, The RefreshControl currently hardcodes refreshing={false}; change it to use the component's real loading state (e.g., refreshing={isRefreshing} or refreshing={isLoading}) so the spinner reflects actual fetch status, and ensure the state variable is set to true at the start of handleRefresh and set back to false when data loading completes (or use your hook's isFetching value if present) so RefreshControl and handleRefresh stay in sync.src/app/routes/stop/[id].tsx-117-120 (1)
117-120:⚠️ Potential issue | 🟡 MinorZero-valued coordinates are treated as missing.
Both guards rely on truthiness, so a stop at latitude/longitude
0loses its geofence and mini-map. Prefer!= nulland finite-number checks here.Also applies to: 287-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/stop/`[id].tsx around lines 117 - 120, The current truthy checks in the geofenceGeoJson useMemo drop zero coordinates (e.g., latitude/longitude = 0); update the guard to explicitly test for null/undefined and numeric finiteness (e.g., stop?.Latitude != null && stop?.Longitude != null && Number.isFinite(stop.Latitude) && Number.isFinite(stop.Longitude) && stop?.GeofenceRadiusMeters != null && Number.isFinite(stop.GeofenceRadiusMeters)) before calling buildGeofenceGeoJson so zero values are preserved; apply the same null/finite checks to any other guards in this file that validate stop.Latitude/stop.Longitude/stop.GeofenceRadiusMeters.src/app/routes/start.tsx-95-106 (1)
95-106:⚠️ Potential issue | 🟡 MinorUse null/finite checks for coordinates here.
filter((s) => s.Latitude && s.Longitude)drops valid0values, so routes on the equator/prime meridian never make it intomapData.🗺️ Suggested guard
- const valid = sortedStops.filter((s) => s.Latitude && s.Longitude); + const valid = sortedStops.filter( + (s) => + s.Latitude != null && + s.Longitude != null && + Number.isFinite(s.Latitude) && + Number.isFinite(s.Longitude) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/start.tsx` around lines 95 - 106, The filter currently used in the mapData useMemo drops valid zero coordinates; update the predicate to explicitly test for finite numeric coordinates (e.g., use Number.isFinite(...) on parsed numeric values) when filtering sortedStops so Latitude and Longitude of 0 are preserved; then use those numeric values for lats/lngs and subsequent calculations in mapData (keep function name mapData and variable sortedStops/Latitude/Longitude to locate the change).src/models/v4/mapping/mappingResults.ts-1-5 (1)
1-5:⚠️ Potential issue | 🟡 MinorFix the import order here before merge.
simple-import-sort/importsis already flagging this block, so lint stays red until it is autofixed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/mapping/mappingResults.ts` around lines 1 - 5, The import block is not sorted per simple-import-sort rules; reorder the imports so external packages come first then internal modules alphabetically: ensure "FeatureCollection" import from 'geojson' appears before the relative imports, and sort the relative imports alphabetically by module path (so BaseV4Request, then customMapResultData, then indoorMapResultData), keeping the exact named symbols FeatureCollection, BaseV4Request, CustomMapResultData, IndoorMapResultData, and IndoorMapFloorResultData unchanged.src/api/routes/routes.ts-1-32 (1)
1-32:⚠️ Potential issue | 🟡 MinorSort this import block; CI is already failing on it.
The pipeline failure is
simple-import-sort/imports, so this file will keep blocking green builds until the imports are autofixed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/routes/routes.ts` around lines 1 - 32, The import block in routes.ts is unsorted and failing the simple-import-sort rule; reorder the imports alphabetically by module/source (grouping external packages vs internal paths) so lint passes—specifically sort the imports that include cacheManager, the various type imports from '@/models/v4/contacts/*' and '@/models/v4/routes/*', and the local imports createCachedApiEndpoint, api, and createApiEndpoint; keep related grouped type imports together and ensure the final order is consistent with the project's import-sort config.src/translations/es.json-705-705 (1)
705-705:⚠️ Potential issue | 🟡 MinorAdd singular/plural variants for the count-based labels.
With the current strings the UI will read
1 pisosand1 paradas. Split these into singular/plural forms so Spanish stays natural.Proposed fix
- "floor_count": "{{count}} pisos", + "floor_count_one": "{{count}} piso", + "floor_count_other": "{{count}} pisos", @@ - "stop_count": "{{count}} paradas", + "stop_count_one": "{{count}} parada", + "stop_count_other": "{{count}} paradas",Also applies to: 769-769
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/es.json` at line 705, The Spanish translation for count-based labels uses a single form (e.g., "floor_count": "{{count}} pisos") which results in incorrect output like "1 pisos"; update the keys to provide singular/plural variants using your project's pluralization convention (for example switch "floor_count" to a pluralized string that returns "1 piso" for one and "{{count}} pisos" for other counts), and apply the same change to the other affected key mentioned (e.g., the stop_count key at the indicated location). Ensure you use the existing i18n ICU/plural syntax or the repo's established pattern so the app picks the correct form at runtime.
🧹 Nitpick comments (28)
src/stores/auth/store.tsx (1)
50-51: Extract Sentry user sync into one helper to avoid drift.These call sites duplicate the same side effect. A single helper keeps payload shape and clearing behavior consistent.
Proposed refactor
+const syncSentryUser = (profile: Pick<ProfileModel, 'sub' | 'name'> | null): void => { + if (!profile) { + Sentry.setUser(null); + return; + } + Sentry.setUser({ id: profile.sub, username: profile.name }); +}; @@ - Sentry.setUser({ id: profileData.sub, username: profileData.name }); + syncSentryUser(profileData); @@ - Sentry.setUser({ id: profileData.sub, username: profileData.name }); + syncSentryUser(profileData); @@ - Sentry.setUser(null); + syncSentryUser(null); @@ - Sentry.setUser({ id: profileData.sub, username: profileData.name }); + syncSentryUser(profileData);Also applies to: 115-116, 155-155, 241-242
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/auth/store.tsx` around lines 50 - 51, Multiple call sites invoke Sentry.setUser with the same profile payload; extract this into a single helper (e.g., setSentryUserFromProfile(profileData) and a companion clearSentryUser()) and replace all direct Sentry.setUser calls with the helper to ensure consistent payload shape and clearing behavior; update places that currently call Sentry.setUser({ id: profileData.sub, username: profileData.name }) (and where the user is cleared) to call the new helper or clearSentryUser() respectively, and ensure the helper handles null/undefined profiles by calling Sentry.setUser(null) to clear the user.src/models/v4/routes/routeInstanceResultData.ts (2)
1-1: Rename this file to lowercase-hyphenated format.
routeInstanceResultData.tsdoes not match the repository filename convention.As per coding guidelines:
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/routes/routeInstanceResultData.ts` at line 1, Rename the file containing the exported interface RouteInstanceResultData to follow the lowercase-hyphenated convention (e.g., route-instance-result-data.ts); update any imports referencing RouteInstanceResultData throughout the codebase to the new filename to avoid broken imports, ensuring the exported symbol name (RouteInstanceResultData) remains unchanged.
5-5: TypeStatusasRouteInstanceStatusinstead ofnumber.Line 5 uses a raw number despite the
RouteInstanceStatusenum being available in the same file (lines 33–39). Using the enum provides compile-time type safety and makes the code more maintainable.♻️ Proposed fix
export interface RouteInstanceResultData { RouteInstanceId: string; RoutePlanId: string; RoutePlanName?: string | null; - Status: number; + Status: RouteInstanceStatus;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/routes/routeInstanceResultData.ts` at line 5, Change the Status property type from a raw number to the RouteInstanceStatus enum to get compile-time safety: locate the RouteInstanceResultData type (where "Status: number;" is declared) and replace the numeric type with RouteInstanceStatus, ensuring the file references the existing enum RouteInstanceStatus (declared in the same file) so no additional imports are needed; update any callers or tests that construct RouteInstanceResultData to use the enum values instead of plain numbers.src/models/v4/routes/routeInstanceStopResultData.ts (2)
1-1: Rename this file to lowercase-hyphenated format for repo consistency.The current filename
routeInstanceStopResultData.tsdoes not follow the project naming convention.As per coding guidelines:
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/routes/routeInstanceStopResultData.ts` at line 1, Rename the file from routeInstanceStopResultData.ts to the lowercase-hyphenated convention (e.g., route-instance-stop-result-data.ts) and update any imports referencing RouteInstanceStopResultData to point to the new filename; ensure the exported interface name RouteInstanceStopResultData remains unchanged and run a repo-wide search to update all import paths accordingly.
5-5: UseRouteStopStatusfor theStatusfield instead of rawnumber.The enum
RouteStopStatusis defined in this file (lines 25–30) but theStatusfield at line 5 usesnumber, which permits invalid status values. This pattern also appears inrouteInstanceResultData.tswith theRouteInstanceStatusenum.♻️ Proposed fix
export interface RouteInstanceStopResultData { RouteInstanceStopId: string; RouteInstanceId: string; RouteStopId: string; - Status: number; + Status: RouteStopStatus;As per coding guidelines: Write concise, type-safe TypeScript code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/routes/routeInstanceStopResultData.ts` at line 5, Replace the loose numeric type on the Status field with the enum type to enforce valid statuses: change the Status declaration from number to RouteStopStatus in routeInstanceStopResultData (the same pattern should be fixed in routeInstanceResultData for RouteInstanceStatus); ensure the file references the RouteStopStatus enum (or imports it if moved) and update any code that constructs instances to use the enum values rather than raw numbers.src/models/v4/routes/routePlanResultData.ts (1)
1-1: Rename this file to align with lowercase-hyphenated naming.
routePlanResultData.tscurrently breaks the repo file naming rule.As per coding guidelines:
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/routes/routePlanResultData.ts` at line 1, The file name routePlanResultData.ts violates the project's lowercase-hyphenated rule; rename the file to route-plan-result-data.ts and update all imports/usages of the RoutePlanResultData interface across the codebase (search for RoutePlanResultData and any import paths referencing routePlanResultData) so they point to the new lowercase-hyphenated filename.src/components/maps/map-view.web.tsx (1)
632-634: ReplaceReact.FC<any>with explicit placeholder prop types.Lines 632–634 use
anyin component definitions; define explicit prop types even for no-op compatibility wrappers.♻️ Proposed refactoring
+interface LayerPlaceholderProps { + children?: React.ReactNode; +} -export const RasterLayer: React.FC<any> = () => null; -export const RasterSource: React.FC<any> = ({ children }) => <>{children}</>; -export const ImageSource: React.FC<any> = ({ children }) => <>{children}</>; +export const RasterLayer: React.FC<LayerPlaceholderProps> = () => null; +export const RasterSource: React.FC<LayerPlaceholderProps> = ({ children }) => <>{children}</>; +export const ImageSource: React.FC<LayerPlaceholderProps> = ({ children }) => <>{children}</>;This aligns with the guideline: avoid
anyand strive for precise types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/maps/map-view.web.tsx` around lines 632 - 634, Define explicit prop types instead of using React.FC<any>: create small interfaces like RasterLayerProps (even empty or with relevant optional fields such as id, source, layout), RasterSourceProps and ImageSourceProps that include children?: React.ReactNode for the source wrappers, then change the component signatures to React.FC<RasterLayerProps>, React.FC<RasterSourceProps>, and React.FC<ImageSourceProps> respectively (and update any places that rely on those props). This removes the use of any while preserving the no-op compatibility wrappers RasterLayer, RasterSource, and ImageSource.src/app/(app)/_layout.tsx (1)
335-335: Use the Lucide component directly for the new routes tab icon.This adds another new
<Icon as={Navigation}>usage, but the repo standard for new TSX code is to render Lucide icons directly in markup.Based on learnings "Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not 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/app/`(app)/_layout.tsx at line 335, Replace the Icon wrapper usage in the routesIcon callback by rendering the Lucide icon component directly: remove the Icon import/usage and return the Navigation component itself with the same props (stroke={color} and className="text-primary-500 dark:text-primary-400") inside the useCallback; update the routesIcon declaration to reference Navigation directly and remove reliance on Icon so it follows the repo standard of using lucide icons in TSX.src/app/routes/directions.tsx (1)
69-69: Avoid usinganyfor the camera ref.The
cameraRefis typed asany, which violates the coding guidelines. Consider using a more precise type from@rnmapbox/maps.♻️ Suggested improvement
- const cameraRef = useRef<any>(null); + const cameraRef = useRef<Camera>(null);You may need to import the
Cameratype or useReact.ElementRef<typeof Camera>if the library exports a proper type.As per coding guidelines: "Avoid using
any; strive for precise types."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/directions.tsx` at line 69, cameraRef is currently typed as any; replace that with a precise Camera ref type from `@rnmapbox/maps` (or use React.ElementRef<typeof Camera> if the library exports a Camera component). Import the Camera type or reference the Camera component and change the declaration of cameraRef (const cameraRef = useRef<any>(null)) to use the proper type so TypeScript can enforce correct ref methods and props.src/app/maps/search.tsx (1)
174-179: Consider adding FlatList performance optimizations.For potentially large search result sets, the FlatList could benefit from performance props.
♻️ Suggested improvement
return ( <FlatList data={searchResults} renderItem={renderResultItem} keyExtractor={(item) => `${item.Type}-${item.Id}`} contentContainerStyle={{ paddingBottom: 20 }} + 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/maps/search.tsx` around lines 174 - 179, The FlatList rendering searchResults lacks performance tuning for large lists; update the FlatList component (the instance using props data={searchResults}, renderItem={renderResultItem}, keyExtractor) to include recommended performance props such as removeClippedSubviews={true}, maxToRenderPerBatch={10} (or a tuned value), initialNumToRender={10}, and windowSize={21} (adjust values as appropriate), and ensure renderResultItem is stable (memoize it or wrap with useCallback) to avoid unnecessary re-renders.src/components/routes/route-card.tsx (1)
28-38: Move helper functions outside the component.
formatDistanceandformatDurationare recreated on every render. Since they don't depend on component state or props, extract them to module scope for better performance.♻️ Suggested improvement
+const formatDistance = (meters: number) => { + if (meters >= 1000) return `${(meters / 1000).toFixed(1)} km`; + return `${Math.round(meters)} m`; +}; + +const formatDuration = (seconds: number) => { + const hours = Math.floor(seconds / 3600); + const minutes = Math.floor((seconds % 3600) / 60); + if (hours > 0) return `${hours}h ${minutes}m`; + return `${minutes}m`; +}; + export const RouteCard: React.FC<RouteCardProps> = ({ route, isActive = false, unitName, isMyUnit = false, }) => { const { t } = useTranslation(); - - const formatDistance = (meters: number) => { - if (meters >= 1000) return `${(meters / 1000).toFixed(1)} km`; - return `${Math.round(meters)} m`; - }; - - const formatDuration = (seconds: number) => { - const hours = Math.floor(seconds / 3600); - const minutes = Math.floor((seconds % 3600) / 60); - if (hours > 0) return `${hours}h ${minutes}m`; - return `${minutes}m`; - };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/route-card.tsx` around lines 28 - 38, Extract the pure helper functions formatDistance and formatDuration out of the component and declare them at module scope (above the component) so they are not recreated on every render; move the existing implementations for formatDistance and formatDuration from inside the component to the top-level of the module, keep their signatures the same, and if they need to be used elsewhere consider exporting them (or leave as internal top-level helpers) and update any references inside the component to call the top-level functions.src/app/(app)/routes.tsx (3)
41-49: Potential infinite loop or redundant fetches due tounits.lengthdependency.Including
units.lengthin the dependency array may cause the effect to re-run afterfetchUnits()populates the units array. Consider removingunits.lengthfrom deps and conditionally fetching inside the effect, or use a ref to track if units were already fetched.♻️ Suggested fix
useEffect(() => { fetchAllRoutePlans(); if (activeUnitId) { fetchActiveRoute(activeUnitId); } - if (units.length === 0) { - fetchUnits(); - } - }, [activeUnitId, fetchAllRoutePlans, fetchActiveRoute, fetchUnits, units.length]); + // Fetch units only once; fetchUnits should be idempotent in the store + fetchUnits(); + }, [activeUnitId, fetchAllRoutePlans, fetchActiveRoute, fetchUnits]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/routes.tsx around lines 41 - 49, The effect currently depends on units.length which triggers a rerun after fetchUnits populates state; remove units.length from the dependency array and instead run fetchUnits conditionally inside the effect using a guard/ref to ensure it only runs once (e.g., create a unitsFetchedRef and check if !unitsFetchedRef.current && units.length === 0 then call fetchUnits() and set unitsFetchedRef.current = true). Keep the other dependencies (activeUnitId, fetchAllRoutePlans, fetchActiveRoute, fetchUnits) and keep the existing conditional call to fetchActiveRoute(activeUnitId).
64-67: Type assertionas anyon router.push suggests route typing issue.Using
as anybypasses TypeScript's route type checking. Consider defining typed routes or using Expo Router's typed routes feature to avoid runtime navigation errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/routes.tsx around lines 64 - 67, The code uses router.push(url as any) and router.push(`/routes/start?planId=${route.RoutePlanId}` as any) which bypasses TS checks; remove the as any casts and pass properly typed arguments to router.push (use router.push({ pathname: '/routes/start', params: { planId: route.RoutePlanId } }) or construct a typed string variable) and/or add a typed route definition for these routes so router.push accepts the correct type; locate usages of router.push and the Route object (route.RoutePlanId) and change to the typed call pattern or update the route type declarations to eliminate the need for as any.
132-148: Anonymous functions inrenderItemandkeyExtractormay cause unnecessary re-renders.Per coding guidelines, avoid anonymous functions in
renderItemor event handlers to prevent re-renders. Extract these to stable callbacks usinguseCallback.♻️ Suggested refactor
+const renderRouteItem = useCallback( + ({ item }: { item: RoutePlanResultData }) => { + const isMyUnit = item.UnitId != null && String(item.UnitId) === String(activeUnitId); + const unitName = item.UnitId != null + ? (unitMap[item.UnitId] || (isMyUnit ? (activeUnit?.Name ?? '') : '')) + : ''; + return ( + <Pressable onPress={() => handleRoutePress(item)}> + <RouteCard + route={item} + isActive={!!activeInstance && activeInstance.RoutePlanId === item.RoutePlanId} + unitName={unitName || undefined} + isMyUnit={isMyUnit} + /> + </Pressable> + ); + }, + [activeUnitId, unitMap, activeUnit, activeInstance, handleRoutePress] +); + +const keyExtractor = useCallback( + (item: RoutePlanResultData) => item.RoutePlanId, + [] +); // Then in FlatList: -renderItem={({ item }: { item: RoutePlanResultData }) => { ... }} -keyExtractor={(item: RoutePlanResultData) => item.RoutePlanId} +renderItem={renderRouteItem} +keyExtractor={keyExtractor}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/routes.tsx around lines 132 - 148, Extract the anonymous renderItem and keyExtractor into stable, memoized callbacks (e.g., create a useCallback named renderRouteItem and another named routeKeyExtractor) and pass those to the FlatList instead of inline functions; compute isMyUnit and unitName inside renderRouteItem, call the existing handleRoutePress from a stable onPress handler (no inline arrow created each render), and ensure the useCallback dependency arrays include activeUnitId, unitMap, activeInstance, activeUnit, and handleRoutePress so the callbacks update only when needed.src/components/routes/stop-card.tsx (2)
20-20: UnusedonPressprop in component interface.The
onPressprop is defined inStopCardPropsbut is never used in the component implementation. Either implement the functionality or remove the prop to avoid confusion.♻️ Option: Remove unused prop
interface StopCardProps { stop: RouteInstanceStopResultData; isCurrent?: boolean; onCheckIn?: () => void; onCheckOut?: () => void; onSkip?: () => void; - onPress?: () => void; } -export const StopCard: React.FC<StopCardProps> = ({ stop, isCurrent = false, onCheckIn, onCheckOut, onSkip, onPress }) => { +export const StopCard: React.FC<StopCardProps> = ({ stop, isCurrent = false, onCheckIn, onCheckOut, onSkip }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/stop-card.tsx` at line 20, The StopCardProps interface declares an onPress prop that is never used by the StopCard component; remove the unused onPress from StopCardProps (and from any places that pass it) or wire it into the component’s root interactive element (e.g., pass onPress to the TouchableOpacity/div that renders the card) so StopCard actually calls the prop; search for StopCardProps, onPress and the StopCard component to locate and update the definitions and any callers accordingly.
41-41: Use lucide-react-native icons directly.Per coding guidelines, use lucide icons directly rather than wrapping them with the gluestack-ui
Iconcomponent.As per coding guidelines: "Use
lucide-react-nativefor icons and use those components directly in the markup; don't use the gluestack-ui icon component."Also applies to: 54-54, 63-63, 70-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/stop-card.tsx` at line 41, Replace usages of the gluestack-ui Icon wrapper with the lucide-react-native icon components directly: import or reference the component stored in config.icon (e.g., IconComponent = config.icon) and render it directly with the appropriate props (size, color/style) instead of using <Icon as={config.icon} ...>; update all instances in this file (the occurrences at the current line and also the ones mentioned at lines 54, 63, 70) and remove the gluestack-ui Icon import if no longer used.src/app/maps/index.tsx (1)
73-73: Use lucide-react-native icons directly instead of gluestackIconwrapper.Per coding guidelines, lucide icons should be used directly without the gluestack-ui Icon component wrapper.
As per coding guidelines: "Use
lucide-react-nativefor icons and use those components directly in the markup; don't use the gluestack-ui icon component."Also applies to: 96-96, 195-195
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/maps/index.tsx` at line 73, The markup is using the gluestack-ui Icon wrapper (e.g., <Icon as={Map} size="md" className="text-blue-600 dark:text-blue-300" />); replace those with the lucide-react-native icon components directly (import { Map } from 'lucide-react-native') and render <Map /> instead of the Icon wrapper, mapping props appropriately (use the icon's size and color props or inline style instead of className, and remove the wrapper-specific props). Update every similar occurrence (the other Icon-as usages flagged) to import and use the corresponding lucide-react-native component directly and adjust styling props accordingly.src/app/(app)/index.tsx (2)
549-561: Anonymous function in map iteration may cause re-renders.The
remainingStops.map((stop) => ...)creates new function instances on each render. Consider extracting the stop marker rendering to a memoized component or callback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/index.tsx around lines 549 - 561, The inline anonymous function passed to remainingStops.map causes new function instances each render; extract the mapping/render logic into a separate memoized component (e.g., RouteStopAnnotations) or memoize the item renderer with React.useCallback so Mapbox.PointAnnotation and StopMarker are created from a stable function. Implement a component that accepts remainingStops and showRouteOverlay props, renders Mapbox.PointAnnotation (using stop.RouteInstanceStopId as key/id) with StopMarker inside, and wrap it with React.memo (or memoize the renderer with useCallback) to prevent unnecessary re-renders.
142-159: Geofence circle calculation has latitude-dependent accuracy issues.The constant
111320meters per degree is only accurate at the equator. Longitude degree distance varies with latitude (111320 * cos(latitude)). For stops at mid-to-high latitudes, the geofence circle will appear as an ellipse rather than a circle.♻️ More accurate calculation
const geofenceGeoJSON = useMemo((): GeoJSON.Feature<GeoJSON.Polygon> | null => { if (!nextStop || !nextStop.GeofenceRadiusMeters) return null; const points = 64; const coords: number[][] = []; - const radiusDeg = nextStop.GeofenceRadiusMeters / 111320; + const metersPerDegreeLat = 111320; + const metersPerDegreeLng = 111320 * Math.cos((nextStop.Latitude * Math.PI) / 180); + const radiusLatDeg = nextStop.GeofenceRadiusMeters / metersPerDegreeLat; + const radiusLngDeg = nextStop.GeofenceRadiusMeters / metersPerDegreeLng; for (let i = 0; i <= points; i++) { const angle = (i / points) * 2 * Math.PI; coords.push([ - nextStop.Longitude + radiusDeg * Math.cos(angle), - nextStop.Latitude + radiusDeg * Math.sin(angle), + nextStop.Longitude + radiusLngDeg * Math.cos(angle), + nextStop.Latitude + radiusLatDeg * Math.sin(angle), ]); } // ... }, [nextStop]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/index.tsx around lines 142 - 159, The geofence calculation in the geofenceGeoJSON useMemo currently uses a fixed meters-per-degree (111320) for both lat and lon which distorts circles at non‑equatorial latitudes; update geofenceGeoJSON to compute separate degree offsets: latitude degrees = GeofenceRadiusMeters / 111320 and longitude degrees = GeofenceRadiusMeters / (111320 * Math.cos(nextStop.Latitude in radians)), then use those two different offsets when building coords (use latitude offset for the y/sin component and longitude offset for the x/cos component) so the polygon represents a true circle on the map; reference the geofenceGeoJSON hook and nextStop.GeofenceRadiusMeters/nextStop.Latitude when making this change.src/app/routes/index.tsx (3)
22-174: Significant code duplication withsrc/app/(app)/routes.tsx.This file (
RouteList) andsrc/app/(app)/routes.tsx(Routes) share nearly identical implementation: same store subscriptions, same filtering logic, same UI structure. Consider extracting shared logic into a custom hook or shared component to reduce maintenance burden.♻️ Suggested approach
Extract a shared hook:
// src/hooks/use-route-list.ts export function useRouteList() { const routePlans = useRoutesStore((state) => state.routePlans); const isLoading = useRoutesStore((state) => state.isLoading); // ... other shared state and logic const filteredRoutes = useMemo(() => { /* ... */ }, [...]); return { routePlans, filteredRoutes, isLoading, /* ... */ }; }Then both screens can use this hook and customize only their specific differences.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/index.tsx` around lines 22 - 174, RouteList duplicates nearly all data fetching, state selection and filtering logic found in Routes; extract that shared logic into a custom hook (e.g. useRouteList) that encapsulates useRoutesStore/useUnitsStore selectors, unitMap creation, the useEffect that calls fetchAllRoutePlans/fetchActiveRoute/fetchUnits, handleRefresh, and the filteredRoutes useMemo, then return the minimal API (routePlans, filteredRoutes, isLoading, error, activeInstance, activeUnitId, activeUnit, handleRefresh, fetchAllRoutePlans, fetchActiveRoute) so both RouteList and Routes can call useRouteList() and keep only UI/rendering differences (like ListHeaderComponent or specific router.push targets) in the components; refer to functions/variables RouteList, filteredRoutes, handleRefresh, handleRoutePress, useRoutesStore, useUnitsStore and activeInstance when moving logic to useRouteList.
41-49: Sameunits.lengthdependency issue as insrc/app/(app)/routes.tsx.Including
units.lengthin the dependency array may cause redundant fetches when units are populated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/index.tsx` around lines 41 - 49, The effect is re-triggering unnecessarily because units.length is included in the dependency array; remove units.length from the useEffect dependencies and keep only stable references (useEffect(() => { fetchAllRoutePlans(); if (activeUnitId) fetchActiveRoute(activeUnitId); if (units.length === 0) fetchUnits(); }, [activeUnitId, fetchAllRoutePlans, fetchActiveRoute, fetchUnits])) so the check for an empty units array remains inside the effect but does not cause redundant runs when the array is populated; target symbols: useEffect, fetchAllRoutePlans, fetchActiveRoute, fetchUnits, activeUnitId, units.length.
116-132: Anonymous functions inrenderItemmay cause re-renders.Same as the other routes file—extract to
useCallbackfor better performance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/index.tsx` around lines 116 - 132, The anonymous renderItem and keyExtractor passed inline create new functions each render and can cause unnecessary re-renders; extract them into stable callbacks using useCallback (e.g., const renderRouteItem = useCallback(( { item }: { item: RoutePlanResultData }) => { ... }, [handleRoutePress, activeUnitId, unitMap, activeInstance, activeUnit]) and const routeKeyExtractor = useCallback((item: RoutePlanResultData) => item.RoutePlanId, [])) and then pass renderRouteItem and routeKeyExtractor to the list; ensure the extracted render uses the same logic for isMyUnit, unitName and calls handleRoutePress and that dependency arrays include any props/state referenced.src/app/maps/custom/[id].tsx (1)
14-14: Use lucide-react-native icons directly instead of wrapping with gluestackIconcomponent.Per coding guidelines: "Use
lucide-react-nativefor icons and use those components directly in the markup; don't use the gluestack-ui icon component."♻️ Example fix for line 227
-import { Icon } from '@/components/ui/icon'; +// Remove Icon import and use lucide icons directly -<Icon as={X} size="sm" className="text-gray-400" /> +<X size={16} color="#9CA3AF" />Apply similar changes to other
<Icon as={...}/>usages.As per coding guidelines: "Use
lucide-react-nativefor icons and use those components directly in the markup; don't use the gluestack-ui icon component."Also applies to: 227-227, 239-239, 250-250, 271-271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/maps/custom/`[id].tsx at line 14, The code currently imports the gluestack Icon component (Icon) and uses it like <Icon as={SomeLucideIcon} .../>; replace those usages by importing the required icons directly from 'lucide-react-native' (e.g., import { SomeLucideIcon, AnotherIcon } from 'lucide-react-native') and render them directly as <SomeLucideIcon .../> (map/adjust props accordingly — pass size/color/weight/etc. that lucide-react-native expects) and remove the Icon import from '@/components/ui/icon'; apply this change for every occurrence where Icon is used with an "as" prop (e.g., the instances that were using Icon as={...}).src/app/routes/active.tsx (1)
110-116: SortinstanceStopsonce before deriving the current stop.This screen picks the first pending/in-progress entry from the raw store array and renders the same raw order below. Other route screens in this PR already sort by
StopOrder; doing the same here avoids the current-step actions drifting to the wrong stop if response order changes.Also applies to: 401-415
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/routes/active.tsx` around lines 110 - 116, The code picks the first pending/in-progress stop from the unsorted instanceStops array which can drift if server response order changes; before deriving currentStop (the useMemo that references instanceStops and RouteStopStatus), sort instanceStops by StopOrder (ascending) and then find the first stop with Status === RouteStopStatus.Pending || Status === RouteStopStatus.InProgress; likewise update the other places in this file that derive/render stops from the raw instanceStops (the similar logic around the other occurrence noted) to use the same StopOrder-sorted array so UI actions always target the correct stop.src/models/v4/mapping/mappingResults.ts (1)
11-16: Use nullable/default factories instead of empty-object assertions here.The current
{} as ...defaults bypass the type system and leave these response models structurally invalid until hydration.nullor concrete defaults keep downstream checks honest.As per coding guidelines "Write concise, type-safe TypeScript code."
Also applies to: 23-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/mapping/mappingResults.ts` around lines 11 - 16, Replace the unsafe "{} as T" empty-object assertions in the response model classes (e.g., GetIndoorMapResult.Data and GetIndoorMapFloorResult.Data, and the other similar classes around lines 23–28) with either a nullable type (Data: IndoorMapResultData | null = null) or a proper concrete default constructed by a small factory function (e.g., createEmptyIndoorMapResultData()) so the models are structurally valid and type-safe until hydrated; update the class property declarations to use the chosen nullable or factory-returned default and adjust any callers that expect non-null accordingly.src/models/v4/routes/routeResults.ts (2)
1-1: Rename this new model file to match the repo convention.
routeResults.tsis camelCase. New files in this repo should stay lowercase and hyphenated before more imports depend on the current path.As per coding guidelines "Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/routes/routeResults.ts` at line 1, Rename the new model file from routeResults.ts to use the repo convention (route-results.ts) and update every import that references it accordingly (search for imports that reference "routeResults" and change them to "route-results"); ensure any export/import symbols defined in the file remain unchanged so references like BaseV4Request imports and any exported model names still resolve after the file rename.
12-17: Avoid defaulting required payloads to empty-object assertions.These
{} as ...initializers makeDatalook fully populated even when none of the required fields exist yet. If any caller readsDatabefore the response overwrites it, this becomes a runtimeundefinedaccess instead of a type error. Prefernullor a real default factory.As per coding guidelines "Write concise, type-safe TypeScript code."
Also applies to: 28-30, 36-37, 44-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/routes/routeResults.ts` around lines 12 - 17, The Data properties in the result classes (e.g., GetRoutePlanResult.Data and GetRouteInstanceResult.Data) must not be initialized with "{} as Type" because that masks missing required fields; change these initializers to a nullable type (e.g., Data: RoutePlanResultData | null = null) or initialize via a real factory function that returns a fully valid default object; update all other similar classes in the same file where Data is set with "{} as ..." to follow the same pattern and adjust any callers to handle null/default objects accordingly.src/api/routes/routes.ts (1)
83-106: Drop theunknown -> Record<string, unknown>casts on the mutation payloads.Those assertions erase the wire shape for every route mutation, so a contract drift will no longer be caught at compile time. It's better to widen
createApiEndpoint.postso these typed inputs can be passed through directly.As per coding guidelines "Write concise, type-safe TypeScript code."
Also applies to: 145-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/routes/routes.ts` around lines 83 - 106, Remove the unnecessary "as unknown as Record<string, unknown>" casts from all route mutation calls (e.g., startRoute, endRoute, pauseRoute, resumeRoute, cancelRoute) so the typed input interfaces (StartRouteInput, EndRouteInput, etc.) are passed directly to createApiEndpoint.post; then widen the createApiEndpoint.post signature to accept a generic request body type (for example, add a type parameter for the request payload like post<Req, Res>(payload: Req): Promise<AxiosResponse<Res>>) so the strong compile-time contract for payload shapes is preserved and no unsafe casts are needed.
| <Pressable onPress={onPress}> | ||
| <Box className="mx-4 mb-2 rounded-lg bg-amber-50 p-3 dark:bg-amber-900/30"> | ||
| <HStack className="items-center justify-between"> | ||
| <HStack className="flex-1 items-center space-x-2"> | ||
| <Icon as={AlertTriangle} size="sm" className="text-amber-600" /> | ||
| <Text className="flex-1 text-sm font-medium text-amber-800 dark:text-amber-200" numberOfLines={1}> | ||
| {latestDeviation.Description} | ||
| </Text> | ||
| </HStack> | ||
|
|
||
| {deviations.length > 1 ? ( | ||
| <Box className="mr-2 rounded-full bg-amber-200 px-2 py-0.5 dark:bg-amber-800"> | ||
| <Text className="text-xs font-bold text-amber-800 dark:text-amber-200">+{deviations.length - 1}</Text> | ||
| </Box> | ||
| ) : null} | ||
|
|
||
| <Pressable | ||
| onPress={(e) => { | ||
| e.stopPropagation?.(); | ||
| onDismiss(latestDeviation.RouteDeviationId); | ||
| }} | ||
| > | ||
| <Icon as={X} size="sm" className="text-amber-500" /> | ||
| </Pressable> |
There was a problem hiding this comment.
Add accessibility metadata and hit padding to these press targets.
The banner action and especially the icon-only dismiss button don't expose what they do to screen readers, and the close target is very small. Add accessibilityRole, localized labels, and hitSlop around the dismiss control.
As per coding guidelines "Ensure the app is accessible, following WCAG guidelines for mobile applications."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/routes/route-deviation-banner.tsx` around lines 25 - 48, The
Pressable wrappers (the main banner Pressable and the inner dismiss Pressable)
lack accessibility metadata and the dismiss control is too small; update the
outer Pressable and the dismiss Pressable to include accessibilityRole
("button"), an accessibilityLabel (use the app's i18n/localization helper, e.g.
a localized string like t('routeDeviation.open') for the banner and
t('routeDeviation.dismiss') for the dismiss icon) and accessibilityHint as
appropriate, and increase the touch target of the dismiss control by adding
hitSlop/pressRetentionOffset (or the platform-equivalent) so the icon-only
button is easier to tap; ensure you still call e.stopPropagation and then
onDismiss(latestDeviation.RouteDeviationId) in the dismiss Pressable handler.
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (11)
src/api/mapping/mapping.ts (1)
1-1: Unused import:FeatureCollection.
FeatureCollectionis imported fromgeojsonbut never used in this file. Remove the unused import.-import { type FeatureCollection } from 'geojson'; -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/mapping/mapping.ts` at line 1, Remove the unused import symbol FeatureCollection from the import statement in mapping.ts; locate the top-level import "import { type FeatureCollection } from 'geojson';" and delete FeatureCollection (or remove the entire import line if nothing else is imported) so there are no unused imports reported.src/components/routes/route-card.tsx (1)
23-33: Move pure helper functions outside the component.
formatDistanceandformatDurationdon't depend on props or state. Defining them inside the component causes recreation on every render. Move them to module scope for a minor performance improvement.+const formatDistance = (meters: number) => { + if (meters >= 1000) return `${(meters / 1000).toFixed(1)} km`; + return `${Math.round(meters)} m`; +}; + +const formatDuration = (seconds: number) => { + const hours = Math.floor(seconds / 3600); + const minutes = Math.floor((seconds % 3600) / 60); + if (hours > 0) return `${hours}h ${minutes}m`; + return `${minutes}m`; +}; + export const RouteCard: React.FC<RouteCardProps> = ({ route, isActive = false, unitName, isMyUnit = false }) => { const { t } = useTranslation(); - - const formatDistance = (meters: number) => { ... }; - const formatDuration = (seconds: number) => { ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/route-card.tsx` around lines 23 - 33, Move the pure helpers formatDistance and formatDuration out of the RouteCard component into module scope (place them above or below the component definition) so they aren't recreated on every render; ensure you keep their signatures the same and update any references inside RouteCard to use the moved functions (no props/state changes required) and run a quick type check to confirm no accidental closures or dependencies remain.src/app/(app)/routes.tsx (2)
86-131: Add FlatList optimization props.Per coding guidelines, optimize FlatList with
removeClippedSubviews,maxToRenderPerBatch, andwindowSizefor better performance, especially on low-end devices.<FlatList<RoutePlanResultData> testID="routes-list" data={filteredRoutes} + removeClippedSubviews={true} + maxToRenderPerBatch={10} + windowSize={5} ListHeaderComponent={...}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)/routes.tsx around lines 86 - 131, The FlatList<RoutePlanResultData> rendering in routes.tsx is missing performance props; update the FlatList component (the one with testID="routes-list") to include removeClippedSubviews={true}, maxToRenderPerBatch={10} (or another reasonable number for your target devices), and windowSize={21} (or an app-appropriate value) to reduce memory/CPU overhead on low-end devices; add these props directly to the FlatList JSX alongside the existing props (adjust numeric values if you need more/less aggressive batching).
117-125: ExtractrenderItemto avoid anonymous function re-renders.Per coding guidelines, avoid anonymous functions in
renderItemto prevent unnecessary re-renders. Extract this to auseCallbackor a separate component.♻️ Proposed fix using useCallback
+ const renderRouteItem = useCallback( + ({ item }: { item: RoutePlanResultData }) => { + const isMyUnit = item.UnitId != null && String(item.UnitId) === String(activeUnitId); + const resolvedUnitName = item.UnitId != null ? unitMap[item.UnitId] || (isMyUnit ? (activeUnit?.Name ?? '') : '') : ''; + return ( + <Pressable onPress={() => handleRoutePress(item)}> + <RouteCard route={item} isActive={!!activeInstance && activeInstance.RoutePlanId === item.RoutePlanId} unitName={resolvedUnitName || undefined} isMyUnit={isMyUnit} /> + </Pressable> + ); + }, + [activeUnitId, unitMap, activeUnit, activeInstance, handleRoutePress] + ); // In FlatList: - renderItem={({ item }: { item: RoutePlanResultData }) => { ... }} + renderItem={renderRouteItem}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)/routes.tsx around lines 117 - 125, The anonymous renderItem passed to the FlatList should be extracted to a stable function to prevent re-renders: create a memoized callback via useCallback (e.g., const renderRouteItem = useCallback(({ item }: { item: RoutePlanResultData }) => { ... }, [activeUnitId, unitMap, activeInstance, activeUnit, handleRoutePress]) ) or move the JSX into a separate memoized component (e.g., RouteItem) that receives item, unitName, isMyUnit, handleRoutePress and renders the Pressable + RouteCard; update the FlatList to use renderRouteItem (or <RouteItem />) and ensure you reference the existing symbols handleRoutePress, RouteCard, RoutePlanResultData, activeUnitId, unitMap, activeInstance, and activeUnit in the dependency list or props so behavior is unchanged.src/components/routes/stop-card.tsx (2)
1-1: Unused import:Phone.The
Phoneicon is imported but never used. Remove it to keep the import list clean.-import { CheckCircle, Circle, Clock, MapPin, Phone, SkipForward, User } from 'lucide-react-native'; +import { CheckCircle, Circle, Clock, MapPin, SkipForward, User } from 'lucide-react-native';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/stop-card.tsx` at line 1, Remove the unused Phone import from the lucide-react-native icon list in stop-card.tsx: update the import statement that currently imports CheckCircle, Circle, Clock, MapPin, Phone, SkipForward, User to exclude Phone so only used icons remain (e.g., CheckCircle, Circle, Clock, MapPin, SkipForward, User).
14-21: UnusedonPressprop.The
onPressprop is declared inStopCardPropsbut never used in the component's rendered output. Either wire it to aPressablewrapper or remove it from the interface to avoid confusing consumers.♻️ Option A: Wire up the prop
+import { Pressable } from 'react-native'; + export const StopCard: React.FC<StopCardProps> = ({ stop, isCurrent = false, onCheckIn, onCheckOut, onSkip, onPress }) => { const { t } = useTranslation(); const config = statusConfig[stop.Status as RouteStopStatus] || statusConfig[RouteStopStatus.Pending]; return ( - <Box className={`mb-2 rounded-xl p-4 shadow-sm ${isCurrent ? 'border-2 border-blue-500 bg-blue-50 dark:bg-blue-900/20' : 'bg-white dark:bg-gray-800'}`}> + <Pressable onPress={onPress}> + <Box className={`mb-2 rounded-xl p-4 shadow-sm ${isCurrent ? 'border-2 border-blue-500 bg-blue-50 dark:bg-blue-900/20' : 'bg-white dark:bg-gray-800'}`}>♻️ Option B: Remove unused prop
interface StopCardProps { stop: RouteInstanceStopResultData; isCurrent?: boolean; onCheckIn?: () => void; onCheckOut?: () => void; onSkip?: () => void; - onPress?: () => void; }Also applies to: 30-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/stop-card.tsx` around lines 14 - 21, The StopCardProps declares an unused onPress; wire it into the StopCard component by wrapping the component's root container in a Pressable (import from react-native/web or react-native as used elsewhere) and call the onPress prop (if provided) when pressed, preserving existing styles/children and accessibility props; ensure the optional signature matches StopCardProps and remove any dead onPress references if you choose instead to delete the prop from StopCardProps and its export.src/api/routes/routes.ts (1)
83-87: Repeatedas unknown as Record<string, unknown>type casts.This double-cast pattern is used throughout the file for POST body payloads. It works but loses compile-time validation of the input shape. Consider updating
createApiEndpoint.postto accept a generic input type directly, or creating a typed wrapper that handles the conversion internally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/routes/routes.ts` around lines 83 - 87, The file repeatedly uses unsafe double-casts like the one in startRoute (startRouteApi.post<SaveRoutePlanResult>(input as unknown as Record<string, unknown>)), which loses type safety; update the API helper instead of every call: modify createApiEndpoint.post (or add a new method like postWithBody<TRequest, TResponse>) to accept a generic request type for the body and perform the Record<string, unknown> conversion internally, then change startRoute to call startRouteApi.post<SaveRoutePlanResult, StartRouteInput>(input) (or the new wrapper) and remove the ad-hoc casts; references to startRoute, startRouteApi.post and cacheManager.remove can be used to locate and verify the change.src/app/maps/index.tsx (1)
159-166: Avoidas anytype assertion on SectionList sections.The
sections as anycast bypasses TypeScript's type safety. Consider defining a proper discriminated union type for the sections.♻️ Suggested type-safe approach
+type MapSection = + | { title: string; data: CustomMapResultData[]; type: 'custom' } + | { title: string; data: IndoorMapResultData[]; type: 'indoor' }; const sections = [ ...(customMaps.length > 0 ? [ { title: t('maps.custom_maps'), data: customMaps, - renderItem: ({ item }: { item: CustomMapResultData }) => renderCustomMapCard(item), + type: 'custom' as const, }, ] : []), ...(indoorMaps.length > 0 ? [ { title: t('maps.indoor_maps'), data: indoorMaps, - renderItem: ({ item }: { item: IndoorMapResultData }) => renderIndoorMapCard(item), + type: 'indoor' as const, }, ] : []), -]; +] as MapSection[]; // Then use a single renderItem that switches on section type🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/maps/index.tsx` around lines 159 - 166, The SectionList is being passed "sections as any", which bypasses TypeScript safety; instead define a proper discriminated union section item type (e.g., a union between CustomMapResultData and IndoorMapResultData with a discriminant property) and a matching Section type for SectionList (title: string, data: ItemType[]), then type the local sections variable to that Section[] and supply the generic to SectionList (SectionList<ItemType | ...>) so you can remove the "as any" cast; ensure keyExtractor and renderSectionHeader signatures use the new ItemType/Section types so TypeScript validates the union correctly.src/models/v4/mapping/mappingResults.ts (3)
49-58: Use a discriminated union forUnifiedSearchResultItemfor stricter typing.Requiring both
FloorIdandLayerIdfor allTypevariants weakens model precision and encourages placeholder values. Split byTypeto keep downstream handling type-safe.As per coding guidelines:
**/*.{ts,tsx}: Write concise, type-safe TypeScript code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/mapping/mappingResults.ts` around lines 49 - 58, UnifiedSearchResultItem currently forces FloorId and LayerId for both Type variants; change it to a discriminated union by splitting into two interfaces (e.g., IndoorZoneResult and CustomRegionResult) keyed by Type: 'indoor_zone' and 'custom_region', each exposing only the relevant fields (for example indoor_zone includes MapId, FloorId, LayerId, Latitude, Longitude; custom_region includes MapId, Latitude, Longitude but omits FloorId/LayerId), then export UnifiedSearchResultItem = IndoorZoneResult | CustomRegionResult so downstream code can narrow on Type safely.
1-1: Rename this file to lowercase-hyphenated format.
mappingResults.tsbreaks the repository naming convention; considermapping-results.tsfor consistency and discoverability.As per coding guidelines:
**/*: Directory and file names should be lowercase and hyphenated (e.g.,user-profile,chat-screen).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/mapping/mappingResults.ts` at line 1, Rename the file mappingResults.ts to mapping-results.ts to follow the lowercase-hyphenated naming convention; then update all references/imports that import symbols from mappingResults.ts (search for "mappingResults" and the import path) to use the new filename "mapping-results" and adjust any barrel/index exports that re-export from mappingResults (e.g., export lines in modules that mention mappingResults) so imports across the codebase continue to resolve.
12-16: Avoid{}type assertions for required payload models.Initializing
Datawith{}casted to concrete types bypasses compile-time guarantees and can hide undefined field access before hydration.Proposed type-safe initialization pattern
export class GetIndoorMapResult extends BaseV4Request { - public Data: IndoorMapResultData = {} as IndoorMapResultData; + public Data: IndoorMapResultData | null = null; } export class GetIndoorMapFloorResult extends BaseV4Request { - public Data: IndoorMapFloorResultData = {} as IndoorMapFloorResultData; + public Data: IndoorMapFloorResultData | null = null; } export class GetCustomMapResult extends BaseV4Request { - public Data: CustomMapResultData = {} as CustomMapResultData; + public Data: CustomMapResultData | null = null; } export class GetCustomMapLayerResult extends BaseV4Request { - public Data: CustomMapResultData = {} as CustomMapResultData; + public Data: CustomMapResultData | null = null; }As per coding guidelines:
**/*.{ts,tsx}: Write concise, type-safe TypeScript code.Also applies to: 24-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/mapping/mappingResults.ts` around lines 12 - 16, The Data properties are wrongly initialized with "{} as ..." which circumvents type safety; for required payload models (e.g., the Data property on the class where Data: IndoorMapResultData and on GetIndoorMapFloorResult.Data: IndoorMapFloorResultData, and similar properties around lines 24-28) remove the "{} as" cast and replace with either a proper default object that fills all required fields or, if the property is populated later, declare it without a fake initializer using the definite assignment modifier (e.g., "public Data!: IndoorMapResultData;") or make the type explicitly nullable/optional (e.g., "public Data: IndoorMapResultData | null = null;") so callers cannot assume fields exist before hydration.
🤖 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/maps/custom/`[id].tsx:
- Line 186: Replace the hard-coded fallback "Region" with a localized string:
import and use the project's i18n helper (e.g., t or useTranslation) and change
the JSX expression inside the Text component that renders {(selectedFeature.name
as string) || (selectedFeature.Name as string) || 'Region'} to use the localized
fallback (for example: || t('region') or || i18n.t('region')). Ensure you add
the import for the translation function and that the translation key ("region")
exists in the locale files.
In `@src/app/maps/indoor/`[id].tsx:
- Line 222: The fallback label in the Text element currently uses a hard-coded
string 'Zone'; replace this with a localized value using the app's i18n utility
(e.g., the translation function from useTranslation or t) so the fallback
becomes something like t('zone') or t('maps.zone'); update the component to
import/use the translation hook (if not already used) and use the localized key
instead of the literal 'Zone' when rendering {(selectedZone.name as string) ||
(selectedZone.Name as string) || 'Zone'} so accessibility and translations work
correctly.
In `@src/app/routes/history/instance/`[id].tsx:
- Around line 63-69: DEVIATION_TYPE_LABELS currently contains hardcoded English
strings; replace it with a mapping of RouteDeviationType to i18n keys (e.g.,
DEVIATION_TYPE_KEYS) and update DeviationCard to call
t(DEVIATION_TYPE_KEYS[deviation.Type]) instead of using DEVIATION_TYPE_LABELS,
ensuring you import and use the translation function t from your i18n
hook/context and keep the RouteDeviationType enum keys unchanged so lookup
works.
- Around line 418-422: Replace the hardcoded English labels "In:", "Out:", and
"Skipped:" inside the HStack rendering (where stop.CheckedInOn /
stop.CheckedOutOn / stop.SkippedOn are formatted with formatDate) with i18n
translation keys by using the project's translation helper (e.g.,
useTranslations or t) and the appropriate keys (e.g., history.instance.in,
history.instance.out, history.instance.skipped); keep using
formatDate(stop.CheckedInOn) etc. so only the label strings change to translated
values and ensure the translation hook is imported and used in this component
(refer to the HStack block and formatDate usage to locate the code).
In `@src/app/routes/stop/contact.tsx`:
- Around line 181-186: The phone labels passed into PhoneRow are hard-coded;
wrap each label string in the i18n translator (t) and ensure t is obtained via
useTranslation from react-i18next in this component; specifically update the
PhoneRow calls (PhoneRow label="Phone" etc.) to use t('Phone'), t('Mobile'),
t('Home'), t('Cell'), t('Office'), t('Fax') and make sure the file
imports/initializes useTranslation and reads const { t } = useTranslation() so
colorScheme and contact props stay unchanged.
- Around line 233-251: The three hard-coded legend labels inside the Contact
route JSX (the Text nodes shown alongside the legend dots when locationGps,
entranceGps, and exitGps are truthy) must be wrapped with the translation
function (e.g., t('...') or using useTranslation) instead of plain strings;
update the Text contents for "Location", "Entrance", and "Exit" to use
translation keys (like t('stop.location'), t('stop.entrance'), t('stop.exit')),
ensure the translation function is imported or obtained via useTranslation in
contact.tsx, and add the corresponding keys to your i18n resource files.
In `@src/models/v4/mapping/mappingResults.ts`:
- Around line 27-28: The Data property on GetCustomMapLayerResult is incorrectly
typed as CustomMapResultData (a map-level type with a Layers array); update the
Data type to CustomMapLayerResultData and adjust its initialization (e.g.,
public Data: CustomMapLayerResultData = {} as CustomMapLayerResultData) and add
or update the import for CustomMapLayerResultData so the class, Data property,
and typing reflect a single layer response.
In `@src/stores/routes/store.ts`:
- Around line 279-294: Update the stop interaction actions (checkIn, checkOut,
skip) so callers can observe failure: change their signatures to return a
Promise<boolean>, return true after successful API call and state update, and in
the catch block set the error state (as now) and return false; use the existing
API helpers (checkInAtStop, checkOutFromStop, skipStop) and the store accessors
(get(), set()) when implementing this so callers can await e.g. await
checkIn(...) and branch on the boolean result instead of relying solely on the
error state.
In `@src/translations/en.json`:
- Line 517: The "floor_count" translation is not pluralization-aware (e.g.,
produces "1 floors"); replace it with language plural forms supported by your
i18n system (split into singular/plural or use ICU-style pluralization) and do
the same for the other similar key referenced at line 671; update any code that
reads the "floor_count" key (and the other key) to pass the numeric count
parameter so the correct singular/plural variant is chosen at runtime.
---
Nitpick comments:
In `@src/api/mapping/mapping.ts`:
- Line 1: Remove the unused import symbol FeatureCollection from the import
statement in mapping.ts; locate the top-level import "import { type
FeatureCollection } from 'geojson';" and delete FeatureCollection (or remove the
entire import line if nothing else is imported) so there are no unused imports
reported.
In `@src/api/routes/routes.ts`:
- Around line 83-87: The file repeatedly uses unsafe double-casts like the one
in startRoute (startRouteApi.post<SaveRoutePlanResult>(input as unknown as
Record<string, unknown>)), which loses type safety; update the API helper
instead of every call: modify createApiEndpoint.post (or add a new method like
postWithBody<TRequest, TResponse>) to accept a generic request type for the body
and perform the Record<string, unknown> conversion internally, then change
startRoute to call startRouteApi.post<SaveRoutePlanResult,
StartRouteInput>(input) (or the new wrapper) and remove the ad-hoc casts;
references to startRoute, startRouteApi.post and cacheManager.remove can be used
to locate and verify the change.
In `@src/app/`(app)/routes.tsx:
- Around line 86-131: The FlatList<RoutePlanResultData> rendering in routes.tsx
is missing performance props; update the FlatList component (the one with
testID="routes-list") to include removeClippedSubviews={true},
maxToRenderPerBatch={10} (or another reasonable number for your target devices),
and windowSize={21} (or an app-appropriate value) to reduce memory/CPU overhead
on low-end devices; add these props directly to the FlatList JSX alongside the
existing props (adjust numeric values if you need more/less aggressive
batching).
- Around line 117-125: The anonymous renderItem passed to the FlatList should be
extracted to a stable function to prevent re-renders: create a memoized callback
via useCallback (e.g., const renderRouteItem = useCallback(({ item }: { item:
RoutePlanResultData }) => { ... }, [activeUnitId, unitMap, activeInstance,
activeUnit, handleRoutePress]) ) or move the JSX into a separate memoized
component (e.g., RouteItem) that receives item, unitName, isMyUnit,
handleRoutePress and renders the Pressable + RouteCard; update the FlatList to
use renderRouteItem (or <RouteItem />) and ensure you reference the existing
symbols handleRoutePress, RouteCard, RoutePlanResultData, activeUnitId, unitMap,
activeInstance, and activeUnit in the dependency list or props so behavior is
unchanged.
In `@src/app/maps/index.tsx`:
- Around line 159-166: The SectionList is being passed "sections as any", which
bypasses TypeScript safety; instead define a proper discriminated union section
item type (e.g., a union between CustomMapResultData and IndoorMapResultData
with a discriminant property) and a matching Section type for SectionList
(title: string, data: ItemType[]), then type the local sections variable to that
Section[] and supply the generic to SectionList (SectionList<ItemType | ...>) so
you can remove the "as any" cast; ensure keyExtractor and renderSectionHeader
signatures use the new ItemType/Section types so TypeScript validates the union
correctly.
In `@src/components/routes/route-card.tsx`:
- Around line 23-33: Move the pure helpers formatDistance and formatDuration out
of the RouteCard component into module scope (place them above or below the
component definition) so they aren't recreated on every render; ensure you keep
their signatures the same and update any references inside RouteCard to use the
moved functions (no props/state changes required) and run a quick type check to
confirm no accidental closures or dependencies remain.
In `@src/components/routes/stop-card.tsx`:
- Line 1: Remove the unused Phone import from the lucide-react-native icon list
in stop-card.tsx: update the import statement that currently imports
CheckCircle, Circle, Clock, MapPin, Phone, SkipForward, User to exclude Phone so
only used icons remain (e.g., CheckCircle, Circle, Clock, MapPin, SkipForward,
User).
- Around line 14-21: The StopCardProps declares an unused onPress; wire it into
the StopCard component by wrapping the component's root container in a Pressable
(import from react-native/web or react-native as used elsewhere) and call the
onPress prop (if provided) when pressed, preserving existing styles/children and
accessibility props; ensure the optional signature matches StopCardProps and
remove any dead onPress references if you choose instead to delete the prop from
StopCardProps and its export.
In `@src/models/v4/mapping/mappingResults.ts`:
- Around line 49-58: UnifiedSearchResultItem currently forces FloorId and
LayerId for both Type variants; change it to a discriminated union by splitting
into two interfaces (e.g., IndoorZoneResult and CustomRegionResult) keyed by
Type: 'indoor_zone' and 'custom_region', each exposing only the relevant fields
(for example indoor_zone includes MapId, FloorId, LayerId, Latitude, Longitude;
custom_region includes MapId, Latitude, Longitude but omits FloorId/LayerId),
then export UnifiedSearchResultItem = IndoorZoneResult | CustomRegionResult so
downstream code can narrow on Type safely.
- Line 1: Rename the file mappingResults.ts to mapping-results.ts to follow the
lowercase-hyphenated naming convention; then update all references/imports that
import symbols from mappingResults.ts (search for "mappingResults" and the
import path) to use the new filename "mapping-results" and adjust any
barrel/index exports that re-export from mappingResults (e.g., export lines in
modules that mention mappingResults) so imports across the codebase continue to
resolve.
- Around line 12-16: The Data properties are wrongly initialized with "{} as
..." which circumvents type safety; for required payload models (e.g., the Data
property on the class where Data: IndoorMapResultData and on
GetIndoorMapFloorResult.Data: IndoorMapFloorResultData, and similar properties
around lines 24-28) remove the "{} as" cast and replace with either a proper
default object that fills all required fields or, if the property is populated
later, declare it without a fake initializer using the definite assignment
modifier (e.g., "public Data!: IndoorMapResultData;") or make the type
explicitly nullable/optional (e.g., "public Data: IndoorMapResultData | null =
null;") so callers cannot assume fields exist before hydration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca2acd71-ec3b-4814-8504-ad1f3bc28b81
📒 Files selected for processing (37)
jest-setup.tssrc/api/mapping/mapping.tssrc/api/routes/routes.tssrc/app/(app)/_layout.tsxsrc/app/(app)/index.tsxsrc/app/(app)/routes.tsxsrc/app/login/index.tsxsrc/app/login/login-form.tsxsrc/app/maps/custom/[id].tsxsrc/app/maps/index.tsxsrc/app/maps/indoor/[id].tsxsrc/app/maps/search.tsxsrc/app/routes/active.tsxsrc/app/routes/directions.tsxsrc/app/routes/history/[planId].tsxsrc/app/routes/history/instance/[id].tsxsrc/app/routes/index.tsxsrc/app/routes/start.tsxsrc/app/routes/stop/[id].tsxsrc/app/routes/stop/contact.tsxsrc/components/maps/map-view.web.tsxsrc/components/routes/route-card.tsxsrc/components/routes/stop-card.tsxsrc/lib/logging/index.tsxsrc/models/v4/mapping/mappingResults.tssrc/models/v4/routes/routeInstanceResultData.tssrc/stores/maps/store.tssrc/stores/routes/store.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
✅ Files skipped from review due to trivial changes (8)
- src/app/login/index.tsx
- src/translations/fr.json
- src/translations/pl.json
- src/translations/sv.json
- src/translations/it.json
- src/translations/uk.json
- src/translations/de.json
- src/models/v4/routes/routeInstanceResultData.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/app/login/login-form.tsx
- src/app/(app)/_layout.tsx
- src/app/maps/search.tsx
- src/app/routes/history/[planId].tsx
- src/lib/logging/index.tsx
- src/app/routes/index.tsx
- src/app/(app)/index.tsx
- src/app/routes/start.tsx
- src/translations/es.json
- src/components/maps/map-view.web.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/maps/indoor/[id].tsx (1)
16-16: Consider using lucide icons directly instead of gluestack-ui Icon wrapper.The coding guidelines specify: "Use
lucide-react-nativefor icons and use those components directly in the markup; don't use the gluestack-ui icon component."Lines 221 and 240 use
<Icon as={Building2} ...>and<Icon as={X} ...>via the gluestack-ui Icon wrapper, while lines 143, 148 useInputIcon as={Search}andInputIcon as={X}.For consistency with coding guidelines, consider using the lucide icons directly:
- <Icon as={Building2} size="sm" className="text-purple-600 dark:text-purple-400" /> + <Building2 size={16} color="#9333ea" />Also applies to: 221-221, 240-240
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/maps/indoor/`[id].tsx at line 16, The Icon wrapper import and usages should be replaced with direct lucide components: remove the gluestack-ui Icon import and instead import the lucide-react-native icons (e.g., Building2, X, Search) and use them directly where you currently have Icon as={Building2} and Icon as={X} (and any Icon/InputIcon usages that proxy to lucide). Update the JSX to render <Building2 .../> and <X .../> (forwarding the same props you relied on: size/width/height, color/style, accessibility props) and adjust any InputIcon usages to import and render the Search/X lucide components directly; ensure props signatures match and remove the Icon wrapper import.
🤖 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/routes/history/instance/`[id].tsx:
- Around line 133-136: The catch handler uses a hardcoded English string; wrap
the message in the i18n translator by changing setError('Failed to load
instance') to setError(t('failed_to_load_instance')) and ensure the component
has access to t (e.g., import/use the useTranslation hook and get const { t } =
useTranslation()). Update the translation resource files to include the
'failed_to_load_instance' key with the appropriate localized strings. Keep the
rest of the getRouteProgress(...).then(...).finally(...) flow unchanged.
---
Nitpick comments:
In `@src/app/maps/indoor/`[id].tsx:
- Line 16: The Icon wrapper import and usages should be replaced with direct
lucide components: remove the gluestack-ui Icon import and instead import the
lucide-react-native icons (e.g., Building2, X, Search) and use them directly
where you currently have Icon as={Building2} and Icon as={X} (and any
Icon/InputIcon usages that proxy to lucide). Update the JSX to render <Building2
.../> and <X .../> (forwarding the same props you relied on: size/width/height,
color/style, accessibility props) and adjust any InputIcon usages to import and
render the Search/X lucide components directly; ensure props signatures match
and remove the Icon wrapper import.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80d4e94b-b4ff-465f-998b-8a0a5658a285
📒 Files selected for processing (16)
CLAUDE.mdsrc/app/maps/custom/[id].tsxsrc/app/maps/indoor/[id].tsxsrc/app/routes/history/instance/[id].tsxsrc/app/routes/stop/contact.tsxsrc/models/v4/mapping/mappingResults.tssrc/stores/routes/store.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
✅ Files skipped from review due to trivial changes (8)
- src/translations/pl.json
- src/translations/de.json
- src/translations/fr.json
- src/translations/uk.json
- src/translations/sv.json
- src/translations/en.json
- src/translations/it.json
- src/models/v4/mapping/mappingResults.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/maps/custom/[id].tsx
- src/stores/routes/store.ts
| getRouteProgress(instanceId) | ||
| .then((r) => setActiveInstance(r.Data)) | ||
| .catch(() => setError('Failed to load instance')) | ||
| .finally(() => setIsLoading(false)); |
There was a problem hiding this comment.
Wrap error message in t() for internationalization.
The error message "Failed to load instance" on line 135 is hardcoded in English. As per coding guidelines, all user-facing text must be wrapped in t() from react-i18next.
🌐 Suggested fix
getRouteProgress(instanceId)
.then((r) => setActiveInstance(r.Data))
- .catch(() => setError('Failed to load instance'))
+ .catch(() => setError(t('common.errorOccurred')))
.finally(() => setIsLoading(false));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/routes/history/instance/`[id].tsx around lines 133 - 136, The catch
handler uses a hardcoded English string; wrap the message in the i18n translator
by changing setError('Failed to load instance') to
setError(t('failed_to_load_instance')) and ensure the component has access to t
(e.g., import/use the useTranslation hook and get const { t } =
useTranslation()). Update the translation resource files to include the
'failed_to_load_instance' key with the appropriate localized strings. Keep the
rest of the getRouteProgress(...).then(...).finally(...) flow unchanged.
|
Approve |
Summary by CodeRabbit
New Features
Improvements
Documentation