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] 7주차 필수 과제 #14

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

[FEAT] 7주차 필수 과제 #14

wants to merge 6 commits into from

Conversation

beom84
Copy link
Contributor

@beom84 beom84 commented Dec 17, 2024

Related issue 🛠

Work Description ✏️

  • MVI 적용

Screenshot 📸

9pr.mp4

Uncompleted Tasks 😅

  • 에러처리
  • 팟장님 코리

To Reviewers 📢

아직 에러처리와 코리 적용을 못했습니다 시험이끝나고 돌아와서 바로 해보겠습니다 ㅠㅠ

Copy link

@MinseoSONG MinseoSONG left a comment

Choose a reason for hiding this comment

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

7차 과제도 수고하셨습니다 !!

Comment on lines +11 to +13
fun Context.serverToast(message: String? = "") {
Toast.makeText(this, message, Toast.LENGTH_SHORT).show()
}

Choose a reason for hiding this comment

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

toast와 serverToast를 무슨 차이일까요 ?

Comment on lines +15 to +17
sealed interface HomeSideEffect : UiSideEffect

sealed class HomeEvent : UiEvent

Choose a reason for hiding this comment

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

효빈님께도 드린 질문인데, 만약 sideeffect와 event가 없더라도 만들어두는게 좋을까요 ? 저는 안만들어서 ..

Comment on lines +49 to +55
LoadState.Idle -> {}

LoadState.Loading -> {}

LoadState.Success -> MyProfileScreen(myUiState = uiState)

LoadState.Error -> {}

Choose a reason for hiding this comment

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

Idle은 뭘까요 ! 그리고 비어있는 건 빈 상태로 두는게 좋은지, 애초에 추가하지 않는게 좋은지 궁금합니다 !

Copy link

@Hyobeen-Park Hyobeen-Park left a comment

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.

저 이제 발견했는데요 파일명에 오타있음!!ㅋㅋㅋㅋ



@ExperimentalPermissionsApi
@OptIn(ExperimentalPermissionsApi::class)

Choose a reason for hiding this comment

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

이 어노테이션을 추가하신 이유가 무엇인가요?? 어떤 역할을 하는 어노테이션일까요?

Comment on lines +48 to +56
when (uiState.loadState) {
LoadState.Idle -> {}

LoadState.Loading -> {}

LoadState.Success -> MyProfileScreen(myUiState = uiState)

LoadState.Error -> {}
}

Choose a reason for hiding this comment

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

success를 제외하면 다 빈 코드인데 else를 사용하지 않고 다 명시해준 이유가 무엇인가요????

Comment on lines +44 to +49
setEvent(
MyProfileContract.ProfileEvent.FetchUserHobby(
loadState = LoadState.Loading,
userHobby = currentState.hobby
)
)

Choose a reason for hiding this comment

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

event를 뷰모델 내에서도 수정하시네요!! 저는

event = 사용자의 intent -> 뷰에서만 변경됨!!

이렇게 생각했었는데 뷰모델 내에서도 setEvent를 해도 괜찮나요??(진짜 모름)

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 -152 to +166
HomeScreen()
HomeRoute()
Copy link
Contributor

Choose a reason for hiding this comment

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

변경하신 이유가 있나요?

}

when (uiState.loadState) {
Copy link
Contributor

Choose a reason for hiding this comment

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

각 상태에 따라 적절한 화면을 보여주는 것이 좋아보여요!

import androidx.compose.material.icons.filled.CheckCircle

object SignUpImage{
val EXTRA_SIGNUP_IMAGE_LIST = listOf(
Copy link
Contributor

Choose a reason for hiding this comment

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

immutable이나 stable하게 선언하면 좋을 것 같네요 !

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주차 필수과제
4 participants