CU-868f7hkrj Minor UI fixes for Light mode, security fix and testing … #163
Conversation
WalkthroughAdds light/dark map theming with runtime style switching and theme-aware analytics; gates call-creation UI and call-detail menu by security permissions and active-unit presence; refactors status bottom-sheet to use BColor/invertColor; updates tab bar visuals; and expands tests across maps, calls, call-detail, status sheet, and security store. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppMap as Map Screen
participant Theme as useColorScheme
participant Mapbox
participant Analytics
User->>AppMap: open map
AppMap->>Theme: read colorScheme
Theme-->>AppMap: 'light' or 'dark'
AppMap->>Mapbox: apply styleURL (getMapStyle)
AppMap->>Analytics: track('map_view_rendered', { theme })
Theme-->>AppMap: theme change
AppMap->>Mapbox: re-apply updated styleURL
sequenceDiagram
participant User
participant Calls as CallsScreen
participant Security as useSecurityStore
participant Router
Calls->>Security: read canUserCreateCalls
alt permitted
Calls-->>User: render FAB
User->>Calls: tap FAB
Calls->>Router: push('/calls/create')
else denied
Calls-->>User: FAB hidden
end
sequenceDiagram
participant User
participant CallDetail
participant Security as useSecurityStore
participant Core as useCoreStore
participant Menu as useCallDetailMenu
CallDetail->>Security: read canUserCreateCalls
CallDetail->>Core: read activeCall, activeUnit
CallDetail->>Menu: init({ canUserCreateCalls })
alt canUserCreateCalls
User->>CallDetail: open header menu
CallDetail->>Menu: open action sheet
else denied
CallDetail-->>User: menu hidden
end
alt activeUnit present and call not active
CallDetail-->>User: show "Set Active" button
else
CallDetail-->>User: hide "Set Active"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
src/components/maps/static-map.tsx (1)
26-33: Fix falsy check for coordinates to avoid incorrectly treating (0, 0) as “no location”0 is a valid coordinate component but is falsy in JS. The current check hides valid locations at the equator/prime meridian.
Apply this diff:
- if (!latitude || !longitude) { + if (!Number.isFinite(latitude) || !Number.isFinite(longitude)) { return ( <Box style={[styles.container, { height }]} className="items-center justify-center bg-gray-200"> <Text className="text-gray-500">{t('call_detail.no_location')}</Text> </Box> ); }src/app/call/[id].tsx (1)
125-131: Coerce canUserCreateCalls to boolean when passing to the menu hook to avoid permissive default.useSecurityStore returns boolean | undefined. The hook currently defaults undefined to allowed (see call-detail-menu). Until the hook is hardened, pass a strict boolean to ensure least-privilege by default.
Apply this diff:
- const { HeaderRightMenu, CallDetailActionSheet } = useCallDetailMenu({ + const { HeaderRightMenu, CallDetailActionSheet } = useCallDetailMenu({ onEditCall: handleEditCall, onCloseCall: handleCloseCall, - canUserCreateCalls, + canUserCreateCalls: Boolean(canUserCreateCalls), });src/components/status/status-bottom-sheet.tsx (1)
467-490: Accessibility: secondary texts need contrast-aware color on colored backgrounds.You set backgroundColor to status.BColor while only the primary status text uses invertColor. The description lines below still use fixed gray classes, which can be unreadable on dark or saturated BColor backgrounds. Apply invertColor for these lines too.
Apply this diff:
- {status.Detail > 0 && ( - <Text className="text-sm text-gray-600 dark:text-gray-400"> + {status.Detail > 0 && ( + <Text + className="text-sm" + style={{ color: invertColor(status.BColor || '#ffffff', true) }} + > {status.Detail === 1 && t('status.station_destination_enabled')} {status.Detail === 2 && t('status.call_destination_enabled')} {status.Detail === 3 && t('status.both_destinations_enabled')} </Text> )} - {status.Note > 0 && ( - <Text className="text-xs text-gray-500 dark:text-gray-500"> + {status.Note > 0 && ( + <Text + className="text-xs" + style={{ color: invertColor(status.BColor || '#ffffff', true) }} + > {status.Note === 1 && t('status.note_optional')} {status.Note === 2 && t('status.note_required')} </Text> )}
🧹 Nitpick comments (34)
src/app/(app)/_layout.tsx (2)
268-277: Make tab bar border/shadow theme-aware instead of hard-coded blackHard-coded black-based values can look heavy in dark mode. Prefer using theme tokens or dynamic values based on the active color scheme for borderTopColor and shadowColor to ensure consistent appearance in both themes.
Example approach:
- Derive colors from the theme (gluestack tokens) or via useColorScheme outside and pass via screenOptions as a function.
- For dark mode, consider a lighter shadowColor (e.g., white with low opacity) instead of pure black.
353-361: Avoid nested NovuProviders to prevent duplicate socket connectionsThis component already conditionally wraps the whole content in a NovuProvider (Lines 353-361). The CreateNotificationButton also wraps NotificationButton with another NovuProvider (Lines 404-407). Nesting providers risks duplicate connections and unnecessary re-renders.
Refactor so only a single, top-level NovuProvider is used. Pass down any needed context to NotificationButton without re-wrapping it.
Also applies to: 404-407
src/stores/security/store.ts (1)
43-50: Normalize exposed booleans to strict boolean types for consistencyCurrently, isUserDepartmentAdmin/canUserCreate* may be undefined. Returning strict booleans from the store simplifies consumers and avoids tri-state logic.
Suggested diff:
return { getRights: store.getRights, - isUserDepartmentAdmin: store.rights?.IsAdmin, - isUserGroupAdmin: (groupId: number) => store.rights?.Groups?.some((right) => right.GroupId === groupId && right.IsGroupAdmin) ?? false, - canUserCreateCalls: store.rights?.CanCreateCalls, - canUserCreateNotes: store.rights?.CanAddNote, - canUserCreateMessages: store.rights?.CanCreateMessage, - canUserViewPII: store.rights?.CanViewPII, + isUserDepartmentAdmin: !!store.rights?.IsAdmin, + isUserGroupAdmin: (groupId: number) => + Boolean(store.rights?.Groups?.some((right) => right.GroupId === groupId && right.IsGroupAdmin)), + canUserCreateCalls: !!store.rights?.CanCreateCalls, + canUserCreateNotes: !!store.rights?.CanAddNote, + canUserCreateMessages: !!store.rights?.CanCreateMessage, + canUserViewPII: !!store.rights?.CanViewPII, departmentCode: store.rights?.DepartmentCode, };src/app/(app)/index.tsx (3)
55-61: Simplify map style state to a string and avoid unnecessary object allocationsStoring styleURL as an object causes extra allocations and noisy access (styleURL.styleURL). A plain string state suffices.
Apply this diff:
- const [styleURL, setStyleURL] = useState({ styleURL: getMapStyle() }); + const [styleURL, setStyleURL] = useState(getMapStyle());- const newStyle = getMapStyle(); - setStyleURL({ styleURL: newStyle }); + const newStyle = getMapStyle(); + setStyleURL(newStyle);Additionally, update MapView usage outside this hunk:
- styleURL={styleURL.styleURL} + styleURL={styleURL}Also applies to: 65-70
296-339: Use useMemo for themed styles instead of creating new objects per rendergetThemedStyles() returns new objects every render, defeating the benefit of useCallback. useMemo stabilizes references and avoids unnecessary recalculations.
Apply this diff:
- // Create dynamic styles based on theme - const getThemedStyles = useCallback(() => { - const isDark = colorScheme === 'dark'; - return { - markerInnerContainer: { - width: 24, - height: 24, - alignItems: 'center' as const, - justifyContent: 'center' as const, - backgroundColor: '#3b82f6', - borderRadius: 12, - borderWidth: 3, - borderColor: isDark ? '#1f2937' : '#ffffff', - elevation: 5, - shadowColor: isDark ? '#ffffff' : '#000000', - shadowOffset: { - width: 0, - height: 2, - }, - shadowOpacity: isDark ? 0.1 : 0.25, - shadowRadius: 3.84, - }, - recenterButton: { - position: 'absolute' as const, - bottom: 20, - right: 20, - width: 48, - height: 48, - borderRadius: 24, - backgroundColor: '#3b82f6', - justifyContent: 'center' as const, - alignItems: 'center' as const, - elevation: 5, - shadowColor: isDark ? '#ffffff' : '#000000', - shadowOffset: { - width: 0, - height: 2, - }, - shadowOpacity: isDark ? 0.1 : 0.25, - shadowRadius: 3.84, - }, - }; - }, [colorScheme]); - - const themedStyles = getThemedStyles(); + // Create dynamic styles based on theme + const themedStyles = React.useMemo(() => { + const isDark = colorScheme === 'dark'; + return { + markerInnerContainer: { + width: 24, + height: 24, + alignItems: 'center' as const, + justifyContent: 'center' as const, + backgroundColor: '#3b82f6', + borderRadius: 12, + borderWidth: 3, + borderColor: isDark ? '#1f2937' : '#ffffff', + elevation: 5, + shadowColor: isDark ? '#ffffff' : '#000000', + shadowOffset: { width: 0, height: 2 }, + shadowOpacity: isDark ? 0.1 : 0.25, + shadowRadius: 3.84, + }, + recenterButton: { + position: 'absolute' as const, + bottom: 20, + right: 20, + width: 48, + height: 48, + borderRadius: 24, + backgroundColor: '#3b82f6', + justifyContent: 'center' as const, + alignItems: 'center' as const, + elevation: 5, + shadowColor: isDark ? '#ffffff' : '#000000', + shadowOffset: { width: 0, height: 2 }, + shadowOpacity: isDark ? 0.1 : 0.25, + shadowRadius: 3.84, + }, + }; + }, [colorScheme]);
404-409: Follow guideline: prefer conditional operator over && for renderingThe coding guidelines specify using the conditional operator (?:) instead of &&.
Apply this diff:
- {showRecenterButton && ( - <TouchableOpacity style={[styles.recenterButton, themedStyles.recenterButton]} onPress={handleRecenterMap} testID="recenter-button"> - <NavigationIcon size={20} color="#ffffff" /> - </TouchableOpacity> - )} + {showRecenterButton ? ( + <TouchableOpacity style={[styles.recenterButton, themedStyles.recenterButton]} onPress={handleRecenterMap} testID="recenter-button"> + <NavigationIcon size={20} color="#ffffff" /> + </TouchableOpacity> + ) : null}src/app/(app)/calls.tsx (1)
106-108: Add accessibility label to the FABImprove accessibility by adding an accessibilityLabel (localized) and role.
Apply this diff:
- <Fab placement="bottom right" size="lg" onPress={handleNewCall} testID="new-call-fab"> + <Fab + placement="bottom right" + size="lg" + onPress={handleNewCall} + accessibilityRole="button" + accessibilityLabel={t('calls.create_new_call')} + testID="new-call-fab" + > <FabIcon as={PlusIcon} size="lg" /> </Fab>Note: Ensure the i18n key calls.create_new_call exists; otherwise, add it to the translations.
src/components/maps/static-map.tsx (1)
1-5: Optional: Extract a shared getMapStyleForScheme utility to avoid duplicationBoth this component and the main Map page compute the map style from the theme. Centralize this logic to a small helper (e.g., src/lib/map-theme.ts) to stay DRY.
Example:
// src/lib/map-theme.ts import Mapbox from '@rnmapbox/maps'; export const getMapStyleForScheme = (scheme: 'light' | 'dark' | null | undefined) => scheme === 'dark' ? Mapbox.StyleURL.Dark : Mapbox.StyleURL.Street;Then use:
const mapStyle = getMapStyleForScheme(colorScheme);src/components/status/__tests__/status-bottom-sheet.test.tsx (1)
2519-2636: Strengthen assertions: verify backgroundColor and text color instead of only presence.These new tests validate rendering but stop short of asserting the color behavior added in the component (BColor background and invertColor-driven text). You can make them more robust by asserting style props of the container and text.
Apply this diff to add concrete assertions in the new tests:
@@ it('should use BColor for background and invertColor for text color in status selection', () => { @@ - // Check that the status text is present - expect(screen.getByText('Available')).toBeTruthy(); + // Status text should be present + const statusText = screen.getByText('Available'); + expect(statusText).toBeTruthy(); + + // Assert the container background uses BColor + // Structure: TouchableOpacity > HStack > VStack > Text + const statusContainer = statusText.parent?.parent?.parent as any; + const styleArray = Array.isArray(statusContainer?.props?.style) ? statusContainer.props.style : [statusContainer?.props?.style]; + expect(styleArray).toEqual(expect.arrayContaining([expect.objectContaining({ backgroundColor: '#28a745' })])); + + // Assert text color is the inverted color for contrast (for #28a745, invertColor(..., true) => '#FFFFFF') + const textStyleArray = Array.isArray(statusText.props.style) ? statusText.props.style : [statusText.props.style]; + expect(textStyleArray).toEqual(expect.arrayContaining([expect.objectContaining({ color: '#FFFFFF' })])); @@ it('should use BColor for background in selected status display on note step', () => { @@ - expect(screen.getByText('Selected Status:')).toBeTruthy(); - expect(screen.getByText('Responding')).toBeTruthy(); + const label = screen.getByText('Selected Status:'); + const value = screen.getByText('Responding'); + expect(label).toBeTruthy(); + expect(value).toBeTruthy(); + + // Container background should use BColor + const pillContainer = value.parent?.parent as any; + const pillStyle = Array.isArray(pillContainer?.props?.style) ? pillContainer.props.style : [pillContainer?.props?.style]; + expect(pillStyle).toEqual(expect.arrayContaining([expect.objectContaining({ backgroundColor: '#ffc107' })])); + + // For #ffc107, invertColor(..., true) => '#000000' + const valueStyle = Array.isArray(value.props.style) ? value.props.style : [value.props.style]; + expect(valueStyle).toEqual(expect.arrayContaining([expect.objectContaining({ color: '#000000' })])); @@ it('should handle status without BColor gracefully', () => { @@ - // Should still render the status text - expect(screen.getByText('Emergency')).toBeTruthy(); + const statusText = screen.getByText('Emergency'); + expect(statusText).toBeTruthy(); + // Background should fall back (no hard assertion on color, but ensure no style crash) + const container = statusText.parent?.parent?.parent as any; + expect(container?.props?.style).toBeDefined();src/components/status/status-bottom-sheet.tsx (1)
467-471: Dark mode fallback for unselected items may look off against dark surfaces.Unselected items use a hardcoded white background. Consider aligning the fallback with theme, e.g., white for light mode and gray-800/900 for dark mode, to blend better.
Example:
- style={{ - backgroundColor: status.BColor || (selectedStatus?.Id.toString() === status.Id.toString() ? '#dbeafe' : '#ffffff'), - }} + style={{ + backgroundColor: + status.BColor || + (selectedStatus?.Id.toString() === status.Id.toString() + ? '#dbeafe' + : colorScheme === 'dark' ? '#1f2937' /* gray-800 */ : '#ffffff'), + }}src/components/calls/call-detail-menu.tsx (3)
24-29: Coerce analytics flags to boolean.Avoid sending undefined to analytics; cast to boolean for consistency.
Apply this diff:
- trackEvent('call_detail_menu_opened', { - hasEditAction: canUserCreateCalls, - hasCloseAction: canUserCreateCalls, - }); + trackEvent('call_detail_menu_opened', { + hasEditAction: !!canUserCreateCalls, + hasCloseAction: !!canUserCreateCalls, + });
36-47: Prefer onPress over onPressIn for the kebab menu.onPressIn fires earlier and can feel jumpy or trigger on unintended touches. onPress is the conventional, accessible trigger for buttons.
Apply this diff:
- return ( - <Pressable onPressIn={openMenu} testID="kebab-menu-button" className="rounded p-2"> + return ( + <Pressable onPress={openMenu} testID="kebab-menu-button" className="rounded p-2"> <MoreVerticalIcon size={24} className="text-gray-700 dark:text-gray-300" /> </Pressable> );
16-16: Optionally make canUserCreateCalls a required boolean to tighten the API.This avoids surprising defaults at the hook boundary and forces callers to opt-in explicitly.
Apply this diff:
-interface CallDetailMenuProps { +interface CallDetailMenuProps { onEditCall: () => void; onCloseCall: () => void; - canUserCreateCalls?: boolean; + canUserCreateCalls: boolean; }And ensure callers pass a boolean, e.g.,
canUserCreateCalls: Boolean(canUserCreateCalls).src/stores/security/__tests__/store.test.ts (1)
110-124: Error path expectation is appropriateConfirming rights remain null on API failure matches the current store implementation. If you later decide to surface error state (e.g., set error in the catch), you can extend this test to assert that too.
src/app/call/__tests__/[id].test.tsx (3)
28-50: Stabilize Button testID to accept explicit overridesDeriving testID from rendered children is brittle (translations, copy changes, or icon-only content can break selectors). Prefer honoring an explicit testID prop when provided and fallback to the derived one only when absent.
jest.mock('@/components/ui/button', () => ({ - Button: jest.fn().mockImplementation(({ children, onPress, disabled, ...props }) => { + Button: jest.fn().mockImplementation(({ children, onPress, disabled, testID, ...props }) => { const React = require('react'); return React.createElement('button', { onPress, onClick: onPress, // For web compatibility disabled, accessibilityRole: 'button', accessibilityLabel: React.Children.toArray(children).map((child: any) => typeof child === 'string' ? child : child?.props?.children || '' ).join(' '), - testID: `button-${React.Children.toArray(children).map((child: any) => - typeof child === 'string' ? child : - child?.props?.children || '' - ).join(' ').toLowerCase().replace(/\s+/g, '-')}`, + testID: testID ?? `button-${React.Children.toArray(children).map((child: any) => + typeof child === 'string' ? child : + child?.props?.children || '' + ).join(' ').toLowerCase().replace(/\s+/g, '-')}`, ...props }, children); }), ButtonIcon: jest.fn().mockImplementation(() => null), ButtonText: jest.fn().mockImplementation(({ children }) => children), }));
619-627: Remove debug() to keep test output clean and fasterThe debug() call is noisy and slows CI logs. Drop it once the test is stable.
- const { getByText, getAllByText, debug, getByTestId } = render(<CallDetail />); - - // Debug the rendered elements - debug(); + const { getByTestId } = render(<CallDetail />);
720-723: Reset useCoreStore.getState overrides to avoid cross-test leakageYou’re mutating useCoreStore.getState directly in multiple tests. While you clear mocks, the reassignment persists. Capture the original and restore it in an afterEach within this describe block.
describe('Set Active functionality', () => { + const originalGetState = useCoreStore.getState; + afterEach(() => { + useCoreStore.getState = originalGetState; + });Also applies to: 787-790, 852-855, 911-914, 971-974
src/app/(app)/__tests__/index.test.tsx (2)
272-293: Fix analytics mocking; jest.doMock here is too late to affect already-imported modulesdoMock won’t rewire Map since it's already imported. Wrap the import in isolateModules and perform the mock before requiring the component so theme analytics can be asserted.
- it('should track analytics with theme information', async () => { - const mockTrackEvent = jest.fn(); - - // We need to mock the useAnalytics hook - jest.doMock('@/hooks/use-analytics', () => ({ - useAnalytics: () => ({ trackEvent: mockTrackEvent }), - })); - - mockUseColorScheme.mockReturnValue({ - colorScheme: 'dark', - setColorScheme: jest.fn(), - toggleColorScheme: jest.fn(), - }); - - render(<Map />); - - await waitFor(() => { - expect(mockLocationService.startLocationUpdates).toHaveBeenCalled(); - }); - - // Note: The analytics tracking is tested indirectly since we can't easily mock it in this setup - }); + it('should track analytics with theme information', async () => { + const mockTrackEvent = jest.fn(); + + mockUseColorScheme.mockReturnValue({ + colorScheme: 'dark', + setColorScheme: jest.fn(), + toggleColorScheme: jest.fn(), + }); + + jest.isolateModules(() => { + jest.doMock('@/hooks/use-analytics', () => ({ + useAnalytics: () => ({ trackEvent: mockTrackEvent }), + })); + const MapIsolated = require('../index').default; + render(<MapIsolated />); + }); + + await waitFor(() => { + expect(mockLocationService.startLocationUpdates).toHaveBeenCalled(); + }); + + expect(mockTrackEvent).toHaveBeenCalled(); + const [, props] = mockTrackEvent.mock.calls[0] ?? []; + expect(props?.theme).toBe('dark'); + });
31-45: Optional: Capture styleURL to actually assert light/dark map stylesRight now the theme tests only check that nothing crashes. If you want to hard-assert the selected Mapbox style, expose MapView as a mock component that records props (e.g., lastProps). Then assert lastProps.styleURL against StyleURL.Dark/Light. If helpful, I can provide a patch.
src/app/(app)/__tests__/calls.test.tsx (4)
200-209: Assert FAB presence explicitly instead of smoke-checkingSwitch from truthy JSON to an explicit presence check for the FAB.
- it('renders the new call FAB button', () => { - const tree = render(<CallsScreen />); - - // Check if the component renders without crashing and FAB is present - const htmlContent = tree.toJSON(); - expect(htmlContent).toBeTruthy(); - - // Since we can see the button in debug output, let's just verify the mock is working - expect(mockSecurityStore.canUserCreateCalls).toBe(true); - }); + it('renders the new call FAB button', () => { + const { getByTestId } = render(<CallsScreen />); + expect(getByTestId('fab-button')).toBeTruthy(); + });
211-221: Actually press the FAB and assert navigationVerify interaction by firing a press and asserting router.push was called.
- it('navigates to new call screen when FAB is pressed', () => { - // Since we can see the FAB button in the HTML output but can't query it, - // let's test the navigation logic by directly calling the onPress handler - render(<CallsScreen />); - - // Verify that the router push function exists (it will be called when FAB is pressed) - expect(router.push).toBeDefined(); - - // The test should pass if the component renders without errors - expect(true).toBe(true); - }); + it('navigates to new call screen when FAB is pressed', () => { + const { getByTestId } = render(<CallsScreen />); + const fab = getByTestId('fab-button'); + // RN RTL maps press to onPress; our mock also wires onClick + fireEvent.press?.(fab); + fireEvent.click?.(fab); + expect(router.push).toHaveBeenCalled(); + });
230-237: Assert FAB absence deterministically when permission is missingQuery the specific FAB testID to avoid brittle role-count checks.
- it('does not render the new call FAB button', () => { - render(<CallsScreen />); - // With the mock structure, when canUserCreateCalls is false, the FAB should not render - const buttons = screen.queryAllByRole('button'); - // Only the search clear button should be present, not the FAB - expect(buttons.length).toBeLessThanOrEqual(1); - }); + it('does not render the new call FAB button', () => { + const { queryByTestId } = render(<CallsScreen />); + expect(queryByTestId('fab-button')).toBeNull(); + });
258-268: Make list rendering assertions concreteYou can assert that each CallCard is present using the data-testid exposed by the mock.
- it('renders call cards for each call', () => { - render(<CallsScreen />); - - // Verify that the mock calls data is set up correctly - expect(mockCallsStore.calls).toHaveLength(2); - expect(mockCallsStore.calls[0].CallId).toBe('call-1'); - expect(mockCallsStore.calls[1].CallId).toBe('call-2'); - - // The component should render without errors when calls are present - expect(true).toBe(true); - }); + it('renders call cards for each call', () => { + const { getByTestId } = render(<CallsScreen />); + expect(getByTestId('call-card-call-1')).toBeTruthy(); + expect(getByTestId('call-card-call-2')).toBeTruthy(); + });src/app/call/__tests__/[id].security.test.tsx (2)
327-354: Assert menu gating to exercise permission-based UIYou already mock the menu pieces with deterministic testIDs. Assert their presence/absence to harden permission coverage.
describe('Security-dependent rendering', () => { it('should render successfully when user has create calls permission', () => { useSecurityStore.mockReturnValue({ canUserCreateCalls: true, }); - expect(() => render(<CallDetail />)).not.toThrow(); + const { getByTestId } = render(<CallDetail />); + expect(getByTestId('header-right-menu')).toBeTruthy(); + expect(getByTestId('call-detail-actionsheet')).toBeTruthy(); }); it('should render successfully when user does not have create calls permission', () => { useSecurityStore.mockReturnValue({ canUserCreateCalls: false, }); - expect(() => render(<CallDetail />)).not.toThrow(); + const { queryByTestId } = render(<CallDetail />); + expect(queryByTestId('header-right-menu')).toBeNull(); + expect(queryByTestId('call-detail-actionsheet')).toBeNull(); }); it('should render call content correctly', () => { const renderResult = render(<CallDetail />); // Check that basic call information is rendered // The component should render without throwing errors, which validates the security logic expect(renderResult).toBeTruthy(); // Verify the component rendered successfully by checking it has content expect(renderResult.toJSON()).toBeTruthy(); }); });
230-234: Button mock doesn’t need to support press here, but consider parity with other testsIn case you later simulate button presses in this suite, mirroring the onClick/onPress dual wiring from other tests will save time. Optional for now.
docs/map-theme-implementation.md (4)
52-56: Clarify NativeWind useColorScheme API shapeThe text suggests the hook yields 'light' | 'dark' | undefined and a setter. NativeWind’s useColorScheme tends to expose an object with colorScheme and setColorScheme/toggleColorScheme. Suggest clarifying the API surface to prevent confusion with React Native’s useColorScheme.
Please confirm the exact API of the NativeWind hook you’re using and adjust the bullets accordingly. If accurate, consider documenting it like:
-The component uses the `useColorScheme` hook from `nativewind` which provides: -- Current theme: `'light' | 'dark' | undefined` -- Theme setter function -- Automatic system theme detection +The component uses NativeWind's `useColorScheme`: +- Returns `{ colorScheme: 'light' | 'dark', setColorScheme, toggleColorScheme }` +- Respects system theme by default; can be overridden via `setColorScheme`
77-83: Avoid “All tests pass” in docs; make it evergreen“All tests pass successfully” will drift. Prefer describing coverage rather than current status.
Apply:
-## Testing Coverage - -All tests pass successfully: +## Testing Coverage + +Current coverage includes:
86-90: Prefer useMemo for themed style objects (not useCallback)If getThemedStyles returns style objects, useMemo is the appropriate primitive; useCallback is for memoizing functions.
If the implementation indeed returns objects, update the note:
-- Theme-aware styling uses `useCallback` for performance optimization +- Theme-aware styling uses `useMemo` for performance optimization
38-41: Minor Markdown polish: remove stray hard line breaksThere are trailing double-spaces after “dark mode” causing hard line breaks; not needed here.
Apply:
-- **Dark theme test**: Verifies map renders correctly in dark mode +- **Dark theme test**: Verifies map renders correctly in dark modesrc/components/calls/__tests__/call-detail-menu.test.tsx (5)
11-11: Avoid any; type the mock hook propsKeep tests type-safe per guidelines. Define a props interface and use it.
Apply:
-const MockCallDetailMenu = ({ onEditCall, onCloseCall, canUserCreateCalls = true }: any) => { +interface MockCallDetailMenuProps { + onEditCall?: () => void; + onCloseCall?: () => void; + canUserCreateCalls?: boolean; +} +const MockCallDetailMenu = ({ onEditCall, onCloseCall, canUserCreateCalls = true }: MockCallDetailMenuProps) => {
29-53: Align mock/test testIDs with production to prevent driftProduction uses testID="call-detail-actionsheet" (see src/components/calls/call-detail-menu.tsx). Using "actionsheet" here hides regressions if the real ID changes. Aligning names improves fidelity even with a mock.
Apply:
- <View testID="actionsheet"> + <View testID="call-detail-actionsheet">And in assertions:
- expect(screen.getByTestId('actionsheet')).toBeTruthy(); + expect(screen.getByTestId('call-detail-actionsheet')).toBeTruthy(); - expect(screen.queryByTestId('actionsheet')).toBeNull(); + expect(screen.queryByTestId('call-detail-actionsheet')).toBeNull();Also applies to: 92-100, 122-132, 141-145
92-100: waitFor likely unnecessary here; the state change is synchronousPressing the button sets state immediately in this mock. You can assert synchronously to speed up the test and reduce flakiness.
Apply:
- fireEvent.press(screen.getByTestId('kebab-menu-button')); - await waitFor(() => { - expect(screen.getByTestId('actionsheet')).toBeTruthy(); - expect(screen.getByTestId('edit-call-button')).toBeTruthy(); - expect(screen.getByTestId('close-call-button')).toBeTruthy(); - }); + fireEvent.press(screen.getByTestId('kebab-menu-button')); + expect(screen.getByTestId('actionsheet')).toBeTruthy(); + expect(screen.getByTestId('edit-call-button')).toBeTruthy(); + expect(screen.getByTestId('close-call-button')).toBeTruthy();
122-132: Also assert closing after “Close” selection for parityYou verify close after selecting “Edit,” but not after “Close.” Add a similar assertion to cover that path.
Proposed addition:
it('closes the action sheet after selecting an option', async () => { render(<TestComponent canUserCreateCalls={true} />); fireEvent.press(screen.getByTestId('kebab-menu-button')); await waitFor(() => { expect(screen.getByTestId('actionsheet')).toBeTruthy(); }); fireEvent.press(screen.getByTestId('edit-call-button')); await waitFor(() => { expect(screen.queryByTestId('actionsheet')).toBeNull(); }); }); +it('closes the action sheet after pressing close', async () => { + render(<TestComponent canUserCreateCalls={true} />); + fireEvent.press(screen.getByTestId('kebab-menu-button')); + await waitFor(() => { + expect(screen.getByTestId('actionsheet')).toBeTruthy(); + }); + fireEvent.press(screen.getByTestId('close-call-button')); + await waitFor(() => { + expect(screen.queryByTestId('actionsheet')).toBeNull(); + }); +});
146-153: This test doesn’t exercise a unique path and adds little signalWithout a way to externally force isOpen while canUserCreateCalls=false, this overlaps prior tests. Either remove it or explicitly simulate the condition (e.g., in the mock, expose a prop to start open).
Possible tweak:
- it('does not allow opening action sheet even if somehow triggered', () => { - // This test ensures that even if the state changed externally, - // the action sheet won't render without permission - render(<TestComponent canUserCreateCalls={false} />); - expect(screen.queryByTestId('actionsheet')).toBeNull(); - expect(screen.queryByTestId('edit-call-button')).toBeNull(); - expect(screen.queryByTestId('close-call-button')).toBeNull(); - }); + it('does not render action sheet when canUserCreateCalls=false even if start-open', () => { + // If you keep the mock, allow bootstrapping open state for this test: + // render(<TestComponent canUserCreateCalls={false} initialOpen />); + // expect(screen.queryByTestId('actionsheet')).toBeNull(); + // For now, this is redundant with previous tests. + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
docs/map-theme-implementation.md(1 hunks)src/__tests__/security-integration.test.ts(1 hunks)src/app/(app)/__tests__/calls.test.tsx(1 hunks)src/app/(app)/__tests__/index.test.tsx(6 hunks)src/app/(app)/_layout.tsx(1 hunks)src/app/(app)/calls.tsx(2 hunks)src/app/(app)/index.tsx(9 hunks)src/app/call/[id].tsx(4 hunks)src/app/call/__tests__/[id].security.test.tsx(1 hunks)src/app/call/__tests__/[id].test.tsx(10 hunks)src/components/calls/__tests__/call-detail-menu.test.tsx(3 hunks)src/components/calls/call-detail-menu.tsx(2 hunks)src/components/maps/static-map.tsx(3 hunks)src/components/status/__tests__/status-bottom-sheet.test.tsx(1 hunks)src/components/status/status-bottom-sheet.tsx(3 hunks)src/stores/security/__tests__/store.test.ts(1 hunks)src/stores/security/store.ts(1 hunks)
🧰 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/app/(app)/calls.tsxsrc/app/call/__tests__/[id].security.test.tsxsrc/__tests__/security-integration.test.tssrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/app/(app)/__tests__/calls.test.tsxsrc/components/calls/call-detail-menu.tsxsrc/stores/security/__tests__/store.test.tssrc/app/(app)/index.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/maps/static-map.tsxsrc/app/(app)/_layout.tsxsrc/components/status/status-bottom-sheet.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsxsrc/stores/security/store.tssrc/app/(app)/__tests__/index.test.tsxsrc/app/call/[id].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/app/(app)/calls.tsxsrc/app/call/__tests__/[id].security.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/app/(app)/__tests__/calls.test.tsxsrc/components/calls/call-detail-menu.tsxsrc/app/(app)/index.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/maps/static-map.tsxsrc/app/(app)/_layout.tsxsrc/components/status/status-bottom-sheet.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/app/call/[id].tsx
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/app/(app)/calls.tsxsrc/app/call/__tests__/[id].security.test.tsxsrc/__tests__/security-integration.test.tsdocs/map-theme-implementation.mdsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/app/(app)/__tests__/calls.test.tsxsrc/components/calls/call-detail-menu.tsxsrc/stores/security/__tests__/store.test.tssrc/app/(app)/index.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/maps/static-map.tsxsrc/app/(app)/_layout.tsxsrc/components/status/status-bottom-sheet.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsxsrc/stores/security/store.tssrc/app/(app)/__tests__/index.test.tsxsrc/app/call/[id].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/(app)/calls.tsxsrc/app/call/__tests__/[id].security.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/app/(app)/__tests__/calls.test.tsxsrc/components/calls/call-detail-menu.tsxsrc/app/(app)/index.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/maps/static-map.tsxsrc/app/(app)/_layout.tsxsrc/components/status/status-bottom-sheet.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/app/call/[id].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/app/call/__tests__/[id].security.test.tsxsrc/__tests__/security-integration.test.tssrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/app/(app)/__tests__/calls.test.tsxsrc/stores/security/__tests__/store.test.tssrc/app/call/__tests__/[id].test.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsxsrc/app/(app)/__tests__/index.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/app/call/__tests__/[id].security.test.tsxsrc/__tests__/security-integration.test.tssrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/app/(app)/__tests__/calls.test.tsxsrc/stores/security/__tests__/store.test.tssrc/app/call/__tests__/[id].test.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsxsrc/app/(app)/__tests__/index.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/app/call/__tests__/[id].security.test.tsxsrc/__tests__/security-integration.test.tssrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/app/(app)/__tests__/calls.test.tsxsrc/stores/security/__tests__/store.test.tssrc/app/call/__tests__/[id].test.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsxsrc/app/(app)/__tests__/index.test.tsx
🧠 Learnings (8)
📚 Learning: 2025-08-12T03:34:25.963Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.963Z
Learning: Applies to **/*.tsx : Ensure support for dark mode and light mode
Applied to files:
docs/map-theme-implementation.mdsrc/app/(app)/index.tsxsrc/app/(app)/__tests__/index.test.tsx
📚 Learning: 2025-08-12T03:33:40.227Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.227Z
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/app/(app)/__tests__/calls.test.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsxsrc/app/(app)/__tests__/index.test.tsx
📚 Learning: 2025-08-12T03:34:25.963Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.963Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated components
Applied to files:
src/app/(app)/__tests__/calls.test.tsx
📚 Learning: 2025-08-12T03:33:40.227Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.227Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Generate tests for all components, services, and logic; ensure tests run without errors
Applied to files:
src/app/(app)/__tests__/calls.test.tsx
📚 Learning: 2025-08-12T03:34:25.963Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.963Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps and navigation
Applied to files:
src/app/(app)/index.tsxsrc/components/maps/static-map.tsxsrc/app/(app)/__tests__/index.test.tsx
📚 Learning: 2025-08-12T03:33:40.227Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.227Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or vehicle navigation
Applied to files:
src/app/(app)/index.tsxsrc/components/maps/static-map.tsxsrc/app/(app)/__tests__/index.test.tsx
📚 Learning: 2025-08-12T03:33:40.227Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.227Z
Learning: Applies to **/*.tsx : Use React Navigation for navigation and deep linking with best practices
Applied to files:
src/app/(app)/index.tsx
📚 Learning: 2025-08-12T03:34:25.963Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.963Z
Learning: Applies to **/*.{ts,tsx} : Use React Navigation for handling navigation and deep linking
Applied to files:
src/app/(app)/index.tsx
🧬 Code Graph Analysis (12)
src/app/(app)/calls.tsx (1)
src/stores/security/store.ts (1)
useSecurityStore(39-51)
src/app/call/__tests__/[id].security.test.tsx (6)
src/stores/calls/detail-store.ts (1)
useCallDetailStore(40-196)src/stores/security/store.ts (1)
useSecurityStore(39-51)src/stores/app/core-store.ts (1)
useCoreStore(48-263)src/stores/app/location-store.ts (1)
useLocationStore(22-52)src/stores/status/store.ts (1)
useStatusBottomSheetStore(48-109)src/app/call/[id].tsx (1)
CallDetail(39-604)
src/__tests__/security-integration.test.ts (1)
src/models/v4/security/departmentRightsResultData.ts (1)
DepartmentRightsResultData(1-13)
src/components/status/__tests__/status-bottom-sheet.test.tsx (1)
src/components/status/status-bottom-sheet.tsx (1)
StatusBottomSheet(26-681)
src/app/(app)/__tests__/calls.test.tsx (3)
src/stores/calls/store.ts (1)
useCallsStore(22-73)src/stores/security/store.ts (1)
useSecurityStore(39-51)src/hooks/use-analytics.ts (1)
useAnalytics(14-22)
src/components/calls/call-detail-menu.tsx (3)
__mocks__/@aptabase/react-native.ts (1)
trackEvent(1-1)src/services/aptabase.service.ts (1)
trackEvent(36-59)src/components/ui/index.tsx (1)
Pressable(18-18)
src/stores/security/__tests__/store.test.ts (3)
src/api/security/security.ts (1)
getCurrentUsersRights(7-10)src/models/v4/security/departmentRightsResultData.ts (1)
DepartmentRightsResultData(1-13)src/stores/security/store.ts (2)
securityStore(15-37)useSecurityStore(39-51)
src/app/(app)/index.tsx (2)
src/hooks/use-map-signalr-updates.ts (1)
useMapSignalRUpdates(8-61)src/services/aptabase.service.ts (1)
trackEvent(36-59)
src/app/call/__tests__/[id].test.tsx (1)
src/app/call/[id].tsx (1)
CallDetail(39-604)
src/components/status/status-bottom-sheet.tsx (1)
src/lib/utils.ts (1)
invertColor(38-62)
src/components/calls/__tests__/call-detail-menu.test.tsx (2)
src/components/ui/index.tsx (1)
TouchableOpacity(18-18)src/components/calls/call-detail-menu.tsx (1)
useCallDetailMenu(16-100)
src/app/call/[id].tsx (1)
src/stores/security/store.ts (1)
useSecurityStore(39-51)
🪛 LanguageTool
docs/map-theme-implementation.md
[grammar] ~3-~3: There might be a mistake here.
Context: # Map Theme Implementation ## Overview This document outlines the implementatio...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...app)/index.tsx) #### Theme Integration - Imported useColorSchemefromnativewi...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...ate Mapbox style based on current theme: - Light mode: Mapbox.StyleURL.Street (...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...zed for low light) #### Dynamic Styling - Created getThemedStyles() function for...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...ecenter button styles to adapt to theme: - Light mode: White borders and dark sha...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...t mode**: White borders and dark shadows - Dark mode: Dark borders and light shad...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...ight shadows #### Analytics Enhancement - Added theme information to analytics tra...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...rrent theme data #### Map Style Updates - Map style now automatically updates when...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...index.test.tsx) #### Mock Enhancements - Updated useColorScheme` mock to include...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...and Dark style URLs #### New Test Cases - Light theme test: Verifies map renders...
(QB_NEW_EN)
[grammar] ~52-~52: There might be a mistake here.
Context: ...hook fromnativewindwhich provides: - Current theme:'light' | 'dark' | undef...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ...rk' | undefined` - Theme setter function - Automatic system theme detection ## Use...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...tion ## User Experience ### Light Mode - Professional street map appearance suita...
(QB_NEW_EN)
[grammar] ~60-~60: There might be a mistake here.
Context: ...nce suitable for all lighting conditions - Balanced contrast with clear, readable t...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ...t with clear, readable text and features - Consistent with the call detail view's s...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...he call detail view's static map styling - Muted colors that reduce eye strain comp...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...ed to bright white themes ### Dark Mode - Reduced brightness for low-light environ...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...ed brightness for low-light environments - Dark backgrounds with light accents - Ey...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...ts - Dark backgrounds with light accents - Eye-strain reduction for nighttime usage...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...nighttime usage ### Automatic Switching - Theme changes are applied immediately wi...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...g Coverage All tests pass successfully: - ✅ Basic map rendering - ✅ Light theme fu...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...ss successfully: - ✅ Basic map rendering - ✅ Light theme functionality - ✅ Dark t...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ... rendering - ✅ Light theme functionality - ✅ Dark theme functionality - ✅ Theme swi...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...tionality - ✅ Dark theme functionality - ✅ Theme switching behavior - ✅ Analytics...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...nctionality - ✅ Theme switching behavior - ✅ Analytics integration with theme data ...
(QB_NEW_EN)
🔇 Additional comments (16)
src/stores/security/store.ts (1)
44-45: LGTM: Guarded group admin check prevents runtime errorsThe null-safe access with a fallback to false avoids crashes when Groups is missing and provides a stable boolean return.
src/app/(app)/index.tsx (1)
227-230: LGTM: Theme added to analytics payloadIncluding theme in the map_view_rendered event improves observability for theme adoption and UX analysis.
src/app/(app)/calls.tsx (1)
104-109: LGTM: Permission-gated FAB aligns with security store and testsConditionally rendering the Create Call FAB based on canUserCreateCalls matches the PR objectives and prevents unauthorized actions in the UI.
src/components/maps/static-map.tsx (1)
23-25: LGTM: Theme-aware map style for static mapUsing nativewind’s colorScheme with Mapbox StyleURL provides consistent light/dark theming. Passing styleURL directly keeps it reactive to theme changes.
Also applies to: 36-36
src/__tests__/security-integration.test.ts (2)
10-14: Permission check logic is precise and safe by default.Using strict equality against true avoids accidental truthiness; null and undefined are correctly treated as false.
75-97: UI gating assertions align with permission checks.The FAB/menu visibility mirrors the canUserCreateCalls outcome. Good coverage for granted/denied cases.
src/app/call/[id].tsx (1)
492-498: Good: “Set Active” now gated by presence of an active unit.Prevents exposing an action that can’t complete when there’s no active unit context.
src/components/status/status-bottom-sheet.tsx (1)
643-647: LGTM: Selected status pill uses BColor with invertColor for contrast.The pill approach is clean and keeps text legible regardless of theme.
src/components/calls/call-detail-menu.tsx (2)
49-91: LGTM: Permission-gated rendering for menu and actionsheet.Returning null when canUserCreateCalls is false prevents exposure of forbidden actions in both header and sheet.
16-16: Verified call sites for useCallDetailMenu defaultsAll production invocations now explicitly pass
canUserCreateCalls, so no unintended permissive behavior will leak into live code. The only places still relying on the default (= true) are test files, which align with their intent:• Production
– src/app/call/[id].tsx (passescanUserCreateCalls)
– src/app/(app)/calls.tsx (passescanUserCreateCalls)• Tests (defaulting to
true)
– src/components/calls/tests/call-detail-menu-integration.test.tsx
– src/components/calls/tests/call-detail-menu-analytics.test.tsxNo further changes required.
src/stores/security/__tests__/store.test.ts (1)
46-69: Strong coverage of store selectors and null-edge casesGreat job exercising both the success and error paths and validating each derived selector (including the Groups predicate defaulting to false). The setup with act + setState keeps the zustand store deterministic across tests.
src/app/(app)/__tests__/index.test.tsx (1)
64-69: Color scheme mocking pattern is solidNice use of a typed mock for useColorScheme with setter stubs; it prevents re-render loops and makes theme flips deterministic.
Also applies to: 93-97, 120-125
docs/map-theme-implementation.md (1)
45-49: Docs already match current Mapbox defaults; prefer constants over hardcoded URLsThe table in docs/map-theme-implementation.md uses streets-v11 and dark-v10, which aligns with the app’s test mocks (see StyleURL.Street → streets-v11 and StyleURL.Dark → dark-v10). Since the code itself references Mapbox.StyleURL.Street and Mapbox.StyleURL.Dark, you can safely:
- Keep the URL column for reference, but avoid hardcoding version numbers.
- Rely on
Mapbox.StyleURL.<…>in docs to stay in sync with the library defaults.- Only update the literal URLs if you upgrade the Mapbox SDK—and then confirm the new default URLs before changing the docs.
Likely an incorrect or invalid review comment.
src/components/calls/__tests__/call-detail-menu.test.tsx (3)
14-27: LGTM: Permission-gated header button behavior is correct in the mockHeaderRightMenu returns null when canUserCreateCalls is false and exposes a clearly testable button otherwise. Matches the production intent.
29-31: LGTM: Action sheet visibility is guarded by both state and permissionThe additional guard ensures the sheet cannot render without permission even if open state flips.
58-60: Integration & Analytics Tests Cover the Real Hook; Unit Tests May Safely Mock ItThe
useCallDetailMenuhook is already exercised by two dedicated specs:
src/components/calls/__tests__/call-detail-menu-analytics.test.tsxsrc/components/calls/__tests__/call-detail-menu-integration.test.tsxThese tests validate:
- Analytics events (
trackEvent)- TestIDs, gating, and UI behavior via the real hook
By contrast, mocking
useCallDetailMenuincall-detail-menu.test.tsxcleanly isolates the component’s rendering logic. Any regressions in the hook implementation will still be caught by the existing integration and analytics tests—so no changes are required here.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/calls/call-detail-menu.tsx (1)
13-16: Least-privilege default applied for canUserCreateCalls — good.Defaulting to false removes the permissive gap before rights load and aligns with security best practices.
🧹 Nitpick comments (5)
src/components/calls/call-detail-menu.tsx (2)
36-47: Use onPress instead of onPressIn for expected gesture semantics and accessibility.onPressIn fires on touch-down which can feel premature and is less conventional for buttons. Prefer onPress.
- return ( - <Pressable onPressIn={openMenu} testID="kebab-menu-button" className="rounded p-2"> + return ( + <Pressable onPress={openMenu} testID="kebab-menu-button" className="rounded p-2"> <MoreVerticalIcon size={24} className="text-gray-700 dark:text-gray-300" /> </Pressable> );Optional: add accessibilityRole/accessibilityLabel for screen readers.
- <Pressable onPress={openMenu} testID="kebab-menu-button" className="rounded p-2"> + <Pressable + onPress={openMenu} + testID="kebab-menu-button" + accessibilityRole="button" + accessibilityLabel={t('call_detail.open_menu')} + className="rounded p-2" + >
49-91: Prevent programmatic opening when not authorized; optionally memoize handlers.Even though the UI is hidden when canUserCreateCalls is false, consumers can still call openMenu directly, which will currently trigger analytics. Guard openMenu. Also consider memoizing the action handlers to avoid re-creation on each render.
Change openMenu (outside this hunk) to guard permission:
- const openMenu = () => { - setIsKebabMenuOpen(true); - }; + const openMenu = () => { + if (!canUserCreateCalls) return; + setIsKebabMenuOpen(true); + };Optionally type the internal components for clarity:
- const HeaderRightMenu = () => { + const HeaderRightMenu: React.FC = () => { ... - const CallDetailActionSheet = () => { + const CallDetailActionSheet: React.FC = () => {src/components/calls/__tests__/call-detail-menu-analytics.test.tsx (1)
95-96: Analytics tests align with permissioned UI — consider adding a denial-path test.Good call explicitly setting canUserCreateCalls: true. Add a complementary test that asserts no analytics is tracked when permission is false, even if openMenu is called (paired with guarding openMenu as suggested in the hook).
Example test to add:
it('should not track analytics when permission is denied even if openMenu is called', () => { const { result } = renderHook(() => useCallDetailMenu({ onEditCall: jest.fn(), onCloseCall: jest.fn(), canUserCreateCalls: false, }) ); act(() => { result.current.openMenu(); }); expect(result.current.isMenuOpen).toBe(false); // if you also short-circuit in openMenu expect(mockTrackEvent).not.toHaveBeenCalled(); });Also applies to: 120-121, 143-144, 184-185, 209-210
src/components/calls/__tests__/call-detail-menu.test.tsx (2)
11-27: Mock honors permission gating — minor fidelity improvement suggested.Behavior is correct. For closer parity with RN, consider importing View/Text/TouchableOpacity from 'react-native' instead of HTML tags to avoid env differences.
122-132: Action sheet closes after selection — consider also asserting for the close button path.You can mirror this check for the close action for completeness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/app/(app)/__tests__/calls.test.tsx(1 hunks)src/components/calls/__tests__/call-detail-menu-analytics.test.tsx(5 hunks)src/components/calls/__tests__/call-detail-menu-integration.test.tsx(1 hunks)src/components/calls/__tests__/call-detail-menu.test.tsx(3 hunks)src/components/calls/call-detail-menu.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/(app)/tests/calls.test.tsx
🧰 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/calls/__tests__/call-detail-menu-integration.test.tsxsrc/components/calls/__tests__/call-detail-menu-analytics.test.tsxsrc/components/calls/call-detail-menu.tsxsrc/components/calls/__tests__/call-detail-menu.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/calls/__tests__/call-detail-menu-integration.test.tsxsrc/components/calls/__tests__/call-detail-menu-analytics.test.tsxsrc/components/calls/call-detail-menu.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsx
**/*
📄 CodeRabbit Inference Engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/components/calls/__tests__/call-detail-menu-integration.test.tsxsrc/components/calls/__tests__/call-detail-menu-analytics.test.tsxsrc/components/calls/call-detail-menu.tsxsrc/components/calls/__tests__/call-detail-menu.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/calls/__tests__/call-detail-menu-integration.test.tsxsrc/components/calls/__tests__/call-detail-menu-analytics.test.tsxsrc/components/calls/__tests__/call-detail-menu.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/calls/__tests__/call-detail-menu-integration.test.tsxsrc/components/calls/__tests__/call-detail-menu-analytics.test.tsxsrc/components/calls/__tests__/call-detail-menu.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/calls/__tests__/call-detail-menu-integration.test.tsxsrc/components/calls/__tests__/call-detail-menu-analytics.test.tsxsrc/components/calls/call-detail-menu.tsxsrc/components/calls/__tests__/call-detail-menu.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/calls/__tests__/call-detail-menu-integration.test.tsxsrc/components/calls/__tests__/call-detail-menu-analytics.test.tsxsrc/components/calls/__tests__/call-detail-menu.test.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-12T03:33:40.227Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.227Z
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/calls/__tests__/call-detail-menu.test.tsx
🧬 Code Graph Analysis (2)
src/components/calls/call-detail-menu.tsx (3)
__mocks__/@aptabase/react-native.ts (1)
trackEvent(1-1)src/services/aptabase.service.ts (1)
trackEvent(36-59)src/components/ui/index.tsx (1)
Pressable(18-18)
src/components/calls/__tests__/call-detail-menu.test.tsx (2)
src/components/ui/index.tsx (1)
TouchableOpacity(18-18)src/components/calls/call-detail-menu.tsx (1)
useCallDetailMenu(16-100)
🔇 Additional comments (10)
src/components/calls/call-detail-menu.tsx (2)
25-27: Analytics now reflects actual permission gating — verify permission semantics.Deriving hasEditAction/hasCloseAction from canUserCreateCalls looks correct. Please confirm your security model truly ties both Edit and Close to the "create calls" right; if there are more granular rights (e.g., close without create), consider splitting the flags.
29-29: Effect deps include canUserCreateCalls — confirm trackEvent stability.Including trackEvent is fine if useAnalytics returns a stable function. If its identity changes per render, you may get spurious analytics. If unstable, wrap/alias to a stable ref or omit and rely on eslint-disable with a comment.
src/components/calls/__tests__/call-detail-menu-integration.test.tsx (1)
76-77: Explicitly enabling permission in the integration path — good coverage.Passing canUserCreateCalls: true ensures the positive path is exercised end-to-end with UI present.
src/components/calls/__tests__/call-detail-menu.test.tsx (7)
30-31: Action sheet gated by both open state and permission — good.
67-73: Threading canUserCreateCalls through the test harness — good.
86-91: Positive-path: header menu button renders with permission — good.
92-100: Opening action sheet via header menu — good interaction coverage.
102-111: Edit action invokes callback — good assertion.
112-120: Close action invokes callback — good assertion.
135-153: Negative-path: UI hidden without permission — good guard rails.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…update.
Summary by CodeRabbit
New Features
Style
Documentation
Tests