feat: expo ui migration phase 1 + 2#2592
Merged
Merged
Conversation
…r exports, migrate LargeTitleHeader/SearchInput to expo-router native patterns
- packages/ui/nativewindui/index.ts: mark Phase 1 + 2 as done, preserve Phase 3–5 tracking structure - biome.json: add organizeImports:off override for the adapter tracker file so Biome doesn't collapse per-phase exports - scripts/lint/nativewindui-migration.ts: CI script counting remaining adapter exports per phase + bypass-import detection - package.json: add check:migration script entry - docs/migrations/nativewindui-to-expo-ui.md: full migration plan with phase breakdown, replacement map, and PR checklist - docs/ideation/2026-06-13-nativewindui-to-expo-ui-ideation.md: ranked approach ideation doc - apps/expo/app/_layout.web.tsx: fix direct @packrat-ai/nativewindui import → adapter - remaining files: Biome import-order corrections applied by bun lint
github-actions
Bot
added
documentation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces ChangesNativeWindUI → Expo Router Header Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Coverage Report for packages/units (./packages/units)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/analytics (./packages/analytics)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/mcp (./packages/mcp)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/overpass (./packages/overpass)
File CoverageNo changed files found. |
Contributor
Coverage Report for apps/expo (./apps/expo)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/api (./packages/api)
File CoverageNo changed files found. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 (5)
apps/expo/features/ai/components/WeatherGenerativeUI.tsx (1)
84-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't wrap error strings in
new Error()beforecaptureException.Lines 84 and 102 create new Error objects from error strings, which generates a misleading stack trace pointing to this wrapping location rather than preserving the original error context. Per coding guidelines, "In apps/expo, never wrap the root error in
new Error(...)before passing to SentrycaptureException."Use
Sentry.captureMessage()for string errors, or if you need exception-level severity, pass the string via thecontextsorextrafield without wrapping.🔧 Proposed fix to use captureMessage for string errors
} else { Sentry.addBreadcrumb({ category: 'ai.weather', message: `Weather tool: failed for "${toolInvocation.input.location}"`, level: 'error', data: { toolCallId, location: toolInvocation.input.location, error: toolInvocation.output.error, }, }); - Sentry.captureException(new Error(toolInvocation.output.error), { + Sentry.captureMessage(toolInvocation.output.error, { + level: 'error', contexts: { ai_tool: { tool: 'getWeatherForLocation', location: toolInvocation.input.location }, }, tags: { ai_tool: 'getWeatherForLocation', ai_mode: 'cloud' }, }); } } else if (toolInvocation.state === 'output-error') { Sentry.addBreadcrumb({ category: 'ai.weather', message: 'Weather tool: stream error', level: 'error', data: { toolCallId, location: toolInvocation.input.location, errorText: toolInvocation.errorText, }, }); - Sentry.captureException(new Error(toolInvocation.errorText), { + Sentry.captureMessage(toolInvocation.errorText, { + level: 'error', contexts: { ai_tool: { tool: 'getWeatherForLocation', location: toolInvocation.input.location }, }, tags: { ai_tool: 'getWeatherForLocation', ai_mode: 'cloud' }, }); }Also applies to: 102-107
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/ai/components/WeatherGenerativeUI.tsx` around lines 84 - 90, The issue is wrapping error strings in new Error() before passing them to Sentry.captureException(), which creates misleading stack traces pointing to the wrapping location rather than the original error context. Replace the new Error(toolInvocation.output.error) pattern with Sentry.captureMessage() for string errors, or alternatively pass the error string directly via the contexts or extra field without creating a new Error object. This pattern occurs at the location shown (line 84) and also at line 102-107, both of which need to be updated with the same approach.Source: Coding guidelines
apps/expo/features/ai-packs/screens/AIPacksScreen.tsx (1)
93-99:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove legacy top inset padding now that native header is enabled.
After adding
Stack.Screenat Line 94, the extra top padding at Line 93 and Line 98 likely double-offsets content on iOS (header + manual inset), leaving a visible gap.Suggested fix
- <SafeAreaView className="flex-1" style={{ paddingTop: Platform.OS === 'ios' ? insets.top : 0 }}> + <SafeAreaView className="flex-1"> <Stack.Screen options={{ title: t('ai.aiPacksAdmin'), headerLargeTitle: true }} /> <View className="px-4 py-6 space-y-6" - style={{ paddingTop: Platform.OS === 'ios' ? insets.top + 22 : 0 }} + style={{ paddingTop: Platform.OS === 'ios' ? 22 : 0 }} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/ai-packs/screens/AIPacksScreen.tsx` around lines 93 - 99, The manual top padding values on both the SafeAreaView component (using insets.top) and the nested View component (using insets.top + 22) are now redundant and causing double-offset on iOS after enabling the native header with Stack.Screen. Remove the style prop with the conditional Platform.OS === 'ios' padding from the SafeAreaView, and remove the style prop with the conditional paddingTop from the View component, since the native header now handles the proper spacing automatically.apps/expo/app/(app)/messages/conversations.android.tsx (1)
6-15:⚠️ Potential issue | 🔴 CriticalTypeScript build failure: missing exports in the adapter. The
@packrat/ui/nativewinduiadapter doesn't export the imported components, causing TS2305 errors.
apps/expo/app/(app)/messages/conversations.android.tsx#L6-L15:ContextMenu,createContextItem,createDropdownItem,DropdownMenu,Toolbar,ToolbarCTAare not exported from the adapter.apps/expo/app/(app)/messages/conversations.tsx#L6-L15:Checkbox,ContextMenu,createContextItem,createDropdownItem,DropdownMenu,Toolbarare not exported from the adapter.Per the migration tracker (docs/migrations/nativewindui-to-expo-ui.md),
ContextMenu/DropdownMenushould come frompackages/ui/src/context-menu.ios.tsxand.android.tsxrespectively.Toolbar/ToolbarCTAare navigation-layer (Stack.Toolbar). Redirect these imports to their correct sources or add missing exports to the adapter if they exist in@packrat-ai/nativewinduiv2.2.1.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/messages/conversations.android.tsx around lines 6 - 15, The import statements in conversations.android.tsx (lines 6-15) are attempting to import ContextMenu, createContextItem, createDropdownItem, DropdownMenu, Toolbar, and ToolbarCTA from the `@packrat/ui/nativewindui` adapter, but these components are not exported there. According to the migration guide, redirect the imports as follows: ContextMenu, createContextItem, createDropdownItem, and DropdownMenu should be imported from packages/ui/src/context-menu.android.tsx, while Toolbar and ToolbarCTA should be imported from the navigation-layer Stack.Toolbar. Similarly, in conversations.tsx (lines 6-15), redirect Checkbox, ContextMenu, createContextItem, createDropdownItem, and DropdownMenu to packages/ui/src/context-menu.ios.tsx (for the context menu components, using the .ios variant), and Toolbar to the navigation-layer Stack.Toolbar. Remove or comment out any imports of these components from the `@packrat/ui/nativewindui` adapter in both files and add the correct import statements from the identified sources.apps/expo/features/catalog/screens/CatalogItemsScreen.tsx (2)
79-91: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMissing
useCallbackon handlers passed to list items.
handleGroupPress,handleItemPress, andhandleRefreshare passed toCatalogItemCard(withinFlatList.renderItemand search resultsmap) but are not wrapped inuseCallback. This breaks memoization and causes unnecessary re-renders of list items.♻️ Wrap handlers in useCallback
- const handleGroupPress = (group: CatalogItemGroup) => { + const handleGroupPress = useCallback((group: CatalogItemGroup) => { setGroupVariants(group.variants); router.push({ pathname: '/catalog/[id]', params: { id: group.representative.id } }); - }; + }, [router, setGroupVariants]); - const handleItemPress = (item: CatalogItem) => { + const handleItemPress = useCallback((item: CatalogItem) => { router.push({ pathname: '/catalog/[id]', params: { id: item.id } }); - }; + }, [router]); - const handleRefresh = async () => { + const handleRefresh = useCallback(async () => { setIsManualRefresh(true); await refetch(); setIsManualRefresh(false); - }; + }, [refetch]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/catalog/screens/CatalogItemsScreen.tsx` around lines 79 - 91, Wrap the three handler functions `handleGroupPress`, `handleItemPress`, and `handleRefresh` with the `useCallback` hook from React. For `handleGroupPress`, include `setGroupVariants` and `router` in the dependency array; for `handleItemPress`, include `router` in the dependency array; and for `handleRefresh`, include `setIsManualRefresh` and `refetch` in the dependency array. This will ensure that the handler references remain stable across re-renders, preserving memoization of the list item components that receive these handlers as props.Source: Coding guidelines
133-259:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAll screens render both search UI and main list simultaneously when searching.
When the user enters a search query, all five screens render both the search results UI (ScrollView or search FlatList) and the main FlatList at the same time. This creates confusing UX (two scrollable areas compete for space), wastes rendering performance, and in most cases displays duplicate data.
apps/expo/features/catalog/screens/CatalogItemsScreen.tsx#L133-L259: conditionally render either the search ScrollView (L133-184) OR the main FlatList (L187-259), not both.apps/expo/features/guides/screens/GuidesListScreen.tsx#L207-L232:renderSearchContent()returns a FlatList with search results, but the main FlatList also renders the sameguidesdata. Render only one FlatList.apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx#L200-L253:renderSearchContent()returns search results ScrollView while main FlatList also rendersfilteredTemplates(same data). Conditionally render one or the other.apps/expo/features/packs/screens/PackListScreen.tsx#L196-L287:SearchResultscomponent and main FlatList both renderfilteredPackswhen searching. Render only one.apps/expo/features/trips/screens/TripListScreen.tsx#L167-L183:renderSearchContent()returns search ScrollView while main FlatList also rendersfilteredTrips. Conditionally render one or the other.🔧 Example fix pattern for CatalogItemsScreen
return ( <> <Stack.Screen ... /> - {isSearching ? ( - <ScrollView contentContainerStyle={{ paddingBottom: 40 }}> - {/* search results */} - </ScrollView> - ) : null} - - <FlatList - data={groupedItems} - ... - /> + {isSearching ? ( + <ScrollView contentContainerStyle={{ paddingBottom: 40 }}> + {/* search results */} + </ScrollView> + ) : ( + <FlatList + data={groupedItems} + ... + /> + )} </> );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/catalog/screens/CatalogItemsScreen.tsx` around lines 133 - 259, The issue spans five files where both search UI and main list components render simultaneously when searching, creating confusing UX and wasting performance. In apps/expo/features/catalog/screens/CatalogItemsScreen.tsx (lines 133-259), wrap the FlatList component (starting around line 187) in a conditional that only renders when isSearching is false, so either the search ScrollView OR the main FlatList displays, never both. In apps/expo/features/guides/screens/GuidesListScreen.tsx (lines 207-232), the renderSearchContent() function returns search results but the main FlatList also renders the guides data; conditionally render only one list based on the search state. In apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx (lines 200-253), conditionally render either the renderSearchContent() ScrollView with search results OR the main FlatList with filteredTemplates, not both. In apps/expo/features/packs/screens/PackListScreen.tsx (lines 196-287), the SearchResults component and main FlatList both render filteredPacks when searching; conditionally display only one based on search state. In apps/expo/features/trips/screens/TripListScreen.tsx (lines 167-183), the renderSearchContent() ScrollView and main FlatList with filteredTrips both render during search; conditionally render one or the other based on search state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/expo/features/packs/screens/PackListScreen.tsx`:
- Around line 196-287: Remove the `as any` type casts from the SearchResults
component's results prop and the PackCard component's pack prop, both of which
are masking a type divergence between pack type definitions. Identify the actual
type definitions for the pack data being passed to these components (from
filteredPacks and the renderItem pack parameter respectively), and resolve the
underlying type mismatch by either aligning the types at their source, creating
a proper type adapter or mapper function, or using a more specific shared type
definition instead of the any cast. This will eliminate the need for the
biome-ignore comments and restore proper type safety throughout the component.
In `@docs/migrations/nativewindui-to-expo-ui.md`:
- Around line 54-55: The replacement map table for useColorScheme and cn entries
does not match the actual implementation introduced in this PR. Update the table
entries to reflect the actual migration targets: useColorScheme should map to
the local wrapper at expo-app/lib/hooks/useColorScheme (which internally wraps
NativeWind's hook), and cn should map to the local wrapper at expo-app/lib/cn
(which internally uses tailwind-merge), rather than showing direct imports from
react-native and tailwind-merge respectively. This ensures the documentation
accurately guides users to the correct migration path.
In `@scripts/lint/nativewindui-migration.ts`:
- Line 22: Add try/catch error handling around all file system operations in the
nativewindui-migration.ts script. At line 22, wrap the readFileSync call for
ADAPTER in a try/catch block that logs a warning and skips processing if the
file cannot be read. Apply the same error handling pattern to the file
operations in lines 67-79 and 82-84 (which appear to contain statSync or similar
operations). In each catch block, log a descriptive warning message and continue
execution rather than allowing the exception to crash the entire script.
- Around line 84-89: The string matching approach using content.includes()
matches false positives like commented-out imports or string literals. Replace
the simple string check in the if condition with a regex pattern that validates
import statement context, such as
/^\s*import\s+.*from\s+['"]`@packrat-ai`\/nativewindui['"]/m. Apply the same regex
pattern to the findIndex call instead of the includes() check so that the line
detection also only matches actual import statements.
---
Outside diff comments:
In `@apps/expo/app/`(app)/messages/conversations.android.tsx:
- Around line 6-15: The import statements in conversations.android.tsx (lines
6-15) are attempting to import ContextMenu, createContextItem,
createDropdownItem, DropdownMenu, Toolbar, and ToolbarCTA from the
`@packrat/ui/nativewindui` adapter, but these components are not exported there.
According to the migration guide, redirect the imports as follows: ContextMenu,
createContextItem, createDropdownItem, and DropdownMenu should be imported from
packages/ui/src/context-menu.android.tsx, while Toolbar and ToolbarCTA should be
imported from the navigation-layer Stack.Toolbar. Similarly, in
conversations.tsx (lines 6-15), redirect Checkbox, ContextMenu,
createContextItem, createDropdownItem, and DropdownMenu to
packages/ui/src/context-menu.ios.tsx (for the context menu components, using the
.ios variant), and Toolbar to the navigation-layer Stack.Toolbar. Remove or
comment out any imports of these components from the `@packrat/ui/nativewindui`
adapter in both files and add the correct import statements from the identified
sources.
In `@apps/expo/features/ai-packs/screens/AIPacksScreen.tsx`:
- Around line 93-99: The manual top padding values on both the SafeAreaView
component (using insets.top) and the nested View component (using insets.top +
22) are now redundant and causing double-offset on iOS after enabling the native
header with Stack.Screen. Remove the style prop with the conditional Platform.OS
=== 'ios' padding from the SafeAreaView, and remove the style prop with the
conditional paddingTop from the View component, since the native header now
handles the proper spacing automatically.
In `@apps/expo/features/ai/components/WeatherGenerativeUI.tsx`:
- Around line 84-90: The issue is wrapping error strings in new Error() before
passing them to Sentry.captureException(), which creates misleading stack traces
pointing to the wrapping location rather than the original error context.
Replace the new Error(toolInvocation.output.error) pattern with
Sentry.captureMessage() for string errors, or alternatively pass the error
string directly via the contexts or extra field without creating a new Error
object. This pattern occurs at the location shown (line 84) and also at line
102-107, both of which need to be updated with the same approach.
In `@apps/expo/features/catalog/screens/CatalogItemsScreen.tsx`:
- Around line 79-91: Wrap the three handler functions `handleGroupPress`,
`handleItemPress`, and `handleRefresh` with the `useCallback` hook from React.
For `handleGroupPress`, include `setGroupVariants` and `router` in the
dependency array; for `handleItemPress`, include `router` in the dependency
array; and for `handleRefresh`, include `setIsManualRefresh` and `refetch` in
the dependency array. This will ensure that the handler references remain stable
across re-renders, preserving memoization of the list item components that
receive these handlers as props.
- Around line 133-259: The issue spans five files where both search UI and main
list components render simultaneously when searching, creating confusing UX and
wasting performance. In
apps/expo/features/catalog/screens/CatalogItemsScreen.tsx (lines 133-259), wrap
the FlatList component (starting around line 187) in a conditional that only
renders when isSearching is false, so either the search ScrollView OR the main
FlatList displays, never both. In
apps/expo/features/guides/screens/GuidesListScreen.tsx (lines 207-232), the
renderSearchContent() function returns search results but the main FlatList also
renders the guides data; conditionally render only one list based on the search
state. In apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx
(lines 200-253), conditionally render either the renderSearchContent()
ScrollView with search results OR the main FlatList with filteredTemplates, not
both. In apps/expo/features/packs/screens/PackListScreen.tsx (lines 196-287),
the SearchResults component and main FlatList both render filteredPacks when
searching; conditionally display only one based on search state. In
apps/expo/features/trips/screens/TripListScreen.tsx (lines 167-183), the
renderSearchContent() ScrollView and main FlatList with filteredTrips both
render during search; conditionally render one or the other based on search
state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01be5b4a-dda8-4acc-9e08-643a03890fa1
📒 Files selected for processing (60)
apps/expo/app/(app)/(tabs)/(home)/index.tsxapps/expo/app/(app)/(tabs)/profile/index.tsxapps/expo/app/(app)/_layout.tsxapps/expo/app/(app)/current-pack/[id].tsxapps/expo/app/(app)/demo/index.tsxapps/expo/app/(app)/gear-inventory.tsxapps/expo/app/(app)/messages/conversations.android.tsxapps/expo/app/(app)/messages/conversations.tsxapps/expo/app/(app)/pack-categories/[id].tsxapps/expo/app/(app)/pack-stats/[id].tsxapps/expo/app/(app)/recent-packs.tsxapps/expo/app/(app)/season-suggestions-results.tsxapps/expo/app/(app)/season-suggestions.tsxapps/expo/app/(app)/shared-packs.tsxapps/expo/app/(app)/shopping-list.tsxapps/expo/app/(app)/trail-conditions.tsxapps/expo/app/(app)/weather-alert-preferences.tsxapps/expo/app/(app)/weather-alerts.tsxapps/expo/app/(app)/weight-analysis/[id].tsxapps/expo/app/_layout.web.tsxapps/expo/components/ErrorState.tsxapps/expo/components/LargeTitleHeaderOverlapFixIOS.tsxapps/expo/components/LargeTitleHeaderSearchContentContainer.tsxapps/expo/components/Markdown.tsxapps/expo/components/SearchInput.tsxapps/expo/components/initial/ExpandableText.tsxapps/expo/features/ai-packs/screens/AIPacksScreen.tsxapps/expo/features/ai/components/ChatBubble.tsxapps/expo/features/ai/components/GuidesRAGGenerativeUI.tsxapps/expo/features/ai/components/LocationContext.tsxapps/expo/features/ai/components/WeatherGenerativeUI.tsxapps/expo/features/ai/screens/ReportedContentScreen.tsxapps/expo/features/catalog/components/CatalogItemImage.tsxapps/expo/features/catalog/screens/CatalogItemsScreen.tsxapps/expo/features/feed/screens/FeedScreen.tsxapps/expo/features/guides/screens/GuidesListScreen.tsxapps/expo/features/pack-templates/components/AddPackTemplateItemActions.tsxapps/expo/features/pack-templates/components/PackTemplateItemImage.tsxapps/expo/features/pack-templates/screens/PackTemplateListScreen.tsxapps/expo/features/pack-templates/utils/getPackTemplateDetailOptions.tsxapps/expo/features/pack-templates/utils/getPackTemplateItemDetailOptions.tsxapps/expo/features/packs/components/AddPackItemActions.tsxapps/expo/features/packs/components/GapAnalysisModal.tsxapps/expo/features/packs/components/PackItemImage.tsxapps/expo/features/packs/screens/PackItemDetailScreen.tsxapps/expo/features/packs/screens/PackListScreen.tsxapps/expo/features/packs/utils/getPackDetailOptions.tsxapps/expo/features/packs/utils/getPackItemDetailOptions.tsxapps/expo/features/trips/components/UpcomingTripsTile.tsxapps/expo/features/trips/screens/TripListScreen.tsxapps/expo/features/trips/utils/getTripDetailOptions.tsxapps/expo/features/weather/screens/LocationSearchScreen.tsxapps/expo/features/weather/screens/LocationsScreen.tsxapps/expo/features/wildlife/screens/WildlifeScreen.tsxbiome.jsondocs/ideation/2026-06-13-nativewindui-to-expo-ui-ideation.mddocs/migrations/nativewindui-to-expo-ui.mdpackage.jsonpackages/ui/nativewindui/index.tsscripts/lint/nativewindui-migration.ts
💤 Files with no reviewable changes (4)
- apps/expo/components/LargeTitleHeaderSearchContentContainer.tsx
- apps/expo/components/LargeTitleHeaderOverlapFixIOS.tsx
- apps/expo/features/weather/screens/LocationSearchScreen.tsx
- apps/expo/app/(app)/_layout.tsx
Comment on lines
+196
to
+287
| {searchValue ? ( | ||
| <SearchResults | ||
| // biome-ignore lint/suspicious/noExplicitAny: Treaty type divergence | ||
| results={(filteredPacks || []) as any} | ||
| searchValue={searchValue} | ||
| onResultPress={handleSearchResultPress} | ||
| /> | ||
| ) : null} | ||
|
|
||
| <FlatList | ||
| data={filteredPacks} | ||
| keyExtractor={(pack) => pack.id} | ||
| stickyHeaderIndices={[0]} | ||
| contentInsetAdjustmentBehavior="automatic" | ||
| renderItem={({ item: pack }) => ( | ||
| <View className="px-4 pt-4"> | ||
| <PackCard | ||
| // biome-ignore lint/suspicious/noExplicitAny: Treaty type divergence | ||
| pack={pack as any} | ||
| onPress={handlePackPress} | ||
| showDuplicateButton={selectedTypeIndex === ALL_PACKS_INDEX} | ||
| /> | ||
| </View> | ||
| )} | ||
| refreshControl={ | ||
| selectedTypeIndex === ALL_PACKS_INDEX ? ( | ||
| <RefreshControl | ||
| refreshing={allPacksQuery.isRefetching} | ||
| onRefresh={allPacksQuery.refetch} | ||
| tintColor={colors.primary} | ||
| /> | ||
| ) : undefined | ||
| } | ||
| ListHeaderComponent={ | ||
| <View className="bg-background"> | ||
| {!isAuthenticated && <SyncBanner title={t('packs.syncBanner')} />} | ||
| {isAuthenticated && ( | ||
| <View className="px-4"> | ||
| <SegmentedControl | ||
| enabled={isAuthenticated} | ||
| values={['My Packs', 'All Packs']} | ||
| selectedIndex={selectedTypeIndex} | ||
| onIndexChange={(index) => { | ||
| setSelectedTypeIndex(index); | ||
| }} | ||
| /> | ||
| </View> | ||
| )} | ||
| <View className="bg-background px-4 py-2"> | ||
| <ScrollView horizontal showsHorizontalScrollIndicator={false} className="py-1"> | ||
| {filterOptions.map(renderFilterChip)} | ||
| </ScrollView> | ||
| <FlatList | ||
| data={filteredPacks} | ||
| keyExtractor={(pack) => pack.id} | ||
| stickyHeaderIndices={[0]} | ||
| contentInsetAdjustmentBehavior="automatic" | ||
| renderItem={({ item: pack }) => ( | ||
| <View className="px-4 pt-4"> | ||
| <PackCard | ||
| // biome-ignore lint/suspicious/noExplicitAny: Treaty type divergence | ||
| pack={pack as any} | ||
| onPress={handlePackPress} | ||
| showDuplicateButton={selectedTypeIndex === ALL_PACKS_INDEX} | ||
| /> | ||
| </View> | ||
| )} | ||
| refreshControl={ | ||
| selectedTypeIndex === ALL_PACKS_INDEX ? ( | ||
| <RefreshControl | ||
| refreshing={allPacksQuery.isRefetching} | ||
| onRefresh={allPacksQuery.refetch} | ||
| tintColor={colors.primary} | ||
| /> | ||
| ) : undefined | ||
| } | ||
| ListHeaderComponent={ | ||
| <View className="bg-background"> | ||
| {!isAuthenticated && <SyncBanner title={t('packs.syncBanner')} />} | ||
| {isAuthenticated && ( | ||
| <View className="px-4"> | ||
| <SegmentedControl | ||
| enabled={isAuthenticated} | ||
| values={['My Packs', 'All Packs']} | ||
| selectedIndex={selectedTypeIndex} | ||
| onIndexChange={(index) => { | ||
| setSelectedTypeIndex(index); | ||
| }} | ||
| /> | ||
| </View> | ||
| {selectedTypeIndex === USER_PACKS_INDEX && ( | ||
| <View className="px-6 py-2 bg-background"> | ||
| <Text className="text-muted-foreground"> | ||
| {filteredPacks?.length || 0} {filteredPacks?.length === 1 ? 'pack' : 'packs'} | ||
| </Text> | ||
| </View> | ||
| )} | ||
| )} | ||
| <View className="bg-background px-4 py-2"> | ||
| <ScrollView horizontal showsHorizontalScrollIndicator={false} className="py-1"> | ||
| {filterOptions.map(renderFilterChip)} | ||
| </ScrollView> | ||
| </View> | ||
| } | ||
| ListEmptyComponent={ | ||
| selectedTypeIndex === ALL_PACKS_INDEX ? ( | ||
| renderAllPacksEmptyState() | ||
| ) : ( | ||
| <View className="flex-1 items-center justify-center p-8"> | ||
| <View className="mb-4 rounded-full bg-muted p-4"> | ||
| <Icon name="cog-outline" size={32} color="text-muted-foreground" /> | ||
| </View> | ||
| <Text className="mb-1 text-lg font-medium text-foreground"> | ||
| {t('packs.noPacksFound')} | ||
| </Text> | ||
| <Text className="mb-6 text-center text-muted-foreground"> | ||
| {activeFilter === 'all' | ||
| ? "You haven't created or found any public packs yet." | ||
| : `You don't have any ${activeFilter} packs.`} | ||
| {selectedTypeIndex === USER_PACKS_INDEX && ( | ||
| <View className="px-6 py-2 bg-background"> | ||
| <Text className="text-muted-foreground"> | ||
| {filteredPacks?.length || 0} {filteredPacks?.length === 1 ? 'pack' : 'packs'} | ||
| </Text> | ||
| <TouchableOpacity | ||
| className="rounded-lg bg-primary px-4 py-2" | ||
| onPress={handleCreatePack} | ||
| > | ||
| <Text className="font-medium text-primary-foreground"> | ||
| {t('packs.createNewPack')} | ||
| </Text> | ||
| </TouchableOpacity> | ||
| </View> | ||
| ) | ||
| } | ||
| ListFooterComponent={<AndroidTabBarInsetFix />} | ||
| contentContainerStyle={{ flexGrow: 1 }} | ||
| /> | ||
| </LargeTitleHeaderOverlapFixIOS> | ||
| )} | ||
| </View> | ||
| } | ||
| ListEmptyComponent={ | ||
| selectedTypeIndex === ALL_PACKS_INDEX ? ( | ||
| renderAllPacksEmptyState() | ||
| ) : ( | ||
| <View className="flex-1 items-center justify-center p-8"> | ||
| <View className="mb-4 rounded-full bg-muted p-4"> | ||
| <Icon name="cog-outline" size={32} color="text-muted-foreground" /> | ||
| </View> | ||
| <Text className="mb-1 text-lg font-medium text-foreground"> | ||
| {t('packs.noPacksFound')} | ||
| </Text> | ||
| <Text className="mb-6 text-center text-muted-foreground"> | ||
| {activeFilter === 'all' | ||
| ? "You haven't created or found any public packs yet." | ||
| : `You don't have any ${activeFilter} packs.`} | ||
| </Text> | ||
| <TouchableOpacity | ||
| className="rounded-lg bg-primary px-4 py-2" | ||
| onPress={handleCreatePack} | ||
| > | ||
| <Text className="font-medium text-primary-foreground"> | ||
| {t('packs.createNewPack')} | ||
| </Text> | ||
| </TouchableOpacity> | ||
| </View> | ||
| ) | ||
| } | ||
| ListFooterComponent={<AndroidTabBarInsetFix />} | ||
| contentContainerStyle={{ flexGrow: 1 }} | ||
| /> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Type casts mask underlying type incompatibility.
Lines 199 and 214 cast to any due to "Treaty type divergence" between pack types. While this works as a temporary workaround, the underlying type mismatch should be resolved to improve type safety.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/packs/screens/PackListScreen.tsx` around lines 196 - 287,
Remove the `as any` type casts from the SearchResults component's results prop
and the PackCard component's pack prop, both of which are masking a type
divergence between pack type definitions. Identify the actual type definitions
for the pack data being passed to these components (from filteredPacks and the
renderItem pack parameter respectively), and resolve the underlying type
mismatch by either aligning the types at their source, creating a proper type
adapter or mapper function, or using a more specific shared type definition
instead of the any cast. This will eliminate the need for the biome-ignore
comments and restore proper type safety throughout the component.
Comment on lines
+54
to
+55
| | `useColorScheme` | 20 | `useColorScheme` from `react-native` (no @expo/ui hook) | RN | — (hook, not a component) | | ||
| | `cn` | 3 | remove — import `tailwind-merge` directly | — | — (utility, not a component) | |
Contributor
There was a problem hiding this comment.
Documentation inconsistency: replacement map doesn't match actual implementation.
The replacement map lists:
useColorScheme→ "useColorSchemefromreact-native"cn→ "remove — importtailwind-mergedirectly"
But the actual implementation (visible in this PR's code changes) creates local wrappers:
useColorScheme→expo-app/lib/hooks/useColorScheme(which internally wraps NativeWind's hook)cn→expo-app/lib/cn(which internally usestailwind-merge)
Update the table to reflect the actual migration targets, or add a note that these are intermediate local wrappers before the final dependency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/migrations/nativewindui-to-expo-ui.md` around lines 54 - 55, The
replacement map table for useColorScheme and cn entries does not match the
actual implementation introduced in this PR. Update the table entries to reflect
the actual migration targets: useColorScheme should map to the local wrapper at
expo-app/lib/hooks/useColorScheme (which internally wraps NativeWind's hook),
and cn should map to the local wrapper at expo-app/lib/cn (which internally uses
tailwind-merge), rather than showing direct imports from react-native and
tailwind-merge respectively. This ensures the documentation accurately guides
users to the correct migration path.
|
|
||
| // ── 1. Count remaining exports per phase ──────────────────────────────────── | ||
|
|
||
| const adapterLines = readFileSync(ADAPTER, 'utf8').split('\n'); |
Contributor
There was a problem hiding this comment.
Add error handling for file operations.
readFileSync and statSync can throw on permission errors, missing files, or symlink cycles. Wrap in try/catch and skip problematic files with a warning rather than crashing the entire script.
🛡️ Proposed fix to add error handling
function walk(dir: string): string[] {
const results: string[] = [];
for (const entry of readdirSync(dir)) {
if (EXCLUDED.has(entry)) continue;
const full = join(dir, entry);
- if (statSync(full).isDirectory()) {
- results.push(...walk(full));
- } else if (/\.(ts|tsx)$/.test(entry)) {
- results.push(full);
+ try {
+ if (statSync(full).isDirectory()) {
+ results.push(...walk(full));
+ } else if (/\.(ts|tsx)$/.test(entry)) {
+ results.push(full);
+ }
+ } catch (err) {
+ console.warn(`Skipping ${full}: ${err.message}`);
}
}
return results;
}
const violations: string[] = [];
for (const file of walk(EXPO_SRC)) {
- const content = readFileSync(file, 'utf8');
+ let content: string;
+ try {
+ content = readFileSync(file, 'utf8');
+ } catch (err) {
+ console.warn(`Cannot read ${file}: ${err.message}`);
+ continue;
+ }Also applies to: 67-79, 82-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/lint/nativewindui-migration.ts` at line 22, Add try/catch error
handling around all file system operations in the nativewindui-migration.ts
script. At line 22, wrap the readFileSync call for ADAPTER in a try/catch block
that logs a warning and skips processing if the file cannot be read. Apply the
same error handling pattern to the file operations in lines 67-79 and 82-84
(which appear to contain statSync or similar operations). In each catch block,
log a descriptive warning message and continue execution rather than allowing
the exception to crash the entire script.
Comment on lines
+84
to
+89
| if (content.includes("from '@packrat-ai/nativewindui'")) { | ||
| const rel = file.replace(ROOT + '/', ''); | ||
| const line = | ||
| content.split('\n').findIndex((l) => l.includes("from '@packrat-ai/nativewindui'")) + 1; | ||
| violations.push(` ${rel}:${line}`); | ||
| } |
Contributor
There was a problem hiding this comment.
String search could match false positives.
The simple content.includes("from '@packrat-ai/nativewindui'") check will match commented-out imports, string literals, and other non-import occurrences. Consider using a regex pattern that requires import statement context, e.g., /^\s*import\s+.*from\s+['"]@PackRat-AI\/nativewindui['"]/m.
🔍 Proposed fix to use regex for import detection
- if (content.includes("from '`@packrat-ai/nativewindui`'")) {
+ const importPattern = /^\s*import\s+.*from\s+['"]`@packrat-ai`\/nativewindui['"]/m;
+ if (importPattern.test(content)) {
const rel = file.replace(ROOT + '/', '');
const line =
- content.split('\n').findIndex((l) => l.includes("from '`@packrat-ai/nativewindui`'")) + 1;
+ content.split('\n').findIndex((l) => importPattern.test(l)) + 1;
violations.push(` ${rel}:${line}`);
}📝 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.
Suggested change
| if (content.includes("from '@packrat-ai/nativewindui'")) { | |
| const rel = file.replace(ROOT + '/', ''); | |
| const line = | |
| content.split('\n').findIndex((l) => l.includes("from '@packrat-ai/nativewindui'")) + 1; | |
| violations.push(` ${rel}:${line}`); | |
| } | |
| const importPattern = /^\s*import\s+.*from\s+['"]`@packrat-ai`\/nativewindui['"]/m; | |
| if (importPattern.test(content)) { | |
| const rel = file.replace(ROOT + '/', ''); | |
| const line = | |
| content.split('\n').findIndex((l) => importPattern.test(l)) + 1; | |
| violations.push(` ${rel}:${line}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/lint/nativewindui-migration.ts` around lines 84 - 89, The string
matching approach using content.includes() matches false positives like
commented-out imports or string literals. Replace the simple string check in the
if condition with a regex pattern that validates import statement context, such
as /^\s*import\s+.*from\s+['"]`@packrat-ai`\/nativewindui['"]/m. Apply the same
regex pattern to the findIndex call instead of the includes() check so that the
line detection also only matches actual import statements.
mikib0
force-pushed
the
feat/expo-ui-migration-phase-1-2
branch
from
ff03fb4 to
53e8790
Compare
Contributor
Deploying packrat-guides with
|
| Latest commit: |
53e8790
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://90e91e8b.packrat-guides-6gq.pages.dev |
| Branch Preview URL: | https://feat-expo-ui-migration-phase.packrat-guides-6gq.pages.dev |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
packrat-admin | 53e8790 | Commit Preview URL Branch Preview URL |
Jun 15 2026, 08:00 AM |
Contributor
Deploying packrat-landing with
|
| Latest commit: |
53e8790
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fc0cd6f3.packrat-landing.pages.dev |
| Branch Preview URL: | https://feat-expo-ui-migration-phase.packrat-landing.pages.dev |
…all list screens Platform-specific component ported from LargeTitleHeader reference: - iOS (SearchOverlay.ios.tsx): headerSearchBarOptions + FadeIn absoluteFill content overlay when focused or has text - Android (SearchOverlay.tsx): magnify icon in headerRight opens a full-screen Portal overlay with pill scale animation (custom Reanimated worklet), FadeInRight TextInput, FadeInUp content area, back-arrow dismiss, hardware-back-button support Applied to all 6 screens that had LargeTitleHeader + searchBar: home, catalog, packs, guides, pack-templates, trips Screens with a create button (packs, pack-templates, trips) pass androidHeaderRightActions so the create button and search icon are composed together in headerRight on Android; on iOS the parent Stack.Screen headerRight is unchanged and merges with SearchOverlay's Stack.Screen (headerSearchBarOptions only). Full parity with pre-migration behavior: - Same placeholder strings per screen - Same search results / empty-state / loading content - "Type to search" prompt when overlay open but query is empty
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/expo/components/SearchOverlay/SearchOverlay.ios.tsx`:
- Around line 25-31: Remove the redundant inner View wrapper that has
StyleSheet.absoluteFill applied. The outer Animated.View component already has
StyleSheet.absoluteFill, so the inner View is unnecessary. Render children
directly inside the Animated.View instead of nesting them within the additional
View component. This simplification maintains the same visual result while
reducing DOM depth.
In `@apps/expo/components/SearchOverlay/SearchOverlay.tsx`:
- Around line 69-73: The Animated.View component in SearchOverlay.tsx has
inconsistent opacity between light and dark modes—it uses bg-muted/25 (25%
opacity) in light mode but dark:bg-card (solid, no opacity) in dark mode. To fix
this, apply consistent opacity in the dark mode variant by changing dark:bg-card
to dark:bg-card/25 so both themes maintain the same visual transparency level
and provide a consistent user experience across themes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 80c9ecec-36a6-406e-997e-09499415eec5
📒 Files selected for processing (11)
apps/expo/app/(app)/(tabs)/(home)/index.tsxapps/expo/components/SearchOverlay/SearchOverlay.ios.tsxapps/expo/components/SearchOverlay/SearchOverlay.tsxapps/expo/components/SearchOverlay/index.tsapps/expo/components/SearchOverlay/types.tsapps/expo/features/catalog/screens/CatalogItemsScreen.tsxapps/expo/features/guides/screens/GuidesListScreen.tsxapps/expo/features/pack-templates/screens/PackTemplateListScreen.tsxapps/expo/features/packs/screens/PackListScreen.tsxapps/expo/features/trips/screens/TripListScreen.tsxapps/expo/lib/i18n/locales/en.json
…mponents, fix transparent overlay
- Move SearchOverlay from apps/expo/components/ to packages/ui/src/search-overlay/
- Fix transparent iOS overlay by adding bg-background to the absoluteFill Animated.View
- Restore LargeTitleHeaderSearchContentContainer in packages/ui/src/ (provides opaque bg + SafeAreaView)
- Restore LargeTitleHeaderOverlapFixIOS in packages/ui/src/ (provides iOS header overlap fix)
- Fix dashboard empty-state text: revert t('dashboard.searchPrompt') back to t('dashboard.searchPlaceholder') = "Search dashboard"
- Remove incorrectly added dashboard.searchPrompt translation key from en.json
- Update all 6 screen imports to use @packrat/ui/src/search-overlay
- Update packages/ui/tsconfig.json to include src/**/*
- Correct migration doc: components should be moved, not deleted
…LargeTitleHeaderOverlapFixIOS usage - Absorb LargeTitleHeaderSearchContentContainer behavior into SearchOverlay.ios.tsx: wrap children in SafeAreaView className="flex-1 bg-background" so the iOS overlay is opaque and content respects safe area insets — matches original container behavior per platform - Delete LargeTitleHeaderSearchContentContainer (no longer needed as standalone component) - Restore LargeTitleHeaderOverlapFixIOS usage in all 4 pre-migration screens: - CatalogItemsScreen: add <LargeTitleHeaderOverlapFixIOS /> as first item in listHeader - GuidesListScreen: same pattern - PackListScreen: wrap FlatList with <LargeTitleHeaderOverlapFixIOS> - LocationsScreen: wrap all visual content with <LargeTitleHeaderOverlapFixIOS> - Update migration doc to reflect SearchContentContainer is absorbed, not moved
- Show prompt text before search starts in CatalogItemsScreen - Fix empty state logic order (loading > no results > prompt) - Add flex-1 to SearchResults empty view for proper centering - Fix SafeAreaView styling in SearchOverlay.ios to use style prop
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/migrations/nativewindui-to-expo-ui.md (1)
54-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDocumentation inconsistency: replacement map does not reflect Phase 1 implementation.
The replacement map still lists direct imports from
react-nativeandtailwind-merge, but the Phase 1 implementation (confirmed in adapter index and PR objectives) routes both through local wrappers:expo-app/lib/hooks/useColorSchemeandexpo-app/lib/cn. Update the table to guide developers to the actual Phase 1 migration targets, not the final direct dependencies.📝 Proposed fix
-| `useColorScheme` | 20 | `useColorScheme` from `react-native` (no `@expo/ui` hook) | RN | — (hook, not a component) | -| `cn` | 3 | remove — import `tailwind-merge` directly | — | — (utility, not a component) | +| `useColorScheme` | 20 | `useColorScheme` from `expo-app/lib/hooks/useColorScheme` (Phase 1 wrapper; no `@expo/ui` hook) | RN | — (hook, not a component) | +| `cn` | 3 | `cn` from `expo-app/lib/cn` (Phase 1 wrapper; uses `tailwind-merge`) | — | — (utility, not a component) |Also add a note above the table clarifying that Phase 1 introduces local wrappers for governance; Phase 3+ may move to direct imports if those wrappers no longer serve a purpose.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/migrations/nativewindui-to-expo-ui.md` around lines 54 - 55, The migration table in the nativewindui-to-expo-ui.md file incorrectly documents the Phase 1 migration targets for useColorScheme and cn. Update the replacement entries: change the useColorScheme row (currently pointing to react-native) to point to expo-app/lib/hooks/useColorScheme, and change the cn row (currently pointing to tailwind-merge) to point to expo-app/lib/cn. Additionally, add a clarifying note above the table explaining that Phase 1 introduces local wrappers in those locations for governance purposes, and that Phase 3+ may transition to direct imports from the underlying libraries if the wrappers no longer serve a purpose.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/large-title-header-overlap-fix-ios.tsx`:
- Around line 4-15: The LargeTitleHeaderOverlapFixIOS component contains
platform-specific branching logic using Platform.OS which should be split into
separate platform files instead. Create two new files:
large-title-header-overlap-fix-ios.ios.tsx containing the iOS implementation
(the SafeAreaView logic that currently runs when Platform.OS is not android) and
large-title-header-overlap-fix-ios.android.tsx containing the Android
implementation (the current android branch logic). Update the existing
large-title-header-overlap-fix-ios.tsx file to be a barrel export that
re-exports from the platform-specific files so that existing call sites remain
unchanged. Remove the Platform.OS === 'android' check since the platform will be
automatically determined by React Native's module resolution based on the .ios
and .android file extensions.
---
Outside diff comments:
In `@docs/migrations/nativewindui-to-expo-ui.md`:
- Around line 54-55: The migration table in the nativewindui-to-expo-ui.md file
incorrectly documents the Phase 1 migration targets for useColorScheme and cn.
Update the replacement entries: change the useColorScheme row (currently
pointing to react-native) to point to expo-app/lib/hooks/useColorScheme, and
change the cn row (currently pointing to tailwind-merge) to point to
expo-app/lib/cn. Additionally, add a clarifying note above the table explaining
that Phase 1 introduces local wrappers in those locations for governance
purposes, and that Phase 3+ may transition to direct imports from the underlying
libraries if the wrappers no longer serve a purpose.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8bbef300-835c-44e0-a3b2-c60a3868a885
📒 Files selected for processing (15)
apps/expo/app/(app)/(tabs)/(home)/index.tsxapps/expo/features/catalog/screens/CatalogItemsScreen.tsxapps/expo/features/guides/screens/GuidesListScreen.tsxapps/expo/features/pack-templates/screens/PackTemplateListScreen.tsxapps/expo/features/packs/components/SearchResults.tsxapps/expo/features/packs/screens/PackListScreen.tsxapps/expo/features/trips/screens/TripListScreen.tsxapps/expo/features/weather/screens/LocationsScreen.tsxdocs/migrations/nativewindui-to-expo-ui.mdpackages/ui/src/large-title-header-overlap-fix-ios.tsxpackages/ui/src/search-overlay/SearchOverlay.ios.tsxpackages/ui/src/search-overlay/SearchOverlay.tsxpackages/ui/src/search-overlay/index.tspackages/ui/src/search-overlay/types.tspackages/ui/tsconfig.json
💤 Files with no reviewable changes (2)
- packages/ui/src/search-overlay/index.ts
- packages/ui/src/search-overlay/types.ts
Comment on lines
+4
to
+15
| export function LargeTitleHeaderOverlapFixIOS({ children }: { children?: ReactNode }) { | ||
| if (Platform.OS === 'android') { | ||
| if (!children) { | ||
| return null; | ||
| } else { | ||
| return children; | ||
| } | ||
| if (!children) return null; | ||
| return <>{children}</>; | ||
| } | ||
|
|
||
| return ( | ||
| <SafeAreaView {...(children ? { className: 'flex-1 bg-background' } : {})}> | ||
| {children} | ||
| {!children && <View className="h-[0.5]" />} | ||
| </SafeAreaView> | ||
| ); | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Split this component into platform files instead of branching on Platform.OS inside a shared .tsx file.
This logic is platform-specific and should be moved to large-title-header-overlap-fix-ios.ios.tsx and large-title-header-overlap-fix-ios.android.tsx with a shared export barrel to keep call sites unchanged.
As per coding guidelines, “Avoid platform-specific code without a corresponding platform file (.ios.ts / .android.ts).”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/large-title-header-overlap-fix-ios.tsx` around lines 4 - 15,
The LargeTitleHeaderOverlapFixIOS component contains platform-specific branching
logic using Platform.OS which should be split into separate platform files
instead. Create two new files: large-title-header-overlap-fix-ios.ios.tsx
containing the iOS implementation (the SafeAreaView logic that currently runs
when Platform.OS is not android) and
large-title-header-overlap-fix-ios.android.tsx containing the Android
implementation (the current android branch logic). Update the existing
large-title-header-overlap-fix-ios.tsx file to be a barrel export that
re-exports from the platform-specific files so that existing call sites remain
unchanged. Remove the Platform.OS === 'android' check since the platform will be
automatically determined by React Native's module resolution based on the .ios
and .android file extensions.
Source: Coding guidelines
…rScheme import Add Checkbox, ContextMenu, createContextItem, DropdownMenu, createDropdownItem, Toolbar, and ToolbarCTA re-exports to packages/ui/nativewindui/index.ts that were missing from the migration tracker. Fix ToolCard.tsx to import useColorScheme from its migrated location (expo-app/lib/hooks/useColorScheme) per Phase 1.
… stale headerShown:false - Remove headerShown:false override for weather/index so Stack.Screen with headerLargeTitle:true takes effect in LocationsScreen - Restore containerClassName="border border-border" on SearchInput in LocationsScreen and LocationSearchScreen - Wrap Stack.Screen inside LargeTitleHeaderOverlapFixIOS to preserve overlap fix
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/expo/app/(app)/(tabs)/(home)/index.tsx (1)
257-305:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse trimmed query consistently for search-mode gating.
filteredTilesalready treats whitespace-only input as empty, but this branch uses rawsearchValue. A whitespace query currently renders the “no results” state instead of the initial prompt.Proposed fix
- {searchValue ? ( + {searchValue.trim() ? (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/(tabs)/(home)/index.tsx around lines 257 - 305, The conditional check for search mode uses raw `searchValue`, which treats whitespace-only input as truthy and renders the "no results" state. However, `filteredTiles` treats whitespace-only input as empty. Replace the condition `{searchValue ? (` with `{searchValue.trim() ? (` to ensure the search mode gating is consistent with how the filtered results are computed, so that whitespace-only queries show the initial prompt instead of "no results".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/nativewindui/index.ts`:
- Line 57: The Checkbox export is currently in Phase 4 but its comment indicates
it should be in Phase 3 since it maps to `@expo/ui` Universal Checkbox. Move the
Checkbox export line from its current position in Phase 4 (around line 57) to
Phase 3 by placing it after line 40 (before the Phase 4 comment block), then
remove the Checkbox export from its current Phase 4 location to ensure the
migration tracking accurately reflects the proper phase placement.
- Around line 57-61: The Checkbox, DropdownMenu, and Toolbar components are
missing type exports, which breaks the established pattern where component
exports are accompanied by their type definitions (as seen with
ContextMenuMethods). Add type export statements for CheckboxProps,
DropdownMenuProps, and ToolbarProps (or the corresponding Method types if these
components have ref-based APIs) from `@packrat-ai/nativewindui` immediately after
their respective component export statements to maintain consistency with the
pattern established by ButtonProps, TextFieldProps, AlertMethods, and
ContextMenuMethods.
---
Outside diff comments:
In `@apps/expo/app/`(app)/(tabs)/(home)/index.tsx:
- Around line 257-305: The conditional check for search mode uses raw
`searchValue`, which treats whitespace-only input as truthy and renders the "no
results" state. However, `filteredTiles` treats whitespace-only input as empty.
Replace the condition `{searchValue ? (` with `{searchValue.trim() ? (` to
ensure the search mode gating is consistent with how the filtered results are
computed, so that whitespace-only queries show the initial prompt instead of "no
results".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1998f87e-048a-421e-b716-3519cfc5ef08
📒 Files selected for processing (5)
apps/expo/app/(app)/(tabs)/(home)/index.tsxapps/expo/app/(app)/_layout.tsxapps/expo/features/ai/components/ToolCard.tsxapps/expo/features/weather/screens/LocationsScreen.tsxpackages/ui/nativewindui/index.ts
💤 Files with no reviewable changes (1)
- apps/expo/app/(app)/_layout.tsx
| CardSubtitle, | ||
| } from '@packrat-ai/nativewindui'; // 8 uses → JC Card (Android) + custom View (iOS) | ||
| export { SegmentedControl } from '@packrat-ai/nativewindui'; // 3 uses → @expo/ui community SegmentedControl | ||
| export { Checkbox } from '@packrat-ai/nativewindui'; // 3 uses → @expo/ui Universal Checkbox |
Contributor
There was a problem hiding this comment.
Checkbox is placed in Phase 4 but comment says "Universal".
The comment → @expo/ui Universal Checkbox indicates this belongs in Phase 3 ("@expo/ui Universal → packages/ui/src/"), not Phase 4 ("platform-specific wrappers"). The lint script uses physical placement to count exports per phase, so this will skew the migration tracking.
Move the Checkbox export to Phase 3 (after line 40, before the Phase 4 comment):
Suggested diff
export { Toggle } from '`@packrat-ai/nativewindui`'; // 1 use → `@expo/ui` Universal Switch
+export { Checkbox } from '`@packrat-ai/nativewindui`'; // 3 uses → `@expo/ui` Universal Checkbox
//
// Phase 4 — `@expo/ui` platform-specific wrappers (.ios.tsx + .android.tsx) in packages/ui/src/And remove from its current position in Phase 4.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/nativewindui/index.ts` at line 57, The Checkbox export is
currently in Phase 4 but its comment indicates it should be in Phase 3 since it
maps to `@expo/ui` Universal Checkbox. Move the Checkbox export line from its
current position in Phase 4 (around line 57) to Phase 3 by placing it after line
40 (before the Phase 4 comment block), then remove the Checkbox export from its
current Phase 4 location to ensure the migration tracking accurately reflects
the proper phase placement.
Comment on lines
+57
to
+61
| export { Checkbox } from '@packrat-ai/nativewindui'; // 3 uses → @expo/ui Universal Checkbox | ||
| export { ContextMenu, createContextItem, createContextSubMenu } from '@packrat-ai/nativewindui'; // multiple uses → SwiftUI ContextMenu + JC DropdownMenu | ||
| export type { ContextMenuMethods } from '@packrat-ai/nativewindui'; | ||
| export { DropdownMenu, createDropdownItem, createDropdownSubMenu } from '@packrat-ai/nativewindui'; // multiple uses → @expo/ui DropdownMenu | ||
| export { Toolbar, ToolbarCTA, ToolbarIcon } from '@packrat-ai/nativewindui'; // multiple uses → platform-specific Toolbar |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if `@packrat-ai/nativewindui` exports types for these components
# by scanning for their usage patterns in the codebase
echo "=== Checking for existing typed usages of Checkbox/DropdownMenu/Toolbar ==="
rg -n --type=ts --type=tsx -C2 'CheckboxProps|DropdownMenuProps|ToolbarProps' apps/expo/ packages/
echo ""
echo "=== Checking nativewindui type imports in codebase ==="
rg -n "type.*from.*nativewindui" apps/expo/ packages/Repository: PackRat-AI/PackRat
Length of output: 3284
🏁 Script executed:
#!/bin/bash
# Search for Checkbox/DropdownMenu/Toolbar component implementations in nativewindui
echo "=== Looking for nativewindui index files and type definitions ==="
find packages/ui -type f \( -name "*.ts" -o -name "*.tsx" \) -path "*nativewindui*" | head -20
echo ""
echo "=== Checking if these components have platform-specific files ==="
find packages/ui -type f \( -name "*[Cc]heckbox*" -o -name "*[Dd]ropdown*" -o -name "*[Tt]oolbar*" \)
echo ""
echo "=== Looking at the full index.ts file to understand export patterns ==="
cat -n packages/ui/nativewindui/index.ts | head -70Repository: PackRat-AI/PackRat
Length of output: 4276
🏁 Script executed:
#!/bin/bash
# Search for actual usage of Checkbox, DropdownMenu, and Toolbar in the codebase
echo "=== Searching for Checkbox usage ==="
rg -n "Checkbox" apps/expo/ packages/ui/src/ --type ts --type tsx -C 2
echo ""
echo "=== Searching for DropdownMenu usage ==="
rg -n "DropdownMenu" apps/expo/ packages/ui/src/ --type ts --type tsx -C 2
echo ""
echo "=== Searching for Toolbar usage ==="
rg -n "Toolbar" apps/expo/ packages/ui/src/ --type ts --type tsx -C 2
echo ""
echo "=== Searching for createContextItem/createDropdownItem usage patterns ==="
rg -n "createContextItem|createDropdownItem" apps/expo/ packages/ --type ts --type tsx -C 1Repository: PackRat-AI/PackRat
Length of output: 383
🏁 Script executed:
#!/bin/bash
# Search without tsx file type restriction
echo "=== Searching for Checkbox usage ==="
rg -n "Checkbox[^a-zA-Z]" apps/expo/ packages/ui/src/ --max-count 5
echo ""
echo "=== Searching for DropdownMenu usage (not DropdownMenuProps) ==="
rg -n "DropdownMenu[^a-zA-Z]" apps/expo/ packages/ui/src/ --max-count 5
echo ""
echo "=== Searching for Toolbar usage ==="
rg -n "Toolbar[^a-zA-Z]" apps/expo/ packages/ui/src/ --max-count 5
echo ""
echo "=== Searching for ContextMenu actual usage to understand patterns ==="
rg -n "ContextMenu[^a-zA-Z]" apps/expo/ --max-count 10 -B 1 -A 1
echo ""
echo "=== Check if any upstream types exist in node_modules or are mentioned in comments ==="
rg -n "CheckboxProps|CheckboxMethods|CheckboxRef" . --max-count 5
rg -n "DropdownMenuProps|DropdownMenuMethods|DropdownMenuRef" . --max-count 5
rg -n "ToolbarProps|ToolbarMethods|ToolbarRef" . --max-count 5Repository: PackRat-AI/PackRat
Length of output: 7671
🏁 Script executed:
#!/bin/bash
# Check if there's any evidence of CheckboxMethods, DropdownMenuMethods, ToolbarMethods in codebase
echo "=== Looking for method usage patterns on these components ==="
rg -n "checkboxRef|dropdownMenuRef|toolbarRef" apps/expo/ packages/
echo ""
echo "=== Check TypeScript prop typing patterns for these components ==="
grep -r "as const" apps/expo/app/auth/\(login\)/reset-password.tsx | head -5
grep -r "<Checkbox" apps/expo/app/auth/\(login\)/reset-password.tsx -A 1 -B 1 | head -10
echo ""
echo "=== Verify if ContextMenuMethods is actually imported/used ==="
rg -n "contextMenuRef|dismissMenu" apps/expo/ --max-count 10Repository: PackRat-AI/PackRat
Length of output: 1065
Add missing type exports for Checkbox, DropdownMenu, and Toolbar to maintain consistency with the established pattern.
The codebase exports types for all re-exported components (ButtonProps, TextFieldProps, AlertMethods, ContextMenuMethods, etc.). Checkbox, DropdownMenu, and Toolbar break this pattern—only ContextMenu exports its type (ContextMenuMethods). Downstream consumers who need to type these components' props will have to import directly from @packrat-ai/nativewindui rather than through the adapter. Export CheckboxProps, DropdownMenuProps, and ToolbarProps (or corresponding Method types if they have ref-based APIs) from packages/ui/nativewindui/index.ts alongside their component exports.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/nativewindui/index.ts` around lines 57 - 61, The Checkbox,
DropdownMenu, and Toolbar components are missing type exports, which breaks the
established pattern where component exports are accompanied by their type
definitions (as seen with ContextMenuMethods). Add type export statements for
CheckboxProps, DropdownMenuProps, and ToolbarProps (or the corresponding Method
types if these components have ref-based APIs) from `@packrat-ai/nativewindui`
immediately after their respective component export statements to maintain
consistency with the pattern established by ButtonProps, TextFieldProps,
AlertMethods, and ContextMenuMethods.
Adds a custom MD3-style Large Top App Bar for Android using a fixed two-row layout (action row + large title) rendered via the Stack.Screen `header:` prop. iOS continues to use native `headerLargeTitle: true`. - AppBarAndroid: static large title (32sp/400) below the action row, no collapse animation - Removed useAppBarScroll and AppBarLargeTitle (no longer needed) - Migrated all 27+ screens from headerLargeTitle to getAppBarOptions()
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/ui/src/app-bar/AppBarAndroid.tsx (1)
18-68: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse platform file naming for Android-only header implementation.
This is Android-specific behavior in a generic
.tsxmodule. Split toAppBarAndroid.android.tsx(and a corresponding iOS-safe file or export path) to keep platform boundaries explicit and avoid accidental cross-platform imports.As per coding guidelines, "
packages/ui/**: Avoid platform-specific code without a corresponding platform file (.ios.ts/.android.ts)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/app-bar/AppBarAndroid.tsx` around lines 18 - 68, The AppBarAndroid component contains Android-specific implementation but uses a generic `.tsx` extension instead of following the platform file naming convention. Rename the file from `AppBarAndroid.tsx` to `AppBarAndroid.android.tsx` to explicitly mark it as Android-only code. Then create a corresponding `AppBarAndroid.ios.tsx` file with iOS-appropriate implementation, or create an `AppBarAndroid.tsx` index file that exports the platform-specific version based on the platform. Update all import statements throughout the codebase that reference this component to ensure they import from the correct platform-specific or index file path.Source: Coding guidelines
packages/ui/src/search-overlay/SearchOverlay.tsx (1)
2-4:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftPackage imports from app code — dependency inversion.
packages/uiimportsIcon,SearchInput, anduseColorSchemefromexpo-app/*. This creates a backwards dependency (package → app) that prevents the UI package from being reusable and violates separation of concerns. Packages should be self-contained or accept these as props.Consider:
- Accept
Icon,SearchInput, andcolorsas props- Move these components into
packages/ui- Use render props for customization points
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui/src/search-overlay/SearchOverlay.tsx` around lines 2 - 4, The SearchOverlay component in packages/ui/src/search-overlay/SearchOverlay.tsx has backwards dependencies on expo-app by importing Icon, SearchInput, and useColorScheme directly. Remove these three imports from the expo-app/* paths and instead accept Icon, SearchInput, and color/theme configuration as props to the SearchOverlay component. This makes the UI package self-contained and reusable by allowing consumers to inject their own implementations of these components and styling preferences, rather than being tightly coupled to the expo-app package.apps/expo/app/(app)/(tabs)/(home)/index.tsx (1)
267-273:⚠️ Potential issue | 🟡 MinorOverlay can remain visible after tapping a tile during navigation.
Confirmed: tile components (CurrentPackTile, FeedTile, etc.) do handle their own navigation via
router.push(). However, the Pressable'sonPress={() => setSearchValue('')}only clears the search value without explicitly closing the overlay.SearchOverlay closes only when:
- Android:
onBlurfires AND value is empty- iOS: overlay auto-hides when
!isFocused && value.length === 0During navigation, the SearchInput may retain focus momentarily, so the blur event won't fire on Android and the overlay remains visible. On iOS, focus isn't immediately lost during the transition either. The overlay should be explicitly closed via a close callback or by having the overlay respond to the same event that triggers navigation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/(tabs)/(home)/index.tsx around lines 267 - 273, The Pressable component's onPress handler only clears the search value but doesn't explicitly close the overlay, which can remain visible during navigation because the SearchInput may retain focus momentarily on both Android and iOS. Modify the onPress handler in the Pressable to not only call setSearchValue('') but also explicitly close or hide the overlay by invoking a close callback or toggling an overlay visibility state. This ensures the overlay is dismissed immediately when a tile is tapped, regardless of the SearchInput's focus state or platform-specific blur behavior.apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx (1)
165-182: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winWrap
listHeaderinuseCallback.The
listHeaderfunction passed to FlatList'sListHeaderComponent(line 230) is not memoized, causing the header to recreate on every render. Per coding guidelines, FlatList callbacks should useuseCallback.⚡ Wrap in useCallback
- const listHeader = () => { - const isSearching = searchValue.trim().length > 0; - if (isSearching) return null; - - return selectedTemplateTypeIndex === 0 ? ( - <View className="bg-background"> - <FeaturedPacksSection onTemplatePress={handleTemplatePress} /> - <View className="px-4 pb-2"> - <Text className="text-muted-foreground"> - {filteredTemplates.length}{' '} - {filteredTemplates.length === 1 - ? t('packTemplates.template') - : t('packTemplates.templates')} - </Text> - </View> - </View> - ) : undefined; - }; + const listHeader = useCallback(() => { + const isSearching = searchValue.trim().length > 0; + if (isSearching) return null; + + return selectedTemplateTypeIndex === 0 ? ( + <View className="bg-background"> + <FeaturedPacksSection onTemplatePress={handleTemplatePress} /> + <View className="px-4 pb-2"> + <Text className="text-muted-foreground"> + {filteredTemplates.length}{' '} + {filteredTemplates.length === 1 + ? t('packTemplates.template') + : t('packTemplates.templates')} + </Text> + </View> + </View> + ) : undefined; + }, [searchValue, selectedTemplateTypeIndex, handleTemplatePress, filteredTemplates.length, t]);As per coding guidelines: "Check for missing
useCallback/useMemoon props passed to list-item components (FlatList / FlashList perf)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx` around lines 165 - 182, The listHeader function is recreated on every render and needs to be memoized for FlatList performance optimization. Wrap the listHeader function with useCallback hook from React, ensuring you include all external dependencies such as searchValue, selectedTemplateTypeIndex, handleTemplatePress, filteredTemplates, and the t function (from your i18n library) in the dependency array. This will prevent unnecessary re-renders of the FlatList header component.Source: Coding guidelines
apps/expo/features/guides/screens/GuidesListScreen.tsx (1)
91-93: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winWrap FlatList callbacks in
useCallbackfor performance.Four functions passed to FlatList props are not memoized:
renderGuide(line 91),renderFooter(line 95),renderEmpty(line 104), andlistHeader(line 180). Per coding guidelines, FlatList callback props should useuseCallbackto prevent unnecessary re-renders.⚡ Wrap callbacks in useCallback
- const renderGuide = ({ item }: { item: Guide }) => ( - <GuideCard guide={item} onPress={() => handleGuidePress(item)} /> - ); + const renderGuide = useCallback( + ({ item }: { item: Guide }) => ( + <GuideCard guide={item} onPress={() => handleGuidePress(item)} /> + ), + [handleGuidePress], + ); - const renderFooter = () => { - if (!isFetchingNextPage) return null; - return ( - <View className="py-4"> - <ActivityIndicator size="small" color={colors.primary} /> - </View> - ); - }; + const renderFooter = useCallback(() => { + if (!isFetchingNextPage) return null; + return ( + <View className="py-4"> + <ActivityIndicator size="small" color={colors.primary} /> + </View> + ); + }, [isFetchingNextPage, colors.primary]); - const renderEmpty = () => { - return ( - <View className="flex-1 items-center justify-center p-8"> - {isLoading ? ( - <ActivityIndicator color={colors.primary} size="large" /> - ) : ( - <Text className="text-center text-gray-500 dark:text-gray-400"> - {isSearchMode - ? t('guides.noGuidesFound', { query: searchQuery }) - : t('guides.noGuidesAvailable')} - </Text> - )} - </View> - ); - }; + const renderEmpty = useCallback(() => { + return ( + <View className="flex-1 items-center justify-center p-8"> + {isLoading ? ( + <ActivityIndicator color={colors.primary} size="large" /> + ) : ( + <Text className="text-center text-gray-500 dark:text-gray-400"> + {isSearchMode + ? t('guides.noGuidesFound', { query: searchQuery }) + : t('guides.noGuidesAvailable')} + </Text> + )} + </View> + ); + }, [isLoading, colors.primary, isSearchMode, t, searchQuery]); - const listHeader = () => { - if (isSearchMode) return null; - - return ( - <> - <LargeTitleHeaderOverlapFixIOS /> - <CategoriesFilter - data={categories} - onFilter={handleCategoryChange} - activeFilter={selectedCategory} - error={categoriesError} - retry={refetchCategories} - className="px-4 pb-2" - /> - </> - ); - }; + const listHeader = useCallback(() => { + if (isSearchMode) return null; + + return ( + <> + <LargeTitleHeaderOverlapFixIOS /> + <CategoriesFilter + data={categories} + onFilter={handleCategoryChange} + activeFilter={selectedCategory} + error={categoriesError} + retry={refetchCategories} + className="px-4 pb-2" + /> + </> + ); + }, [isSearchMode, categories, handleCategoryChange, selectedCategory, categoriesError, refetchCategories]);As per coding guidelines: "Check for missing
useCallback/useMemoon props passed to list-item components (FlatList / FlashList perf)."Also applies to: 95-102, 104-118, 180-196
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/guides/screens/GuidesListScreen.tsx` around lines 91 - 93, The FlatList callback props renderGuide, renderFooter, renderEmpty, and listHeader are not memoized with useCallback, causing unnecessary re-renders. Wrap each of these four functions with useCallback hook: renderGuide (lines 91-93) needs to be wrapped with useCallback with dependency array including item and handleGuidePress; renderFooter (lines 95-102) should be wrapped with useCallback with appropriate dependencies; renderEmpty (lines 104-118) should be wrapped with useCallback with appropriate dependencies; and listHeader (lines 180-196) should be wrapped with useCallback with appropriate dependencies. Ensure each function maintains referential equality across renders to prevent FlatList from re-rendering list items unnecessarily.Source: Coding guidelines
apps/expo/features/weather/screens/LocationsScreen.tsx (1)
54-56: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winWrap event handlers in
useCallback.Multiple handler functions passed as props are not memoized:
handleSearchChange(line 54, passed to SearchInput),handleLocationPress(line 85, used in .map()),handleSetActive(line 89),handleRemoveLocation(line 106), andhandleAddLocation(line 110). This causes child components to re-render unnecessarily.⚡ Wrap in useCallback
- const handleSearchChange = (text: string) => { - setSearchQuery(text); - }; + const handleSearchChange = useCallback((text: string) => { + setSearchQuery(text); + }, [setSearchQuery]); - const handleLocationPress = (locationId: number) => { - router.push(`/weather/${locationId}`); - }; + const handleLocationPress = useCallback((locationId: number) => { + router.push(`/weather/${locationId}`); + }, [router]); - const handleSetActive = (locationId: number) => { - setActiveLocation(locationId); - - // Show confirmation - const location = locations.find((loc) => loc.id === locationId); - if (location) { - Alert.alert( - t('weather.locationSet'), - t('weather.locationSetMessage', { name: location.name }), - [{ text: t('common.ok') }], - { - cancelable: true, - }, - ); - } - }; + const handleSetActive = useCallback((locationId: number) => { + setActiveLocation(locationId); + + const location = locations.find((loc) => loc.id === locationId); + if (location) { + Alert.alert( + t('weather.locationSet'), + t('weather.locationSetMessage', { name: location.name }), + [{ text: t('common.ok') }], + { + cancelable: true, + }, + ); + } + }, [setActiveLocation, locations, t]); - const handleRemoveLocation = (locationId: number) => { - removeLocation(locationId); - }; + const handleRemoveLocation = useCallback((locationId: number) => { + removeLocation(locationId); + }, [removeLocation]); - const handleAddLocation = () => { - router.push('/weather/search'); - }; + const handleAddLocation = useCallback(() => { + router.push('/weather/search'); + }, [router]);As per coding guidelines: "Check for missing
useCallback/useMemoon props passed to list-item components (FlatList / FlashList perf)."Also applies to: 85-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/features/weather/screens/LocationsScreen.tsx` around lines 54 - 56, Multiple event handlers (handleSearchChange, handleLocationPress, handleSetActive, handleRemoveLocation, and handleAddLocation) are not memoized, causing unnecessary re-renders of child components that receive them as props. Wrap each of these handler functions in useCallback to memoize them. Import useCallback from React if not already imported, then refactor each handler function to use useCallback with appropriate dependency arrays to maintain referential equality across re-renders.Source: Coding guidelines
apps/expo/app/(app)/messages/conversations.tsx (1)
90-95:⚠️ Potential issue | 🟠 MajorUse the
useHeaderSearchBarhook and connect search state to content.The current implementation only sets
hideWhenScrolling: truewithout any search handlers or state. TheSearchBarContentcomponent is hardcoded and receives no search value. Use the existinguseHeaderSearchBarhook (found inapps/expo/lib/hooks/useHeaderSearchBar.tsx) to manage search state with properonChangeTexthandlers, then pass the search value toSearchBarContentto conditionally filter or display content based on the search query.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/expo/app/`(app)/messages/conversations.tsx around lines 90 - 95, Import and use the useHeaderSearchBar hook from apps/expo/lib/hooks/useHeaderSearchBar.tsx to manage search state in the conversations component. Update the headerSearchBarOptions configuration to include the onChangeText handler from the hook (in addition to the existing hideWhenScrolling property) to capture search input changes. Then pass the search value from the hook's state to the SearchBarContent component as a prop so it can filter or conditionally display conversation content based on the active search query.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/expo/app/`(app)/current-pack/[id].tsx:
- Line 133: The Stack.Screen options at line 133 in the current-pack/[id] route
are spreading getAppBarOptions() but the parent layout still has headerShown:
false set for this route. Since getAppBarOptions() does not explicitly set
headerShown: true, the parent's suppression persists on iOS. Add headerShown:
true to the options object (either within the spread or after it) to explicitly
override the parent route's header suppression and ensure the migrated header is
visible on iOS.
In `@apps/expo/features/weather/screens/LocationsScreen.tsx`:
- Around line 128-130: The Pressable component contains an inline style object
with padding that violates NativeWind guidelines. Replace the style prop
`style={{ padding: 14 }}` with the appropriate NativeWind/Tailwind class (such
as `p-3` or `p-4` depending on your design system's spacing scale) to align with
the coding standard of using Tailwind classes for styling instead of inline
style objects.
In `@packages/ui/src/app-bar/AppBarAndroid.tsx`:
- Around line 1-2: The AppBarAndroid.tsx component in packages/ui has direct
imports from expo-app internals (Icon from expo-app/components and
useColorScheme from expo-app/lib/hooks), which violates the portability
requirement of the shared UI package. Remove these direct imports and instead
accept the Icon component and colorScheme values as props passed into
AppBarAndroid, or use a context provider pattern to supply these dependencies.
This way the UI component remains decoupled from expo-app specifics and can be
reused across different consumers.
---
Outside diff comments:
In `@apps/expo/app/`(app)/(tabs)/(home)/index.tsx:
- Around line 267-273: The Pressable component's onPress handler only clears the
search value but doesn't explicitly close the overlay, which can remain visible
during navigation because the SearchInput may retain focus momentarily on both
Android and iOS. Modify the onPress handler in the Pressable to not only call
setSearchValue('') but also explicitly close or hide the overlay by invoking a
close callback or toggling an overlay visibility state. This ensures the overlay
is dismissed immediately when a tile is tapped, regardless of the SearchInput's
focus state or platform-specific blur behavior.
In `@apps/expo/app/`(app)/messages/conversations.tsx:
- Around line 90-95: Import and use the useHeaderSearchBar hook from
apps/expo/lib/hooks/useHeaderSearchBar.tsx to manage search state in the
conversations component. Update the headerSearchBarOptions configuration to
include the onChangeText handler from the hook (in addition to the existing
hideWhenScrolling property) to capture search input changes. Then pass the
search value from the hook's state to the SearchBarContent component as a prop
so it can filter or conditionally display conversation content based on the
active search query.
In `@apps/expo/features/guides/screens/GuidesListScreen.tsx`:
- Around line 91-93: The FlatList callback props renderGuide, renderFooter,
renderEmpty, and listHeader are not memoized with useCallback, causing
unnecessary re-renders. Wrap each of these four functions with useCallback hook:
renderGuide (lines 91-93) needs to be wrapped with useCallback with dependency
array including item and handleGuidePress; renderFooter (lines 95-102) should be
wrapped with useCallback with appropriate dependencies; renderEmpty (lines
104-118) should be wrapped with useCallback with appropriate dependencies; and
listHeader (lines 180-196) should be wrapped with useCallback with appropriate
dependencies. Ensure each function maintains referential equality across renders
to prevent FlatList from re-rendering list items unnecessarily.
In `@apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx`:
- Around line 165-182: The listHeader function is recreated on every render and
needs to be memoized for FlatList performance optimization. Wrap the listHeader
function with useCallback hook from React, ensuring you include all external
dependencies such as searchValue, selectedTemplateTypeIndex,
handleTemplatePress, filteredTemplates, and the t function (from your i18n
library) in the dependency array. This will prevent unnecessary re-renders of
the FlatList header component.
In `@apps/expo/features/weather/screens/LocationsScreen.tsx`:
- Around line 54-56: Multiple event handlers (handleSearchChange,
handleLocationPress, handleSetActive, handleRemoveLocation, and
handleAddLocation) are not memoized, causing unnecessary re-renders of child
components that receive them as props. Wrap each of these handler functions in
useCallback to memoize them. Import useCallback from React if not already
imported, then refactor each handler function to use useCallback with
appropriate dependency arrays to maintain referential equality across
re-renders.
In `@packages/ui/src/app-bar/AppBarAndroid.tsx`:
- Around line 18-68: The AppBarAndroid component contains Android-specific
implementation but uses a generic `.tsx` extension instead of following the
platform file naming convention. Rename the file from `AppBarAndroid.tsx` to
`AppBarAndroid.android.tsx` to explicitly mark it as Android-only code. Then
create a corresponding `AppBarAndroid.ios.tsx` file with iOS-appropriate
implementation, or create an `AppBarAndroid.tsx` index file that exports the
platform-specific version based on the platform. Update all import statements
throughout the codebase that reference this component to ensure they import from
the correct platform-specific or index file path.
In `@packages/ui/src/search-overlay/SearchOverlay.tsx`:
- Around line 2-4: The SearchOverlay component in
packages/ui/src/search-overlay/SearchOverlay.tsx has backwards dependencies on
expo-app by importing Icon, SearchInput, and useColorScheme directly. Remove
these three imports from the expo-app/* paths and instead accept Icon,
SearchInput, and color/theme configuration as props to the SearchOverlay
component. This makes the UI package self-contained and reusable by allowing
consumers to inject their own implementations of these components and styling
preferences, rather than being tightly coupled to the expo-app package.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2a5a930e-36af-4f54-a661-dd91b10d04a1
📒 Files selected for processing (31)
apps/expo/app/(app)/(tabs)/(home)/index.tsxapps/expo/app/(app)/(tabs)/profile/index.tsxapps/expo/app/(app)/_layout.tsxapps/expo/app/(app)/current-pack/[id].tsxapps/expo/app/(app)/demo/index.tsxapps/expo/app/(app)/gear-inventory.tsxapps/expo/app/(app)/messages/conversations.tsxapps/expo/app/(app)/pack-categories/[id].tsxapps/expo/app/(app)/pack-stats/[id].tsxapps/expo/app/(app)/recent-packs.tsxapps/expo/app/(app)/season-suggestions-results.tsxapps/expo/app/(app)/season-suggestions.tsxapps/expo/app/(app)/shared-packs.tsxapps/expo/app/(app)/shopping-list.tsxapps/expo/app/(app)/trail-conditions.tsxapps/expo/app/(app)/weather-alert-preferences.tsxapps/expo/app/(app)/weather-alerts.tsxapps/expo/app/(app)/weight-analysis/[id].tsxapps/expo/features/ai-packs/screens/AIPacksScreen.tsxapps/expo/features/ai/screens/ReportedContentScreen.tsxapps/expo/features/catalog/screens/CatalogItemsScreen.tsxapps/expo/features/feed/screens/FeedScreen.tsxapps/expo/features/guides/screens/GuidesListScreen.tsxapps/expo/features/pack-templates/screens/PackTemplateListScreen.tsxapps/expo/features/packs/screens/PackListScreen.tsxapps/expo/features/trips/screens/TripListScreen.tsxapps/expo/features/weather/screens/LocationsScreen.tsxapps/expo/features/wildlife/screens/WildlifeScreen.tsxpackages/ui/src/app-bar/AppBarAndroid.tsxpackages/ui/src/app-bar/index.tsxpackages/ui/src/search-overlay/SearchOverlay.tsx
| return ( | ||
| <SafeAreaView className="flex-1" edges={['bottom']}> | ||
| <LargeTitleHeader title={t('packs.currentPack')} /> | ||
| <Stack.Screen options={{ ...getAppBarOptions(), title: t('packs.currentPack') }} /> |
Contributor
There was a problem hiding this comment.
Migrated header is likely still suppressed on iOS due parent route options.
Line 133 adds getAppBarOptions(), but apps/expo/app/(app)/_layout.tsx still sets headerShown: false for current-pack/[id]. Since iOS options from getAppBarOptions() do not set headerShown: true, this route can keep the header hidden on iOS.
Suggested fix
- <Stack.Screen
- name="current-pack/[id]"
- options={{
- headerShown: false,
- presentation: 'modal',
- animation: 'slide_from_bottom',
- }}
- />
+ <Stack.Screen
+ name="current-pack/[id]"
+ options={{
+ presentation: 'modal',
+ animation: 'slide_from_bottom',
+ }}
+ />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/app/`(app)/current-pack/[id].tsx at line 133, The Stack.Screen
options at line 133 in the current-pack/[id] route are spreading
getAppBarOptions() but the parent layout still has headerShown: false set for
this route. Since getAppBarOptions() does not explicitly set headerShown: true,
the parent's suppression persists on iOS. Add headerShown: true to the options
object (either within the spread or after it) to explicitly override the parent
route's header suppression and ensure the migrated header is visible on iOS.
Comment on lines
+128
to
+130
| <Pressable onPress={handleAddLocation} style={{ padding: 14 }}> | ||
| <Icon name="plus" size={28} color={colors.foreground} /> | ||
| </Pressable> |
Contributor
There was a problem hiding this comment.
Replace inline style with NativeWind class.
The style={{ padding: 14 }} inline object violates the NativeWind coding guideline. Use Tailwind classes instead.
🎨 Use NativeWind class
- <Pressable onPress={handleAddLocation} style={{ padding: 14 }}>
+ <Pressable onPress={handleAddLocation} className="p-3.5">
<Icon name="plus" size={28} color={colors.foreground} />
</Pressable>As per coding guidelines: "Use NativeWind (Tailwind) classes for styling — avoid StyleSheet objects for layout/color."
📝 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.
Suggested change
| <Pressable onPress={handleAddLocation} style={{ padding: 14 }}> | |
| <Icon name="plus" size={28} color={colors.foreground} /> | |
| </Pressable> | |
| <Pressable onPress={handleAddLocation} className="p-3.5"> | |
| <Icon name="plus" size={28} color={colors.foreground} /> | |
| </Pressable> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/expo/features/weather/screens/LocationsScreen.tsx` around lines 128 -
130, The Pressable component contains an inline style object with padding that
violates NativeWind guidelines. Replace the style prop `style={{ padding: 14 }}`
with the appropriate NativeWind/Tailwind class (such as `p-3` or `p-4` depending
on your design system's spacing scale) to align with the coding standard of
using Tailwind classes for styling instead of inline style objects.
Source: Coding guidelines
Comment on lines
+1
to
+2
| import { Icon } from 'expo-app/components/Icon'; | ||
| import { useColorScheme } from 'expo-app/lib/hooks/useColorScheme'; |
Contributor
There was a problem hiding this comment.
packages/ui should not depend on expo-app internals.
Line 1 and Line 2 import app-layer modules into a shared UI package. That couples @packrat/ui to one app and breaks portability/integration for other consumers. Move these dependencies behind @packrat/ui interfaces (props/context) or relocate the component into apps/expo.
As per coding guidelines, "packages/ui/**: Exports must be compatible with both iOS and Android."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui/src/app-bar/AppBarAndroid.tsx` around lines 1 - 2, The
AppBarAndroid.tsx component in packages/ui has direct imports from expo-app
internals (Icon from expo-app/components and useColorScheme from
expo-app/lib/hooks), which violates the portability requirement of the shared UI
package. Remove these direct imports and instead accept the Icon component and
colorScheme values as props passed into AppBarAndroid, or use a context provider
pattern to supply these dependencies. This way the UI component remains
decoupled from expo-app specifics and can be reused across different consumers.
Source: Coding guidelines
Replaces the near-invisible absolute-positioned pill animation with a flex-1 rounded container that wraps the magnify icon + input + clear button, matching the MD3-style search bar visible in the reference screenshot.
…ap and extra icon SearchInput from NativeWindUI is already a rounded-full pill with a built-in magnify icon. Wrapping it in an additional pill View and adding a second Icon caused two icons and no visible text. Now the SearchInput is the pill; only the back arrow sits outside it.
Reference shows bare ← |Search... with no pill or icon. Replace SearchInput (which always renders a rounded-full pill container) with a plain RN TextInput. Also fixes the className TS error on Animated.View.
…font (20sp), taller row (64dp)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements Phase 1 and Phase 2 of the NativeWindUI → Expo UI migration plan (
docs/migrations/nativewindui-to-expo-ui.md).Phase 1 — utility cleanup
useColorSchemecall sites (20 files) migrated from@packrat/ui/nativewindui→expo-app/lib/hooks/useColorSchemecncall sites (3 files) migrated from@packrat/ui/nativewindui→expo-app/lib/cnPhase 2 — Expo Router native header patterns
LargeTitleHeader(~19 screens) replaced with<Stack.Screen options={{ title, headerLargeTitle: true }} />inline optionsSearchInput(NativeWindUI wrapper) rewritten as a nativeTextInputwrapper withuseKeyboardHideBlurheaderSearchBarOptionswired up on screens that had a search bar (home,catalog,packs,guides,templates,trips,locations,wildlife)LargeTitleSearchBarMethodsrefs removed; programmaticclearText()calls replaced withsetSearchValue('')state clearingAdaptiveSearchHeaderremoved from all consumersLargeTitleHeaderOverlapFixIOS.tsxandLargeTitleHeaderSearchContentContainer.tsxdeleted (workarounds no longer needed)headerShown: falseremoved from 8 Stack.Screen entries so the native header becomes visibleTooling added
scripts/lint/nativewindui-migration.ts— CI script that counts remaining adapter export groups per phase and detects any direct@packrat-ai/nativewinduibypass imports inapps/expopackage.jsoncheck:migrationscriptbiome.jsonoverride:organizeImports: offfor the adapter tracker file (prevents Biome from collapsing per-phase export blocks)packages/ui/nativewindui/index.tsupdated to mark Phase 1 + 2 as done; Phases 3–5 remain trackedMigration progress after this PR
Test plan
bun check:migrationpasses with no adapter bypass violationsbun lintpasses (Biome, including import ordering)SearchInput(catalog, packs, guides, etc.)headerSearchBarOptionsdifferently — confirm no regressions)from '@packrat-ai/nativewindui'imports inapps/expoSummary by CodeRabbit
Release Notes
New Features
Refactor
Bug Fixes
Documentation