Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReplace per-adapter sort with callbacks, add three notification-aware ItemSorts and comparison helpers, expose view-holder sort preference, change reminders default to UPCOMING (chip swap), and restrict DAO reminders query to Changes
Sequence Diagram(s)sequenceDiagram
participant Fragment as RemindersFragment / NotallyFragment
participant Adapter as BaseNoteAdapter
participant SortImpl as ItemSort (NEXT / LAST / MOST_RECENT)
participant SortedList as SortedList
Fragment->>Adapter: setNotesSortCallback(callbackForFilter)
Fragment->>Adapter: set data / applyFilter()
Adapter->>Adapter: notesSortCallback(this) rgba(100,150,240,0.5)
Adapter->>SortImpl: instantiate sort (adapter, direction)
Adapter->>SortedList: replaceAll / resync using SortImpl comparator
SortedList-->>Adapter: ordered items
Adapter-->>Fragment: bound views reflect sorted order
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt (1)
313-315:⚠️ Potential issue | 🔴 CriticalPreference changes silently override fragment-specific sort behavior.
When a user changes the sort preference (lines 313-315),
setNotesSort()unconditionally replaces the adapter's sort callback viasetNotesSortCallback(), overwriting any custom callback provided by subclasses likeRemindersFragment. WhileRemindersFragmentcan restore its filter-aware sorting viaapplyFilter()andsetNotesSortCallback(), this only occurs on explicit filter UI interaction—if the user changes the preference without using the filter, the custom sort is silently lost until they next interact with the filter controls.To prevent regression, either:
- Expose a way to check if a custom callback is active, and skip the preference update in that case
- Have subclasses re-establish their callbacks after a preference change
- Introduce a priority system (e.g., prefer fragment-specific callbacks over preference-driven ones)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt` around lines 313 - 315, The observer on model.preferences.notesSorting should not unconditionally overwrite fragment-specific sorting callbacks; change the logic in the notesSorting observer (the block that calls notesAdapter?.setNotesSort(notesSort)) to first check whether the adapter currently has a fragment-provided/custom sort callback (e.g., add and use a new NotesAdapter method hasCustomSortCallback() or isUsingFragmentCallback()), and only call setNotesSort(notesSort) when no custom callback is active; alternatively, after applying the preference change invoke a fragment hook (e.g., RemindersFragment.applyFilter() or a new restoreFragmentSortCallback() method) so subclasses can re-establish their filter-aware callback via setNotesSortCallback().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt`:
- Around line 167-173: The mapping in NotesSort.createCallback incorrectly
assigns NotesSortBy.MODIFIED_DATE to BaseNoteNextNotificationSort (sorting by
reminder) instead of the intended BaseNoteModifiedDateSort; update the when
branch for NotesSortBy.MODIFIED_DATE to construct and return
BaseNoteModifiedDateSort(adapter, sortDirection) so modification-date sorting
uses the correct comparator while keeping the adapter and sortDirection
parameters intact.
---
Outside diff comments:
In
`@app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt`:
- Around line 313-315: The observer on model.preferences.notesSorting should not
unconditionally overwrite fragment-specific sorting callbacks; change the logic
in the notesSorting observer (the block that calls
notesAdapter?.setNotesSort(notesSort)) to first check whether the adapter
currently has a fragment-provided/custom sort callback (e.g., add and use a new
NotesAdapter method hasCustomSortCallback() or isUsingFragmentCallback()), and
only call setNotesSort(notesSort) when no custom callback is active;
alternatively, after applying the preference change invoke a fragment hook
(e.g., RemindersFragment.applyFilter() or a new restoreFragmentSortCallback()
method) so subclasses can re-establish their filter-aware callback via
setNotesSortCallback().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 594e3058-1f78-4b37-98f4-2600660f3b94
📒 Files selected for processing (10)
app/src/main/java/com/philkes/notallyx/data/dao/BaseNoteDao.ktapp/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.ktapp/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/RemindersFragment.ktapp/src/main/java/com/philkes/notallyx/presentation/activity/note/PickNoteActivity.ktapp/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.ktapp/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteVH.ktapp/src/main/java/com/philkes/notallyx/presentation/view/main/sorting/BaseNoteLastNotificationSort.ktapp/src/main/java/com/philkes/notallyx/presentation/view/main/sorting/BaseNoteMostRecentNotificationSort.ktapp/src/main/java/com/philkes/notallyx/presentation/view/main/sorting/BaseNoteNextNotificationSort.ktapp/src/main/res/layout/fragment_notes.xml
app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt
Show resolved
Hide resolved
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.
Closes #913
UpcomingFilter is set in Reminders ViewUpcoming,Elapsed,AllUpcoming: Sorted by the next notification (ascending)Elapsed: Sorted by last notification (descending)All: Sorted by next notification (descending) for upcoming reminders, and by last notification (descending) for elapsed remindersSummary by CodeRabbit
New Features
Bug Fixes
UI