설정 화면 미완성 기능 구현#167
Conversation
|
Warning Review limit reached
More reviews will be available in 52 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough이 PR은 설정 화면의 미완성 기능을 구현합니다. 알림 설정 API 및 도메인 모델을 추가하고, 프로필 업데이트를 위한 PATCH 엔드포인트를 추가합니다. 뷰모델은 닉네임 서버 업데이트와 알림 토글 상태를 관리하도록 확장되며, 공통 컴포넌트(CommonSwitch, ValidateUnderlineTextField)가 개선됩니다. 설정 화면은 닉네임 편집, 언어 선택, 문의 링크, 알림 토글을 포함하고, 계정 화면은 초대 코드와 커플 결합 해제 기능을 추가합니다. 새로운 알림 설정 화면이 추가되며 네비게이션 그래프는 사이드이펙트 기반 로그인 처리로 리팩토링됩니다. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 10
🧹 Nitpick comments (1)
core/design-system/src/main/res/drawable/ic_arrow_down_circle.xml (1)
6-15: ⚡ Quick win하드코딩 색상으로 다크모드 대응이 제한됩니다.
왜 문제인가:
#171717,#ffffff하드코딩은 테마 전환 시 아이콘 대비/일관성을 깨뜨릴 수 있습니다.
어떻게 개선할까:@color/...(또는 테마 attr)로 분리해 라이트/다크 팔레트에서 관리해 주세요. 아이콘 tint 전략을 쓰는 방식도 좋습니다.As per coding guidelines
**/res/**: 색상과 테마가 디자인 시스템에 포함되는가?,core/design-system/**: Dark Mode 지원 여부.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/design-system/src/main/res/drawable/ic_arrow_down_circle.xml` around lines 6 - 15, The drawable ic_arrow_down_circle.xml currently hardcodes colors (`#171717` and `#ffffff`) on the two <path> elements which prevents dark-mode theming; update the first path's android:fillColor and the second path's android:strokeColor (and optionally its android:fillColor) to use theme-aware resources (e.g., `@color/`… or theme attrs like ?attr/colorOnBackground or ?attr/colorControlNormal) or convert the drawable to be tintable and rely on ImageView/AppCompatImageView tinting; modify the path attributes in ic_arrow_down_circle.xml (replace the literal hex values on the two <path> elements) and ensure corresponding colors/attrs are added to the design-system color palette so light/dark variants are available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/design-system/src/main/java/com/twix/designsystem/components/text_field/ValidateUnderlineTextField.kt`:
- Line 84: isValidLength currently uses internalValue.length but the commit
check (around the code that trims input at line ~61) uses internalValue.trim(),
causing mismatch; update the length checks (the isValidLength at
ValidateUnderlineTextField.kt line 84 and the analogous check around line 127)
to use internalValue.trim().length and compare that against validLengthRange so
the guide and commit validation use the same trimmed-string criterion.
- Around line 118-122: Clear 버튼인 Image에 contentDescription이 null로 설정되어 있어 스크린리더가
기능을 알리지 못합니다; noRippleClickable을 사용하는 해당 Image의 contentDescription을 null 대신
로컬라이즈된 설명(예: string resource "clear_text" 또는 "clear input")으로 설정하고, 필요하면 동작을
설명하는 문구(예: "입력 지우기")를 사용해 접근성을 보장하세요. 또한 Image를 담당하는 컴포저블(예: the Image with
modifier = Modifier.noRippleClickable { internalValue = "" })에서
contentDescription이 로컬라이즈된 리소스를 사용하도록 교체하고, 만약 시맨틱 개선이 더 필요하면 semantics/role 등을
추가해 버튼임을 명시하세요.
In `@core/design-system/src/main/res/values/strings.xml`:
- Around line 113-115: Update the three string resources to use the corrected,
consistent wording "푸시 알림": change settings_poke_push_notification value to "찌르기
푸시 알림", settings_marketing_push_notification to "마케팅 정보 푸시 알림", and
settings_night_marketing_push_notification to "야간 마케팅 정보 푸시 알림" so all labels
use the same "푸시 알림" phrasing.
In `@core/util/src/main/java/com/twix/util/extension/ContextExt.kt`:
- Around line 17-23: The Intent created for opening external URLs (val intent =
Intent(Intent.ACTION_VIEW, url.toUri()) ... then startActivity(intent)) must
include Intent.FLAG_ACTIVITY_NEW_TASK so startActivity works from non-Activity
contexts; update the intent construction to call
addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) (or addFlag) before startActivity and
verify callers of this Context extension guarantee an Activity context if you
choose not to add the flag. Ensure you still keep
addCategory(Intent.CATEGORY_BROWSABLE) and handle the try/catch around
startActivity unchanged.
In `@feature/goal-editor/src/main/java/com/twix/goal_editor/GoalEditorScreen.kt`:
- Around line 217-225: ValidateUnderlineTextField's commit behavior delays
updating uiState.goalTitle (and thus GoalEditorUiState.isSaveEnabled /
AppButton(enabled = uiState.canSave)) until focus loss/IME Done and only when
trimmed.length in its validLengthRange, which can break real-time "save"
enablement and silently enforce a default 2..14 max; update the GoalEditorScreen
call to either (a) pass a separate realtime validity callback from
ValidateUnderlineTextField to drive canSave while keeping onCommitTitle for
final SetTitle, or (b) explicitly set validLengthRange to the intended bounds
(e.g., 2..8) so onCommit semantics match prior behavior, and remove the
duplicated modifier.height(96.dp) on the call to avoid double-sizing.
In
`@feature/settings/src/main/java/com/twix/settings/account/SettingsAccountScreen.kt`:
- Around line 50-57: The onUnlinkCouple handler in SettingsAccountScreen is
wired to viewModel.dispatch(SettingsIntent.WithdrawAccount) which will delete
the account while the UI/dialogue indicate only unlinking; fix by either wiring
onUnlinkCouple to the correct intent (SettingsIntent.UnlinkCouple) once
available, or temporarily hide/disable the unlink UI and remove its callbacks
(or change labels/dialog copy to reflect account withdrawal) so users cannot
accidentally trigger WithdrawAccount; update all occurrences where
onUnlinkCouple or the Unlink menu/dialog (lines around the current mapping and
the other spots called out) are wired to WithdrawAccount and ensure the
viewModel.dispatch call uses the correct intent or is removed until the API
exists.
In `@feature/settings/src/main/java/com/twix/settings/component/ProfileInfo.kt`:
- Around line 39-45: Summary: Clickable profile/edit Image composables lack
proper accessibility labels and use hardcoded text. Fix: In ProfileInfo.kt
update the Image composables (the Image call around the profile icon and the
Image used for the edit icon at lines ~77-85) to use string resources via
stringResource(R.string.profile_icon) and
stringResource(R.string.edit_profile_icon) for contentDescription when the icon
conveys meaning, and set contentDescription = null only for purely decorative
images; ensure the edit icon (which is clickable) gets a non-null localized
description. Add R.string entries (profile_icon, edit_profile_icon) to
strings.xml and use contentDescription = stringResource(...) in the Image
composables.
In `@feature/settings/src/main/java/com/twix/settings/model/SettingsLanguage.kt`:
- Around line 4-7: Change the SettingsLanguage enum to stop embedding UI
strings: remove the displayName property and any hard-coded display values
(e.g., the Korean("한국어") entry) and instead give each enum a stable identifier
field (e.g., languageTag or localeId like "ko" or "ko-KR") that the UI can map
to string resources; update references to SettingsLanguage.displayName to use
the new identifier (e.g., SettingsLanguage.languageTag) and move all user-facing
text lookup into the UI/resource layer.
In `@feature/settings/src/main/java/com/twix/settings/SettingsScreen.kt`:
- Around line 154-161: The language selection UI is hardcoded to
SettingsLanguage.Korean and not wired to state or intents; lift the selected
language into the immutable SettingsUiState (e.g., add a selectedLanguage:
SettingsLanguage property), define a clear Intent such as
ChangeLanguageIntent(selected: SettingsLanguage) and/or
OpenLanguageDialogIntent/ConfirmLanguageIntent to represent user actions, update
SettingsMenuItem/LanguageMenuRight to read and display state.selectedLanguage
instead of SettingsLanguage.Korean, make showLanguageDialog dispatch
OpenLanguageDialogIntent on click, have the dialog’s selection bind to
state.selectedLanguage (or a temporary dialogSelection in state) and on confirm
dispatch ConfirmLanguageIntent/ChangeLanguageIntent to update state and apply
the app language change. Ensure SettingsUiState is an immutable data class and
all user interactions use the new Intent types so selection, dialog state, and
actual language application are synchronized.
In `@feature/settings/src/main/java/com/twix/settings/SettingsViewModel.kt`:
- Around line 90-94: The onError branch of the launchResult call currently only
restores the nickName (reduce { copy(nickName = originalNickName) }) but does
not surface the failure to the user; update the onError handler for launchResult
(the call that invokes onBoardingRepository.updateProfile) to both restore the
nickName and emit a user-visible failure event — e.g., after reduce {
copy(nickName = originalNickName) } also dispatch a toast/alert UI event (use
your ViewModel’s event emitter such as a sendViewEffect/showToast/sendUiEvent
function or set an errorMessage field in state) so the user sees a failure
message explaining why the change was reverted.
---
Nitpick comments:
In `@core/design-system/src/main/res/drawable/ic_arrow_down_circle.xml`:
- Around line 6-15: The drawable ic_arrow_down_circle.xml currently hardcodes
colors (`#171717` and `#ffffff`) on the two <path> elements which prevents dark-mode
theming; update the first path's android:fillColor and the second path's
android:strokeColor (and optionally its android:fillColor) to use theme-aware
resources (e.g., `@color/`… or theme attrs like ?attr/colorOnBackground or
?attr/colorControlNormal) or convert the drawable to be tintable and rely on
ImageView/AppCompatImageView tinting; modify the path attributes in
ic_arrow_down_circle.xml (replace the literal hex values on the two <path>
elements) and ensure corresponding colors/attrs are added to the design-system
color palette so light/dark variants are available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 19fb7941-6a23-4ba5-9cee-2fd75481c95f
📒 Files selected for processing (33)
app/build.gradle.ktscore/design-system/src/main/java/com/twix/designsystem/components/common/CommonSwitch.ktcore/design-system/src/main/java/com/twix/designsystem/components/text_field/ValidateUnderlineTextField.ktcore/design-system/src/main/res/drawable/ic_arrow_down_circle.xmlcore/design-system/src/main/res/drawable/ic_language.xmlcore/design-system/src/main/res/drawable/ic_notification.xmlcore/design-system/src/main/res/drawable/ic_question.xmlcore/design-system/src/main/res/values/strings.xmlcore/navigation/src/main/java/com/twix/navigation/NavRoutes.ktcore/network/src/main/java/com/twix/network/model/request/notification/UpdateNotificationSettingRequest.ktcore/network/src/main/java/com/twix/network/model/response/notification/mapper/NotificationMapper.ktcore/network/src/main/java/com/twix/network/model/response/notification/model/NotificationSettingsResponse.ktcore/network/src/main/java/com/twix/network/service/NotificationService.ktcore/network/src/main/java/com/twix/network/service/OnboardingService.ktcore/util/src/main/java/com/twix/util/extension/ContextExt.ktdata/src/main/java/com/twix/data/repository/DefaultNotificationRepository.ktdata/src/main/java/com/twix/data/repository/DefaultOnboardingRepository.ktdomain/src/main/java/com/twix/domain/model/notification/NotificationSettings.ktdomain/src/main/java/com/twix/domain/repository/NotificationRepository.ktdomain/src/main/java/com/twix/domain/repository/OnBoardingRepository.ktfeature/goal-editor/src/main/java/com/twix/goal_editor/GoalEditorScreen.ktfeature/main/src/main/java/com/twix/main/navigation/MainNavGraph.ktfeature/settings/build.gradle.ktsfeature/settings/src/main/java/com/twix/settings/SettingsIntent.ktfeature/settings/src/main/java/com/twix/settings/SettingsScreen.ktfeature/settings/src/main/java/com/twix/settings/SettingsViewModel.ktfeature/settings/src/main/java/com/twix/settings/account/SettingsAccountScreen.ktfeature/settings/src/main/java/com/twix/settings/component/ProfileInfo.ktfeature/settings/src/main/java/com/twix/settings/model/SettingMenuUiModel.ktfeature/settings/src/main/java/com/twix/settings/model/SettingsLanguage.ktfeature/settings/src/main/java/com/twix/settings/model/SettingsUiState.ktfeature/settings/src/main/java/com/twix/settings/navigation/SettingsNavGraph.ktfeature/settings/src/main/java/com/twix/settings/notification/SettingsNotificationScreen.kt
chanho0908
left a comment
There was a problem hiding this comment.
고생했어 현수야 !!
카카오톡 오픈채팅 주소한 한번 슬랙으로 공유 부탁해 :)
이슈 번호
작업내용
리뷰어에게 추가로 요구하는 사항 (선택)
스토어 최신 버전은 아직 api가 완성되지 않아서 구현하지 않았고 커플 끊기는 ios와 동일하게 탈퇴로 처리하도록 구현했습니다! 저번에 구현하신 공용 로딩, 에러 처리가 잘 반영됐는지도 한번 확인해 주세요