-
Notifications
You must be signed in to change notification settings - Fork 0
[feat/#11] 홈 화면 작업 #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis PR implements the main Home screen UI for the application, replacing a placeholder Text component with a fully composed LazyColumn layout featuring multiple UI components including carousel, content sections, user cards, post lists, and bottom navigation content. Additionally, the device streaming configuration is expanded with numerous device entries, and supporting drawable resources and string values are added. Changes
Sequence DiagramsequenceDiagram
participant HomeScreen
participant LazyColumn
participant HomeCarousel
participant HomeContentField
participant HomeCardItem
participant HomePostContainer
participant HomeBottomContent
HomeScreen->>LazyColumn: render with PaddingValues
LazyColumn->>HomeCarousel: display image pager
HomeCarousel-->>LazyColumn: show page indicator
LazyColumn->>HomeContentField: display section header
HomeContentField-->>LazyColumn: show "All" with arrow
LazyColumn->>HomeCardItem: render user profile cards
HomeCardItem-->>LazyColumn: display user info + icon
LazyColumn->>HomePostContainer: display post list
HomePostContainer-->>LazyColumn: show posts with dividers
LazyColumn->>HomeBottomContent: display footer
HomeBottomContent-->>LazyColumn: show logo + policy links
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (14)
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCarousel.kt (1)
54-58: Consider usingContentScale.Cropfor better aspect ratio preservation.Currently using
ContentScale.FillBoundsmay distort images that don't match the 180dp height aspect ratio. Consider usingContentScale.Cropto maintain aspect ratios while filling the space.Apply this diff:
- contentScale = ContentScale.FillBounds, + contentScale = ContentScale.Crop,app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeBottomContent.kt (1)
53-64: Consider separate click handlers for each link.Both clickable texts currently use the same
onClickcallback, which means they'll trigger the same action. For better flexibility and to allow different navigation destinations, consider accepting separate callbacks.Example refactor:
@Composable fun HomeBottomContent( - onClick: () -> Unit, + onTermsClick: () -> Unit, + onReviewPolicyClick: () -> Unit, modifier: Modifier = Modifier, ) { Column( ... ) { ... Text( text = "이용약관 및 개인정보 취급방침", color = HsLinkTheme.colors.Grey400, style = HsLinkTheme.typography.btm_M, - modifier = Modifier.noRippleClickable(onClick = onClick) + modifier = Modifier.noRippleClickable(onClick = onTermsClick) ) Text( text = "리뷰운영정책", color = HsLinkTheme.colors.Grey400, style = HsLinkTheme.typography.btm_M, - modifier = Modifier.noRippleClickable(onClick = onClick) + modifier = Modifier.noRippleClickable(onClick = onReviewPolicyClick) ) } }app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCardItem.kt (2)
75-80: Minor formatting inconsistency.Line 78 places
style,maxLines, andoverflowon a single line, while the previous text fields (lines 59-65, 67-73) split these across multiple lines. Consider consistent formatting for better readability.Apply this diff:
Text( text = userInfo, color = HsLinkTheme.colors.Grey400, - style = HsLinkTheme.typography.caption_12Normal, maxLines = 1, + style = HsLinkTheme.typography.caption_12Normal, + maxLines = 1, overflow = TextOverflow.Ellipsis )
83-83: Consider removingweight(1f)from the Spacer.With both the
Column(line 56) andSpacerhavingweight(1f), they'll each consume 50% of available space, creating a large gap between the user info and the arrow icon. Typically, you'd want the Column to fill available space and the icon to be right-aligned without extra spacing.Apply this diff:
- Spacer(modifier = Modifier.weight(1f)) + Spacer(modifier = Modifier)Or simply remove the Spacer if no minimum spacing is needed:
- Spacer(modifier = Modifier.weight(1f)) - Icon(app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt (7)
37-44: Avoid re-wrapping the app theme at the Route levelWrapping HsLinkTheme here can duplicate status-bar SideEffect and lead to nested themes. Prefer a single theme at the app root; keep previews themed locally.
If the app already wraps the NavHost with HsLinkTheme, drop the theme here:
-@Composable -fun HomeRoute( - paddingValues: PaddingValues, -) { - HsLinkTheme { - HomeScreen(paddingValues) - } -} +@Composable +fun HomeRoute( + paddingValues: PaddingValues, +) { + HomeScreen(paddingValues) +}
61-69: Prevent logo distortion in TopBarContentScale.FillBounds will stretch the logo. Use Fit to preserve aspect ratio.
Image( painter = painterResource(id = R.drawable.img_home_logo), contentDescription = null, modifier = Modifier .height(48.dp) .width(92.dp), - contentScale = ContentScale.FillBounds, + contentScale = ContentScale.Fit, )
86-94: Wire up “전체보기” action for this sectionHomeContentField shows an “전체보기” affordance but it’s not clickable. Expose an onAllClick lambda and pass it from here.
In HomeScreen:
HomeContentField( mainText = "멘토링 신청", subText = "한성선배님께 멘토링을\n신청해보세요!", + onAllClick = { /* TODO: navigate to mentoring list */ }, )Change the component API (see comment in HomeContentField.kt) to accept onAllClick: () -> Unit = {} and make the trailing "전체보기" + arrow clickable.
98-104: HomeCardItem layout likely over-constrains content widthHomeCardItem internally uses Column.weight(1f) plus Spacer.weight(1f), which splits remaining width in half and can truncate text unnecessarily. Prefer a single weighted content column and a fixed Spacer before the trailing Icon.
Proposed change in app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCardItem.kt:
- Column( - modifier = Modifier.weight(1f), + Column( + modifier = Modifier.weight(1f), verticalArrangement = Arrangement.spacedBy(4.dp) ) { /* ... */ } - - Spacer(modifier = Modifier.weight(1f)) + Spacer(modifier = Modifier.width(8.dp))Also applies to: 161-168
118-149: Avoid rebuilding the posts list on every recompositionWrap the static list with remember (or provide from ViewModel/state) to reduce allocations.
+import androidx.compose.runtime.remember @@ - HomePostContainer( - posts = listOf( + HomePostContainer( + posts = remember { + listOf( HomePost( id = "1", title = "2026 카카오 신입 공채", route = "route1" ), HomePost( id = "2", title = "선배가 알려주는 이력서 꿀팁", route = "route2" ), HomePost( id = "3", title = "선배가 알려주는 이력서 꿀팁", route = "route3" ), HomePost( id = "4", title = "선배가 알려주는 이력서 꿀팁", route = "route4" ), HomePost( id = "5", title = "선배가 알려주는 이력서 꿀팁", route = "route5" ) - - ), + ) + }, onPostClick = {}, modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp) )
89-92: Externalize user-facing strings for i18nThese literals should live in strings.xml to support localization.
Example:
- strings.xml: add home_section_mentoring_title, home_section_mentoring_sub, etc.
- Replace hard-coded strings with stringResource(id = …).
Also applies to: 109-113, 155-159
179-183: Hook up bottom links to real actionsonClick is empty. Provide dedicated callbacks (e.g., onTermsClick, onPolicyClick) or route via NavController.
I can draft a small nav wiring if you share the destination routes.
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeContentField.kt (3)
21-26: Add onAllClick and make the trailing affordance accessibleExpose a click handler and wrap “전체보기” + arrow in a single clickable Row with proper semantics.
@Composable -fun HomeContentField( - mainText: String, - subText: String, - modifier: Modifier = Modifier, -) { +fun HomeContentField( + mainText: String, + subText: String, + modifier: Modifier = Modifier, + onAllClick: () -> Unit = {}, +) { @@ - Row( - modifier = Modifier - .fillMaxWidth(), - ) { + Row( + modifier = Modifier.fillMaxWidth(), + ) { Text( text = subText, color = HsLinkTheme.colors.Grey700, style = HsLinkTheme.typography.title_20Strong ) Spacer(modifier = Modifier.weight(1F)) - Text( - text = stringResource(id = R.string.home_content_field_all), - color = HsLinkTheme.colors.DeepBlue500, - style = HsLinkTheme.typography.btm_M - ) - - Icon( - imageVector = ImageVector.vectorResource(id = R.drawable.ic_home_textall_arrow), - contentDescription = null, - tint = HsLinkTheme.colors.DeepBlue500, - ) + Row( + modifier = Modifier + .padding(start = 8.dp) + .clickable(onClick = onAllClick), + verticalAlignment = Alignment.CenterVertically + ) { + Text( + text = stringResource(id = R.string.home_content_field_all), + color = HsLinkTheme.colors.DeepBlue500, + style = HsLinkTheme.typography.btm_M + ) + Icon( + imageVector = ImageVector.vectorResource(id = R.drawable.ic_home_textall_arrow), + contentDescription = null, + tint = HsLinkTheme.colors.DeepBlue500, + ) + } }Remember to add imports:
import androidx.compose.foundation.clickable import androidx.compose.ui.AlignmentAnd update call sites (e.g., in HomeScreen) to pass onAllClick as needed.
Also applies to: 38-61
38-46: Center-align the row content verticallyKeeps the “전체보기” affordance vertically aligned with subText.
- Row( - modifier = Modifier - .fillMaxWidth(), - ) { + Row( + modifier = Modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically + ) {Add:
import androidx.compose.ui.Alignment
65-74: Preview: add onAllClick for completenessHook the preview to the new API to catch compile issues early.
-HsLinkTheme { - HomeContentField( - mainText = "Main Text", - subText = "Sub Text", - ) -} +HsLinkTheme { + HomeContentField( + mainText = "Main Text", + subText = "Sub Text", + onAllClick = {} + ) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/src/main/res/drawable/img_home_logo.pngis excluded by!**/*.png
📒 Files selected for processing (11)
.idea/caches/deviceStreaming.xml(6 hunks)app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt(2 hunks)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeBottomContent.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCardItem.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCarousel.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeContentField.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomePostContainer.kt(1 hunks)app/src/main/res/drawable/ic_home_arrow_right.xml(1 hunks)app/src/main/res/drawable/ic_home_post_arrow.xml(1 hunks)app/src/main/res/drawable/ic_home_textall_arrow.xml(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeContentField.kt (1)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCarousel.kt (1)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeBottomContent.kt (1)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomePostContainer.kt (1)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)
app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt (7)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTopBar.kt (1)
HsLinkTopBar(21-61)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCarousel.kt (1)
HomeCarousel(38-68)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeContentField.kt (1)
HomeContentField(21-63)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCardItem.kt (1)
HomeCardItem(37-91)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomePostContainer.kt (1)
HomePostContainer(45-75)app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeBottomContent.kt (1)
HomeBottomContent(32-67)
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCardItem.kt (1)
app/src/main/java/com/hsLink/hslink/core/designsystem/theme/Theme.kt (1)
HsLinkTheme(37-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (14)
.idea/caches/deviceStreaming.xml (1)
1-1242: The original review comment is incorrect. The file is properly gitignored and is not being committed.The verification confirms that:
.idea/is properly configured in.gitignore- Git status shows the file is not staged and the working tree is clean
- The file is not being tracked by version control
The appearance of this file in the PR review is a display artifact from the review interface showing local IDE state, not an actual commit. No action is required.
Likely an incorrect or invalid review comment.
app/src/main/res/drawable/ic_home_arrow_right.xml (1)
1-9: LGTM!The vector drawable is properly defined with standard Android XML attributes and will serve as a navigation indicator in the Home UI components.
app/src/main/res/values/strings.xml (1)
10-10: LGTM!The string resource addition follows Android conventions and provides localization for the "View All" action in the Home screen.
app/src/main/res/drawable/ic_home_textall_arrow.xml (1)
1-9: LGTM!The vector drawable is properly defined. The blue color (#0047BB) provides visual distinction from other arrow icons in the Home UI.
app/src/main/res/drawable/ic_home_post_arrow.xml (1)
1-9: LGTM!The vector drawable is properly defined with a slightly larger size (24dp) for use in post list items.
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomePostContainer.kt (3)
39-43: LGTM!The
HomePostdata class is well-defined with clear fields for representing post items in the UI.
45-75: LGTM!The
HomePostContainerimplementation is correct:
- Properly handles the list rendering with dividers between items
- Correctly avoids adding a divider after the last item (line 67)
- Uses appropriate theme colors and spacing
77-103: LGTM!The
HomePostContentcomposable correctly implements the individual post item with:
- Proper clickable handling using
noRippleClickable- Spacer with weight to push the icon to the right
- Color.Unspecified tint to preserve the drawable's original color
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCarousel.kt (2)
38-68: LGTM!The
HomeCarouselimplementation correctly usesHorizontalPagerwith proper state management and image loading via Coil'sAsyncImage.
70-89: LGTM!The indicator correctly displays the current page number (1-indexed for user display) with appropriate styling.
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeBottomContent.kt (1)
32-67: LGTM!The component layout and styling are well-implemented with proper use of theme colors and spacing.
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCardItem.kt (1)
37-91: LGTM!The card item component is well-structured with proper text overflow handling and theme integration.
app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt (1)
51-56: Structure looks solidUsing LazyColumn with padding(paddingValues) and themed background is clean and matches Scaffold usage.
app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeContentField.kt (1)
56-60: Resources verified and accessibleBoth the drawable
ic_home_textall_arrowand the stringhome_content_field_allexist in the codebase and will be accessible at runtime. The drawable is located atapp/src/main/res/drawable/ic_home_textall_arrow.xmland the string is defined inapp/src/main/res/values/strings.xml. No locale-specific resource variants are present, indicating single-language resource configuration.
| item { | ||
| HomeCarousel( | ||
| pagerImages = listOf( | ||
| "https://cdn.edujin.co.kr/news/photo/202209/39788_81397_67.png", | ||
| "image2", | ||
| "image3" | ||
| ), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Replace invalid carousel items or add image placeholders
The entries "image2" and "image3" are not valid URIs and will fail image loading. Either supply valid URLs or ensure HomeCarousel’s AsyncImage has placeholder/error.
Run to confirm placeholder/error handling in HomeCarousel:
Expected: AsyncImage configured with placeholder() and error() or a fallback painter.
🏁 Script executed:
#!/bin/bash
rg -n -C2 'AsyncImage\(' app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCarousel.ktLength of output: 191
Running verification to see the complete AsyncImage configuration:
---
🏁 Script executed:
```shell
#!/bin/bash
rg -n -C5 'AsyncImage\(' app/src/main/java/com/hsLink/hslink/presentation/home/component/HomeCarousel.kt
Length of output: 385
Add placeholder/error handling or replace invalid carousel URLs
The AsyncImage in HomeCarousel lacks placeholder/error parameters. Invalid URIs ("image2", "image3") will fail to load with no visual fallback. Either supply valid URLs or add placeholder() and error() parameters to AsyncImage (lines 54-58 of HomeCarousel.kt).
🤖 Prompt for AI Agents
In app/src/main/java/com/hsLink/hslink/presentation/home/HomeScreen.kt around
lines 76-84, HomeCarousel is being given invalid image URIs ("image2", "image3")
and the AsyncImage used inside HomeCarousel (see HomeCarousel.kt lines ~54-58)
has no placeholder/error handling; update either the passed pagerImages to valid
URLs or, preferably, modify the AsyncImage call to include placeholder() and
error() parameters (using a local drawable/painter or default composable) and
optionally a fallback() for nulls so users see a sensible visual when loading
fails; ensure the placeholder and error painters are imported and used
consistently and that any test images are replaced or guarded before being
passed in.
ISSUE
❗ WORK DESCRIPTIONAdd commentMore actions
📸 SCREENSHOT
Android.Emulator.-.google_5554.2025-10-27.14-29-57.mp4
📢 TO REVIEWERS
Summary by CodeRabbit
Release Notes