fix(search): empty list when 'Hide seen' filters all (#574)#582
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (8)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughWhen the "hide seen" filter hides all search results, the UI auto-loads more pages when available; if exhausted, it shows a localized full-screen banner with a "show all results" button that dispatches a new SearchAction handled by the ViewModel to disable the hide-seen preference. ChangesSearch hide-seen filter empty state
Sequence DiagramsequenceDiagram
participant User
participant SearchScreen
participant SearchViewModel
participant tweaksRepository
User->>SearchScreen: view search results
SearchScreen->>SearchScreen: rawRepos non-empty, visibleRepos empty, hide-seen enabled
alt more pages available
SearchScreen->>SearchScreen: LaunchedEffect -> dispatch LoadMore
else no more pages
SearchScreen->>User: display empty-state banner with "show all results" button
User->>SearchScreen: tap "show all results"
SearchScreen->>SearchViewModel: dispatch OnDisableHideSeenForResults
SearchViewModel->>tweaksRepository: setHideSeenEnabled(false)
tweaksRepository-->>SearchViewModel: preference updated
SearchViewModel-->>SearchScreen: state updated (hide-seen=false)
SearchScreen->>User: previously hidden repositories shown
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
Greptile Summary
Confidence Score: 4/5Functional P1 bug in the auto-paginate loop must be fixed before merge; all other changes are straightforward. Single P1 — infinite retry loop on network error during auto-pagination. The rest of the PR (spinner, banner, strings, ViewModel action) is clean. No P0s present. feature/search/presentation/src/commonMain/kotlin/zed/rainxch/search/presentation/SearchRoot.kt — the auto-paginate LaunchedEffect condition at lines 245–252. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Page loaded
repositories.isNotEmpty] --> B{visibleRepos
empty?}
B -- No --> C[Show grid normally]
B -- Yes --> D{isHideSeenEnabled?}
D -- No --> E[Show empty state
or zero-result UI]
D -- Yes --> F{hasMorePages?}
F -- Yes --> G[Show spinner
'Searching for unseen repos']
G --> H{errorMessage
null?}
H -- Yes --> I[LaunchedEffect fires
LoadMore]
H -- No --> J[⚠️ Loop: LaunchedEffect
fires LoadMore again]
I --> K[isLoadingMore = true
next page fetches]
K --> L{More unseen
pages exist?}
L -- Yes, all seen --> F
L -- No unseen found --> M[hasMorePages = false]
F -- No --> N[Show banner
'All results hidden by Hide seen'
+ Disable filter button]
N --> O{User taps
Disable filter}
O --> P[setHideSeenEnabled false
global preference updated
all repos become visible]
Reviews (3): Last reviewed commit: "fix(search): show progress during hide-s..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+583
to
+588
| GithubStoreButton( | ||
| text = stringResource(Res.string.show_all_results), | ||
| onClick = { | ||
| onAction(SearchAction.OnDisableHideSeenForResults) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
OnDisableHideSeenForResults calls tweaksRepository.setHideSeenEnabled(false), which persists the change app-wide and also affects the home screen. The button label "Show all results" (and all 13 locale equivalents) reads like a search-session action, not a settings toggle. Users who enabled Hide Seen intentionally may be confused when it stays off after leaving the search screen.
Consider either updating the label to make the scope explicit (e.g. "Disable 'Hide seen' filter") or scoping the override to the current search session only.
Comment on lines
+245
to
+252
| if (state.repositories.isNotEmpty() && | ||
| state.visibleRepos.isEmpty() && | ||
| state.isHideSeenEnabled && | ||
| state.hasMorePages && | ||
| !state.isLoadingMore && | ||
| !state.isLoading | ||
| ) { | ||
| currentOnAction(SearchAction.LoadMore) |
There was a problem hiding this comment.
When a page load fails, the catch blocks in performSearch set isLoadingMore = false but leave hasMorePages unchanged (still true). This change of isLoadingMore is a LaunchedEffect key, so the effect re-runs immediately, the condition passes again (!isLoadingMore, !isLoading, hasMorePages), and LoadMore fires again — looping until the process crashes or the user navigates away. The errorMessage is also set on each error but cleared immediately by the next attempt, so the error is never shown.
Add state.errorMessage == null to the guard:
Suggested change
| if (state.repositories.isNotEmpty() && | |
| state.visibleRepos.isEmpty() && | |
| state.isHideSeenEnabled && | |
| state.hasMorePages && | |
| !state.isLoadingMore && | |
| !state.isLoading | |
| ) { | |
| currentOnAction(SearchAction.LoadMore) | |
| if (state.repositories.isNotEmpty() && | |
| state.visibleRepos.isEmpty() && | |
| state.isHideSeenEnabled && | |
| state.hasMorePages && | |
| !state.isLoadingMore && | |
| !state.isLoading && | |
| state.errorMessage == null | |
| ) { |
This was referenced May 12, 2026
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.
Search screen showed "N results found" over an empty grid when the global 'Hide seen' tweak filtered out every API hit. UI had no explanation and no way to recover from the search screen itself — the user had to dig into Tweaks to figure out why.
Add a centered banner + 'Show all results' CTA that disables the global Hide-seen flag in one tap. Triggers only when
repositories.isNotEmpty() && visibleRepos.isEmpty() && isHideSeenEnabled— never fires for genuine zero-result searches.Strings localized across 13 locales (en + 12). Closes #574.
Not compile-verified locally — ~/.gradle/wrapper unreachable (SSD unmounted). Please run before merge:
:feature:search:presentation:compileDebugKotlinAndroid:feature:search:presentation:compileKotlinJvmSummary by CodeRabbit
New Features
Chores