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/#246] 달력 로직 대폭 수정 #248

Merged
merged 17 commits into from
Sep 16, 2024

Conversation

boiledEgg-s
Copy link
Member

⛳️ Work Description

  • 달력 페이지 관리를 LazyListState에서 PagerState로 전환
  • 주간 화면에서 월 전환 지원
  • 주간 -> 목록 전환 시 목록 -> 주간으로 전환
  • 세 화면 간 페이지 동기화
  • 기타 코드 정리

📸 Screenshot

KakaoTalk_Video_2024-09-14-19-34-13.mp4

📢 To Reviewers

  • 기획 측에서 요청한 달력 전환 로직을 수정했습니다!
    기존엔 "월간 -> 주간 -> 목록 -> 월간"으로 전환이 됐다면, 이젠 "월간 -> 주간 -> 목록 -> 주간"으로 전환이 이뤄집니다!
  • 좌측으로 스크롤할 때와 우측으로 스크롤할 때 API 호출 시점의 차이가 거슬려서 기존 liststate에서 pagerstate로 변경했습니다!
  • 월간 달력 화면에서 스크롤할 때 약간의 지연이 발생하는데, 이게 pagerState의 currentPage 값이 바뀌면서 여러 연산이 발생해 메인스레드에 부하가 생겨서 그러는 것 같아요,, 이건 나중에 해결해보도록 할게요~!!

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 +31 to 38
fun updateCurrentDate(date: LocalDate) =
viewModelScope.launch {
_uiState.update { currentState ->
currentState.copy(
currentDate = date
)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

오 여기에 viewModelScope.launch가 추가된 이유가 궁금해요!

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 +88 to +89
CompositionLocalProvider(
LocalPagerState provides pagerState
Copy link
Contributor

Choose a reason for hiding this comment

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

우와,..이것뭐예요....헐

Copy link
Member Author

Choose a reason for hiding this comment

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

특정 컴포저블의 하위 컴포저블에서 변수를 전역으로 사용할 수 있게끔 해주는 CompositionLocal입니다!
PagerState로 이전하면서 매 컴포저블마다 매개변수로 넣는 것 보다 그냥 CompositionLocal로 구현해봤습니다~

Comment on lines +14 to +16
val LocalPagerState = compositionLocalOf<PagerState> {
error("No PagerState provided")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

요 부분 하드코딩하신 이유가 따로 있으신가용?!

Copy link
Member Author

Choose a reason for hiding this comment

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

UI적으로 필요한 부분이 아니라 그냥 하드코딩으로 넣어놨습니다!
딱히 재사용되는 것도 아니라 리소스화할 필요성을 못느꼈는데 저런 부분도 리소스화해야 할까요??

Copy link
Contributor

Choose a reason for hiding this comment

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

UI 관련이 아니군요 딱콩 ㅎ ㅅ ㅎ

Comment on lines +48 to +50
snapshotFlow { pagerState.currentPage }
.collect { currentPage->
viewModel.getScrapMonth(currentPage)
Copy link
Contributor

Choose a reason for hiding this comment

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

ㅋ ㅑ

Copy link
Member

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

캘린더 코드는 볼 때마다 감탄이 나오네요 진짜 짱이다.. 멋있다...

onClickListButton: () -> Unit,
modifier: Modifier = Modifier,
) {
val coroutineScope = rememberCoroutineScope()

val listState = rememberLazyListState(
initialFirstVisibleItemIndex = uiState.calendarModel.initialPage
val pagerState = rememberPagerState(
Copy link
Member

Choose a reason for hiding this comment

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

pagerState.... 신기방기...

Comment on lines +86 to +87
UiState.Loading -> emptyMap()
UiState.Empty -> emptyMap()
Copy link
Member

Choose a reason for hiding this comment

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

Loading이랑 Empty는 앞에 is를 안붙여도 되는군요....!😮 처음 알았어요ㅎㅎㅎ

Copy link
Member Author

Choose a reason for hiding this comment

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

그러게요..? 왜 쟤넨 없지?

@boiledEgg-s boiledEgg-s merged commit ffe8ea4 into develop Sep 16, 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