-
Notifications
You must be signed in to change notification settings - Fork 0
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
#8 [ui] Daily Routine - 추가하기 #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
양옆 뷰페이저 저도 찾아보겠습니다... 고생했어요!
state: RecyclerView.State | ||
) { | ||
super.getItemOffsets(outRect, view, parent, state) | ||
outRect.left = divHeight |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
position == 0일 경우와 아닌 경우 나눠서 divHeight을 두면 첫 margin만 구분을 줄 수 있습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하 ! 꼼꼼한 리뷰 감사합니다
@@ -0,0 +1,4 @@ | |||
<?xml version="1.0" encoding="utf-8"?> | |||
<resources> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xml이랑 kotlin사이 황금비 3.5 겨우 찾았는데
dimen쓰면 해결되는거였네...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오빠 황금비율 대신 내가 가져옴ㅎ.ㅎ
app/build.gradle.kts
Outdated
@@ -124,6 +124,9 @@ dependencies { | |||
testImplementation("junit:junit:4.13.2") | |||
androidTestImplementation("androidx.test.ext:junit:1.1.5") | |||
androidTestImplementation("androidx.test.espresso:espresso-core:3.5.1") | |||
|
|||
//indicator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인디케이터 잘 쓰겠습니다^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^ 오빠가 기쁘니 좋다
dailyRoutineAddViewModel.mockThemeList.observe(this) { | ||
dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyFirstCardList.value) | ||
dailyRoutineAddThemeAdapter.submitList(dailyRoutineAddViewModel.mockThemeList.value) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수로 따로 빼주세요!~ setupAdapter와 맞지 않는 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하 넵!
when (item.themeId) { | ||
1 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyFirstCardList.value) | ||
2 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailySecondCardList.value) | ||
3 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyThirdCardList.value) | ||
4 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyFourthCardList.value) | ||
5 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyFifthCardList.value) | ||
6 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailySixthCardList.value) | ||
7 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailySeventhCardList.value) | ||
8 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyEighthCardList.value) | ||
9 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyNinthCardList.value) | ||
10 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyTenthCardList.value) | ||
11 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyEleventhCardList.value) | ||
12 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyTwelfthCardList.value) | ||
|
||
else -> dailyRoutineAddCardPagerAdapter.submitList( | ||
dailyRoutineAddViewModel.mockDailyFirstCardList.value | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val themeDailyRoutineList = listOf(startNewDayroutineList, healthyBodyRoutineList, ...)
dailyRoutineAddCardPagerAdapter.submitList(themeDailyRoutineList[item.themeId].value)
로 바꾸면 when을 안써도 되지 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
강희언니 리뷰처럼 하나의 리스트로 처리해도 될 것 같습니다! 어처피 저희 routineId가 각각 themeId와 상관없이 모두 다르니까요...!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 좋습니당!! 바꿔보겠슴당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
뷰페이저 하느라 정말 수고했다...!!
when (item.themeId) { | ||
1 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyFirstCardList.value) | ||
2 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailySecondCardList.value) | ||
3 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyThirdCardList.value) | ||
4 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyFourthCardList.value) | ||
5 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyFifthCardList.value) | ||
6 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailySixthCardList.value) | ||
7 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailySeventhCardList.value) | ||
8 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyEighthCardList.value) | ||
9 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyNinthCardList.value) | ||
10 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyTenthCardList.value) | ||
11 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyEleventhCardList.value) | ||
12 -> dailyRoutineAddCardPagerAdapter.submitList(dailyRoutineAddViewModel.mockDailyTwelfthCardList.value) | ||
|
||
else -> dailyRoutineAddCardPagerAdapter.submitList( | ||
dailyRoutineAddViewModel.mockDailyFirstCardList.value | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
강희언니 리뷰처럼 하나의 리스트로 처리해도 될 것 같습니다! 어처피 저희 routineId가 각각 themeId와 상관없이 모두 다르니까요...!!
true -> { | ||
binding.ivDailyRoutineAddThemeBackground.setBackgroundResource(R.drawable.ic_daily_theme_background_click) | ||
binding.tvDailyRoutineAddThemeName.setTextColor( | ||
ContextCompat.getColor( | ||
binding.root.context, | ||
R.color.gray700 | ||
) | ||
) | ||
} | ||
|
||
false -> { | ||
binding.ivDailyRoutineAddThemeBackground.setBackgroundResource(R.drawable.ic_daily_theme_background) | ||
binding.tvDailyRoutineAddThemeName.setTextColor( | ||
ContextCompat.getColor( | ||
binding.root.context, | ||
R.color.gray400 | ||
) | ||
) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 imageView background와 textView color 바꾸는 부분도 따로 함수로 빼주면 더 좋을 것 같습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하 넵!! 잘게잘게 쪼개볼게요
<!-- <ImageView--> | ||
<!-- android:id="@+id/iv_daily_routine_add_theme_icon"--> | ||
<!-- setImage="@{data.iconImageUrl}"--> | ||
<!-- android:layout_width="30dp"--> | ||
<!-- android:layout_height="30dp"--> | ||
<!-- android:layout_gravity="center"--> | ||
<!-- android:src="@color/red"--> | ||
<!-- app:layout_constraintBottom_toBottomOf="@id/iv_daily_routine_add_theme_background"--> | ||
<!-- app:layout_constraintEnd_toEndOf="@id/iv_daily_routine_add_theme_background"--> | ||
<!-- app:layout_constraintStart_toStartOf="@id/iv_daily_routine_add_theme_background"--> | ||
<!-- app:layout_constraintTop_toTopOf="@id/iv_daily_routine_add_theme_background" />--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
삭제 부탁드립니닷...!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생했어요~!~~!
val decoration = PageDecoration(decoMargin) | ||
|
||
binding.vpDailyRoutineAddCard.also { | ||
it.offscreenPageLimit = 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3이 쓰인 곳이 많아서, ,companion object중에 의미가 맞는 게 있는지 한 번 살펴보는 것도 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 한번 찾아보겠습니당
offscreenPageLimit = 3 | ||
|
||
val dpValue = 40 | ||
val margin = (dpValue * resources.displayMetrics.density).toInt() | ||
setPadding(margin, 0, margin, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 친구들도 상수로 빼면 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵넵!
<!-- <TextView--> | ||
<!-- android:id="@+id/tv_daily_routine_add_theme_name"--> | ||
<!-- android:layout_width="wrap_content"--> | ||
<!-- android:layout_height="wrap_content"--> | ||
<!-- android:layout_marginTop="6dp"--> | ||
<!-- android:text="@{data.name}"--> | ||
<!-- android:textAppearance="@style/caption1"--> | ||
<!-- android:textColor="@color/gray400"--> | ||
<!-- app:layout_constraintEnd_toEndOf="@id/fl_daily_routine_add_theme"--> | ||
<!-- app:layout_constraintStart_toStartOf="@id/fl_daily_routine_add_theme"--> | ||
<!-- app:layout_constraintTop_toBottomOf="@id/fl_daily_routine_add_theme"--> | ||
<!-- tools:text="테마" />--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
역시 삭제 부탁드립니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생했어요~
app/build.gradle.kts
Outdated
|
||
// indicator | ||
implementation("com.tbuonomo:dotsindicator:5.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 삭제해주세요~
class DailyRoutineAddCardPagerAdapter : | ||
ListAdapter<DailyCard, DailyRoutineAddCardPagerAdapter.DailyPagerViewHolder>( | ||
ItemDiffCallback<DailyCard>( | ||
onItemsTheSame = { oldItem, newItem -> oldItem == newItem }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분에 item이 고유한 값으로 지닌 값으로 비교해주세요~
예시로,
onItemsTheSame = { oldItem, newItem -> oldItem == newItem }, | |
onItemsTheSame = { oldItem, newItem -> oldItem.themeId == newItem.themeId }, |
📑 Work Description
🛠️ Issue
📷 Screenshot
Screen_recording_20240112_234226.mp4
💬 To Reviewers