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.

Sign up for GitHub

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

Jump to bottom

[FEAT/#236] 여행 프로필 뷰 / 취향 태그 수정 서버 통신 구현 #243

Merged
merged 15 commits into from
Mar 10, 2024
Merged

[FEAT/#236] 여행 프로필 뷰 / 취향 태그 수정 서버 통신 구현 #243

merged 15 commits into from
Mar 10, 2024

Conversation

leeeyubin
Copy link
Member

@leeeyubin leeeyubin commented Mar 9, 2024 •
edited
Loading

⛳️ Work Description

  • 취향 태그 값 수정 시, 서버 통신 구현
  • 아워 투두 뷰에서 클릭 리스너 구현
  • 기존 뷰랑 잘 합치기
  • isOwner 값에 따라서 버튼 visibility 설정

📸 Screenshot

Screen_Recording_20240310_091901_doorip.mp4

📢 To Reviewers

  • 취향 태그 수정 구현 완료!!!!!
  • 피드백 전부 환영입니둥

@leeeyubin leeeyubin added 유빈 ❄ FEAT ✨ 새로운 기능 구현 labels Mar 9, 2024
@leeeyubin leeeyubin added this to the 2차 스프린트 milestone Mar 9, 2024
@leeeyubin leeeyubin self-assigned this Mar 9, 2024
Marchbreeze
Marchbreeze approved these changes Mar 9, 2024
View reviewed changes
Copy link
Member

@Marchbreeze Marchbreeze left a comment

Choose a reason for hiding this comment

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

음 ~ 마싯다
역시 리드 유빈은 뭔가 달라도 다르넹요

...ation/src/main/java/com/going/presentation/profile/participant/ParticipantProfileActivity.kt Outdated
Comment on lines 38 to 40
private val tripId: Long by lazy {
intent.getLongExtra(TRIP_ID, 0)
}
Copy link
Member

Choose a reason for hiding this comment

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

이런 친구들은 사실 뷰모델에 저장해주는게 로직 상 더 어울리기는 합니다 !

...ation/src/main/java/com/going/presentation/profile/participant/ParticipantProfileActivity.kt
Comment on lines +56 to +61
override fun onResume() {
super.onResume()

getParticipantProfile()
}

Copy link
Member

Choose a reason for hiding this comment

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

onResume함수는 화면이 실행될때 같이 실행이 되기 때문에,
아마 지금 로직 상으로는 getParticipantProfile() 함수가 두번 실행될 것 같습니다 !
한번 로그 확인해보고, 만약 그렇게 진행이 된다면 onCreate에서 지워주세요 ~

Copy link
Member Author

Choose a reason for hiding this comment

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

앗 그렇네요! 생명주기 항상 명심해야겠어요,,

...tion/src/main/java/com/going/presentation/profile/participant/ParticipantProfileViewModel.kt
@@ -22,13 +19,17 @@ class ParticipantProfileViewModel @Inject constructor(
MutableSharedFlow<ParticipantProfileResponseModel?>()
val participantProfile: SharedFlow<ParticipantProfileResponseModel?> = _participantProfile

lateinit var profileTmp: ParticipantProfileResponseModel
Copy link
Member

Choose a reason for hiding this comment

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

요 친구가 아래 함수 이외에 사용되는 곳이 없어보이는데 (아닐수도)
그렇다면 굳이 전역변수 선언 대신에, 아래 emit에서 바로 보내주는 건 어떨까요!

...ava/com/going/presentation/profile/participant/profiletag/ParticipantProfileTagDecoration.kt Outdated

import android.content.Context
import android.graphics.Rect
import android.view.View
import androidx.recyclerview.widget.RecyclerView

class TripProfileTagDecoration(val context: Context) : RecyclerView.ItemDecoration() {
class ParticipantProfileTagDecoration(val context: Context) : RecyclerView.ItemDecoration() {
Copy link
Member

Choose a reason for hiding this comment

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

데코레이션 부분 제가 core-ui에 따로 확장함수처럼 만들어뒀는데, 확인해보고 적용가능해보이면 그거로 공동사용 해봅시다 !

Copy link
Member Author

Choose a reason for hiding this comment

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

네! 저는 적용방법이 살짝 달라서 첫 아이템 부분의 top과 마지막 아이템 부분의 bottom에 값 주는 확장함수 만들어보았는데 확인부탁드립니당

.../java/com/going/presentation/profile/participant/profiletag/ParticipantProfileTagFragment.kt Outdated
Comment on lines 23 to 27
private val participantViewModel by lazy {
ViewModelProvider(requireActivity())[ParticipantProfileViewModel::class.java]
}

private val tagViewModel by activityViewModels<ParticipantProfileTagViewModel>()
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.

오 큰 단위의 뷰모델에서 같이 사용해도 문제가 되지 않네요!! 조언 감사합니당

.../java/com/going/presentation/profile/participant/profiletag/ParticipantProfileTagFragment.kt Outdated
Comment on lines 43 to 45
participantViewModel.participantProfile.flowWithLifecycle(lifecycle).onEach {
getPreferenceIndex()
}.launchIn(lifecycleScope)
Copy link
Member

Choose a reason for hiding this comment

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

이 함수는 옵저버로 보이는데,
onCreate에서 선언해준 옵저버는 이후에 resume이 되어도 계속해서 갑지하도록 작동됩니다 !
확인해보시구, 옵저버는 다시 onCreate에 해주셔도 괜찮을 것 같아요 !

chattymin
chattymin approved these changes Mar 10, 2024
View reviewed changes
Copy link
Member

@chattymin chattymin left a comment

Choose a reason for hiding this comment

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

코드 너무 좋네요~~~!!!

.../java/com/going/presentation/profile/participant/profiletag/ParticipantProfileTagFragment.kt Outdated
Comment on lines 55 to 71
adapter.submitList(
tagViewModel.setPreferenceData(
this.styleA,
this.styleB,
this.styleC,
this.styleD,
this.styleE
)
)
sendPreferenceWithClickListener(
this.styleA,
this.styleB,
this.styleC,
this.styleD,
this.styleE
)
checkIsOwner(this.isOwner)
Copy link
Member

Choose a reason for hiding this comment

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

제가 어제 정신이 없어서 후다닥 말했는데 this는 생략가능합니다 ㅎㅎ....

Copy link
Member Author

Choose a reason for hiding this comment

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

네! 고쳐놓겠습니닷 어제 설명 감사했습니다,,ㅜㅜ

...ation/src/main/java/com/going/presentation/profile/participant/ParticipantProfileActivity.kt
Comment on lines +71 to +74
it?.let { bindData(it) } ?: customSnackBar(
binding.root,
getString(R.string.server_error)
)
Copy link
Member

Choose a reason for hiding this comment

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

절대 let을 쓰지 말라는게 아닙니다!!!
let에 대한 안좋은 의견도 알고 써야 좋은 코드가 될 것 같아서 링크 하나 첨부합니다 :)
왜 let을 쓰는지 알고, 다른 스코프 함수와도 비교하며 쓴다면 완벽하겠죠!

참고링크

@leeeyubin leeeyubin merged commit d2c9e58 into develop Mar 10, 2024
1 check passed
crownjoe
crownjoe reviewed Mar 10, 2024
View reviewed changes
Copy link
Contributor

@crownjoe crownjoe left a comment

Choose a reason for hiding this comment

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

굿굿 늦어서 미안합니다..~...역시 안드오비리드이유빈 다르다! 수고해써요

core-ui/src/main/java/com/going/ui/util/RvItemLastDecoration.kt
@@ -5,7 +5,7 @@ import android.graphics.Rect
import android.view.View
import androidx.recyclerview.widget.RecyclerView

class RvItemDecoration(
class RvItemLastDecoration(
Copy link
Contributor

Choose a reason for hiding this comment

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

네이밍 변경 좋네요!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crownjoe crownjoe crownjoe left review comments

@chattymin chattymin chattymin approved these changes

@Marchbreeze Marchbreeze Marchbreeze approved these changes

Assignees

@leeeyubin leeeyubin

Labels
FEAT ✨ 새로운 기능 구현 유빈 ❄
Projects
None yet
Milestone
2차 스프린트
Development

Successfully merging this pull request may close these issues.

[FEAT] 여행 프로필 뷰 / 취향 태그 수정 서버 통신 구현
4 participants
@leeeyubin @chattymin @Marchbreeze @crownjoe