bug/ADFA-4854: Filter bar invisible - #1592
Open
dara-abijo-adfa wants to merge 9 commits into
Open
Conversation
dara-abijo-adfa
commented
Jul 28, 2026
dara-abijo-adfa
commented
Contributor
- Show filter bar even when filter result is empty
- Dismiss keyboard on log filter bar and search dialog close
- Handle state during rotation
dara-abijo-adfa
marked this pull request as ready for review
July 28, 2026 18:57
![claude[bot]](https://cdn.github.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
📝 Walkthrough
WalkthroughThe PR separates source and filtered emptiness, updates output rendering and bottom-sheet actions accordingly, adds no-match feedback for filtering and search, dismisses soft keyboards when controls close, and adds coverage for the new state transitions. ChangesOutput and search feedback
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant OutputFragment
participant EmptyStateFragment
participant EditorBottomSheet
OutputFragment->>EmptyStateFragment: updateEmptyState(sourceEmpty, filterActive)
EmptyStateFragment-->>EditorBottomSheet: emit source-empty state
EditorBottomSheet->>EditorBottomSheet: update output actions
sequenceDiagram
participant EditorSearchLayout
participant IDEEditorSearcher
participant flashInfo
EditorSearchLayout->>IDEEditorSearcher: navigate to next or previous match
IDEEditorSearcher-->>EditorSearchLayout: return no match
EditorSearchLayout->>IDEEditorSearcher: check completed search state
EditorSearchLayout->>flashInfo: show no-search-matches message
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
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 (2)
app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt (1)
148-155: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
clearOutputbypasses theisEmptycache contract.
emptyStateViewModel.setEmpty(true)is called directly instead of throughEmptyStateFragment.isEmpty's setter (orupdateEmptyState). Only that setter also updatescachedIsEmpty; calling the ViewModel directly leaves the cache stale. If the fragment is later detached/reattached (e.g. on rotation),onViewCreated's resync logic will overwrite the correctly-cleared empty state with the stale cached value — undermining the PR's rotation-correctness goal.🐛 Proposed fix
override fun clearOutput() { wasLastFilteredResultEmpty = false hasInitialContentRendered = false viewModel.clear() _binding?.editor?.setText("")?.also { - emptyStateViewModel.setEmpty(true) + isEmpty = true } }🤖 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 `@app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt` around lines 148 - 155, Update LogViewFragment.clearOutput to set the empty state through the EmptyStateFragment.isEmpty setter or its updateEmptyState helper, rather than calling emptyStateViewModel.setEmpty directly. Preserve the existing clear/reset behavior while ensuring cachedIsEmpty is updated for correct state restoration after detachment and reattachment.app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt (1)
316-333: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLive-append paths never clear
wasLastFilteredResultEmpty, so a later real "no matches" event can be silently suppressed.Both fragments duplicate the same "last filtered result empty" flash-info tracking (in
renderFiltered/setText), and both independently miss updating it from their live-append path. If a "no matches" flash already fired (wasLastFilteredResultEmpty = true), then a live line matching the current filter arrives viaappendBatch, the flag should reset tofalsesince the filtered result is no longer empty — otherwise a subsequent genuine empty-transition won't re-flash the notice.
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt#L316-L333: aftereditor?.appendBatch(visibleText)succeeds (both the immediate and deferred paths), also setwasLastFilteredResultEmpty = false.app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt#L285-L295: aftereditor.appendBatch(chars.toString()), also setwasLastFilteredResultEmpty = false.Longer term, consider extracting this duplicated tracking (the two flags + flash-info gating) into a shared helper so this edge case isn't independently reintroduced.
🤖 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 `@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt` around lines 316 - 333, Reset wasLastFilteredResultEmpty to false whenever live filtered output is appended. In BuildOutputFragment’s appendBatch flow, update the flag after appendBatch succeeds in both immediate and deferred paths; in LogViewFragment’s corresponding appendBatch(chars.toString()) path, make the same update. Do not change the existing empty-result flash gating.
🧹 Nitpick comments (1)
editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt (1)
84-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd KDoc for the public no-match contract.
Document that
truemeans a valid completed search has zero matches, whilefalsealso covers missing, invalid, or uninspectable result state.Suggested KDoc
+/** + * Reports whether the current valid search completed with no matches. + * + * Returns false when the query/results are unavailable or cannot be inspected. + */ fun isSearchCompleteWithNoMatches(): Boolean {🤖 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 `@editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt` at line 84, In the public isSearchCompleteWithNoMatches() method, add KDoc defining that true indicates a valid completed search with zero matches, while false includes missing, invalid, or uninspectable result state.Source: Coding guidelines
🤖 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 `@app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt`:
- Around line 691-732: The observeCurrentFragmentEmptyState and
updateActionButtonsEnabledState flow incorrectly uses EmptyStateFragment.isEmpty
for action-button gating, because that value is overridden by filter UI state.
Expose and collect a separate source/content-empty signal from the relevant
EmptyStateFragment implementations, including LogViewFragment and
BuildOutputFragment, and pass that signal to updateActionButtonsEnabledState
while preserving isEmpty for ViewFlipper behavior.
In `@editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt`:
- Around line 84-122: Update isSearchCompleteWithNoMatches() to use a supported
search-result API instead of probing hasMatch and lastResults through
reflection; if compatibility reflection is unavoidable, isolate it and return an
explicit unknown state that EditorSearchLayout can handle rather than mapping
failures to false. Document the public method and replace broad Throwable
catches with narrowly scoped reflection/error handling that does not swallow
fatal VM errors.
---
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`:
- Around line 316-333: Reset wasLastFilteredResultEmpty to false whenever live
filtered output is appended. In BuildOutputFragment’s appendBatch flow, update
the flag after appendBatch succeeds in both immediate and deferred paths; in
LogViewFragment’s corresponding appendBatch(chars.toString()) path, make the
same update. Do not change the existing empty-result flash gating.
In `@app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt`:
- Around line 148-155: Update LogViewFragment.clearOutput to set the empty state
through the EmptyStateFragment.isEmpty setter or its updateEmptyState helper,
rather than calling emptyStateViewModel.setEmpty directly. Preserve the existing
clear/reset behavior while ensuring cachedIsEmpty is updated for correct state
restoration after detachment and reattachment.
---
Nitpick comments:
In `@editor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt`:
- Line 84: In the public isSearchCompleteWithNoMatches() method, add KDoc
defining that true indicates a valid completed search with zero matches, while
false includes missing, invalid, or uninspectable result 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 53ddc2a8-5752-497a-9dc0-18a05048c631
📒 Files selected for processing (10)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.ktapp/src/main/java/com/itsaky/androidide/logs/LogBuffer.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/java/com/itsaky/androidide/viewmodel/LogViewModel.kteditor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kteditor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.ktresources/src/main/res/values/strings.xml
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 (1)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt (1)
176-181: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winMove restored-output filtering off the main thread.
Line 179 runs
filterLineson the lifecycle main dispatcher. Large build output can stall restoration; mirrorrenderFilteredand useDispatchers.Default.Proposed fix
- val content = BuildOutputViewModel.filterLines(window, query) + val content = + withContext(Dispatchers.Default) { + BuildOutputViewModel.filterLines(window, query) + }As per coding guidelines, "Move disk, network, database, large parsing, decoding, serialization, cryptography, and image/APK processing off the main thread."
🤖 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 `@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt` around lines 176 - 181, Update restoreWindowFromViewModel so the BuildOutputViewModel.filterLines call runs inside withContext(Dispatchers.Default), matching renderFiltered, while preserving the existing window, query, and filtering behavior.Source: Coding guidelines
🤖 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
`@app/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.kt`:
- Around line 82-97: add KDoc for the public isVisible property, toggle(), and
hide() in LogFilterBarController, documenting their visibility behavior;
explicitly note that hide() dismisses the filter input IME and invokes
onVisibilityChanged.
In `@app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt`:
- Around line 148-152: Update the repeatOnLifecycle collection around the
restored snapshot handling near SetText and the noMatchTracker usage so each new
collection primes the tracker with its initial snapshot without invoking
onRender. Preserve onRender for subsequent snapshots, preventing no-match
feedback from flashing after resume or rotation.
In
`@app/src/test/java/com/itsaky/androidide/fragments/output/FilterNoMatchTrackerTest.kt`:
- Around line 20-22: Update FilterNoMatchTrackerTest.kt (20-22) to use JUnit
Jupiter’s test annotation and Truth assertions throughout the new tracker suite.
In LogBufferTest.kt (23, 98-108) and LogViewModelTest.kt (30, 184-194), replace
JUnit 4 assertion imports and new emptiness assertions with Truth APIs while
retaining JUnit Jupiter test execution. Apply only these test API migrations and
preserve existing test behavior.
---
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt`:
- Around line 176-181: Update restoreWindowFromViewModel so the
BuildOutputViewModel.filterLines call runs inside
withContext(Dispatchers.Default), matching renderFiltered, while preserving the
existing window, query, and filtering behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c5b4fcd5-6706-4b66-ba08-d31cc756df34
📒 Files selected for processing (11)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/output/FilterNoMatchTracker.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogFilterBarController.ktapp/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/test/java/com/itsaky/androidide/fragments/output/FilterNoMatchTrackerTest.ktapp/src/test/java/com/itsaky/androidide/logs/LogBufferTest.ktapp/src/test/java/com/itsaky/androidide/viewmodel/LogViewModelTest.ktcommon/src/main/java/com/itsaky/androidide/utils/KeyboardUtils.kteditor/src/main/java/com/itsaky/androidide/editor/ui/EditorSearchLayout.kteditor/src/main/java/io/github/rosemoe/sora/widget/IDEEditorSearcher.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt
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
`@app/src/main/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModel.kt`:
- Around line 49-54: Add KDoc to the public setEmptyState method describing that
isEmpty controls the displayed overall empty state, isSourceEmpty controls
whether the underlying source has no items, and divergent values are valid when
the source is empty but other content still makes the overall state non-empty.
In
`@app/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt`:
- Around line 20-53: Update EmptyStateFragmentViewModelTest to use JUnit
Jupiter’s `@Test` and Truth’s assertThat assertions instead of JUnit 4 imports and
Assert methods. Replace every assertTrue/assertFalse call in both states start
empty, setEmpty sets both states together, and setEmptyState sets the states
independently with equivalent isTrue()/isFalse() assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 03b4fdb0-f483-41e7-bf90-8ec12997dae7
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModel.ktapp/src/test/java/com/itsaky/androidide/viewmodel/EmptyStateFragmentViewModelTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt
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.