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

#39 [ui] 행복루틴 추가하기 상세뷰 #43

Merged
merged 20 commits into from
Jan 14, 2024

Conversation

pump9918
Copy link
Collaborator

@pump9918 pump9918 commented Jan 12, 2024

📑 Work Description

  • 행복루틴 추가하기(상세) 뷰 작업
  • ViewPager2 사용, 양 옆 보이게 설정
  • 인디케이터 적용

🛠️ Issue

📷 Screenshot

Screen_Recording_20240114_183940_Softie.mp4

💬 To Reviewers

  • 행복루틴 진행중 뷰 짜겠습니다~

@pump9918 pump9918 added UI ui 관련 작업 호연🐻‍❄️ 호연이가 작업함! labels Jan 14, 2024
@pump9918 pump9918 self-assigned this Jan 14, 2024
@pump9918 pump9918 added the Pull Request pr 날림! label Jan 14, 2024
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.

비율 맞추느라 고생하셨습니다!

with(binding.vpHappyAddDetailCard) {
clipChildren = false
clipToPadding = false
offscreenPageLimit = 3
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
Collaborator Author

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.

고생하셨습니다! 반복되는 부분들을 함수로 빼거나, 변수명 조금만 다듬어주세요~

) {
data class Routines(
val routineId: Int,
@DrawableRes val cardImageUrl: Int,
Copy link
Contributor

Choose a reason for hiding this comment

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

꼼꼼하네요

android:name=".ui.main.happy.addlist.HappyAddListActivity"
android:exported="false"
android:name=".ui.happyroutine.addlist.HappyAddListActivity"
android:exported="true"
Copy link
Contributor

Choose a reason for hiding this comment

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

false로 바꿔서 올려주세요!


<activity
android:name=".ui.happyroutine.adddetail.HappyDetailActivity"
android:exported="true"
Copy link
Contributor

Choose a reason for hiding this comment

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

false로 바꿔서 올려주세요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ㅇㅊㅊ

Comment on lines 34 to 39
happyCard?.let {
binding.tvHappyAddDetailTitle.text = happyCard.name
binding.ivHappyAddDetailIcon.setImageResource(happyCard.iconImageUrl)
binding.tvHappyAddDetailSubtitle.text = happyCard.title
binding.tvHappyAddDetailTitle.setTextColor(Color.parseColor(happyCard.nameColor))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

with(binding)으로 묶어주세요

Copy link
Contributor

Choose a reason for hiding this comment

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

onCreate 안이 아닌 따로 함수로 빼주세요

}
}

private fun moveToIng() {
Copy link
Contributor

Choose a reason for hiding this comment

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

moveToProgress처럼 바꿔주세요!

Comment on lines 26 to 31
binding.ivHappyRoutineAddCard.setImageResource(data.cardImageUrl)
binding.tvHappyRoutineAddCardDetailTitle.text = data.detailTitle
binding.tvHappyRoutineAddCardDetailTitleBack.text = data.detailTitle
binding.tvHappyRoutineAddCardDetailContentBack.text = data.detailContent
binding.tvHappyRoutineAddCardDetailTimeBack.text = data.detailTime
binding.tvHappyRoutineAddCardDetailPlaceBack.text = data.detailPlace
Copy link
Contributor

Choose a reason for hiding this comment

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

다음 번에는 item.xml에서 data binding 써보세요! activity, fragment에서 쓰는 것처럼 코드가 간결해집니다.

  • with(binding) 써주세요

Comment on lines 35 to 36
binding.clHappyRoutineAddCard.visibility = View.INVISIBLE
binding.clHappyRoutineAddCardBack.visibility = View.VISIBLE
Copy link
Contributor

Choose a reason for hiding this comment

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

이 로직은 뒷면이 보이는 로직이고
else 아래는 앞면이 보이는 로직이네요!

아래에도 공통의 로직이 있는데, 함수로 빼봅시다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오 그게 더 깔끔하겠네요 함수화하겠습니다!

Comment on lines 44 to 49
if (isVisible) {
binding.clHappyRoutineAddCardBack.visibility = View.INVISIBLE
binding.clHappyRoutineAddCard.visibility = View.VISIBLE
} else {
binding.clHappyRoutineAddCardBack.visibility = View.VISIBLE
binding.clHappyRoutineAddCard.visibility = View.INVISIBLE
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (isVisible) {
binding.clHappyRoutineAddCardBack.visibility = View.INVISIBLE
binding.clHappyRoutineAddCard.visibility = View.VISIBLE
} else {
binding.clHappyRoutineAddCardBack.visibility = View.VISIBLE
binding.clHappyRoutineAddCard.visibility = View.INVISIBLE
if (isVisible) {
showCardFront()
} else {
showCardBack()

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 +226 to +228
fun getHappyCardListForId(categoryId: Int): List<HappyCard> {
return _mockHappyCardList.value?.filter { it.categoryId == categoryId } ?: emptyList()
}
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 38 to 41
<string name="happy_routine_add_card_detail_title">이상형의 특성 10가지 적어보고,\n절대 포기 못할 2가지와\n나와 비슷한 3가지 찾아보기</string>
<string name="happy_routine_add_card_detail_content">평소에 바빠서 연락하지 못한 사람이 있다면 안부인사 개인톡을 보내 봐. 꼭 만나서 밥을 먹거나 하지 않아도 연락 한 통이 나와 그 사람을 연결하는 방법이 될 수 있어</string>
<string name="happy_routine_add_card_detail_time">5~10분</string>
<string name="happy_routine_add_card_detail_place">회사 옥상, 점심식사 후 돌아가는 길</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

서버가 주는 값들은 tools:text로 처리한 뒤에 string으로 추출한 거 제거해주세요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영했습니다!!

Copy link
Member

@emjayMJkim emjayMJkim 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 44 to 49
if (isVisible) {
binding.clHappyRoutineAddCardBack.visibility = View.INVISIBLE
binding.clHappyRoutineAddCard.visibility = View.VISIBLE
} else {
binding.clHappyRoutineAddCardBack.visibility = View.VISIBLE
binding.clHappyRoutineAddCard.visibility = View.INVISIBLE
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 15 to 16
onItemsTheSame = { oldItem, newItem -> oldItem.routineId == newItem.routineId },
onContentsTheSame = { oldItem, newItem -> oldItem.routineId == newItem.routineId }
Copy link
Member

Choose a reason for hiding this comment

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

onContentsTheSame은 { oldItem, newItem -> oldItem == newItem }으로 바꿔주세요!

onItemsTheSame = { oldItem, newItem -> oldItem.routineId == newItem.routineId },
onContentsTheSame = { oldItem, newItem -> oldItem == newItem }

Copy link
Collaborator 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.

앗차차~ 거긴 제가 잘못했네요~

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.

정말Ing로 가는 거야,,?

@pump9918 pump9918 merged commit affb3f6 into develop Jan 14, 2024
1 check passed
@pump9918 pump9918 deleted the feature/#39-ui-happy-add-view-detail branch January 14, 2024 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pull Request pr 날림! UI ui 관련 작업 호연🐻‍❄️ 호연이가 작업함!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui] 행복루틴 추가하기(상세) 뷰
4 participants