SCRUM-259 design: add home layout - #61
Merged
gdaegeun539 merged 13 commits intoJun 24, 2026
Merged
Conversation
Implement the initial home screen layout, integrating banner, artist list, feed tabs, and empty state components. Set up HomeViewModel to manage UI states and actions like pull-to-refresh, toggling likes, and bookmarks. Add unit tests for ViewModel, UI state, and tab state logic.
Add a compatibility layer (`legacyMode`) that forces the UI to hide tabs/filters and render the "Artists" feed with the "Whole" filter. This allows seamless behavior toggling between the legacy artist feed and the new dynamic layout, verified with comprehensive tests.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughIntroduces a complete HomeScreen feature: a new ChangesNotification Icon Refactor
HomeScreen Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeScreen
participant HomeViewModel
participant NavController
User->>HomeScreen: compose/enter screen
HomeScreen->>HomeViewModel: loadHomeData()
HomeViewModel-->>HomeScreen: uiState(isInitialLoading=true)
HomeViewModel-->>HomeScreen: uiState(banner, artists, tabStates, isInitialLoading=false)
User->>HomeScreen: pull to refresh
HomeScreen->>HomeViewModel: refresh()
HomeViewModel-->>HomeScreen: uiState(isRefreshing=true)
HomeViewModel-->>HomeScreen: uiState(updated tabState, isRefreshing=false)
User->>HomeScreen: click artist bubble
HomeScreen->>NavController: navigate(ArtistRecord.createRoute(artistId))
User->>HomeScreen: click banner
HomeScreen->>NavController: navigate(InternalWebView(url))
User->>HomeScreen: click note
HomeScreen->>NavController: navigate(NoteDetail.createRoute(noteId))
User->>HomeScreen: like/bookmark note
HomeScreen->>HomeViewModel: toggleLike(noteId) / toggleBookmark(noteId)
HomeViewModel-->>HomeScreen: uiState(optimistic note update)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0011c70ba6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+77
to
+80
| FilterButton( | ||
| data = filter, | ||
| isSelect = currentTabState.selectedFilter?.id == filter.id, | ||
| onClick = { onFilterClick(filter) } |
There was a problem hiding this comment.
신규 모드에서 카테고리 필터처럼 id가 없는 항목들이 들어오면 null == null이 되어 모든 칩이 동시에 선택 상태로 그려집니다. 현재 mockFilters()도 전체, 인기, 해석공유 모두 id = null이라 legacyMode = false일 때 재현되며, 사용자는 어떤 필터가 실제로 선택됐는지 알 수 없습니다. FilterButtonData 자체 비교나 필터 타입별 stable key를 사용해 단일 칩만 선택되게 해주세요.
Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this 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: 6
🧹 Nitpick comments (3)
app/src/main/java/com/lyrics/feelin/presentation/view/home/component/HomeHeader.kt (1)
26-29: ⚡ Quick winAlign
HomeHeaderparameter order with the project’s Compose convention.Please place
modifieras the first optional parameter and keep lambda parameters at the end for this composable signature.Proposed refactor
fun HomeHeader( hasUnreadNotification: Boolean, - onNotificationClick: () -> Unit, modifier: Modifier = Modifier, + onNotificationClick: () -> Unit, ) {As per coding guidelines, in
app/src/main/java/com/lyrics/feelin/presentation/**/*.ktkeepmodifieras the first optional parameter and lambda parameters as the last parameter in composable functions.🤖 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/lyrics/feelin/presentation/view/home/component/HomeHeader.kt` around lines 26 - 29, The parameter order in the HomeHeader composable function does not follow the project's Compose convention. Reorder the parameters so that modifier is positioned as the first optional parameter after required parameters, and lambda parameters like onNotificationClick are placed at the end. In HomeHeader, move the modifier parameter before the onNotificationClick lambda parameter to align with the coding guidelines.Source: Coding guidelines
app/src/main/java/com/lyrics/feelin/presentation/view/home/component/BannerSection.kt (1)
20-23: ⚡ Quick winReorder
BannerSectionparameters to match Compose API conventions used in this project.Move
modifierbefore callback params and keep lambda params at the end.Proposed refactor
fun BannerSection( banner: Banner, - onBannerClick: (String) -> Unit, modifier: Modifier = Modifier, + onBannerClick: (String) -> Unit, ) {As per coding guidelines, in
app/src/main/java/com/lyrics/feelin/presentation/**/*.ktkeepmodifieras the first optional parameter and lambda parameters as the last parameter in composable functions.🤖 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/lyrics/feelin/presentation/view/home/component/BannerSection.kt` around lines 20 - 23, Reorder the parameters in the BannerSection function to follow Compose API conventions. Move the modifier parameter to come before the onBannerClick lambda parameter, ensuring that required parameters like banner come first, followed by the optional modifier parameter, and then all lambda/callback parameters at the end.Source: Coding guidelines
app/src/main/java/com/lyrics/feelin/presentation/view/home/component/ArtistRow.kt (1)
27-33: ⚡ Quick winMatch
ArtistRowsignature order to the project’s composable conventions.Please keep
modifieras the first optional parameter and callbacks after it.Proposed refactor
fun ArtistRow( artists: List<ArtistBubbleComponentData>, - onArtistClick: (Long) -> Unit, - onShowAllArtistsClick: () -> Unit, - onFindArtistsClick: () -> Unit, modifier: Modifier = Modifier, + onArtistClick: (Long) -> Unit, + onShowAllArtistsClick: () -> Unit, + onFindArtistsClick: () -> Unit, ) {As per coding guidelines, in
app/src/main/java/com/lyrics/feelin/presentation/**/*.ktkeepmodifieras the first optional parameter and lambda parameters as the last parameter in composable functions.🤖 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/lyrics/feelin/presentation/view/home/component/ArtistRow.kt` around lines 27 - 33, The ArtistRow composable function has parameters in the wrong order according to project conventions. Reorder the parameters so that modifier is the first optional parameter (after all required parameters like artists), and place all callback parameters (onArtistClick, onShowAllArtistsClick, onFindArtistsClick) after the modifier parameter. The correct order should be: artists (required), modifier with default value, then the three callback functions in sequence.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/lyrics/feelin/presentation/view/home/component/FeedSection.kt`:
- Around line 91-100: The forEach loop inside Column in the FeedSection is
eagerly composing all notes at once, which defeats the lazy virtualization
provided by the parent LazyColumn in HomeScreen, causing performance issues on
long feeds. Instead of rendering all NoteComponent items within the Column using
forEach, restructure the code so that the parent LazyColumn in HomeScreen
directly iterates over the notes and emits each note as a lazy item, allowing
Compose to properly virtualize and only render visible items. This requires
moving the note iteration logic from FeedSection's Column forEach to the parent
HomeScreen's LazyColumn items builder.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/home/component/HomeHeader.kt`:
- Around line 67-68: The backgroundColor parameter in the `@Preview` annotation is
using an incorrect hexadecimal color literal `0xFF00000` which is missing a
digit and does not represent opaque black. Update the backgroundColor value from
`0xFF00000` to `0xFF000000` to provide the correct opaque black color for the
preview background. This typo appears in two locations in the HomeHeader.kt
file, so ensure both instances of the backgroundColor parameter are corrected
with the proper 8-digit hex format.
In `@app/src/main/java/com/lyrics/feelin/presentation/view/home/HomeScreen.kt`:
- Around line 105-113: The same uiState.errorMessage state is being used to
render error messages in multiple UI locations (both an inline body error around
line 105-113 and a modal around line 178-185), causing duplicate error displays.
Refactor the error state to either use separate state properties for different
error UI paths (such as bodyErrorMessage and modalErrorMessage) or consolidate
to a single error display path. Update the condition checking
uiState.errorMessage to use the appropriate separated state, ensuring only one
error UI renders at a time for the same error event.
- Around line 118-123: The ArtistRow composable in HomeScreen is receiving empty
lambda callbacks for onShowAllArtistsClick and onFindArtistsClick, rendering the
"전체보기" and "찾아보기" buttons non-functional. Replace the empty lambda placeholders
{} with actual callback function references that implement the intended
navigation or action behavior. These callbacks should be defined in the
HomeScreen function signature and wired from the caller, similar to how
onArtistClick is already properly connected. Apply this fix to all instances
where ArtistRow is used with empty callbacks, including the occurrence mentioned
at lines 135-137.
In `@app/src/main/java/com/lyrics/feelin/presentation/view/home/HomeViewModel.kt`:
- Around line 295-311: The third filter chip in the mockFilters() function
within HomeViewModel.kt has incorrect text that does not match the design
specification. Update the name parameter of the third FilterButtonData object
from "해석공유" to "핫식공유" to align with the specified UI copy and ensure the home
filter renders correctly according to the design/spec.
In
`@app/src/test/java/com/lyrics/feelin/presentation/view/home/HomeUiStateTest.kt`:
- Around line 47-51: In the errorStateExposesMessage() test function, replace
the assertNotNull check on state.errorMessage with an assertEquals assertion
that verifies the error message matches the exact expected value "홈 피드를 불러오지
못했어요." that was passed to the HomeUiState constructor. This ensures the test
validates that the error message is not only present but also correctly
propagated with the expected content.
---
Nitpick comments:
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/home/component/ArtistRow.kt`:
- Around line 27-33: The ArtistRow composable function has parameters in the
wrong order according to project conventions. Reorder the parameters so that
modifier is the first optional parameter (after all required parameters like
artists), and place all callback parameters (onArtistClick,
onShowAllArtistsClick, onFindArtistsClick) after the modifier parameter. The
correct order should be: artists (required), modifier with default value, then
the three callback functions in sequence.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/home/component/BannerSection.kt`:
- Around line 20-23: Reorder the parameters in the BannerSection function to
follow Compose API conventions. Move the modifier parameter to come before the
onBannerClick lambda parameter, ensuring that required parameters like banner
come first, followed by the optional modifier parameter, and then all
lambda/callback parameters at the end.
In
`@app/src/main/java/com/lyrics/feelin/presentation/view/home/component/HomeHeader.kt`:
- Around line 26-29: The parameter order in the HomeHeader composable function
does not follow the project's Compose convention. Reorder the parameters so that
modifier is positioned as the first optional parameter after required
parameters, and lambda parameters like onNotificationClick are placed at the
end. In HomeHeader, move the modifier parameter before the onNotificationClick
lambda parameter to align with the coding guidelines.
🪄 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: bc687c3e-2306-4401-8f28-c52aaffe482f
📒 Files selected for processing (26)
app/build.gradle.ktsapp/src/main/java/com/lyrics/feelin/core/designsystem/component/FeelinTopAppBar.ktapp/src/main/java/com/lyrics/feelin/core/designsystem/icon/Icons.ktapp/src/main/java/com/lyrics/feelin/navigation/FeelinDestination.ktapp/src/main/java/com/lyrics/feelin/navigation/FeelinNavHost.ktapp/src/main/java/com/lyrics/feelin/navigation/HomeNavigation.ktapp/src/main/java/com/lyrics/feelin/presentation/view/community/CommunityMainScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/artist/ArtistBubbleComponentData.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/artist/ArtistProfileComponent.ktapp/src/main/java/com/lyrics/feelin/presentation/view/component/note/NoteComponent.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/HomeScreen.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/HomeUiState.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/HomeViewModel.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/ArtistRow.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/Banner.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/BannerSection.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/EmptyState.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/FeedSection.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/FeedTab.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/FeedTabState.ktapp/src/main/java/com/lyrics/feelin/presentation/view/home/component/HomeHeader.ktapp/src/main/java/com/lyrics/feelin/presentation/view/mypage/MyPageScreen.ktapp/src/test/java/com/lyrics/feelin/presentation/view/home/FeedTabStateTest.ktapp/src/test/java/com/lyrics/feelin/presentation/view/home/HomeUiStateTest.ktapp/src/test/java/com/lyrics/feelin/presentation/view/home/HomeViewModelTest.ktgradle/libs.versions.toml
Consume pointer input on the dim layer in HomeScreen to stop touch eventsfrom passing through to underlying components.
Change FeedSection from a Composable function to a LazyListScope extension to preserve parent LazyColumn virtualization. Split the section into smaller helper functions and use items() with unique keys for the note list to improve rendering performance.
Replace assertNotNull with assertEquals to verify the exact error message is propagated through the UI state.
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 710c6f4c7b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // MARK(@이대근): 담고 있는 데이터의 형태가 달라 같은 FliterButton을 유지할지 결정 필요 | ||
| FilterButton( | ||
| data = filter, | ||
| isSelect = selectedFilter?.id == filter.id, |
There was a problem hiding this comment.
현재 모드(legacyMode == false)에서 서버나 mock 데이터처럼 여러 필터의 id가 null이면 selectedFilter?.id == filter.id가 모든 항목에 대해 true가 되어 전체/인기/해석공유 버튼이 동시에 선택 상태로 표시됩니다. FilterButtonData 주석상 전체보기는 ID가 없을 수 있고, 이 커밋의 mockFilters()도 모든 ID를 null로 두고 있으므로, 필터 객체 자체나 이름/인덱스 등 null을 구분할 수 있는 값으로 선택 여부를 비교해야 합니다.
Useful? React with 👍 / 👎.
Member
Author
|
병합하겠습니다. 필터칩 문제는 이를 사용할때 수정하겠습니다. |
2 tasks
1 task
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)?
홈 화면의 디자인을 구현합니다. 또한 해당 화면을 내비게이션에 등록합니다.
홈 화면은 기존 서버의 API호환을 위한 기존 디자인과 개편 이후 디자인을 모두 대응합니다.
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)
Legacy layout
Light mode
Dark mode
New layout
Light mode
Dark mode
Other information:
신규 모드일 경우, 필터칩 내부의 데이터가 구분되지 않아 여러 개의 칩이 동시에 눌려져 있습니다.
메인화면에서 사용하는 필터칩의 디자인이 마이페이지에서 사용하는 것과 같아 같은 컴포넌트로 분류했는데, 이 컴포넌트가 가지고 있어야 하는 데이터의 형태가 아티스트/카테고리 역할에 따라 달라져야 해 컴포넌트를 분리할지 고민중입니다.
Summary by CodeRabbit