SCRUM-265 design: add note search screen#32
Open
gdaegeun539 wants to merge 5 commits intoproject-lyrics:developfrom
Open
SCRUM-265 design: add note search screen#32gdaegeun539 wants to merge 5 commits intoproject-lyrics:developfrom
gdaegeun539 wants to merge 5 commits intoproject-lyrics:developfrom
Conversation
- remove top, bottom line MusicComponent at .SearchNoteByMusic
- title not to break the trailing
1 task
Member
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe changes refactor the note search feature by relocating Changes
Sequence DiagramsequenceDiagram
participant User
participant Screen as NoteSearchScreen
participant ViewModel as NoteSearchViewModel
participant StateFlow as StateFlow
participant LazyColumn as LazyColumn
User->>Screen: Trigger initial composition
Screen->>ViewModel: loadInitialNotes()
ViewModel->>ViewModel: Load dummy dataset, emit LOADING state
ViewModel->>StateFlow: Update to SUCCESS_LOAD with results
StateFlow-->>Screen: Emit updated viewState
Screen->>LazyColumn: Render MusicComponent items
User->>Screen: Type search keyword
Screen->>Screen: Debounce input (1000ms, distinctUntilChanged)
Screen->>ViewModel: searchNotes(keyword)
ViewModel->>ViewModel: Filter results, emit LOADING state
ViewModel->>StateFlow: Update to SUCCESS_LOAD with filtered results
StateFlow-->>Screen: Emit updated viewState
Screen->>LazyColumn: Re-render filtered items
User->>LazyColumn: Scroll near end (prefetch threshold)
LazyColumn->>Screen: Trigger onLoadNextPage
Screen->>ViewModel: loadNextPage()
ViewModel->>ViewModel: Emit NEW_PAGE_LOADING, append next page
ViewModel->>StateFlow: Update results + hasNextPage
StateFlow-->>Screen: Emit updated viewState
Screen->>LazyColumn: Append new items to list
User->>Screen: Pull to refresh
Screen->>ViewModel: refresh()
ViewModel->>ViewModel: Reset and reload initial data
StateFlow-->>Screen: Emit refreshed viewState
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 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/lyrics/feelin/presentation/view/note/search/NoteSearchScreen.kt`:
- Around line 123-126: The FeelinTopAppBarWithBack in NoteSearchScreen currently
has a no-op onBackClick; either remove/hide the back affordance when this screen
is a main-tab root or propagate a real navigation handler: modify
NoteSearchScreen to accept a navigation callback (e.g., onBack or navController)
and pass it into FeelinTopAppBarWithBack's onBackClick, or conditionally set
showBack=false when this destination is the tab root; update the
NoteSearchScreen composable signature and call sites accordingly and ensure
FeelinTopAppBarWithBack receives the actual onBackClick lambda or is rendered
without the back button.
- Around line 91-99: The function NoteSearchScreenContent currently requires a
non-null onMusicClick causing callers to pass an empty lambda and MusicComponent
to render as interactive; change the parameter signature onMusicClick:
(MusicComponentData.SearchNoteByMusic) -> Unit to nullable onMusicClick:
((MusicComponentData.SearchNoteByMusic) -> Unit)? (keep the name), update any
invocation sites inside NoteSearchScreenContent to only call
onMusicClick?.invoke(...) (or pass it directly to MusicComponent so it receives
null), and update callers to pass null instead of {} so MusicComponent sees a
null onClick and renders non-interactive.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eb38f0ca-9d6f-478b-bcf2-0c768a87e82c
📒 Files selected for processing (6)
app/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/music/MusicComponent.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/NoteSearchScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/search/NoteSearchScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/search/NoteSearchViewModel.ktapp/src/main/java/com/lyrics/feelin/presentation/view/note/search/NoteSearchViewState.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/com/lyrics/feelin/presentation/view/note/NoteSearchScreen.kt
Comment on lines
+123
to
+126
| FeelinTopAppBarWithBack( | ||
| title = "노트 검색", | ||
| showDivider = false, | ||
| onBackClick = { /* TODO: 이전화면 라우팅*/ } |
There was a problem hiding this comment.
Wire the back button or remove it for now.
The UI renders a back affordance, but the current handler is a no-op TODO. If this destination is a main-tab root, hide the back arrow; otherwise pass a real onBackClick from navigation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/note/search/NoteSearchScreen.kt`
around lines 123 - 126, The FeelinTopAppBarWithBack in NoteSearchScreen
currently has a no-op onBackClick; either remove/hide the back affordance when
this screen is a main-tab root or propagate a real navigation handler: modify
NoteSearchScreen to accept a navigation callback (e.g., onBack or navController)
and pass it into FeelinTopAppBarWithBack's onBackClick, or conditionally set
showBack=false when this destination is the tab root; update the
NoteSearchScreen composable signature and call sites accordingly and ensure
FeelinTopAppBarWithBack receives the actual onBackClick lambda or is rendered
without the back button.
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.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
What is the current behavior?
곡을 통해 노트를 검색하는 화면이 구현되지 않은 상태입니다.
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No breaking changes
ScreenShots (If needed)
Screen_Recording_20260410_001331_Feelin.mp4
Summary by CodeRabbit
Release Notes