-
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/#12] 7주차 과제 구현 #13
base: develop
Are you sure you want to change the base?
Conversation
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.
mvi 적용이 넘넘 깔끔하네요!! 고생하셨습니다 ~~
data class SnackBar(@StringRes val message: Int): MySideEffect() | ||
data class SnackBarText(val message: String): MySideEffect() | ||
data object Logout: MySideEffect() | ||
data object NavigateToSignIn: MySideEffect() |
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.
네이밍 변경 좋네요!
val intent = _intent.asSharedFlow() | ||
override suspend fun handleEvent(event: MyEvent) { | ||
when(event) { | ||
MyEvent.OnLogoutButtonClick -> { |
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.
is를 붙이고 안붙이고의 차이는 뭔가요?!
fun onLogOutButtonClick() = viewModelScope.launch { | ||
_intent.emit(MySideEffect.Logout) | ||
private fun navigateToSignIn() = viewModelScope.launch { | ||
delay(100) |
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.
delay를 주는 이유가 무엇인가용??
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.
저도 궁금합니당
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.
고생하셨습니다!! 깔끔하네용ㅎㅎ
is SignInSideEffect.SnackBarText -> snackBarHostState.showSnackbar(sideEffect.message) | ||
SignInSideEffect.NavigateToSignUp -> { | ||
navigateToSignUp() |
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.
위에도 있는 질문인데 is의 유무로 인해 어떤 차이가 있는 건가요!?
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 onLogOutButtonClick() = viewModelScope.launch { | ||
_intent.emit(MySideEffect.Logout) | ||
private fun navigateToSignIn() = viewModelScope.launch { | ||
delay(100) |
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.
그동안 과제하느라 너무 고생많으셨어요!!
fun onLogOutButtonClick() = viewModelScope.launch { | ||
_intent.emit(MySideEffect.Logout) | ||
private fun navigateToSignIn() = viewModelScope.launch { | ||
delay(100) |
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 clearUserPreference() = viewModelScope.launch { | ||
userRepository.clearUserPreference() | ||
} | ||
|
||
suspend fun handleMyIntentError(message: String) { | ||
_intent.emit(MySideEffect.SnackBarText(message)) | ||
fun handleMyIntentError(message: String) = 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.
UI단에서 이미 코루틴을 사용하는 것으로 보이는데, suspend를 지우고 viewModelScope를 넣으신 이유가 있나요?
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.
고생했어요
is SignInSideEffect.SnackBarText -> snackBarHostState.showSnackbar(sideEffect.message) | ||
SignInSideEffect.NavigateToSignUp -> { | ||
navigateToSignUp() |
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.
저도 궁금해요
_state.update { | ||
it.copy(hobby = result.hobby) | ||
} | ||
setState { copy(hobby = result.hobby) } |
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.
석준 오빠 코드리뷰 읽어보시면 좋을 듯 합니다! 이해 안 되면 저 디코로 호출하3
Related issue 🛠
Work Description ✏️
Screenshot 📸
화면 바뀐 부분은 없습니다 !
To Reviewers 📢