Skip to content
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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

kangyein9892
Copy link
Contributor

@kangyein9892 kangyein9892 commented Dec 17, 2024

Related issue 🛠

Work Description ✏️

  • mvi 설정(baseviewmodel, uistate, uiSideEffect, uiEvent)

Screenshot 📸

화면 바뀐 부분은 없습니다 !

To Reviewers 📢

  • 화면 이동 되면서 스낵바가 안 떠서 delay를 줬는데 좋은 방법은 아닌 것 같아요... 다른 방법이 있을까요 ? ㅠㅠ
  • 찐 mvi는 처음이라... 많은 피드백 부탁드립니다...~~
  • 그리고 myevent에서 getMyHobby 이런식으로 진행을 했는데... event 처리나 그런것떄문에 당장은 이렇게 했는데... 이것도 뭔가 좋은 방법이 아닌 것 같아서... 피드백 부탁드려요...~

@kangyein9892 kangyein9892 self-assigned this Dec 17, 2024
@kangyein9892 kangyein9892 linked an issue Dec 17, 2024 that may be closed by this pull request
3 tasks
Copy link

@serioushyeon serioushyeon left a 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()

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 -> {

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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay를 주는 이유가 무엇인가용??

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 있고 아래도 있던데 저도 궁금합니다!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 궁금합니당

Copy link

@jm991014 jm991014 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!! 깔끔하네용ㅎㅎ

Comment on lines +68 to +70
is SignInSideEffect.SnackBarText -> snackBarHostState.showSnackbar(sideEffect.message)
SignInSideEffect.NavigateToSignUp -> {
navigateToSignUp()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에도 있는 질문인데 is의 유무로 인해 어떤 차이가 있는 건가요!?

Copy link
Contributor

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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 있고 아래도 있던데 저도 궁금합니다!

Copy link

@boiledEgg-s boiledEgg-s left a 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)

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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI단에서 이미 코루틴을 사용하는 것으로 보이는데, suspend를 지우고 viewModelScope를 넣으신 이유가 있나요?

Copy link
Contributor

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생했어요

Comment on lines +68 to +70
is SignInSideEffect.SnackBarText -> snackBarHostState.showSnackbar(sideEffect.message)
SignInSideEffect.NavigateToSignUp -> {
navigateToSignUp()
Copy link
Contributor

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) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

석준 오빠 코드리뷰 읽어보시면 좋을 듯 합니다! 이해 안 되면 저 디코로 호출하3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 7주차 과제
5 participants