CU-868ex18rd Fix for lint issue, bug fixes, status fix, side menu work. #60
Conversation
|
Caution Review failedThe pull request is closed. 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. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/status/personnel-status-bottom-sheet.tsx (1)
214-219: Auto-select can override an explicit “No Destination” choice.When responseType is 'none' and an activeCall exists, this effect always selects the active call—even if the user intentionally chose “No Destination.” That’s a UX bug and contradicts “No Destination is always valid.”
Apply this diff to auto-select only once on open and only when nothing has been selected yet:
-import React, { useCallback, useEffect } from 'react'; +import React, { useCallback, useEffect, useRef } from 'react'; @@ -// Auto-select active call if available and no destination is selected -React.useEffect(() => { - if (activeCall && currentStep === 'select-responding-to' && responseType === 'none') { - setSelectedCall(activeCall); - } -}, [activeCall, currentStep, responseType, setSelectedCall]); +// Auto-select active call once on open if nothing is selected yet +const hasAutoSelectedRef = useRef(false); +useEffect(() => { + if (!isOpen || hasAutoSelectedRef.current) return; + if ( + activeCall && + currentStep === 'select-responding-to' && + responseType === 'none' && + !selectedCall && + !selectedGroup + ) { + setSelectedCall(activeCall); + hasAutoSelectedRef.current = true; + } +// Intentionally exclude responseType to avoid overriding explicit "none" choice +}, [isOpen, activeCall, currentStep, selectedCall, selectedGroup, setSelectedCall]);
🧹 Nitpick comments (43)
src/components/ui/drawer/index.tsx (3)
205-209: Spring parameters: good switch; consider sync with Backdrop fade to avoid exit desyncThe move to a spring feels nicer. On some devices the content can still be moving while the Backdrop has fully faded (250ms), creating a brief “floating” look on close. Two small options:
- Nudge the spring slightly snappier or add a tiny Backdrop exit delay to keep them visually in lockstep.
- Hoist the spring config to a shared constant so both places stay tuned together.
Example tweaks within this block:
transition={{ - type: 'spring', - damping: 20, - stiffness: 300, - mass: 0.8, + type: 'spring', + damping: 22, + stiffness: 280, + mass: 0.9, }}Optional: in Backdrop, add a tiny exit delay (e.g., 50–75ms) or match fade duration to the perceived spring settle time.
13-14: Orientation changes won’t recompute drawer sizescreenWidth/screenHeight are captured at module load. On rotation, DrawerContent sizing won’t update until a remount. Prefer useWindowDimensions inside the component.
-const screenWidth = Dimensions.get('window').width; -const screenHeight = Dimensions.get('window').height; +// Remove these module-level constants and compute via hook // ... const DrawerContent = React.forwardRef<React.ElementRef<typeof UIDrawer.Content>, IDrawerContentProps>(({ className, ...props }, ref) => { const { size: parentSize, anchor: parentAnchor } = useStyleContext(SCOPE); + const { width: screenWidth, height: screenHeight } = Dimensions.get('window'); // or useWindowDimensions() const drawerHeight = screenHeight * (sizes[parentSize] || sizes.md); const drawerWidth = screenWidth * (sizes[parentSize] || sizes.md);If you want live updates without remount, use useWindowDimensions() rather than Dimensions.get('window') here.
Also applies to: 185-187
58-60: Tailwind “overflow-scroll” is not a valid React Native valueRN only supports ‘visible/hidden’ for overflow; “overflow-scroll” is a web concept. Since Body is already a ScrollView, consider setting Content to overflow-hidden to avoid RN warnings and clipping glitches.
- base: 'bg-background-0 overflow-scroll border-outline-100 p-6 absolute', + base: 'bg-background-0 overflow-hidden border-outline-100 p-6 absolute',tsconfig.json (1)
33-33: Include broadened: OK; consider isolating test/mocks to a test tsconfig for faster app type-checksBringing in src//*.d.ts and mocks//*.tsx helps IDE/test types, but it can marginally slow regular app type-checks. Common pattern: keep app tsconfig lean and add a tsconfig.spec.json that extends it and includes tests/mocks.
Minimal change now (optional): no action required. If you want, I can draft a tsconfig.spec.json and wire scripts (type-check:tests).
.eslintrc.js (1)
50-56: Good call: disable type-aware parsing for .d.tsSetting parserOptions.project to null for declaration files avoids costly/fragile typed linting there.
If you see perf issues on large repos, you can also add:
- rules: { '@typescript-eslint/triple-slash-reference': 'off' } for legacy d.ts,
- and explicitly turn off unused-imports in d.ts (usually irrelevant).
src/components/home/status-buttons.tsx (1)
15-16: Type-safety and render perf: avoid any, precompute filtered statuses, and stable handlers
- Avoid any for statusData; use a concrete type.
- Precompute filtered statuses to keep render light.
- Avoid inline arrow in onPress per guidelines; provide a stable callback.
Apply the following refactor:
import React from 'react'; import { useTranslation } from 'react-i18next'; @@ export const StatusButtons: React.FC = () => { const { t } = useTranslation(); - const { isLoadingOptions } = useHomeStore(); - const { activeStatuses } = useCoreStore(); + const { isLoadingOptions } = useHomeStore(); + const { activeStatuses } = useCoreStore(); const { setIsOpen } = usePersonnelStatusBottomSheetStore(); - const handleStatusPress = (statusId: number, statusData: any) => { - // Open the bottom sheet with the selected status - setIsOpen(true, statusData); - }; + // Narrow type to fields we actually use; replace with an app-wide type if available. + type ActiveStatus = { Id: number; Text: string; BColor: string }; + const EXCLUDED_STATUS_IDS = React.useMemo(() => [4, 5, 6, 7] as const, []); + const filteredActiveStatuses = React.useMemo( + () => (activeStatuses ?? []).filter((s: ActiveStatus) => !EXCLUDED_STATUS_IDS.includes(s.Id as (typeof EXCLUDED_STATUS_IDS)[number])), + [activeStatuses, EXCLUDED_STATUS_IDS] + ); + const handleStatusPress = React.useCallback( + (status: ActiveStatus) => () => { + setIsOpen(true, status); + }, + [setIsOpen] + ); @@ - if (activeStatuses?.length === 0) { + if ((activeStatuses?.length ?? 0) === 0) { return ( <VStack className="p-4"> <Text className="text-center text-gray-500">{t('home.status.no_options_available')}</Text> </VStack> ); } @@ - {activeStatuses - ?.filter((status) => ![4, 5, 6, 7].includes(status.Id)) - .map((status) => ( + {filteredActiveStatuses.map((status: ActiveStatus) => ( <Button key={status.Id} variant="solid" className="w-full justify-center px-3 py-2" action="primary" size="lg" - style={{ - backgroundColor: status.BColor, - }} - onPress={() => handleStatusPress(status.Id, status)} + style={{ backgroundColor: status.BColor }} + onPress={handleStatusPress(status)} testID={`status-button-${status.Id}`} > - <ButtonText style={{ color: invertColor(status.BColor, true) }}>{status.Text}</ButtonText> + <ButtonText style={{ color: invertColor(status.BColor, true) }}>{status.Text}</ButtonText> </Button> - ))} + ))}Please confirm:
- Whether a shared Status/ActiveStatus type exists (e.g., in stores). If yes, I’ll switch to that import instead of the local type.
- That BColor is always present; if not, we should default to a theme color to avoid runtime crashes in invertColor(undefined).
Also applies to: 28-28, 38-38, 47-49, 53-53
src/stores/status/personnel-status-store.ts (5)
241-247: Rename isDestinationRequired or update comment to reflect “No Destination is always valid” rule.The helper returns selectedStatus.Detail > 0, implying a requirement exists, but the new rule allows proceeding without any destination. Either rename (e.g., destinationSelectionAllowed) or clarify comments to avoid misinterpretation in future conditionals.
168-173: User‑facing toast strings should be localized.Hardcoded messages bypass i18n. Consider using i18n.t(...) or standardized toast keys to meet localization guidelines.
If you share your i18n setup entry point, I can propose concrete replacements and keys for these toasts.
69-73: Redundant ternary — simplify for readability.currentStep is set to 'select-responding-to' in both branches.
Apply:
- set({ - isOpen, - selectedStatus: status || null, - currentStep: status ? 'select-responding-to' : 'select-responding-to', - }); + set({ isOpen, selectedStatus: status || null, currentStep: 'select-responding-to' });
156-224: submitStatus: consider telemetry/error detail and queue failure handling.Current catch only shows a generic toast. Optionally add error context to logs and guard offlineQueueProcessor.addPersonnelStatusToQueue with try/catch, so queue failures also surface.
263-268: Function name implies accuracy but returns a boolean.getRequiredGpsAccuracy returns a boolean “requires GPS”. Consider renaming to requiresGps to match behavior.
src/components/sidebar/side-menu.tsx (5)
35-35: Type safety: avoid typing React.memo result as React.FC.React.memo returns MemoExoticComponent<FC<...>>, which isn’t exactly React.FC and can confuse props inference. Prefer defining the component, then exporting a memoized version.
Apply:
-export const SideMenu: React.FC<SideMenuProps> = React.memo(({ onNavigate }) => { +const SideMenuInner: React.FC<SideMenuProps> = ({ onNavigate }) => { // ...component body unchanged... -}); +}; + +export const SideMenu = React.memo(SideMenuInner); + SideMenu.displayName = 'SideMenu';Also applies to: 273-276
111-123: Navigation sequencing: replace rAF + magic 50ms delay with InteractionManager for robustness.requestAnimationFrame + setTimeout(50) can be flaky across devices and frame rates. InteractionManager.runAfterInteractions ensures work runs after animations and interactions settle.
Apply:
+import { InteractionManager } from 'react-native'; @@ - const handleNavigation = useCallback( - (route: string) => { - // Use requestAnimationFrame to ensure navigation happens smoothly - requestAnimationFrame(() => { - router.push(route as any); - // Add a small delay before closing to allow navigation to start - setTimeout(() => { - onNavigate?.(); - }, 50); - }); - }, - [router, onNavigate] - ); + const handleNavigation = useCallback( + (route: string) => { + InteractionManager.runAfterInteractions(() => { + router.push(route as any); + onNavigate?.(); + }); + }, + [router, onNavigate] + );If you keep the delay, consider exposing it as a constant and documenting why 50ms is chosen.
145-156: Remove dead branch — both paths do the same.handlePTTPress sets the same state for both conditions.
Apply:
- const handlePTTPress = useCallback(() => { - if (isConnected) { - // If connected, either toggle microphone or re-open bottom sheet - // For better UX, let's re-open the bottom sheet to show connection status - setIsBottomSheetVisible(true); - } else { - // Open LiveKit bottom sheet if not connected - setIsBottomSheetVisible(true); - } - }, [isConnected, setIsBottomSheetVisible]); + const handlePTTPress = useCallback(() => { + setIsBottomSheetVisible(true); + }, [setIsBottomSheetVisible]);
245-263: Avoid inline arrow functions in render to reduce re-renders.Guidelines recommend avoiding inline handlers. You can memoize menuItems and generate stable handlers.
Additional code (outside selected lines):
// Above return: const menuItems = React.useMemo<MenuItem[]>(() => [ // ...same items... ], [t]); const getOnPress = useCallback((route: string) => () => handleNavigation(route), [handleNavigation]);Then:
- onPress={() => handleNavigation(item.route)} + onPress={getOnPress(item.route)}
40-44: Store usage consistency: prefer a single source for security state.You read departmentCode via useSecurityStore() and rights via securityStore(). Mixing hook and non-hook access may lead to inconsistent updates. Prefer the hook selectors for both or document the intent.
src/lib/storage/index.tsx (3)
18-21: Harden JSON parsing to avoid crashes on legacy/plain values.If legacy data wasn’t JSON-encoded, JSON.parse will throw. Wrap with try/catch and return null on parse failure.
Apply:
export function getItem<T>(key: string): T | null { - const value = storage.getString(key); - return value ? JSON.parse(value) : null; + const value = storage.getString(key); + if (value == null) return null; + try { + return JSON.parse(value) as T; + } catch { + return null; + } }
23-29: setItem/removeItem are synchronous — consider dropping async to reflect reality.Not a blocker, but the async signatures may mislead and complicate call chains.
Apply:
-export async function setItem<T>(key: string, value: T) { +export function setItem<T>(key: string, value: T) { storage.set(key, JSON.stringify(value)); } -export async function removeItem(key: string) { +export function removeItem(key: string) { storage.delete(key); }If callers rely on awaiting these, keep the current signatures.
31-42: zustandStorage: ensure values are strings (as required) and consider try/catch on getItem.You’re compliant with set/get/delete being strings. Optionally wrap getString with try/catch similar to getItem() above for resilience.
src/components/sidebar/__tests__/side-menu.test.tsx (1)
12-16: Router mock can also assert route correctness.Optionally capture and assert that push was called with the expected route when tapping menu items to increase coverage.
Example:
const mockedUseRouter = useRouter as jest.Mock; const push = jest.fn(); mockedUseRouter.mockReturnValue({ push }); fireEvent.press(screen.getByTestId('side-menu-home')); expect(push).toHaveBeenCalledWith('/(app)/home');docs/personnel-status-bottom-sheet-final-refactor.md (2)
97-101: Hyphenation nit: “backward compatible” → “backward-compatible”.Minor grammar polish for the “Breaking Changes” section.
- **None**: All changes are backward compatible + **None**: All changes are backward-compatible
21-25: Add accessibility guidance for touch targets and the new close button.Recommend explicitly documenting accessibility props (accessibilityRole, accessibilityLabel, accessibilityHint) for selection tiles and the header close button, to match the “Better Accessibility” claim and ensure consistent implementation.
Would you like a short snippet you can paste into the docs showing recommended props?
Also applies to: 55-65, 89-94, 119-124
docs/personnel-status-refactoring.md (1)
44-52: Grammar + clarity nits.Tiny polish to make benefits more direct.
- **Better User Control**: Visual close button provides clear way to exit the bottom sheet + **Better User Control**: The visual close button provides a clear way to exit the bottom sheetsrc/components/status/__tests__/personnel-status-bottom-sheet.test.tsx (4)
147-180: Remove unused radio component mocks.These mocks appear to be legacy and unused after the touchable refactor; keeping them may mask accidental regressions and increases test surface.
-jest.mock('@/components/ui/radio', () => ({ - RadioGroup: ({ children, value, onChange, ...props }: any) => { - const { View } = require('react-native'); - const React = require('react'); - const enhancedChildren = React.Children.map(children, (child: any) => { - if (child?.props?.value) { - return React.cloneElement(child, { - onPress: () => onChange && onChange(child.props.value), - }); - } - return child; - }); - return <View testID="radio-group" {...props}>{enhancedChildren}</View>; - }, - Radio: ({ children, value, onPress, ...props }: any) => { - const { TouchableOpacity } = require('react-native'); - return <TouchableOpacity testID={`radio-${value}`} onPress={onPress} {...props}>{children}</TouchableOpacity>; - }, - RadioIndicator: ({ children, ...props }: any) => { - const { View } = require('react-native'); - return <View testID="radio-indicator" {...props}>{children}</View>; - }, - RadioIcon: () => { - const { View } = require('react-native'); - return <View testID="radio-icon" />; - }, - RadioLabel: ({ children, ...props }: any) => { - const { View } = require('react-native'); - return <View testID="radio-label" {...props}>{children}</View>; - }, -}));
2-2: Remove unused import “waitFor”.-import { render, screen, fireEvent, waitFor } from '@testing-library/react-native'; +import { render, screen, fireEvent } from '@testing-library/react-native';
924-942: Add a test for close analytics and explicit close interaction.Currently we assert reset is available, and in analytics we don’t simulate the actual close path. Once the component exposes a testID on the close TouchableOpacity (see component suggestion), add a test that presses that element and verifies the “personnel_status_bottom_sheet_closed” event.
I can add the test once the component exposes testID="close-button".
Also applies to: 1242-1271
778-803: Coverage for the “auto-select active call” edge case.Add a test that: selects “No Destination” with an active call present and verifies it stays ‘none’ (i.e., auto-select does not re-select the call). This will guard against the regression flagged in the component.
I can contribute this test once the component fix lands.
src/components/status/personnel-status-bottom-sheet.tsx (13)
57-63: Consistently use the same style of useEffect import/call.The file mixes React.useEffect and useEffect. Prefer a single style for readability (and to satisfy lint rules).
-// Fetch calls and groups when bottom sheet opens -React.useEffect(() => { +// Fetch calls and groups when bottom sheet opens +useEffect(() => { if (isOpen) { fetchCalls(); fetchGroups(); } }, [isOpen, fetchCalls, fetchGroups]);(This is partially addressed by the previous diff; ensure both effects consistently use useEffect.)
Also applies to: 214-219, 249-254
501-507: Follow guideline: prefer ?: over && for conditional rendering.Per repo guidelines, replace && with ternaries to avoid falsey surprises.
- {respondingTo && ( + {respondingTo ? ( <VStack space="xs"> <Text className="font-medium">{t('personnel.status.custom_responding_to')}:</Text> <Text className="text-sm">{respondingTo}</Text> </VStack> - )} + ) : null} @@ - {note && ( + {note ? ( <VStack space="xs"> <Text className="font-medium">{t('personnel.status.note')}:</Text> <Text className="text-sm">{note}</Text> </VStack> - )} + ) : null} @@ - {group.Address && <Text className="text-sm text-gray-600 dark:text-gray-400">{group.Address}</Text>} - {group.GroupType && <Text className="text-xs text-gray-500 dark:text-gray-500">{group.GroupType}</Text>} + {group.Address ? <Text className="text-sm text-gray-600 dark:text-gray-400">{group.Address}</Text> : null} + {group.GroupType ? <Text className="text-xs text-gray-500 dark:text-gray-500">{group.GroupType}</Text> : null}Also applies to: 508-513, 434-436
339-350: Unify color tokens; avoid hard-coded hex (#3b82f6).Parent elements use primary-500 classes, but the small selection badge uses inline hex. Use tailwind/nativewind tokens for consistency and theming.
- <VStack - className="flex size-5 items-center justify-center rounded border-2" - style={{ - borderColor: responseType === 'none' ? '#3b82f6' : '#9ca3af', - backgroundColor: responseType === 'none' ? '#3b82f6' : 'transparent', - }} - > + <VStack + className={`flex size-5 items-center justify-center rounded border-2 ${responseType === 'none' ? 'border-primary-500 bg-primary-500' : 'border-neutral-400 bg-transparent'}`} + > {responseType === 'none' && <Check size={12} color="#fff" />} </VStack>Apply the same pattern to the call and station selection badges in the other two ranges.
Also applies to: 386-393, 425-432
360-365: Prefer theme tokens over “blue-600”; align with primary palette.To keep styling centralized and themeable, use primary-600 for active tabs/buttons.
- className={`flex-1 rounded-lg py-3 ${selectedTab === 'calls' ? 'bg-blue-600' : 'bg-gray-200 dark:bg-gray-700'}`} + className={`flex-1 rounded-lg py-3 ${selectedTab === 'calls' ? 'bg-primary-600' : 'bg-gray-200 dark:bg-gray-700'}`} @@ - <Button onPress={handleNext} isDisabled={!canProceedFromCurrentStep()} className="bg-blue-600"> + <Button onPress={handleNext} isDisabled={!canProceedFromCurrentStep()} className="bg-primary-600"> @@ - <Button onPress={handleNext} isDisabled={!canProceedFromCurrentStep()} className="flex-1 bg-blue-600"> + <Button onPress={handleNext} isDisabled={!canProceedFromCurrentStep()} className="flex-1 bg-primary-600">Also applies to: 448-453, 478-481
451-452: Redundant color conditional.color is '#fff' in both branches.
- <ArrowRight size={16} color={colorScheme === 'dark' ? '#fff' : '#fff'} /> + <ArrowRight size={16} color="#fff" />
22-22: Type the component as React.FC and export a stable type.Matches repo guidelines for TSX components.
-export const PersonnelStatusBottomSheet = () => { +export const PersonnelStatusBottomSheet: React.FC = () => {
16-17: Remove unused imports/store selectors.Input, InputField and setCurrentStep aren’t used in this component; trimming avoids lint noise.
-import { Input, InputField } from '../ui/input'; +// (removed unused Input/InputField) @@ - setCurrentStep, + // setCurrentStep, (unused)Also applies to: 25-50
369-446: Use FlatList for better large-list performance.Calls/stations can be large. Replace ScrollView + map with FlatList, and supply getItemLayout, keyExtractor, and windowing props (removeClippedSubviews, maxToRenderPerBatch, windowSize) per project guidelines.
I can push a focused refactor using FlatList for both tabs if you want me to.
316-326: Accessibility: add roles and labels to touchables.Add accessibilityRole="button" (or "tab" for tabs) and accessibilityLabel/Hint for the close button, “No Destination,” call rows, and station rows to improve screen reader UX.
- <TouchableOpacity onPress={handleClose} className="p-1"> + <TouchableOpacity testID="close-button" accessibilityRole="button" accessibilityLabel={t('common.close')} onPress={handleClose} className="p-1">(Apply similar props to selectable tiles and tabs.)
Also applies to: 336-356, 379-402, 418-440
378-383: Expose stable testIDs for list items to de-flake tests.Relying on text matching is brittle. Add testID attributes derived from ids.
- <TouchableOpacity - key={call.CallId} + <TouchableOpacity + key={call.CallId} + testID={`call-option-${call.CallId}`} onPress={() => handleCallSelect(call.CallId)}(Do the same for groups with testID={
group-option-${group.GroupId}}.)Also applies to: 417-422
449-453: Button composition nit: keep icons in ButtonText or provide accessible name.Consider including icons inside ButtonText or ensuring accessibleLabel includes action text to improve SR experience.
Also applies to: 516-523
281-294: canProceedFromCurrentStep() could be in the store for single source of truth.Since navigation rules also exist in the store (nextStep), moving this check to the store avoids drift and simplifies testing.
I can add a selector like canProceed(currentStep) to the store and wire it here.
220-247: Analytics callback dependency width.trackViewAnalytics depends on large objects (calls, groups) and will re-create frequently. That’s OK, but if you want fewer re-emissions, consider selecting derived counts from the stores to reduce object identity churn.
src/components/status/__tests__/personnel-status-integration.test.tsx (3)
388-415: Prefer a dedicated close-button testID over pressing parent of icon.Target the TouchableOpacity directly to avoid tree-shape coupling. This requires adding testID="close-button" on the component (see component comment).
- const closeButton = screen.getByTestId('x-icon'); - fireEvent.press(closeButton.parent); // Press the TouchableOpacity parent + const closeButton = screen.getByTestId('close-button'); + fireEvent.press(closeButton);
245-356: Add a regression test for the “No Destination” override.After fixing the auto-select logic, add an integration test: activeCall present, user taps “No Destination,” ensure responseType stays 'none' and selectedCall remains null.
I can author this test once the component fix is merged.
358-371: Stabilize selection by using item testIDs instead of text.Once component adds testIDs, replace getByText('CALL-001 - Test Call 1') with getByTestId('call-option-1') to reduce flakiness due to localization or formatting.
📜 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 ignored due to path filters (2)
assets/adaptive-icon.pngis excluded by!**/*.pngyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.eslintrc.js(1 hunks)docs/personnel-status-bottom-sheet-final-refactor.md(1 hunks)docs/personnel-status-refactoring.md(1 hunks)package.json(2 hunks)src/app/(app)/_layout.tsx(2 hunks)src/components/home/status-buttons.tsx(4 hunks)src/components/sidebar/__tests__/side-menu.test.tsx(3 hunks)src/components/sidebar/side-menu.tsx(6 hunks)src/components/status/__tests__/personnel-status-bottom-sheet.test.tsx(5 hunks)src/components/status/__tests__/personnel-status-integration.test.tsx(1 hunks)src/components/status/personnel-status-bottom-sheet.tsx(5 hunks)src/components/ui/drawer/index.tsx(1 hunks)src/lib/storage/index.tsx(1 hunks)src/stores/status/personnel-status-store.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
tsconfig.json
📄 CodeRabbit inference engine (.cursorrules)
Enable strict typing in tsconfig.json
Files:
tsconfig.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
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
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
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)/_layout.tsxsrc/components/sidebar/side-menu.tsxsrc/components/ui/drawer/index.tsxsrc/components/status/__tests__/personnel-status-integration.test.tsxsrc/stores/status/personnel-status-store.tssrc/components/home/status-buttons.tsxsrc/components/status/__tests__/personnel-status-bottom-sheet.test.tsxsrc/lib/storage/index.tsxsrc/components/sidebar/__tests__/side-menu.test.tsxsrc/components/status/personnel-status-bottom-sheet.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
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 to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/app/(app)/_layout.tsxsrc/components/sidebar/side-menu.tsxsrc/components/ui/drawer/index.tsxsrc/components/status/__tests__/personnel-status-integration.test.tsxsrc/components/home/status-buttons.tsxsrc/components/status/__tests__/personnel-status-bottom-sheet.test.tsxsrc/lib/storage/index.tsxsrc/components/sidebar/__tests__/side-menu.test.tsxsrc/components/status/personnel-status-bottom-sheet.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/app/(app)/_layout.tsxsrc/components/sidebar/side-menu.tsxsrc/components/ui/drawer/index.tsxsrc/components/status/__tests__/personnel-status-integration.test.tsxsrc/stores/status/personnel-status-store.tssrc/components/home/status-buttons.tsxsrc/components/status/__tests__/personnel-status-bottom-sheet.test.tsxsrc/lib/storage/index.tsxsrc/components/sidebar/__tests__/side-menu.test.tsxsrc/components/status/personnel-status-bottom-sheet.tsx
src/components/ui/**/*.tsx
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui components from components/ui; if unavailable, style via StyleSheet.create or styled-components
Files:
src/components/ui/drawer/index.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/components/status/__tests__/personnel-status-integration.test.tsxsrc/components/status/__tests__/personnel-status-bottom-sheet.test.tsxsrc/components/sidebar/__tests__/side-menu.test.tsx
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/status/__tests__/personnel-status-integration.test.tsxsrc/components/status/__tests__/personnel-status-bottom-sheet.test.tsxsrc/components/sidebar/__tests__/side-menu.test.tsx
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to **/*.tsx : Ensure UI is intuitive, user-friendly, and works across different devices and screen sizes
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to **/*.tsx : Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.705Z
Learning: Applies to **/*.{ts,tsx} : Use React Navigation for navigation and deep linking following best practices
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.705Z
Learning: Applies to **/*.tsx : Ensure responsive design across screen sizes and orientations
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to **/*.tsx : Ensure responsive design for different screen sizes and orientations
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to src/**/*.test.{ts,tsx} : Create and use Jest tests to validate all generated components
Applied to files:
tsconfig.json
📚 Learning: 2025-08-21T02:46:42.705Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.705Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create Jest tests for all generated components, services, and logic
Applied to files:
tsconfig.json
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Use React Navigation for handling navigation and deep linking with best practices
Applied to files:
src/app/(app)/_layout.tsx
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to **/*.{ts,tsx} : Use react-i18next for internationalization
Applied to files:
.eslintrc.js
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to src/translations/**/*.{ts,tsx,json} : Store translation dictionary files in src/translations
Applied to files:
.eslintrc.js
📚 Learning: 2025-08-21T02:46:42.705Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.705Z
Learning: Applies to **/*.tsx : Use React.memo for components with static props to prevent unnecessary re-renders
Applied to files:
src/components/sidebar/side-menu.tsx
📚 Learning: 2025-08-21T02:46:42.705Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.705Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Applied to files:
src/components/sidebar/side-menu.tsxsrc/components/status/personnel-status-bottom-sheet.tsx
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use gluestack-ui icon component
Applied to files:
src/components/sidebar/side-menu.tsxsrc/components/status/personnel-status-bottom-sheet.tsx
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to **/*.{ts,tsx} : Use react-native-mmkv for local storage
Applied to files:
src/lib/storage/index.tsx
📚 Learning: 2025-08-21T02:46:42.705Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.705Z
Learning: Applies to **/*.{ts,tsx} : Use Expo SecureStore for sensitive data storage
Applied to files:
src/lib/storage/index.tsx
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Use Expo's SecureStore for sensitive data
Applied to files:
src/lib/storage/index.tsx
📚 Learning: 2025-08-21T02:47:24.070Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.070Z
Learning: Applies to src/components/ui/**/*.tsx : Use gluestack-ui components from components/ui; if unavailable, style via StyleSheet.create or styled-components
Applied to files:
src/components/status/personnel-status-bottom-sheet.tsx
🧬 Code graph analysis (3)
src/components/status/__tests__/personnel-status-integration.test.tsx (3)
src/stores/app/core-store.ts (1)
useCoreStore(48-222)src/stores/status/personnel-status-store.ts (1)
usePersonnelStatusBottomSheetStore(55-269)src/components/status/personnel-status-bottom-sheet.tsx (1)
PersonnelStatusBottomSheet(22-531)
src/components/status/__tests__/personnel-status-bottom-sheet.test.tsx (1)
__mocks__/@gorhom/bottom-sheet.js (1)
require(3-3)
src/components/sidebar/__tests__/side-menu.test.tsx (2)
src/stores/app/livekit-store.ts (1)
useLiveKitStore(95-329)src/components/sidebar/side-menu.tsx (1)
SideMenu(35-273)
🪛 LanguageTool
docs/personnel-status-refactoring.md
[grammar] ~9-~9: There might be a mistake here.
Context: ... Changes Made ### 1. UI/UX Improvements - **Replaced Radio Buttons with Outline Styl...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...lity ### 2. Bug Fixes and Logic Changes - Removed Destination Requirements: "No ...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...selection ### 3. Technical Improvements - Removed Radio Dependencies: Eliminated...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...n should be enabled ### 4. Test Updates - Updated Component Tests: Modified exis...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...ter User Control**: Visual close button provides clear way to exit the bottom sheet 5. *...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...uch targets ## Testing All tests pass: - 49 existing component tests ✅ - 9 enhanc...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...ts pass: - 49 existing component tests ✅ - 9 enhanced integration tests ✅ - 31 sto...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...tests ✅ - 9 enhanced integration tests ✅ - 31 store tests ✅ - 1 analytics test ✅ - ...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ... integration tests ✅ - 31 store tests ✅ - 1 analytics test ✅ - 3 analytics integra...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ... - 31 store tests ✅ - 1 analytics test ✅ - 3 analytics integration tests ✅ Total: ...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...1 skipped ## Visual Changes ### Before - Small, hard-to-see radio buttons - Minim...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...efore - Small, hard-to-see radio buttons - Minimal visual feedback for selection - ...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ... - Minimal visual feedback for selection - Next button sometimes blocked by destina...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...imes blocked by destination requirements - No visual close button ### After - Larg...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ... Large, clearly outlined selection areas - Prominent checkmark icons for selected i...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...inent checkmark icons for selected items - "No Destination" always valid regardless...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ..." always valid regardless of status type - Visual "X" close button in header - Cons...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...type - Visual "X" close button in header - Consistent with app's design language - ...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... - Consistent with app's design language - Reliable Next button functionality ## L...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...ic Changes ### Destination Requirements Previously, the logic checked the status...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...pically expect one. ### UI Improvements - Added close button positioned in the top...
(QB_NEW_EN)
docs/personnel-status-bottom-sheet-final-refactor.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...et - Final Refactor Summary ## Overview Complete refactor of the personnel statu...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...s Implemented ### 1. UI/UX Improvements - Radio Buttons → Outline Styling: Repla...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...mobile users ### 2. Functionality Fixes - Next Button: Fixed non-working progres...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...avigation ### 3. Business Logic Updates - Destination Requirements Removed: "No ...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...ination ### 4. Enhanced User Experience - Close Button: Added visual "X" icon in...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...` ## Testing Updates ### Test Coverage - Component Tests: Updated mocks for Luc...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...pdated mocks for Lucide icons (Check, X) - Integration Tests: New test cases for ...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...est cases for close button functionality - Store Tests: Revised logic tests for s...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ...ests for simplified destination handling - Mock Updates: Enhanced icon mocks for ...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...etter test reliability ### Test Results - ✅ 93 tests passing - ✅ 1 test skipped (e...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...y ### Test Results - ✅ 93 tests passing - ✅ 1 test skipped (expected) - ✅ 4 test s...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...ts passing - ✅ 1 test skipped (expected) - ✅ 4 test suites completed - ✅ No TypeScr...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...d (expected) - ✅ 4 test suites completed - ✅ No TypeScript compilation errors ## Q...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ... ## Quality Assurance ### Code Quality - TypeScript: Strict typing maintained, ...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ...abels and roles ### Mobile Optimization - Touch Targets: Adequate size for mobil...
(QB_NEW_EN)
[grammar] ~90-~90: There might be a mistake here.
Context: ...**: Adequate size for mobile interaction - Visual Feedback: Clear selection state...
(QB_NEW_EN)
[grammar] ~91-~91: There might be a mistake here.
Context: ...Clear selection states and hover effects - Cross-Platform: Tested for both iOS an...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...d for both iOS and Android compatibility - Dark Mode: Full support for light and ...
(QB_NEW_EN)
[uncategorized] ~98-~98: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ing Changes - None: All changes are backward compatible - API Stability: No changes to comp...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~103-~103: There might be a mistake here.
Context: ...r visual feedback and easier interaction - Reduced Friction: No forced destinatio...
(QB_NEW_EN)
[grammar] ~104-~104: There might be a mistake here.
Context: ...ction**: No forced destination selection - Enhanced Accessibility: Better mobile ...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ... Better mobile and screen reader support - Consistent UX: Aligned with app-wide d...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ... Readiness ### Pre-Deployment Checklist - ✅ All unit tests passing - ✅ Integration...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...ent Checklist - ✅ All unit tests passing - ✅ Integration tests updated and passing ...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ... ✅ Integration tests updated and passing - ✅ TypeScript compilation clean - ✅ ESLin...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...passing - ✅ TypeScript compilation clean - ✅ ESLint validation complete - ✅ Dark mo...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...ion clean - ✅ ESLint validation complete - ✅ Dark mode support verified - ✅ Accessi...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ... complete - ✅ Dark mode support verified - ✅ Accessibility standards met - ✅ Mobile...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...verified - ✅ Accessibility standards met - ✅ Mobile optimization confirmed ### Pos...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...onfirmed ### Post-Deployment Monitoring - Monitor user interaction patterns with n...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...status/personnel-status-bottom-sheet.tsx** - Complete UI refactor to outline styling ...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: .../stores/status/personnel-status-store.ts** - Simplified nextStep() logic - Removed...
(QB_NEW_EN)
[grammar] ~137-~137: There might be a mistake here.
Context: ... backward compatibility 3. Test Files - Updated icon mocks - Added close butt...
(QB_NEW_EN)
[grammar] ~138-~138: There might be a mistake here.
Context: .... Test Files - Updated icon mocks - Added close button tests - Revised bu...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ...icon mocks - Added close button tests - Revised business logic tests - Enhanc...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ... tests - Revised business logic tests - Enhanced test coverage 4. **Documentati...
(QB_NEW_EN)
[grammar] ~143-~143: There might be a mistake here.
Context: ...hanced test coverage 4. Documentation - Comprehensive refactor documentation ...
(QB_NEW_EN)
[grammar] ~144-~144: There might be a mistake here.
Context: ... - Comprehensive refactor documentation - Implementation guides - Quality assur...
(QB_NEW_EN)
[grammar] ~145-~145: There might be a mistake here.
Context: ...documentation - Implementation guides - Quality assurance reports ## Conclusion...
(QB_NEW_EN)
🪛 Gitleaks (8.27.2)
src/lib/storage/index.tsx
13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
src/components/home/status-buttons.tsx (1)
24-26: Loading gate uses home store while data comes from core storeMinor consistency check: isLoadingOptions comes from useHomeStore but the data now comes from useCoreStore. Make sure the loading flag reflects the same fetch lifecycle to avoid flicker.
If core store exposes a loading state for activeStatuses, consider using it here for tighter coupling.
package.json (2)
136-136: react-dom aligned to 18.3.1 with react 18.3.1: LGTMThis keeps RN Web parity and avoids the pre-19 interop issues.
121-121: expo-secure-store version is correct for Expo SDK 52According to the Expo SDK 52 documentation, the bundled version of
expo-secure-storeis exactly~14.0.1—so the change in yourpackage.jsonalready matches the official recommendation and no adjustment is needed. (docs.expo.dev)Likely an incorrect or invalid review comment.
src/stores/status/personnel-status-store.ts (1)
117-129: Confirmed consistent use ofnextStep/previousStepAPIs
All call sites across stores, components, and tests invokenextStep(andpreviousStepfor back navigation). No instances ofgoToNextStepwere found after auditing with ripgrep. This verifies there’s a single step-advance API in use.src/app/(app)/_layout.tsx (2)
47-51: Drawer close handler LGTM.Memoized handleNavigate cleanly closes the drawer and avoids re-renders. This aligns with the SideMenu’s onNavigate sequencing.
250-251: SideMenu wiring in Drawer LGTM.Passing onNavigate={handleNavigate} ensures the drawer closes after navigation. For landscape (permanent menu), omitting onNavigate is appropriate.
src/lib/storage/index.tsx (1)
44-50: useIsFirstTime hook LGTM.Clear, simple Boolean gate using useMMKVBoolean with sensible defaults.
src/components/sidebar/__tests__/side-menu.test.tsx (1)
98-142: Solid test scaffolding and store mocks.Nice isolation of stores and deterministic UI theme. This keeps navigation tests focused and reliable.
docs/personnel-status-bottom-sheet-final-refactor.md (1)
8-12: Solid, clear summary that mirrors the UI and logic changes.The document concisely captures the radio→touchable refactor, always-valid “No Destination,” and the new close button. It aligns with the component and store changes.
Also applies to: 17-20, 21-25
docs/personnel-status-refactoring.md (2)
9-14: Good change log; mirrors the implementation and tests.The refactor scope and the migration safety notes are clear and match code.
Also applies to: 20-24
80-92: Document the “auto-select active call” caveat.Current implementation auto-selects an active call when responseType is 'none'. This can unintentionally override a user’s explicit “No Destination” choice (see component comment). Consider adding a note: “Auto-selection occurs only on initial open and never after the user explicitly selects ‘No Destination.’”
Would you like me to submit a docs PR note once the code fix lands?
| describe('Navigation Animation Improvements', () => { | ||
| it('should handle navigation with improved timing to prevent animation stuttering', async () => { | ||
| const mockOnNavigate = jest.fn(); | ||
|
|
||
| // Reset all mocks to ensure proper setup | ||
| const mockUseLiveKitStore = useLiveKitStore as jest.MockedFunction<typeof useLiveKitStore>; | ||
| mockUseLiveKitStore.mockReturnValue({ | ||
| isConnected: false, | ||
| setIsBottomSheetVisible: jest.fn(), | ||
| toggleMicrophone: jest.fn(), | ||
| }); | ||
|
|
||
| const mockUseAudioStreamStore = useAudioStreamStore as jest.MockedFunction<typeof useAudioStreamStore>; | ||
| mockUseAudioStreamStore.mockReturnValue({ | ||
| currentStream: null, | ||
| isPlaying: false, | ||
| setIsBottomSheetVisible: jest.fn(), | ||
| }); | ||
|
|
||
| render(<SideMenu onNavigate={mockOnNavigate} />); | ||
|
|
||
| const homeMenuItem = screen.getByTestId('side-menu-home'); | ||
| fireEvent.press(homeMenuItem); | ||
|
|
||
| // onNavigate should be called after the navigation and delay | ||
| await waitFor( | ||
| () => { | ||
| expect(mockOnNavigate).toHaveBeenCalled(); | ||
| }, | ||
| { timeout: 150 } // Increase timeout to account for requestAnimationFrame + 50ms delay | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make timing test deterministic by using fake timers and a rAF shim.
Depending on device/CI load, waitFor with a 150ms window can flake. Use Jest fake timers and shim requestAnimationFrame to a zero-timeout, then advance timers.
Apply:
describe('Navigation Animation Improvements', () => {
it('should handle navigation with improved timing to prevent animation stuttering', async () => {
const mockOnNavigate = jest.fn();
+ jest.useFakeTimers();
+ const originalRaf = global.requestAnimationFrame;
+ // Shim rAF to timers for deterministic behavior
+ (global as any).requestAnimationFrame = (cb: FrameRequestCallback) => setTimeout(cb, 0);
// Reset all mocks to ensure proper setup
@@
- // onNavigate should be called after the navigation and delay
- await waitFor(
- () => {
- expect(mockOnNavigate).toHaveBeenCalled();
- },
- { timeout: 150 } // Increase timeout to account for requestAnimationFrame + 50ms delay
- );
+ // Flush rAF and the 50ms timeout used by the component
+ jest.runOnlyPendingTimers(); // rAF -> setTimeout 0
+ jest.advanceTimersByTime(50); // close delay
+ expect(mockOnNavigate).toHaveBeenCalled();
+
+ // Restore timer state and rAF
+ jest.useRealTimers();
+ (global as any).requestAnimationFrame = originalRaf;
});If you adopt InteractionManager in the component (no 50ms timeout), adjust the timer flushing accordingly.
📝 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('Navigation Animation Improvements', () => { | |
| it('should handle navigation with improved timing to prevent animation stuttering', async () => { | |
| const mockOnNavigate = jest.fn(); | |
| // Reset all mocks to ensure proper setup | |
| const mockUseLiveKitStore = useLiveKitStore as jest.MockedFunction<typeof useLiveKitStore>; | |
| mockUseLiveKitStore.mockReturnValue({ | |
| isConnected: false, | |
| setIsBottomSheetVisible: jest.fn(), | |
| toggleMicrophone: jest.fn(), | |
| }); | |
| const mockUseAudioStreamStore = useAudioStreamStore as jest.MockedFunction<typeof useAudioStreamStore>; | |
| mockUseAudioStreamStore.mockReturnValue({ | |
| currentStream: null, | |
| isPlaying: false, | |
| setIsBottomSheetVisible: jest.fn(), | |
| }); | |
| render(<SideMenu onNavigate={mockOnNavigate} />); | |
| const homeMenuItem = screen.getByTestId('side-menu-home'); | |
| fireEvent.press(homeMenuItem); | |
| // onNavigate should be called after the navigation and delay | |
| await waitFor( | |
| () => { | |
| expect(mockOnNavigate).toHaveBeenCalled(); | |
| }, | |
| { timeout: 150 } // Increase timeout to account for requestAnimationFrame + 50ms delay | |
| ); | |
| }); | |
| describe('Navigation Animation Improvements', () => { | |
| it('should handle navigation with improved timing to prevent animation stuttering', async () => { | |
| const mockOnNavigate = jest.fn(); | |
| jest.useFakeTimers(); | |
| const originalRaf = global.requestAnimationFrame; | |
| // Shim rAF to timers for deterministic behavior | |
| (global as any).requestAnimationFrame = (cb: FrameRequestCallback) => setTimeout(cb, 0); | |
| // Reset all mocks to ensure proper setup | |
| const mockUseLiveKitStore = useLiveKitStore as jest.MockedFunction<typeof useLiveKitStore>; | |
| mockUseLiveKitStore.mockReturnValue({ | |
| isConnected: false, | |
| setIsBottomSheetVisible: jest.fn(), | |
| toggleMicrophone: jest.fn(), | |
| }); | |
| const mockUseAudioStreamStore = useAudioStreamStore as jest.MockedFunction<typeof useAudioStreamStore>; | |
| mockUseAudioStreamStore.mockReturnValue({ | |
| currentStream: null, | |
| isPlaying: false, | |
| setIsBottomSheetVisible: jest.fn(), | |
| }); | |
| render(<SideMenu onNavigate={mockOnNavigate} />); | |
| const homeMenuItem = screen.getByTestId('side-menu-home'); | |
| fireEvent.press(homeMenuItem); | |
| // Flush rAF and the 50ms timeout used by the component | |
| jest.runOnlyPendingTimers(); // rAF -> setTimeout(…, 0) | |
| jest.advanceTimersByTime(50); // advance the component’s delay | |
| expect(mockOnNavigate).toHaveBeenCalled(); | |
| // Restore timer state and rAF | |
| jest.useRealTimers(); | |
| (global as any).requestAnimationFrame = originalRaf; | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In src/components/sidebar/__tests__/side-menu.test.tsx around lines 469 to 500,
the timing-based assertion is flaky because it relies on a real 150ms wait;
update the test to use Jest fake timers and shim requestAnimationFrame to a
zero-timeout before rendering, trigger the press, then advance timers (and flush
any pending rAF callbacks) so the onNavigate assertion is deterministic; if the
component uses InteractionManager instead of a 50ms timeout, advance/fush timers
accordingly to cover the InteractionManager callbacks as well.
| if (Platform.OS === 'web') { | ||
| storage = new MMKV({ | ||
| id: 'ResgridResponder', | ||
| }); | ||
| } else { | ||
| // Initialize storage asynchronously | ||
| let storageInitialized = false; | ||
| const initializeStorage = async () => { | ||
| if (storageInitialized) return; | ||
| storageInitialized = true; | ||
| try { | ||
| storage = await getGeneralStorage(); | ||
| } catch (error) { | ||
| storageInitialized = false; | ||
| throw error; | ||
| } | ||
| }; | ||
| // Initialize secure storage | ||
| initializeStorage().catch((error) => { | ||
| console.error('Failed to initialize secure storage:', error); | ||
| storage = new MMKV({ | ||
| id: 'ResgridResponder', | ||
| encryptionKey: 'a330fb72-1916-4cc2-99fe-efd4986b254f', | ||
| }); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Web support for react-native-mmkv is uncertain — add a safe fallback.
Directly instantiating MMKV on web can break builds. Consider falling back to a no-op/in-memory storage on web or AsyncStorage.
I can wire a web fallback adapter with the same StateStorage interface so zustand persists seamlessly across platforms.
🌐 Web query:
Does react-native-mmkv support Expo Web as of August 2025? If not, what is the recommended storage fallback on web (e.g., AsyncStorage, localStorage, or an MMKV-compatible shim)?
💡 Result:
As of August 2025, react-native-mmkv does not support Expo Web. Although MMKV has some support for web via shims, official and practical support for Expo Web is lacking—meaning you cannot reliably use MMKV when targeting Expo Web builds[1][3][4][5].
Details and Current Status
- MMKV is a JSI-based native storage library, which requires native code access not available in the standard Expo managed workflow[1][4]. This makes it unavailable for Expo Web, which does not run native code but relies on JavaScript-only solutions.
- MMKV’s own documentation and tutorials continue to say it works only in bare React Native or ejected (custom dev client) Expo projects—not in the managed Expo workflow, and not on Expo Web[1][3][4].
- There has been progress on C++-based modules for React Native and improvements to cross-platform support with the new architecture[5][9], but there is no evidence from any authoritative source that MMKV natively supports Expo Web as of mid-2025.
Recommended Storage Fallbacks on Expo Web
When building for Expo Web, the standard and recommended local storage solutions remain unchanged:
- AsyncStorage: Works on web, using
window.localStorageunder the hood. Suitable for small to moderate amounts of persistent, non-sensitive data[6][8]. - localStorage: You can also use the native Web API (
window.localStorage) directly if you do not need a cross-platform abstraction. - SecureStore: Not available on web, only on mobile platforms[6].
- SQLite: For structured/local database storage,
expo-sqliteis available and works on web, but is heavier-weight than simple key-value storage[6][7].
There is no reliable MMKV-compatible shim for Expo Web in 2025. If you want the same API across native and web, you will need to write your own polyfill or use a library that can swap backend implementations based on platform.
Suggested Implementation Pattern
If you need a unified storage API across native and web:
import { Platform } from 'react-native';
import AsyncStorage from '@react-native-async-storage/async-storage';
// Prefer MMKV on native, AsyncStorage on web
let Storage;
if (Platform.OS === 'web') {
Storage = AsyncStorage; // or a direct localStorage wrapper
} else {
// Use MMKV; be sure to import only when not on web
const { MMKV } = require('react-native-mmkv');
Storage = new MMKV();
}
// Usage example
Storage.setItem('key', 'value');Summary:
If your app must support Expo Web, you should use AsyncStorage or localStorage for persistence. MMKV can be reserved for native builds only. There is no official, production-grade MMKV replacement or shim for Expo Web as of August 2025[1][4][5][6][8].
If you need more advanced features (like encryption or structured storage), consider SQLite (via expo-sqlite)[6][7]. For sensitive data, use SecureStore on mobile, but fall back to regular storage (with appropriate warning) on web[6].
Citations:
- 1: https://blog.logrocket.com/using-react-native-mmkv-improve-app-performance/
- 2: https://bobcares.com/blog/alternatives-to-async-storage-react-native/
- 3: https://www.youtube.com/watch?v=7N5fug4O-zg
- 4: https://ignitecookbook.com/docs/recipes/MigratingToMMKV/
- 5: https://github.com/mrousavy/react-native-mmkv
- 6: https://dev.to/snehasishkonger/methods-of-storing-local-data-in-react-native-expo-mc0
- 7: https://docs.expo.dev/guides/local-first/
- 8: https://reactnativeexpert.com/blog/mmkv-vs-asyncstorage-in-react-native/
- 9: https://reactnative.dev/blog/2024/10/23/the-new-architecture-is-here
- 10: https://www.metacto.com/blogs/expo-alternatives-and-competitors-the-definitive-guide-for-2024
Add a web-safe storage fallback for Expo Web builds
MMKV is a JSI-based native module and is not supported in the Expo managed workflow or on Expo Web as of August 22, 2025. Trying to instantiate it on web will break your build. You should switch to a JavaScript-only storage backend—such as AsyncStorage (which uses window.localStorage under the hood) or a simple localStorage wrapper—when Platform.OS === 'web', and reserve MMKV for native platforms.
• File src/lib/storage/index.tsx, lines 6–15:
Replace the direct MMKV instantiation on web with an AsyncStorage/localStorage fallback.
• You’ll need to install and import @react-native-async-storage/async-storage (or write a minimal localStorage wrapper) to preserve your StateStorage interface across platforms.
Suggested diff:
import { Platform } from 'react-native';
+import AsyncStorage from '@react-native-async-storage/async-storage';
import { MMKV } from 'react-native-mmkv';
let storage: StateStorage;
-if (Platform.OS === 'web') {
- storage = new MMKV({
- id: 'ResgridResponder',
- });
-} else {
- storage = new MMKV({
- id: 'ResgridResponder',
- encryptionKey: 'a330fb72-1916-4cc2-99fe-efd4986b254f',
- });
-}
+if (Platform.OS === 'web') {
+ // Use AsyncStorage (or a direct localStorage wrapper) on web
+ storage = {
+ getItem: AsyncStorage.getItem,
+ setItem: AsyncStorage.setItem,
+ removeItem: AsyncStorage.removeItem,
+ };
+} else {
+ // Use encrypted MMKV on native
+ storage = new MMKV({
+ id: 'ResgridResponder',
+ encryptionKey: 'a330fb72-1916-4cc2-99fe-efd4986b254f',
+ });
+}
export default storage;This ensures your persistence layer won’t break on Expo Web while keeping MMKV’s performance and encryption on iOS/Android.
📝 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.
| if (Platform.OS === 'web') { | |
| storage = new MMKV({ | |
| id: 'ResgridResponder', | |
| }); | |
| } else { | |
| // Initialize storage asynchronously | |
| let storageInitialized = false; | |
| const initializeStorage = async () => { | |
| if (storageInitialized) return; | |
| storageInitialized = true; | |
| try { | |
| storage = await getGeneralStorage(); | |
| } catch (error) { | |
| storageInitialized = false; | |
| throw error; | |
| } | |
| }; | |
| // Initialize secure storage | |
| initializeStorage().catch((error) => { | |
| console.error('Failed to initialize secure storage:', error); | |
| storage = new MMKV({ | |
| id: 'ResgridResponder', | |
| encryptionKey: 'a330fb72-1916-4cc2-99fe-efd4986b254f', | |
| }); | |
| } | |
| import { Platform } from 'react-native'; | |
| import AsyncStorage from '@react-native-async-storage/async-storage'; | |
| import { MMKV } from 'react-native-mmkv'; | |
| let storage: StateStorage; | |
| if (Platform.OS === 'web') { | |
| // Use AsyncStorage (or a direct localStorage wrapper) on web | |
| storage = { | |
| getItem: AsyncStorage.getItem, | |
| setItem: AsyncStorage.setItem, | |
| removeItem: AsyncStorage.removeItem, | |
| }; | |
| } else { | |
| // Use encrypted MMKV on native | |
| storage = new MMKV({ | |
| id: 'ResgridResponder', | |
| encryptionKey: 'a330fb72-1916-4cc2-99fe-efd4986b254f', | |
| }); | |
| } | |
| export default storage; |
🧰 Tools
🪛 Gitleaks (8.27.2)
13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In src/lib/storage/index.tsx around lines 6 to 15, the code instantiates MMKV on
web which breaks Expo Web/managed workflow; replace the MMKV creation for
Platform.OS === 'web' with a JS-only storage fallback (e.g.,
@react-native-async-storage/async-storage or a minimal localStorage wrapper)
that implements the same StateStorage interface used by the app, and only
instantiate MMKV for native platforms (iOS/Android) with the encryptionKey.
Install and import the chosen async-storage package (or add a small wrapper) and
wire it so consumers of this module use the same API regardless of platform,
ensuring no MMKV code runs when Platform.OS === 'web'.
| id: 'ResgridResponder', | ||
| encryptionKey: 'a330fb72-1916-4cc2-99fe-efd4986b254f', | ||
| }); | ||
| } |
There was a problem hiding this comment.
Hardcoded encryptionKey committed to source — remove immediately and rotate.
A static encryptionKey is a secret and was flagged by Gitleaks. This risks data exposure if APK/IPA or repo is leaked.
Apply one of these immediate mitigations (Option A is simplest and safe if MMKV only stores non-PII):
Option A — Remove encryption entirely (and ensure PII uses SecureStore):
storage = new MMKV({
id: 'ResgridResponder',
- encryptionKey: 'a330fb72-1916-4cc2-99fe-efd4986b254f',
});Option B — Load a per-install key from SecureStore (requires minor refactor to async-init at app bootstrap). I can generate a SecureStore-backed initializer if you want to adopt this approach.
📝 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.
| storage = new MMKV({ | |
| id: 'ResgridResponder', | |
| encryptionKey: 'a330fb72-1916-4cc2-99fe-efd4986b254f', | |
| }); | |
| storage = new MMKV({ | |
| id: 'ResgridResponder', | |
| }); |
🧰 Tools
🪛 Gitleaks (8.27.2)
13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In src/lib/storage/index.tsx around lines 11-14, a hardcoded encryptionKey was
committed; remove the static key immediately and rotate it. If MMKV stores only
non-PII use Option A: remove the encryptionKey argument so storage is created
without a hardcoded secret and ensure any PII is moved to a SecureStore
implementation. If MMKV must be encrypted use Option B: refactor initialization
to async at app bootstrap, read a per-install key from SecureStore (generating
and storing one on first run), and use that key for MMKV; after deployment
rotate the compromised key and invalidate any data encrypted with it as needed.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores
Tests