fix(expo): convert null packId to undefined in TripForm submission#2287
Merged
Conversation
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/c06bd591-dda9-4bcd-a776-5e77ee05fddf Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/0d5003cc-1c2f-4b67-89e6-70823c2c812f Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/0d5003cc-1c2f-4b67-89e6-70823c2c812f Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/0d5003cc-1c2f-4b67-89e6-70823c2c812f Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/0d5003cc-1c2f-4b67-89e6-70823c2c812f Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/5f9c1470-ddc4-4e24-a27d-68261ccb873f Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/bb733d52-2184-48a4-a4a5-b6130c47c621 Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
…terns Unify search UI pattern across Guides, Pack Templates, and Trips screens
- Fix check-android-textinput.sh: - Use awk for multiline-capable TextInput import detection - Replace PCRE lookahead with portable grep -v pipeline for third-party check - Change --diff-filter=A to --diff-filter=AM to catch modifications too - Fix android-keyboard-focus-prevention-strategies.md: - Replace non-existent Keyboard.emit() with proper addListener mock pattern - Fix cleanup test to assert remove() on subscription object - Add event type assertion to both unit and integration test examples - Fix android-textinput-keyboard-focus-loss.md: - Correct broken cross-reference paths (../CLAUDE.md -> ../../../CLAUDE.md) - Fix android-keyboard-prevention-implementation-summary.md: - Remove references to non-existent test files - Remove reference to non-existent eslint-android-textinput-rules.js Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/804a88c4-bdca-492f-b2e8-a62b2174dfbe Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
- Delete scripts/check-android-textinput.sh - Remove android-textinput-check entry from lefthook.yml - Update all docs to remove references to the deleted script: - android-keyboard-prevention-implementation-summary.md - android-keyboard-focus-prevention-strategies.md - android-textinput-checklist.md Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/4d463d8b-d0b7-4c89-943d-fb9c1b4f8177
…ndroid fix(expo): TextInput not losing focus after dismissing keyboard
fix(expo): resolve packId type incompatibility in TripForm
Main -> Development
…prevention-strategies.md Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/c75eaa7f-9950-4180-8099-f638817ae495 Co-authored-by: mikib0 <54102880+mikib0@users.noreply.github.com>
Fixed 8 file(s) based on 8 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/dc3113b9-cce0-4714-9844-ab04fd5624cb Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
TypeScript 5.9.3 (pinned in bun.lock) rejects "6.0" as an invalid value for ignoreDeprecations, causing TS5103 errors in CI. The "6.0" value was added to silence the baseUrl deprecation warning which only appears in TypeScript 6.x, not in the 5.9.x series used by CI. https://claude.ai/code/session_01SypcK5vavfiCGcM8BPtTmY
github-actions
Bot
added
documentation
Contributor
📝 WalkthroughWalkthroughThis pull request implements Android keyboard focus management across the Expo app by introducing custom TextInput and SearchInput components that apply a useKeyboardHideBlur hook, updates import statements across multiple screens to use these new components, removes the TextInputDebug screen, enhances search UI in several screens, and bumps package versions from 2.0.22 to 2.0.23 across all packages. Changes
Sequence DiagramsequenceDiagram
participant Screen as Expo Screen
participant Component as TextInput/SearchInput
participant Hook as useKeyboardHideBlur
participant RN as React Native Keyboard
participant NativeInput as Native Input
Screen->>Component: Render with ref
Component->>Hook: Pass ref on mount
Hook->>RN: Subscribe to keyboardDidHide
Note over NativeInput: User dismisses keyboard
RN->>Hook: Fire keyboardDidHide event
Hook->>Hook: Check ref validity
Hook->>NativeInput: Call blur()
NativeInput->>NativeInput: Lose focus
NativeInput-->>Component: Blur confirmed
Note over Component: Input focus cleared, keyboard won't reopen on tap
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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 docstrings
🧪 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 Expo Unit Tests Coverage (./apps/expo)
File CoverageNo changed files found. |
Contributor
Coverage Report for API Unit Tests Coverage (./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: 5
🧹 Nitpick comments (3)
apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx (1)
168-185: Avoid invokinglistHeader()twice per render.
listHeader()is called both forstickyHeaderIndices(line 232) andListHeaderComponent(line 234). Each call constructs a fresh React element (including theFeaturedPacksSectionsubtree) on every render. Compute it once and reuse the result.♻️ Proposed refactor
+ const headerNode = listHeader(); <FlatList data={filteredTemplates} keyExtractor={(item) => item.id} renderItem={({ item }) => ( <View className="px-4 pt-4"> <PackTemplateCard templateId={item.id} onPress={handleTemplatePress} /> </View> )} - stickyHeaderIndices={listHeader() ? [0] : undefined} + stickyHeaderIndices={headerNode ? [0] : undefined} stickyHeaderHiddenOnScroll - ListHeaderComponent={listHeader()} + ListHeaderComponent={headerNode}Place
const headerNode = listHeader();just before thereturn(or memoize viauseMemokeyed onsearchValue,selectedTemplateTypeIndex,filteredTemplates.length).Also applies to: 232-234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx` around lines 168 - 185, The listHeader() function is being invoked twice per render (for stickyHeaderIndices and ListHeaderComponent) which recreates the FeaturedPacksSection subtree each time; compute the header once and reuse it by assigning const headerNode = listHeader() (or memoize via useMemo with dependencies [searchValue.trim(), selectedTemplateTypeIndex, filteredTemplates.length]) just before the return, then pass headerNode to both stickyHeaderIndices/ListHeaderComponent instead of calling listHeader() again.apps/expo/features/trips/components/TripForm.tsx (1)
115-119: Fix is correct; consider a slightly simpler equivalent.The normalization correctly maps
''(from the Picker's "no pack selected" item withvalue=""),null(fromtrip?.packIddefaults), andundefinedall toundefined. The logic is sound.Optional simplification (since
packIdis a non-empty string id when set, falsy coalescing is safe here):♻️ Optional simplification
const submitData = { ...value, location: location ?? value.location, - packId: value.packId === '' ? undefined : (value.packId ?? undefined), + packId: value.packId || undefined, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/features/trips/components/TripForm.tsx` around lines 115 - 119, The submitData normalization for packId can be simplified: in the object build inside submitData (referencing submitData and value.packId in TripForm.tsx), replace the ternary and nullish combo (value.packId === '' ? undefined : (value.packId ?? undefined)) with a simpler falsy-coalescing expression like value.packId || undefined so '' / null / undefined all map to undefined; leave the location mapping (location ?? value.location) as-is.apps/expo/components/SearchInput.tsx (1)
11-27: Optional: simplify via React 19 ref-as-prop.With React 19,
forwardRefis no longer required —refcan be passed as a normal prop, which would let you dropforwardRef/useImperativeHandle/asNonNullableRef/assertPresenthere and just attach both refs to the underlying component. The current implementation works, but it is more ceremony than necessary andassertPresentwould throw if the imperative handle callback ever fires before the child has mounted its ref.♻️ Possible simplification
-import { assertPresent } from '@packrat/guards'; -import { SearchInput as NativeWindUISearchInput } from '@packrat/ui/nativewindui'; +import { SearchInput as NativeWindUISearchInput } from '@packrat/ui/nativewindui'; import { useKeyboardHideBlur } from 'expo-app/lib/hooks/useKeyboardHideBlur'; import { asNonNullableRef } from 'expo-app/lib/utils/asNonNullableRef'; -import { forwardRef, useImperativeHandle, useRef } from 'react'; +import { type ComponentProps, type ComponentRef, type Ref, useRef } from 'react'; -export const SearchInput = forwardRef< - React.ComponentRef<typeof NativeWindUISearchInput>, - React.ComponentProps<typeof NativeWindUISearchInput> ->((props, ref) => { - const searchInputRef = useRef<React.ComponentRef<typeof NativeWindUISearchInput>>(null); - - useKeyboardHideBlur(asNonNullableRef(searchInputRef)); - - useImperativeHandle(ref, () => { - assertPresent(searchInputRef.current); - return searchInputRef.current; - }, []); - - return <NativeWindUISearchInput ref={searchInputRef} {...props} />; -}); - -SearchInput.displayName = 'SearchInput'; +type SearchInputRef = ComponentRef<typeof NativeWindUISearchInput>; +type SearchInputProps = ComponentProps<typeof NativeWindUISearchInput> & { + ref?: Ref<SearchInputRef>; +}; + +export const SearchInput = ({ ref, ...props }: SearchInputProps) => { + const searchInputRef = useRef<SearchInputRef>(null); + useKeyboardHideBlur(asNonNullableRef(searchInputRef)); + return ( + <NativeWindUISearchInput + ref={(node) => { + searchInputRef.current = node; + if (typeof ref === 'function') ref(node); + else if (ref) ref.current = node; + }} + {...props} + /> + ); +};As per library documentation: in React 19 "ref is now a regular prop; no need for forwardRef".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/expo/components/SearchInput.tsx` around lines 11 - 27, The SearchInput wrapper is unnecessarily using forwardRef, useImperativeHandle, asNonNullableRef, and assertPresent; simplify by treating ref as a normal prop (drop forwardRef) and stop calling useImperativeHandle/assertPresent. Keep the internal searchInputRef for useKeyboardHideBlur, then pass both the external ref prop and searchInputRef to NativeWindUISearchInput (merge or pass as an array/merged ref) so useKeyboardHideBlur(searchInputRef) still works and the external ref is forwarded; remove useImperativeHandle, asNonNullableRef, and assertPresent usages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx`:
- Around line 151-163: The Icon component in PackTemplateListScreen is being
passed a Tailwind class string via the color prop (e.g.,
color="text-muted-foreground") which expects a real color value; update the Icon
usages in this file (the one shown near the filteredTemplates empty state and
the other preexisting occurrence around the later code) to derive a color from
the theme using useColorScheme() (for example use const { colors } =
useColorScheme(); then pass colors.grey2 or the appropriate token to Icon's
color prop) so the color prop receives an actual color value instead of a class
name.
In `@apps/expo/features/trips/screens/TripListScreen.tsx`:
- Around line 131-141: The Icon color prop is receiving a Tailwind class string
instead of a color value; update renderSearchContent in TripListScreen to pass a
real color token (e.g., colors.grey2 or the appropriate token from
useColorScheme()) to <Icon ... color={...}> instead of "text-muted-foreground",
and ensure you import/derive colors from the existing useColorScheme() call in
TripListScreen; also fix the identical misuse at the other Icon usage around the
earlier preexisting line (the other Icon that currently uses
"text-muted-foreground").
In `@docs/android-keyboard-focus-prevention-strategies.md`:
- Around line 191-221: The test is asserting the wrong handler:
useKeyboardHideBlur triggers ref.current.blur() on keyboardDidHide, not the
onBlur prop, so update the test to verify blur() was called instead of onBlur;
specifically, in the test that sets up the mocked Keyboard.addListener and
capturedCallback, spy on or mock the underlying TextInput instance's blur method
(or supply a ref with a mocked blur) and assert that blur() was invoked after
invoking capturedCallback, referencing useKeyboardHideBlur, keyboardDidHide,
capturedCallback, and the rendered TextInput/testID "input".
- Around line 69-77: The docs currently suggest an ESLint custom rule
("no-direct-textinput-import") which is incorrect for this repo; replace that
block with a Biome configuration instructing contributors to add a
"noRestrictedImports" rule in biome.json (under linter.rules.style) that targets
the "react-native" package and restricts the "TextInput" import with a clear
message directing people to use expo-app/components/TextInput instead; update
the section header to "2.2 Biome Restriction" and remove the ESLint snippet.
In `@docs/solutions/ui-bugs/android-textinput-keyboard-focus-loss.md`:
- Around line 195-211: The example OTPField snippet is inconsistent: it types
the ref as React.useRef<TextInput>(null) while importing TextInput as a
component and then renders <TextField ...>, causing compilation/copy-paste
errors; fix by making the ref type and rendered component match—either change
the ref to React.ComponentRef<typeof TextInput> (or RNTextInput if you mean the
native class) when rendering <TextInput ...> and calling
useKeyboardHideBlur(inputRef), or change the rendered <TextField ...> to
<TextInput ...> so OTPField, useKeyboardHideBlur, TextInput, and the ref type
are consistent.
---
Nitpick comments:
In `@apps/expo/components/SearchInput.tsx`:
- Around line 11-27: The SearchInput wrapper is unnecessarily using forwardRef,
useImperativeHandle, asNonNullableRef, and assertPresent; simplify by treating
ref as a normal prop (drop forwardRef) and stop calling
useImperativeHandle/assertPresent. Keep the internal searchInputRef for
useKeyboardHideBlur, then pass both the external ref prop and searchInputRef to
NativeWindUISearchInput (merge or pass as an array/merged ref) so
useKeyboardHideBlur(searchInputRef) still works and the external ref is
forwarded; remove useImperativeHandle, asNonNullableRef, and assertPresent
usages.
In `@apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx`:
- Around line 168-185: The listHeader() function is being invoked twice per
render (for stickyHeaderIndices and ListHeaderComponent) which recreates the
FeaturedPacksSection subtree each time; compute the header once and reuse it by
assigning const headerNode = listHeader() (or memoize via useMemo with
dependencies [searchValue.trim(), selectedTemplateTypeIndex,
filteredTemplates.length]) just before the return, then pass headerNode to both
stickyHeaderIndices/ListHeaderComponent instead of calling listHeader() again.
In `@apps/expo/features/trips/components/TripForm.tsx`:
- Around line 115-119: The submitData normalization for packId can be
simplified: in the object build inside submitData (referencing submitData and
value.packId in TripForm.tsx), replace the ternary and nullish combo
(value.packId === '' ? undefined : (value.packId ?? undefined)) with a simpler
falsy-coalescing expression like value.packId || undefined so '' / null /
undefined all map to undefined; leave the location mapping (location ??
value.location) as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d86a46c3-e124-4c18-bcb9-4bd97037bc27
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
apps/admin/package.jsonapps/expo/app.config.tsapps/expo/app/(app)/ai-chat.tsxapps/expo/app/(app)/messages/chat.android.tsxapps/expo/app/(app)/messages/chat.tsxapps/expo/app/(app)/textinputdebug.tsxapps/expo/app/(app)/trip/location-search.tsxapps/expo/app/auth/one-time-password.tsxapps/expo/components/SearchInput.tsxapps/expo/components/TextInput.tsxapps/expo/features/ai-packs/screens/AIPacksScreen.tsxapps/expo/features/ai/components/ReportModal.tsxapps/expo/features/catalog/components/CatalogBrowserModal.tsxapps/expo/features/catalog/screens/AddCatalogItemDetailsScreen.tsxapps/expo/features/catalog/screens/PackSelectionScreen.tsxapps/expo/features/feed/screens/CreatePostScreen.tsxapps/expo/features/feed/screens/PostDetailScreen.tsxapps/expo/features/guides/screens/GuidesListScreen.tsxapps/expo/features/pack-templates/screens/PackTemplateListScreen.tsxapps/expo/features/trail-conditions/components/SubmitConditionReportForm.tsxapps/expo/features/trips/components/TripForm.tsxapps/expo/features/trips/screens/TripListScreen.tsxapps/expo/features/weather/screens/LocationSearchScreen.tsxapps/expo/features/weather/screens/LocationsScreen.tsxapps/expo/features/wildlife/screens/IdentificationScreen.tsxapps/expo/lib/hooks/useKeyboardHideBlur.tsxapps/expo/lib/i18n/locales/en.jsonapps/expo/package.jsonapps/guides/package.jsonapps/landing/package.jsondocs/android-keyboard-focus-prevention-strategies.mddocs/android-keyboard-prevention-implementation-summary.mddocs/android-textinput-checklist.mddocs/solutions/ui-bugs/android-textinput-keyboard-focus-loss.mdpackage.jsonpackages/analytics/package.jsonpackages/api-client/package.jsonpackages/api/container_src/package.jsonpackages/api/package.jsonpackages/checks/package.jsonpackages/cli/package.jsonpackages/config/package.jsonpackages/env/package.jsonpackages/guards/package.jsonpackages/mcp/package.jsonpackages/ui/package.jsonpackages/web-ui/package.json
💤 Files with no reviewable changes (1)
- apps/expo/app/(app)/textinputdebug.tsx
Comment on lines
+151
to
+163
| {filteredTemplates.length === 0 && ( | ||
| <View className="flex-1 items-center justify-center p-8"> | ||
| <View className="mb-4 rounded-full bg-muted p-4"> | ||
| <Icon name="magnify" size={32} color="text-muted-foreground" /> | ||
| </View> | ||
| <Text className="mb-1 text-lg font-medium text-foreground"> | ||
| {t('packTemplates.noTemplatesFound')} | ||
| </Text> | ||
| <Text className="text-center text-muted-foreground"> | ||
| {t('packTemplates.tryDifferentSearch')} | ||
| </Text> | ||
| </View> | ||
| )} |
Contributor
There was a problem hiding this comment.
color prop receives a Tailwind class string, not a color value.
Same issue as in TripListScreen.tsx: <Icon ... color="text-muted-foreground" /> passes a className where a color value is expected. Use a color token from useColorScheme() (e.g., colors.grey2) instead. The same anti-pattern exists at the preexisting line 238 — worth aligning at the same time.
🐛 Proposed fix
{filteredTemplates.length === 0 && (
<View className="flex-1 items-center justify-center p-8">
<View className="mb-4 rounded-full bg-muted p-4">
- <Icon name="magnify" size={32} color="text-muted-foreground" />
+ <Icon name="magnify" size={32} color={colors.grey2} />
</View>📝 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
| {filteredTemplates.length === 0 && ( | |
| <View className="flex-1 items-center justify-center p-8"> | |
| <View className="mb-4 rounded-full bg-muted p-4"> | |
| <Icon name="magnify" size={32} color="text-muted-foreground" /> | |
| </View> | |
| <Text className="mb-1 text-lg font-medium text-foreground"> | |
| {t('packTemplates.noTemplatesFound')} | |
| </Text> | |
| <Text className="text-center text-muted-foreground"> | |
| {t('packTemplates.tryDifferentSearch')} | |
| </Text> | |
| </View> | |
| )} | |
| {filteredTemplates.length === 0 && ( | |
| <View className="flex-1 items-center justify-center p-8"> | |
| <View className="mb-4 rounded-full bg-muted p-4"> | |
| <Icon name="magnify" size={32} color={colors.grey2} /> | |
| </View> | |
| <Text className="mb-1 text-lg font-medium text-foreground"> | |
| {t('packTemplates.noTemplatesFound')} | |
| </Text> | |
| <Text className="text-center text-muted-foreground"> | |
| {t('packTemplates.tryDifferentSearch')} | |
| </Text> | |
| </View> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/features/pack-templates/screens/PackTemplateListScreen.tsx` around
lines 151 - 163, The Icon component in PackTemplateListScreen is being passed a
Tailwind class string via the color prop (e.g., color="text-muted-foreground")
which expects a real color value; update the Icon usages in this file (the one
shown near the filteredTemplates empty state and the other preexisting
occurrence around the later code) to derive a color from the theme using
useColorScheme() (for example use const { colors } = useColorScheme(); then pass
colors.grey2 or the appropriate token to Icon's color prop) so the color prop
receives an actual color value instead of a class name.
Comment on lines
+131
to
+141
| {filteredTrips.length === 0 && ( | ||
| <View className="flex-1 items-center justify-center p-8"> | ||
| <View className="mb-4 rounded-full bg-muted p-4"> | ||
| <Icon name="magnify" size={32} color="text-muted-foreground" /> | ||
| </View> | ||
| <Text className="mb-1 text-lg font-medium text-foreground"> | ||
| {t('trips.noTripsFound')} | ||
| </Text> | ||
| <Text className="text-center text-muted-foreground">{t('trips.noSearchResults')}</Text> | ||
| </View> | ||
| )} |
Contributor
There was a problem hiding this comment.
color prop receives a Tailwind class string, not a color value.
<Icon ... color="text-muted-foreground" /> passes a className where a color value is expected. The icon will likely fall back to a default/invalid color rather than render in the muted-foreground theme color. Use colors.grey2 (or the appropriate token from useColorScheme()) instead.
🐛 Proposed fix
{filteredTrips.length === 0 && (
<View className="flex-1 items-center justify-center p-8">
<View className="mb-4 rounded-full bg-muted p-4">
- <Icon name="magnify" size={32} color="text-muted-foreground" />
+ <Icon name="magnify" size={32} color={colors.grey2} />
</View>You'll also need to pull colors from useColorScheme() in TripsListScreen (already done at the top scope; just reference it inside renderSearchContent). Note: the same anti-pattern exists at the preexisting line 92 — worth fixing in the same pass for consistency.
📝 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
| {filteredTrips.length === 0 && ( | |
| <View className="flex-1 items-center justify-center p-8"> | |
| <View className="mb-4 rounded-full bg-muted p-4"> | |
| <Icon name="magnify" size={32} color="text-muted-foreground" /> | |
| </View> | |
| <Text className="mb-1 text-lg font-medium text-foreground"> | |
| {t('trips.noTripsFound')} | |
| </Text> | |
| <Text className="text-center text-muted-foreground">{t('trips.noSearchResults')}</Text> | |
| </View> | |
| )} | |
| {filteredTrips.length === 0 && ( | |
| <View className="flex-1 items-center justify-center p-8"> | |
| <View className="mb-4 rounded-full bg-muted p-4"> | |
| <Icon name="magnify" size={32} color={colors.grey2} /> | |
| </View> | |
| <Text className="mb-1 text-lg font-medium text-foreground"> | |
| {t('trips.noTripsFound')} | |
| </Text> | |
| <Text className="text-center text-muted-foreground">{t('trips.noSearchResults')}</Text> | |
| </View> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/expo/features/trips/screens/TripListScreen.tsx` around lines 131 - 141,
The Icon color prop is receiving a Tailwind class string instead of a color
value; update renderSearchContent in TripListScreen to pass a real color token
(e.g., colors.grey2 or the appropriate token from useColorScheme()) to <Icon ...
color={...}> instead of "text-muted-foreground", and ensure you import/derive
colors from the existing useColorScheme() call in TripListScreen; also fix the
identical misuse at the other Icon usage around the earlier preexisting line
(the other Icon that currently uses "text-muted-foreground").
Comment on lines
+69
to
+77
| #### 2.2 ESLint Custom Rules | ||
| Add to ESLint config: | ||
| ```json | ||
| { | ||
| "rules": { | ||
| "no-direct-textinput-import": "error" | ||
| } | ||
| } | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'biome.json'
fd -t f '.eslintrc*'
rg -n '"no-restricted-imports"|"noRestrictedImports"' -C2Repository: PackRat-AI/PackRat
Length of output: 1802
🌐 Web query:
Biome noRestrictedImports rule configuration 2024
💡 Result:
The Biome linter rule noRestrictedImports (under the "style" group) disallows specified modules when loaded by import or require. It supports configuration via biome.json. Basic enablement: { "linter": { "rules": { "style": { "noRestrictedImports": "error" } } } } With options for paths: { "linter": { "rules": { "style": { "noRestrictedImports": { "options": { "paths": { "lodash": "Using lodash is not encouraged", "underscore": "Using underscore is not encouraged" } } } } } } } Advanced options include per-path objects with "importNames" (forbidden import names like ["Bar"], "default", ""), "allowImportNames" (allowed names), and "message"; plus "patterns" array for gitignore-style globs (e.g., [{"group": ["import-foo/", "!import-foo/bar"]}]), and regex via "importNamePattern". This feature parity with ESLint's no-restricted-imports was added in 2024 via PRs like #4596 (Nov 2024, importNames/allowImportNames) and related work for patterns. Official docs confirm full support as of 2026.
Citations:
- 1: https://biomejs.dev/linter/rules/no-restricted-imports/
- 2: feat(noRestrictedImports): add import_names and allow_import_names settings biomejs/biome#4596
- 3: feat(linter): noRestrictedImports add
patternsoption biomejs/biome#4302 - 4: Any plans for noRestrictedImports feature parity? biomejs/biome#3216
- 5: https://next.biomejs.dev/linter/rules/no-restricted-imports
- 6: https://github.com/biomejs/biome/blob/main/crates/biome_js_analyze/src/lint/style/no_restricted_imports.rs
🌐 Web query:
Biome 2.0 forbid imports react-native rule
💡 Result:
Biome 2.0+ does not have a generic built-in rule named “forbid imports react-native rule”, but you can achieve the same goal using Biome’s linter rule noRestrictedImports. How to forbid React Native imports (e.g., block deep imports like react-native/...) with Biome 1) Use linter rule lint/style/noRestrictedImports. - The rule “disallow specified modules when loaded by import or require” and can be configured via paths and importName restrictions. - Example configuration (biome.jsonc) to forbid any import whose module specifier starts with "react-native/": { "linter": { "rules": { "style": { "noRestrictedImports": { "options": { "patterns": [ { "group": [ "react-native/*" ], "message": "Avoid deep React Native imports; use the package root instead." } ] } } } } } } Notes - noRestrictedImports supports path restrictions and (since v2.2.0) also supports a patterns option for gitignore-style include/exclude groups, plus regex-based importNamePattern. - If what you actually mean by “react-native rule” is a dedicated rule for React Native deep imports, Biome added a nursery rule named noReactNativeDeepImports (designed to permit top-level react-native imports while flagging specifiers starting with react-native/). If you tell me which exact behavior you want (block entire react-native package vs. only deep imports vs. only specific symbols like Clipboard), I can provide the exact Biome config snippet to match it.
Citations:
- 1: https://biomejs.dev/linter/rules/no-restricted-imports/
- 2: https://www.biomejs.dev/linter/rules/no-restricted-imports
- 3: https://biomejs.dev/internals/changelog/version/2-2-0
- 4: feat(lint): add react native deep import rule biomejs/biome#10023
- 5: https://biomejs.dev/internals/changelog/version/2-0-0...latest
Replace ESLint custom rule recommendation with Biome configuration.
The project enforces linting via Biome 2.0, not ESLint. The ESLint rule suggestion on lines 69-77 will mislead contributors. Use Biome's noRestrictedImports rule instead to forbid direct TextInput imports from react-native:
Replacement using Biome
-#### 2.2 ESLint Custom Rules
-Add to ESLint config:
-```json
-{
- "rules": {
- "no-direct-textinput-import": "error"
- }
-}
-```
+#### 2.2 Biome Restriction
+Add to `biome.json` to forbid direct `TextInput` imports from `react-native`:
+```json
+{
+ "linter": {
+ "rules": {
+ "style": {
+ "noRestrictedImports": {
+ "level": "error",
+ "options": {
+ "paths": {
+ "react-native": {
+ "importNames": ["TextInput"],
+ "message": "Use TextInput from expo-app/components/TextInput instead."
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+}
+```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/android-keyboard-focus-prevention-strategies.md` around lines 69 - 77,
The docs currently suggest an ESLint custom rule ("no-direct-textinput-import")
which is incorrect for this repo; replace that block with a Biome configuration
instructing contributors to add a "noRestrictedImports" rule in biome.json
(under linter.rules.style) that targets the "react-native" package and restricts
the "TextInput" import with a clear message directing people to use
expo-app/components/TextInput instead; update the section header to "2.2 Biome
Restriction" and remove the ESLint snippet.
Comment on lines
+191
to
+221
| // __tests__/TextInput.integration.test.tsx | ||
| import { render, fireEvent } from '@testing-library/react-native'; | ||
| import { Keyboard } from 'react-native'; | ||
| import { TextInput } from '../components/TextInput'; | ||
|
|
||
| describe('Enhanced TextInput', () => { | ||
| it('should automatically blur on keyboard dismiss', async () => { | ||
| let capturedCallback: (() => void) | undefined; | ||
| jest.spyOn(Keyboard, 'addListener').mockImplementation((event, callback) => { | ||
| expect(event).toBe('keyboardDidHide'); | ||
| capturedCallback = callback; | ||
| return { remove: jest.fn() }; | ||
| }); | ||
|
|
||
| const { getByTestId } = render( | ||
| <TextInput testID="input" placeholder="Test input" /> | ||
| ); | ||
|
|
||
| const input = getByTestId('input'); | ||
|
|
||
| // Focus the input | ||
| fireEvent(input, 'focus'); | ||
|
|
||
| // Simulate keyboard dismissal by invoking the captured listener callback | ||
| capturedCallback?.(); | ||
|
|
||
| // Verify blur was called (you may need to mock the blur method) | ||
| expect(input.props.onBlur).toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
| ``` |
Contributor
There was a problem hiding this comment.
Integration test example asserts on the wrong handler.
useKeyboardHideBlur calls .blur() on the ref's imperative handle via keyboardDidHide; it does not invoke the component's onBlur prop directly. The assertion expect(input.props.onBlur).toHaveBeenCalled() therefore won't reflect the hook's behavior (and input.props.onBlur isn't a jest mock by default). A more faithful integration test would assert that blur() was invoked on the input ref (e.g., by spying on the RN TextInput prototype or providing a mock ref), matching the actual hook implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/android-keyboard-focus-prevention-strategies.md` around lines 191 - 221,
The test is asserting the wrong handler: useKeyboardHideBlur triggers
ref.current.blur() on keyboardDidHide, not the onBlur prop, so update the test
to verify blur() was called instead of onBlur; specifically, in the test that
sets up the mocked Keyboard.addListener and capturedCallback, spy on or mock the
underlying TextInput instance's blur method (or supply a ref with a mocked blur)
and assert that blur() was invoked after invoking capturedCallback, referencing
useKeyboardHideBlur, keyboardDidHide, capturedCallback, and the rendered
TextInput/testID "input".
Comment on lines
+195
to
+211
| ```tsx | ||
| // apps/expo/app/auth/one-time-password.tsx | ||
| function OTPField({ /* props */ }) { | ||
| const inputRef = React.useRef<TextInput>(null); | ||
|
|
||
| // Apply keyboard hide blur fix | ||
| useKeyboardHideBlur(inputRef); | ||
|
|
||
| return ( | ||
| <TextField | ||
| ref={inputRef} | ||
| value={value} | ||
| // ... other props | ||
| /> | ||
| ); | ||
| } | ||
| ``` |
Contributor
There was a problem hiding this comment.
Example code is internally inconsistent.
- Line 198:
React.useRef<TextInput>(null)usesTextInputas a type, but the surrounding docs importTextInputfromexpo-app/components/TextInput, which is a component, not a type. This will not compile; in user code the type should be something likeReact.ComponentRef<typeof TextInput>(orRNTextInputif referring to the RN class). - Line 204: renders
<TextField ...>even though the field is introduced as an OTP input using the enhancedTextInput. The component name should match theuseReftype or vice versa.
These are small but likely to trip up readers copy-pasting the snippet.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/solutions/ui-bugs/android-textinput-keyboard-focus-loss.md` around lines
195 - 211, The example OTPField snippet is inconsistent: it types the ref as
React.useRef<TextInput>(null) while importing TextInput as a component and then
renders <TextField ...>, causing compilation/copy-paste errors; fix by making
the ref type and rendered component match—either change the ref to
React.ComponentRef<typeof TextInput> (or RNTextInput if you mean the native
class) when rendering <TextInput ...> and calling useKeyboardHideBlur(inputRef),
or change the rendered <TextField ...> to <TextInput ...> so OTPField,
useKeyboardHideBlur, TextInput, and the ref type are consistent.
This was referenced May 1, 2026
3 tasks
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
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.
Agent-Logs-Url: https://github.com/PackRat-AI/PackRat/sessions/c06bd591-dda9-4bcd-a776-5e77ee05fddf
Co-authored-by: mikib0 54102880+mikib0@users.noreply.github.com
Summary by CodeRabbit
New Features
Bug Fixes
Chores