[Feat] 마이페이지 디자인 반영 및 Compose 전환#518
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the My Page screen to Jetpack Compose and introduces a new term selection activity. Feedback includes fixing a layout bug where vertical spacing was used instead of horizontal, correcting a tracking ID for Instagram links, and simplifying the Compose hierarchy by removing redundant wrappers. The reviewer also suggested using the activity context for navigation, removing commented-out code, and ensuring user-facing strings are translatable for future localization.
| startWebView( | ||
| getString(R.string.eatssu_instagram_url), | ||
| getString(R.string.eatssu_instagram), | ||
| ScreenId.EXTERNAL_TERMS |
There was a problem hiding this comment.
인스타그램 링크 클릭 시 ScreenId.EXTERNAL_TERMS를 전달하고 있습니다. 이는 서비스 이용약관 관련 화면 ID로 보이는데, 인스타그램 방문 트래킹을 위한 별도의 ID(예: EXTERNAL_INSTAGRAM)를 사용하거나 의도된 것인지 확인이 필요합니다.
| contentDescription = stringResource(R.string.mypage), | ||
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(16.dp)) |
There was a problem hiding this comment.
Row 레이아웃 내에서 프로필 이미지와 닉네임 정보 사이의 가로 간격을 설정하고 있으므로, height 대신 width를 사용하는 것이 올바릅니다.
| Spacer(modifier = Modifier.height(16.dp)) | |
| Spacer(modifier = Modifier.width(16.dp)) |
| // MyPageMenuItem( | ||
| // title = stringResource(R.string.oss_licenses), | ||
| // onClick = onOssClick, | ||
| // ) |
There was a problem hiding this comment.
주석 처리된 코드는 코드의 가독성을 저해할 수 있습니다. 현재 디자인에서 오픈소스 라이브러리 메뉴가 제외된 것이라면 해당 코드를 삭제하고, 나중에 추가할 예정이라면 TODO 주석을 남겨 관리하는 것을 권장합니다.
| Column { | ||
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .clickable(onClick = onClick) | ||
| .padding(horizontal = 24.dp, vertical = 12.dp), | ||
| horizontalArrangement = Arrangement.SpaceBetween, | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| ) { | ||
| Text( | ||
| text = title, | ||
| style = EatssuTheme.typography.body1, | ||
| color = Gray700, | ||
| ) | ||
| Icon( | ||
| modifier = Modifier.size(18.dp), | ||
| painter = painterResource(R.drawable.ic_arrow_right), | ||
| contentDescription = null, | ||
| tint = Gray300, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
MyPageMenuItem 컴포저블 내부에서 Row를 Column으로 감싸고 있는데, 현재 구조에서는 Column이 불필요한 레이아웃 계층을 형성하고 있습니다. 성능 최적화 및 코드 단순화를 위해 Column을 제거하는 것이 좋습니다.
Row(
modifier = Modifier
.fillMaxWidth()
.clickable(onClick = onClick)
.padding(horizontal = 24.dp, vertical = 12.dp),
horizontalArrangement = Arrangement.SpaceBetween,
verticalAlignment = Alignment.CenterVertically,
) {
Text(
text = title,
style = EatssuTheme.typography.body1,
color = Gray700,
)
Icon(
modifier = Modifier.size(18.dp),
painter = painterResource(R.drawable.ic_arrow_right),
contentDescription = null,
tint = Gray300,
)
}| } | ||
|
|
||
| private fun startWebView(url: String, title: String, screenId: ScreenId) { | ||
| val intent = Intent(applicationContext, WebViewActivity::class.java).apply { |
There was a problem hiding this comment.
Activity를 시작할 때 applicationContext를 사용하고 있습니다. 일반적인 Activity 전환 시에는 Activity의 Context(this)를 사용하는 것이 테마 리소스 접근 및 메모리 관리 측면에서 더 적절합니다.
| val intent = Intent(applicationContext, WebViewActivity::class.java).apply { | |
| val intent = Intent(this, WebViewActivity::class.java).apply { |
| <string name="policy_url" translatable="false">https://github.com/EAT-SSU/Docs/wiki/EAT%E2%80%90SSU-%EA%B0%9C%EC%9D%B8%EC%A0%95%EB%B3%B4%EC%B2%98%EB%A6%AC%EB%B0%A9%EC%B9%A8</string> | ||
| <string name="terms_url" translatable="false">https://github.com/EAT-SSU/Docs/wiki/EAT%E2%80%90SSU-%EC%84%9C%EB%B9%84%EC%8A%A4-%EC%9D%B4%EC%9A%A9%EC%95%BD%EA%B4%80</string> | ||
| <string name="anyone_but_me_url" translatable="false">https://eatssu-coffee.figma.site/</string> | ||
| <string name="eatssu_instagram_link" translatable="false">EAT-SSU 인스타그램 바로가기</string> |
There was a problem hiding this comment.
사용자에게 노출되는 텍스트인 eatssu_instagram_link에 translatable="false"가 설정되어 있습니다. 향후 다국어 지원을 고려한다면 번역이 필요한 문자열에서는 이 속성을 제거해야 합니다.
PeraSite
left a comment
PeraSite
left a comment
There was a problem hiding this comment.
좋네요! 언어 선택 페이지는 추후에 피그마 디자인 맞춰서 수정하면 될 것 같아용
2 participants
Summary
마이페이지 리뉴얼을 진행하면서 기존 MyPageFragment를 Composable UI(MyPageScreen)로 전환했습니다.
[약관 및 정책] 스크린도 추가하면서 일단 지금 뷰 시스템 구조에 맞게 Activity로 한번 더 감쌌습니다
Describe your changes
Issue
To reviewers
언어선택 Screen이 만들어진지 얼마 안됐는데 이거 지금 피그마에 맞춰서 수정해야하나요?