fix(ios): hide catalog item count labels and category filter behind search overlay#1897
Conversation
…alog search on iOS Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
|
@coderabbitai Please review this PR for code quality, best practices, and potential issues. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 115 🏁 Script executed: Length of output: 9847
The overall approach is correct and clean. Here's my review: 🐛 Critical –
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduced a debounced readiness flag and adjusted search state handling in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/catalog/screens/CatalogItemsScreen.tsx`:
- Around line 164-173: The UI gating currently uses isSearching derived from
debouncedSearchValue which causes a 400ms lag; change isSearching in
CatalogItemsScreen to derive from the raw searchValue (e.g.,
Boolean(searchValue?.trim())) so the CategoriesFilter and list header hide/show
immediately, while keeping debouncedSearchValue only for triggering the vector
query and data fetch; update any places that check isSearching (filter render
and header render) to use the new isSearching derived from searchValue and leave
debouncedSearchValue usage confined to the query logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 059688ca-45d6-4d2f-be4b-9abefdf29e89
📒 Files selected for processing (1)
apps/expo/features/catalog/screens/CatalogItemsScreen.tsx
|
@copilot Please address the CodeRabbit review feedback on this PR (1 inline comments). |
|
@copilot CodeRabbit has reviewed this PR. Please address the feedback. |
|
@copilot CodeRabbit has reviewed this PR. Please address the feedback. |
… for query Co-authored-by: andrew-bierman <94939237+andrew-bierman@users.noreply.github.com>
Addressed in c3e7a85. Changed |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Default searchResults to empty array to prevent TypeError when vectorResult is undefined (e.g., before results arrive or on error) - Remove unused _isVectorFetching destructured variable
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@copilot CodeRabbit has reviewed this PR. Please address the feedback. |
There was a problem hiding this comment.
Pull request overview
Fixes an iOS layering issue in the Catalog screen where list header/count text and the category filter could remain visible underneath the LargeTitleHeader search overlay during active search.
Changes:
- Make
isSearchingderive from the raw (non-debounced) search input so header/filter hide immediately (no debounce lag). - Add
isQueryReadyto show a loading state during the debounce window before vector search results are ready. - Conditionally hide
CategoriesFilterandFlatList’sListHeaderComponentwhile searching.
You can also share your feedback on Copilot code review. Take the survey.
Deploying packrat-landing with
|
| Latest commit: |
c729966
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://850eb3e0.packrat-landing.pages.dev |
| Branch Preview URL: | https://copilot-fix-catalog-search-o.packrat-landing.pages.dev |
Isthisanmol
left a comment
There was a problem hiding this comment.
Looks good to me and I have removed animate-pulse for now from categoryfilter as it was giving error. In future we can make custom custom component just like animate-pulse.
…verlay-issue-again fix(ios): hide catalog item count labels and category filter behind search overlay
On iOS, the
FlatList'sListHeaderComponent("Showing X of Y items") andCategoriesFilterremained rendered beneath theLargeTitleHeadersearch overlay, causing them to peek through at the top of the screen during active search.Changes
CatalogItemsScreen.tsxisSearchingnow derives from rawsearchValue.trim().length > 0instead of the debounced value, soCategoriesFilterandListHeaderComponenthide/show immediately when the user starts or clears the search — no 400ms lagisQueryReady = debouncedSearchValue.trim().length > 0to gate when vector search results are ready to displayCategoriesFilterrender on!isSearchingnullfromListHeaderComponentwhenisSearchingis true!isQueryReady || isVectorLoading, covering the debounce window before the vector fetch firesBoth elements are restored immediately when the user clears the search.
Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.
Summary by CodeRabbit