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

#17 [feat] 온보딩 루틴 선택 뷰 #26

Merged
merged 13 commits into from
Jan 10, 2024

Conversation

emjayMJkim
Copy link
Member

📑 Work Description

  • 온보딩 루틴 선택 뷰 구현 완

🛠️ Issue

📷 Screenshot

feat.onboarding-routine.choice.webm

💬 To Reviewers

일단 피그마에는 루틴이 아무것도 선택되지 않았을 때도 버튼이 활성화 상태여서 버튼 비활성화는 따로 구현하지 않았습니다. "선택된 루틴이 0개일 경우 버튼 활성화/비활성화"는 기획 쌤들께 물어본 후 로직 추가하겠습니다

@emjayMJkim emjayMJkim changed the title #17 [feat] 온보딩 - 루틴 선택 뷰 #17 [feat] 온보딩 루틴 선택 뷰 Jan 10, 2024
@emjayMJkim emjayMJkim self-assigned this Jan 10, 2024
@emjayMJkim emjayMJkim added Pull Request pr 날림! Feat 새로운 기능 추가 민정🦊 민정이가 작업함! labels Jan 10, 2024
Copy link
Collaborator

@pump9918 pump9918 left a comment

Choose a reason for hiding this comment

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

앞서나가는 코드 잘 보고 갑니다..

class RoutineChoiceViewModel : ViewModel() {

private val _mockRoutineList: MutableLiveData<List<Routine>> = MutableLiveData(
mutableListOf(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mutable로 선언하는 것이 그냥 listOf로 선언하는 것보다 어떤 이점이 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

이점이 있다기 보다는 mutablelist는 가변적이고 list는 불변합니다 지금은 둘다 상관없다고 생각하지만 후에 서버통신 생각해서 mutablelist로 선언했습니당

Copy link
Member

@minemi00 minemi00 left a comment

Choose a reason for hiding this comment

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

작업속도무선일 ㅠ


class RoutineChoiceViewModel : ViewModel() {

private val _mockRoutineList: MutableLiveData<List<Routine>> = MutableLiveData(
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.

여기도 마찬가지로 서버통신할 때처럼 변경되는 데이터를 가져오고 싶어서 설정했습니다! 참고로 livedata는 fragment의 수명 주기를 따르기 때문에 따로 메모리 해제를 하지 않아도 됩니당

private fun routineSelection(binding: ItemOnboardingChoiceRoutineBinding, routine: Routine) {
val isRoutineSelected: Boolean = selectedRoutineArray.contains(routine.routineId)

if (selectedRoutineArray.size == MAXIMUM_ROUTINE_SELECTION) {
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.

화이팅!! 잘 하고 있습니당

),
Routine(
8,
"마라탕 먹고 싶다"
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
Contributor

@stellar-halo stellar-halo left a comment

Choose a reason for hiding this comment

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

깔끔한 코드 잘 봤어요!! 멋있다 우리 막내

binding: ItemOnboardingChoiceRoutineBinding
) {
routineArray.removeAt(selectedIndex)
changeRoutineBackground(binding, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

깔쌈하고 좋네요.,, 역시 갓기

Comment on lines 93 to 99
binding.tvRoutineContent.setBackgroundResource(R.drawable.shape_gray100_fill_gray400_stroke_99_rect)
binding.tvRoutineContent.setTextColor(
ContextCompat.getColor(
binding.root.context,
R.color.gray700
)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 drawble이랑 color 변수로 넘겨서 함수 하나 만드는 건 어떤가요? 제가 함수에 미친 놈 같나요?

fun set~(shape: Int, color: Int)
binding.tvRoutineContent.setBackgroundResource(shape)
                binding.tvRoutineContent.setTextColor(
                    ContextCompat.getColor(
                        binding.root.context,
                        color
                    )
                )

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 +55 to +57
routineViewModel.mockRoutineList.observe(viewLifecycleOwner) {
choiceRoutineAdapter.submitList(it)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

하나의 함수는 하나의 행동만 해야 합니다! makeRoutineAdapter라기보다는 updateRoutineList라던가 등의 함수로 따로 빼는 것이 좋아보이네요

Copy link
Member Author

Choose a reason for hiding this comment

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

감사합니다!! 수정하겠습니당

),
Routine(
10,
"집 언제 가지"
Copy link
Contributor

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋ

@@ -79,11 +79,11 @@ class ChoiceThemeAdapter :
) {
when (selected) {
true -> {
binding.ivThemeBackground.setBackgroundResource(R.drawable.shape_gray100_filll_100_circle)
binding.ivThemeBackground.setBackgroundResource(R.drawable.shape_gray100_fill_gray400_stroke_100_circle)
Copy link
Contributor

Choose a reason for hiding this comment

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

파일명 바꾼 것 아주 좋아요^^

@emjayMJkim emjayMJkim merged commit 18884a8 into develop Jan 10, 2024
1 check passed
@emjayMJkim emjayMJkim deleted the feature/#17-onboarding-routine branch January 10, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feat 새로운 기능 추가 Pull Request pr 날림! 민정🦊 민정이가 작업함!
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[ui] 온보딩 - 루틴 선택
4 participants