Merged
Conversation
leeeyubin
approved these changes
Aug 26, 2024
Comment on lines
+10
to
+11
| import androidx.compose.runtime.remember | ||
| import javax.inject.Singleton |
Comment on lines
+46
to
+51
| fun CalendarWeekRoute( | ||
| modifier: Modifier = Modifier, | ||
| calendarUiState: CalendarUiState, | ||
| navController: NavController = rememberNavController(), | ||
| viewModel: CalendarViewModel = hiltViewModel() | ||
| navigateToAnnouncement: (Long) -> Unit, | ||
| updateSelectedDate: (LocalDate) -> Unit, | ||
| viewModel: CalendarWeekViewModel = hiltViewModel() |
Member
There was a problem hiding this comment.
modifier는 옵셔널인 매개변수 아래에 넣어주면 좋을 것 같아요!
Comment on lines
+142
to
+143
| CalendarWeekSuccess( | ||
| scrapList = uiState.loadState.data.toImmutableList(), |
arinming
approved these changes
Aug 26, 2024
Contributor
arinming
left a comment
arinming
left a comment
There was a problem hiding this comment.
너무잘하시는데요,,,?ㅜㅜ 캘린더 고생하셨어요~~~~~!!!!!!!
Comment on lines
+49
to
+57
| BackHandler { | ||
| if (uiState.isWeekEnabled) { | ||
| viewModel.updateSelectedDate(uiState.selectedDate) | ||
| } else if (uiState.isListEnabled) { | ||
| viewModel.updateListVisibility(false) | ||
| } else { | ||
| navigateUp() | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
네이밍이 전체적으로 편안해서 백 핸들러가 어떻게 구현되어있는지 알기 쉬운 것 가타용
Comment on lines
+62
to
+63
| updateSelectedDate = viewModel::updateSelectedDate, | ||
| updatePage = viewModel::updatePage, |
Member
Author
There was a problem hiding this comment.
굳이 람다 안쓰고 깔끔하게 함수 호출이 가능해서 써봤습니당
Comment on lines
+32
to
57
| fun updatePage(page: Int) = viewModelScope.launch { | ||
| _uiState.update { currentState -> | ||
| currentState.copy( | ||
| isScrapButtonClicked = !currentState.isScrapButtonClicked, | ||
| scrapId = scrapId | ||
| currentPage = page | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| fun updateInternDialogVisible(visibility: Boolean = false) { | ||
| fun updateListVisibility( | ||
| visibility: Boolean | ||
| ) { | ||
| _uiState.update { currentState -> | ||
| currentState.copy( | ||
| isInternshipClicked = visibility | ||
| isListEnabled = visibility | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| fun updateInternshipModel(scrapDetailModel: CalendarScrapDetail?) { | ||
| fun updateWeekVisibility( | ||
| visibility: Boolean | ||
| ) { | ||
| _uiState.update { currentState -> | ||
| currentState.copy( | ||
| internshipModel = scrapDetailModel | ||
| isWeekEnabled = visibility | ||
| ) | ||
| } |
| scrapList: List<CalendarScrapDetail>, | ||
| onScrapButtonClicked: (Long) -> Unit, | ||
| onInternshipClicked: (CalendarScrapDetail) -> Unit, | ||
| modifier: Modifier = Modifier, |
Contributor
There was a problem hiding this comment.
modifier 사용 안하는 상태인데 일단 추가해두신 건가요??
Member
Author
There was a problem hiding this comment.
네넵!! 뭔가 빼긴 아쉬워서 추가만 해놨습니다,,
Comment on lines
+143
to
+155
| private fun getColorIndex(color: Color): Int = listOf( | ||
| CalRed, | ||
| CalOrange1, | ||
| CalOrange2, | ||
| CalYellow, | ||
| CalGreen1, | ||
| CalGreen2, | ||
| CalBlue1, | ||
| CalBlue2, | ||
| CalPurple, | ||
| CalPink | ||
| ).indexOf(color) | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
getColorIndex 함수가 모든 뷰모델마다 있는데 해당 함수를 전역적으로 사용할 수는 없을까용...!?! 궁금
Member
Author
There was a problem hiding this comment.
나중에 domain/type 패키지 안에 enum 클래스들을 만들어서 저런 선택지들을 관리하는게 좋을 것 같아요!!
다음 안드회의때 논의해봅시닷
Hyobeen-Park
approved these changes
Aug 28, 2024
Member
Hyobeen-Park
left a comment
Hyobeen-Park
left a comment
There was a problem hiding this comment.
정말 수고 많으셨습니다~~~!!!! 진짜 짱이다...💚
Comment on lines
+62
to
+63
| updateSelectedDate = viewModel::updateSelectedDate, | ||
| updatePage = viewModel::updatePage, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
⛳️ Work Description
📸 Screenshot
UI는 바뀌지 않았습니다!
📢 To Reviewers
길고 길었던 달력 코드 리팩토링이 끝났습니다!
원래 화면별로 나눠서 하려다, 하나 하고 리뷰받는 작업이 번거로워서 그냥 리팩토링을 한번에 진행했습니다!
바꾼게 많아서 제가 놓친 부분이 많을텐데 잘 봐주세요~:)