ADFA-4386: Show active filter chips and indicator dot on Recent Projects#1427
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughRelease Notes: Show Active Filter Chips and Indicator Dot on Recent Projects
|
| Layer / File(s) | Summary |
|---|---|
Filter state model and synchronization app/src/main/java/com/itsaky/androidide/viewmodel/RecentProjectsViewModel.kt |
Introduces FilterState, exposes filterState: StateFlow<FilterState>, updates applyFilters() to publish current filter inputs, changes active-filter detection, and adds clearSort() to reset sort state. |
Filter bar layout and chip resources app/src/main/res/layout/layout_project_filters_bar.xml, app/src/main/res/layout/chip_active_filter.xml, resources/src/main/res/drawable/bg_filter_active_dot.xml, resources/src/main/res/values/strings.xml |
Restructures the filter bar layout, adds the active-filters dot and scrollable chip container, and introduces the chip, drawable, and string resources used by the active filter UI. |
Fragment chip rendering and dismissal app/src/main/java/com/itsaky/androidide/fragments/RecentProjectsFragment.kt |
Collects filterState, builds removable sort/search chips, animates chip updates, stores the filters dialog for dismiss synchronization, and refactors sort label mapping into labelRes(). |
Sequence Diagram(s)
sequenceDiagram
participant RecentProjectsFragment
participant RecentProjectsViewModel
participant ChipGroup
participant TransitionManager
RecentProjectsFragment->>RecentProjectsViewModel: collect filterState
RecentProjectsViewModel-->>RecentProjectsFragment: FilterState
RecentProjectsFragment->>TransitionManager: beginDelayedTransition(AutoTransition)
RecentProjectsFragment->>ChipGroup: clear and rebuild chips
RecentProjectsFragment->>ChipGroup: add sort chip / search chip
RecentProjectsFragment->>RecentProjectsViewModel: clearSort() or onSearchQuery("")
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- appdevforall/CodeOnTheGo#700: Shares the recent-projects filtering path and
applyFilters()/sort-query state management that this PR builds on.
Suggested reviewers
- itsaky-adfa
- dara-abijo-adfa
- jatezzz
- hal-eisen-adfa
Poem
🐇 A chip for the sort, a chip for the search,
Tiny dots now gleam on their perch.
The filter bar scrolls, neat and bright,
With close-icon taps that feel just right.
Hop, hop—filters dance in view ✨
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | ❓ Inconclusive | No pull request description was provided, so there is nothing meaningful to evaluate beyond the code changes. | Add a short description of the feature and its UI/state changes so reviewers can understand the intent quickly. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately summarizes the main change: active filter chips and the indicator dot on Recent Projects. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
ADFA-4386
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 help to get the list of available commands.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/res/drawable/bg_filter_active_dot.xml (1)
1-11: ⚡ Quick winMove this drawable to the shared
resourcesmoduleThis new drawable is in
app/src/main/res/drawable/, but project convention keeps shared drawables inresources/src/main/res/drawable/to avoid duplication and keep assets centralized.Based on learnings, drawable resources in this project should live under
resources/src/main/res/drawable/rather than the app module drawable folder.🤖 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/res/drawable/bg_filter_active_dot.xml` around lines 1 - 11, The drawable file bg_filter_active_dot.xml is currently located in the app module's drawable directory (app/src/main/res/drawable/) but it should be moved to the shared resources module's drawable directory (resources/src/main/res/drawable/) to follow project convention and avoid duplication. Move the entire bg_filter_active_dot.xml file from app/src/main/res/drawable/ to resources/src/main/res/drawable/ to keep shared drawable assets centralized.Source: Learnings
🤖 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/RecentProjectsViewModel.kt`:
- Line 42: The hasAny property in FilterState does not account for the
ascending/descending sort order state. Update the boolean expression in the
hasAny getter to include a check for when ascending is false, in addition to the
existing checks for sort != null and query.isNotEmpty(). This will ensure that
descending-only filters are properly recognized as active and consistently
reflected in the filter indicator state.
---
Nitpick comments:
In `@app/src/main/res/drawable/bg_filter_active_dot.xml`:
- Around line 1-11: The drawable file bg_filter_active_dot.xml is currently
located in the app module's drawable directory (app/src/main/res/drawable/) but
it should be moved to the shared resources module's drawable directory
(resources/src/main/res/drawable/) to follow project convention and avoid
duplication. Move the entire bg_filter_active_dot.xml file from
app/src/main/res/drawable/ to resources/src/main/res/drawable/ to keep shared
drawable assets centralized.
🪄 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
Run ID: d6183531-b48f-49d4-8a80-ae189c449b4c
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/fragments/RecentProjectsFragment.ktapp/src/main/java/com/itsaky/androidide/viewmodel/RecentProjectsViewModel.ktapp/src/main/res/drawable/bg_filter_active_dot.xmlapp/src/main/res/layout/chip_active_filter.xmlapp/src/main/res/layout/layout_project_filters_bar.xmlresources/src/main/res/values/strings.xml
…ilter dot drawable to resources module
hal-eisen-adfa
left a comment
Collaborator
There was a problem hiding this comment.
Code review for the active-filter chips + indicator dot. Findings posted inline, most-impactful first. The headline is the dot-vs-chip truth-source split (descending-only lights the dot with no clearable chip — the exact ticket gap); the rest are leak/double-work and reuse cleanups.
jatezzz
approved these changes
Jun 22, 2026
…leak & double filter passes - Make sort direction subordinate to a sort criteria: hasAny = sort != null || query.isNotEmpty() and applyFilters() only reverses when a criteria is set, so a descending-only state no longer lights the indicator dot with no clearable chip. hasActiveFilters now delegates to filterState.value.hasAny so the dot and the sheet's Clear button share one definition of "active". - Collect filterEvents once for the view lifetime and dismiss a single filtersDialog ref instead of launching a per-open collector on every sheet open. - Sort chip removal: add clearSort() (one applyFilters pass) - no dot flicker. - Search chip removal: clear the EditText only and let the debounced watcher drive the VM - one filter pass. Search chip body now focuses the search field instead of opening the unrelated sort sheet. - Extract SortCriteria.labelRes() shared by setupSortUI and renderActiveFilters. - Announce active state on the filter button's contentDescription and mark the decorative dot importantForAccessibility=no for TalkBack. - Run beginDelayedTransition before chip mutations so chip enter/exit animates.
Contributor
There was a problem hiding this comment.
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/viewmodel/RecentProjectsViewModel.kt (1)
97-117: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
applyFilters()mixes mutable shared state across coroutine boundaries
applyFilters()publishes_filterStatefirst, then readscurrentQuery/currentSort/isAscendingduring background computation. Concurrent calls can produce a filtered list that does not match the emittedfilterState(or overwrite newer results with older computation).Use a single immutable snapshot for both emission and filtering/sorting, and compute from that snapshot only.
Suggested fix
private suspend fun applyFilters() { - _filterState.value = FilterState(currentQuery, currentSort, isAscending) + val snapshot = FilterState(currentQuery, currentSort, isAscending) + _filterState.value = snapshot withContext(Dispatchers.Default) { var result = allProjects - if (currentQuery.isNotEmpty()) { - result = result.filter { it.name.contains(currentQuery, ignoreCase = true) } + if (snapshot.query.isNotEmpty()) { + result = result.filter { it.name.contains(snapshot.query, ignoreCase = true) } } - val criteria = currentSort + val criteria = snapshot.sort if (criteria != null) { result = when (criteria) { SortCriteria.NAME -> result.sortedBy { it.name.lowercase() } SortCriteria.DATE_CREATED -> result.sortedBy { it.createdAt } SortCriteria.DATE_MODIFIED -> result.sortedBy { it.lastModified } } - if (!isAscending) { + if (!snapshot.ascending) { result = result.reversed() } } _projects.postValue(result) } }🤖 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/viewmodel/RecentProjectsViewModel.kt` around lines 97 - 117, In the `applyFilters()` method, the issue is that `_filterState` is published before the background computation begins, but the filtering logic reads the current values of `currentQuery`, `currentSort`, and `isAscending` from shared state. If `applyFilters()` is called again before the first coroutine completes, the emitted filter state will not match the actual filtered results due to the race condition. Create a local immutable snapshot of `currentQuery`, `currentSort`, and `isAscending` before the `FilterState` assignment, then use this snapshot both for emitting to `_filterState` and for the filtering and sorting logic within the `withContext` block to ensure consistency between the emitted state and the computed results.
♻️ Duplicate comments (1)
app/src/main/java/com/itsaky/androidide/viewmodel/RecentProjectsViewModel.kt (1)
42-42: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
hasAnyno longer reflects descending-only active sort directionAt Line 42,
hasAnydropped!ascending, and Line 67 now fully depends on this value. That can hide active state when direction is changed to descending before selecting a criterion, creating inconsistent filter-state signaling.Suggested fix
data class FilterState( val query: String = "", val sort: SortCriteria? = null, val ascending: Boolean = true ) { - val hasAny: Boolean get() = sort != null || query.isNotEmpty() + val hasAny: Boolean get() = sort != null || !ascending || query.isNotEmpty() }Also applies to: 67-67
🤖 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/viewmodel/RecentProjectsViewModel.kt` at line 42, The hasAny property in RecentProjectsViewModel is missing a check for the ascending flag that was previously included. Currently, it only checks if sort is not null or query is not empty, but it should also evaluate whether the sort direction is descending (when ascending is false). Add the !ascending condition to the hasAny property logic using a logical OR operator so that it properly reflects an active descending sort direction state, which is especially important since line 67 depends on this value to signal filter state correctly.
🤖 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.
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/viewmodel/RecentProjectsViewModel.kt`:
- Around line 97-117: In the `applyFilters()` method, the issue is that
`_filterState` is published before the background computation begins, but the
filtering logic reads the current values of `currentQuery`, `currentSort`, and
`isAscending` from shared state. If `applyFilters()` is called again before the
first coroutine completes, the emitted filter state will not match the actual
filtered results due to the race condition. Create a local immutable snapshot of
`currentQuery`, `currentSort`, and `isAscending` before the `FilterState`
assignment, then use this snapshot both for emitting to `_filterState` and for
the filtering and sorting logic within the `withContext` block to ensure
consistency between the emitted state and the computed results.
---
Duplicate comments:
In
`@app/src/main/java/com/itsaky/androidide/viewmodel/RecentProjectsViewModel.kt`:
- Line 42: The hasAny property in RecentProjectsViewModel is missing a check for
the ascending flag that was previously included. Currently, it only checks if
sort is not null or query is not empty, but it should also evaluate whether the
sort direction is descending (when ascending is false). Add the !ascending
condition to the hasAny property logic using a logical OR operator so that it
properly reflects an active descending sort direction state, which is especially
important since line 67 depends on this value to signal filter state correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19a52ca4-36f1-4e35-9c61-24b95fab907f
📒 Files selected for processing (3)
app/src/main/java/com/itsaky/androidide/fragments/RecentProjectsFragment.ktapp/src/main/java/com/itsaky/androidide/viewmodel/RecentProjectsViewModel.ktapp/src/main/res/layout/layout_project_filters_bar.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/res/layout/layout_project_filters_bar.xml
- app/src/main/java/com/itsaky/androidide/fragments/RecentProjectsFragment.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
4 participants
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.
No description provided.