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

[REFACTOR/#193] 홈 뷰 리팩토링 #203

Merged
merged 23 commits into from
Aug 30, 2024
Merged

Conversation

Hyobeen-Park
Copy link
Member

⛳️ Work Description

  • homeState 추가
  • 홈 뷰 로직 수정

📸 Screenshot

UI는 바뀐거 없습니다!!

📢 To Reviewers

  • 다이얼로그나 디자인 수정된 부분들은 제외하고 진행했습니다.

  • 여러 곳에서 동시에 사용되는 enum class들을 한 곳에 모아두면 좋을 것 같은데 안드 회의 때 논의해보면 좋을 것 같아요!!

  • 석준오빠가 저번 코리로 달아준 부분(when문을 통해 UiState.Success일 때만 뷰를 보여주는 방법)은 해보니까 홈 뷰에서 '딱 맞는 맞춤 공고 부분'에서만 적용이 가능할 것 같아요...! 다만 그 부분이 현재는 다이얼로그랑 너무 복잡하게 엮여있어서 다이얼로그 리팩토링 후 거기에 맞춰서 수정해야 할 것 같습니다..!! 일단 homeRoute가 리컴포지션이 발생해서 로직을 homeScreen 안쪽으로 옮겼는데 layout inspector가 갑자기 말을 안들어서 이후에 확인은 못했어요,,,,

  • 분명히 까먹은 부분이 많을 것 같은데 혹시 발견하면 말씀해주세요🥹

# 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
Copy link
Member

@leeeyubin leeeyubin 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 +233 to +235
viewModel.updateScrapDialogVisible(true)
viewModel.updateIsToday(false)
scrapId = index
Copy link
Member

Choose a reason for hiding this comment

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

뷰모델도 리컴포지션의 대상이 될 수 있어서 이 부분들도 Route로 빼주면 좋을 것 같긴 하네요..!

Copy link
Member Author

Choose a reason for hiding this comment

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

오호!! 뷰모델도 리컴포지션이 될 수 있군요..!!!! 알려주셔서 감사합니다🥰🥰

Comment on lines 202 to 207
HorizontalDivider(
thickness = 4.dp,
color = Grey150,
modifier = Modifier
.background(White)
) {
ShowRecommendTitle()
ShowInternFilter(homeFilteringInfo = homeFilteringInfo, onChangeFilterClick)
.fillMaxWidth(),
)
Copy link
Member

Choose a reason for hiding this comment

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

HorizontalDivider를 들어가보면 modifier에 fillMaxWidth가 적용되어 있는 걸 볼 수 있을 거에요!
그래서 따로 지정해주지 않아도 될 것 같습니당

Copy link
Member Author

Choose a reason for hiding this comment

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

오 정말 그렇네요!! 새로운 사실 알아갑니다ㅎㅎ

Comment on lines +36 to +37
val homeState get() = _homeState.asStateFlow()

Copy link
Member

Choose a reason for hiding this comment

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

오호 여러 개의 state를 하나로 묶은 건가요? 관리하기 더 용이해진 것 같네요!

Copy link
Contributor

Choose a reason for hiding this comment

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

ViewModel에서 한번에 관리하니까 Route 단 코드가 넘나 간결해졌네용 ㅜ 대박

Copy link
Member Author

Choose a reason for hiding this comment

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

😊

Copy link
Contributor

@arinming arinming 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 +36 to +37
val homeState get() = _homeState.asStateFlow()

Copy link
Contributor

Choose a reason for hiding this comment

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

ViewModel에서 한번에 관리하니까 Route 단 코드가 넘나 간결해졌네용 ㅜ 대박

Copy link
Member

@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.

코드가 많이 간결해지고 깔끔해졌네요!!👍
너무 수고하셨슴다~

@@ -96,7 +96,7 @@ fun ChangeFilterScreen(
topBar = {
BackButtonTopAppBar(
title = stringResource(id = R.string.change_filter_top_bar_title),
onBackButtonClick = { navController.popBackStack() },
onBackButtonClick = { navigateToHome() },
Copy link
Member

Choose a reason for hiding this comment

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

이 부분은 중괄호가 필요없지 않나요??

Suggested change
onBackButtonClick = { navigateToHome() },
onBackButtonClick = navigateToHome,

Copy link
Member Author

Choose a reason for hiding this comment

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

웁씨... 맞아요ㅎㅎ 알려주셔서 감사합니당!!!

@Hyobeen-Park Hyobeen-Park merged commit 65777d3 into develop Aug 30, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REFACTOR ♻️ 전면 수정 효빈💚 효빈
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REFACTOR] 홈, 필터링 재설정 리팩토링
4 participants