Conversation
WalkthroughNew string resources for the "My Page" screen were added to the app's resource files. The Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
home/presentation/src/main/java/com/hyunjung/home/presentation/my_page/MyPageScreen.kt (6)
47-56: Add proper click handlers for top bar icons.The notification and search icon buttons have empty click handlers, which means they're non-functional. Consider implementing proper navigation or state management for these actions.
Would you like me to suggest a proper implementation pattern for these click handlers?
94-94: Use string resource instead of hardcoded text.The "고객 센터" text is hardcoded. Consider adding this to the string resources for consistency and localization support.
- text = "고객 센터", + text = stringResource(id = R.string.my_page_customer_center),You'll need to add this string resource:
<string name="my_page_customer_center">고객 센터</string>
103-121: Use string resources for menu items.The menu items ("공지 사항", "1:1 문의", "자주 묻는 질문", "이용 가이드") are hardcoded. Consider adding these to string resources for consistency and maintainability.
Add these to your string resources and replace the hardcoded strings:
<string name="my_page_notice">공지 사항</string> <string name="my_page_inquiry">1:1 문의</string> <string name="my_page_faq">자주 묻는 질문</string> <string name="my_page_guide">이용 가이드</string>
171-207: Extract button styling to reduce code duplication.The withdrawal and logout buttons share identical styling. Consider extracting this into a reusable component or style.
+@Composable +private fun MyPageActionButton( + text: String, + onClick: () -> Unit, + modifier: Modifier = Modifier +) { + Button( + onClick = onClick, + modifier = modifier, + contentPadding = PaddingValues(horizontal = 18.dp, vertical = 9.dp), + colors = ButtonColors( + containerColor = CherrydanColors.Gray2, + contentColor = CherrydanColors.Gray5, + disabledContainerColor = CherrydanColors.Gray2, + disabledContentColor = CherrydanColors.Gray5 + ), + shape = RoundedCornerShape(2.dp) + ) { + Text( + text = text, + style = CherrydanTypography.Main5_R, + color = CherrydanColors.Gray5 + ) + } +}Then replace the buttons:
- Button( - onClick = { /* TODO: Handle logout */ }, - modifier = Modifier - .padding(end = 8.dp), - contentPadding = PaddingValues(horizontal = 18.dp, vertical = 9.dp), - colors = ButtonColors( - containerColor = CherrydanColors.Gray2, - contentColor = CherrydanColors.Gray5, - disabledContainerColor = CherrydanColors.Gray2, - disabledContentColor = CherrydanColors.Gray5 - ), - shape = RoundedCornerShape(2.dp) - ) { - Text( - text = stringResource(id = R.string.my_page_withdrawal), - style = CherrydanTypography.Main5_R, - color = CherrydanColors.Gray5 - ) - } + MyPageActionButton( + text = stringResource(id = R.string.my_page_withdrawal), + onClick = { /* TODO: Handle withdrawal */ }, + modifier = Modifier.padding(end = 8.dp) + )
172-191: Implement proper logout and withdrawal functionality.Both buttons have TODO comments with empty implementations. These critical user actions should have proper implementations.
Would you like me to help implement proper logout and withdrawal functionality with confirmation dialogs?
213-227: Consider adding click functionality to MyPageItemText.The helper composable renders text items but doesn't support click actions. Since these appear to be menu items, consider adding click support.
@Composable -private fun MyPageItemText(text: String) { +private fun MyPageItemText( + text: String, + onClick: (() -> Unit)? = null +) { Row( modifier = Modifier .fillMaxWidth() .padding(horizontal = 16.dp, vertical = 8.dp), + .run { if (onClick != null) clickable { onClick() } else this }, verticalAlignment = Alignment.CenterVertically ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/presentation/ui/src/main/res/values/strings.xml(1 hunks)home/presentation/src/main/java/com/hyunjung/home/presentation/my_page/MyPageScreen.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (3)
core/presentation/ui/src/main/res/values/strings.xml (1)
49-60: LGTM! Well-structured string resources.The new string resources for the My Page screen follow consistent naming conventions and are properly formatted. The Korean localization aligns with the existing app structure.
home/presentation/src/main/java/com/hyunjung/home/presentation/my_page/MyPageScreen.kt (2)
103-121: Add click handlers for menu items.The menu items are missing click handlers, making them non-interactive. Each item should have an appropriate onClick behavior.
These menu items appear to be clickable UI elements but lack functionality. Please verify if these should navigate to other screens or trigger specific actions.
229-235: LGTM! Proper preview implementation.The preview composable follows best practices with proper theme wrapping.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/presentation/designsystem/src/main/java/com/hyunjung/core/presentation/designsystem/component/CherrydanDefaultButton.kt (1)
62-100: Consider extracting common modifier logic.The button implementation has duplicated modifier chains between SMALL and BIG sizes. Consider extracting the common logic to improve maintainability.
@Composable fun CherrydanDefaultButton( text: String, onClick: () -> Unit, modifier: Modifier = Modifier, style: CherrydanButtonStyle = CherrydanButtonStyle.GRAY, shape: CherrydanButtonShape = CherrydanButtonShape.ROUNDED_SMALL, size: CherrydanButtonSize = CherrydanButtonSize.BIG, enabled: Boolean = true, textStyle: TextStyle = CherrydanTypography.Main5_R ) { + val baseModifier = modifier + .clip(RoundedCornerShape(shape.cornerRadius)) + .background(if (enabled) style.containerColor else style.disabledContainerColor) + .clickable(enabled = enabled) { onClick() } + Box( modifier = when (size) { CherrydanButtonSize.SMALL -> { - modifier + baseModifier .height(36.dp) - .clip(RoundedCornerShape(shape.cornerRadius)) - .background(if (enabled) style.containerColor else style.disabledContainerColor) - .clickable(enabled = enabled) { onClick() } } CherrydanButtonSize.BIG -> { - modifier + baseModifier .fillMaxWidth() - .clip(RoundedCornerShape(shape.cornerRadius)) - .background(if (enabled) style.containerColor else style.disabledContainerColor) - .clickable(enabled = enabled) { onClick() } .padding(vertical = 15.dp) } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/presentation/designsystem/src/main/java/com/hyunjung/core/presentation/designsystem/component/CherrydanDefaultButton.kt(1 hunks)core/presentation/ui/src/main/res/values/strings.xml(1 hunks)home/presentation/src/main/java/com/hyunjung/home/presentation/my_page/MyPageScreen.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/presentation/ui/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- home/presentation/src/main/java/com/hyunjung/home/presentation/my_page/MyPageScreen.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
core/presentation/designsystem/src/main/java/com/hyunjung/core/presentation/designsystem/component/CherrydanDefaultButton.kt (1)
core/presentation/designsystem/src/main/java/com/hyunjung/core/presentation/designsystem/Theme.kt (1)
CherrydanTheme(24-42)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (4)
core/presentation/designsystem/src/main/java/com/hyunjung/core/presentation/designsystem/component/CherrydanDefaultButton.kt (4)
1-25: LGTM! Clean imports and package structure.The imports are well-organized and include all necessary dependencies for the button component. The package structure follows Android conventions.
26-50: Well-structured enum for button styles.The
CherrydanButtonStyleenum provides a clean way to define different button variants with their respective colors. The inclusion of both normal and disabled states is a good practice for accessibility.
52-60: Simple and effective enum definitions.The
CherrydanButtonShapeandCherrydanButtonSizeenums are well-designed with appropriate naming and values.
102-196: Comprehensive preview functions.The preview functions provide excellent coverage of different button states and configurations. They demonstrate enabled/disabled states, different styles, and both size variants effectively.
...rc/main/java/com/hyunjung/core/presentation/designsystem/component/CherrydanDefaultButton.kt
Show resolved
Hide resolved
...rc/main/java/com/hyunjung/core/presentation/designsystem/component/CherrydanDefaultButton.kt
Outdated
Show resolved
Hide resolved
2 participants
PULL REQUEST
Description
CherrydanDefaultButtoncomponentScreenShots
Summary by CodeRabbit
New Features
Style