Conversation
Closed
Contributor
📝 WalkthroughWalkthrough로그인 흐름을 약관(terms) 화면으로 라우팅하도록 네비게이션·뷰모델·계약을 추가·변경했고, 약관 화면(및 상세 모달)과 관련된 UI/상태/이펙트를 새로 도입했습니다. 디자인시스템에서는 Changes
Possibly related PRs
🚥 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.kt (1)
89-103: 미사용 변수 및 함수 네이밍 개선
Line 103의
resources변수가 선언되었지만 사용되지 않습니다. Line 121에서resources.getString()을 사용하지만, 이는stringResource()로 대체 가능합니다.
TermsScreenScreen함수명에 "Screen"이 중복됩니다.TermsScreenContent또는TermsContent로 변경하는 것이 좋습니다.♻️ 리팩토링 제안
`@OptIn`(ExperimentalMaterial3Api::class) `@Composable` -private fun TermsScreenScreen( +private fun TermsScreenContent( uiState: TermsUiState, onBack: () -> Unit, ... modifier: Modifier = Modifier, ) { - val resources = LocalResources.current - Column(modifier = modifier.fillMaxSize()) { TermsScreenTopAppBar(onBack = onBack) ... PrezelButtonArea { MainButton( - label = resources.getString(R.string.feature_login_impl_terms_continue_button_text), + label = stringResource(R.string.feature_login_impl_terms_continue_button_text), enabled = uiState.isRequiredChecked, onClick = onContinue, ) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.kt` around lines 89 - 103, The composable TermsScreenScreen has an unused LocalResources variable (resources) and its name duplicates "Screen"; remove the unused resources declaration and replace any resources.getString(...) calls with Compose's stringResource(...) usage, then rename the function TermsScreenScreen to a clearer name such as TermsScreenContent or TermsContent and update all call sites to the new name (search for TermsScreenScreen to refactor references).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelCheckbox.kt`:
- Around line 40-52: The component lost accessibility role semantics when
switching to PrezelTouchArea; ensure the checkbox exposes Role.Checkbox to
screen readers by updating the touch area’s modifier (in PrezelCheckbox / where
PrezelTouchArea is used) to include checkbox semantics—either replace or augment
the modifier with a toggleable(role = Role.Checkbox) or add Modifier.semantics {
role = Role.Checkbox; stateDescription = if (checked) "checked" else "unchecked"
} so TalkBack recognizes the control and its checked state; apply this change on
the PrezelTouchArea invocation that currently wraps the Icon.
In `@Prezel/feature/login/impl/build.gradle.kts`:
- Around line 14-17: The build fails if privacy/terms keys are missing because
buildConfigField currently calls localProperty(...).get() directly; change to
use the Provider.map pattern so missing properties don't throw (e.g. replace
direct .get() with localProperty("privacy.policy.url").map { value ->
"\"${value}\"" }.orElse("\"\"") or provide a safe default), updating the two
buildConfigField calls for PRIVACY_POLICY_URL and TERMS_OF_SERVICE_URL inside
defaultConfig to use the mapped Provider rather than .get(); reference the
buildConfigField and localProperty usages to locate the spots to change.
In
`@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginScreen.kt`:
- Around line 29-31: The import
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel is deprecated; replace
it with androidx.hilt.navigation.compose.hiltViewModel in the LoginScreen file
so usages of hiltViewModel() (e.g., in composable functions or where ViewModel
is obtained) reference the navigation-compatible provider; update the import
line and verify references to hiltViewModel() still resolve (no other code
changes needed).
In
`@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginViewModel.kt`:
- Around line 51-57: In LoginViewModel, when BuildConfig.DEBUG short-circuits to
LoginUiEffect.NavigateToTerms the isLoading flag remains true because
handleLoginResult isn't invoked; update the DEBUG branch to clear or reset the
loading state (set isLoading = false or call the existing
handleLoginResult/resetLoading method) before sending
LoginUiEffect.NavigateToTerms so the UI button is re-enabled when returning,
ensuring the same state transition used by the LaunchLogin/provider path is
applied.
In
`@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/component/TermsDetailModal.kt`:
- Around line 84-106: The rememberWebView currently creates a WebView but never
cleans it up; wrap the created WebView in a DisposableEffect inside
rememberWebView (add import androidx.compose.runtime.DisposableEffect) and in
onDispose call proper cleanup on the WebView instance (stopLoading(),
removeAllViews(), clearHistory(), and destroy()) so the WebView created in
remember(url) is destroyed when the composable leaves composition; reference the
rememberWebView function and the WebView instance returned (and keep your
existing webViewClient/NOTION_STYLE_PATCH injection intact).
- Around line 62-68: The AndroidView update lambda is forcing a reload every
recomposition via view.url?.let(view::loadUrl), causing unnecessary network
calls and flicker; remove the update parameter entirely (since the WebView is
recreated with remember(url) / rememberWebView and initial loadUrl(url) is
already called) or change the update to only load when the incoming url differs
from view.url (compare the new url param to view.url before calling loadUrl).
Target the AndroidView usage and its update block in TermsDetailModal.kt and
adjust accordingly.
- Around line 41-50: PrezelTopAppBar is being used without passing windowInsets
so it defaults to WindowInsets() and can overlap the status bar in the
full-screen modal; fix by either passing a status-bar-aware insets instance
(e.g., windowInsets = WindowInsets.statusBars()) into PrezelTopAppBar or by
applying status bar padding to the modal content (e.g., add
Modifier.statusBarsPadding() to the container that uses fillMaxSize), ensuring
the app bar (PrezelTopAppBar) no longer overlaps the system status bar.
In
`@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.kt`:
- Line 29: The file imports the deprecated symbol
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel; replace that import
with androidx.hilt.navigation.compose.hiltViewModel and keep existing usages of
hiltViewModel in TermsScreen (or any ViewModel retrieval calls) unchanged so
they resolve to the non-deprecated function from the navigation.compose package.
- Around line 82-83: BuildConfig URLs can be null/empty because localProperty()
may return null; update the click handlers and the consumer to validate URLs
before use: when setting activeDetailUrl from BuildConfig.TERMS_OF_SERVICE_URL
or BuildConfig.PRIVACY_POLICY_URL, guard against null/blank and either set
activeDetailUrl to a safe fallback (e.g., empty string / show error state) or
disable opening the TermsDetailModal; also add validation inside
TermsDetailModal/NotionWebView to check the url passed to
NotionWebView.loadUrl(url) and skip loading (and surface a user-friendly error)
if the URL is null/blank. Ensure you reference activeDetailUrl,
BuildConfig.TERMS_OF_SERVICE_URL, BuildConfig.PRIVACY_POLICY_URL,
TermsDetailModal, and NotionWebView.loadUrl when making these changes.
---
Nitpick comments:
In
`@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.kt`:
- Around line 89-103: The composable TermsScreenScreen has an unused
LocalResources variable (resources) and its name duplicates "Screen"; remove the
unused resources declaration and replace any resources.getString(...) calls with
Compose's stringResource(...) usage, then rename the function TermsScreenScreen
to a clearer name such as TermsScreenContent or TermsContent and update all call
sites to the new name (search for TermsScreenScreen to refactor references).
🪄 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.yaml
Review profile: CHILL
Plan: Pro
Run ID: c2148e13-9bcb-4425-ac51-c2803ba9a483
📒 Files selected for processing (21)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelAccordion.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelCheckbox.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/TopAppBar.ktPrezel/feature/login/impl/build.gradle.ktsPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginScreen.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginViewModel.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiEffect.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiIntent.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiState.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/model/LoginUiMessage.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/model/LoginUiMessage.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/navigation/LoginEntryBuilder.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/navigation/LoginTermsNavKey.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsViewModel.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/component/TermsDetailModal.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiEffect.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiIntent.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiState.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/viewModel/LoginUiState.ktPrezel/feature/login/impl/src/main/res/values/strings.xml
💤 Files with no reviewable changes (2)
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/model/LoginUiMessage.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/viewModel/LoginUiState.kt
...ore/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelCheckbox.kt
Show resolved
Hide resolved
...l/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginScreen.kt
Show resolved
Hide resolved
...eature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginViewModel.kt
Show resolved
Hide resolved
...in/impl/src/main/java/com/team/prezel/feature/login/impl/terms/component/TermsDetailModal.kt
Show resolved
Hide resolved
...in/impl/src/main/java/com/team/prezel/feature/login/impl/terms/component/TermsDetailModal.kt
Show resolved
Hide resolved
...in/impl/src/main/java/com/team/prezel/feature/login/impl/terms/component/TermsDetailModal.kt
Show resolved
Hide resolved
Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.kt
Show resolved
Hide resolved
Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.kt
Show resolved
Hide resolved
`PrezelCheckbox` 컴포넌트에 접근성 지원을 위한 시맨틱 정보를 추가하였습니다. * `modifier`에 `Role.Checkbox`를 지정하여 컴포넌트의 역할을 명시했습니다. * `ToggleableState`를 통해 현재 체크 상태(`checked`)를 시맨틱 트리에 반영했습니다.
* refactor: 디버그 환경에서 로그인 시 로딩 상태를 해제하도록 수정 MVP 개발용 디버그 로직에서 약관 화면(`NavigateToTerms`)으로 이동하기 전, `isLoading` 상태를 `false`로 업데이트하여 로딩 인디케이터가 남지 않도록 개선했습니다.
moondev03
force-pushed
the
feat/#89-약관동의-화면-구현
branch
from
eb906a2 to
52177dd
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/TopAppBar.kt (1)
31-43:⚠️ Potential issue | 🟠 Major기본값
WindowInsets()는 기존 호출부의 동작을 깨뜨립니다.Material3
TopAppBar의 기본windowInsets는TopAppBarDefaults.windowInsets로, 상태바 등 시스템 인셋을 올바르게 처리합니다. 하지만 현재WindowInsets()는 빈 인셋을 생성하므로,windowInsets파라미터를 전달하지 않는 기존 호출부들(TermsScreen,TermsDetailModal,PrezelDatePicker)에서 상태바와 TopAppBar 콘텐츠가 겹칠 수 있습니다.모달 내부에서 인셋을 제거해야 하는 경우에는 해당 호출부에서 명시적으로
WindowInsets()를 전달하는 것이 바람직합니다.🛠️ 기존 동작을 유지하면서 커스텀 인셋을 허용하는 수정 제안
`@Composable` fun PrezelTopAppBar( modifier: Modifier = Modifier, title: `@Composable` () -> Unit = {}, leadingIcon: `@Composable` () -> Unit = {}, trailingIcons: `@Composable` RowScope.() -> Unit = {}, scrollBehavior: TopAppBarScrollBehavior? = null, - windowInsets: WindowInsets = WindowInsets(), + windowInsets: WindowInsets = TopAppBarDefaults.windowInsets, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/TopAppBar.kt` around lines 31 - 43, 기본 파라미터 windowInsets가 WindowInsets()로 설정돼 있어 기존 호출부의 동작을 깨뜨리므로 TopAppBar 컴포저블의 파라미터 기본값을 TopAppBarDefaults.windowInsets로 변경하고, 모달 등에서 인셋을 제거해야 하는 경우에는 해당 호출부(TermsScreen, TermsDetailModal, PrezelDatePicker 등)에 명시적으로 WindowInsets()를 전달하도록 수정하세요; 관련 식별자: TopAppBar (컴포저블), windowInsets 파라미터, TopAppBarDefaults.windowInsets, 호출부 예시 TermsScreen/TermsDetailModal/PrezelDatePicker.
🧹 Nitpick comments (1)
Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginScreen.kt (1)
90-93:animatedVisibilityScope계약은 시그니처에 드러내는 편이 안전합니다.Line 92처럼 내부에서 scope를 조회하도록 바뀌면, 이 화면이 어떤 호스트 전제조건을 필요로 하는지 호출 지점에서 바로 드러나지 않습니다. 이전처럼 파라미터로 유지하면 재사용 지점과 테스트 코드에서 계약이 더 명확해집니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginScreen.kt` around lines 90 - 93, The LoginScreen composable should explicitly accept animatedVisibilityScope as a parameter rather than querying LocalNavAnimatedContentScope.current inside; update the LoginScreen signature to include an animatedVisibilityScope parameter (e.g., animatedVisibilityScope: NavAnimatedContentScopeType) and restore the call site to pass LocalNavAnimatedContentScope.current (as shown in the diff) so the host requirement is visible to callers; update any other usages and tests of LoginScreen to provide the scope and remove internal accesses to LocalNavAnimatedContentScope.current within LoginScreen.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/TopAppBar.kt`:
- Around line 31-43: 기본 파라미터 windowInsets가 WindowInsets()로 설정돼 있어 기존 호출부의 동작을
깨뜨리므로 TopAppBar 컴포저블의 파라미터 기본값을 TopAppBarDefaults.windowInsets로 변경하고, 모달 등에서 인셋을
제거해야 하는 경우에는 해당 호출부(TermsScreen, TermsDetailModal, PrezelDatePicker 등)에 명시적으로
WindowInsets()를 전달하도록 수정하세요; 관련 식별자: TopAppBar (컴포저블), windowInsets 파라미터,
TopAppBarDefaults.windowInsets, 호출부 예시
TermsScreen/TermsDetailModal/PrezelDatePicker.
---
Nitpick comments:
In
`@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginScreen.kt`:
- Around line 90-93: The LoginScreen composable should explicitly accept
animatedVisibilityScope as a parameter rather than querying
LocalNavAnimatedContentScope.current inside; update the LoginScreen signature to
include an animatedVisibilityScope parameter (e.g., animatedVisibilityScope:
NavAnimatedContentScopeType) and restore the call site to pass
LocalNavAnimatedContentScope.current (as shown in the diff) so the host
requirement is visible to callers; update any other usages and tests of
LoginScreen to provide the scope and remove internal accesses to
LocalNavAnimatedContentScope.current within LoginScreen.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2be57404-4728-447c-b9e8-6885e0707cd0
📒 Files selected for processing (21)
Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelAccordion.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelCheckbox.ktPrezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/TopAppBar.ktPrezel/feature/login/impl/build.gradle.ktsPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginScreen.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/LoginViewModel.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiEffect.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiIntent.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiState.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/model/LoginUiMessage.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/model/LoginUiMessage.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/navigation/LoginEntryBuilder.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/navigation/LoginTermsNavKey.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsViewModel.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/component/TermsDetailModal.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiEffect.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiIntent.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiState.ktPrezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/viewModel/LoginUiState.ktPrezel/feature/login/impl/src/main/res/values/strings.xml
💤 Files with no reviewable changes (2)
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/viewModel/LoginUiState.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/model/LoginUiMessage.kt
✅ Files skipped from review due to trivial changes (7)
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/navigation/LoginTermsNavKey.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiState.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiIntent.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/model/LoginUiMessage.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiEffect.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiIntent.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/contract/TermsUiState.kt
🚧 Files skipped from review as they are similar to previous changes (8)
- Prezel/feature/login/impl/build.gradle.kts
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/landing/contract/LoginUiEffect.kt
- Prezel/core/designsystem/src/main/java/com/team/prezel/core/designsystem/component/PrezelCheckbox.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/navigation/LoginEntryBuilder.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsViewModel.kt
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsScreen.kt
- Prezel/feature/login/impl/src/main/res/values/strings.xml
- Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/component/TermsDetailModal.kt
* refactor: `TermsUiIntent` 네이밍 변경 `TermsUiIntent`의 각 항목에서 불필요한 `On` 접두사를 제거하여 네이밍을 간결하게 수정했습니다. - `OnToggleAll` -> `ToggleAll` - `OnToggleTermsOfService` -> `ToggleTermsOfService` - `OnTogglePrivacyPolicy` -> `TogglePrivacyPolicy` - `OnToggleMarketingConsent` -> `ToggleMarketingConsent` - `OnClickContinue` -> `ClickContinue` * refactor: TermsViewModel 코드 포맷팅 및 구조 정리 - `TermsViewModel` 생성자 및 내부 메서드의 들여쓰기(Indentation)를 수정하여 가독성을 개선했습니다. - 변경된 `TermsUiIntent` 명칭에 맞춰 `onIntent` 내 `when` 분기 로직을 업데이트했습니다. * refactor: TermsScreen 내 Intent 호출부 업데이트 - `TermsScreen` 컴포저블에서 `ViewModel`로 Intent를 전달하는 코드를 신규 네이밍 규칙에 맞게 수정했습니다.
* refactor: `TermsUiIntent` 네이밍 변경 `TermsUiIntent`의 각 항목에서 불필요한 `On` 접두사를 제거하여 네이밍을 간결하게 수정했습니다. - `OnToggleAll` -> `ToggleAll` - `OnToggleTermsOfService` -> `ToggleTermsOfService` - `OnTogglePrivacyPolicy` -> `TogglePrivacyPolicy` - `OnToggleMarketingConsent` -> `ToggleMarketingConsent` - `OnClickContinue` -> `ClickContinue` * refactor: TermsViewModel 코드 포맷팅 및 구조 정리 - `TermsViewModel` 생성자 및 내부 메서드의 들여쓰기(Indentation)를 수정하여 가독성을 개선했습니다. - 변경된 `TermsUiIntent` 명칭에 맞춰 `onIntent` 내 `when` 분기 로직을 업데이트했습니다. * refactor: TermsScreen 내 Intent 호출부 업데이트 - `TermsScreen` 컴포저블에서 `ViewModel`로 Intent를 전달하는 코드를 신규 네이밍 규칙에 맞게 수정했습니다.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsViewModel.kt (2)
44-54: 상태 읽기를update블록 내부로 이동 고려 (선택사항)
currentState.isAllChecked를update블록 외부에서 읽고 있어, 이론적으로 읽기와 업데이트 사이에 상태가 변경될 수 있습니다. 실제로 UI 이벤트는 순차적으로 처리되어 문제가 될 가능성은 낮지만, 더 안전한 패턴을 원하신다면:♻️ 더 안전한 패턴 제안
private fun toggleAll() { - val newChecked = !currentState.isAllChecked - update { + val newChecked = !isAllChecked copy( isTermsOfServiceChecked = newChecked, isPrivacyPolicyChecked = newChecked, isMarketingConsentChecked = newChecked, ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsViewModel.kt` around lines 44 - 54, 현재 toggleAll()에서 currentState.isAllChecked를 update 블록 외부에서 읽고 있어 상태가 중간에 바뀔 가능성이 있으니, 읽기를 update의 인자로 전달된 상태 내부로 이동해 안전하게 처리하세요; 즉 toggleAll()의 update { ... } 호출을 state -> copy(...) 형태로 사용하고, newChecked 값을 외부가 아닌 전달된 state.isAllChecked로 계산하도록 변경해 현재의 currentState, toggleAll, update 식별자를 사용해 수정하세요.
27-28: UI Effect 채널에 버퍼 추가 권장기본
Channel()은 RENDEZVOUS 용량(0)을 사용하므로, 수신자가 준비되지 않은 상태에서send()를 호출하면 suspend됩니다. Configuration change 중에 이벤트가 지연되거나 유실될 수 있습니다.♻️ 버퍼 추가 제안
- private val _uiEffect = Channel<TermsUiEffect>() + private val _uiEffect = Channel<TermsUiEffect>(Channel.BUFFERED)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsViewModel.kt` around lines 27 - 28, The _uiEffect Channel is created with the default RENDEZVOUS (0) capacity which can suspend senders and risk lost/delayed events during configuration changes; change the declaration to create a buffered channel (e.g., use Channel(Channel.BUFFERED) or Channel(1)) and keep uiEffect as _uiEffect.receiveAsFlow() so senders can use _uiEffect.trySend(...) or _uiEffect.send(...) without unexpected suspension; update references to _uiEffect and uiEffect in TermsViewModel accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsViewModel.kt`:
- Around line 44-54: 현재 toggleAll()에서 currentState.isAllChecked를 update 블록 외부에서
읽고 있어 상태가 중간에 바뀔 가능성이 있으니, 읽기를 update의 인자로 전달된 상태 내부로 이동해 안전하게 처리하세요; 즉
toggleAll()의 update { ... } 호출을 state -> copy(...) 형태로 사용하고, newChecked 값을 외부가
아닌 전달된 state.isAllChecked로 계산하도록 변경해 현재의 currentState, toggleAll, update 식별자를
사용해 수정하세요.
- Around line 27-28: The _uiEffect Channel is created with the default
RENDEZVOUS (0) capacity which can suspend senders and risk lost/delayed events
during configuration changes; change the declaration to create a buffered
channel (e.g., use Channel(Channel.BUFFERED) or Channel(1)) and keep uiEffect as
_uiEffect.receiveAsFlow() so senders can use _uiEffect.trySend(...) or
_uiEffect.send(...) without unexpected suspension; update references to
_uiEffect and uiEffect in TermsViewModel accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c045f2ab-652b-480f-ad90-22cb00ec7c6c
📒 Files selected for processing (1)
Prezel/feature/login/impl/src/main/java/com/team/prezel/feature/login/impl/terms/TermsViewModel.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 작업 내용
Login -> Terms -> Home흐름으로 연결landing,terms,navigation기준으로 재정리BuildConfig설정 추가PrezelCheckbox,PrezelAccordion,PrezelTopAppBar)를 보강🧩 관련 이슈
📸 스크린샷
Screen_recording_20260402_233753.webm
📢 논의하고 싶은 내용
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항
기타