-
Notifications
You must be signed in to change notification settings - Fork 0
๐(::63) SignIn page๋ฅผ ํผ๋ธ๋ฆฌ์ฑ ํ์ต๋๋ค. #64
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
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough์ด PR์ ์ฌ๋ฌ ๊ตฌ์ฑ ํ์ผ ๋ฐ ์์ค ์ฝ๋์ ๊ฑธ์ณ ๋ค์ํ ๋ณ๊ฒฝ ์ฌํญ์ ํฌํจํฉ๋๋ค. XML ํ์ผ์์ ๋ถํ์ํ Changes
Sequence Diagram(s)sequenceDiagram
participant User as ์ฌ์ฉ์
participant Screen as SignInScreen
participant WebView as GAuthSigninWebView
User->>Screen: ๋ก๊ทธ์ธ ๋ฒํผ ํด๋ฆญ
Screen-->>WebView: ํด๋ผ์ด์ธํธ ID, ๋ฆฌ๋ค์ด๋ ํธ URI ์ ๋ฌ
WebView-->>Screen: ์ธ์ฆ ์ฝ๋ ๋ฐํ
Screen-->>User: ๋ก๊ทธ์ธ ์ฒ๋ฆฌ ๊ฒฐ๊ณผ ์ ๋ฌ
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
โจ Finishing Touches
๐ชง TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 12
๐ญ Outside diff range comments (1)
presentation/src/main/java/view/theme/color/ColorTheme.kt (1)
6-14: ๐ ๏ธ Refactor suggestion์์ ์์ ๋ช ๋ช ๊ท์น์ ์ผ๊ด์ฑ์ด ํ์ํฉ๋๋ค.
์์ ์์๋ค์ ๋ช ๋ช ๊ท์น์ด ์ผ๊ด๋์ง ์์ต๋๋ค:
- ์ผ๋ถ๋ ๋๋ฌธ์(MAIN, WHITE)
- ์ผ๋ถ๋ ํ์ค์นผ ์ผ์ด์ค(Gold, Broze)
๋ํ 'Broze'๋ 'Bronze'์ ์คํ๋ก ๋ณด์ ๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ๋ชจ๋ ์์ ์์๋ฅผ ๋๋ฌธ์๋ก ํต์ผํ๋ ๊ฒ์ ์ ์๋๋ฆฝ๋๋ค:
- abstract val MAIN: Color - abstract val Black: Color - abstract val WHITE: Color - abstract val GRAY: Color - abstract val Gold: Color - abstract val silver: Color - abstract val Broze: Color + abstract val MAIN: Color + abstract val BLACK: Color + abstract val WHITE: Color + abstract val GRAY: Color + abstract val GOLD: Color + abstract val SILVER: Color + abstract val BRONZE: Color
๐งน Nitpick comments (6)
presentation/src/main/java/view/theme/color/DoMaColor.kt (1)
15-18: ์ฃผ์์ ๋ ์์ธํ๊ฒ ์์ฑํด์ฃผ์ธ์.ํ์ฌ ์ฃผ์์ด ๊ฐ๋จํ๊ฒ ์์ฑ๋์ด ์์ต๋๋ค. ๊ฐ ์์์ ์ฌ์ฉ ๋ชฉ์ ๊ณผ ์ ์ฉ ์์น๋ฅผ ๋ ์์ธํ ์ค๋ช ํด์ฃผ์๋ฉด ์ข๊ฒ ์ต๋๋ค.
์์:
- override val SYSTEM = Color(0xFF14C38E) // ์์คํ ์์ (๊ฐ์กฐ ์์ผ๋ก ์ค์ ) + override val SYSTEM = Color(0xFF14C38E) // ์์คํ ๊ฐ์กฐ ์์ - ์ฃผ์ ์ก์ ๋ฒํผ, ์งํ ์ํ ํ์ ๋ฑ์ ์ฌ์ฉpresentation/src/main/java/view/signin/component/DoMaInputID.kt (1)
51-56: ์์ ๊ฐ์ ํ ๋ง๋ก ๋ถ๋ฆฌํด์ฃผ์ธ์.ํ๋์ฝ๋ฉ๋ ์์ ๊ฐ๋ค์ ํ ๋ง ์์คํ ์ผ๋ก ์ด๋ํ์ฌ ์ผ๊ด์ฑ ์๋ ๋์์ธ์ ์ ์งํ๋ ๊ฒ์ด ์ข์ต๋๋ค.
- backgroundColor = Color(0xFF1E1E1E), - cursorColor = Color.White, - focusedBorderColor = Color(0xFFB9B9B9), - unfocusedBorderColor = Color(0xFFB9B9B9) + backgroundColor = colors.inputBackground, + cursorColor = colors.cursor, + focusedBorderColor = colors.borderFocused, + unfocusedBorderColor = colors.borderUnfocusedpresentation/src/main/java/view/signin/component/DoMaLoginButton.kt (1)
41-48: ์ ๊ทผ์ฑ ๊ฐ์ ์ด ํ์ํฉ๋๋ค.์คํฌ๋ฆฐ ๋ฆฌ๋ ์ฌ์ฉ์๋ฅผ ์ํด contentDescription์ ์ถ๊ฐํด์ฃผ์ธ์.
Text( text = "๋ก๊ทธ์ธ", + modifier = Modifier.semantics { + contentDescription = "๋ก๊ทธ์ธ ๋ฒํผ" + }, style = TextStyle( fontSize = 16.sp, fontWeight = FontWeight(700), color = colors.WHITE, ) )presentation/src/main/java/view/signin/Screen/SignInScreen.kt (3)
23-29: ํจ์ ๋ฌธ์ํ ๋ฐ ๋ค์ด๋ฐ ๊ฐ์ ์ด ํ์ํฉ๋๋ค.ํจ์์ ๊ฐ๋ ์ฑ๊ณผ ์ ์ง๋ณด์์ฑ์ ๋์ด๊ธฐ ์ํด ๋ค์ ์ฌํญ๋ค์ ์ ์๋๋ฆฝ๋๋ค:
@Composableํจ์์ KDoc ๋ฌธ์๋ฅผ ์ถ๊ฐํ์ฌ ๊ฐ ๋งค๊ฐ๋ณ์์ ์ญํ ์ ์ค๋ช- ์ฝ๋ฐฑ ํจ์๋ช ์ ๋ ๊ด์ฉ์ ์ผ๋ก ๋ณ๊ฒฝ (
leIsClickLoginButtonโonLoginClick)@Composable +/** + * ๋ก๊ทธ์ธ ํ๋ฉด์ ํ์ํ๋ Composable ํจ์ + * + * @param modifier ๋ ์ด์์ ์์ ์ + * @param clickButton Google ์ธ์ฆ ์น๋ทฐ ํ์ ์ฌ๋ถ + * @param gAuthLogin Google ์ธ์ฆ ์๋ฃ ์ ํธ์ถ๋๋ ์ฝ๋ฐฑ + * @param onLoginClick ๋ก๊ทธ์ธ ๋ฒํผ ํด๋ฆญ ์ ํธ์ถ๋๋ ์ฝ๋ฐฑ + */ fun SignInScreen( modifier: Modifier = Modifier, clickButton: Boolean, gAuthLogin: (String) -> Unit, - leIsClickLoginButton: () -> Unit, + onLoginClick: () -> Unit, )
37-53: ๋ ์ด์์ ๊ตฌ์กฐ ์ต์ ํ๊ฐ ํ์ํฉ๋๋ค.
- ์ค์ฒฉ๋ Column์์ modifier๊ฐ ๋ถํ์ํ๊ฒ ์ฌ์ ์ธ๋๊ณ ์์ต๋๋ค.
- Typography ์คํ์ผ์ ๋งค๋ฒ copyํ๋ ๋์ ๋ณ๋์ ์คํ์ผ์ ์ ์ํ๋ ๊ฒ์ด ์ข์ต๋๋ค.
Column( - modifier = modifier + modifier = Modifier .fillMaxWidth() .padding(start = 20.dp, end = 20.dp, bottom = 20.dp) .weight(1f), horizontalAlignment = Alignment.Start, verticalArrangement = Arrangement.Bottom ) { + val greetingStyle = DoMaTypography.headlineSmall Text( text = "์๋ ํ์ธ์!", - style = DoMaTypography.headlineSmall.copy(color = colors.WHITE), + style = greetingStyle.copy(color = colors.WHITE), ) Text( text = "๋ก๊ทธ์ธ์ ํด๋ณผ๊น์?", - style = DoMaTypography.headlineSmall.copy(color = colors.Green), + style = greetingStyle.copy(color = colors.Green), ) }
133-141: ํ๋ฆฌ๋ทฐ ํจ์ ๊ฐ์ ์ด ํ์ํฉ๋๋ค.๋ค์ํ ์ํ๋ฅผ ํ์ธํ ์ ์๋๋ก ํ๋ฆฌ๋ทฐ ํจ์๋ฅผ ํ์ฅํ๋ฉด ์ข์ ๊ฒ ๊ฐ์ต๋๋ค.
@Preview @Composable -fun PreviewSignInScreen() { +fun PreviewSignInScreen() = DoMaAndroidTheme { SignInScreen( clickButton = false, gAuthLogin = {}, leIsClickLoginButton = {} ) } + +@Preview(name = "Google Auth WebView Visible") +@Composable +fun PreviewSignInScreenWithWebView() = DoMaAndroidTheme { + SignInScreen( + clickButton = true, + gAuthLogin = {}, + leIsClickLoginButton = {} + ) +}
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
โ Files ignored due to path filters (3)
buildSrc/build/kotlin/compileKotlin/cacheable/last-build.binis excluded by!**/*.binbuildSrc/build/kotlin/compileKotlin/local-state/build-history.binis excluded by!**/*.binbuildSrc/build/libs/buildSrc.jaris excluded by!**/*.jar
๐ Files selected for processing (11)
.idea/deploymentTargetSelector.xml(0 hunks)buildSrc/build/kotlin/compileKotlin/cacheable/caches-jvm/lookups/counters.tab(1 hunks)gradle/libs.versions.toml(2 hunks)presentation/build.gradle.kts(1 hunks)presentation/src/main/java/view/signin/Screen/SignInScreen.kt(1 hunks)presentation/src/main/java/view/signin/Screen/SignInSubScreen.kt(1 hunks)presentation/src/main/java/view/signin/component/DoMaInputID.kt(1 hunks)presentation/src/main/java/view/signin/component/DoMaInputPassWord.kt(1 hunks)presentation/src/main/java/view/signin/component/DoMaLoginButton.kt(1 hunks)presentation/src/main/java/view/theme/color/ColorTheme.kt(1 hunks)presentation/src/main/java/view/theme/color/DoMaColor.kt(1 hunks)
๐ค Files with no reviewable changes (1)
- .idea/deploymentTargetSelector.xml
โ Files skipped from review due to trivial changes (1)
- buildSrc/build/kotlin/compileKotlin/cacheable/caches-jvm/lookups/counters.tab
โฐ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
๐ Additional comments (1)
gradle/libs.versions.toml (1)
16-16:Details
โ Verification successful
Firebase Annotations ๋ฒ์ ์ ๋ฐ์ดํธ๊ฐ ํ์ํฉ๋๋ค.
ํ์ฌ ์ค์ ๋ Firebase Annotations ๋ฒ์ (16.2.0)์ด ์ต์ ๋ฒ์ ์ด ์๋ ์ ์์ต๋๋ค. ๋ค๋ฅธ ์์กด์ฑ๋ค๊ณผ์ ํธํ์ฑ๋ ํ์ธ์ด ํ์ํฉ๋๋ค.
Also applies to: 37-37
๐ Web query:
What is the latest version of Firebase Annotations library and its compatibility with Firebase SDK?๐ก Result:
The latest version of Firebase Annotations (Java library) is 16.2.0, as referenced by its bindings in Xamarin.Firebase.Annotions v116.2.0[1]. This annotation library provides tooling support for ProGuard rules and API stability markers like
@KeepForSdkand@PublicApi[2].Compatibility with Firebase SDKs:
Android:
- Integrated into modern Firebase Android SDKs via Bill of Materials (BoM) v33.x[7][9].
- Compatible with
.NET for Androidprojects targetingnet8.0-android34+when using Xamarin bindings[1].Cross-SDK Usage:
- Annotations are honored across all Android-based Firebase libraries (e.g., Firestore v24.x[7], Crashlytics v19.x[7]) through shared ProGuard configurations[2].
- Kotlin Multiplatform support exists experimentally via Jetpack Annotation v1.x[4][10].
Key Features:
Annotation Purpose ProGuard Behavior @KeepPreserve APIs during optimization Enforced by default rules[2] @KeepForSdkRestrict usage to Firebase SDKs Linter enforcement in Android Studio[2] @PublicApiVersioning guidance for API changes Tooling integration[2] For .NET developers using Xamarin bindings (
Xamarin.Firebase.Annotions), ensure compatibility with.NET 8+and verify dependencies likeXamarin.JavaX.Inject v1+[1].Citations:
- 1: https://www.nuget.org/packages/Xamarin.Firebase.Annotations/
- 2: https://github.com/firebase/firebase-android-sdk
- 3: https://github.com/firebase/firebase-ios-sdk/releases
- 4: https://developer.android.com/jetpack/androidx/releases/annotation
- 5: https://firebase.google.com/docs/firestore/client/libraries
- 6: https://swiftpackageindex.com/firebase/firebase-ios-sdk
- 7: https://developers.google.com/android/guides/releases
- 8: https://rnfirebase.io
- 9: https://firebase.google.com/support/releases
- 10: https://firebaseopensource.com/projects/gitliveapp/firebase-kotlin-sdk/
Firebase Annotations ๋ฒ์ ํ์ธ ์๋ฃ โ ์ ๋ฐ์ดํธ ๋ถํ์
gradle/libs.versions.toml์ 16๋ฒ ๋ฐ 37๋ฒ ๋ผ์ธ์ ์ค์ ๋firebaseAnnotations = "16.2.0"์ ์ต์ ๋ฒ์ ์์ด ํ์ธ๋์์ต๋๋ค.- ์ต์ Firebase Annotations ๋ผ์ด๋ธ๋ฌ๋ฆฌ ๋ฒ์ ์ 16.2.0์ผ๋ก, ๋ค๋ฅธ Firebase SDK ์์กด์ฑ๊ณผ์ ํธํ์ฑ์๋ ๋ฌธ์ ๊ฐ ์์ต๋๋ค.
| implementation(Dependency.Compose.COMPOSE_MATERIAL3) | ||
| implementation(Dependency.Compose.COMPOSE_MATERIAL) | ||
| implementation(Dependency.Compose.COMPOSE_NAVIGATION) | ||
| implementation(libs.firebase.annotations) |
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.
๐ก Verification agent
๐งฉ Analysis chain
Firebase Annotations ๋ฒ์ ๊ฒ์ฆ์ด ํ์ํฉ๋๋ค.
Firebase Annotations ๋ผ์ด๋ธ๋ฌ๋ฆฌ์ ๋ฒ์ ์ด 16.2.0์ผ๋ก ์ค์ ๋์ด ์์ต๋๋ค. ๋ณด์ ๋ฐ ๊ธฐ๋ฅ ํฅ์์ ์ํด ์ต์ ๋ฒ์ ์ฌ์ฉ์ ๊ถ์ฅ๋๋ฆฝ๋๋ค.
๐ Web query:
What is the latest version of Firebase Annotations library?
๐ก Result:
The latest version of Firebase Annotations depends on its implementation context:
Java/Kotlin (Android)
For native Android development:
- Version:
16.0([source][10]) - Artifact:
com.google.firebase:firebase-annotations
This version appears stable but has not been updated recently in public repositories like Maven Central[10].
Xamarin (.NET for Android)
For .NET bindings via NuGet:
- Version:
116.2([source][1]) - Package:
Xamarin.Firebase.Annotaโtions
This package provides bindings for Googleโs Java-basedfirebase-annotationslibrary and follows Googleโs major versioning scheme with incremental updates[1].
Key Observations
1๏ธโฃ AndroidX vs Firebase
The androidx.annotation library (version 1โโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโโ.8` as of Mayย 2024) serves similar purposes but is distinct from Firebase-specific annotations[2].
2๏ธโฃ SDK Integration
Firebase SDKs use annotations like @KeepForSdk internally for ProGuard rules and API stability checks[8]. These are part of core SDK dependencies rather than standalone libraries[9].
3๏ธโฃ Version Alignment
Firebaseโs Bill of Materials (BoM) simplifies dependency management by aligning SDK versions[9]. For example:
implementation platform("com.google.firebase:firebase-bom:33โโโโโโโโโโโโโโโโโโโโโโโโโโโ`.8``")Citations:
- 1: https://www.nuget.org/packages/Xamarin.Firebase.Annotations/
- 2: https://developer.android.com/jetpack/androidx/releases/annotation
- 3: https://rnfirebase.io/releases
- 4: https://developers.google.com/android/guides/releases
- 5: https://www.nuget.org/packages/Xamarin.Firebase.Annotations/116.2.0.10
- 6: https://github.com/firebase/firebase-ios-sdk/releases
- 7: https://firebase.google.com/support/releases
- 8: https://github.com/firebase/firebase-android-sdk
- 9: https://firebase.google.com/support/release-notes/android
- 10: https://mvnrepository.com/artifact/com.google.firebase/firebase-annotations/16.0.0
Firebase Annotations ๋ผ์ด๋ธ๋ฌ๋ฆฌ ๋ฒ์ ๋ถ์ผ์น ๋ฌธ์ ํ์ธ
ํ์ฌ presentation/build.gradle.kts์์ Firebase Annotations ๋ผ์ด๋ธ๋ฌ๋ฆฌ๊ฐ libs.firebase.annotations๋ฅผ ํตํด 16.2.0 ๋ฒ์ ์ผ๋ก ์ฌ์ฉ๋๊ณ ์๋ ๊ฒ์ผ๋ก ๋ณด์
๋๋ค. Maven Repository ([source][10]) ๋ฑ ์ต์ ์๋ฃ์ ๋ฐ๋ฅด๋ฉด, ๋ค์ดํฐ๋ธ Android ํ๊ฒฝ์์ ๊ณต์์ ์ผ๋ก ๋ฆด๋ฆฌ์ฆ๋ ์์ ๋ฒ์ ์ 16.0.0์
๋๋ค.
- ์กฐ์น ์ฌํญ
- Firebase Annotations ๋ฒ์ ์ด 16.2.0 ๋์ 16.0.0์ผ๋ก ์ค์ ๋์ด์ผ ํ๋์ง ํ์ธ ๋ฐ๋๋๋ค.
- ๋ง์ฝ ์๋ํ ๋ฒ์ ์ด ์๋๋ผ๋ฉด, ๋ณด์ ๋ฐ ๊ธฐ๋ฅ ํฅ์์ ์ํด ๋ฒ์ ๋ฒํธ๋ฅผ 16.0.0์ผ๋ก ์์ ํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
| abstract val Background: Color | ||
| abstract val Green: Color |
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.
๐ ๏ธ Refactor suggestion
์๋ก์ด ์์ ์์๋ ๋ช ๋ช ๊ท์น์ ๋ฐ๋ผ์ผ ํฉ๋๋ค.
์๋ก ์ถ๊ฐ๋ ์์ ์์๋ค๋ ์ผ๊ด๋ ๋ช ๋ช ๊ท์น์ ๋ฐ๋ผ์ผ ํฉ๋๋ค.
- abstract val Background: Color
- abstract val Green: Color
+ abstract val BACKGROUND: Color
+ abstract val GREEN: Color๐ 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.
| abstract val Background: Color | |
| abstract val Green: Color | |
| abstract val BACKGROUND: Color | |
| abstract val GREEN: Color |
| override val Background = Color(0xFF1E1E1E) | ||
| override val Green = Color(0xFF9AD54A) |
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.
๐ ๏ธ Refactor suggestion
์์ ์์ ์ด๋ฆ์ ColorTheme ํด๋์ค์ ์ผ์น์์ผ์ผ ํฉ๋๋ค.
ColorTheme ํด๋์ค์ ๋ช ๋ช ๊ท์น ๋ณ๊ฒฝ์ ๋ง์ถฐ ๊ตฌํ๋ ์์ ์ด ํ์ํฉ๋๋ค.
- override val Background = Color(0xFF1E1E1E)
- override val Green = Color(0xFF9AD54A)
+ override val BACKGROUND = Color(0xFF1E1E1E)
+ override val GREEN = Color(0xFF9AD54A)๐ 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.
| override val Background = Color(0xFF1E1E1E) | |
| override val Green = Color(0xFF9AD54A) | |
| override val BACKGROUND = Color(0xFF1E1E1E) | |
| override val GREEN = Color(0xFF9AD54A) |
| fun DoMaInputID( | ||
| modifier: Modifier = Modifier, | ||
| ) { | ||
| val idState = remember { mutableStateOf(TextFieldValue("")) } // viewmodel ๊ตฌํ ํ ์์ ํ๊ฒ ์ต๋๋ค. |
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.
๐ ๏ธ Refactor suggestion
ViewModel๋ก ์ํ ๊ด๋ฆฌ ๋ฆฌํฉํ ๋ง์ด ํ์ํฉ๋๋ค.
TODO ์ฃผ์์์ ์ธ๊ธ๋ ๋๋ก ์ํ ๊ด๋ฆฌ๋ฅผ ViewModel๋ก ์ด๋ํ๋ ๊ฒ์ด ์ข์ต๋๋ค. ํ์ฌ ๊ตฌํ์ ์ํ๋ฅผ ์ปดํฌ์ ๋ธ ๋ด๋ถ์์ ๊ด๋ฆฌํ๊ณ ์์ด ํ ์คํธ์ ์ ์ง๋ณด์๊ฐ ์ด๋ ค์ธ ์ ์์ต๋๋ค.
- val idState = remember { mutableStateOf(TextFieldValue("")) } // viewmodel ๊ตฌํ ํ ์์ ํ๊ฒ ์ต๋๋ค.
+ // ViewModel์์ ์ํ๋ฅผ ๊ด๋ฆฌํ๋๋ก ์์
+ val idState: TextFieldValue,
+ val onIdChange: (TextFieldValue) -> Unit,๐ 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.
| val idState = remember { mutableStateOf(TextFieldValue("")) } // viewmodel ๊ตฌํ ํ ์์ ํ๊ฒ ์ต๋๋ค. | |
| // ViewModel์์ ์ํ๋ฅผ ๊ด๋ฆฌํ๋๋ก ์์ | |
| val idState: TextFieldValue, | |
| val onIdChange: (TextFieldValue) -> Unit, |
| fun DoMaLoginButton( | ||
| modifier: Modifier = Modifier, | ||
| onClick: () -> Unit, | ||
| ) { |
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.
๐ ๏ธ Refactor suggestion
๋ก๋ฉ ์ํ ์ฒ๋ฆฌ๊ฐ ํ์ํฉ๋๋ค.
๋ก๊ทธ์ธ ๋ฒํผ์ ๋ก๋ฉ ์ํ๋ฅผ ์ถ๊ฐํ์ฌ ์ฌ์ฉ์์๊ฒ ๋ก๊ทธ์ธ ์งํ ์ํ๋ฅผ ํ์ํด์ฃผ๋ ๊ฒ์ด ์ข์ต๋๋ค.
fun DoMaLoginButton(
modifier: Modifier = Modifier,
+ isLoading: Boolean = false,
onClick: () -> Unit,
) {๐ 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.
| fun DoMaLoginButton( | |
| modifier: Modifier = Modifier, | |
| onClick: () -> Unit, | |
| ) { | |
| fun DoMaLoginButton( | |
| modifier: Modifier = Modifier, | |
| isLoading: Boolean = false, | |
| onClick: () -> Unit, | |
| ) { |
| clientId = "ghskfend", | ||
| redirectUri = "ghskfen" |
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.
๋ณด์ ์ทจ์ฝ์ : ํด๋ผ์ด์ธํธ ์๊ฒฉ ์ฆ๋ช ์ด ํ๋์ฝ๋ฉ๋์ด ์์ต๋๋ค.
ํด๋ผ์ด์ธํธ ID์ ๋ฆฌ๋ค์ด๋ ํธ URI๋ฅผ ํ๊ฒฝ ์ค์ ์ด๋ ๋ณด์ ์ ์ฅ์๋ก ์ด๋ํด์ผ ํฉ๋๋ค.
- clientId = "ghskfend",
- redirectUri = "ghskfen"
+ clientId = BuildConfig.GAUTH_CLIENT_ID,
+ redirectUri = BuildConfig.GAUTH_REDIRECT_URI๐ 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.
| clientId = "ghskfend", | |
| redirectUri = "ghskfen" | |
| clientId = BuildConfig.GAUTH_CLIENT_ID, | |
| redirectUri = BuildConfig.GAUTH_REDIRECT_URI |
| if (clickButton) { | ||
| GAuthSigninWebView( | ||
| clientId = "ghskfend", | ||
| redirectUri = "ghskfen" | ||
| ) { code -> | ||
| gAuthLogin(code) | ||
| } | ||
| } |
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.
๐ ๏ธ Refactor suggestion
๋ก๊ทธ์ธ ์ค๋ฅ ์ฒ๋ฆฌ๊ฐ ํ์ํฉ๋๋ค.
Google ์ธ์ฆ ๊ณผ์ ์์ ๋ฐ์ํ ์ ์๋ ์ค๋ฅ ์ฒ๋ฆฌ ๋ก์ง์ ์ถ๊ฐํด์ฃผ์ธ์.
if (clickButton) {
GAuthSigninWebView(
clientId = BuildConfig.GAUTH_CLIENT_ID,
redirectUri = BuildConfig.GAUTH_REDIRECT_URI,
+ onError = { error ->
+ // ์ค๋ฅ ์ฒ๋ฆฌ ๋ก์ง
+ onLoginError(error)
+ }
) { code ->
gAuthLogin(code)
}
}๐ 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.
| if (clickButton) { | |
| GAuthSigninWebView( | |
| clientId = "ghskfend", | |
| redirectUri = "ghskfen" | |
| ) { code -> | |
| gAuthLogin(code) | |
| } | |
| } | |
| if (clickButton) { | |
| GAuthSigninWebView( | |
| clientId = BuildConfig.GAUTH_CLIENT_ID, | |
| redirectUri = BuildConfig.GAUTH_REDIRECT_URI, | |
| onError = { error -> | |
| // ์ค๋ฅ ์ฒ๋ฆฌ ๋ก์ง | |
| onLoginError(error) | |
| } | |
| ) { code -> | |
| gAuthLogin(code) | |
| } | |
| } |
| if (clickButton) { | ||
| GAuthSigninWebView( | ||
| clientId = "ghskfend", | ||
| redirectUri = "ghskfen" | ||
| ) { code -> | ||
| gAuthLogin(code) | ||
| } | ||
| } |
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.
๋ณด์ ๊ฐ์ ๋ฐ ๋ณ์๋ช ์์ ์ด ํ์ํฉ๋๋ค.
- ํด๋ผ์ด์ธํธ ID์ ๋ฆฌ๋ค์ด๋ ํธ URI๊ฐ ํ๋์ฝ๋ฉ๋์ด ์์ด ๋ณด์์ ์ํํฉ๋๋ค.
clickButton๋ณ์๋ช ์ด ๋ช ํํ์ง ์์ต๋๋ค.
-if (clickButton) {
+if (showGoogleAuthWebView) {
GAuthSigninWebView(
- clientId = "ghskfend",
- redirectUri = "ghskfen"
+ clientId = BuildConfig.GOOGLE_AUTH_CLIENT_ID,
+ redirectUri = BuildConfig.GOOGLE_AUTH_REDIRECT_URI
) { code ->
gAuthLogin(code)
}
}๐ 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.
| if (clickButton) { | |
| GAuthSigninWebView( | |
| clientId = "ghskfend", | |
| redirectUri = "ghskfen" | |
| ) { code -> | |
| gAuthLogin(code) | |
| } | |
| } | |
| if (showGoogleAuthWebView) { | |
| GAuthSigninWebView( | |
| clientId = BuildConfig.GOOGLE_AUTH_CLIENT_ID, | |
| redirectUri = BuildConfig.GOOGLE_AUTH_REDIRECT_URI | |
| ) { code -> | |
| gAuthLogin(code) | |
| } | |
| } |
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(bottom = 20.dp), | ||
| horizontalArrangement = Arrangement.Center, | ||
| verticalAlignment = Alignment.CenterVertically, | ||
|
|
||
| ) { | ||
| Text( | ||
| text = "์์ง ํจ๊ปํ์ง ๋ชปํ๋ค๋ฉด?", | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| ) | ||
|
|
||
| Spacer(modifier = Modifier.width(4.dp)) | ||
|
|
||
| Text( | ||
| text = "ํ์๊ฐ์ ํ๊ธฐ", | ||
| modifier = Modifier | ||
| .clickable { /* TODO: ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ์ด๋ */ } | ||
| .semantics { contentDescription = "ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋" }, | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| textAlign = TextAlign.Center | ||
| ) | ||
| } |
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.
๐ก Verification agent
๐งฉ Analysis chain
ํ์๊ฐ์ ๋ค๋น๊ฒ์ด์ ๊ตฌํ ๋ฐ ์ ๊ทผ์ฑ ๊ฐ์ ์ด ํ์ํฉ๋๋ค.
- ํ์๊ฐ์ ํ๋ฉด์ผ๋ก์ ๋ค๋น๊ฒ์ด์ ๊ตฌํ์ด ํ์ํฉ๋๋ค.
- ์ ๊ทผ์ฑ ์ค๋ช ์ ๋ ์์ธํ๊ฒ ์ ๊ณตํ๋ฉด ์ข์ ๊ฒ ๊ฐ์ต๋๋ค.
ํ์๊ฐ์ ํ๋ฉด์ด ๊ตฌํ๋์ด ์๋์ง ํ์ธ์ด ํ์ํฉ๋๋ค:
Text(
text = "ํ์๊ฐ์
ํ๊ธฐ",
modifier = Modifier
- .clickable { /* TODO: ํ์๊ฐ์
ํ๋ฉด์ผ๋ก ์ด๋ */ }
- .semantics { contentDescription = "ํ์๊ฐ์
ํ์ด์ง๋ก ์ด๋" },
+ .clickable { /* TODO: implement navigation to SignUpScreen */ }
+ .semantics {
+ contentDescription = "์๋ก์ด ๊ณ์ ์ ๋ง๋ค๊ธฐ ์ํ ํ์๊ฐ์
ํ์ด์ง๋ก ์ด๋ํฉ๋๋ค"
+ },
style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
textAlign = TextAlign.Center
)๐ Script executed:
#!/bin/bash
# ํ์๊ฐ์
๊ด๋ จ ํ์ผ ๊ฒ์
rg -l "ํ์๊ฐ์
|SignUp" --type kotlinLength of output: 97
ํ์๊ฐ์ ๋ค๋น๊ฒ์ด์ ๋ฐ ์ ๊ทผ์ฑ ๊ฐ์ ํ์
- ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ๋ค๋น๊ฒ์ด์ ํ๋ ๋ก์ง์ด ๊ตฌํ๋์ง ์์์ต๋๋ค. ์ค์ ํ๋ฉด ์ ํ ๊ธฐ๋ฅ์ ์ถ๊ฐํด์ฃผ์ธ์.
- ์ ๊ทผ์ฑ ์ค๋ช ์ด ๊ฐ๊ฒฐํ์ง๋ง, ๊ตฌ์ฒด์ ์ธ ์ค๋ช ์ผ๋ก ์ ๋ฐ์ดํธํ๋ฉด ์ข๊ฒ ์ต๋๋ค. ์๋ฅผ ๋ค์ด, "ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋" ๋์ "์๋ก์ด ๊ณ์ ์ ๋ง๋ค๊ธฐ ์ํ ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋ํฉ๋๋ค"์ฒ๋ผ ๋ช ํํ ๊ธฐ์ ํด์ฃผ์ธ์.
Text(
text = "ํ์๊ฐ์
ํ๊ธฐ",
modifier = Modifier
- .clickable { /* TODO: ํ์๊ฐ์
ํ๋ฉด์ผ๋ก ์ด๋ */ }
- .semantics { contentDescription = "ํ์๊ฐ์
ํ์ด์ง๋ก ์ด๋" },
+ .clickable { /* TODO: ํ์๊ฐ์
ํ๋ฉด์ผ๋ก ์ด๋ํ๋ ๋ก์ง ๊ตฌํ */ }
+ .semantics {
+ contentDescription = "์๋ก์ด ๊ณ์ ์ ๋ง๋ค๊ธฐ ์ํ ํ์๊ฐ์
ํ์ด์ง๋ก ์ด๋ํฉ๋๋ค"
+ },
style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
textAlign = TextAlign.Center
)๐ 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.
| Row( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(bottom = 20.dp), | |
| horizontalArrangement = Arrangement.Center, | |
| verticalAlignment = Alignment.CenterVertically, | |
| ) { | |
| Text( | |
| text = "์์ง ํจ๊ปํ์ง ๋ชปํ๋ค๋ฉด?", | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| Spacer(modifier = Modifier.width(4.dp)) | |
| Text( | |
| text = "ํ์๊ฐ์ ํ๊ธฐ", | |
| modifier = Modifier | |
| .clickable { /* TODO: ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ์ด๋ */ } | |
| .semantics { contentDescription = "ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋" }, | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| textAlign = TextAlign.Center | |
| ) | |
| } | |
| Row( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(bottom = 20.dp), | |
| horizontalArrangement = Arrangement.Center, | |
| verticalAlignment = Alignment.CenterVertically, | |
| ) { | |
| Text( | |
| text = "์์ง ํจ๊ปํ์ง ๋ชปํ๋ค๋ฉด?", | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| Spacer(modifier = Modifier.width(4.dp)) | |
| Text( | |
| text = "ํ์๊ฐ์ ํ๊ธฐ", | |
| modifier = Modifier | |
| .clickable { /* TODO: ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ์ด๋ํ๋ ๋ก์ง ๊ตฌํ */ } | |
| .semantics { | |
| contentDescription = "์๋ก์ด ๊ณ์ ์ ๋ง๋ค๊ธฐ ์ํ ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋ํฉ๋๋ค" | |
| }, | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| textAlign = TextAlign.Center | |
| ) | |
| } |
| Row( | ||
| horizontalArrangement = Arrangement.End, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(horizontal = 20.dp), | ||
| ) { | ||
| Text( | ||
| text = "์์ด๋ ์ฐพ๊ธฐ", | ||
| modifier = Modifier | ||
| .clickable( | ||
| interactionSource = remember { MutableInteractionSource() }, | ||
| indication = rememberRipple(), | ||
| onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ์ฒ๋ฆฌ */ } | ||
| ), | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| ) | ||
| Text( | ||
| text = " | ", | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| ) | ||
| Text( | ||
| text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", | ||
| modifier = Modifier | ||
| .clickable( | ||
| interactionSource = remember { MutableInteractionSource() }, | ||
| indication = rememberRipple(), | ||
| onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ์ฒ๋ฆฌ */ } | ||
| ), | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| ) | ||
| } |
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.
๐ก Verification agent
๐ ๏ธ Refactor suggestion
๐งฉ Analysis chain
์ฝ๋ ์ค๋ณต์ ์ ๊ฑฐํ๊ณ TODO ํญ๋ชฉ ๊ตฌํ์ด ํ์ํฉ๋๋ค.
- ์์ด๋/๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ํ ์คํธ์ ์ค๋ณต๋ ์ฝ๋๋ฅผ ์ปดํฌ๋ํธ๋ก ๋ถ๋ฆฌ
- TODO ํญ๋ชฉ์ ๋ํ ๊ตฌ์ฒด์ ์ธ ๊ตฌํ์ด ํ์
+@Composable
+private fun ClickableText(
+ text: String,
+ onClick: () -> Unit,
+ modifier: Modifier = Modifier,
+ style: TextStyle = DoMaTypography.labelLarge,
+ color: Color = colors.WHITE
+) {
+ Text(
+ text = text,
+ modifier = modifier.clickable(
+ interactionSource = remember { MutableInteractionSource() },
+ indication = rememberRipple(),
+ onClick = onClick
+ ),
+ style = style.copy(color = color)
+ )
+}
Row(
horizontalArrangement = Arrangement.End,
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 20.dp),
) {
- Text(
- text = "์์ด๋ ์ฐพ๊ธฐ",
- modifier = Modifier.clickable(...),
- style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
- )
+ ClickableText(
+ text = "์์ด๋ ์ฐพ๊ธฐ",
+ onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ }
+ )
Text(
text = " | ",
style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
)
- Text(
- text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ",
- modifier = Modifier.clickable(...),
- style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
- )
+ ClickableText(
+ text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ",
+ onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ }
+ )
}์์ด๋/๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ๊ธฐ๋ฅ์ด ๊ตฌํ๋์ด ์๋์ง ํ์ธ์ด ํ์ํฉ๋๋ค:
๐ Script executed:
#!/bin/bash
# ์์ด๋/๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ๊ด๋ จ ํ์ผ ๊ฒ์
rg -l "์์ด๋ ์ฐพ๊ธฐ|๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ" --type kotlinLength of output: 100
์ค๋ณต ์ฝ๋ ์ ๊ฑฐ ๋ฐ TODO ๊ตฌํ ํ์
- ์์ด๋ ์ฐพ๊ธฐ์ ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ๋ฅผ ์ฒ๋ฆฌํ๋ ์ฝ๋๊ฐ ์ค๋ณต๋์ด ์์ผ๋ฏ๋ก, ์ด๋ฅผ ๋ณ๋์ Composable ์ปดํฌ๋ํธ(์:
ClickableText)๋ก ๋ถ๋ฆฌํด ์ฌ์ฌ์ฉ์ฑ์ ๋์ฌ์ฃผ์ธ์. - ํ์ฌ ๋ ํญ๋ชฉ์
TODO์ฃผ์์ด ๋จ์์์ผ๋ฏ๋ก, ์ดํ ํ๋ฉด ์ด๋ ๋ฑ ๊ตฌ์ฒด์ ์ธ ๊ตฌํ ์ฌํญ์ ์ถ๊ฐํด ๊ธฐ๋ฅ์ ์์ฑํด์ผ ํฉ๋๋ค.
+ @Composable
+ private fun ClickableText(
+ text: String,
+ onClick: () -> Unit,
+ modifier: Modifier = Modifier,
+ style: TextStyle = DoMaTypography.labelLarge,
+ color: Color = colors.WHITE
+ ) {
+ Text(
+ text = text,
+ modifier = modifier.clickable(
+ interactionSource = remember { MutableInteractionSource() },
+ indication = rememberRipple(),
+ onClick = onClick
+ ),
+ style = style.copy(color = color)
+ )
+ }
Row(
horizontalArrangement = Arrangement.End,
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 20.dp),
) {
- Text(
- text = "์์ด๋ ์ฐพ๊ธฐ",
- modifier = Modifier.clickable(
- interactionSource = remember { MutableInteractionSource() },
- indication = rememberRipple(),
- onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ์ฒ๋ฆฌ */ }
- ),
- style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
- )
+ ClickableText(
+ text = "์์ด๋ ์ฐพ๊ธฐ",
+ onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ }
+ )
Text(
text = " | ",
style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
)
- Text(
- text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ",
- modifier = Modifier.clickable(
- interactionSource = remember { MutableInteractionSource() },
- indication = rememberRipple(),
- onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ์ฒ๋ฆฌ */ }
- ),
- style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
- )
+ ClickableText(
+ text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ",
+ onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ }
+ )
}๐ 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.
| Row( | |
| horizontalArrangement = Arrangement.End, | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(horizontal = 20.dp), | |
| ) { | |
| Text( | |
| text = "์์ด๋ ์ฐพ๊ธฐ", | |
| modifier = Modifier | |
| .clickable( | |
| interactionSource = remember { MutableInteractionSource() }, | |
| indication = rememberRipple(), | |
| onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ์ฒ๋ฆฌ */ } | |
| ), | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| Text( | |
| text = " | ", | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| Text( | |
| text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", | |
| modifier = Modifier | |
| .clickable( | |
| interactionSource = remember { MutableInteractionSource() }, | |
| indication = rememberRipple(), | |
| onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ์ฒ๋ฆฌ */ } | |
| ), | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| } | |
| @Composable | |
| private fun ClickableText( | |
| text: String, | |
| onClick: () -> Unit, | |
| modifier: Modifier = Modifier, | |
| style: TextStyle = DoMaTypography.labelLarge, | |
| color: Color = colors.WHITE | |
| ) { | |
| Text( | |
| text = text, | |
| modifier = modifier.clickable( | |
| interactionSource = remember { MutableInteractionSource() }, | |
| indication = rememberRipple(), | |
| onClick = onClick | |
| ), | |
| style = style.copy(color = color) | |
| ) | |
| } | |
| Row( | |
| horizontalArrangement = Arrangement.End, | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| .padding(horizontal = 20.dp), | |
| ) { | |
| ClickableText( | |
| text = "์์ด๋ ์ฐพ๊ธฐ", | |
| onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ } | |
| ) | |
| Text( | |
| text = " | ", | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| ClickableText( | |
| text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", | |
| onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ } | |
| ) | |
| } |
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 (3)
presentation/src/main/java/view/signin/Screen/SignInSubScreen.kt (1)
93-100:โ ๏ธ Potential issue๋ณด์ ์ทจ์ฝ์ : ํด๋ผ์ด์ธํธ ์๊ฒฉ ์ฆ๋ช ์ด ํ๋์ฝ๋ฉ๋์ด ์์ต๋๋ค.
ํด๋ผ์ด์ธํธ ID์ ๋ฆฌ๋ค์ด๋ ํธ URI๋ฅผ ํ๊ฒฝ ์ค์ ์ด๋ ๋ณด์ ์ ์ฅ์๋ก ์ด๋ํด์ผ ํฉ๋๋ค.
if (clickButton) { GAuthSigninWebView( - clientId = "ghskfend", - redirectUri = "ghskfen" + clientId = BuildConfig.GAUTH_CLIENT_ID, + redirectUri = BuildConfig.GAUTH_REDIRECT_URI ) { code -> gAuthLogin(code) } }presentation/src/main/java/view/signin/Screen/SignInScreen.kt (2)
111-119: ๐ ๏ธ Refactor suggestionํ์๊ฐ์ ๋ค๋น๊ฒ์ด์ ๋ฐ ์ ๊ทผ์ฑ ๊ฐ์ ์ด ํ์ํฉ๋๋ค.
ํ์๊ฐ์ ๋ฒํผ์ ๋ค๋น๊ฒ์ด์ ๊ตฌํ์ด ๋๋ฝ๋์ด ์์ผ๋ฉฐ, ์ ๊ทผ์ฑ ์ค๋ช ์ด ๋ ์์ธํ ํ์๊ฐ ์์ต๋๋ค.
Text( text = "ํ์๊ฐ์ ํ๊ธฐ", modifier = Modifier - .clickable { /* TODO: ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ์ด๋ */ } - .semantics { contentDescription = "ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋" }, + .clickable { navController.navigate("signup") } + .semantics { + contentDescription = "์๋ก์ด ๊ณ์ ์ ๋ง๋ค๊ธฐ ์ํ ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋ํฉ๋๋ค" + }, style = DoMaTypography.labelLarge.copy(color = colors.WHITE), textAlign = TextAlign.Center )
121-128:โ ๏ธ Potential issue๋ณด์ ์ทจ์ฝ์ : ํด๋ผ์ด์ธํธ ์๊ฒฉ ์ฆ๋ช ์ด ํ๋์ฝ๋ฉ๋์ด ์์ต๋๋ค.
ํด๋ผ์ด์ธํธ ID์ ๋ฆฌ๋ค์ด๋ ํธ URI๋ฅผ ํ๊ฒฝ ์ค์ ์ด๋ ๋ณด์ ์ ์ฅ์๋ก ์ด๋ํด์ผ ํฉ๋๋ค.
if (clickButton) { GAuthSigninWebView( - clientId = "ghskfend", - redirectUri = "ghskfen" + clientId = BuildConfig.GAUTH_CLIENT_ID, + redirectUri = BuildConfig.GAUTH_REDIRECT_URI ) { code -> gAuthLogin(code) } }
๐งน Nitpick comments (1)
presentation/src/main/java/view/signin/Screen/SignInSubScreen.kt (1)
26-31: ๋งค๊ฐ๋ณ์ ์ด๋ฆ์ ๋ ๋ช ํํ๊ฒ ๊ฐ์ ํด์ฃผ์ธ์.๋งค๊ฐ๋ณ์ ์ด๋ฆ์ด ์๋๋ฅผ ๋ช ํํ๊ฒ ์ ๋ฌํ์ง ๋ชปํ๊ณ ์์ต๋๋ค:
clickButton์showGAuthWebView๋isGAuthEnabled๋ก ๋ณ๊ฒฝํ๋ ๊ฒ์ด ๋ ๋ช ํํฉ๋๋ค.leIsClickLoginButton์ Kotlin ๋ค์ด๋ฐ ์ปจ๋ฒค์ ์ ๋ฐ๋ผonLoginClick์ผ๋ก ๋ณ๊ฒฝํ๋ ๊ฒ์ด ์ข์ต๋๋ค.fun SignInSubScreen( modifier: Modifier = Modifier, - clickButton: Boolean, + showGAuthWebView: Boolean, gAuthLogin: (String) -> Unit, - leIsClickLoginButton: () -> Unit, + onLoginClick: () -> Unit, )
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
๐ Files selected for processing (4)
presentation/src/main/java/view/signin/Screen/SignInScreen.kt(1 hunks)presentation/src/main/java/view/signin/Screen/SignInSubScreen.kt(1 hunks)presentation/src/main/java/view/theme/color/ColorTheme.kt(1 hunks)presentation/src/main/java/view/theme/color/DoMaColor.kt(1 hunks)
๐ง Files skipped from review as they are similar to previous changes (2)
- presentation/src/main/java/view/theme/color/DoMaColor.kt
- presentation/src/main/java/view/theme/color/ColorTheme.kt
โฐ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
๐ Additional comments (1)
presentation/src/main/java/view/signin/Screen/SignInScreen.kt (1)
62-92: ์ค๋ณต ์ฝ๋ ์ ๊ฑฐ๋ฅผ ์ํด ์ฌ์ฌ์ฉ ๊ฐ๋ฅํ ์ปดํฌ๋ํธ๋ก ๋ถ๋ฆฌํด์ฃผ์ธ์.์์ด๋ ์ฐพ๊ธฐ์ ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ํ ์คํธ์ ์ค๋ณต๋ ์ฝ๋๋ฅผ ๋ณ๋์ ์ปดํฌ๋ํธ๋ก ๋ถ๋ฆฌํ๋ฉด ์ฝ๋์ ์ฌ์ฌ์ฉ์ฑ์ด ํฅ์๋ฉ๋๋ค.
+@Composable +private fun ClickableText( + text: String, + onClick: () -> Unit, + modifier: Modifier = Modifier, + style: TextStyle = DoMaTypography.labelLarge, + color: Color = colors.WHITE +) { + Text( + text = text, + modifier = modifier.clickable( + interactionSource = remember { MutableInteractionSource() }, + indication = rememberRipple(), + onClick = onClick + ), + style = style.copy(color = color) + ) +} Row( horizontalArrangement = Arrangement.End, modifier = Modifier .fillMaxWidth() .padding(horizontal = 20.dp), ) { - Text( - text = "์์ด๋ ์ฐพ๊ธฐ", - modifier = Modifier.clickable(...), - style = DoMaTypography.labelLarge.copy(color = colors.WHITE), - ) + ClickableText( + text = "์์ด๋ ์ฐพ๊ธฐ", + onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ } + ) Text( text = " | ", style = DoMaTypography.labelLarge.copy(color = colors.WHITE), ) - Text( - text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", - modifier = Modifier.clickable(...), - style = DoMaTypography.labelLarge.copy(color = colors.WHITE), - ) + ClickableText( + text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", + onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ } + ) }
| DoMaInputID( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| ) | ||
|
|
||
| Spacer(modifier = Modifier.height(12.dp)) | ||
|
|
||
| DoMaInputPassword( | ||
| 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.
๐ ๏ธ Refactor suggestion
์ ๋ ฅ ์ ํจ์ฑ ๊ฒ์ฌ์ ์ค๋ฅ ์ฒ๋ฆฌ๋ฅผ ์ถ๊ฐํด์ฃผ์ธ์.
์ฌ์ฉ์ ์ ๋ ฅ์ ๋ํ ์ ํจ์ฑ ๊ฒ์ฌ์ ์ค๋ฅ ์ฒ๋ฆฌ๊ฐ ์์ต๋๋ค. ๋ค์ ๊ธฐ๋ฅ๋ค์ ์ถ๊ฐํ๋ ๊ฒ์ด ์ข์ต๋๋ค:
- ID/๋น๋ฐ๋ฒํธ ํ์ ๊ฒ์ฆ
- ์ค๋ฅ ๋ฉ์์ง ํ์
- ์ ๋ ฅ ์ ํ ์ฒ๋ฆฌ
DoMaInputID(
modifier = Modifier
- .fillMaxWidth()
+ .fillMaxWidth(),
+ onValueChange = { id ->
+ validateId(id)?.let { error ->
+ // ์ค๋ฅ ๋ฉ์์ง ํ์
+ }
+ }
)
DoMaInputPassword(
modifier = Modifier
- .fillMaxWidth()
+ .fillMaxWidth(),
+ onValueChange = { password ->
+ validatePassword(password)?.let { error ->
+ // ์ค๋ฅ ๋ฉ์์ง ํ์
+ }
+ }
)๐ 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.
| DoMaInputID( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| ) | |
| Spacer(modifier = Modifier.height(12.dp)) | |
| DoMaInputPassword( | |
| modifier = Modifier | |
| .fillMaxWidth() | |
| ) | |
| DoMaInputID( | |
| modifier = Modifier | |
| .fillMaxWidth(), | |
| onValueChange = { id -> | |
| validateId(id)?.let { error -> | |
| // ์ค๋ฅ ๋ฉ์์ง ํ์ | |
| } | |
| } | |
| ) | |
| Spacer(modifier = Modifier.height(12.dp)) | |
| DoMaInputPassword( | |
| modifier = Modifier | |
| .fillMaxWidth(), | |
| onValueChange = { password -> | |
| validatePassword(password)?.let { error -> | |
| // ์ค๋ฅ ๋ฉ์์ง ํ์ | |
| } | |
| } | |
| ) |
|
TextField ์ ์ ๋ ฅ๋ง ๋๊ฒ ํ์ง๋ง๊ณ ์ต๋๊ธ์์, keyboardActions, ์๋ฌ ์ฒ๋ฆฌ ์ ์ถ๊ฐํด์ฃผ์ธ์ |
| fun DoMaInputID( | ||
| modifier: Modifier = Modifier, | ||
| ) { | ||
| val idState = remember { mutableStateOf(TextFieldValue("")) } // viewmodel ๊ตฌํ ํ ์์ ํ๊ฒ ์ต๋๋ค. |
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.
๊ทธ๋ฆฌ๊ณ ์ด๊ฑธ viewmodel ๊ด๋ฆฌํ๊ฒ ๋ฐ๊ฟ์ฃผ์๊ธธ ๋ฐ๋๋๋ค,
์ง๊ธ ์ฝ๋๋ฅผ ๋ณด๋ฉด stasteful ํฉ๋๋ค. Statelessํ Composable ํจ์๋ฅผ ๋ง๋ค์ด์ผ ํ ๊ฒ ๊ฐ์ต๋๋ค.
| fun DoMaInputPassword( | ||
| modifier: Modifier = Modifier, | ||
| ) { | ||
| val passwordState = remember { mutableStateOf(TextFieldValue("")) } // viewmodel ๊ตฌํ ํ ์์ ํ๊ฒ ์ต๋๋ค. |
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.
๊ทธ๋ฆฌ๊ณ ์ด๊ฑธ viewmode ์์ l ๊ด๋ฆฌํ๊ฒ ๋ฐ๊ฟ์ฃผ์๊ธธ ๋ฐ๋๋๋ค,
์ง๊ธ ์ฝ๋๋ฅผ ๋ณด๋ฉด stasteful ํฉ๋๋ค. Statelessํ Composable ํจ์๋ฅผ ๋ง๋ค์ด์ผ ํ ๊ฒ ๊ฐ์ต๋๋ค.
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .clickablesingle(onClick = onClick) | ||
| .background(color = Color(0xFF494949), shape = RoundedCornerShape(size = 10.dp)) |
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.
domaandroidTheme ์ colors ์ ์ด์ฉํด ์๊น์ ์ฌ์ฉํด์ฃผ์ธ์
| jetbrainsKotlinJvm = "1.9.0" | ||
| identityJvm = "202411.1" | ||
| lifecycleViewmodelAndroid = "2.8.6" | ||
| firebaseAnnotations = "16.2.0" |
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.
firebase ๋ ์ ์ถ๊ฐํ์ จ๋์ง ๊ถ๊ธํฉ๋๋ค
| .background(color = colors.BACKGROUND) | ||
| ) { | ||
| Column( | ||
| modifier = modifier |
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.
๋๋ฌธ์ Modifier ๋ก ์ ์ด์ฃผ์ธ์
| ) | ||
| }, | ||
| colors = TextFieldDefaults.outlinedTextFieldColors( | ||
| backgroundColor = Color(0xFF1E1E1E), |
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.
DomaColortheme ์ ์ด์ฉํด ์๊น์ ์ง์ ํด์ฃผ์ธ์
| DoMaAndroidTheme { colors, typography -> | ||
|
|
||
| Column( | ||
| modifier = Modifier |
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.
์ฒ์ modifier ์ ์๋ฌธ์ ์ ๋๋ค
| ) | ||
| }, | ||
| colors = TextFieldDefaults.outlinedTextFieldColors( | ||
| backgroundColor = Color(0xFF1E1E1E), |
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.
DomaColortheme ์ ์ด์ฉํด ์๊น์ ์ง์ ํด์ฃผ์ธ์
| ), | ||
|
|
||
| ) { | ||
| Text( |
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.
DomaAndroid ๋ฅผ ์ด์ฉํด ๊ธ์์ฒด๋ฅผ ์ง์ ํด์ฃผ์ธ์
| .padding(bottom = 20.dp), | ||
| horizontalArrangement = Arrangement.Center, | ||
| verticalAlignment = Alignment.CenterVertically, | ||
|
|
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: 2
โป๏ธ Duplicate comments (3)
presentation/src/main/java/view/signin/Screen/SignInScreen.kt (3)
62-92: ๐ ๏ธ Refactor suggestion์ค๋ณต ์ฝ๋ ์ ๊ฑฐ ๋ฐ TODO ๊ตฌํ ํ์
์์ด๋ ์ฐพ๊ธฐ์ ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ๋ฅผ ์ฒ๋ฆฌํ๋ ์ฝ๋๊ฐ ์ค๋ณต๋์ด ์์ผ๋ฏ๋ก, ์ด๋ฅผ ๋ณ๋์ Composable ์ปดํฌ๋ํธ๋ก ๋ถ๋ฆฌํด ์ฌ์ฌ์ฉ์ฑ์ ๋์ฌ์ฃผ์ธ์. ํ์ฌ ๋ ํญ๋ชฉ์
TODO์ฃผ์์ด ๋จ์์์ผ๋ฏ๋ก, ์ดํ ํ๋ฉด ์ด๋ ๋ฑ ๊ตฌ์ฒด์ ์ธ ๊ตฌํ ์ฌํญ์ ์ถ๊ฐํด์ผ ํฉ๋๋ค.+ @Composable + private fun ClickableText( + text: String, + onClick: () -> Unit, + modifier: Modifier = Modifier, + style: TextStyle = DoMaTypography.labelLarge, + color: Color = colors.WHITE + ) { + Text( + text = text, + modifier = modifier.clickable( + interactionSource = remember { MutableInteractionSource() }, + indication = rememberRipple(), + onClick = onClick + ), + style = style.copy(color = color) + ) + } Row( horizontalArrangement = Arrangement.End, modifier = Modifier .fillMaxWidth() .padding(horizontal = 20.dp), ) { - Text( - text = "์์ด๋ ์ฐพ๊ธฐ", - modifier = Modifier.clickable( - interactionSource = remember { MutableInteractionSource() }, - indication = rememberRipple(), - onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ์ฒ๋ฆฌ */ } - ), - style = DoMaTypography.labelLarge.copy(color = colors.WHITE), - ) + ClickableText( + text = "์์ด๋ ์ฐพ๊ธฐ", + onClick = { /* TODO: ์์ด๋ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ } + ) Text( text = " | ", style = DoMaTypography.labelLarge.copy(color = colors.WHITE), ) - Text( - text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", - modifier = Modifier.clickable( - interactionSource = remember { MutableInteractionSource() }, - indication = rememberRipple(), - onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ์ฒ๋ฆฌ */ } - ), - style = DoMaTypography.labelLarge.copy(color = colors.WHITE), - ) + ClickableText( + text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", + onClick = { /* TODO: ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ํ๋ฉด์ผ๋ก ์ด๋ */ } + ) }
110-117: ๐ ๏ธ Refactor suggestionํ์๊ฐ์ ๋ค๋น๊ฒ์ด์ ๋ฐ ์ ๊ทผ์ฑ ๊ฐ์ ํ์
ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ๋ค๋น๊ฒ์ด์ ํ๋ ๋ก์ง์ด ๊ตฌํ๋์ง ์์์ต๋๋ค. ์ค์ ํ๋ฉด ์ ํ ๊ธฐ๋ฅ์ ์ถ๊ฐํด์ฃผ์ธ์. ๋ํ ์ ๊ทผ์ฑ ์ค๋ช ์ด ๊ฐ๊ฒฐํ์ง๋ง, ๋ ๊ตฌ์ฒด์ ์ธ ์ค๋ช ์ผ๋ก ์ ๋ฐ์ดํธํ๋ฉด ์ข๊ฒ ์ต๋๋ค.
Text( text = "ํ์๊ฐ์ ํ๊ธฐ", modifier = Modifier - .clickable { /* TODO: ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ์ด๋ */ } - .semantics { contentDescription = "ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋" }, + .clickable { /* TODO: ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ์ด๋ํ๋ ๋ก์ง ๊ตฌํ */ } + .semantics { + contentDescription = "์๋ก์ด ๊ณ์ ์ ๋ง๋ค๊ธฐ ์ํ ํ์๊ฐ์ ํ์ด์ง๋ก ์ด๋ํฉ๋๋ค" + }, style = DoMaTypography.labelLarge.copy(color = colors.WHITE), textAlign = TextAlign.Center )
120-127:โ ๏ธ Potential issue๋ณด์ ๊ฐ์ ๋ฐ ๋ณ์๋ช ์์ ์ด ํ์ํฉ๋๋ค.
ํด๋ผ์ด์ธํธ ID์ ๋ฆฌ๋ค์ด๋ ํธ URI๊ฐ ํ๋์ฝ๋ฉ๋์ด ์์ด ๋ณด์์ ์ํํฉ๋๋ค. ๋ํ
clickButton๋ณ์๋ช ์ด ๋ช ํํ์ง ์์ต๋๋ค.-if (clickButton) { +if (showGoogleAuthWebView) { GAuthSigninWebView( - clientId = "ghskfend", - redirectUri = "ghskfen" + clientId = BuildConfig.GOOGLE_AUTH_CLIENT_ID, + redirectUri = BuildConfig.GOOGLE_AUTH_REDIRECT_URI ) { code -> gAuthLogin(code) } }
๐งน Nitpick comments (5)
presentation/src/main/java/view/theme/color/DoMaColor.kt (1)
5-14: ์์ ์์ ๋ช ๋ช ๊ท์น์ ์ผ๊ด๋๊ฒ ์ ์ฉํด์ฃผ์ธ์.ํ์ฌ ํ์ผ์๋ ๋๋ฌธ์๋ก ๋ ์์ ์์(
MAIN,WHITE,GRAY)์ ํ์ค์นผ ์ผ์ด์ค๋ก ๋ ์์ ์์(Black,Gold,silver,Broze)๊ฐ ํผํฉ๋์ด ์์ต๋๋ค. ์๋ก ์ถ๊ฐ๋ ์์ ์์๋ค์ฒ๋ผ ๊ธฐ์กด ์์ ์์๋ค๋ ๋ชจ๋ ๋๋ฌธ์๋ก ํต์ผํ๋ ๊ฒ์ด ์ข์ต๋๋ค.override val MAIN = Color(0xFF252525) -override val Black = Color(0xFF000000) +override val BLACK = Color(0xFF000000) override val WHITE = Color(0xFFFFFFFF) override val GRAY = Color(0xFF74C27D) -override val Gold = Color(0xFFFFD651) -override val silver = Color(0xFFBEBEBE) -override val Broze = Color(0xFFF7A080) +override val GOLD = Color(0xFFFFD651) +override val SILVER = Color(0xFFBEBEBE) +override val BRONZE = Color(0xFFF7A080)์ฐธ๊ณ : "Broze"๋ ์ฒ ์ ์ค๋ฅ๊ฐ ์์ด "BRONZE"๋ก ์์ ํ์ต๋๋ค.
presentation/src/main/java/view/textfield/DoMaTextField.kt (4)
102-102: ํ๋์ฝ๋ฉ๋ ์์ ๊ฐ์ ์์ ์์๋ก ๊ต์ฒดํด์ฃผ์ธ์.์์ ๊ฐ
Color(0xFF4C4C4C)๊ฐ ํ๋์ฝ๋ฉ๋์ด ์์ต๋๋ค. ์ด ๊ฐ์DoMaColor.kt์ ์ถ๊ฐ๋LOGIN์์๊ณผ ๋์ผํฉ๋๋ค. ์์ ํ ๋ง ์ผ๊ด์ฑ์ ์ํด ํ๋์ฝ๋ฉ๋ ๊ฐ ๋์colors.LOGIN์ ์ฌ์ฉํ๋ ๊ฒ์ด ์ข์ต๋๋ค.- color = if (isError) colors.RED else Color(0xFF4C4C4C), + color = if (isError) colors.RED else colors.LOGIN,
81-84: ๊ณต๋ฐฑ ์ ๊ฑฐ์ ์ต๋ ๊ธธ์ด ์ ํ ๋ก์ง์ด ํจ๊ป ์์ต๋๋ค.ํ์ฌ ๊ตฌํ์ ์ ๋ ฅ๊ฐ์ ๊ณต๋ฐฑ์ ๋ชจ๋ ์ ๊ฑฐํ๊ณ ์์ต๋๋ค. ์ด๋ ์์ด๋๋ ๋น๋ฐ๋ฒํธ ์ ๋ ฅ์๋ ์ ํฉํ ์ ์์ง๋ง, ๋ค๋ฅธ ์ฉ๋์ ํ ์คํธ ํ๋์์๋ ์์น ์๋ ๋์์ผ ์ ์์ต๋๋ค. ๊ณต๋ฐฑ ์ ๊ฑฐ ์ต์ ์ ๋งค๊ฐ๋ณ์๋ก ๋ถ๋ฆฌํ๋ ๊ฒ์ ๊ณ ๋ คํด๋ณด์ธ์.
fun DoMaTextField( ... + removeWhitespace: Boolean = true, ... ) { ... onValueChange = { if (it.length <= maxLength) { // ์ต๋ ๊ธ์ ์ ์ ํ - val filteredText = it.filterNot { text -> text.isWhitespace() } + val filteredText = if (removeWhitespace) it.filterNot { text -> text.isWhitespace() } else it onValueChange(filteredText) } }, ... }
35-35: ๋ถํ์ํ ์์ ์ ์์ ๋๋ค.
EMPTY์์๋ ๋จ์ํ ๋น ๋ฌธ์์ด์ ๋ํ๋ด๋๋ฐ, ์ด๋ ์ฝ๋ ๊ฐ๋ ์ฑ์ ํฅ์์ํค์ง ์์ผ๋ฉฐ Kotlin์์๋ ์ผ๋ฐ์ ์ผ๋ก ์ง์ ""๋ฅผ ์ฌ์ฉํฉ๋๋ค. ํด๋น ์์๋ฅผ ์ ๊ฑฐํ๊ณ ํ์ํ ๊ณณ์ ์ง์ ๋น ๋ฌธ์์ด์ ์ฌ์ฉํ๋ ๊ฒ์ด ์ข์ต๋๋ค.-const val EMPTY = "" - @Composable fun DoMaTextField( modifier: Modifier = Modifier, debounceTime: Long = 300L, - placeHolder: String = EMPTY, + placeHolder: String = "", ... ) { ... }
137-140: Preview ํจ์์์ ๋ ๋ค์ํ ์ํ๋ฅผ ์์ฐํ๋ฉด ์ข์ ๊ฒ ๊ฐ์ต๋๋ค.ํ์ฌ Preview ํจ์๋ ๊ธฐ๋ณธ ์ํ๋ง ๋ณด์ฌ์ฃผ๊ณ ์์ต๋๋ค. ์๋ฌ ์ํ, ํฌ์ปค์ค ์ํ ๋ฑ ๋ค์ํ ์ํ๋ฅผ ๋ฏธ๋ฆฌ ๋ณผ ์ ์๋๋ก ์ฌ๋ฌ Preview ํจ์๋ฅผ ์ถ๊ฐํ๋ ๊ฒ์ด ์ข์ต๋๋ค.
@Preview @Composable private fun PreView() { DoMaTextField( setText = "๊ฒ์", onSearchTextChange = {} ) } + +@Preview +@Composable +private fun PreViewWithError() { + DoMaTextField( + setText = "๊ฒ์", + isError = true, + errorMessage = "์๋ชป๋ ์ ๋ ฅ์ ๋๋ค.", + onSearchTextChange = {} + ) +}
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
โ Files ignored due to path filters (3)
buildSrc/build/kotlin/compileKotlin/cacheable/last-build.binis excluded by!**/*.binbuildSrc/build/kotlin/compileKotlin/local-state/build-history.binis excluded by!**/*.binbuildSrc/build/libs/buildSrc.jaris excluded by!**/*.jar
๐ Files selected for processing (13)
.idea/other.xml(7 hunks)presentation/src/main/java/until/Result.kt(1 hunks)presentation/src/main/java/view/signin/Screen/SignInScreen.kt(1 hunks)presentation/src/main/java/view/signin/Screen/SignInSubScreen.kt(1 hunks)presentation/src/main/java/view/signin/component/DoMaInputID.kt(1 hunks)presentation/src/main/java/view/signin/component/DoMaInputPassWord.kt(1 hunks)presentation/src/main/java/view/signin/component/DoMaLoginButton.kt(1 hunks)presentation/src/main/java/view/textfield/DoMaTextField.kt(1 hunks)presentation/src/main/java/view/theme/color/ColorTheme.kt(1 hunks)presentation/src/main/java/view/theme/color/DoMaColor.kt(1 hunks)presentation/src/main/java/viewModel/admin/AdminViewModel.kt(6 hunks)presentation/src/main/java/viewModel/homes/HomesViewmodel.kt(1 hunks)presentation/src/main/java/viewModel/users/UsersViewModel.kt(1 hunks)
โ Files skipped from review due to trivial changes (4)
- presentation/src/main/java/until/Result.kt
- presentation/src/main/java/viewModel/users/UsersViewModel.kt
- presentation/src/main/java/viewModel/admin/AdminViewModel.kt
- presentation/src/main/java/viewModel/homes/HomesViewmodel.kt
๐ง Files skipped from review as they are similar to previous changes (5)
- presentation/src/main/java/view/signin/component/DoMaInputID.kt
- presentation/src/main/java/view/theme/color/ColorTheme.kt
- presentation/src/main/java/view/signin/component/DoMaLoginButton.kt
- presentation/src/main/java/view/signin/component/DoMaInputPassWord.kt
- presentation/src/main/java/view/signin/Screen/SignInSubScreen.kt
โฐ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
๐ Additional comments (11)
.idea/other.xml (8)
17-27: OnePlus CPH2415 ๊ธฐ๊ธฐ ์ถ๊ฐ ํ์ธ:
์๋ก์ด OnePlus CPH2415 ๊ธฐ๊ธฐ์ ๋ํ ๋ฐ์ดํฐ๊ฐ ์ถ๊ฐ๋์์ต๋๋ค. ๋ชจ๋ ์ต์ ( api, brand, codename, id, manufacturer, name, screenDensity, screenX, screenY )์ด ๋ช ์ธ์ ๋ง๊ฒ ์ฌ๋ฐ๋ฅด๊ฒ ์ค์ ๋์๋์ง ํ์ธํด ์ฃผ์ธ์.
72-82: Samsung A35 ๊ธฐ๊ธฐ ์ถ๊ฐ ํ์ธ:
์๋ก์ด Samsung A35 ๊ธฐ๊ธฐ ๋ฐ์ดํฐ๊ฐ ์ถ๊ฐ๋์์ต๋๋ค. ์ ๋ ฅ๋ ๊ฐ( api, brand, codename, id, manufacturer, name, screenDensity, screenX, screenY )๋ค์ด ๊ธฐ๊ธฐ ์คํ๊ณผ ์ผ์นํ๋์ง ๊ฒํ ํด ์ฃผ์ธ์.
116-126: Motorola moto g 5G (2022) ๊ธฐ๊ธฐ ์ถ๊ฐ ํ์ธ:
์ ๊ท Motorola ๊ธฐ๊ธฐ 'moto g 5G (2022)' ๋ฐ์ดํฐ๊ฐ ์ถ๊ฐ๋์์ต๋๋ค. ๊ธฐ์ข ๋ฐ ํ๋ฉด ์ ๋ณด(api, screenDensity, screenX, screenY)๊ฐ ์ต์ ์ฌ์์ ๋ฐ์ํ๋์ง ํ์ธ ๋ถํ๋๋ฆฝ๋๋ค.
293-301: Samsung ๊ธฐ๊ธฐ ๋ณ๊ฒฝ์ฌํญ (SM-T738U):
๊ธฐ์กด Samsung ๊ธฐ๊ธฐ์ codename๊ณผ id๊ฐ "gts7xllite"๋ก ๋ณ๊ฒฝ๋์๊ณ , ๊ธฐ๊ธฐ ์ด๋ฆ์ด "SM-T738U"๋ก ์ ๋ฐ์ดํธ๋์์ต๋๋ค. ๋ณ๊ฒฝ ์๋๊ฐ ๋ช ํํด ๋ณด์ด๋ฉฐ, ๊ด๋ จ ์์ฑ๋ค์ด ์ผ๊ด์ฑ ์๊ฒ ์ ์ฉ๋์๋์ง ์ฌ๊ฒํ ํด์ฃผ์ธ์.
314-324: Samsung Galaxy Tab S8 ๊ธฐ๊ธฐ ์ถ๊ฐ ํ์ธ:
์๋ก์ด Samsung Galaxy Tab S8 ๊ธฐ๊ธฐ ๋ฐ์ดํฐ๊ฐ ์ถ๊ฐ๋์์ต๋๋ค. ํนํ ์คํฌ๋ฆฐ ๋ฐ๋์ ํด์๋ ๊ด๋ จ ์ต์ ๋ค์ด ์ฌ๋ฐ๋ฅด๊ฒ ์ค์ ๋์๋์ง ํ์ธํด ์ฃผ์ธ์.
325-335: Samsung Galaxy Tab S9 FE 5G ๊ธฐ๊ธฐ ์ถ๊ฐ ํ์ธ:
์ ๊ท Samsung Galaxy Tab S9 FE 5G ๊ธฐ๊ธฐ ๋ฐ์ดํฐ๊ฐ ์ถ๊ฐ๋์์ต๋๋ค. ๊ธฐ๊ธฐ ์ฌ์์ ๋ถํฉํ๋์ง, ํ๋ฉด ๊ด๋ จ ์ต์ ( screenDensity, screenX, screenY )๋ค์ด ์ ํํ์ง ๊ฒํ ๋ถํ๋๋ฆฝ๋๋ค.
391-401: Samsung Galaxy S21 ๊ธฐ๊ธฐ ์ถ๊ฐ ํ์ธ:
์๋ก์ด Samsung Galaxy S21 ๊ธฐ๊ธฐ ๋ฐ์ดํฐ๊ฐ ์ถ๊ฐ๋์์ต๋๋ค. ์ ๋ ฅ๋ ์ต์ ๊ฐ๋ค์ด ์ต์ ๊ธฐ๊ธฐ ์คํ๊ณผ ์ผ์นํ๋์ง ํ์ธํด ์ฃผ์ธ์.
491-501: Samsung Galaxy S21 Plus ๊ธฐ๊ธฐ ์ถ๊ฐ ํ์ธ:
์ ๊ท Samsung Galaxy S21 Plus ๊ธฐ๊ธฐ ๋ฐ์ดํฐ๊ฐ ์ถ๊ฐ๋์์ต๋๋ค. ๋ชจ๋ ์ต์ (ํนํ screenDensity, screenX, screenY)์ด ์ ํํ์ง ์ฌ๊ฒํ ๋ถํ๋๋ฆฝ๋๋ค.presentation/src/main/java/view/theme/color/DoMaColor.kt (1)
20-23: ์์ ์์ ์ถ๊ฐ๋ฅผ ์ ๊ตฌํํ์ต๋๋ค!์ถ๊ฐ๋ ์์ ์์๋ค์ด ๋ชจ๋ ๋๋ฌธ์๋ก ์ผ๊ด์ฑ ์๊ฒ ๊ตฌํ๋์ด ์ข์ต๋๋ค. ์ด๋ ์ด์ ๋ฆฌ๋ทฐ์์ ์ ์ํ ๋ช ๋ช ๊ท์น์ ์ ์ ํ ๋ฐ์ํ์์ต๋๋ค.
presentation/src/main/java/view/textfield/DoMaTextField.kt (1)
37-62: ๋งค๊ฐ๋ณ์ ๊ตฌ์ฑ์ด ์ ๋์ด ์์ต๋๋ค.TextField ์ปดํฌ๋ํธ์ ํ์ํ ๋งค๊ฐ๋ณ์๋ค์ด ์ ๊ตฌ์ฑ๋์ด ์์ต๋๋ค. ์ต๋ ๋ฌธ์ ์ ์ ํ, ์๋ฌ ์ํ ์ฒ๋ฆฌ, ํค๋ณด๋ ์ก์ ๋ฑ ๋ค์ํ ๊ธฐ๋ฅ์ ์ง์ํ๋๋ก ์ค๊ณ๋์์ต๋๋ค.
presentation/src/main/java/view/signin/Screen/SignInScreen.kt (1)
38-44: ์ปฌ๋ผ ๋ฐฐ์น ๋ฐ ์ ๋ ฌ์ด ์ ๊ตฌํ๋์ด ์์ต๋๋ค.ํ๋ฉด์ ์๋จ, ์ค๋จ, ํ๋จ ๋ฐฐ์น๊ฐ ์ ๊ตฌํ๋์ด ์์ผ๋ฉฐ, ํนํ
weight์์ฑ์ ์ฌ์ฉํ์ฌ ์ ๋์ ์ธ ๊ณต๊ฐ ๋ฐฐ์น๋ฅผ ํ ์ ์ด ์ข์ต๋๋ค.
| @Composable | ||
| fun PreviewSignInScreen() { | ||
| SignInScreen( | ||
| clickButton = false, | ||
| gAuthLogin = {}, | ||
| leIsClickLoginButton = {} | ||
| ) | ||
| } No newline at end of 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.
๐ ๏ธ Refactor suggestion
Preview ํจ์์์ ๋งค๊ฐ๋ณ์ ์ด๋ฆ ๋ณ๊ฒฝ์ด ํ์ํฉ๋๋ค.
Preview ํจ์์์๋ clickButton ๋งค๊ฐ๋ณ์ ์ด๋ฆ์ showGoogleAuthWebView๋ก ๋ณ๊ฒฝํ๊ณ , leIsClickLoginButton์ onLoginButtonClick์ผ๋ก ๋ณ๊ฒฝํด์ผ ํฉ๋๋ค.
@Preview
@Composable
fun PreviewSignInScreen() {
SignInScreen(
- clickButton = false,
+ showGoogleAuthWebView = false,
gAuthLogin = {},
- leIsClickLoginButton = {}
+ onLoginButtonClick = {}
)
}๐ 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.
| @Composable | |
| fun PreviewSignInScreen() { | |
| SignInScreen( | |
| clickButton = false, | |
| gAuthLogin = {}, | |
| leIsClickLoginButton = {} | |
| ) | |
| } | |
| @Composable | |
| fun PreviewSignInScreen() { | |
| SignInScreen( | |
| - clickButton = false, | |
| + showGoogleAuthWebView = false, | |
| gAuthLogin = {}, | |
| - leIsClickLoginButton = {} | |
| + onLoginButtonClick = {} | |
| ) | |
| } |
| modifier: Modifier = Modifier, | ||
| clickButton: Boolean, | ||
| gAuthLogin: (String) -> Unit, | ||
| leIsClickLoginButton: () -> Unit, |
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.
๐ ๏ธ Refactor suggestion
๋งค๊ฐ๋ณ์ ์ด๋ฆ์ ๋ ๋ช ํํ๊ฒ ์ง์ ํด์ฃผ์ธ์.
clickButton ๋งค๊ฐ๋ณ์ ์ด๋ฆ์ด ๋ช
ํํ์ง ์์ต๋๋ค. ์ด ๋ถ๋ฆฌ์ธ ๊ฐ์ด ๋ฌด์์ ๋ํ๋ด๋์ง ๋ช
ํํ ์ ์ ์๋๋ก showGoogleAuthWebView์ ๊ฐ์ด ๋ ๊ตฌ์ฒด์ ์ธ ์ด๋ฆ์ผ๋ก ๋ณ๊ฒฝํ๋ ๊ฒ์ด ์ข์ต๋๋ค. ๋ํ leIsClickLoginButton ํจ์๋ช
๋ ๋ช
ํํ์ง ์์ผ๋ onLoginButtonClick๊ณผ ๊ฐ์ด ๋ณ๊ฒฝํ๋ ๊ฒ์ด ์ข์ต๋๋ค.
fun SignInScreen(
modifier: Modifier = Modifier,
- clickButton: Boolean,
+ showGoogleAuthWebView: Boolean,
gAuthLogin: (String) -> Unit,
- leIsClickLoginButton: () -> Unit,
+ onLoginButtonClick: () -> Unit,
) {๐ 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.
| modifier: Modifier = Modifier, | |
| clickButton: Boolean, | |
| gAuthLogin: (String) -> Unit, | |
| leIsClickLoginButton: () -> Unit, | |
| fun SignInScreen( | |
| modifier: Modifier = Modifier, | |
| showGoogleAuthWebView: Boolean, | |
| gAuthLogin: (String) -> Unit, | |
| onLoginButtonClick: () -> Unit, | |
| ) { |
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: 5
๐งน Nitpick comments (5)
presentation/src/main/java/view/signin/screen/SignInScreen.kt (2)
27-28: ๋งค๊ฐ๋ณ์ ์ด๋ฆ ๊ฐ์ ํ์
leIsClickLoginButton์ ์ด๋ฆ์ด ์ง๊ด์ ์ด์ง ์์ต๋๋ค. ๋งค๊ฐ๋ณ์ ์ด๋ฆ์ ๊ทธ ์ฉ๋๋ฅผ ๋ช ํํ ํด์ผ ํฉ๋๋ค.๋ค์๊ณผ ๊ฐ์ด ๋งค๊ฐ๋ณ์ ์ด๋ฆ์ ๊ฐ์ ํ๋ ๊ฒ์ด ์ข์ต๋๋ค:
- leIsClickLoginButton: () -> Unit, + onLoginButtonClick: () -> Unit,๊ทธ๋ฆฌ๊ณ ๋งค๊ฐ๋ณ์ ์ฌ์ฉ ์์น๋ ํจ๊ป ๋ณ๊ฒฝํด์ผ ํฉ๋๋ค:
- onClick = leIsClickLoginButton, + onClick = onLoginButtonClick,
119-127: PreviewSignInScreen ํจ์์ ๋งค๊ฐ๋ณ์ ์ด๋ฆ ๋ณ๊ฒฝ ํ์๋งค๊ฐ๋ณ์ ์ด๋ฆ์ ๋ณ๊ฒฝํ๋ฉด ๋ฏธ๋ฆฌ๋ณด๊ธฐ ํจ์๋ ์ผ๊ด์ฑ ์๊ฒ ์์ ํด์ผ ํฉ๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ์์ ํ์ธ์:
@Preview @Composable fun PreviewSignInScreen() { SignInScreen( clickButton = false, gAuthLogin = {}, - leIsClickLoginButton = {} + onLoginButtonClick = {} ) }presentation/src/main/java/view/signin/screen/SignInSubScreen.kt (3)
32-32: ๋งค๊ฐ๋ณ์ ์ด๋ฆ ๊ฐ์ ํ์
leIsClickLoginButton์ ์ด๋ฆ์ด ์ง๊ด์ ์ด์ง ์์ต๋๋ค. ๋งค๊ฐ๋ณ์ ์ด๋ฆ์ ๊ทธ ์ฉ๋๋ฅผ ๋ช ํํ ํด์ผ ํฉ๋๋ค.๋ค์๊ณผ ๊ฐ์ด ๋งค๊ฐ๋ณ์ ์ด๋ฆ์ ๊ฐ์ ํ๋ ๊ฒ์ด ์ข์ต๋๋ค:
- leIsClickLoginButton: () -> Unit, + onLoginButtonClick: () -> Unit,๊ทธ๋ฆฌ๊ณ ๋งค๊ฐ๋ณ์ ์ฌ์ฉ ์์น๋ ํจ๊ป ๋ณ๊ฒฝํด์ผ ํฉ๋๋ค:
- onClick = leIsClickLoginButton, + onClick = onLoginButtonClick,
95-103: PreviewSignInSubScreen ํจ์์ ๋งค๊ฐ๋ณ์ ์ด๋ฆ ๋ณ๊ฒฝ ํ์๋งค๊ฐ๋ณ์ ์ด๋ฆ์ ๋ณ๊ฒฝํ๋ฉด ๋ฏธ๋ฆฌ๋ณด๊ธฐ ํจ์๋ ์ผ๊ด์ฑ ์๊ฒ ์์ ํด์ผ ํฉ๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ์์ ํ์ธ์:
@Preview @Composable fun PreviewSignInSubScreen() { SignInSubScreen( clickButton = false, gAuthLogin = {}, - leIsClickLoginButton = {} + onLoginButtonClick = {} ) }
1-103: SignInScreen.kt์ ์ค๋ณต ์ฝ๋ ๋ฐ ๊ตฌ์กฐ ์ฌ๊ฒํ ํ์SignInScreen.kt์ SignInSubScreen.kt ํ์ผ ๊ฐ์ ์ค๋ณต๋ ์ฝ๋์ ์ ์ฌํ ๊ตฌ์กฐ๊ฐ ์์ต๋๋ค. ๋ ํ๋ฉด์ ์ฐจ์ด์ ๊ณผ ๊ณตํต์ ์ ๊ณ ๋ คํ์ฌ ์ฝ๋๋ฅผ ์ฌ๊ตฌ์ฑํ๋ ๊ฒ์ด ์ข์ ๊ฒ ๊ฐ์ต๋๋ค.
๋ค์ ์ฌํญ์ ๊ณ ๋ คํด๋ณด์ธ์:
- ๋ ํ๋ฉด์ ๊ด๊ณ๋ฅผ ๋ค์ ๊ฒํ ํ๊ณ ํ์ํ ๊ฒฝ์ฐ ํ๋์ ํ๋ฉด์ผ๋ก ํตํฉํ๊ฑฐ๋ ๋ ์ด์์์ ๊ณต์ ํ๋ ๋ฐฉ์์ผ๋ก ์ฌ๊ตฌ์ฑ
- ๊ณตํต ์์(์: GAuthSigninWebView, ๋ฒํผ ์ฒ๋ฆฌ ๋ฑ)๋ ๋ณ๋์ ์ปดํฌ๋ํธ๋ก ์ถ์ถ
- ์ฝ๋ ๊ตฌ์กฐ๋ฅผ ๊ฒํ ํ์ฌ SignInScreen์ด SignInSubScreen์ ํฌํจํ๊ฑฐ๋ ๊ทธ ๋ฐ๋์ ๊ตฌ์กฐ๊ฐ ๋ ์ ์ ํ์ง ํ์ธ
์๋ฅผ ๋ค์ด:
- SignInScreen์ด ๋ฉ์ธ ์ปจํ ์ด๋ ์ญํ ์ ํ๊ณ SignInSubScreen์ ๋ด๋ถ์ ํฌํจํ๋ ๊ตฌ์กฐ
- ๋๋ ๋ ํ๋ฉด์ ์์ ํ ๋ณ๊ฐ์ ํ๋ฉด์ผ๋ก ๋ถ๋ฆฌํ๊ณ ๊ณตํต ๋ก์ง์ ViewModel์ด๋ ๊ณต์ ์ปดํฌ๋ํธ๋ก ์ถ์ถ
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
โ Files ignored due to path filters (3)
buildSrc/build/kotlin/compileKotlin/cacheable/last-build.binis excluded by!**/*.binbuildSrc/build/kotlin/compileKotlin/local-state/build-history.binis excluded by!**/*.binbuildSrc/build/libs/buildSrc.jaris excluded by!**/*.jar
๐ Files selected for processing (2)
presentation/src/main/java/view/signin/screen/SignInScreen.kt(1 hunks)presentation/src/main/java/view/signin/screen/SignInSubScreen.kt(1 hunks)
| clientId = "ghskfend", | ||
| redirectUri = "ghskfen" |
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.
ํด๋ผ์ด์ธํธ ID์ ๋ฆฌ๋ค์ด๋ ํธ URI๋ฅผ ์์๋ก ๋ถ๋ฆฌ ํ์
ํ์ฌ "ghskfend"์ "ghskfen"์ ํ ์คํธ ๋๋ ์์ ๊ฐ์ผ๋ก ๋ณด์ ๋๋ค. ์ด๋ฌํ ๊ฐ์ ์์๋ก ๋ถ๋ฆฌํ๊ฑฐ๋ ๊ตฌ์ฑ ํ์ผ์์ ๊ฐ์ ธ์ค๋ ๊ฒ์ด ์ข์ต๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ์์ ํ์ธ์:
- ์์ ํด๋์ค๋ฅผ ์์ฑํ๊ฑฐ๋ ๊ธฐ์กด ์์ ํด๋์ค์ ์ถ๊ฐ:
object AuthConfig {
const val CLIENT_ID = "์ค์ _ํด๋ผ์ด์ธํธ_ID"
const val REDIRECT_URI = "์ค์ _๋ฆฌ๋ค์ด๋ ํธ_URI"
}- ํด๋น ์์๋ฅผ ์ฌ์ฉ:
GAuthSigninWebView(
- clientId = "ghskfend",
- redirectUri = "ghskfen"
+ clientId = AuthConfig.CLIENT_ID,
+ redirectUri = AuthConfig.REDIRECT_URI
) { code ->
gAuthLogin(code)
}| Text( | ||
| text = "ํ์๊ฐ์ ํ๊ธฐ", | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| textAlign = TextAlign.Center | ||
| ) |
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.
๐ ๏ธ Refactor suggestion
ํ์๊ฐ์ ํ ์คํธ์ clickable ์์ ์ ์ถ๊ฐ ํ์
"ํ์๊ฐ์ ํ๊ธฐ" ํ ์คํธ๋ ์ฌ์ฉ์๊ฐ ํด๋ฆญํ ์ ์๋ ์์๋ก ๋ณด์ด์ง๋ง, clickable ์์ ์๊ฐ ์์ด ์ค์ ๋ก๋ ํด๋ฆญํ ์ ์์ต๋๋ค.
๋ค์๊ณผ ๊ฐ์ด clickable ์์ ์์ ํด๋ฆญ ํธ๋ค๋ฌ๋ฅผ ์ถ๊ฐํ์ธ์:
Text(
text = "ํ์๊ฐ์
ํ๊ธฐ",
+ modifier = Modifier
+ .clickable { /* ํ์๊ฐ์
ํ๋ฉด์ผ๋ก ์ด๋ */ }
+ .semantics { contentDescription = "ํ์๊ฐ์
ํ๊ธฐ" },
style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
textAlign = TextAlign.Center
)๐ 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.
| Text( | |
| text = "ํ์๊ฐ์ ํ๊ธฐ", | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| textAlign = TextAlign.Center | |
| ) | |
| Text( | |
| text = "ํ์๊ฐ์ ํ๊ธฐ", | |
| modifier = Modifier | |
| .clickable { /* ํ์๊ฐ์ ํ๋ฉด์ผ๋ก ์ด๋ */ } | |
| .semantics { contentDescription = "ํ์๊ฐ์ ํ๊ธฐ" }, | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| textAlign = TextAlign.Center | |
| ) |
| Text( | ||
| text = "์์ด๋ ์ฐพ๊ธฐ", | ||
| modifier = Modifier, | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| ) | ||
| Text( | ||
| text = " | ", | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| ) | ||
| Text( | ||
| text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", | ||
| modifier = Modifier, | ||
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | ||
| ) | ||
| } |
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.
๐ ๏ธ Refactor suggestion
ํด๋ฆญ ๊ฐ๋ฅํ ํ ์คํธ์ clickable ์์ ์ ์ถ๊ฐ ํ์
"์์ด๋ ์ฐพ๊ธฐ"์ "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ" ํ ์คํธ๋ ์ฌ์ฉ์๊ฐ ํด๋ฆญํ ์ ์๋ ์์๋ก ๋ณด์ด์ง๋ง, clickable ์์ ์๊ฐ ์์ด ์ค์ ๋ก๋ ํด๋ฆญํ ์ ์์ต๋๋ค.
๋ค์๊ณผ ๊ฐ์ด clickable ์์ ์์ ํด๋ฆญ ํธ๋ค๋ฌ๋ฅผ ์ถ๊ฐํ์ธ์:
Text(
text = "์์ด๋ ์ฐพ๊ธฐ",
- modifier = Modifier,
+ modifier = Modifier
+ .clickable { /* ์์ด๋ ์ฐพ๊ธฐ ๊ธฐ๋ฅ ์คํ */ }
+ .semantics { contentDescription = "์์ด๋ ์ฐพ๊ธฐ" },
style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
)
Text(
text = " | ",
style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
)
Text(
text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ",
- modifier = Modifier,
+ modifier = Modifier
+ .clickable { /* ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ๊ธฐ๋ฅ ์คํ */ }
+ .semantics { contentDescription = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ" },
style = DoMaTypography.labelLarge.copy(color = colors.WHITE),
)๐ 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.
| Text( | |
| text = "์์ด๋ ์ฐพ๊ธฐ", | |
| modifier = Modifier, | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| Text( | |
| text = " | ", | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| Text( | |
| text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", | |
| modifier = Modifier, | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| } | |
| Text( | |
| text = "์์ด๋ ์ฐพ๊ธฐ", | |
| modifier = Modifier | |
| .clickable { /* ์์ด๋ ์ฐพ๊ธฐ ๊ธฐ๋ฅ ์คํ */ } | |
| .semantics { contentDescription = "์์ด๋ ์ฐพ๊ธฐ" }, | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| Text( | |
| text = " | ", | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| Text( | |
| text = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ", | |
| modifier = Modifier | |
| .clickable { /* ๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ ๊ธฐ๋ฅ ์คํ */ } | |
| .semantics { contentDescription = "๋น๋ฐ๋ฒํธ ์ฐพ๊ธฐ" }, | |
| style = DoMaTypography.labelLarge.copy(color = colors.WHITE), | |
| ) | |
| } |
| clientId = "ghskfend", | ||
| redirectUri = "ghskfen" |
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.
ํด๋ผ์ด์ธํธ ID์ ๋ฆฌ๋ค์ด๋ ํธ URI๋ฅผ ์์๋ก ๋ถ๋ฆฌ ํ์
ํ์ฌ "ghskfend"์ "ghskfen"์ ํ ์คํธ ๋๋ ์์ ๊ฐ์ผ๋ก ๋ณด์ ๋๋ค. ์ด๋ฌํ ๊ฐ์ ์์๋ก ๋ถ๋ฆฌํ๊ฑฐ๋ ๊ตฌ์ฑ ํ์ผ์์ ๊ฐ์ ธ์ค๋ ๊ฒ์ด ์ข์ต๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ์์ ํ์ธ์:
- ์์ ํด๋์ค๋ฅผ ์์ฑํ๊ฑฐ๋ ๊ธฐ์กด ์์ ํด๋์ค์ ์ถ๊ฐ:
object AuthConfig {
const val CLIENT_ID = "์ค์ _ํด๋ผ์ด์ธํธ_ID"
const val REDIRECT_URI = "์ค์ _๋ฆฌ๋ค์ด๋ ํธ_URI"
}- ํด๋น ์์๋ฅผ ์ฌ์ฉ:
GAuthSigninWebView(
- clientId = "ghskfend",
- redirectUri = "ghskfen"
+ clientId = AuthConfig.CLIENT_ID,
+ redirectUri = AuthConfig.REDIRECT_URI
) { code ->
gAuthLogin(code)
}| DoMaInputID(modifier = Modifier.fillMaxWidth()) | ||
|
|
||
| Spacer(modifier = Modifier.height(12.dp)) | ||
|
|
||
| DoMaInputPassword(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.
๐ ๏ธ Refactor suggestion
์ ๋ ฅ ํ๋์ ์ํ ๊ด๋ฆฌ ๋ฐ ์ถ๊ฐ ๊ธฐ๋ฅ ํ์
DoMaInputID์ DoMaInputPassword ์ปดํฌ๋ํธ์๋ ์
๋ ฅ ๊ฐ์ ๊ด๋ฆฌํ๋ ์ํ์ PR ์ฝ๋ฉํธ์์ ์ธ๊ธ๋ ์ต๋ ๊ธ์ ์ ์ ํ, ํค๋ณด๋ ์ก์
, ์ค๋ฅ ์ฒ๋ฆฌ ๋ฑ์ ๊ธฐ๋ฅ์ด ์์ต๋๋ค.
์๋์ ๊ฐ์ด ์ํ ๊ด๋ฆฌ์ ์ถ๊ฐ ๊ธฐ๋ฅ์ ๊ตฌํํ์ธ์:
+ import androidx.compose.runtime.mutableStateOf
+ import androidx.compose.runtime.remember
+ import androidx.compose.ui.text.input.ImeAction
@Composable
fun SignInSubScreen(
modifier: Modifier = Modifier,
clickButton: Boolean,
gAuthLogin: (String) -> Unit,
- leIsClickLoginButton: () -> Unit,
+ onLoginButtonClick: () -> Unit,
) {
DoMaAndroidTheme { colors, typography ->
+ val (id, setId) = remember { mutableStateOf("") }
+ val (password, setPassword) = remember { mutableStateOf("") }
+ val (idError, setIdError) = remember { mutableStateOf<String?>(null) }
+ val (passwordError, setPasswordError) = remember { mutableStateOf<String?>(null) }
Column(
/* ... ๊ธฐ์กด ์ฝ๋ ... */
) {
/* ... ๊ธฐ์กด ์ฝ๋ ... */
- DoMaInputID(modifier = Modifier.fillMaxWidth())
+ DoMaInputID(
+ value = id,
+ onValueChange = {
+ if (it.length <= 20) { // ์ต๋ ๊ธธ์ด ์ ํ
+ setId(it)
+ setIdError(null)
+ }
+ },
+ error = idError,
+ imeAction = ImeAction.Next,
+ modifier = Modifier.fillMaxWidth()
+ )
Spacer(modifier = Modifier.height(12.dp))
- DoMaInputPassword(modifier = Modifier.fillMaxWidth())
+ DoMaInputPassword(
+ value = password,
+ onValueChange = {
+ if (it.length <= 20) { // ์ต๋ ๊ธธ์ด ์ ํ
+ setPassword(it)
+ setPasswordError(null)
+ }
+ },
+ error = passwordError,
+ imeAction = ImeAction.Done,
+ onDone = {
+ // ์ ํจ์ฑ ๊ฒ์ฌ ๋ก์ง
+ if (id.isEmpty()) {
+ setIdError("์์ด๋๋ฅผ ์
๋ ฅํด์ฃผ์ธ์")
+ } else if (password.isEmpty()) {
+ setPasswordError("๋น๋ฐ๋ฒํธ๋ฅผ ์
๋ ฅํด์ฃผ์ธ์")
+ } else {
+ onLoginButtonClick()
+ }
+ },
+ modifier = Modifier.fillMaxWidth()
+ )์ถ๊ฐ๋ก DoMaInputID.kt์ DoMaInputPassword.kt ํ์ผ์๋ ํ์ํ ๋งค๊ฐ๋ณ์์ ๊ธฐ๋ฅ์ ๊ตฌํํด์ผ ํฉ๋๋ค.
๐ 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.
| DoMaInputID(modifier = Modifier.fillMaxWidth()) | |
| Spacer(modifier = Modifier.height(12.dp)) | |
| DoMaInputPassword(modifier = Modifier.fillMaxWidth()) | |
| import androidx.compose.runtime.mutableStateOf | |
| import androidx.compose.runtime.remember | |
| import androidx.compose.ui.text.input.ImeAction | |
| @Composable | |
| fun SignInSubScreen( | |
| modifier: Modifier = Modifier, | |
| clickButton: Boolean, | |
| gAuthLogin: (String) -> Unit, | |
| onLoginButtonClick: () -> Unit, | |
| ) { | |
| DoMaAndroidTheme { colors, typography -> | |
| val (id, setId) = remember { mutableStateOf("") } | |
| val (password, setPassword) = remember { mutableStateOf("") } | |
| val (idError, setIdError) = remember { mutableStateOf<String?>(null) } | |
| val (passwordError, setPasswordError) = remember { mutableStateOf<String?>(null) } | |
| Column( | |
| /* ... ๊ธฐ์กด ์ฝ๋ ... */ | |
| ) { | |
| /* ... ๊ธฐ์กด ์ฝ๋ ... */ | |
| DoMaInputID( | |
| value = id, | |
| onValueChange = { | |
| if (it.length <= 20) { // ์ต๋ ๊ธธ์ด ์ ํ | |
| setId(it) | |
| setIdError(null) | |
| } | |
| }, | |
| error = idError, | |
| imeAction = ImeAction.Next, | |
| modifier = Modifier.fillMaxWidth() | |
| ) | |
| Spacer(modifier = Modifier.height(12.dp)) | |
| DoMaInputPassword( | |
| value = password, | |
| onValueChange = { | |
| if (it.length <= 20) { // ์ต๋ ๊ธธ์ด ์ ํ | |
| setPassword(it) | |
| setPasswordError(null) | |
| } | |
| }, | |
| error = passwordError, | |
| imeAction = ImeAction.Done, | |
| onDone = { | |
| // ์ ํจ์ฑ ๊ฒ์ฌ ๋ก์ง | |
| if (id.isEmpty()) { | |
| setIdError("์์ด๋๋ฅผ ์ ๋ ฅํด์ฃผ์ธ์") | |
| } else if (password.isEmpty()) { | |
| setPasswordError("๋น๋ฐ๋ฒํธ๋ฅผ ์ ๋ ฅํด์ฃผ์ธ์") | |
| } else { | |
| onLoginButtonClick() | |
| } | |
| }, | |
| 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.
Actionable comments posted: 1
โป๏ธ Duplicate comments (5)
presentation/src/main/java/view/signin/component/DoMaInputPassWord.kt (2)
23-63:โ ๏ธ Potential issue๋น๋ฐ๋ฒํธ ์ ๋ ฅ ํ๋์ ๋ณด์ ๊ธฐ๋ฅ์ด ๋๋ฝ๋์์ต๋๋ค.
๋น๋ฐ๋ฒํธ ์ ๋ ฅ ํ๋์ ๋ค์ ๋ณด์ ๊ธฐ๋ฅ๋ค์ ์ถ๊ฐํด์ฃผ์ธ์:
- ๋น๋ฐ๋ฒํธ ๋ง์คํน (PasswordVisualTransformation)
- ๋น๋ฐ๋ฒํธ ํ์/์จ๊น ํ ๊ธ
- ๋น๋ฐ๋ฒํธ ์ ๋ ฅ ํ์ ์ค์
๋ค์๊ณผ ๊ฐ์ด ์์ ํด์ฃผ์ธ์:
+import androidx.compose.material.Icon +import androidx.compose.material.IconButton +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Visibility +import androidx.compose.material.icons.filled.VisibilityOff +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.text.input.PasswordVisualTransformation +import androidx.compose.ui.text.input.VisualTransformation @Composable fun DoMaInputPassword( modifier: Modifier = Modifier, passwordValue: TextFieldValue, onPasswordValueChange: (TextFieldValue) -> Unit ) { + var passwordVisible by remember { mutableStateOf(false) } DoMaAndroidTheme { colors, typography -> Column( modifier = modifier ) { // ... ๊ธฐ์กด ์ฝ๋ ... OutlinedTextField( modifier = Modifier.fillMaxWidth(), value = passwordValue, onValueChange = onPasswordValueChange, + visualTransformation = if (passwordVisible) VisualTransformation.None else PasswordVisualTransformation(), + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Password), + trailingIcon = { + IconButton(onClick = { passwordVisible = !passwordVisible }) { + Icon( + imageVector = if (passwordVisible) Icons.Default.Visibility else Icons.Default.VisibilityOff, + contentDescription = if (passwordVisible) "๋น๋ฐ๋ฒํธ ์จ๊ธฐ๊ธฐ" else "๋น๋ฐ๋ฒํธ ํ์", + tint = colors.WHITE + ) + } + }, // ... ๊ธฐ์กด ์ฝ๋ ... ) } } }
54-59: ๐ ๏ธ Refactor suggestion์์ ์ฐธ์กฐ๊ฐ ์ผ๊ด๋์ง ์์ต๋๋ค.
DoMaAndroidTheme์์ ์ ๊ณตํ๋ colors ๋ณ์ ๋์ ์ง์ DoMaColor๋ฅผ ์ฐธ์กฐํ๊ณ ์์ต๋๋ค. ์ปดํฌ๋ํธ ์ ์ฒด์์ ์ผ๊ด๋ ๋ฐฉ์์ผ๋ก ์์์ ์ฐธ์กฐํด์ผ ํฉ๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ์์ ํด์ฃผ์ธ์:
colors = TextFieldDefaults.outlinedTextFieldColors( - backgroundColor = DoMaColor.BACKGROUND, - cursorColor = DoMaColor.WHITE, - focusedBorderColor = DoMaColor.GRAY, - unfocusedBorderColor = DoMaColor.GRAY + backgroundColor = colors.BACKGROUND, + cursorColor = colors.WHITE, + focusedBorderColor = colors.GRAY, + unfocusedBorderColor = colors.GRAY )presentation/src/main/java/view/signin/screen/SignInSubScreen.kt (3)
76-88: ๐ ๏ธ Refactor suggestion์ ๋ ฅ ํ๋์ ์ ํจ์ฑ ๊ฒ์ฌ์ ์ค๋ฅ ์ฒ๋ฆฌ ๊ธฐ๋ฅ์ด ์์ต๋๋ค.
ID์ ๋น๋ฐ๋ฒํธ ์ ๋ ฅ ํ๋์ ์ ํจ์ฑ ๊ฒ์ฌ์ ์ค๋ฅ ์ฒ๋ฆฌ ๊ธฐ๋ฅ์ ์ถ๊ฐํด์ผ ํฉ๋๋ค. PR ์ฝ๋ฉํธ์์ ์ธ๊ธ๋ ๊ฒ์ฒ๋ผ ์ต๋ ๊ธ์ ์ ์ ํ, ์ค๋ฅ ์ฒ๋ฆฌ ๋ฑ์ ๊ธฐ๋ฅ์ด ํ์ํฉ๋๋ค.
DoMaInputID์ DoMaInputPassword ์ปดํฌ๋ํธ์ ์ค๋ฅ ์ํ์ ๋ฉ์์ง๋ฅผ ์ ๋ฌํ๊ณ , ์ด๋ฅผ ํ๋ฉด์์ ๊ด๋ฆฌํด์ผ ํฉ๋๋ค. ๋ค์๊ณผ ๊ฐ์ด ์ํ๋ฅผ ์ถ๊ฐํ๊ณ ๊ด๋ฆฌํ๋ ๊ฒ์ด ์ข๊ฒ ์ต๋๋ค:
fun SignInSubScreen( // ๊ธฐ์กด ํ๋ผ๋ฏธํฐ๋ค ) { DoMaAndroidTheme { colors, typography -> + val (idError, setIdError) = remember { mutableStateOf<String?>(null) } + val (passwordError, setPasswordError) = remember { mutableStateOf<String?>(null) } // ๊ธฐ์กด ์ฝ๋ DoMaInputID( modifier = Modifier.fillMaxWidth(), idValue = idValue, onIdValueChange = { newValue -> + // ์ต๋ ๊ธธ์ด ์ ํ (์: 20์) + if (newValue.text.length <= 20) { onIdValueChange(newValue) + setIdError(null) + } }, + error = idError, + imeAction = ImeAction.Next ) Spacer(modifier = Modifier.height(12.dp)) DoMaInputPassword( modifier = Modifier.fillMaxWidth(), passwordValue = passwordValue, onPasswordValueChange = { newValue -> + // ์ต๋ ๊ธธ์ด ์ ํ (์: 20์) + if (newValue.text.length <= 20) { onPasswordValueChange(newValue) + setPasswordError(null) + } }, + error = passwordError, + imeAction = ImeAction.Done, + onDone = { + // ๋ก๊ทธ์ธ ๋ฒํผ ํด๋ฆญ ์ ์ ํจ์ฑ ๊ฒ์ฌ + if (idValue.text.isEmpty()) { + setIdError("์์ด๋๋ฅผ ์ ๋ ฅํด์ฃผ์ธ์") + } else if (passwordValue.text.isEmpty()) { + setPasswordError("๋น๋ฐ๋ฒํธ๋ฅผ ์ ๋ ฅํด์ฃผ์ธ์") + } else { + onLoginButtonClick() + } + } )
99-104:โ ๏ธ Potential issueํด๋ผ์ด์ธํธ ID์ ๋ฆฌ๋ค์ด๋ ํธ URI๋ฅผ ์์๋ก ๋ถ๋ฆฌ ํ์
ํ์ฌ "ghskfend"์ "ghskfen"์ ํ ์คํธ ๋๋ ์์ ๊ฐ์ผ๋ก ๋ณด์ ๋๋ค. ์ด๋ฌํ ๊ฐ์ ์์๋ก ๋ถ๋ฆฌํ๊ฑฐ๋ ๊ตฌ์ฑ ํ์ผ์์ ๊ฐ์ ธ์ค๋ ๊ฒ์ด ์ข์ต๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ์์ ํ์ธ์:
- ์์ ํด๋์ค๋ฅผ ์์ฑํ๊ฑฐ๋ ๊ธฐ์กด ์์ ํด๋์ค์ ์ถ๊ฐ:
object AuthConfig { const val CLIENT_ID = "์ค์ _ํด๋ผ์ด์ธํธ_ID" const val REDIRECT_URI = "์ค์ _๋ฆฌ๋ค์ด๋ ํธ_URI" }
- ํด๋น ์์๋ฅผ ์ฌ์ฉ:
GAuthSigninWebView( - clientId = "ghskfend", - redirectUri = "ghskfen" + clientId = AuthConfig.CLIENT_ID, + redirectUri = AuthConfig.REDIRECT_URI ) { code -> gAuthLogin(code) }
30-40: ๐ ๏ธ Refactor suggestionํจ์ ์ด๋ฆ๊ณผ ํ๋ผ๋ฏธํฐ ๋ค์ด๋ฐ์ด ์ผ๊ด๋์ง ์์ต๋๋ค.
ํจ์ ํ๋ผ๋ฏธํฐ ์ค
leIsClickLoginButton์ ๋ช ํํ์ง ์๊ณ ์ผ๊ด๋ ๋ค์ด๋ฐ ์ปจ๋ฒค์ ์ ๋ฐ๋ฅด์ง ์์ต๋๋ค.๋ค์๊ณผ ๊ฐ์ด ์์ ํด์ฃผ์ธ์:
fun SignInSubScreen( modifier: Modifier = Modifier, clickButton: Boolean, gAuthLogin: (String) -> Unit, - leIsClickLoginButton: () -> Unit, + onLoginButtonClick: () -> Unit, idValue: TextFieldValue, onIdValueChange: (TextFieldValue) -> Unit, passwordValue: TextFieldValue, onPasswordValueChange: (TextFieldValue) -> Unit )๊ทธ๋ฆฌ๊ณ ์ฌ์ฉํ๋ ๊ณณ๋ ํจ๊ป ์์ ํด์ฃผ์ธ์:
DoMaLoginButton( - onClick = leIsClickLoginButton, + onClick = onLoginButtonClick, // ... ๋๋จธ์ง ์ฝ๋ )
๐งน Nitpick comments (2)
presentation/src/main/java/view/signin/component/DoMaInputID.kt (1)
37-56: TextField์ ์ถ๊ฐ ๊ธฐ๋ฅ ๊ตฌํ์ด ํ์ํฉ๋๋ค.PR ์ฝ๋ฉํธ์์ ์ธ๊ธ๋ ๊ฒ์ฒ๋ผ TextField์ ์ต๋ ๊ธ์ ์ ์ ํ, ํค๋ณด๋ ์ก์ , ์ค๋ฅ ์ฒ๋ฆฌ ๋ฑ์ ๊ธฐ๋ฅ์ด ๊ตฌํ๋์ด ์์ง ์์ต๋๋ค.
๋ค์๊ณผ ๊ฐ์ ๊ธฐ๋ฅ์ ์ถ๊ฐํ๋ ๊ฒ์ด ์ข๊ฒ ์ต๋๋ค:
OutlinedTextField( modifier = Modifier.fillMaxWidth(), value = idValue, onValueChange = onIdValueChange, + isError = error != null, + keyboardOptions = KeyboardOptions( + imeAction = imeAction, + keyboardType = KeyboardType.Text + ), + keyboardActions = KeyboardActions( + onNext = { onNext?.invoke() }, + onDone = { onDone?.invoke() } + ), textStyle = DoMaTypography.labelLarge.copy(color = colors.WHITE), shape = RoundedCornerShape(12.dp), singleLine = true, placeholder = { Text( text = "์์ด๋๋ฅผ ์ ๋ ฅํด์ฃผ์ธ์.", style = DoMaTypography.labelLarge.copy(color = colors.silver) ) }, + supportingText = error?.let { + { Text(text = it, color = colors.ERROR) } + }, colors = TextFieldDefaults.outlinedTextFieldColors( backgroundColor = colors.BACKGROUND, cursorColor = colors.WHITE, focusedBorderColor = colors.GRAY, unfocusedBorderColor = colors.GRAY ) )๊ทธ๋ฆฌ๊ณ ํจ์ ํ๋ผ๋ฏธํฐ์ ๋ค์์ ์ถ๊ฐํด์ฃผ์ธ์:
fun DoMaInputID( modifier: Modifier = Modifier, idValue: TextFieldValue, onIdValueChange: (TextFieldValue) -> Unit, + error: String? = null, + imeAction: ImeAction = ImeAction.Next, + onNext: (() -> Unit)? = null, + onDone: (() -> Unit)? = null )presentation/src/main/java/view/signin/component/DoMaInputPassWord.kt (1)
67-74: ๋น๋ฐ๋ฒํธ ์ปดํฌ๋ํธ์ ์ถ๊ฐ ๊ธฐ๋ฅ ๊ตฌํ์ด ํ์ํฉ๋๋ค.ID ์ ๋ ฅ ์ปดํฌ๋ํธ์ ๋ง์ฐฌ๊ฐ์ง๋ก ์ต๋ ๊ธ์ ์ ์ ํ, ํค๋ณด๋ ์ก์ , ์ค๋ฅ ์ฒ๋ฆฌ ๋ฑ์ ๊ธฐ๋ฅ์ ์ถ๊ฐํด์ผ ํฉ๋๋ค.
๋ค์๊ณผ ๊ฐ์ ํ๋ผ๋ฏธํฐ์ ๊ธฐ๋ฅ์ ์ถ๊ฐํ๋ ๊ฒ์ด ์ข๊ฒ ์ต๋๋ค:
fun DoMaInputPassword( modifier: Modifier = Modifier, passwordValue: TextFieldValue, onPasswordValueChange: (TextFieldValue) -> Unit, + error: String? = null, + imeAction: ImeAction = ImeAction.Done, + onDone: (() -> Unit)? = null )๊ทธ๋ฆฌ๊ณ TextField์ ๊ด๋ จ ์์ฑ๋ค์ ์ถ๊ฐํด์ฃผ์ธ์.
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
โ Files ignored due to path filters (3)
buildSrc/build/kotlin/compileKotlin/cacheable/last-build.binis excluded by!**/*.binbuildSrc/build/kotlin/compileKotlin/local-state/build-history.binis excluded by!**/*.binbuildSrc/build/libs/buildSrc.jaris excluded by!**/*.jar
๐ Files selected for processing (4)
presentation/src/main/java/view/signin/component/DoMaInputID.kt(1 hunks)presentation/src/main/java/view/signin/component/DoMaInputPassWord.kt(1 hunks)presentation/src/main/java/view/signin/screen/SignInSubScreen.kt(1 hunks)presentation/src/main/java/view/theme/color/DoMaColor.kt(2 hunks)
๐ง Files skipped from review as they are similar to previous changes (1)
- presentation/src/main/java/view/theme/color/DoMaColor.kt
๐ Additional comments (1)
presentation/src/main/java/view/signin/component/DoMaInputID.kt (1)
19-59: ์ปดํฌ๋ํธ๊ฐ Stateless๋ก ๊ตฌํ๋์ด ์ข์ต๋๋ค.์ํ๋ฅผ ์ธ๋ถ์์ ์ฃผ์ ๋ฐ๋๋ก ๊ตฌํํ์ฌ ์ปดํฌ๋ํธ์ ์ฌ์ฌ์ฉ์ฑ๊ณผ ํ ์คํธ ์ฉ์ด์ฑ์ด ํฅ์๋์์ต๋๋ค. ์ด๋ ์ด์ ๋ฆฌ๋ทฐ ์๊ฒฌ์ ์ ๋ฐ์ํ ๊ฒ์ ๋๋ค.
| DoMaLoginButton( | ||
| onClick = leIsClickLoginButton, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(bottom = 30.dp) | ||
| ) |
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.
๐ ๏ธ Refactor suggestion
๋ก๊ทธ์ธ ๋ฒํผ ํด๋ฆญ ์ ์ ๋ ฅ ์ ํจ์ฑ ๊ฒ์ฌ ๋ก์ง์ด ์์ต๋๋ค.
๋ก๊ทธ์ธ ๋ฒํผ ํด๋ฆญ ์ ์์ด๋์ ๋น๋ฐ๋ฒํธ์ ์ ํจ์ฑ์ ๊ฒ์ฌํ๋ ๋ก์ง์ด ์ถ๊ฐ๋์ด์ผ ํฉ๋๋ค.
๋ก๊ทธ์ธ ๋ฒํผ ํด๋ฆญ ํธ๋ค๋ฌ๋ฅผ ๋ค์๊ณผ ๊ฐ์ด ์์ ํ์ธ์:
DoMaLoginButton(
- onClick = leIsClickLoginButton,
+ onClick = {
+ // ๋ก๊ทธ์ธ ๋ฒํผ ํด๋ฆญ ์ ์ ํจ์ฑ ๊ฒ์ฌ
+ if (idValue.text.isEmpty()) {
+ setIdError("์์ด๋๋ฅผ ์
๋ ฅํด์ฃผ์ธ์")
+ } else if (passwordValue.text.isEmpty()) {
+ setPasswordError("๋น๋ฐ๋ฒํธ๋ฅผ ์
๋ ฅํด์ฃผ์ธ์")
+ } else {
+ leIsClickLoginButton()
+ }
+ },
modifier = Modifier
.fillMaxWidth()
.padding(bottom = 30.dp)
)Committable suggestion skipped: line range outside the PR's diff.
| fun DoMaInputID( | ||
| modifier: Modifier = Modifier, | ||
| idValue: TextFieldValue, | ||
| onIdValueChange: (TextFieldValue) -> Unit |
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.
์ด๋ฌ๋ฉด ๋ญ๊ฐ ์ํ๊ฐ ๊ทธ๋๋ก ์ผ๊ฒ ๊ฐ์ผ๋ viewmodel ์ด์ฉํด์ฃผ์ธ์
๐ก ๊ฐ์
SignIn ํ์ด์ง๋ฅผ ํผ๋ธ๋ฆฌ์ฑ ํ์ต๋๋ค.
๐ ์์ ๋ด์ฉ
DoMaInputID, DoMaInputPassWord,DoMaLoginButton, Signinํ์ด์ง ๊ตฌํ
๐ ๋ณ๊ฒฝ์ฌํญ
Signin Page์ screen๊ณผ component
๐โโ๏ธ ์ง๋ฌธ์ฌํญ
๐ด ์ฌ์ฉ๋ฐฉ๋ฒ
๐ธ ๊ธฐํ
Summary by CodeRabbit