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

[Feature/mypage] 2차 스프린트 mypage 수정 #158

Merged
merged 9 commits into from
Jul 18, 2023
Merged

Conversation

librarywon
Copy link
Contributor

@librarywon librarywon commented Jul 14, 2023

이슈 코드

📸 스크린샷

스크린샷 2023-07-14 오후 3 14 13

🍀 관련 이슈

  • 2차 api가 나오면 네트워크 관련 로직도 수정하겠습니다!
  • 프로필 뒤에 배경은 아직 임시로 넣은 이미지입니다. 추후 일러스트가 완성되면 교체할 예정입니다.

@librarywon librarywon added 🦈서재원 서재원이 했다 ✨ Feature 기능 개발 labels Jul 14, 2023
@librarywon librarywon requested a review from a team as a code owner July 14, 2023 06:16
@librarywon librarywon self-assigned this Jul 14, 2023
Copy link
Member

@kez-lab kez-lab left a comment

Choose a reason for hiding this comment

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

LGTM🍀 고생하셨습니다. 수정만 하고 머지 부탁드려욥

Comment on lines 76 to 78
android:id="@+id/iv_mypage_profile"
android:layout_width="72dp"
android:layout_height="72dp"
android:layout_marginTop="66dp"
Copy link
Member

Choose a reason for hiding this comment

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

dimentionRatio 도 활용하면 좋을 것 같네욥

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 195 to 204
<ImageView
android:id="@+id/iv_mypage_story_ic"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:src="@drawable/ic_chevron_right"
app:layout_constraintBottom_toBottomOf="@id/tv_mypage_story_title"
app:layout_constraintStart_toEndOf="@id/tv_mypage_story_title"
app:layout_constraintTop_toTopOf="@id/tv_mypage_story_title" />
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.

아 그럴수가 있군요 고정크기를 잡아서 반영하겠습니다 ㅎㅎ

Comment on lines 76 to 87
private fun setOnClickListener() {
with(binding) {
//setting 이동
ivToolbarSetting.setOnClickListener {
startActivity(Intent(requireContext(), SettingActivity::class.java))
}
}

gridLayoutManager.spanSizeLookup = object : GridLayoutManager.SpanSizeLookup() {
override fun getSpanSize(position: Int): Int {
return when (myPageAdapter?.getItemViewType(position)) {
VIEW_TYPE_PROFILE -> 3
VIEW_TYPE_PHOTO -> 1
VIEW_TYPE_EMPTY -> 3
else -> 1
}
//share 이동
layoutMypageShare.setOnClickListener{
//TODO Intent to Share
}
//stroy 이동
layoutMypageStory.setOnClickListener{
//TODO Intent to Story
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

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

색상값만 잘 수정하면 머지 바로 가능할 듯

AlbumDetailActivity.newIntent(requireContext(), photos.photo).let(::startActivity)
private fun setOnClickListener() {
with(binding) {
//setting 이동
Copy link
Member

Choose a reason for hiding this comment

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

주석 삭제 plz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!

<shape android:shape="rectangle"
xmlns:android="http://schemas.android.com/apk/res/android">
<corners android:radius="16dp"/>
<solid android:color="@color/gray_20"/>
Copy link
Member

Choose a reason for hiding this comment

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

?colorOnSurface20

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영하겠습니다!

android:layout_width="match_parent"
android:layout_height="0dp"
android:layout_height="124dp"
android:background="#EBE8FF"
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.

일러스트가 들어올 영역을 잡아둔 친구라서 일러스트가 들어오면 삭제 예정입니다!

Comment on lines 158 to 159
android:layout_width="114dp"
android:layout_height="103dp"
Copy link
Member

Choose a reason for hiding this comment

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

wrap_content나 parent에 margin 걸어서 해결할 순 없을까?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

해결해보겠습니다!

@@ -13,6 +13,7 @@
android:background="?colorSurface"
app:layout_constraintBottom_toTopOf="@id/div_mypage"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintHorizontal_bias="0.0"
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.

자동으로 추가된 부분 같습니다 삭제하겠습니다!

@l2hyunwoo l2hyunwoo marked this pull request as draft July 16, 2023 07:47
@l2hyunwoo l2hyunwoo added this to the v1.1.0 milestone Jul 16, 2023
@@ -58,7 +59,8 @@ class MyPageFragment : Fragment() {
hideLoading()
with(binding) {
val profile = myPageInfoState.data
tvMypageToolbarNickname.text = "@${profile.nickname}"
toast(myPageInfoState.data.toString())
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.

넵 삭제하였습니다!

val photoCount: Int
) {
fun toProfile(): Profile {
return Profile(id,realName, nickname, profileImageUrl,photoCount)
Copy link
Member

Choose a reason for hiding this comment

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

lint!

Comment on lines +83 to +88
layoutMypageShare.setOnClickListener {
//TODO Intent to Share
}
//stroy 이동
layoutMypageStory.setOnClickListener {
//TODO Intent to Story
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

@l2hyunwoo l2hyunwoo left a comment

Choose a reason for hiding this comment

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

👍🏻

@librarywon librarywon marked this pull request as ready for review July 17, 2023 17:58
@librarywon librarywon merged commit 502c5d3 into develop Jul 18, 2023
@librarywon librarywon deleted the feature/mypage branch July 18, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 기능 개발 🦈서재원 서재원이 했다
Projects
None yet
3 participants