-
Notifications
You must be signed in to change notification settings - Fork 0
[Ui/#100] 마이페이지, 설정 화면 리디자인 변경 사항 반영 #101
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
Walkthrough마이페이지·세팅 화면 리디자인 적용, 세팅의 탈퇴 흐름 제거 및 로그아웃 흐름 단순화, 디자인 시스템 버튼·아이콘·벡터 리소스 수정, 홈 하단 네비게이션 바 상단에 구분선 추가가 적용되었습니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
participant UI as SettingScreen
participant VM as SettingViewModel
participant UC as LogoutUseCase
participant NAV as Navigation
UI->>VM: ShowLogoutConfirmDialog (유저 표시 요청)
VM-->>UI: state.logoutConfirmDialogVisible = true
UI->>VM: logout() (유저 확인)
VM->>VM: state.logoutConfirmDialogVisible = false
VM->>UC: logoutUseCase()
UC-->>VM: success / failure
alt success
VM->>NAV: NavigateToLogin (LogoutSuccess)
else failure
VM-->>UI: emit LogoutFailure (상태 업데이트)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 2
🔭 Outside diff range comments (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt (1)
64-64
: 업데이트 클릭 액션 미연결: 사용자 액션이 동작하지 않습니다Container에서 onClickUpdate가 빈 람다로 전달되어(동작 없음) 실제 업데이트 흐름이 막혀 있습니다. 필수로 업데이트 진입(마켓 이동/내부 업데이트 화면) 로직을 연결하세요.
다음과 같이 연결을 제안합니다(뷰모델 함수명은 예시):
- onClickUpdate = {}, + onClickUpdate = viewModel::onClickUpdate,뷰모델 내 구현이 없다면, SideEffect로 앱스토어 이동을 발행하거나, 링크 오픈 유틸을 통해 직접 처리하도록 보완하겠습니다. 필요하시면 구현 코드까지 도와드릴게요.
🧹 Nitpick comments (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt (1)
128-133
: 버전 비교 로직 보완 고려(선택): 단순 동등성 비교는 프리릴리즈/빌드가 최신인 경우 오판 가능
state.version == state.latestVersion
만 비교하면 로컬 버전이 더 높거나(내부 테스트/프리릴리즈) 형식 차이(예: 1.0.0 vs 1.0.0+1)인 경우에도 "업데이트"로 표시될 수 있습니다. SemVer 파싱 비교로 보완을 추천합니다.예시 유틸(외부 범위 참고용):
private fun isUpToDate(current: String, latest: String): Boolean { fun parse(v: String) = v.split('.', '-', '+') .mapNotNull { it.toIntOrNull() } .let { parts -> parts + List(maxOf(0, 3 - parts.size)) { 0 } } // 최소 메이저/마이너/패치 val (c1, c2, c3) = parse(current) val (l1, l2, l3) = parse(latest) return when { c1 != l1 -> c1 > l1 c2 != l2 -> c2 > l2 c3 != l3 -> c3 > l3 else -> true } }그 후
val isLatest = isUpToDate(state.version, state.latestVersion)
식으로 적용 가능합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/threegap/bitnagil/navigation/home/HomeBottomNavigationBar.kt
(2 hunks)core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/block/BitnagilOptionButton.kt
(1 hunks)core/designsystem/src/main/res/drawable/ic_setting.xml
(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/mypage/MyPageScreen.kt
(3 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt
(2 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/component/atom/settingtitle/SettingTitle.kt
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
presentation/src/main/java/com/threegap/bitnagil/presentation/mypage/MyPageScreen.kt (1)
core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/atom/BitnagilIcon.kt (1)
BitnagilIconButton
(35-58)
core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/block/BitnagilOptionButton.kt (1)
core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/atom/BitnagilIcon.kt (1)
BitnagilIcon
(21-33)
⏰ 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
🔇 Additional comments (13)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/component/atom/settingtitle/SettingTitle.kt (1)
18-18
: 색상 업데이트가 디자인 시스템과 일관성을 유지합니다.coolGray60으로의 색상 변경이 전체 PR의 디자인 리뉴얼과 일치하며, 텍스트 가독성을 적절히 유지합니다.
app/src/main/java/com/threegap/bitnagil/navigation/home/HomeBottomNavigationBar.kt (2)
14-14
: Import 추가가 적절합니다.HorizontalDivider 컴포넌트 사용을 위한 import가 올바르게 추가되었습니다.
34-63
: 하단 네비게이션 바 상단에 구분선 추가가 UI 개선에 기여합니다.Column으로 래핑하여 HorizontalDivider를 추가한 구조가 깔끔하며, 기존 네비게이션 로직은 그대로 유지되었습니다. 구분선 스타일(1dp 두께, coolGray98 색상)이 디자인 시스템과 일치합니다.
core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/block/BitnagilOptionButton.kt (2)
34-35
: 텍스트 스타일 업데이트가 디자인 리뉴얼을 반영합니다.색상을 coolGray10으로, 타이포그래피를 body1Medium으로 변경한 것이 전체 디자인 시스템의 일관성을 유지합니다.
40-41
: 아이콘 업데이트가 새로운 디자인 가이드라인을 따릅니다.ic_chevron_right_md로의 아이콘 변경과 coolGray10 색상 적용이 적절하며, 전체 UI의 일관성을 향상시킵니다.
presentation/src/main/java/com/threegap/bitnagil/presentation/mypage/MyPageScreen.kt (3)
6-6
: PaddingValues import 추가가 적절합니다.BitnagilIconButton의 paddingValues 파라미터 사용을 위한 import가 올바르게 추가되었습니다.
29-29
: BitnagilIconButton import로 변경이 적절합니다.기존 BitnagilIcon에서 BitnagilIconButton으로 변경하여 더 표준화된 버튼 컴포넌트를 사용합니다.
71-76
: 아이콘 버튼 구현이 개선되었습니다.BitnagilIconButton으로 변경하면서 onClick 핸들러가 명시적으로 분리되고, paddingValues를 통해 패딩 관리가 더 명확해졌습니다. tint를 null로 설정하여 원본 아이콘 색상을 사용하는 것도 적절합니다.
core/designsystem/src/main/res/drawable/ic_setting.xml (4)
6-8
: 새로운 fill path 추가가 아이콘 디자인을 개선합니다.#878A93 색상으로 기어 외곽을 채우는 새로운 path가 추가되어 아이콘의 시각적 완성도가 향상되었습니다.
11-15
: 기존 stroke path의 색상 업데이트가 일관성을 유지합니다.strokeColor를 #878A93으로 변경하여 새로운 디자인 시스템 색상 팔레트와 일치시켰습니다.
16-18
: 내부 디테일 path 추가가 아이콘의 완성도를 높입니다.#ffffff 색상의 내부 디테일 path가 추가되어 기어 아이콘의 세밀한 표현이 개선되었습니다.
21-25
: 마지막 path의 색상 통일이 적절합니다.strokeColor를 #878A93으로 변경하여 전체 아이콘의 색상 일관성을 유지했습니다.
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt (1)
119-119
: 타이포그래피(body1Medium) 변경 적절"버전" 레이블에 body1Medium 적용은 정보 계층상 적합해 보입니다. 가독성도 OK.
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt
Show resolved
Hide resolved
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt
Show resolved
Hide resolved
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.
고생하셨습니다!
- 로그아웃 다이얼로그 디자인 수정 - 회원탈퇴 로직 제거
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: 4
♻️ Duplicate comments (2)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt (2)
29-29
: clickableWithoutRipple 사용 시 접근성 개선 필요
clickableWithoutRipple
은 시각적 피드백을 제거하지만 세만틱스 role을 자동으로 설정하지 않습니다. 스크린 리더 사용자를 위해 Role.Button을 명시적으로 설정해야 합니다.#!/bin/bash # clickableWithoutRipple 확장 함수의 role 파라미터 지원 여부 확인 ast-grep --pattern 'fun Modifier.clickableWithoutRipple($_) { $$$ }'
127-139
: 배경색과 클릭 영역이 패딩을 포함하지 않는 문제현재 modifier 순서(background → clickable → padding)로 인해:
- 배경색이 패딩 영역을 포함하지 않아 시각적으로 작게 보임
- 클릭 가능 영역이 패딩을 제외하여 터치 영역이 좁음
또한
clickableWithoutRipple
이enabled
파라미터를 지원하므로 조건부 적용보다 직접 사용하는 것이 더 깔끔합니다.Text( text = if (isLatest) "최신" else "업데이트", color = if (isLatest) BitnagilTheme.colors.coolGray70 else BitnagilTheme.colors.orange500, style = BitnagilTheme.typography.button2, modifier = Modifier + .padding(horizontal = 10.dp, vertical = 5.dp) .background( color = if (isLatest) BitnagilTheme.colors.coolGray98 else BitnagilTheme.colors.orange50, shape = RoundedCornerShape(8.dp), ) - .let { if (!isLatest) it.clickableWithoutRipple(onClick = onClickUpdate) else it } - .padding(horizontal = 10.dp, vertical = 5.dp), + .clickableWithoutRipple( + enabled = !isLatest, + onClick = onClickUpdate + ), )
🧹 Nitpick comments (3)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt (1)
13-13
: ShowConfirmDialog가 파라미터 없이 변경됨에 따른 확장성 고려현재
ShowConfirmDialog
가data object
로 변경되어 로그아웃 전용으로만 사용되고 있습니다. 향후 다른 종류의 확인 다이얼로그(예: 알림 설정 변경 확인)가 필요할 경우를 대비해 파라미터를 유지하거나, 명확하게ShowLogoutConfirmDialog
로 이름을 변경하는 것이 좋겠습니다.- data object ShowConfirmDialog : SettingIntent() + data object ShowLogoutConfirmDialog : SettingIntent()presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (2)
48-54
: ConfirmDialog 의도 vs 상태 필드 네이밍 불일치, 그리고 when 매칭 스타일 일관성
- Intent 이름은 일반화된 ConfirmDialog인데, 상태 필드는 logoutConfirmDialogVisible로 로그아웃 전용입니다. 의도를 맞추려면 둘 중 하나를 통일하는 것이 좋습니다.
- 옵션 A: Intent를 ShowLogoutConfirmDialog/HideLogoutConfirmDialog로 명확히 변경.
- 옵션 B: 상태 필드를 confirmDialogVisible 등 일반화된 이름으로 변경.
- 또한 동일한 파일 내에서 일부 Intent는
is
로, 일부는 값 매칭으로 사용되고 있어 스타일이 섞여 있습니다. 본 케이스는 객체 Intent라 값 매칭으로 통일하는 것이 읽기 쉬워 보입니다.아래는 스타일 통일용(값 매칭) 제안입니다.
- is SettingIntent.ShowConfirmDialog -> { + SettingIntent.ShowConfirmDialog -> { return state.copy(logoutConfirmDialogVisible = true) } - is SettingIntent.HideConfirmDialog -> { + SettingIntent.HideConfirmDialog -> { return state.copy(logoutConfirmDialogVisible = false) }필요 시 Intent/State 명칭을 어느 쪽으로 통일할지 결정 부탁드립니다. 원하시면 관련 파일(SettingIntent, SettingState, SettingScreen)까지 포함한 리팩터링 패치 제안 드리겠습니다.
71-73
: 메서드 페어 네이밍 정합성showLogoutDialog와 hideConfirmDialog가 페어를 이루지 않습니다. 일관성을 위해 hideConfirmDialog를 hideLogoutDialog로 맞추거나, showLogoutDialog를 showConfirmDialog로 맞추는 편이 가독성에 좋습니다. 참고: Intent 이름과도 함께 정렬되면 더 좋습니다.
UI 호출부(SettingScreen 등)에서 해당 메서드명을 어떻게 사용 중인지 점검 부탁드립니다. 원하시면 전역 치환 스크립트 제안 가능합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/atom/BitnagilTextButton.kt
(1 hunks)core/designsystem/src/main/res/drawable/ic_modal_warning.xml
(0 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt
(5 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt
(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/component/block/LogoutConfirmDialog.kt
(3 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/ConfirmDialogType.kt
(0 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt
(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingState.kt
(2 hunks)
💤 Files with no reviewable changes (2)
- presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/ConfirmDialogType.kt
- core/designsystem/src/main/res/drawable/ic_modal_warning.xml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-21T10:38:49.104Z
Learnt from: l5x5l
PR: YAPP-Github/Bitnagil-Android#38
File: presentation/src/main/java/com/threegap/bitnagil/presentation/writeroutine/component/atom/textbutton/TextButton.kt:30-35
Timestamp: 2025-07-21T10:38:49.104Z
Learning: presentation/src/main/java/com/threegap/bitnagil/presentation/writeroutine/component/atom/textbutton/TextButton.kt의 TextButton 컴포넌트는 임시로 구현된 컴포넌트로, 디자인 시스템 구현시 대체 예정입니다.
Applied to files:
core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/atom/BitnagilTextButton.kt
📚 Learning: 2025-07-23T13:32:26.263Z
Learnt from: l5x5l
PR: YAPP-Github/Bitnagil-Android#41
File: presentation/src/main/java/com/threegap/bitnagil/presentation/mypage/model/MyPageIntent.kt:6-6
Timestamp: 2025-07-23T13:32:26.263Z
Learning: In the Bitnagil Android project's MVI architecture, Intent classes like `LoadMyPageSuccess` are named to represent successful API response results that carry loaded data, not just user actions. This naming convention is used for future API integration where the intent will be triggered when my page data loading succeeds from the server.
Applied to files:
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt
📚 Learning: 2025-07-23T13:31:46.809Z
Learnt from: l5x5l
PR: YAPP-Github/Bitnagil-Android#41
File: presentation/src/main/java/com/threegap/bitnagil/presentation/mypage/model/MyPageState.kt:6-14
Timestamp: 2025-07-23T13:31:46.809Z
Learning: In the Bitnagil Android project, MviState interface extends Parcelable, so any class implementing MviState automatically implements Parcelable. Therefore, Parcelize annotation works correctly without explicitly adding Parcelable implementation.
Applied to files:
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingState.kt
🧬 Code Graph Analysis (3)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/common/mviviewmodel/MviViewModel.kt (2)
sendSideEffect
(23-23)sendIntent
(30-37)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/component/block/LogoutConfirmDialog.kt (1)
core/designsystem/src/main/java/com/threegap/bitnagil/designsystem/component/atom/BitnagilTextButton.kt (1)
BitnagilTextButton
(33-82)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/component/block/LogoutConfirmDialog.kt (1)
LogoutConfirmDialog
(23-84)
⏰ 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
🔇 Additional comments (2)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingState.kt (1)
13-13
: LGTM! 로그아웃 다이얼로그 상태 관리 개선
showConfirmDialog: ConfirmDialogType?
에서logoutConfirmDialogVisible: Boolean
로 변경한 것이 좋습니다. 단일 목적의 명확한 상태 관리로 코드 가독성이 향상되었습니다.presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingScreen.kt (1)
165-168
: 탈퇴하기 버튼의 onClick이 빈 람다로 설정됨탈퇴하기 버튼이 UI에는 표시되지만
onClickWithdrawal = {}
로 기능이 구현되지 않았습니다. 사용자에게 혼란을 줄 수 있으므로 기능이 준비되지 않았다면 버튼을 숨기거나 비활성화하는 것이 좋겠습니다.탈퇴 기능이 향후 구현 예정인지, 아니면 이번 리디자인에서 제거된 것인지 확인이 필요합니다. 제거된 기능이라면 UI에서도 완전히 제거해야 합니다.
- BitnagilOptionButton( - title = "탈퇴하기", - onClick = onClickWithdrawal, - ) + // TODO: 탈퇴 기능 구현 예정 + // BitnagilOptionButton( + // title = "탈퇴하기", + // onClick = onClickWithdrawal, + // )
...system/src/main/java/com/threegap/bitnagil/designsystem/component/atom/BitnagilTextButton.kt
Show resolved
Hide resolved
.../main/java/com/threegap/bitnagil/presentation/setting/component/block/LogoutConfirmDialog.kt
Show resolved
Hide resolved
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt
Show resolved
Hide resolved
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt
Show resolved
Hide resolved
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
♻️ Duplicate comments (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (1)
61-67
: 로그아웃 로딩 상태 처리 OK (성공 시 명시적 해제 불필요)LogoutLoading에서 true, LogoutFailure에서 false로 다루는 현재 처리는 합리적입니다. 성공 시에는 네비게이션으로 ViewModel 라이프사이클이 종료되므로 별도 해제가 필요 없다는 이전 논의와 학습 맥락을 확인했습니다.
🧹 Nitpick comments (3)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (3)
48-54
: Show/HideConfirmDialog 분기: 객체 상수 매칭으로 스타일 일관화 제안다른 Intent 분기(
Toggle*
,Logout*
)와 동일하게 객체 상수 매칭 형태로 통일하면 가독성이 좋아집니다. 동작상 차이는 없습니다.- is SettingIntent.ShowConfirmDialog -> { + SettingIntent.ShowConfirmDialog -> { return state.copy(logoutConfirmDialogVisible = true) } - is SettingIntent.HideConfirmDialog -> { + SettingIntent.HideConfirmDialog -> { return state.copy(logoutConfirmDialogVisible = false) }
65-67
: 실패 시 사용자 피드백 SideEffect 추가 고려실패 시 단순히 loading만 false로 두면 사용자에게 원인/상태가 불명확할 수 있습니다. Snackbar/Toast 등으로 에러를 전달하는 SideEffect(예: ShowErrorMessage)를 발행하는 것을 권장합니다. 필요 시 SettingSideEffect에 항목을 추가해 드릴 수 있습니다.
71-73
: 네이밍 일관성 제안: hideConfirmDialog → hideLogoutDialog공개 API가
showLogoutDialog()
인데 반대 동작은hideConfirmDialog()
라 다소 비대칭입니다.hideLogoutDialog()
로 정리하면 의도 파악이 더 쉬워집니다. 파일 내/호출부 전반 rename이 필요하므로 스코프가 크면 유지하셔도 됩니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-13T09:06:19.008Z
Learnt from: wjdrjs00
PR: YAPP-Github/Bitnagil-Android#101
File: presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt:61-67
Timestamp: 2025-08-13T09:06:19.008Z
Learning: In Android ViewModels, when logout success triggers navigation to a different screen (like NavigateToLogin), the current ViewModel's lifecycle ends, so loading states don't need to be explicitly reset in the success case since the ViewModel will be destroyed.
Applied to files:
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt
🧬 Code Graph Analysis (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/common/mviviewmodel/MviViewModel.kt (2)
sendSideEffect
(23-23)sendIntent
(30-37)
⏰ 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
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt
Show resolved
Hide resolved
...entation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt
Outdated
Show resolved
Hide resolved
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.
회원탈퇴 분리 반영사항 확인했습니다!
- ShowConfirmDialog -> ShowLogoutConfirmDialog
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: 0
♻️ Duplicate comments (2)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt (1)
13-13
: 명확한 네이밍 반영 LGTM (ShowLogoutConfirmDialog).이전 피드백대로 명시적 네이밍이 적용되어 의도 전달이 분명해졌습니다. MVI 이벤트 추적 및 유지보수에 도움이 됩니다.
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (1)
79-92
: logoutUseCase 예외 누락 시 무응답 상태 가능성 — try/catch로 보강 필요.logoutUseCase()가 Result 외부로 예외를 던지면 fold 블록에 진입하지 않아 LogoutFailure가 발행되지 않습니다. 이 경우 loading=true가 유지되어 UI가 멈춘 것처럼 보일 수 있습니다. CancellationException만 재던지고, 그 외 Throwable은 LogoutFailure로 전환하세요.
적용 diff:
fun logout() { if (container.stateFlow.value.loading) return sendIntent(SettingIntent.HideConfirmDialog) sendIntent(SettingIntent.LogoutLoading) viewModelScope.launch { - logoutUseCase().fold( - onSuccess = { - sendIntent(SettingIntent.LogoutSuccess) - }, - onFailure = { - sendIntent(SettingIntent.LogoutFailure) - }, - ) + try { + logoutUseCase().fold( + onSuccess = { + sendIntent(SettingIntent.LogoutSuccess) + }, + onFailure = { + sendIntent(SettingIntent.LogoutFailure) + }, + ) + } catch (e: kotlinx.coroutines.CancellationException) { + throw e + } catch (t: Throwable) { + sendIntent(SettingIntent.LogoutFailure) + } } }추가로, 위의 Hide intent를 HideLogoutConfirmDialog로 변경하실 경우, 본 함수 내 사용처도 함께 갱신해야 합니다:
- sendIntent(SettingIntent.HideConfirmDialog) + sendIntent(SettingIntent.HideLogoutConfirmDialog)보완 잘 하신 점: LogoutLoading을 코루틴 시작 전에 동기적으로 설정해 중복 클릭 레이스를 줄이신 점은 매우 좋습니다.
🧹 Nitpick comments (3)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt (1)
13-15
: HideConfirmDialog 명칭을 HideLogoutConfirmDialog로 맞춰 대칭성/명확성 확보 제안.현재 Show는 로그아웃 전용이지만 Hide는 범용처럼 읽혀 혼동 여지가 있습니다. 의도에 맞게 HideLogoutConfirmDialog로 통일하면 의미가 더 분명해집니다.
적용 diff:
- data object HideConfirmDialog : SettingIntent() + data object HideLogoutConfirmDialog : SettingIntent()참고: 해당 리네이밍은 ViewModel/Screen의 사용처도 함께 변경해야 합니다(아래 ViewModel 코멘트에 사용처 diff 포함).
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (2)
48-54
: when 매칭 스타일 통일 + Hide intent 네이밍 대칭화 제안.
- data object는 객체 단일 인스턴스라 when에서
is
보다 상수 매칭(SettingIntent.X ->
)이 더 간결하고 일관적입니다.- ShowLogoutConfirmDialog에 맞춰 Hide도 HideLogoutConfirmDialog로 변경하면 의미가 선명해집니다.
적용 diff:
- is SettingIntent.ShowLogoutConfirmDialog -> { + SettingIntent.ShowLogoutConfirmDialog -> { return state.copy(logoutConfirmDialogVisible = true) } - is SettingIntent.HideConfirmDialog -> { - return state.copy(logoutConfirmDialogVisible = false) - } + SettingIntent.HideLogoutConfirmDialog -> { + return state.copy(logoutConfirmDialogVisible = false) + }참고: SettingIntent.kt의 HideConfirmDialog도 HideLogoutConfirmDialog로 리네이밍 필요(해당 파일 코멘트 참고). 또한 hideConfirmDialog() 호출부도 함께 업데이트 필요합니다(아래 추가 제안 참고).
61-67
: 로딩 상태 전환은 적절함. 실패 UX 보강을 고려해 주세요.
- LogoutLoading/LogoutFailure에서 loading 토글은 적절합니다.
- 실패 시 사용자에게 피드백(토스트/스낵바 등)을 SideEffect로 발행하면 UX가 좋아집니다. 예: SettingSideEffect.ShowErrorMessage("로그아웃에 실패했어요. 잠시 후 다시 시도해주세요.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt
(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-23T13:32:26.263Z
Learnt from: l5x5l
PR: YAPP-Github/Bitnagil-Android#41
File: presentation/src/main/java/com/threegap/bitnagil/presentation/mypage/model/MyPageIntent.kt:6-6
Timestamp: 2025-07-23T13:32:26.263Z
Learning: In the Bitnagil Android project's MVI architecture, Intent classes like `LoadMyPageSuccess` are named to represent successful API response results that carry loaded data, not just user actions. This naming convention is used for future API integration where the intent will be triggered when my page data loading succeeds from the server.
Applied to files:
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt
📚 Learning: 2025-08-13T09:06:19.008Z
Learnt from: wjdrjs00
PR: YAPP-Github/Bitnagil-Android#101
File: presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt:61-67
Timestamp: 2025-08-13T09:06:19.008Z
Learning: In Android ViewModels, when logout success triggers navigation to a different screen (like NavigateToLogin), the current ViewModel's lifecycle ends, so loading states don't need to be explicitly reset in the success case since the ViewModel will be destroyed.
Applied to files:
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt
🧬 Code Graph Analysis (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (1)
presentation/src/main/java/com/threegap/bitnagil/presentation/common/mviviewmodel/MviViewModel.kt (2)
sendSideEffect
(23-23)sendIntent
(30-37)
⏰ 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
🔇 Additional comments (2)
presentation/src/main/java/com/threegap/bitnagil/presentation/setting/model/mvi/SettingIntent.kt (1)
13-13
: Kotlin 1.9+ “data object” 호환성 확인됨
- gradle/libs.versions.toml에서
kotlin = "2.1.20"
로 설정되어 있어 Kotlin 1.9 이상의 기능 사용에 문제가 없습니다.- 별도 조치 없이
data object ShowLogoutConfirmDialog
를 그대로 유지하셔도 됩니다.presentation/src/main/java/com/threegap/bitnagil/presentation/setting/SettingViewModel.kt (1)
71-73
: showLogoutDialog 간소화 LGTM.의도가 명확하고, 새로운 Intent와 상태 모델에 잘 부합합니다.
[ PR Content ]
마이페이지, 설정 화면 리디자인 변경 사항을 반영했습니다.
Related issue
Screenshot 📸
Work Description
To Reviewers 📢
Summary by CodeRabbit
뉴 기능
스타일
변경/정리