Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds weather push-notification parsing, modal actions, cold-start replay, and response deduplication. It also centralizes fixed-size header back buttons, improves login error rendering, keeps notification details alongside the inbox, and adds localized push-notification strings. ChangesShared navigation and notification UI
Weather push notifications
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ExpoNotifications
participant PushNotificationService
participant PushNotificationModalStore
participant PushNotificationModal
participant WeatherAlertsStore
ExpoNotifications->>PushNotificationService: deliver notification or launch response
PushNotificationService->>PushNotificationService: validate eventCode and deduplicate response
PushNotificationService->>PushNotificationModalStore: show notification modal
PushNotificationModal->>WeatherAlertsStore: load weather alert
WeatherAlertsStore-->>PushNotificationModal: return alert data
PushNotificationModal->>PushNotificationModal: navigate to weather alert detail
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/push-notification/push-notification-modal.tsx (1)
159-168: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
NotificationIconPropsand pass the calculatediconColor.The
iconColorvariable is computed on line 159 to support the new amber color for weather alerts, but it is never passed toNotificationIcon. Furthermore, theNotificationIconcomponent currently ignores its definedNotificationIconPropsinterface and hardcodes the color to red (#EF4444), preventing your new color from being applied.Pass the
iconColorto the icon and update the component signature to utilize the interface.🐛 Proposed fixes
Update the rendering of the icon here:
- const iconColor = getNotificationColor(notification.type); - const typeText = getNotificationTypeText(notification.type); - - return ( - <Modal isOpen={isOpen} onClose={handleClose} size="md"> - <ModalBackdrop /> - <ModalContent className="mx-4"> - <ModalHeader className="pb-4"> - <HStack className="items-center space-x-3"> - <NotificationIcon type={notification.type} /> + const iconColor = getNotificationColor(notification.type); + const typeText = getNotificationTypeText(notification.type); + + return ( + <Modal isOpen={isOpen} onClose={handleClose} size="md"> + <ModalBackdrop /> + <ModalContent className="mx-4"> + <ModalHeader className="pb-4"> + <HStack className="items-center space-x-3"> + <NotificationIcon type={notification.type} color={iconColor} />And update the
NotificationIcondefinition (around line 21) to use the ignored interface:const NotificationIcon = ({ type, color, size }: NotificationIconProps) => { const iconProps = { size: size || 24, color: color || '`#EF4444`', testID: 'notification-icon', };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/push-notification/push-notification-modal.tsx` around lines 159 - 168, Update NotificationIcon to destructure and use NotificationIconProps, applying the optional color and size values with the existing red and 24px defaults. In the modal rendering near iconColor, pass the calculated iconColor to NotificationIcon so weather alerts use the intended color.
🧹 Nitpick comments (3)
src/app/login/index.tsx (1)
109-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid conditionally rendering the
Modalcomponent to preserve exit animations.Gluestack-UI's
Modalhandles its own conditional rendering (viaAnimatePresence). Wrapping it in{isErrorModalVisible ? <Modal .../> : null}forces it to unmount instantly when closed, which breaks the exit transition.Consider rendering the
Modalunconditionally and letting theisOpenprop dictate visibility.🛠️ Proposed refactor
- {isErrorModalVisible ? ( - <Modal isOpen={isErrorModalVisible} onClose={() => setIsErrorModalVisible(false)} size="full"> - <ModalBackdrop /> - <ModalContent className="m-4 w-full max-w-3xl rounded-2xl"> - <ModalHeader> - <Text className="text-xl font-semibold">{t('login.errorModal.title')}</Text> - </ModalHeader> - <ModalBody> - <Text>{t('login.errorModal.message')}</Text> - {error ? <Text className="mt-2 text-sm text-gray-500">{error}</Text> : null} - </ModalBody> - <ModalFooter> - <Button - variant="solid" - size="sm" - action="primary" - onPress={() => { - setIsErrorModalVisible(false); - }} - > - <ButtonText>{t('login.errorModal.confirmButton')}</ButtonText> - </Button> - </ModalFooter> - </ModalContent> - </Modal> - ) : null} + <Modal isOpen={isErrorModalVisible} onClose={() => setIsErrorModalVisible(false)} size="full"> + <ModalBackdrop /> + <ModalContent className="m-4 w-full max-w-3xl rounded-2xl"> + <ModalHeader> + <Text className="text-xl font-semibold">{t('login.errorModal.title')}</Text> + </ModalHeader> + <ModalBody> + <Text>{t('login.errorModal.message')}</Text> + {error ? <Text className="mt-2 text-sm text-gray-500">{error}</Text> : null} + </ModalBody> + <ModalFooter> + <Button + variant="solid" + size="sm" + action="primary" + onPress={() => { + setIsErrorModalVisible(false); + }} + > + <ButtonText>{t('login.errorModal.confirmButton')}</ButtonText> + </Button> + </ModalFooter> + </ModalContent> + </Modal>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/login/index.tsx` around lines 109 - 134, Update the error modal rendering around isErrorModalVisible to always mount the Modal component, relying on its isOpen prop to control visibility and preserve exit animations. Keep the existing modal contents, close handler, and error message behavior unchanged.src/services/__tests__/push-notification.test.ts (2)
347-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the new
anyescape hatches.Define typed mock input, construct requests with their identifiers, and use a precise test interface instead of
as any.As per coding guidelines, “Avoid using any; use precise types.”
Also applies to: 441-457
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/__tests__/push-notification.test.ts` around lines 347 - 355, Update createMockResponse and the related mock setup to eliminate all any casts. Define a typed input for createMockNotification, construct the notification request with its identifier rather than mutating it through as any, and introduce a precise test response interface/type instead of casting to Notifications.NotificationResponse.Source: Coding guidelines
436-468: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest cold-start replay through initialization.
The test configures the response after singleton initialization and invokes the private helper directly. Removing the initialization call at
src/services/push-notification.tsLine 190 would still pass. Mock the response before importing a fresh service instance and assert automatic presentation.As per coding guidelines, “Create Jest tests for all generated components, services, and logic.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/__tests__/push-notification.test.ts` around lines 436 - 468, Update the cold-start test around presentLaunchNotificationResponse to mock getLastNotificationResponseAsync before importing a fresh pushNotificationService instance, then trigger normal service initialization and assert the modal is presented automatically. Remove the direct private-helper invocation so the test fails if the initialization call in the service is removed, while preserving the existing notification payload assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/notifications/NotificationInbox.tsx`:
- Around line 266-286: Update the Select All condition in NotificationInbox to
require a non-empty notifications list before comparing
selectedNotificationIds.size with notifications.length, preventing the empty
state from being treated as fully selected. Use this condition consistently for
the toggle handler, accessibility label, and icon, rendering CheckCircle when
all notifications are selected and Circle otherwise.
---
Outside diff comments:
In `@src/components/push-notification/push-notification-modal.tsx`:
- Around line 159-168: Update NotificationIcon to destructure and use
NotificationIconProps, applying the optional color and size values with the
existing red and 24px defaults. In the modal rendering near iconColor, pass the
calculated iconColor to NotificationIcon so weather alerts use the intended
color.
---
Nitpick comments:
In `@src/app/login/index.tsx`:
- Around line 109-134: Update the error modal rendering around
isErrorModalVisible to always mount the Modal component, relying on its isOpen
prop to control visibility and preserve exit animations. Keep the existing modal
contents, close handler, and error message behavior unchanged.
In `@src/services/__tests__/push-notification.test.ts`:
- Around line 347-355: Update createMockResponse and the related mock setup to
eliminate all any casts. Define a typed input for createMockNotification,
construct the notification request with its identifier rather than mutating it
through as any, and introduce a precise test response interface/type instead of
casting to Notifications.NotificationResponse.
- Around line 436-468: Update the cold-start test around
presentLaunchNotificationResponse to mock getLastNotificationResponseAsync
before importing a fresh pushNotificationService instance, then trigger normal
service initialization and assert the modal is presented automatically. Remove
the direct private-helper invocation so the test fails if the initialization
call in the service is removed, while preserving the existing notification
payload assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec91efc0-cc28-4ce8-b5fb-458bf289a273
📒 Files selected for processing (24)
src/app/call/[id]/index.tsxsrc/app/login/index.tsxsrc/app/login/login-form.tsxsrc/app/poi/[id].tsxsrc/components/calls/call-detail-menu.tsxsrc/components/common/__tests__/header-back-button.test.tsxsrc/components/common/header-back-button.tsxsrc/components/notifications/NotificationInbox.tsxsrc/components/push-notification/__tests__/push-notification-modal.test.tsxsrc/components/push-notification/push-notification-modal.tsxsrc/services/__tests__/push-notification-channels.test.tssrc/services/__tests__/push-notification.test.tssrc/services/push-notification.tssrc/stores/push-notification/__tests__/store.test.tssrc/stores/push-notification/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
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit