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

#36 [ui] home 작업 #44

Merged
merged 28 commits into from
Jan 14, 2024
Merged

#36 [ui] home 작업 #44

merged 28 commits into from
Jan 14, 2024

Conversation

stellar-halo
Copy link
Contributor

📑 Work Description

  • home뷰 작업했습니다!

🛠️ Issue

📷 Screenshot

Screen_recording_20240112_232751.mp4

💬 To Reviewers

  • 이슈 번호를 중간에 착각해서 commit message issue number를 잘못 썼습니다,! ㅠㅠ
  • 서버 통신만 하면 연결될 수 있게, 최대한 코드를 짰습니다. 참고 부탁드려요!
  • 코드를 예쁘게 짜고 싶었는데, 분기처리가 많아서 쉽지 않네요..

@stellar-halo stellar-halo added UI ui 관련 작업 Pull Request pr 날림! 강희🐬 강희가 작업함! labels Jan 12, 2024
@stellar-halo stellar-halo requested a review from minemi00 January 12, 2024 15:31
@stellar-halo stellar-halo self-assigned this Jan 12, 2024
@pump9918 pump9918 requested a review from thguss January 12, 2024 15:51
Copy link
Member

@thguss thguss 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 +37 to +46
private fun setUserLottieList() {
viewModel.getHome()
userLottieList = when (viewModel.homeResponse.value?.dollType) {
"BROWN" -> brownBearLottieList
"GRAY" -> grayBearLottieList
"RED" -> redBearLottieList
"PANDA" -> pandaBearLottieList
else -> brownBearLottieList
}
}
Copy link
Member

Choose a reason for hiding this comment

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

리스트 형식과 switch도 좋지만 Map 형식은 어떤가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

찾아오겠습니다.! ! !

Comment on lines +69 to +70
val intentToSetting = Intent(activity, SettingActivity::class.java)
startActivity(intentToSetting)
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 +100 to +101
viewModel.patchSom(cottonType)
playLottieAnimation(userLottieList[DAILY])
Copy link
Member

Choose a reason for hiding this comment

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

2줄을 담당하는 메소드를 따로 지정하면 해당 메소드의 책임이 더 줄 수 있을 것 같아요!

Comment on lines +33 to +36
val changeData = when (cotton) {
Cotton.DAILY -> homeResponse.value?.copy(dailyCottonCount = cottonCount - 1)
Cotton.HAPPINESS -> homeResponse.value?.copy(happinessCottonCount = cottonCount - 1)
}
Copy link
Member

Choose a reason for hiding this comment

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

switch문만 담당하는 메소드를 따로 분리해서, 메소드를 호출하는 방식도 가독성에 좋을 것 같아요!

Copy link
Member

Choose a reason for hiding this comment

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

daily와 happiness가 반복되니까 따로 메소드로 빼주는 것도 좋을 것 같습니당

Copy link
Contributor Author

Choose a reason for hiding this comment

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

같은 줄 같지만 미묘하게 다릅니다! 변수로 넘겨서 처리하는 건 또 같은 얘기일 것 같아서 분리는 안했습니다만,,, 어디까지가 함수화가 좋을지 고민이 매번 됩니다

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.

메인작업 고생하셨습니다~

@@ -0,0 +1,5 @@
package com.sopetit.softie.domain.entity

enum class Cotton {
Copy link
Collaborator

Choose a reason for hiding this comment

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

DAILY와 HAPPINESS를 왜 enum으로 묶은 건가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

각 상황에 따라서 로직은 동일한데 넣는 파일이 달라서 분류해주려고 만들었습니다!
enum class를 만들면 when에서 else 처리가 따로 필요없어서 만들었습니다.

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.

홈뷰 작업 수고하셨습니당!

dollType = "PANDA",
dailyCottonCount = 5,
happinessCottonCount = 3,
conversations = listOf("야", "메롱", "루틴이나 해", "솔직히 나 귀엽지", "안드로이드 사랑해", "나 소프티야")
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

Choose a reason for hiding this comment

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

안드소프리티 사랑해

Comment on lines +60 to +65
private fun setClickListener() {
setClickSetting()
setClickBear()
setClickDaily()
setClickHappy()
}
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 Author

Choose a reason for hiding this comment

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

아앗 부끄럽다,,,😥 더 열심히 코드 짤게요,,,

setRandomMessage()
}

private fun setRandomMessage() {
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 Author

Choose a reason for hiding this comment

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

랜덤 메세지 list를 받고 그 리스트 안에서 띄우는 건 클라가 처리합니다!

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 22 to 27
android:exported="true"
android:screenOrientation="portrait" />

<activity
android:name=".ui.onboarding.OnboardingActivity"
android:exported="false"
android:exported="true"
Copy link
Member

Choose a reason for hiding this comment

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

mainActivity랑 onboardingActivity는 false로 바꿔주세요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 감사합니다~

dollType = "PANDA",
dailyCottonCount = 5,
happinessCottonCount = 3,
conversations = listOf("야", "메롱", "루틴이나 해", "솔직히 나 귀엽지", "안드로이드 사랑해", "나 소프티야")
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 +33 to +36
val changeData = when (cotton) {
Cotton.DAILY -> homeResponse.value?.copy(dailyCottonCount = cottonCount - 1)
Cotton.HAPPINESS -> homeResponse.value?.copy(happinessCottonCount = cottonCount - 1)
}
Copy link
Member

Choose a reason for hiding this comment

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

daily와 happiness가 반복되니까 따로 메소드로 빼주는 것도 좋을 것 같습니당

@stellar-halo stellar-halo merged commit a0da019 into develop Jan 14, 2024
1 check passed
@stellar-halo stellar-halo deleted the feature/#36-ui-home branch January 14, 2024 09:54
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
Status: Done
Development

Successfully merging this pull request may close these issues.

[ui] 홈 뷰
5 participants