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

#7 [feat] 우리 집 규칙 기능 구현 #8

Merged
merged 20 commits into from
Sep 5, 2022

Conversation

murjune
Copy link
Member

@murjune murjune commented Sep 2, 2022

관련 이슈

작업 뷰

empty view 1개 2개
image image image
3개 3개 이상
image image

작업한 내용

  • 우리집 규칙 뷰 navigation graph 기초 세팅
  • 멀티뷰타입 enum class 추가
  • domain-entity에 RuleInfo 추가
  • 우리집 규칙 리사이클러뷰 구현
  • itemDecoration Util 패키지에 추가
  • safeLet Util 패키지 추가
  • repeatOnStarted Util 패키지 추가
  • Theme NoActionBar로 수정

PR 포인트

safeLet 확장함수(사용 방법은 아래 링크 참조해주세요 ~)

TeamHous/Hous-Android#179 (comment)

repeatOnStarted Lifecycle확장함수

  • collect하기 편하게 해주기 위한 확장함수

build gradle 추가

  • navigation 컴포넌트 사용하구 싶어서 추가했어요~
  • hilt버전이랑 kotlin버전이랑 호환이 안되면 에러가 뜨더라구요.. 😢 (엄청 고생했습니다..)
    Unsupported metadata version when using Hilt 2.42 with Kotlin 1.7.10 google/dagger#3470
  • 액티비티뷰모델을 사용해도 되지만 hiltNavgraphViewModel이란 걸 써보고 싶어서 사용했습니다 😄
  • 규칙의 개수에 따라 emptyView의 visiblity를 조절했습니다~

@murjune murjune added 준원🐻 준원이가 작업함! feat 새로운 기능 추가 Pull Request🔥 풀리퀘 날림! labels Sep 2, 2022
@murjune murjune added this to the Hous 1차 릴리즈 milestone Sep 2, 2022
@murjune murjune requested a review from KWY0218 September 2, 2022 12:43
@murjune murjune self-assigned this Sep 2, 2022
Copy link
Member

@KWY0218 KWY0218 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 +18 to +20

private val viewModel: OurRulesViewModel by hiltNavGraphViewModels(R.id.nav_our_rules)
private var ourRulesAdapter: OurRulesAdapter? = null
Copy link
Member

Choose a reason for hiding this comment

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

hiltNavGraphViewModels 의도가 궁금합니다

Copy link
Member Author

@murjune murjune Sep 2, 2022

Choose a reason for hiding this comment

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

저희는 hilt를 사용하기 때문이지요 :D

 private val viewModel: OurRulesViewModel by navGraphViewModels(R.id.nav_our_rules)

위의 방식으로 생성된 모든 ViewModel 객체는 연결된 NavHost와 ViewModelStore가 삭제되거나 nav_graph가 백 스택에서 소멸되기 전까지 유지되기 때문에 by navGraphViewModels(layoutRes)을 사용하는 것입니다.

그런데 우리는 hilt를 사용하기 때문에 by hiltNavGraphViewModels(R.id.nav_our_rules)을 사용해줬습니당

If your ViewModel is scoped to the navigation graph, use the hiltNavGraphViewModels function that works with fragments that are annotated with @androidentrypoint.

관련 공식 문서: https://developer.android.com/training/dependency-injection/hilt-jetpack#viewmodel-navigation

Comment on lines +8 to 10
@AndroidEntryPoint
class OurRulesActivity : AppCompatActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
Copy link
Member

Choose a reason for hiding this comment

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

one Activity 를 의도한 것인지 아닌지 궁금합니다.

one Activity 를 의도했음 이 OurRulesActivity 가 없어도 된다고 생각하고,
one Activity 를 의도하지 않았으면 OurRulesFragment 의 로직이 OurRulesActivity 로 옮겨져야 한다고 생각하는데

어떤 의도인지 궁금합니다

Copy link
Member Author

Choose a reason for hiding this comment

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

맞습니다 제가 맡은 우리 집 규칙뷰에서는 oneActivity를 사용할 생각입니다.
image
위에서 Our Rules를 클릭하면 우리 집 규칙으로 넘어오게 됩니다.
image
그래서, by activityViewModels()로 ViewModel을 생성해줘도 되지만, 저는 hiltNavGraphViewModels를 적용해보고 싶었습니다 하하~

Comment on lines 13 to 21
private var _uiState = MutableStateFlow(OurRulesUiState())
val uiState: StateFlow<OurRulesUiState>
get() = _uiState.asStateFlow()
private var _isEmptyRepresentativeRuleList = MutableStateFlow(true)
val isEmptyRepresentativeRuleList: StateFlow<Boolean>
get() = _isEmptyRepresentativeRuleList.asStateFlow()
private var _isEmptyGeneralRuleList = MutableStateFlow(true)
val isEmptyGeneralRuleList: StateFlow<Boolean>
get() = _isEmptyGeneralRuleList.asStateFlow()
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
private var _uiState = MutableStateFlow(OurRulesUiState())
val uiState: StateFlow<OurRulesUiState>
get() = _uiState.asStateFlow()
private var _isEmptyRepresentativeRuleList = MutableStateFlow(true)
val isEmptyRepresentativeRuleList: StateFlow<Boolean>
get() = _isEmptyRepresentativeRuleList.asStateFlow()
private var _isEmptyGeneralRuleList = MutableStateFlow(true)
val isEmptyGeneralRuleList: StateFlow<Boolean>
get() = _isEmptyGeneralRuleList.asStateFlow()
private var _uiState = MutableStateFlow(OurRulesUiState())
val uiState: StateFlow<OurRulesUiState> = _uiState.asStateFlow()
private var _isEmptyRepresentativeRuleList = MutableStateFlow(true)
val isEmptyRepresentativeRuleList: StateFlow<Boolean> = _isEmptyRepresentativeRuleList.asStateFlow()
private var _isEmptyGeneralRuleList = MutableStateFlow(true)
val isEmptyGeneralRuleList: StateFlow<Boolean> = _isEmptyGeneralRuleList.asStateFlow()

이렇게 해도 동작을 할텐데 get()을 붙인 이유가 있을까요??

Copy link
Member Author

@murjune murjune Sep 2, 2022

Choose a reason for hiding this comment

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

아 제가 맨날 LiveData만 사용하다보니.. get()이 습관이 들어서 히히.. 감사합니다 😄

Comment on lines 56 to 58
_uiState.value = _uiState.value.copy(
ourRuleList = tmpRuleList.toList()
)
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
_uiState.value = _uiState.value.copy(
ourRuleList = tmpRuleList.toList()
)
_uiState.value = _uiState.value.copy(
ourRuleList = tmpRuleList
)

toList() 는 없애도 될 것 같습니다

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 82 to 101
// dummyData
// listOf(
// RuleInfo("1111", "우리 집 대장은 최코코"),
// RuleInfo("2222", "10시, 18시 코코님 밥 챙겨드리기"),
// RuleInfo("3333", "자기가 먹은 건 자기가 치우기!"),
// RuleInfo("4444", "아침에 일어나면 이불 정리하기"),
// RuleInfo("5555", "밤 10시 지나면 이어폰 끼기"),
// RuleInfo("11412311", "우리 집 대장은 최코코"),
// RuleInfo("4444", "아침에 일어나면 이불 정리하기"),
// RuleInfo("512355", "밤 10시 지나면 이어폰 끼기"),
// RuleInfo("612366", "냄새나는 음식 먹고나서 환기하기"),
// RuleInfo("7721377", "맛있는 식당 찾으면 공유하기"),
// RuleInfo("112311", "우리 집 대장은 최코코"),
// RuleInfo("2221322", "10시, 18시 코코님 밥 챙겨드리기"),
// RuleInfo("3331233", "자기가 먹은 건 자기가 치우기!"),
// RuleInfo("4412344", "아침에 일어나면 이불 정리하기"),
// RuleInfo("5552135", "밤 10시 지나면 이어폰 끼기"),
// RuleInfo("6612366", "냄새나는 음식 먹고나서 환기하기"),
// RuleInfo("7771237", "맛있는 식당 찾으면 공유하기")
// )
Copy link
Member

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.

넵넵!!

Comment on lines +16 to +33
inline fun LifecycleOwner.repeatOnStarted(crossinline block: suspend () -> Unit) {
when (this) {
is AppCompatActivity -> {
lifecycleScope.launch {
lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
block()
}
}
}
is Fragment -> {
lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
block()
}
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

reapeatOnLifeCycle 함수 자체가 LifecycleonStart 상태일 때 수집을 시작하고, onResume 상태가 되면 알아서 수집을 멈춰주는 함수인 것으로 압니다.

이 확장함수를 만든 의도가 궁금합니다

Copy link
Member Author

Choose a reason for hiding this comment

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

class LocationActivity : AppCompatActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        // Create a new coroutine since repeatOnLifecycle is a suspend function
        lifecycleScope.launch {
            // The block passed to repeatOnLifecycle is executed when the lifecycle
            // is at least STARTED and is cancelled when the lifecycle is STOPPED.
            // It automatically restarts the block when the lifecycle is STARTED again.
            repeatOnLifecycle(Lifecycle.State.STARTED) {
                // Safely collect from locationFlow when the lifecycle is STARTED
                // and stops collection when the lifecycle is STOPPED
                locationProvider.locationFlow().collect {
                    // New location! Update the map
                }
            }
        }
    }
}

reapeatOnLifeCycle함수는 onResume 상태가 아닌 onStop상태일 때 collect가 cancel됩니다!!
사실, collect를 한 번만 하게 된다면 Flow.flowWithLifecycle를 사용해도 되긴합니다만..
추후에 기능이 추가된다면 collect를 또 할 수도 있을 것 같아서 저는 reapeatOnLifeCycle을 선호하는 편입니다!
관련 링크

또한, 이 뷰 뿐만 아니라 다른 뷰(Fragment/Activity)에서도 collect해줄 때마다 더 간결하게 사용하기 위해 확장함수를 만들었습니다!!

 lifecycleScope.launch {
                lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
                    // do something~
                }
            }

Comment on lines -3 to 4
<style name="Theme.HousReleaseAos" parent="Theme.MaterialComponents.DayNight.DarkActionBar">
<style name="Theme.HousReleaseAos" parent="Theme.MaterialComponents.DayNight.NoActionBar">
<!-- Primary brand color. -->
Copy link
Member

Choose a reason for hiding this comment

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

센스 👍

Comment on lines 22 to 32
android:exported="true" >
<intent-filter>
<action android:name="android.intent.action.MAIN" />

<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>
<activity
android:name=".MainActivity"
android:exported="true">
</activity>
Copy link
Member

Choose a reason for hiding this comment

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

PR 올릴 땐, MainActivity를 시작점으로 설정해주세요!

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

Choose a reason for hiding this comment

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

PR 올릴 땐, MainActivity를 시작점으로 설정해주세요!

LoginActivity도 false로 올려야 할까요?

Comment on lines +54 to +64
repeatOnStarted {
viewModel.uiState.collect { uiState ->
ourRulesAdapter?.let { ourRulesAdapter ->
// this callback runs when the list is updated
ourRulesAdapter.submitList(uiState.ourRuleList) {
binding.rvOurRules.invalidateItemDecorations()
}
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

uiState만을 수집할 땐, reapeatLifeCycle 보단 flowWithLifeCycle 함수를 사용해주세요!

Copy link
Member Author

Choose a reason for hiding this comment

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

#8 (comment)
혹시, 이유를 알 수 있을까요?! 하나만 수집할 때는 reapeatLifeCycle보다 flowWithLifeCycle가 더 성능이 좋나요?!

Comment on lines 3 to 6
data class RuleInfo(
val id: String = "",
val description: String = ""
)
Copy link
Member

Choose a reason for hiding this comment

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

어제 제가 슬랙에 올린 글을 보면

의미 있게 구분하라 (불용어 -noise word- 를 쓰지 말자)

부분을 한번 읽어주세요!

이거 보니까 뒤 Info 사용을 지양하고 싶단 생각이 들었습니다.

그냥 제 생각이긴 한데 data 모듈에선 ruleDto domain 모듈에선 rule 사용은 어떨까요?
다음 안드 회의가 있을 때 다같이 논의해 봅시다

Copy link
Member Author

Choose a reason for hiding this comment

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

아 맞다.. 이거 쓰면서도 찜찜했는데 굿입니다!!

@murjune murjune requested a review from KWY0218 September 2, 2022 15:42
@murjune
Copy link
Member Author

murjune commented Sep 5, 2022

@KWY0218 repeatOnLifecycle에 launchIn해도 잘 되긴하네요.

        repeatOnStarted {
            viewModel.uiState.onEach { uiState ->
                ourRulesAdapter?.let { ourRulesAdapter ->
                    // this callback runs when the list is updated
                    ourRulesAdapter.submitList(uiState.ourRuleList) {
                        binding.rvOurRules.invalidateItemDecorations()
                    }
                }
            }.launchIn(lifecycleScope)
        }
  • 애초에 flowWithLifecycle가 내부적으로 repeatOnLifecycle을 구현하고 있었네요 ㅎ ㅎ ㅎ
@OptIn(ExperimentalCoroutinesApi::class)
public fun <T> Flow<T>.flowWithLifecycle(
    lifecycle: Lifecycle,
    minActiveState: Lifecycle.State = Lifecycle.State.STARTED
): Flow<T> = callbackFlow {
    lifecycle.repeatOnLifecycle(minActiveState) {
        this@flowWithLifecycle.collect {
            send(it)
        }
    }
    close()
}

이 부분에 대해서 다음주 심화스터디 flow 발표할 때 자세히 파보고 정리해볼게요~~

@murjune murjune merged commit fbcdd40 into develop Sep 5, 2022
@yjooooo yjooooo deleted the #7-create-our-hous-rules branch October 23, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat 새로운 기능 추가 Pull Request🔥 풀리퀘 날림! 준원🐻 준원이가 작업함!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feat] 우리집 규칙 뷰 기능 구현
3 participants