-
Notifications
You must be signed in to change notification settings - Fork 0
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
[feat] 로그인페이지 네비게이션 #69
Conversation
core/data/build.gradle.kts
Outdated
@@ -2,6 +2,7 @@ plugins { | |||
alias(libs.plugins.dreamdiary.android.library) | |||
alias(libs.plugins.ksp) | |||
alias(libs.plugins.room) | |||
id("com.google.gms.google-services") |
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.
version catalog로 통일하는 것이 더 좋을 것 같습니다.
// firebase | ||
implementation(libs.firebase.firestore) | ||
implementation(libs.googleid) | ||
implementation(libs.play.services.auth) | ||
implementation(libs.firebase.auth.ktx) |
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.
주석과 함께 카테고리를 통일해주시는 것이 좋을 것 같습니다
private val googleLogInDataSource: GoogleLogInDataSource, | ||
private val authRepository: AuthRepository, | ||
) { | ||
suspend operator fun invoke(data: Intent?): Result<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.
result 클래스를 사용할 때
특별한 목적이 없다면 runcatching 블럭을 사용하는 것이 좋지 않을까요?
} catch (e: Exception) { | ||
Result.failure(e) | ||
} |
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.
모듈이 분리되어 있기 때문에
throwable을 그냥 전달하는 것은 위험할 수 있을 것 같습니다.
object NotLogin : LoginState() | ||
|
||
object Success : LoginState() |
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.
data object를 사용하는 것은 어떨까요?
gradle/libs.versions.toml
Outdated
firebase-firestore = { group = "com.google.firebase", name = "firebase-firestore", version.ref = "firebaseFirestore" } | ||
googleid = { group = "com.google.android.libraries.identity.googleid", name = "googleid", version.ref = "googleid" } | ||
play-services-auth = { module = "com.google.android.gms:play-services-auth", version.ref = "playServicesAuth" } |
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.
그래들 플러그인과 다른 부분에 작성해주시는 것이 좋을 것 같습니다
} | ||
|
||
fun loginWithGoogle(data: Intent?) { | ||
viewModelScope.launch { |
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.
잘 몰라서 여쭙습니다.
dispatcher.io
io 스레드에서 작동하는 것과 차이가 있을까요?
return try { | ||
val account = task.await() | ||
account.idToken | ||
} catch (e: ApiException) { |
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.
ApiExeption말고 처리가 안되는 exception이 있을 수도 있으니 catch (e : Excetpion)을 쓰는 것은 어떨까요?
@@ -0,0 +1,4 @@ | |||
<resources> | |||
<string name="app_name">dreamdiary</string> | |||
<string name="google_client_id">105289032792-nfs3tv76a867aajjt5q4n95rogtp1dje.apps.googleusercontent.com</string> |
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.
이것은 들어가도 되는거 맞겠죠..?
onGitHubLogInClick: () -> Unit = {}, | ||
onGoogleLogInClick: () -> Unit = {}, | ||
onNotLogInClick: () -> Unit = {}, | ||
modifier: 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.
여기에 기본값이 있어야하나요? 기본값이 있어야한다면 modifer는 선택적 매개변수의 첫번째입니다.
Co-authored-by: easyhak
close #6
close #7