-
Notifications
You must be signed in to change notification settings - Fork 0
[feat/#12] my page UI #19
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 MyPage UI feature by restructuring navigation (changing start destination from Onboarding to Home, reorganizing navigation paths), introducing three new edit screens (Profile, Career, SNS) with form state management, and adding supporting UI components and drawable resources. The old simplified MyPage screen is replaced with a new structured screen hierarchy. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MypageScreen
participant ProfileEditScreen
participant UnsavedChangesDialog
participant NavController
User->>MypageScreen: Opens MyPage
Note over MypageScreen: Displays profile detail<br/>and navigation items
User->>MypageScreen: Clicks profile item
MypageScreen->>NavController: navigateToProfileEdit()
NavController->>ProfileEditScreen: Navigate to edit
User->>ProfileEditScreen: Modifies form fields
Note over ProfileEditScreen: State tracking<br/>hasUnsavedChanges = true
User->>ProfileEditScreen: Clicks back/close
rect rgb(255, 200, 100)
ProfileEditScreen->>ProfileEditScreen: Check hasUnsavedChanges()
alt Has unsaved changes
ProfileEditScreen->>UnsavedChangesDialog: Show dialog
User->>UnsavedChangesDialog: Click "나가기" (leave)
UnsavedChangesDialog->>NavController: Call navigateUp()
NavController->>MypageScreen: Return to MyPage
else No changes
ProfileEditScreen->>NavController: Call navigateUp()
NavController->>MypageScreen: Return to MyPage
end
end
User->>ProfileEditScreen: Completes form & clicks save
rect rgb(100, 200, 100)
Note over ProfileEditScreen: Validate form<br/>isFormValid = true
ProfileEditScreen->>NavController: Call onSaveClick()
NavController->>MypageScreen: Navigate back after save
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.idea/caches/deviceStreaming.xml (1)
1-990: This IDE cache file should not be included in the PR.The
.idea/caches/deviceStreaming.xmlfile is an Android Studio IDE configuration artifact that contains device definitions for emulator/device testing. This file is auto-managed by the IDE and is unrelated to the MyPage UI feature implementation objectives.Recommendation: Ensure the
.idea/directory is properly gitignored in your repository to prevent IDE-specific cache and configuration files from being committed. Add or verify the following in your.gitignore:.idea/ *.imlThis keeps the repository clean and focuses PRs on functional code changes only.
🧹 Nitpick comments (12)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/SNS/SNSNavigation.kt (1)
16-26: Consider renaming parameter for consistency.The function parameter is named
padding, but it's passed toSNSEditRouteaspaddingValues. For clarity, consider renaming the parameter to match the destination's expectation.Apply this diff:
fun NavGraphBuilder.snsNavGraph( - padding: PaddingValues, + paddingValues: PaddingValues, navController: NavController, ) { composable<SNSEdit> { SNSEditRoute( - paddingValues = padding, + paddingValues = paddingValues, navController = navController ) } }app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/ErrorTextField.kt (2)
17-28: Consider exposingkeyboardOptionsfor different input types.Since this component is used for form fields like student ID (which needs numeric keyboard) and name fields, exposing the
keyboardOptionsparameter fromHsLinkTextFieldwould improve UX by allowing appropriate keyboard types for each field.Apply this diff to add keyboard options support:
@Composable fun ErrorTextField( label: String, value: String, onValueChange: (String) -> Unit, placeholder: String, isFocused: Boolean, onFocusChanged: (Boolean) -> Unit, modifier: Modifier = Modifier, + keyboardOptions: KeyboardOptions = KeyboardOptions.Default, isRequired: Boolean = true, isError: Boolean = false, errorMessage: String = "", )Then pass it to
HsLinkTextFieldon line 50:import androidx.compose.foundation.text.KeyboardOptionsHsLinkTextField( value = value, placeholder = placeholder, onValueChanged = onValueChange, + keyboardOptions = keyboardOptions, borderColor = when {
43-43: Consider using theme colors for error states.
Color.Redis used directly for the required asterisk, error border, and error message. For consistency with the design system, consider defining an error color inHsLinkTheme.colors(e.g.,ErrororRed500) and using it throughout.Also applies to: 55-55, 68-68
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/SNSCard.kt (2)
67-67: Add content description for accessibility.The icon's
contentDescriptionis set tonull, which reduces accessibility for users relying on screen readers. While the card itself is clickable and provides textual context throughtitleandcontent, the icon still conveys navigational intent.Apply this diff to improve accessibility:
Icon( imageVector = ImageVector.vectorResource(id = R.drawable.ic_mypage_item_lefrarrow), - contentDescription = null, + contentDescription = "Navigate to edit", tint = Black )
54-54: Consider using theme colors instead of hardcoded Black.Lines 54 and 68 use the hardcoded
Blackcolor directly. For better theme consistency and potential dark mode support, consider using theme-defined colors.Apply this diff if your theme defines appropriate text and icon colors:
Text( text = title, - color = Black, + color = HsLinkTheme.colors.TextPrimary, style = HsLinkTheme.typography.title_20Strong )Icon( imageVector = ImageVector.vectorResource(id = R.drawable.ic_mypage_item_lefrarrow), contentDescription = "Navigate to edit", - tint = Black + tint = HsLinkTheme.colors.IconPrimary )Note: Adjust color names (
TextPrimary,IconPrimary) to match your actual theme color definitions.Also applies to: 68-68
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt (1)
75-97: Refactor to eliminate hardcoded value duplication.The
hasUnsavedChanges()function duplicates the initial state values from lines 75-80. This creates a maintenance burden: if you change the initial values in one place, you must remember to update them in both locations.Apply this diff to store original values and compare against them:
// State 관리 + val initialStartDate = "2024.04" + val initialEndDate = "2024.08" + val initialIsCurrentlyEmployed = false + val initialCompanyName = "한성대학교" + val initialJobName = "영업직" + val initialSelectedJobType = JobType.FULL_TIME + - var startDate by remember { mutableStateOf("2024.04") } - var endDate by remember { mutableStateOf("2024.08") } - var isCurrentlyEmployed by remember { mutableStateOf(false) } - var companyName by remember { mutableStateOf("한성대학교") } - var jobName by remember { mutableStateOf("영업직") } - var selectedJobType by remember { mutableStateOf<JobType?>(JobType.FULL_TIME) } + var startDate by remember { mutableStateOf(initialStartDate) } + var endDate by remember { mutableStateOf(initialEndDate) } + var isCurrentlyEmployed by remember { mutableStateOf(initialIsCurrentlyEmployed) } + var companyName by remember { mutableStateOf(initialCompanyName) } + var jobName by remember { mutableStateOf(initialJobName) } + var selectedJobType by remember { mutableStateOf<JobType?>(initialSelectedJobType) } // Focus state들 var companyFocused by remember { mutableStateOf(false) } var jobNameFocused by remember { mutableStateOf(false) } var startDateFocused by remember { mutableStateOf(false) } var endDateFocused by remember { mutableStateOf(false) } var showExitDialog by remember { mutableStateOf(false) } // 변경사항이 있는지 체크하는 함수 (함수 내부로 이동) fun hasUnsavedChanges(): Boolean { - return startDate != "2024.04" || - endDate != "2024.08" || - companyName != "한성대학교" || - jobName != "영업직" || - selectedJobType != JobType.FULL_TIME + return startDate != initialStartDate || + endDate != initialEndDate || + isCurrentlyEmployed != initialIsCurrentlyEmployed || + companyName != initialCompanyName || + jobName != initialJobName || + selectedJobType != initialSelectedJobType }Note: The comparison now also includes
isCurrentlyEmployed, which was missing from the original check.app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/SNS/SNSEditScreen.kt (3)
88-98: Refactor to eliminate hardcoded value duplication.The
hasUnsavedChanges()function duplicates the initial state values from lines 88-89, creating the same maintenance issue found in CareerEditScreen.Apply this diff to store original values and compare against them:
// State 관리 + val initialSelectedLinkType = "기타" + val initialUrl = "https://www.instagram.com/02_sing_song/" + - var selectedLinkType by remember { mutableStateOf("기타") } - var url by remember { mutableStateOf("https://www.instagram.com/02_sing_song/") } + var selectedLinkType by remember { mutableStateOf(initialSelectedLinkType) } + var url by remember { mutableStateOf(initialUrl) } var isUrlFocused by remember { mutableStateOf(false) } var showExitDialog by remember { mutableStateOf(false) } // ← 추가 // 변경사항이 있는지 체크하는 함수 fun hasUnsavedChanges(): Boolean { - return selectedLinkType != "기타" || - url != "https://www.instagram.com/02_sing_song/" + return selectedLinkType != initialSelectedLinkType || + url != initialUrl }
195-199: Remove commented-out code.The commented-out Icon code should be removed to keep the codebase clean. If needed later, it can be retrieved from version control.
Apply this diff:
color = HsLinkTheme.colors.Grey700 ) -// Icon( -// imageVector = ImageVector.vectorResource(R.drawable.ic_dropdown), -// contentDescription = null, -// tint = HsLinkTheme.colors.Grey400 -// ) } }
281-326: Remove unused LinkTypeDropdown function.The
LinkTypeDropdowncomposable is defined but never used in this file. It appears to be an alternative implementation that was replaced by the simpler DropdownMenu approach (lines 203-221).Remove the unused function to reduce code clutter:
-@Composable -private fun LinkTypeDropdown( - query: String, - onLinkTypeSelect: (String) -> Unit -) { - val linkTypes = listOf("기타", "링크드인", "인스타그램", "깃허브", "노션", "구글 드라이브") - val filteredTypes = linkTypes.filter { - it.contains(query, ignoreCase = true) - } - - Column( - modifier = Modifier - .fillMaxWidth() - .heightIn(max = 300.dp) - .clip(RoundedCornerShape(8.dp)) - .border(1.dp, HsLinkTheme.colors.Grey200, RoundedCornerShape(8.dp)) - .background(HsLinkTheme.colors.Common) - ) { - LazyColumn { - items(filteredTypes.size) { index -> - val linkType = filteredTypes[index] - - Column( - modifier = Modifier - .fillMaxWidth() - .clickable { onLinkTypeSelect(linkType) } - .padding(horizontal = 16.dp, vertical = 12.dp) - ) { - Text( - text = linkType, - style = HsLinkTheme.typography.body_14Normal, - color = HsLinkTheme.colors.Grey700 - ) - } - - if (index < filteredTypes.size - 1) { - HorizontalDivider( - modifier = Modifier.padding(horizontal = 16.dp), - thickness = 1.dp, - color = HsLinkTheme.colors.Grey200 - ) - } - } - } - } -}app/src/main/java/com/hsLink/hslink/presentation/main/MainTab.kt (1)
20-46: Replace placeholder icons with tab-specific icons.All tabs (HOME, SEARCH, COMMUNITY, MYPAGE) currently use the same
ic_mypageicon for both selected and unselected states. Each tab should have distinct, semantically appropriate icons to provide clear visual differentiation for users.app/src/main/java/com/hsLink/hslink/presentation/mypage/component/main/MyPageCardItem.kt (1)
41-45: Consider removing or documenting the unusedroutefield.The
routefield inMyPageItemDatais currently unused in the navigation logic (seeMypageScreen.ktlines 98-100, 115-117 where click handlers are empty). If this field is intended for future use, consider adding a TODO comment. Otherwise, remove it to avoid confusion.If keeping for future use:
data class MyPageItemData( val id: String, val title: String, - val route: String, + val route: String, // TODO: Use for navigation once API integration is complete )app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/main/MypageScreen.kt (1)
83-89: Consider parameterizing hardcoded profile data.The profile information (name, major, status) is currently hardcoded. For better testability and reusability, consider accepting these as parameters or from a state holder.
@Composable fun MypageScreen( modifier: Modifier = Modifier, paddingValues: PaddingValues, + profileName: String = "송효재", // Default for preview/testing + profileTitle: String = "21학번 회계재무경영", + profileSubtitle: String = "구직 중 · 재직 중 · 졸업", onNavigateToProfile: () -> Unit = {}, ... ) { ... MyPageDetailItemContent( - name = "송효재", - title = "21학번 회계재무경영", - subtitle = "구직 중 · 재직 중 · 졸업", + name = profileName, + title = profileTitle, + subtitle = profileSubtitle, onClick = onNavigateToProfile )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
.idea/caches/deviceStreaming.xml(3 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt(3 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainNavigator.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/main/MainTab.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/MypageScreen.kt(0 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/UnsavedChangesDialog.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/main/MyPageCardItem.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/main/MyPageDetailItemContent.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/CareerCard.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/ErrorTextField.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/SNSCard.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/SNS/SNSNavigation.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/main/MypageNavigation.kt(2 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/profile/ProfileEditNavigation.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/SNS/SNSEditScreen.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/main/MypageScreen.kt(1 hunks)app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/profile/ProfileEditScreen.kt(1 hunks)app/src/main/res/drawable/ic_mypage_item_lefrarrow.xml(1 hunks)app/src/main/res/drawable/ic_profile_search.xml(1 hunks)app/src/main/res/drawable/ic_topbar_arrowleft.xml(1 hunks)app/src/main/res/drawable/ic_topbar_close.xml(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/hsLink/hslink/presentation/mypage/MypageScreen.kt
🧰 Additional context used
🧬 Code graph analysis (15)
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/SNSCard.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/mypage/component/main/MyPageDetailItemContent.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/mypage/component/profile/CareerCard.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/mypage/screen/profile/ProfileEditScreen.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(22-68)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTextField.kt (1)
HsLinkTextField(35-107)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/CareerCard.kt (1)
CareerCard(45-101)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkSelectButton.kt (1)
HsLinkSelectButton(38-94)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/SNSCard.kt (1)
SNSCard(24-72)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkActionButton.kt (1)
HsLinkActionButton(37-93)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/main/MypageNavigation.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/main/MypageScreen.kt (1)
MypageRoute(33-44)
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/UnsavedChangesDialog.kt (2)
app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkSelectButton.kt (1)
HsLinkSelectButton(38-94)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkActionButton.kt (1)
HsLinkActionButton(37-93)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/SNS/SNSNavigation.kt (2)
app/src/main/java/com/hsLink/hslink/presentation/main/MainNavigator.kt (1)
navigate(36-54)app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/SNS/SNSEditScreen.kt (1)
SNSEditRoute(62-76)
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/ErrorTextField.kt (1)
app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTextField.kt (1)
HsLinkTextField(35-107)
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt (6)
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(22-68)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTextField.kt (1)
HsLinkTextField(35-107)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkSelectButton.kt (1)
HsLinkSelectButton(38-94)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkActionButton.kt (1)
HsLinkActionButton(37-93)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/UnsavedChangesDialog.kt (1)
UnsavedChangesDialog(18-66)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt (2)
app/src/main/java/com/hsLink/hslink/presentation/main/MainNavigator.kt (1)
navigate(36-54)app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt (1)
CareerEditRoute(50-64)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/profile/ProfileEditNavigation.kt (1)
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/profile/ProfileEditScreen.kt (1)
ProfileEditScreenRoute(70-88)
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/main/MyPageCardItem.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/mypage/screen/SNS/SNSEditScreen.kt (5)
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(22-68)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTextField.kt (1)
HsLinkTextField(35-107)app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkActionButton.kt (1)
HsLinkActionButton(37-93)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/UnsavedChangesDialog.kt (1)
UnsavedChangesDialog(18-66)
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/main/MypageScreen.kt (3)
app/src/main/java/com/hsLink/hslink/core/designsystem/component/HsLinkTopBar.kt (1)
HsLinkTopBar(22-68)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/main/MyPageDetailItemContent.kt (1)
MyPageDetailItemContent(45-101)app/src/main/java/com/hsLink/hslink/presentation/mypage/component/main/MyPageCardItem.kt (1)
MyPageCardItemContainer(47-81)
app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt (4)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/main/MypageNavigation.kt (1)
mypageNavGraph(17-27)app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/profile/ProfileEditNavigation.kt (1)
profileEditNavGraph(18-31)app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt (1)
careerNavGraph(16-26)app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/SNS/SNSNavigation.kt (1)
snsNavGraph(16-26)
🔇 Additional comments (22)
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/SNS/SNSNavigation.kt (1)
12-29: Clean type-safe navigation implementation.The navigation wiring correctly follows the modern Jetpack Compose Navigation pattern with serializable routes and extension functions. The structure is consistent with the rest of the codebase and provides a clean API for navigating to the SNS edit screen.
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/ErrorTextField.kt (2)
34-47: LGTM!The label row correctly handles the required indicator with appropriate theme styling.
65-71: LGTM!The error message display logic is defensive and correct. The preview functions comprehensively cover the different states (normal, error, focused).
Also applies to: 75-123
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/SNSCard.kt (2)
74-84: LGTM!The preview function is well-structured with appropriate sample data and theme wrapping for accurate design-time rendering.
66-66: The icon is semantically correct; the review concern was based on a false premise.After verifying the XML vector content,
ic_mypage_item_lefrarrowis actually a right-pointing arrow, not a left-pointing one. The vector path geometrically moves from left to right (x-coordinates: 1.138 → 7.804), making it appropriate for a trailing navigation indicator. While the filename is misleading and "lefrarrow" appears to be a typo, the icon functions correctly in its intended semantic role across all four files where it's used (SNSCard, MyPageDetailItemContent, MyPageCardItem, CareerCard).Likely an incorrect or invalid review comment.
app/src/main/res/drawable/ic_profile_search.xml (1)
1-9: LGTM!The vector drawable resource is properly defined with correct dimensions and standard Android XML structure.
app/src/main/res/drawable/ic_topbar_close.xml (1)
1-9: LGTM!The close icon is properly defined with correct dimensions and standard Android vector drawable structure.
app/src/main/res/drawable/ic_topbar_arrowleft.xml (1)
1-20: LGTM!The arrow-left icon is properly defined with stroked paths and correct dimensions for top bar navigation.
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/career/CareerEditNavigation.kt (3)
12-14: LGTM!The navigation extension function follows standard Compose Navigation patterns for navigating to the Career Edit screen.
16-26: LGTM!The navigation graph builder properly registers the CareerEdit route and wires up the CareerEditRoute composable with required dependencies.
28-29: LGTM!The serializable route object is correctly defined for type-safe navigation.
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/CareerCard.kt (1)
45-101: LGTM!The CareerCard composable is well-structured with proper theming, clear layout hierarchy, and appropriate onClick handling.
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/career/UnsavedChangesDialog.kt (1)
18-66: LGTM!The unsaved changes dialog is well-implemented with clear messaging, proper button layout, and correct callback wiring. The explicit
dismissButton = nullwith comment clearly documents the design decision.app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/career/CareerEditScreen.kt (2)
50-64: LGTM!The CareerEditRoute properly wires navigation callbacks to the screen and follows standard Compose Navigation patterns.
108-325: LGTM!The form validation, LazyColumn layout, field management, and dialog integration are well-implemented with proper state management and user feedback.
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/SNS/SNSEditScreen.kt (2)
62-76: LGTM!The SNSEditRoute properly wires navigation callbacks following standard Compose Navigation patterns.
109-279: LGTM!The form layout, validation, dialog integration, and user interaction handling are well-implemented with proper state management.
app/src/main/java/com/hsLink/hslink/presentation/main/MainNavHost.kt (1)
36-65: LGTM! Navigation wiring is well-structured.The navigation graph composition properly wires the mypage, profile edit, career, and SNS navigation graphs with consistent use of named parameters and appropriate dependencies (padding and navController).
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/main/MypageNavigation.kt (1)
17-26: LGTM! Navigation graph properly extended with NavController.The addition of the
navControllerparameter enables navigation from the Mypage screen to child destinations (profile edit, career, SNS), and the use of named parameters improves code clarity.app/src/main/java/com/hsLink/hslink/presentation/main/MainNavigator.kt (1)
24-24: Verify the start destination change from Onboarding to Home.The start destination has been changed from
OnboardingtoHome, which skips the onboarding flow entirely. Ensure this change is intentional and aligns with the product requirements for this PR.If onboarding should still be shown on first launch, consider implementing a conditional start destination based on user state:
val startDestination = if (isFirstLaunch) Onboarding else Homeapp/src/main/java/com/hsLink/hslink/presentation/mypage/component/main/MyPageCardItem.kt (1)
84-122: LGTM! Card component is well-structured.The composable follows Material Design patterns with proper theming, spacing, and click handling. The row layout with text and trailing icon provides clear affordance for navigation.
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/profile/ProfileEditNavigation.kt (1)
12-16: LGTM! Navigation extension follows best practices.The
navigateToProfileEditfunction correctly uses the Navigation Compose API with proper route object and optional NavOptions parameter.
| data class MyPageDetailItemData( | ||
| val id: String, | ||
| val name : String, | ||
| val title: String, | ||
| val subtitle: String, | ||
| val route: String, | ||
| ) |
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
Verify usage of MyPageDetailItemData or consider removing it.
The MyPageDetailItemData class is defined in this file but not used by the CareerCard composable. The composable takes individual parameters (name, title, dateRange) instead of using this data class. Additionally, there's a naming inconsistency: the data class has a subtitle field while the composable uses dateRange.
Run the following script to check if this data class is used elsewhere:
🏁 Script executed:
#!/bin/bash
# Search for usages of MyPageDetailItemData across the codebase
rg -n "MyPageDetailItemData" --type kotlin -C3Length of output: 1562
I can see the data class is defined in two files. Let me search for actual usages (instantiations, parameter usage) to determine if this class is used elsewhere:
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for function/composable signatures and usages of MyPageDetailItemData
# Look for: function parameters, return types, and instantiations
rg -n "MyPageDetailItemData\(" --type kotlin -C5
Length of output: 2500
🏁 Script executed:
#!/bin/bash
# Search for imports of MyPageDetailItemData to understand cross-file usage
rg -n "import.*MyPageDetailItemData|MyPageDetailItemData\s*=" --type kotlin -C3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Get the full CareerCard.kt file to understand the context
cat "app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/CareerCard.kt"Length of output: 3126
🏁 Script executed:
#!/bin/bash
# Get the full MyPageDetailItemContent.kt file to understand the context
cat "app/src/main/java/com/hsLink/hslink/presentation/mypage/component/main/MyPageDetailItemContent.kt"Length of output: 3137
Remove unused MyPageDetailItemData class from CareerCard.kt.
The data class is defined but never instantiated or used anywhere in the codebase. The CareerCard composable uses individual parameters (name, title, dateRange) instead. Additionally, the data class field subtitle does not match the composable's parameter name dateRange, indicating this class was not properly integrated with the composable signature.
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/mypage/component/profile/CareerCard.kt
around lines 37 to 43, remove the unused data class MyPageDetailItemData (id,
name, title, subtitle, route) because the CareerCard composable uses separate
parameters (name, title, dateRange) and the class is never referenced; delete
the entire data class declaration, ensure there are no remaining imports or
references to it elsewhere, run a project build/IDE sync to verify no usages
remain, and if a consolidated data model is intended later, move/create it in
the appropriate domain/model package with matching field names.
| fun NavGraphBuilder.profileEditNavGraph( | ||
| padding: PaddingValues, | ||
| navController: NavController, // ← 추가 | ||
| navigateUp: () -> Unit, | ||
| ) { | ||
| composable<ProfileEdit> { | ||
| ProfileEditScreenRoute( | ||
| paddingValues = padding, | ||
| navController = navController, // ← 추가 | ||
| onBackClick = navigateUp, | ||
| onCloseClick = navigateUp | ||
| ) | ||
| } | ||
| } |
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.
Remove Korean development comments.
The Korean comments "← 추가" (meaning "← added") on lines 20 and 26 are development artifacts that should be removed before merging.
Apply this diff:
fun NavGraphBuilder.profileEditNavGraph(
padding: PaddingValues,
- navController: NavController, // ← 추가
+ navController: NavController,
navigateUp: () -> Unit,
) {
composable<ProfileEdit> {
ProfileEditScreenRoute(
paddingValues = padding,
- navController = navController, // ← 추가
+ navController = navController,
onBackClick = navigateUp,
onCloseClick = navigateUp
)
}
}🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/profile/ProfileEditNavigation.kt
around lines 18 to 31, remove the Korean inline development comments "← 추가" on
the NavController parameter line and the navController argument line so the code
contains only the parameter and argument without comment artifacts; ensure
spacing and commas remain correct after removing the comments.
| @@ -0,0 +1,29 @@ | |||
| package com.hsLink.hslink.presentation.mypage.navigation.sns | |||
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.
Critical: Package name doesn't match directory structure.
The package declaration uses lowercase sns, but the file path indicates the directory is SNS (uppercase). In Kotlin/Java, the package must exactly match the directory structure. This will cause a compilation error.
Apply this diff to fix the package declaration:
-package com.hsLink.hslink.presentation.mypage.navigation.sns
+package com.hsLink.hslink.presentation.mypage.navigation.SNSAlternatively, if the directory should be lowercase, rename the directory from SNS/ to sns/ to match the package declaration.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| package com.hsLink.hslink.presentation.mypage.navigation.sns | |
| package com.hsLink.hslink.presentation.mypage.navigation.SNS |
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/mypage/navigation/SNS/SNSNavigation.kt
around line 1, the package declaration uses lowercase "sns" but the directory is
"SNS", causing a mismatch; update the package declaration to exactly match the
directory path (change package to
com.hsLink.hslink.presentation.mypage.navigation.SNS) OR rename the directory
from SNS to sns so the filesystem path and package name are identical.
| HsLinkSelectButton( | ||
| label = "멘토로도 참여할래요", | ||
| onClick = { | ||
| selectedMentorType = "mentor" | ||
| }, | ||
| size = HsLinkButtonSize.Large, | ||
| isSelected = selectedMentorType == "", | ||
| modifier = Modifier.fillMaxWidth() | ||
| ) | ||
|
|
||
| HsLinkSelectButton( | ||
| label = "멘티로만 참여할래요", | ||
| onClick = { | ||
| selectedMentorType = "mentee" | ||
| }, | ||
| size = HsLinkButtonSize.Large, | ||
| isSelected = selectedMentorType == "mentee", // ← 선택 상태 | ||
| modifier = Modifier.fillMaxWidth() | ||
| ) |
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.
Fix mentor/job selection state wiring.
Both first options stay highlighted before any choice, yet lose their highlight right after being tapped, because isSelected compares against "" instead of the values you write into state ("mentor" / "seeking"). This makes the UI misleading and leaves users wondering why the save button is disabled until they toggle the other option. Switch the comparisons to the actual state values so selection and validation stay in sync.
HsLinkSelectButton(
label = "멘토로도 참여할래요",
onClick = {
selectedMentorType = "mentor"
},
size = HsLinkButtonSize.Large,
- isSelected = selectedMentorType == "",
+ isSelected = selectedMentorType == "mentor",
modifier = Modifier.fillMaxWidth()
)
...
HsLinkSelectButton(
label = "구직 중이에요",
onClick = {
selectedJobStatus = "seeking"
},
size = HsLinkButtonSize.Large,
- isSelected = selectedJobStatus == "",
+ isSelected = selectedJobStatus == "seeking",
modifier = Modifier.fillMaxWidth()
)Also applies to: 386-404
| import androidx.compose.material3.Icon | ||
| import androidx.compose.material3.SegmentedButtonDefaults.Icon |
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.
Remove conflicting Icon import.
Line 25 imports SegmentedButtonDefaults.Icon which conflicts with the Icon import on line 24 and is not used in this file.
Apply this diff to remove the unused import:
import androidx.compose.material3.Icon
-import androidx.compose.material3.SegmentedButtonDefaults.Icon
import androidx.compose.material3.Text📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import androidx.compose.material3.Icon | |
| import androidx.compose.material3.SegmentedButtonDefaults.Icon | |
| import androidx.compose.material3.Icon |
🤖 Prompt for AI Agents
In
app/src/main/java/com/hsLink/hslink/presentation/mypage/screen/SNS/SNSEditScreen.kt
around lines 24-25, there's a conflicting and unused import:
SegmentedButtonDefaults.Icon (line 25) conflicts with
androidx.compose.material3.Icon (line 24); remove the
SegmentedButtonDefaults.Icon import to eliminate the conflict and unused import.
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:width="8dp" | ||
| android:height="15dp" | ||
| android:viewportWidth="8" | ||
| android:viewportHeight="15"> | ||
| <path | ||
| android:pathData="M1.138,0.195C0.877,-0.065 0.456,-0.065 0.195,0.195C-0.065,0.456 -0.065,0.877 0.195,1.138L6.391,7.333L0.195,13.529L0.15,13.579C-0.064,13.841 -0.049,14.227 0.195,14.471C0.439,14.715 0.825,14.73 1.087,14.517L1.138,14.471L7.804,7.804C8.065,7.544 8.065,7.122 7.804,6.862L1.138,0.195Z" | ||
| android:fillColor="#54565A"/> | ||
| </vector> |
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.
Fix incorrect filename—icon draws a right arrow, not left.
The filename ic_mypage_item_lefrarrow.xml suggests a left-pointing arrow, but the path data draws a right-pointing chevron. This naming inconsistency will cause confusion for developers referencing this drawable.
Rename the file to accurately reflect the icon direction:
-ic_mypage_item_lefrarrow.xml
+ic_mypage_item_rightarrow.xmlThen update all references to use the corrected name in:
MyPageCardItem.kt(line 116)MyPageDetailItemContent.kt(referenced in AI summary)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/src/main/res/drawable/ic_mypage_item_lefrarrow.xml lines 1-9: the
filename indicates a left arrow but the vector path draws a right-pointing
chevron; rename the drawable file to a correct name (e.g.,
ic_mypage_item_rightarrow.xml or ic_mypage_item_chevron_right.xml) and update
all usages to the new resource name, specifically change the reference at
MyPageCardItem.kt line 116 and the reference in MyPageDetailItemContent.kt (and
run a project-wide search to update any other occurrences), then rebuild to
ensure resource linking is correct.
|
고생했으요! |
ISSUE
❗ WORK DESCRIPTION
마이페이지 프로필 수정 및 커리어/SNS 관리 기능 구현
주요 구현 사항:
ProfileEditScreen 완성
CareerEditScreen 신규 개발
SNSEditScreen 신규 개발
공통 컴포넌트
네비게이션 구조
📸 SCREENSHOT
2025-11-08.10.11.10.mov
📢 TO REVIEWERS
Summary by CodeRabbit
New Features
Refactoring
UI Components