[Feature] 온보딩 모듈 추가 #438

ThirFir · 2024-10-28T04:06:31Z

🐥 개요

closed [Campus] 식단 페이지 UI 변경 #429
식단 UI 일부 변경
툴팁 모듈 분리

👾 상세 작업 내용

식단 좋아요 삭제
기능 소개를 위해 최초 1회에만 띄우던 툴팁 기능 -> 모듈 분리해서 하나의 클래스로 구현 (원래 있던 것을 모듈 분리 및 리팩토링 한 것임)
알림 액티비티에 공지 키워드 알림 페이지 진입점 추가

👀 부가 설명

(클린 아키텍쳐 제외) 처음 해보는 모듈 분리라 미흡한 부분 바로 지적해주시면 감사드리겠습니다 🙏

🐰 결과

Jokwanhee

고생하셨습니다! 🙇🏻

코드 작성할 때, 함수화 좀 신경써주면 좋을 것 같아요! 리뷰할 때 코드 읽기가 너무 힘들어서 뭐하는 코드인지 다시 한 번 확인하게 되네요 ㅠㅠ 🥲

추가로 궁금한거 몇 개 작성해놨는데, 답변 부탁드립니다~ 😎

[추가 질문]

onboarding 모듈이 정확히 하는게 뭔가요?

Jokwanhee · 2024-10-28T13:44:43Z

core/onboarding/src/main/java/in/koreatech/koin/core/onboarding/OnboardingManager.kt

+            repeatOnLifecycle(Lifecycle.State.STARTED) {
+                val shouldShow = onboardingRepository.getShouldOnboarding(type.name)
+                delay(500)
+                withContext(mainDispatcher) {


이미 lifecycleScope 와 repeatOnLifecycle를 사용하는데, Main 디스패처를 주입해서 사용한 이유가 있나요?

launch에 IO Dispatcher를 지정하려 했는데 안했네요 ㅎ.. 🤐

Jokwanhee · 2024-10-28T13:45:41Z

core/onboarding/src/main/java/in/koreatech/koin/core/onboarding/OnboardingManager.kt

+        lifecycleScope.launch {
+            repeatOnLifecycle(Lifecycle.State.STARTED) {
+                val shouldShow = onboardingRepository.getShouldOnboarding(type.name)
+                withContext(mainDispatcher) {


이것도 마찬가지로 위와 같은 질문!

Jokwanhee · 2024-10-28T13:52:57Z

core/onboarding/src/main/java/in/koreatech/koin/core/onboarding/OnboardingModule.kt

+        onboardingRepository: OnboardingRepository,
+        @ApplicationContext context: Context
+    ): OnboardingManager {
+        return OnboardingManager(onboardingRepository, context, Dispatchers.Main)


Dispatchers.Main 이렇게 주입해서 사용할거면 클래스 생성자 dafult 파라미터는 어떠신가요~?
추가적으로 immediate를 사용하지 않고 그냥 Main 디스패처를 사용한 이유가 있나요?

말씀대로 생성자로 옮겨도 될 거 같네요. 반영하겠습니다~
그리고 위 답(launch에 IO Dispatcher) 에 이어서 부모 코루틴이 IO Dispatcher CoroutineContext를 가지기 때문에, 어차피 Context Switching이 일어나야 하므로 Main 디스패처로 두었고, Main.Immediate는 구현하면서 고려는 하지 않았었습니다.

Jokwanhee · 2024-10-28T14:16:02Z

data/src/main/java/in/koreatech/koin/data/repository/DiningRepositoryImpl.kt

@@ -13,16 +13,4 @@ class DiningRepositoryImpl @Inject constructor(
    override suspend fun getDining(date: String): List<Dining> {
        return diningRemoteDataSource.getDining(date).map(DiningResponse::toDining)


메서드 참조로 함수 호출한 이유가 있나요?

예전에 한거라 잘 기억은 안나네요...
특별한 이유는 없었을 겁니다...!

Jokwanhee · 2024-10-28T14:18:48Z

domain/src/main/java/in/koreatech/koin/domain/usecase/dining/GetDiningUseCase.kt

-            return kotlin.runCatching {
-                diningRepository.getAuthDining(date)
-            }
+        return kotlin.runCatching {


Suggested change

return kotlin.runCatching {

return runCatching {

kotlin 지워도 좋을 것 같네요~

Jokwanhee · 2024-10-28T14:26:21Z

koin/src/main/java/in/koreatech/koin/ui/article/ArticleListFragment.kt

@@ -268,7 +275,8 @@ class ArticleListFragment : Fragment() {
    }

    private fun createChip(text: String, isCheckable: Boolean, onChipClicked: () -> Unit): Chip {
-        val chip = layoutInflater.inflate(R.layout.chip_layout, binding.chipGroupMyKeywords, false) as Chip
+        val chip =
+            layoutInflater.inflate(R.layout.chip_layout, binding.chipGroupMyKeywords, false) as Chip


다른 곳들도 as를 사용하셨네요! 안전성의 문제이고 논리적으로 원하는 형변환을 할 수 있지만 그래도 모든 상황을 예측할 수 없으니 as?로 통일해보는 것은 어떨까? 하는 의견을 내봅니다~ ☺️

반영하겠습니다 ~

Jokwanhee · 2024-10-28T14:31:13Z

koin/src/main/java/in/koreatech/koin/ui/article/viewmodel/ArticleListViewModel.kt

-        .stateIn(
+    val articlePagination: StateFlow<ArticlePaginationState> =
+        combine(currentBoard, currentPage, selectedKeyword) { board, page, query ->
+            _isLoading.value = true


flow onStart 중간 연산자 사용하는 건 어떠신가요?

onStart는 flow가 처음 구독될 때만 실행되므로, currentBoard, currentPage, selectedKeyword가 바뀔 때마다 실행되지는 않기에 로딩 상태를 제대로 반영하지 못했습니다 !

Jokwanhee · 2024-10-28T14:32:17Z

koin/src/main/java/in/koreatech/koin/ui/article/viewmodel/ArticleListViewModel.kt

+                articlePagination.toArticlePaginationState()
+            }
+        }.onEach {
+            _isLoading.value = false


이것도 onCompletion 사용하면 가독성이 더 좋아질 것 같아요! 개인적으로 onStart, onCompletion 사용하지 않으시면 편하실 때루 해도 괜춘해여~

Hot Stream이기 때문에 Flow가 끝나지 않아, onCompletion은 호출되지 않는 것으로 알고 있습니다 ~!

Jokwanhee · 2024-10-28T14:39:55Z

koin/src/main/java/in/koreatech/koin/ui/dining/DiningItemsFragment.kt

+            viewLifecycleOwner.showOnboardingIfNeeded(
+                OnboardingType.DINING_SHARE,
+            ) {
+                lifecycleScope.launch(Dispatchers.Default) {


백그라운드 스레드 사용한 이유가 있나요?
코루틴 스코프 때문이라면 showOnboardingIfNeeded 고차함수의 action 파라미터를 suspend로 사용하는건 어떠세요?

백그라운드 쓴 건 delay주려고 했던 건데, 다시 보니 백그라운드로 안해도 될 것 같네요 !

action을 suspend로 두지 않은 건, action 동작에서 UI 작업이 주를 이룰 것으로 예상하고 구현했기 때문입니다.

Jokwanhee · 2024-10-28T14:43:27Z

koin/src/main/java/in/koreatech/koin/ui/dining/DiningItemsFragment.kt

+            ) {
+                lifecycleScope.launch(Dispatchers.Default) {
+                    delay(200)
+                    withContext(Dispatchers.Main) {


만약에 백그라운드 스레드가 제거되면 해당 디스패처 context 바꾸는 코드도 없어져도 좋을 것 같네요!

ThirFir · 2024-10-29T06:26:34Z

코드 작성할 때, 함수화 좀 신경써주면 좋을 것 같아요! 리뷰할 때 코드 읽기가 너무 힘들어서 뭐하는 코드인지 다시 한 번 확인하게 되네요 ㅠㅠ 🥲

이번 pr에서 어떤 부분에서 그런 점이 느껴졌는지 알려주시면 감사드리겠습니다 !
사실 이 PR이 코드 작성 자체가 많진 않은데, IDE에 의해 자동으로 코드 formatting이 일어나서 변경이 많아 보인 것 같고, 아마 그것 때문에 많이 헷갈리시지 않았을까 예상해봅니다...
그래서 예전에 작성했던 부분에 대해서 리뷰를 달아주신 부분도 많이 있는 것 같아요 ㅠ

IDE 코드 포맷팅을 꺼놔야겠습니다...

onboarding 모듈이 정확히 하는게 뭔가요?

사용자에게 최초 1회에만 실행시킬 동작을 제시할 때 사용하는 것으로 알아주시면 될 것 같습니다.
캠퍼스팀 작업하면서, 신기능 구현하면서 그에 대한 소개를 툴팁으로 제공하고 있는데, 툴팁 쓰이는 부분이 점점 많이지기에 모듈화해봤습니다 ~

Jokwanhee · 2024-10-29T07:14:05Z

그렇군용 IDE 포맷팅이 많이 들어가서 더 그랬던 것 같기도 하네요~

오늘 안드 회의하는데, 오프회의로 PR 보면서 리뷰에 대해서 복기해보는건 어떨까요? (어프도 바로 해드리고 ☺️)

ThirFir · 2024-10-29T07:19:49Z

오늘 안드 회의하는데, 오프회의로 PR 보면서 리뷰에 대해서 복기해보는건 어떨까요? (어프도 바로 해드리고 ☺️)

좋습니다 !!! 👊

Jokwanhee

고생했습니다!

hsgo2430

모듈화 힘드셨을텐데 고생하셨어요!

ThirFir added 26 commits October 28, 2024 00:27

del: 좋아요 기능 삭제

0bac2b8

fix: 키워드 알림 상태 버그

bfa6f6e

del: onResume override

0e5ec8e

fix: 식단 카드 간격 조정

26e7a33

fix: 식단 카드 Radius

a372212

fix: 식단 상세 정보 ui

7279ad8

fix: 식단 공유 버튼 ui

03c0c19

add: onboarding 모듈

107a6dd

refactor: OnboardingManager 추가, MainActivity 식단 툴팁 변경

d1ae376

add: 화살표 위치 매개변수

26deffb

refactor: 공지 키워드 툴팁 코드 리팩토링

49d14dc

del: Koin 모듈에서 balloon lib 의존성 제거

1647ad9

fix: tooltip initialize 여부 조건 추가

39e520c

del: Unused onboarding codes

c038ed3

refactor: Onboarding Hilt Component: Singleton -> ActivityComponent

d0b8ece

refactor: 온보딩 바텀시트 액션 모듈화 이용

535d4c6

refactor: DataStore DI 로직 변경

d9d15a4

fix: 함수명 포괄적으로 변경

d864ea5

add: 툴팁 닫기 API

85f533d

add: 식단 공유 툴팁

b3511fb

add: 이벤트 로깅

64b5bf7

add: 알림 액티비티에 공지 키워드 진입점 추가

dded9d7

refactor: enum 위치 이동

2792822

add: 원본 공지 진입버튼

37f9ad5

feat: 식단 공유 툴팁 gif로 변경

7304ca3

fix: 빌드 에러

9f2792a

ThirFir self-assigned this Oct 28, 2024

ThirFir requested a review from a team as a code owner October 28, 2024 04:06

ThirFir force-pushed the feature/onboarding branch from b1fba07 to 9f2792a Compare October 28, 2024 04:07

Jokwanhee reviewed Oct 28, 2024

View reviewed changes

fix: as -> as?, Dispatchers

1fb52b3

Jokwanhee approved these changes Oct 29, 2024

View reviewed changes

hsgo2430 approved these changes Oct 29, 2024

View reviewed changes

ThirFir added 3 commits October 30, 2024 23:58

fix: Dispatchers Switching 제거

9065122

Merge branch 'feature/onboarding' into feature/dining-abtest

8211f45

Merge remote-tracking branch 'origin/develop' into feature/onboarding

f621430

ThirFir merged commit 7861aab into develop Oct 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] 온보딩 모듈 추가 #438

[Feature] 온보딩 모듈 추가 #438

ThirFir commented Oct 28, 2024

Jokwanhee left a comment •

edited

Loading

Jokwanhee Oct 28, 2024

ThirFir Oct 29, 2024 •

edited

Loading

Jokwanhee Oct 28, 2024

Jokwanhee Oct 28, 2024

ThirFir Oct 29, 2024 •

edited

Loading

Jokwanhee Oct 28, 2024

ThirFir Oct 29, 2024

Jokwanhee Oct 28, 2024

Jokwanhee Oct 28, 2024

ThirFir Oct 29, 2024

Jokwanhee Oct 28, 2024

ThirFir Oct 29, 2024

Jokwanhee Oct 28, 2024

ThirFir Oct 29, 2024

Jokwanhee Oct 28, 2024

ThirFir Oct 29, 2024

Jokwanhee Oct 28, 2024

ThirFir commented Oct 29, 2024 •

edited

Loading

Jokwanhee commented Oct 29, 2024

ThirFir commented Oct 29, 2024

Jokwanhee left a comment

hsgo2430 left a comment

		@@ -13,16 +13,4 @@ class DiningRepositoryImpl @Inject constructor(
		override suspend fun getDining(date: String): List<Dining> {
		return diningRemoteDataSource.getDining(date).map(DiningResponse::toDining)

[Feature] 온보딩 모듈 추가 #438

[Feature] 온보딩 모듈 추가 #438

Conversation

ThirFir commented Oct 28, 2024

🐥 개요

👾 상세 작업 내용

👀 부가 설명

🐰 결과

Jokwanhee left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThirFir Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThirFir Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThirFir commented Oct 29, 2024 • edited Loading

Jokwanhee commented Oct 29, 2024

ThirFir commented Oct 29, 2024

Jokwanhee left a comment

Choose a reason for hiding this comment

hsgo2430 left a comment

Choose a reason for hiding this comment

Jokwanhee left a comment •

edited

Loading

ThirFir Oct 29, 2024 •

edited

Loading

ThirFir Oct 29, 2024 •

edited

Loading

ThirFir commented Oct 29, 2024 •

edited

Loading