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

#16 [ui] 홈 / to do 리사이클러뷰 #20

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

2zerozu
Copy link
Collaborator

@2zerozu 2zerozu commented Jul 8, 2022

작업 사진

image

작업 개요

  • 홈 to do 리사이클러뷰 생성

작업 설명

  • ToDoData 생성
  • item_home_to_do 생성
  • fragment_home에 리사이클러뷰 생성
  • ToDoAdapter 생성
  • adapter와 리사이클러뷰 연결

궁금한점

  • 체크박스를 어떻게 바인딩할지, 그리고 그 후에 체크한 값들을 어떻게 넘길지 고민해봐야겠다.

어려웠던점

  • 체크박스에 android:button="@android:color/transparent"를 넣지 않으면 기존 체크박스가 없어지지 않는다.

@murjune
Copy link
Member

murjune commented Jul 8, 2022

고생하셨습니다 :D

Comment on lines +59 to 60
android:layout_width="170dp"
android:layout_height="172dp"
Copy link
Member

Choose a reason for hiding this comment

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

wrap과 0dp를 사용해서 container 크기를 조정해주세요!!

Comment on lines +87 to +88
android:layout_width="170dp"
android:layout_height="172dp"
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 +20 to +21
android:layout_width="16dp"
android:layout_height="16dp"
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나 0dp 사용하기!

@2zerozu 2zerozu requested a review from murjune July 8, 2022 12:59
Copy link
Member

@KWY0218 KWY0218 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 +28 to +30

<TextView
android:id="@+id/tv_home_to_do"
Copy link
Member

Choose a reason for hiding this comment

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

현재 fragment_home 에도 tv_home_to_do 라는 id가 사용되고 있습니다.
type_where_what 중 where 쪽을 구분해야 될 것 같습니다

Copy link
Member

Choose a reason for hiding this comment

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

한가지 궁금한 점이 다른 layout인데 textView id값앞에 where을 다르게 해야하는 이유가 있을까요??

Copy link
Member

Choose a reason for hiding this comment

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

@murjune 일단 늦어서 죄송합니다.
layout id를 달리하는 이유는 제가 안드로이드 처음 공부할 때 findVIewById 를 사용하다가 다른 레이아웃에 id가 같아서 오류가 생긴 기억 때문에 항상 그렇게 했습니다.

형 말 듣고 생각해보니까, dataBinding, ViewBinding 은 한 레이아웃만 보니까 굳이 달라야 할 이유가 없네요...?
그래서 컨벤션에 where 빼도 될 것 같다는 생각이 드네요..? 제가 컨벤션에 where은 넣을렸던 이유는 where을 넣으면 id 중복을 피할 수 있을 것 같다는 생각했기 때문에 넣었거든요..

다른분들 생각은 어떤가요??

Comment on lines +27 to +33
class ToDoViewHolder(
private val binding: ItemHomeToDoBinding
) : RecyclerView.ViewHolder(binding.root) {
fun onBind(data: ToDoData) {
binding.tvHomeToDo.text = data.rules
}
}
Copy link
Member

Choose a reason for hiding this comment

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

item 쪽엔 data로 variable을 받는다고 선언했지만 사용하지 않았으며,
data class가 한개 일 때 굳이 data class를 만들지 않아도 String을 사용해도 recyclerview 동작이 가능합니다

현재로선 후자의 방식이 나아 보이지만, api가 나오면 data class 중 하나의 값을 바인딩할 것 같으니
전자의 방식으로 수정해주시면 좋을 것 같습니다

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 +7 to +9

class ToDoAdapter : RecyclerView.Adapter<ToDoAdapter.ToDoViewHolder>() {
val toDoList = mutableListOf<ToDoData>()
Copy link
Member

Choose a reason for hiding this comment

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

adapter는 model 패키지 보다 home 패키지 안에 있는 것이 좋을 것 같습니다

Copy link
Member

Choose a reason for hiding this comment

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

아 맞아 이거 말해주고 싶었는데 ㅋㅋㅋ
첨언으로 model패키지는 ui패키지가 아니라 data패키지에 넣어주시면 좋을 것 같슴니다!!
MainActivity도 ui.home이 아닌 ui패키지에 넣어주시면 좋을 것 같숩니다 ㅎ ㅅ ㅎ

@murjune murjune requested a review from KWY0218 July 9, 2022 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants