RU-T44 Inbox tweak, fixing map marker. #186
Conversation
WalkthroughUpdates: dependency bump for Changes
Sequence Diagram(s)sequenceDiagram
participant Parent as Parent Component
participant Picker as LocationPicker
participant Effect as useEffect (initialLocation)
participant Camera as CameraRef
Note over Picker,Effect: initialLocation prop update
Parent->>Picker: provide new initialLocation
Picker->>Effect: effect runs on prop change
activate Effect
Effect->>Camera: setCamera({centerCoordinate: [lng, lat], zoomLevel: 15, animationDuration: 1000})
activate Camera
Camera-->>Effect: camera animation completes (async)
Effect->>Picker: set currentLocation state to initialLocation
deactivate Effect
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/maps/location-picker.tsx (1)
61-77: Potential excessive re-renders ifinitialLocationreference changes.The
useEffecttriggers wheneverinitialLocationchanges (line 77). If the parent component passes a new object reference on every render—even with identical values—the camera will animate repeatedly.Consider adding a deep comparison or memoization:
useEffect(() => { if (initialLocation) { setCurrentLocation(initialLocation); // Move camera to the new location if (cameraRef.current) { cameraRef.current.setCamera({ centerCoordinate: [initialLocation.longitude, initialLocation.latitude], zoomLevel: 15, animationDuration: 1000, }); } } else { getUserLocation().catch((error) => { console.error('Failed to get user location:', error); }); } - }, [initialLocation, getUserLocation]); + }, [initialLocation?.latitude, initialLocation?.longitude, getUserLocation]);This change ensures the effect only runs when the actual coordinate values change, not just the object reference.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.json(1 hunks)src/components/maps/location-picker.tsx(1 hunks)src/components/notifications/NotificationInbox.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/components/maps/location-picker.tsxsrc/components/notifications/NotificationInbox.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for React component names (e.g., UserProfile, ChatScreen)
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo() for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Optimize images using react-native-fast-image
Use React Navigation for navigation and deep linking with best practices
Wrap all user-facing text in t() from react-i18next
Use react-hook-form for form handling
Use @rnmapbox/maps for maps or vehicle navigation
Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Use the conditional operator (?:) for conditional rendering instead of &&
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect/useState and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers
Optimize image handling using react-native-fast-image
Wrap all user-facing text in t() from react-i18next
Ensure support for dark mode and light mode
Ensure the app is accessible following WCAG for mobile
Use react-hook-form for form handling
Use @rnmapbox/maps for maps and navigation
Use lucide-react-native for icons directly in markup; do not us...
Files:
src/components/maps/location-picker.tsxsrc/components/notifications/NotificationInbox.tsx
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/components/maps/location-picker.tsxsrc/components/notifications/NotificationInbox.tsxpackage.json
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/components/maps/location-picker.tsxsrc/components/notifications/NotificationInbox.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps and navigation
Applied to files:
src/components/maps/location-picker.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or vehicle navigation
Applied to files:
src/components/maps/location-picker.tsx
🔇 Additional comments (4)
src/components/notifications/NotificationInbox.tsx (2)
197-197: Displaying title instead of body may confuse users.The notification now displays
notification.title(which containsitem.subjectfrom line 165) instead ofnotification.body. If the subject is typically shorter or less detailed than the body, users may see less information at a glance.Consider whether this change improves the user experience or if both subject and body should be displayed. Review the Novu v3 notification schema to understand the intended usage of these fields.
162-173: The field mapping is correct for Novu v3 and requires no changes.The code properly adapts to Novu v3's API schema, which uses
subjectinstead oftitle. The mapping (title: item.subject, body: item.body) correctly transforms the API response to the app'sNotificationPayloadstructure. The display logic in both the list view (showingnotification.title) and detail view (showing bothtitleandbody) is consistent and intentional. This is the proper way to handle the Novu v3 migration.src/components/maps/location-picker.tsx (1)
64-71: Camera animation enhances the map UX.The addition of camera animation when
initialLocationchanges improves the user experience by smoothly transitioning to the new location. The coordinate order[longitude, latitude]is correct for Mapbox's GeoJSON format.package.json (1)
92-92: Upgrade to @novu/react-native v3.11.0 is correctly adapted.Breaking changes in v3 include component renames (NotificationCenter→Notifications), props changes (styles→appearance), and new provider/hooks (useNovu). The field mapping in
NotificationInbox.tsx(lines 165, 197) correctly adapts to the v3 API: v3 Notification payload includes subject and body fields, which are properly mapped to the internalNotificationPayloadtype that usestitleinstead. The mapping is appropriately implemented and the upgrade is compatible with the changes shown.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/react-native-cicd.yml (1)
305-320: Consolidate HTML comment removal into a single sed expression for clarity.Lines 319–320 chain three separate sed commands to remove HTML comments. These can be combined into a single pass for better readability and performance.
- # Third pass: Remove any remaining HTML comment lines - cleaned_body="$(printf '%s\n' "$cleaned_body" | sed '/^<!--.*-->$/d' | sed '/^<!--/d' | sed '/^-->$/d')" + # Third pass: Remove any remaining HTML comment lines + cleaned_body="$(printf '%s\n' "$cleaned_body" | sed -e '/^<!--.*-->$/d' -e '/^<!--/d' -e '/^-->$/d')"Note: If the workflow encounters multi-line HTML comments where
<!--and-->appear on separate lines, the current approach may not catch them. Consider using a more robust pattern if multi-line comments are a concern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/react-native-cicd.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps and navigation
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or vehicle navigation
🔇 Additional comments (1)
.github/workflows/react-native-cicd.yml (1)
325-331: Release notes extraction logic is sound, but verify edge cases.The multi-pass extraction correctly extracts content under "## Release Notes" (lines 326–327) and falls back to the full cleaned body when no section is found (lines 330–331). This removes the prior 500-character truncation, which aligns with the PR objective.
Verify that:
- Markdown headings with variations (e.g.,
## release notes,### Release Notes) are handled as expected.- The awk pattern
f && /^## /{exit}correctly stops at the next heading without including it.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/notifications/__tests__/NotificationInbox.test.tsx (1)
210-210: Consistent mock update across all test scenarios.The addition of
seenAll: jest.fn()to the mockeduseNotificationsreturn value is correctly applied in all three locations (default setup, loading state, and empty state scenarios), maintaining consistency with the updated hook API.Optional: Consider adding test coverage for
seenAllfunctionality.While the mock is properly configured, there are currently no tests that verify the behavior or usage of the
seenAllfunction. If this function is actively used by the NotificationInbox component, consider adding test cases to verify it's called appropriately (e.g., when notifications are displayed, after certain user interactions, etc.).Also applies to: 357-357, 378-378
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
expo-env.d.ts(1 hunks)src/components/notifications/__tests__/NotificationInbox.test.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- expo-env.d.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for React component names (e.g., UserProfile, ChatScreen)
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo() for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Optimize images using react-native-fast-image
Use React Navigation for navigation and deep linking with best practices
Wrap all user-facing text in t() from react-i18next
Use react-hook-form for form handling
Use @rnmapbox/maps for maps or vehicle navigation
Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Use the conditional operator (?:) for conditional rendering instead of &&
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect/useState and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers
Optimize image handling using react-native-fast-image
Wrap all user-facing text in t() from react-i18next
Ensure support for dark mode and light mode
Ensure the app is accessible following WCAG for mobile
Use react-hook-form for form handling
Use @rnmapbox/maps for maps and navigation
Use lucide-react-native for icons directly in markup; do not us...
Files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit inference engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps and navigation
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or vehicle navigation
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
Repo: Resgrid/Unit PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated components
Applied to files:
src/components/notifications/__tests__/NotificationInbox.test.tsx
Summary by CodeRabbit
New Features
Bug Fixes
Chores