Highlight keyword in notes search and jump to keyword in note #716
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughPropagates search keywords from fragment into adapter/viewholders and EditActivity, implements keyword-aware snippet extraction and highlighting UI, swaps text views for highlightable variants in layouts, and tweaks edit-text focus/back-key handling and related imports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Fragment as NotallyFragment
participant Adapter as BaseNoteAdapter
participant VH as BaseNoteVH
participant EditAct as EditActivity
participant HTV as HighlightableTextView
User->>Fragment: Type search query / change text
Fragment->>Adapter: setSearchKeyword(query)
Adapter->>VH: bind() -> setSearchKeyword(query)
VH->>HTV: extract/show snippet & apply highlight
User->>Fragment: Open item from search
Fragment->>EditAct: startActivity(intent + EXTRA_INITIAL_SEARCH_QUERY)
EditAct->>EditAct: onStart() reads EXTRA_INITIAL_SEARCH_QUERY -> populate search -> trigger search
EditAct->>HTV: highlight initial query in opened note
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
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}
There was a problem hiding this comment.
Actionable comments posted: 4
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/view/main/BaseNoteVH.kt (1)
250-282: Reset marginStart for non-child items to prevent recycling issues.Similar to the issue in
bindListSearch, the marginStart is set for child items but never explicitly reset for non-child items, causing recycled views to retain incorrect margins.Apply this diff to fix the margin handling:
view.apply { text = item.body handleChecked(this, item.checked) visibility = VISIBLE - if (item.isChild) { - updateLayoutParams<LinearLayout.LayoutParams> { - marginStart = 20.dp - } - } + updateLayoutParams<LinearLayout.LayoutParams> { + marginStart = if (item.isChild) 20.dp else 0 + } if (index == filteredList.lastIndex) { updatePadding(bottom = 0) } }
🧹 Nitpick comments (5)
app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt (1)
59-64: Consider usingnotifyItemRangeChanged()for better performance.The current implementation uses
notifyDataSetChanged()which rebinds all visible items. For large note lists, this could cause noticeable UI lag during search. Consider usingnotifyItemRangeChanged(0, itemCount)instead, which provides the same full refresh but with better animation support and slightly better performance.Apply this diff for the optimization:
fun setSearchKeyword(keyword: String) { if (searchKeyword != keyword) { searchKeyword = keyword - notifyDataSetChanged() + notifyItemRangeChanged(0, itemCount) } }app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt (1)
220-222: Consider documenting the hardcoded delay.The 10ms delay in
postOnAnimationDelayedappears arbitrary. Either add a comment explaining why this specific delay is needed, or consider removing the delay ifpostis sufficient.If the delay is necessary for layout completion:
[email protected]?.MainListView?.apply { - postOnAnimationDelayed({ scrollToPosition(0) }, 10) + // Delay ensures adapter has finished updating before scroll + postOnAnimationDelayed({ scrollToPosition(0) }, 10) }Or if the delay is unnecessary:
[email protected]?.MainListView?.apply { - postOnAnimationDelayed({ scrollToPosition(0) }, 10) + post { scrollToPosition(0) } }app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableTextView.kt (3)
31-36: Add bounds validation for safer span operations.The method doesn't validate that
startIdxandendIdxare within bounds or thatstartIdx <= endIdx. While the current callers may guarantee valid indices, defensive programming would prevent potential crashes.Apply this diff to add validation:
fun highlight(startIdx: Int, endIdx: Int) { + if (startIdx < 0 || endIdx > text.length || startIdx > endIdx) return this.text = SpannableString(this.text.toString()).apply { setSpan(HighlightSpan(highlightColor), startIdx, endIdx, SPAN_EXCLUSIVE_EXCLUSIVE) } }
38-53: Consider preserving or making maxLines configurable.Setting
maxLines = Int.MAX_VALUEunconditionally may interfere with the view's originalmaxLinessetting. This could lead to unexpected layout behavior if the view was configured with specific line limits.Consider one of these approaches:
- Document that this method overrides maxLines
- Store and restore the original maxLines value
- Make maxLines a parameter with Int.MAX_VALUE as default
76-89: Consider removing or completing the ellipsizeMultilineTextToFit function.This function contains commented-out code (lines 79-83) and doesn't appear to be called anywhere in the provided context. If it's not part of the current implementation, consider removing it to reduce code clutter. If it's intended for future use, complete the implementation and document its purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt(5 hunks)app/src/main/java/com/philkes/notallyx/presentation/activity/note/EditActivity.kt(2 hunks)app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt(2 hunks)app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteVH.kt(6 hunks)app/src/main/java/com/philkes/notallyx/presentation/view/misc/EditTextAutoClearFocus.kt(1 hunks)app/src/main/java/com/philkes/notallyx/presentation/view/misc/StylableEditTextWithHistory.kt(1 hunks)app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightSpan.kt(1 hunks)app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableEditText.kt(1 hunks)app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableTextView.kt(1 hunks)app/src/main/java/com/philkes/notallyx/utils/changehistory/EditTextWithHistoryChange.kt(1 hunks)app/src/main/res/layout/recycler_base_note.xml(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteVH.kt (2)
app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableTextView.kt (4)
extractSearchSnippet(61-74)showSearchSnippet(38-53)highlight(26-29)highlight(31-36)app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableEditText.kt (1)
highlight(62-83)
app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt (1)
app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteVH.kt (2)
setSearchKeyword(63-65)bind(102-163)
app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableTextView.kt (2)
app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableEditText.kt (1)
highlight(62-83)app/src/main/java/com/philkes/notallyx/presentation/UiExtensions.kt (1)
setSpan(198-202)
app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt (3)
app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt (1)
setSearchKeyword(59-64)app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteVH.kt (1)
setSearchKeyword(63-65)app/src/main/java/com/philkes/notallyx/presentation/view/misc/EditTextWithWatcher.kt (2)
setText(29-31)setText(33-35)
🔇 Additional comments (22)
app/src/main/java/com/philkes/notallyx/presentation/view/misc/EditTextAutoClearFocus.kt (1)
6-6: LGTM! Import updated for package refactor.The import correctly reflects the reorganization of
HighlightableEditTextinto thehighlightableviewsubpackage as part of the PR-wide refactoring effort.app/src/main/java/com/philkes/notallyx/utils/changehistory/EditTextWithHistoryChange.kt (1)
7-7: LGTM!The import path update correctly reflects the refactored package structure for highlight-related components.
app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightSpan.kt (1)
1-6: LGTM!Clean extraction of
HighlightSpaninto a dedicated file enables better reuse across the highlightable view components.app/src/main/java/com/philkes/notallyx/presentation/activity/note/EditActivity.kt (1)
1259-1259: LGTM!The constant is properly defined and follows Android intent extra naming conventions.
app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableEditText.kt (1)
1-10: LGTM!The package restructuring and import updates correctly reflect the extraction of
HighlightSpanto a separate file. No functional changes to the existing highlight logic.app/src/main/java/com/philkes/notallyx/presentation/view/misc/StylableEditTextWithHistory.kt (1)
27-27: LGTM!Import path correctly updated to reflect the package restructuring.
app/src/main/res/layout/recycler_base_note.xml (3)
83-92: LGTM!The replacement of
TextViewwithHighlightableTextViewfor the title enables keyword highlighting while preserving all existing attributes and behavior.
154-164: LGTM!The note body now supports keyword highlighting with the
HighlightableTextViewreplacement.
175-232: LGTM!Consistent replacement of
TextViewwithHighlightableTextViewacross all list item views enables keyword highlighting in list note previews.app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteAdapter.kt (2)
32-32: LGTM!The
searchKeywordfield is properly initialized and scoped as private.
50-57: LGTM!The binding logic correctly propagates the search keyword to each view holder before binding the note data.
app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt (4)
30-30: LGTM!The import addition enables passing the initial search query to
EditActivity.
191-191: LGTM!Correctly sets the adapter's search keyword when entering the Search destination to highlight matches in the list.
200-200: LGTM!Properly clears the search keyword when leaving the Search destination to restore normal display.
309-314: LGTM!Correctly passes the search keyword to
EditActivitywhen launching from the Search destination, enabling immediate keyword highlighting in the opened note.app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteVH.kt (4)
61-65: LGTM!The searchKeyword property and setter follow standard ViewHolder patterns for adapter-driven state updates.
127-132: LGTM!The title snippet extraction and highlighting logic is correct, with appropriate fallback to the full title when no snippet is found.
165-179: LGTM!The keyword-aware note binding correctly handles both search and non-search scenarios with appropriate snippet extraction and fallback behavior.
241-248: LGTM!The remaining items indicator logic correctly handles visibility and displays the appropriate count.
app/src/main/java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableTextView.kt (3)
12-15: LGTM!The snippet sizing constants are well-defined and appropriately scoped.
61-74: LGTM!The snippet extraction logic correctly handles keyword matching, boundary calculations, and edge cases including missing keywords and short text.
92-98: LGTM!The Snippet data class is well-structured with clear properties for representing search snippets.
app/src/main/java/com/philkes/notallyx/presentation/activity/main/fragment/NotallyFragment.kt
Show resolved
Hide resolved
app/src/main/java/com/philkes/notallyx/presentation/activity/note/EditActivity.kt
Show resolved
Hide resolved
app/src/main/java/com/philkes/notallyx/presentation/view/main/BaseNoteVH.kt
Show resolved
Hide resolved
.../java/com/philkes/notallyx/presentation/view/misc/highlightableview/HighlightableTextView.kt
Show resolved
Hide resolved
Crustack
force-pushed
the
feature/search-improvments
branch
from
5bc70b1 to
3453a3c
Compare
Crustack
force-pushed
the
feature/search-improvments
branch
from
3453a3c to
e9172bd
Compare
Closes #158
This PR improves the search note functionality.
For Text Note:
notallyx_issues_158_notes.webm
For List Note:
notallyx_issues_158_lists.webm
Summary by CodeRabbit
New Features
Improvements
UI