[UI/#28] Profile / UI 구현 #30
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kkk5474096
approved these changes
Jul 14, 2023
Comment on lines
64
to
69
| val name = profileFriendModel.name | ||
| val yelloId = profileFriendModel.yelloId | ||
| val school = profileFriendModel.school | ||
| val thumbnail = profileFriendModel.thumbnail ?: "" | ||
| ProfileFriendItemBottomSheet.newInstance(name, yelloId, school, thumbnail) | ||
| .show(parentFragmentManager, "dialog") |
There was a problem hiding this comment.
ProfileFriendItemBottomSheet.newInstance(profileFriendModel.name, profileFriendModel.yelloId, profileFriendModel.school, profileFriendModel.thumbnail ?: "")
.show(parentFragmentManager, "dialog")이렇게 해도 되지 않을까용
profileFriendModel.apply {
ProfileFriendItemBottomSheet.newInstance(name, yelloId, school, thumbnail ?: "")
.show(parentFragmentManager, "dialog")
}아니면 이렇게도
There was a problem hiding this comment.
다른 곳에서 안쓰이는 새로운 변수를 굳이 따로 만들어서 보내는거를 피해야 하는거죠 ?!
Comment on lines
22
to
26
| private fun initReturnButton(activity: Activity) { | ||
| binding.btnProfileQuitForSureBack.setOnSingleClickListener { | ||
| activity.finish() | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| private fun initReturnButton(activity: Activity) { | |
| binding.btnProfileQuitForSureBack.setOnSingleClickListener { | |
| activity.finish() | |
| } | |
| } | |
| private fun initReturnButton() { | |
| binding.btnProfileQuitForSureBack.setOnSingleClickListener { | |
| finish() | |
| } | |
| } |
이렇게 구현해도 되지 않나용
Comment on lines
15
to
18
| private lateinit var modelName: String | ||
| private lateinit var modelId: String | ||
| private lateinit var modelSchool: String | ||
| private lateinit var modelThumbnail: String |
There was a problem hiding this comment.
이 값들을 뷰모델에 저장하고 fragment나 activity에서 뷰모델을 공유해서 사용할 수도 있지 않을까 생각이 듭니다.
Comment on lines
22
to
32
| private fun initBackButton(activity: Activity) { | ||
| binding.btnProfileQuitBack.setOnSingleClickListener { | ||
| activity.finish() | ||
| } | ||
| } | ||
|
|
||
| private fun initReturnButton(activity: Activity) { | ||
| binding.btnProfileQuitReturn.setOnSingleClickListener { | ||
| activity.finish() | ||
| } | ||
| } |
There was a problem hiding this comment.
여기도 함수 매개변수로 Activity없이 그냥 finish() 호출해도 될듯합니다
Comment on lines
16
to
20
| if (item.thumbnail != null) { | ||
| binding.ivProfileFriendItemThumbnail.load(item.thumbnail) { | ||
| transformations(CircleCropTransformation()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (item.thumbnail != null) { | |
| binding.ivProfileFriendItemThumbnail.load(item.thumbnail) { | |
| transformations(CircleCropTransformation()) | |
| } | |
| } | |
| item.thumbnail?.let { thumbnail -> | |
| binding.ivProfileFriendItemThumbnail.load(thumbnail) { | |
| transformations(CircleCropTransformation()) | |
| } | |
| } |
이렇게 사용해도 되지 않을까용
There was a problem hiding this comment.
let을 ?.랑 함께 사용하면 null 부분을 대체할 수 있는거로 이해하면 될까요 !
b1urrrr
approved these changes
Jul 14, 2023
Comment on lines
21
to
23
| binding.btnProfileAddGroup.setOnSingleClickListener { | ||
| // TODO: 그룹 추가 로직 | ||
| } |
Comment on lines
41
to
50
| private fun setItemData() { | ||
| binding.tvProfileFriendName.text = modelName | ||
| binding.tvProfileFriendId.text = modelId | ||
| binding.tvProfileFriendSchool.text = modelSchool | ||
| if (modelThumbnail != "") { | ||
| binding.ivProfileFriendThumbnail.load(modelThumbnail) { | ||
| transformations(CircleCropTransformation()) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
SharedViewModel 사용하면 데이터바인딩으로 처리 가넝할 듯 합니다
Comment on lines
55
to
60
| ProflieFriendDeleteBottomSheet.newInstance( | ||
| modelName, | ||
| modelId, | ||
| modelSchool, | ||
| modelThumbnail | ||
| ).show(parentFragmentManager, "Dialog") |
There was a problem hiding this comment.
요부분 바텀시트 새로 띄우는 식으로 구현해도 되는지 한번 여쭤보는 거 어때요?
Comment on lines
65
to
80
| @JvmStatic | ||
| fun newInstance( | ||
| modelName: String, | ||
| modelId: String, | ||
| modelSchool: String, | ||
| modelThumbnail: String | ||
| ) = | ||
| ProfileFriendItemBottomSheet().apply { | ||
| val args = Bundle() | ||
| args.putString("modelName", modelName) | ||
| args.putString("modelId", modelId) | ||
| args.putString("modelSchool", modelSchool) | ||
| args.putString("modelThumbnail", modelThumbnail) | ||
| arguments = args | ||
| } | ||
| } |
There was a problem hiding this comment.
newInstance 활용 아주 굿굿티비!!! 깔끔하다!!!
There was a problem hiding this comment.
최고 근데 뷰모델로 갈아엎었어요 ㅎㅎ
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="10dp" | ||
| android:gravity="center" | ||
| android:text="지금 계정을 탈퇴하시면\n이와 같은 데이터 및 엑세스 권한을 전부 잃게 됩니다." |
Comment on lines
+16
to
+18
| <ImageView | ||
| android:layout_width="80dp" | ||
| android:layout_height="4dp" |
There was a problem hiding this comment.
고정 dp는 최대한 지양하는 것이 기기 호환성에 좋을 것 같아요!
전 웬만하면 wrap_content나 0dp 활용해서 구현합니당
다른 컴포넌트들도 한번씩 확인 부탁드립니다!!
| android:textAppearance="?textAppearanceHeadline3" | ||
| android:textColor="@color/white" | ||
| app:layout_constraintEnd_toStartOf="@id/tv_profile_friend_id" | ||
| app:layout_constraintHorizontal_chainStyle="packed" |
There was a problem hiding this comment.
아주 체인 마스터를 하셧군요 최고다!!!!!!!!!!!!!!
minju1459
approved these changes
Jul 14, 2023
| } | ||
| dialog?.setCanceledOnTouchOutside(false) | ||
| dialog?.setCancelable(true) | ||
| } |
| } | ||
| } | ||
| } | ||
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
⛳️ Work Description
TODO
📸 Screenshot
KakaoTalk_Video_2023-07-14-04-49-33.mp4
📢 To Reviewers