Week7 7주차 필수과제 구현 #13
Conversation
| val imageOverviews: List<ImageOverviewViewState> = emptyList(), | ||
| val imageItemsType1: List<Int> = emptyList(), | ||
| val imageItemsType2: List<Int> = emptyList(), |
| ) | ||
| val categories: LiveData<List<String>> = _categories | ||
| ) : BaseViewModel<HomeContract.State, HomeContract.Event, HomeContract.Effect>( | ||
| initialState = HomeContract.State() |
| ) : UiState | ||
|
|
||
| sealed class Event : UiEvent { | ||
| data class SignUpButtonClicked( | ||
| val username: String, | ||
| val userId: String, |
| @@ -23,7 +23,7 @@ import org.sopt.and.presentation.theme.WavveTheme | |||
|
|
|||
| @Composable | |||
| fun ProfileContainer( | |||
| hobbyName: String, | |||
| hobbyName: String = "", | |||
There was a problem hiding this comment.
Optional parameter는 Modifier의 아래로 내려가면 좋을 것 같습니다!!
| checkSingUpEnable() | ||
| } | ||
|
|
||
| private fun checkSingUpEnable() { |
There was a problem hiding this comment.
| private fun checkSingUpEnable() { | |
| private fun checkSignUpEnable() { |
깨알 오타...ㅎㅎ
| val imageItemsType1 by viewModel.imageItemsType1.observeAsState(emptyList()) | ||
| val imageItemsType2 by viewModel.imageItemsType2.observeAsState(emptyList()) | ||
| val uiState by viewModel.uiState.collectAsStateWithLifecycle() | ||
| val effects = viewModel.effect.collectAsState(initial = null).value |
There was a problem hiding this comment.
effect를 상태로 선언하기 보다는, collect하는 형태가 되면 좋을 것 같습니당
| sealed class Effect : UiEffect { | ||
| data object NavigateToSearch : Effect() | ||
| data object NavigateToDetailScreen : Effect() | ||
| } |
There was a problem hiding this comment.
UiEffect 는 어떤 의미에서의 이름일까요?? 뭔가 거꾸로 된 것 같은 느낌도 있네요 Effect 라는 Base 를 상속받은 HomeEffect 라는 이름의 클래스로 있는게 조금 더 익숙해보여요 Event 도요!
| val categories by viewModel.categories.observeAsState(emptyList()) | ||
| val imageItemsType1 by viewModel.imageItemsType1.observeAsState(emptyList()) | ||
| val imageItemsType2 by viewModel.imageItemsType2.observeAsState(emptyList()) | ||
| val uiState by viewModel.uiState.collectAsStateWithLifecycle() |
There was a problem hiding this comment.
uiState 필드 값 하나만 바뀌면 recomposition 이 어떻게 이루어질까요!?
| onDismissRequest = { viewModel.updateDialogStatus(false) }, | ||
| title = { Text(stringResource(id = R.string.login_fail)) }, | ||
| text = { Text(stringResource(id = R.string.login_fail_message)) }, | ||
| confirmButton = { | ||
| TextButton( | ||
| onClick = { viewModel.dismissDialog() } | ||
| onClick = { viewModel.updateDialogStatus(false) } |
There was a problem hiding this comment.
해당 이벤트들은 SignInContract.Event 로 안 빼신 이유가 있을까요??
| fun updateDialogStatus(isDialogShown: Boolean) { | ||
| updateState(currentState.copy(isDialogShown = isDialogShown)) | ||
| } | ||
|
|
||
| fun onPasswordInputChange(newInput: String) { | ||
| passwordInput = newInput | ||
| fun updateId(userId: String) { | ||
| updateState(currentState.copy(userId = userId)) | ||
| } | ||
|
|
||
| fun dismissDialog() { | ||
| showDialog = false | ||
| fun updatePassword(password: String) { | ||
| updateState(currentState.copy(password = password)) |
There was a problem hiding this comment.
해당 함수들은 오로지 이벤트로만 동작되게 하기 위해서 private 처리 한 후에 작업하면 패턴 적용이 조금 더 빠르게 될 것 같아요!
There was a problem hiding this comment.
이벤트로 viewModel에게 알려주는 부분과, 직접 viewModel 을 접근하는 부분을 통일 시켜야 할 것 같아요 Intent 로 알려줄 수 있게요!
| val isLoading: Boolean = false, | ||
| val categories: List<String> = emptyList(), | ||
| val imageOverviews: List<ImageOverviewViewState> = emptyList(), | ||
| val imageItemsType1: List<Int> = emptyList(), | ||
| val imageItemsType2: List<Int> = emptyList(), |
There was a problem hiding this comment.
앱잼 전에 persistantlist에 대해서도 알아가시면 좋을 거 같아요!
| sealed class Event : UiEvent { | ||
| data class ComponentClicked( | ||
| val id: Int | ||
| ) : Event() // 특정 이미지 클릭시 특정 화면으로 이동(미구현) | ||
|
|
||
| data class CategoryClicked( | ||
| val category: String | ||
| ) : Event() | ||
| } | ||
|
|
||
| sealed class Effect : UiEffect { | ||
| data object NavigateToSearch : Effect() | ||
| data object NavigateToDetailScreen : Effect() |
There was a problem hiding this comment.
혹시 uievent, uieffect, uistate을 분리된 파일에서 사용해 보신 적이 있으실까요..?
저는 아직 얘네를 한 파일에서 사용하는 것이 익숙치 않은데, 이렇게 사용하는 것이 코드를 구현하기에 한눈에 보여서 더 쉽거나, 편리했을지 궁금합니다!
There was a problem hiding this comment.
state를 변경하는 로직을 한 곳에서 관리해도 좋을 것 같습니다 ~
Related issue 🛠
Work Description ✏️
Screenshot 📸
이전과 화면은 동일합니다.
Uncompleted Tasks 😅
To Reviewers 📢
기존에 이미 MVI 패턴이 적용된 상태라 기존 Status 관리가 미흡했던 부분 수정했습니다.