CU-868frp6xf Fixing onboarding, toast and new call creation. #174
Conversation
|
Task linked: CU-868frp6xf Unable to select Recipients |
WalkthroughAdds Android tablet safe-area handling and edge-to-edge status bar to the call screen, centralizes toast usage with a ToastContainer and hook, changes dispatch list entries to raw values in call API, adds toast tests and translation keys, plus minor formatting and onboarding layout tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Screen as Call New Screen
participant SA as SafeAreaHook
participant SB as FocusAwareStatusBar
participant ToastHook as useToast
participant ToastStore as Toast Store
participant ToastC as ToastContainer
User->>Screen: open call screen
Screen->>SB: mount (edge-to-edge)
Screen->>SA: get insets
SA-->>Screen: {top,bottom}
Screen->>Screen: apply paddingTop/paddingBottom = max(inset,16)
User->>Screen: triggers validation/error
Screen->>ToastHook: toast.error(msg)
ToastHook->>ToastStore: showToast({type:'error', message})
ToastStore-->>ToastC: state change
ToastC->>ToastC: compute topOffset = insets.top + 70
ToastC->>User: render toast at safe offset
sequenceDiagram
autonumber
participant UI as Call Form
participant API as createCall()
participant Builder as Dispatch Builder
UI->>API: submit form with recipients
API->>Builder: build dispatch list
Builder-->>API: ["val1","val2"...] (raw values)
API->>API: join with "|", send payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/api/calls/calls.ts (1)
81-126: Critical: Inconsistent dispatch list formatting between createCall and updateCall.The
createCallfunction now pushes raw unprefixed values (lines 91, 95, 99, 103), butupdateCall(lines 137-146) still uses the prefixed format (U:,G:,R:). This inconsistency will cause the backend to parse dispatch lists differently depending on whether a call is created or updated, leading to incorrect dispatch routing or data corruption.Additionally, remove the commented-out code (lines 90, 94, 98, 102) to improve readability.
Apply this diff to align
createCallwithupdateCall(revert to prefixed format):if (callData.dispatchUsers) { - //dispatchEntries.push(...callData.dispatchUsers.map((user) => `U:${user}`)); - dispatchEntries.push(...callData.dispatchUsers); + dispatchEntries.push(...callData.dispatchUsers.map((user) => `U:${user}`)); } if (callData.dispatchGroups) { - //dispatchEntries.push(...callData.dispatchGroups.map((group) => `G:${group}`)); - dispatchEntries.push(...callData.dispatchGroups); + dispatchEntries.push(...callData.dispatchGroups.map((group) => `G:${group}`)); } if (callData.dispatchRoles) { - //dispatchEntries.push(...callData.dispatchRoles.map((role) => `R:${role}`)); - dispatchEntries.push(...callData.dispatchRoles); + dispatchEntries.push(...callData.dispatchRoles.map((role) => `R:${role}`)); } if (callData.dispatchUnits) { - //dispatchEntries.push(...callData.dispatchUnits.map((unit) => `U:${unit}`)); - dispatchEntries.push(...callData.dispatchUnits); + dispatchEntries.push(...callData.dispatchUnits.map((unit) => `U:${unit}`)); }OR, if the backend has been updated to accept unprefixed values, apply this diff to align
updateCallwithcreateCall:if (callData.dispatchUsers) { - dispatchEntries.push(...callData.dispatchUsers.map((user) => `U:${user}`)); + dispatchEntries.push(...callData.dispatchUsers); } if (callData.dispatchGroups) { - dispatchEntries.push(...callData.dispatchGroups.map((group) => `G:${group}`)); + dispatchEntries.push(...callData.dispatchGroups); } if (callData.dispatchRoles) { - dispatchEntries.push(...callData.dispatchRoles.map((role) => `R:${role}`)); + dispatchEntries.push(...callData.dispatchRoles); } if (callData.dispatchUnits) { - dispatchEntries.push(...callData.dispatchUnits.map((unit) => `U:${unit}`)); + dispatchEntries.push(...callData.dispatchUnits); }Verify which format the backend expects before merging.
src/app/call/new/index.tsx (1)
177-220: Validate priority and type before callingcreateCall. The handler currently falls back to0and''when lookups fail, which may send invalid data to the API; add guards to ensure bothpriority.Idandtype.Idare found (e.g. disable submit or show an error) instead of using default fallbacks.
🧹 Nitpick comments (2)
src/app/onboarding.tsx (1)
44-44: Consider moving inline styles to StyleSheet.create.Inline styles are re-allocated on every render. For static styles like
{ width, height: '100%' }and{ paddingHorizontal: 20 }, useStyleSheet.createto improve performance.Example refactor:
+import { Dimensions, Image, StyleSheet } from 'react-native'; + +const styles = StyleSheet.create({ + onboardingItem: { + width, + height: '100%', + }, + description: { + paddingHorizontal: 20, + }, +}); const OnboardingItem: React.FC<OnboardingItemProps> = ({ title, description, icon }) => { return ( - <View className="items-center justify-center px-8" style={{ width, height: '100%' }}> + <View className="items-center justify-center px-8" style={styles.onboardingItem}> <View className="mb-8 items-center justify-center">{icon}</View> <Text className="mb-6 text-center text-3xl font-bold text-gray-900 dark:text-white">{title}</Text> - <Text className="text-center text-lg leading-6 text-gray-600 dark:text-gray-300" style={{ paddingHorizontal: 20 }}> + <Text className="text-center text-lg leading-6 text-gray-600 dark:text-gray-300" style={styles.description}> {description} </Text> </View>Also applies to: 47-49
src/app/call/new/index.tsx (1)
2-2: Remove unused testing import.The
renderimport from@testing-library/react-nativeis not used in this component file. Testing utilities should only be imported in test files.Apply this diff to remove the unused import:
-import { render } from '@testing-library/react-native';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/android-tablet-safe-area-fix.md(1 hunks)expo-env.d.ts(1 hunks)jest-platform-setup.ts(1 hunks)src/api/calls/calls.ts(1 hunks)src/app/_layout.tsx(2 hunks)src/app/call/new/index.tsx(20 hunks)src/app/onboarding.tsx(3 hunks)src/components/toast/__tests__/toast-container.test.tsx(1 hunks)src/components/toast/toast-container.tsx(1 hunks)src/components/toast/toast.tsx(1 hunks)src/components/ui/bottomsheet/index.tsx(2 hunks)src/hooks/__tests__/use-toast.test.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{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/toast/__tests__/toast-container.test.tsxsrc/components/toast/toast-container.tsxsrc/app/call/new/index.tsxsrc/components/toast/toast.tsxexpo-env.d.tssrc/components/ui/bottomsheet/index.tsxjest-platform-setup.tssrc/app/onboarding.tsxsrc/hooks/__tests__/use-toast.test.tsxsrc/app/_layout.tsxsrc/api/calls/calls.ts
**/*.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/toast/__tests__/toast-container.test.tsxsrc/components/toast/toast-container.tsxsrc/app/call/new/index.tsxsrc/components/toast/toast.tsxsrc/components/ui/bottomsheet/index.tsxsrc/app/onboarding.tsxsrc/hooks/__tests__/use-toast.test.tsxsrc/app/_layout.tsx
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/components/toast/__tests__/toast-container.test.tsxsrc/components/toast/toast-container.tsxsrc/app/call/new/index.tsxsrc/components/toast/toast.tsxexpo-env.d.tssrc/components/ui/bottomsheet/index.tsxdocs/android-tablet-safe-area-fix.mdjest-platform-setup.tssrc/app/onboarding.tsxsrc/hooks/__tests__/use-toast.test.tsxsrc/app/_layout.tsxsrc/api/calls/calls.ts
**/*.{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/toast/__tests__/toast-container.test.tsxsrc/hooks/__tests__/use-toast.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/toast/__tests__/toast-container.test.tsxsrc/hooks/__tests__/use-toast.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/toast/__tests__/toast-container.test.tsxsrc/components/toast/toast-container.tsxsrc/app/call/new/index.tsxsrc/components/toast/toast.tsxsrc/components/ui/bottomsheet/index.tsxsrc/app/onboarding.tsxsrc/hooks/__tests__/use-toast.test.tsxsrc/app/_layout.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/components/toast/__tests__/toast-container.test.tsxsrc/hooks/__tests__/use-toast.test.tsx
src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Build and maintain shared Gluestack UI wrappers in src/components/ui
Files:
src/components/ui/bottomsheet/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#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/toast/__tests__/toast-container.test.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use getItemLayout for FlatList when items have consistent size
Applied to files:
src/app/onboarding.tsx
🧬 Code graph analysis (3)
src/components/toast/__tests__/toast-container.test.tsx (1)
src/components/toast/toast-container.tsx (1)
ToastContainer(9-24)
src/app/call/new/index.tsx (1)
src/components/ui/focus-aware-status-bar.tsx (1)
FocusAwareStatusBar(9-68)
src/app/_layout.tsx (1)
src/components/toast/toast-container.tsx (1)
ToastContainer(9-24)
🔇 Additional comments (18)
src/app/onboarding.tsx (1)
69-69: LGTM: Improved type safety.Replacing
anywith the properly typedFlatList<OnboardingItemProps>aligns with the guideline to avoidanyand use precise types.expo-env.d.ts (1)
3-3: EOF newline normalization – no issues.This is a formatting-only change adding a trailing newline. No functional impact.
jest-platform-setup.ts (1)
22-22: EOF newline normalization – no issues.This is a formatting-only change. The Platform mock setup remains functionally identical.
src/components/toast/toast.tsx (1)
19-19: Toast styling simplified correctly.The removal of
mx-4margin and type-specific styles is appropriate, as theToastContainernow handles positioning and spacing via safe-area insets and container-level padding.src/components/toast/toast-container.tsx (2)
11-15: Safe-area implementation follows best practices.The use of
useSafeAreaInsets()correctly handles dynamic insets across devices (notches, gesture navigation, etc.).Based on learnings.
15-15: No custom header height overrides detected; 70px offset is consistent
A search of all.tsx/.tsfiles found no instances ofheaderStyleorheaderHeight, so the hardcoded 70px offset applies uniformly across your navigation configurations.src/components/ui/bottomsheet/index.tsx (1)
45-46: Formatting-only changes – no issues.The spacing adjustments in the
BottomSheetContextdefault values and theonKeyDownhandler are formatting-only. No functional or API changes.Also applies to: 169-176
docs/android-tablet-safe-area-fix.md (1)
1-87: Well-documented fix for Android tablet safe area handling.This documentation clearly explains the problem, root cause, solution, and implementation details for the safe area handling fix. The examples and testing recommendations are helpful.
src/app/_layout.tsx (2)
25-25: LGTM - ToastContainer import added.The import is correctly placed with other component imports.
185-185: LGTM - ToastContainer properly positioned in render tree.The ToastContainer is appropriately placed within the BottomSheetModalProvider alongside other global UI components (LiveKitBottomSheet, PushNotificationModal, FlashMessage). The z-index and positioning logic is handled within the ToastContainer component itself using safe area insets.
src/app/call/new/index.tsx (8)
104-162: LGTM!Component initialization is clean. Safe area insets and toast hook are properly initialized, and the form setup follows React Hook Form best practices with proper zod validation.
274-326: LGTM!The address search handler is well-implemented with proper validation, error handling, and user feedback via toasts. The migration to the new toast API is correct.
357-406: LGTM!The what3words handler includes proper format validation using regex and comprehensive error handling. Toast migration is correct.
421-464: LGTM!The plus code handler is clean with appropriate error handling and toast notifications.
574-586: LGTM!Safe area handling is correctly implemented:
FocusAwareStatusBarenables edge-to-edge UI behaviorinsets.topandinsets.bottomare properly applied to ScrollView with sensible fallback minimums- This ensures the form content respects device safe areas on all Android tablets and iOS devices
Based on learnings.
857-872: LGTM!The dispatch selection card and bottom action bar are well-implemented. The safe area insets are correctly applied to the action bar's padding to prevent overlap with system UI elements.
877-916: LGTM!The modals and overlays (location picker, dispatch selection, address selection bottom sheet) are properly structured with appropriate z-indexing and state management.
479-558: No changes required:toast.info()is available in the toast API
Confirmed that theuseToasthook defines aninfomethod, so line 539’stoast.info()call is valid.
| describe('ToastContainer', () => { | ||
| const mockUseToastStore = useToastStore as jest.MockedFunction<typeof useToastStore>; | ||
|
|
||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('renders nothing when no toasts are present', () => { | ||
| mockUseToastStore.mockReturnValue([]); | ||
|
|
||
| const { queryByTestId } = render(<ToastContainer />); | ||
|
|
||
| expect(queryByTestId('toast-success')).toBeNull(); | ||
| expect(queryByTestId('toast-error')).toBeNull(); | ||
| }); | ||
|
|
||
| it('renders toasts when they are present in the store', () => { | ||
| const mockToasts = [ | ||
| { id: '1', type: 'success' as const, message: 'Success message' }, | ||
| { id: '2', type: 'error' as const, message: 'Error message', title: 'Error Title' }, | ||
| ]; | ||
|
|
||
| mockUseToastStore.mockReturnValue(mockToasts); | ||
|
|
||
| const { getByTestId } = render(<ToastContainer />); | ||
|
|
||
| expect(getByTestId('toast-success')).toBeTruthy(); | ||
| expect(getByTestId('toast-error')).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('renders toast with title and message correctly', () => { | ||
| const mockToasts = [ | ||
| { id: '1', type: 'warning' as const, message: 'Warning message', title: 'Warning Title' }, | ||
| ]; | ||
|
|
||
| mockUseToastStore.mockReturnValue(mockToasts); | ||
|
|
||
| const { getByTestId } = render(<ToastContainer />); | ||
|
|
||
| const toastElement = getByTestId('toast-warning'); | ||
| expect(toastElement.props.children).toEqual(['Warning Title: ', 'Warning message']); | ||
| }); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
Fix incorrect useToastStore mock implementation.
The tests mock useToastStore incorrectly. Zustand stores use a selector pattern: useToastStore((state) => state.toasts). The current mock returns arrays directly, but it should return a function that accepts a selector and returns the result.
Apply this diff to fix the mock implementation:
beforeEach(() => {
jest.clearAllMocks();
});
it('renders nothing when no toasts are present', () => {
- mockUseToastStore.mockReturnValue([]);
+ mockUseToastStore.mockImplementation((selector: any) =>
+ selector({ toasts: [] })
+ );
const { queryByTestId } = render(<ToastContainer />);
expect(queryByTestId('toast-success')).toBeNull();
expect(queryByTestId('toast-error')).toBeNull();
});
it('renders toasts when they are present in the store', () => {
const mockToasts = [
{ id: '1', type: 'success' as const, message: 'Success message' },
{ id: '2', type: 'error' as const, message: 'Error message', title: 'Error Title' },
];
- mockUseToastStore.mockReturnValue(mockToasts);
+ mockUseToastStore.mockImplementation((selector: any) =>
+ selector({ toasts: mockToasts })
+ );
const { getByTestId } = render(<ToastContainer />);
expect(getByTestId('toast-success')).toBeTruthy();
expect(getByTestId('toast-error')).toBeTruthy();
});
it('renders toast with title and message correctly', () => {
const mockToasts = [
{ id: '1', type: 'warning' as const, message: 'Warning message', title: 'Warning Title' },
];
- mockUseToastStore.mockReturnValue(mockToasts);
+ mockUseToastStore.mockImplementation((selector: any) =>
+ selector({ toasts: mockToasts })
+ );
const { getByTestId } = render(<ToastContainer />);
const toastElement = getByTestId('toast-warning');
expect(toastElement.props.children).toEqual(['Warning Title: ', 'Warning message']);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('ToastContainer', () => { | |
| const mockUseToastStore = useToastStore as jest.MockedFunction<typeof useToastStore>; | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| }); | |
| it('renders nothing when no toasts are present', () => { | |
| mockUseToastStore.mockReturnValue([]); | |
| const { queryByTestId } = render(<ToastContainer />); | |
| expect(queryByTestId('toast-success')).toBeNull(); | |
| expect(queryByTestId('toast-error')).toBeNull(); | |
| }); | |
| it('renders toasts when they are present in the store', () => { | |
| const mockToasts = [ | |
| { id: '1', type: 'success' as const, message: 'Success message' }, | |
| { id: '2', type: 'error' as const, message: 'Error message', title: 'Error Title' }, | |
| ]; | |
| mockUseToastStore.mockReturnValue(mockToasts); | |
| const { getByTestId } = render(<ToastContainer />); | |
| expect(getByTestId('toast-success')).toBeTruthy(); | |
| expect(getByTestId('toast-error')).toBeTruthy(); | |
| }); | |
| it('renders toast with title and message correctly', () => { | |
| const mockToasts = [ | |
| { id: '1', type: 'warning' as const, message: 'Warning message', title: 'Warning Title' }, | |
| ]; | |
| mockUseToastStore.mockReturnValue(mockToasts); | |
| const { getByTestId } = render(<ToastContainer />); | |
| const toastElement = getByTestId('toast-warning'); | |
| expect(toastElement.props.children).toEqual(['Warning Title: ', 'Warning message']); | |
| }); | |
| }); | |
| describe('ToastContainer', () => { | |
| const mockUseToastStore = useToastStore as jest.MockedFunction<typeof useToastStore>; | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| }); | |
| it('renders nothing when no toasts are present', () => { | |
| mockUseToastStore.mockImplementation((selector: any) => | |
| selector({ toasts: [] }) | |
| ); | |
| const { queryByTestId } = render(<ToastContainer />); | |
| expect(queryByTestId('toast-success')).toBeNull(); | |
| expect(queryByTestId('toast-error')).toBeNull(); | |
| }); | |
| it('renders toasts when they are present in the store', () => { | |
| const mockToasts = [ | |
| { id: '1', type: 'success' as const, message: 'Success message' }, | |
| { id: '2', type: 'error' as const, message: 'Error message', title: 'Error Title' }, | |
| ]; | |
| mockUseToastStore.mockImplementation((selector: any) => | |
| selector({ toasts: mockToasts }) | |
| ); | |
| const { getByTestId } = render(<ToastContainer />); | |
| expect(getByTestId('toast-success')).toBeTruthy(); | |
| expect(getByTestId('toast-error')).toBeTruthy(); | |
| }); | |
| it('renders toast with title and message correctly', () => { | |
| const mockToasts = [ | |
| { id: '1', type: 'warning' as const, message: 'Warning message', title: 'Warning Title' }, | |
| ]; | |
| mockUseToastStore.mockImplementation((selector: any) => | |
| selector({ toasts: mockToasts }) | |
| ); | |
| const { getByTestId } = render(<ToastContainer />); | |
| const toastElement = getByTestId('toast-warning'); | |
| expect(toastElement.props.children).toEqual(['Warning Title: ', 'Warning message']); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In src/components/toast/__tests__/toast-container.test.tsx around lines 36-78,
the tests mock useToastStore incorrectly by returning arrays directly; because
the real hook is used with a selector (useToastStore(s => s.toasts)), change
each mockUseToastStore.mockReturnValue(...) to return a function that accepts
the selector and invokes it with a fake state object, e.g.
mockUseToastStore.mockReturnValue(selector => selector({ toasts: [] })) for the
empty case and selector => selector({ toasts: mockToasts }) for the populated
cases so the selector receives a state shape matching the real store.
| describe('useToast', () => { | ||
| const mockShowToast = jest.fn(); | ||
| const mockUseToastStore = useToastStore as jest.MockedFunction<typeof useToastStore>; | ||
|
|
||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| mockUseToastStore.mockReturnValue({ | ||
| showToast: mockShowToast, | ||
| }); | ||
| }); | ||
|
|
||
| it('should provide show method that calls showToast', () => { | ||
| const { result } = renderHook(() => useToast()); | ||
|
|
||
| result.current.show('info', 'Test message', 'Test title'); | ||
|
|
||
| expect(mockShowToast).toHaveBeenCalledWith('info', 'Test message', 'Test title'); | ||
| }); | ||
|
|
||
| it('should provide success method that calls showToast with success type', () => { | ||
| const { result } = renderHook(() => useToast()); | ||
|
|
||
| result.current.success('Success message', 'Success title'); | ||
|
|
||
| expect(mockShowToast).toHaveBeenCalledWith('success', 'Success message', 'Success title'); | ||
| }); | ||
|
|
||
| it('should provide error method that calls showToast with error type', () => { | ||
| const { result } = renderHook(() => useToast()); | ||
|
|
||
| result.current.error('Error message', 'Error title'); | ||
|
|
||
| expect(mockShowToast).toHaveBeenCalledWith('error', 'Error message', 'Error title'); | ||
| }); | ||
|
|
||
| it('should provide warning method that calls showToast with warning type', () => { | ||
| const { result } = renderHook(() => useToast()); | ||
|
|
||
| result.current.warning('Warning message', 'Warning title'); | ||
|
|
||
| expect(mockShowToast).toHaveBeenCalledWith('warning', 'Warning message', 'Warning title'); | ||
| }); | ||
|
|
||
| it('should provide info method that calls showToast with info type', () => { | ||
| const { result } = renderHook(() => useToast()); | ||
|
|
||
| result.current.info('Info message', 'Info title'); | ||
|
|
||
| expect(mockShowToast).toHaveBeenCalledWith('info', 'Info message', 'Info title'); | ||
| }); | ||
|
|
||
| it('should work without title parameter', () => { | ||
| const { result } = renderHook(() => useToast()); | ||
|
|
||
| result.current.success('Success message'); | ||
|
|
||
| expect(mockShowToast).toHaveBeenCalledWith('success', 'Success message', undefined); | ||
| }); | ||
| }); No newline at end of file |
There was a problem hiding this comment.
Fix incorrect useToastStore mock implementation.
Similar to the ToastContainer tests, the useToastStore mock should implement the Zustand selector pattern. The current mock returns an object directly, but Zustand stores expect a selector function.
Apply this diff to fix the mock:
beforeEach(() => {
jest.clearAllMocks();
- mockUseToastStore.mockReturnValue({
- showToast: mockShowToast,
- });
+ mockUseToastStore.mockImplementation((selector: any) =>
+ selector({ showToast: mockShowToast })
+ );
});The test cases themselves are comprehensive and well-structured, covering all public methods of the useToast hook.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('useToast', () => { | |
| const mockShowToast = jest.fn(); | |
| const mockUseToastStore = useToastStore as jest.MockedFunction<typeof useToastStore>; | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| mockUseToastStore.mockReturnValue({ | |
| showToast: mockShowToast, | |
| }); | |
| }); | |
| it('should provide show method that calls showToast', () => { | |
| const { result } = renderHook(() => useToast()); | |
| result.current.show('info', 'Test message', 'Test title'); | |
| expect(mockShowToast).toHaveBeenCalledWith('info', 'Test message', 'Test title'); | |
| }); | |
| it('should provide success method that calls showToast with success type', () => { | |
| const { result } = renderHook(() => useToast()); | |
| result.current.success('Success message', 'Success title'); | |
| expect(mockShowToast).toHaveBeenCalledWith('success', 'Success message', 'Success title'); | |
| }); | |
| it('should provide error method that calls showToast with error type', () => { | |
| const { result } = renderHook(() => useToast()); | |
| result.current.error('Error message', 'Error title'); | |
| expect(mockShowToast).toHaveBeenCalledWith('error', 'Error message', 'Error title'); | |
| }); | |
| it('should provide warning method that calls showToast with warning type', () => { | |
| const { result } = renderHook(() => useToast()); | |
| result.current.warning('Warning message', 'Warning title'); | |
| expect(mockShowToast).toHaveBeenCalledWith('warning', 'Warning message', 'Warning title'); | |
| }); | |
| it('should provide info method that calls showToast with info type', () => { | |
| const { result } = renderHook(() => useToast()); | |
| result.current.info('Info message', 'Info title'); | |
| expect(mockShowToast).toHaveBeenCalledWith('info', 'Info message', 'Info title'); | |
| }); | |
| it('should work without title parameter', () => { | |
| const { result } = renderHook(() => useToast()); | |
| result.current.success('Success message'); | |
| expect(mockShowToast).toHaveBeenCalledWith('success', 'Success message', undefined); | |
| }); | |
| }); | |
| describe('useToast', () => { | |
| const mockShowToast = jest.fn(); | |
| const mockUseToastStore = useToastStore as jest.MockedFunction<typeof useToastStore>; | |
| beforeEach(() => { | |
| jest.clearAllMocks(); | |
| mockUseToastStore.mockImplementation((selector: any) => | |
| selector({ showToast: mockShowToast }) | |
| ); | |
| }); | |
| it('should provide show method that calls showToast', () => { | |
| const { result } = renderHook(() => useToast()); | |
| result.current.show('info', 'Test message', 'Test title'); | |
| expect(mockShowToast).toHaveBeenCalledWith('info', 'Test message', 'Test title'); | |
| }); | |
| // ...rest of tests unchanged | |
| }); |
🤖 Prompt for AI Agents
In src/hooks/__tests__/use-toast.test.tsx around lines 11-69, the test currently
returns a plain object from the useToastStore mock, but Zustand stores receive a
selector function; update the mock to implement the selector pattern by making
mockUseToastStore a jest mock implementation that accepts a selector function
and invokes it with an object containing showToast: mockShowToast (so callers
get the selected value), and adjust beforeEach to set that implementation
instead of returning the object directly; no other test changes required.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/api/calls/calls.ts (1)
136-151: Consider extracting dispatch list logic to reduce duplication.The dispatch list building logic is identical between
createCallandupdateCall. Consider extracting this into a shared helper function to follow DRY principles.Example refactor:
// Add this helper function before createCall const buildDispatchList = (callData: { dispatchEveryone?: boolean; dispatchUsers?: string[]; dispatchGroups?: string[]; dispatchRoles?: string[]; dispatchUnits?: string[]; }): string => { if (callData.dispatchEveryone) { return '0'; } const dispatchEntries: string[] = []; if (callData.dispatchUsers) { dispatchEntries.push(...callData.dispatchUsers); } if (callData.dispatchGroups) { dispatchEntries.push(...callData.dispatchGroups); } if (callData.dispatchRoles) { dispatchEntries.push(...callData.dispatchRoles); } if (callData.dispatchUnits) { dispatchEntries.push(...callData.dispatchUnits); } return dispatchEntries.join('|'); };Then use it in both functions:
export const createCall = async (callData: CreateCallRequest) => { - let dispatchList = ''; - - if (callData.dispatchEveryone) { - dispatchList = '0'; - } else { - const dispatchEntries: string[] = []; - - if (callData.dispatchUsers) { - dispatchEntries.push(...callData.dispatchUsers); - } - if (callData.dispatchGroups) { - dispatchEntries.push(...callData.dispatchGroups); - } - if (callData.dispatchRoles) { - dispatchEntries.push(...callData.dispatchRoles); - } - if (callData.dispatchUnits) { - dispatchEntries.push(...callData.dispatchUnits); - } - - dispatchList = dispatchEntries.join('|'); - } + const dispatchList = buildDispatchList(callData); const data = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/api/calls/calls.ts(2 hunks)src/app/call/new/index.tsx(21 hunks)src/app/onboarding.tsx(3 hunks)src/translations/ar.json(1 hunks)src/translations/en.json(1 hunks)src/translations/es.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/translations/es.json
🧰 Additional context used
📓 Path-based instructions (6)
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/translations/en.jsonsrc/translations/ar.jsonsrc/app/onboarding.tsxsrc/app/call/new/index.tsxsrc/api/calls/calls.ts
src/translations/**
📄 CodeRabbit inference engine (.cursorrules)
Store translation dictionaries in src/translations
Files:
src/translations/en.jsonsrc/translations/ar.json
src/translations/**/*.{json,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store translation dictionaries in src/translations
Files:
src/translations/en.jsonsrc/translations/ar.json
**/*.{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/app/onboarding.tsxsrc/app/call/new/index.tsxsrc/api/calls/calls.ts
**/*.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/app/onboarding.tsxsrc/app/call/new/index.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/app/onboarding.tsxsrc/app/call/new/index.tsx
🧬 Code graph analysis (1)
src/app/call/new/index.tsx (2)
src/api/calls/calls.ts (1)
createCall(81-126)src/components/ui/focus-aware-status-bar.tsx (1)
FocusAwareStatusBar(9-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (9)
src/app/onboarding.tsx (2)
69-69: LGTM! Improved type safety.Changing the ref type from
anytoFlatList<OnboardingItemProps>aligns with the coding guidelines to avoidanyand use precise TypeScript types.
113-127: Good fix for responsive layout.Wrapping the FlatList in a
flex-1container and applyingclassName="flex-1"to the FlatList itself removes the previous fixed height constraint. This addresses the earlier concern about fixed height not adapting to different screen sizes.Based on learnings
src/translations/en.json (1)
234-235: LGTM!The new validation messages are clear, actionable, and consistent with the existing translation patterns.
src/translations/ar.json (1)
234-235: LGTM!The Arabic translations are correctly placed and formatted, providing equivalent validation messages for the call creation flow.
src/app/call/new/index.tsx (4)
12-12: LGTM!Safe area insets and toast hook are correctly imported and initialized following React best practices.
Also applies to: 24-24, 31-31, 107-107, 111-111
220-220: LGTM!The toast API migration is complete and consistent throughout the file. All toast messages are properly internationalized and use appropriate severity levels (success, error, warning, info).
Also applies to: 228-228, 286-286, 318-318, 326-326, 332-332, 351-351, 369-369, 376-376, 403-403, 406-406, 412-412, 433-433, 461-461, 464-464, 470-470, 491-491, 500-500, 509-509, 537-537, 549-549, 564-564
586-586: LGTM!Safe area handling is correctly implemented using
useSafeAreaInsetswith appropriate fallback values. TheFocusAwareStatusBarprovides edge-to-edge layout, and padding is applied to both the scroll content and bottom action bar to respect device safe areas.Based on learnings
Also applies to: 596-596, 874-874
204-205: LGTM!Using
priority.Idandtype.Idinstead of names is correct for the API call and aligns with the validation logic that resolves these objects before submission.src/api/calls/calls.ts (1)
89-104: Ensure DispatchList payload matches backend requirements.The code now omits prefixes (e.g.
U:,G:,R:) from dispatch entries in src/api/calls/calls.ts, altering the API contract. Confirm with the backend team or documentation that raw IDs are supported, add or update integration tests to validate the new format, and remove the commented-out legacy prefix code once confirmed.
| // Validate priority and type before proceeding | ||
| const priority = callPriorities.find((p) => p.Name === data.priority); | ||
| const type = callTypes.find((t) => t.Name === data.type); | ||
|
|
||
| if (!priority) { | ||
| toast.error(t('calls.invalid_priority')); | ||
| return; | ||
| } | ||
|
|
||
| if (!type) { | ||
| toast.error(t('calls.invalid_type')); | ||
| return; | ||
| } | ||
|
|
||
| console.log('Creating new call with data:', data); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consider removing or replacing console.log for production.
The validation logic is excellent and prevents invalid API calls. However, the console.log at line 199 may expose sensitive call data in production environments.
Consider one of these approaches:
Option 1: Remove the log entirely
- console.log('Creating new call with data:', data);Option 2: Use a conditional logger (if you have one)
- console.log('Creating new call with data:', data);
+ if (__DEV__) {
+ console.log('Creating new call with data:', data);
+ }Option 3: Use a proper logging framework
- console.log('Creating new call with data:', data);
+ logger.debug('Creating new call', { callName: data.name, priority: data.priority });🤖 Prompt for AI Agents
In src/app/call/new/index.tsx around lines 185 to 199, there's a
console.log('Creating new call with data:', data) that can leak sensitive call
data in production; remove this console.log or replace it with a non-production
debug-level log. If your codebase has a logger, change it to
logger.debug('Creating new call', { maskedData }) and ensure sensitive fields
are masked; otherwise wrap the log in a runtime check (e.g., only log when
NODE_ENV !== 'production' or a DEBUG flag) so no raw call data is emitted in
production.
| <Text className="mb-6 text-center text-3xl font-bold text-gray-900 dark:text-white">{title}</Text> | ||
| <Text className="text-center text-lg leading-6 text-gray-600 dark:text-gray-300" style={{ paddingHorizontal: 20 }}> | ||
| {description} | ||
| </Text> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap user-facing text with i18n translation function.
Per coding guidelines, all user-facing text should be wrapped in t() from react-i18next. The title and description props are rendered directly without translation support.
Consider updating the component to accept translation keys or wrap the rendered text:
- <Text className="mb-6 text-center text-3xl font-bold text-gray-900 dark:text-white">{title}</Text>
- <Text className="text-center text-lg leading-6 text-gray-600 dark:text-gray-300" style={{ paddingHorizontal: 20 }}>
- {description}
- </Text>
+ <Text className="mb-6 text-center text-3xl font-bold text-gray-900 dark:text-white">{t(title)}</Text>
+ <Text className="text-center text-lg leading-6 text-gray-600 dark:text-gray-300" style={{ paddingHorizontal: 20 }}>
+ {t(description)}
+ </Text>Alternatively, pass pre-translated strings from the parent component if onboardingData is already internationalized.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/onboarding.tsx around lines 46 to 49, the title and description are
rendered directly without i18n translation; update the component to call the
react-i18next t() function when rendering these props (e.g., import and use t to
render t(title) and t(description)), or alternatively change the prop
types/documentation to require pre-translated strings and ensure callers pass
translated text; make sure to import useTranslation from react-i18next and
handle missing keys gracefully.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Summary by CodeRabbit