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

[feat] 핑글 개최 프로세스 - 카테고리 선택 뷰 구현 #56

Merged
merged 19 commits into from
Jan 7, 2024

Conversation

HAJIEUN02
Copy link
Collaborator

@HAJIEUN02 HAJIEUN02 commented Jan 7, 2024

Related issue 🛠

Work Description ✏️

  • 카테고리 레이아웃 뷰 구현
  • 카테고리 선택 뷰 구현
  • 카테고리 단일선택 구현
    • 선택 시 아웃라인 변화
    • 뷰모델에 선택한 데이터 저장

Screenshot 📸

XRecorder_08012024_005936.mp4

Uncompleted Tasks 😅

  • 버튼 활성화 로직은 마지막에 한 번에 추가할 예정!

To Reviewers 📢

우헤헤 바인딩어댑터 안 되는거 싹다 지우고 다시 하고 variable이랑 import 둘다 하니까 됐음요
이게 왜 되지..? 싶지만은 일단 급하니까 올립니당.
넘 신난다 돼서ㅠㅠ!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

Copy link
Collaborator

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

수고핑 일단 머지하구 나중에 리팩합시당

# Conflicts:
#	app/src/main/java/org/sopt/pingle/presentation/ui/main/plan/PlanViewModel.kt
#	app/src/main/java/org/sopt/pingle/util/base/BindingAdapter.kt
#	app/src/main/res/values/strings.xml
@HAJIEUN02 HAJIEUN02 merged commit 9c72461 into develop Jan 7, 2024
1 check passed
@jihyunniiii jihyunniiii deleted the feat-plan-category branch January 7, 2024 20:10
Comment on lines +23 to +25
private fun initLayout() {
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

사용하지 않는 함수 지워주세요.

super.onViewCreated(view, savedInstanceState)

binding.planViewModel = planViewModel
binding.lifecycleOwner = this
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 이미 BindingFragmen에 있어서 적용해주지 않아도 됩니다.
또한, Fragment에서 lifecycleOwner를 잘 설정해주는 것이 중요한데요. 여기서 this를 사용하는 것이 적절할까요?
어떤 걸 쓰면 좋을지 고민해보구 그 이유를 알려주세용

Comment on lines +26 to +37
private fun addListeners() {
with(binding) {
includePlanCategoryTypePlay.root.setOnClickListener {
}
includePlanCategoryTypeStudy.root.setOnClickListener {
}
includePlanCategoryTypeMulti.root.setOnClickListener {
}
includePlanCategoryTypeOthers.root.setOnClickListener {
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 지워주세요

Copy link
Collaborator

Choose a reason for hiding this comment

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

shape_line_white_radius_10

네이밍 컨벤션 지켜주세요

그리고 다음과 같은 형식으로 작성해주세요
(shape 따로 지정 필요 X, Solid 값은 색상 따로 지정해두지 말고 backgroundTint 사용해주세요)

<shape xmlns:android="http://schemas.android.com/apk/res/android">
    <stroke
        android:width="1dp"
        android:color="@color/white"/>
    <corners android:radius="10dp"/>
</shape>

Copy link
Collaborator

Choose a reason for hiding this comment

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

shape_line_g_10_radius_10
네이밍 컨벤션 지켜주세요

그리고 이 파일 없어져도 될 것 같습니다!
backgroundTint로 배경 색상 지정해주면 되기 때문에 shape_border_radius_10 파일 사용하면 됩니다

<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:background="@drawable/selector_item_plan_location_category">
Copy link
Collaborator

Choose a reason for hiding this comment

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

요거 배경 바꾸면서 background tint 조절 고고

android:layout_marginTop="@dimen/spacing3"
android:background="@color/g_10"
android:text="@{categoryType.categoryNameRes}"
android:textAppearance="@style/TextAppearance.Pingle.Title.Semi.20"
Copy link
Collaborator

Choose a reason for hiding this comment

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

bold 18

Comment on lines +78 to +79
android:layout_width="wrap_content"
android:layout_height="0dp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

width -> 0dp
height -> wrap_content

이유는 왤까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

여기 뷰 잘 짰네요!!

@@ -6,6 +6,10 @@
<string name="category_study">STUDY</string>
<string name="category_multi">MULTI</string>
<string name="category_others">OTHERS</string>
<string name="category_play_detail">노는게 제일 좋아!</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

타입에서 선언한 이름대로
category_타입이름_description 으로 해주는 게 좋을 것 같네요

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[feat] 핑글 개최 프로세스 - 카테고리 선택 뷰 구현
2 participants