-
Notifications
You must be signed in to change notification settings - Fork 1
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
[REFACTOR/#193] 홈 뷰 리팩토링 #203
Conversation
# Conflicts: # feature/src/main/java/com/terning/feature/home/changefilter/ChangeFilterRoute.kt # feature/src/main/java/com/terning/feature/home/home/HomeRoute.kt # feature/src/main/java/com/terning/feature/home/home/HomeViewModel.kt
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.
리팩토링까지 아주 굿입니당~!!!
viewModel.updateScrapDialogVisible(true) | ||
viewModel.updateIsToday(false) | ||
scrapId = index |
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.
뷰모델도 리컴포지션의 대상이 될 수 있어서 이 부분들도 Route로 빼주면 좋을 것 같긴 하네요..!
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.
오호!! 뷰모델도 리컴포지션이 될 수 있군요..!!!! 알려주셔서 감사합니다🥰🥰
HorizontalDivider( | ||
thickness = 4.dp, | ||
color = Grey150, | ||
modifier = Modifier | ||
.background(White) | ||
) { | ||
ShowRecommendTitle() | ||
ShowInternFilter(homeFilteringInfo = homeFilteringInfo, onChangeFilterClick) | ||
.fillMaxWidth(), | ||
) |
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.
HorizontalDivider
를 들어가보면 modifier에 fillMaxWidth
가 적용되어 있는 걸 볼 수 있을 거에요!
그래서 따로 지정해주지 않아도 될 것 같습니당
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 homeState get() = _homeState.asStateFlow() | ||
|
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를 하나로 묶은 건가요? 관리하기 더 용이해진 것 같네요!
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.
ViewModel에서 한번에 관리하니까 Route 단 코드가 넘나 간결해졌네용 ㅜ 대박
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.
수고하셨어요 ㅎ ㅎ 이넘클래스는 한번 얘기해봐야 될 것 같네욥.>!
val homeState get() = _homeState.asStateFlow() | ||
|
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.
ViewModel에서 한번에 관리하니까 Route 단 코드가 넘나 간결해졌네용 ㅜ 대박
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.
코드가 많이 간결해지고 깔끔해졌네요!!👍
너무 수고하셨슴다~
@@ -96,7 +96,7 @@ fun ChangeFilterScreen( | |||
topBar = { | |||
BackButtonTopAppBar( | |||
title = stringResource(id = R.string.change_filter_top_bar_title), | |||
onBackButtonClick = { navController.popBackStack() }, | |||
onBackButtonClick = { navigateToHome() }, |
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.
이 부분은 중괄호가 필요없지 않나요??
onBackButtonClick = { navigateToHome() }, | |
onBackButtonClick = navigateToHome, |
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.
웁씨... 맞아요ㅎㅎ 알려주셔서 감사합니당!!!
⛳️ Work Description
📸 Screenshot
UI는 바뀐거 없습니다!!
📢 To Reviewers
다이얼로그나 디자인 수정된 부분들은 제외하고 진행했습니다.
여러 곳에서 동시에 사용되는 enum class들을 한 곳에 모아두면 좋을 것 같은데 안드 회의 때 논의해보면 좋을 것 같아요!!
석준오빠가 저번 코리로 달아준 부분(when문을 통해 UiState.Success일 때만 뷰를 보여주는 방법)은 해보니까 홈 뷰에서 '딱 맞는 맞춤 공고 부분'에서만 적용이 가능할 것 같아요...! 다만 그 부분이 현재는 다이얼로그랑 너무 복잡하게 엮여있어서 다이얼로그 리팩토링 후 거기에 맞춰서 수정해야 할 것 같습니다..!! 일단 homeRoute가 리컴포지션이 발생해서 로직을 homeScreen 안쪽으로 옮겼는데 layout inspector가 갑자기 말을 안들어서 이후에 확인은 못했어요,,,,
분명히 까먹은 부분이 많을 것 같은데 혹시 발견하면 말씀해주세요🥹