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

#176 [ui] 루틴 추가하기 - 리스트 뷰 #177

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

Conversation

pump9918
Copy link
Collaborator

@pump9918 pump9918 commented Jun 23, 2024

📑 Work Description

  • [4] 루틴 추가하기의 리스트 뷰 : ui/addroutine/list
  • MockAddRoutineRepository로 가짜 데이터 주입
  • dim dialog 적용, WindowManager 사용 => 위치 조정
  • 서버에서 받은 themeId 값으로 로컬 테마 이미지 대응
  • Recycler 뷰, Viewpager 적용

🛠️ Issue

📷 Screenshot

Screen_Recording_20240623_223637_Softie.mp4

💬 To Reviewers

  • 가능한 dimen 안쓰고 싶은데 dialog 위치를 조정하는 방법을 window 말고는 잘 모르겠네요..ㅜ(너무 지저분함)
  • MakerCard는 아직 api 명세서가 제대로 다 나오지 않아 임의로 데이터 클래스 만든겁니당 추후 수정 가능
  • 뷰페이저는 이전처럼 dot indicator를 써야할 지, 현재대로 유지할지 디쌤과 논의 필요

@pump9918 pump9918 added UI ui 관련 작업 호연🐻‍❄️ 호연이가 작업함! labels Jun 23, 2024
@pump9918 pump9918 self-assigned this Jun 23, 2024
Copy link
Contributor

@stellar-halo stellar-halo 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 49 to 53
<activity
android:name=".ui.addroutine.list.AddListActivity"
android:exported="false"
android:screenOrientation="portrait" />

Copy link
Contributor

Choose a reason for hiding this comment

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

AddRoutineActivty는 어떤가요? AddList는 조금 모호한 클래스명같습니다!


@AndroidEntryPoint
class AddListActivity : BindingActivity<ActivityAddListBinding>(R.layout.activity_add_list) {
private lateinit var viewPager: ViewPager2
Copy link
Contributor

Choose a reason for hiding this comment

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

뷰페이저가 아닌 recylerView로 개발해주시면 됩니다. vertical이 아닌 horizontal로 구현하면 되고
전에 이야기 했다 싶이, 1개일 경우와 n개일 때의 카드 뷰 크기가 달라집니다. (figma 확인해보세요!)
-> 1개일 경우, recyclerView를 invisible 하고 개별 컴포넌트 하나를 visible 처리
or 1개일 경우, recyclerView item 너비를 조정하는 코드를 추가하는 방법이 어떤가 싶습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

리사이클러뷰로 다시 짜보겠습니다...

import javax.inject.Inject

@HiltViewModel
class AddListViewModel @Inject constructor(
Copy link
Contributor

Choose a reason for hiding this comment

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

마찬가지입니다. 추후에 혼동을 줄수있는 클래스명인 것 같습니다. 길더라도 좀 더 명시적으로 짓는 것은 어떨까요?

import com.sopetit.softie.databinding.ItemAddListMakerCardBinding
import com.sopetit.softie.domain.entity.MakerCard

class MakerCardPagerAdapter : RecyclerView.Adapter<MakerCardPagerAdapter.MakerPagerViewHolder>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

viewPager와 RecyclerView 구현은 크게 다르지 않으니 고치기 어렵진 않을겁니다!
지금은 데이터를 뿌려주는 코드만 적혀있는데, 이후에 페이지로 진입해야 되는 걸 감안하면, 클릭한 아이템의 id를 반환하는 함수를 미리 추가해도 좋아보입니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영해보겠습니다

</data>

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/cl_add_list"
Copy link
Contributor

Choose a reason for hiding this comment

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

뷰 이름이 바뀌면 여기도 전부 바뀌어야겠죠,,~,,

Comment on lines 43 to 49
<androidx.core.widget.NestedScrollView
android:layout_width="0dp"
android:layout_height="0dp"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/tv_add_list_title">
Copy link
Contributor

Choose a reason for hiding this comment

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

nestedScrollView를 쓰면 recyclerView가 재활용되지 않는 단점이 있다고 해요!
recyclerView를 쓰는 이유가 없어지는 겁니다. 그래서 3중첩 recyclerView를 해야 되는 이유가 여기서 나오게 되는 건데, 지금은 2중접밖에 안됐습니다.
물론 저희가 theme이 많진 않아서 recylerView의 이점을 크게는 활용하지 않아도 문제되지 않을 것 같긴,,합니다만 좋은 방법은 아니라 생각합니다!
만일 고칠 의향이 있으시다면, 멀티뷰타입을 적용해(YB시절 적용해본 경험이 있을 것이에요~) recyclerView로 바깥을 감싸주는 방법이 있습니다!
https://velog.io/@suev72/%EC%8A%A4%ED%81%AC%EB%A1%A4%EB%B7%B0-%EC%97%86%EC%95%A0%EA%B3%A0-%EB%A6%AC%EC%82%AC%EC%9D%B4%ED%81%B4%EB%9F%AC%EB%B7%B0%EB%A1%9C-%EB%A0%88%EC%9D%B4%EC%95%84%EC%9B%83-%EA%B3%A0%EC%B9%98%EA%B8%B0

app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="@id/iv_add_list_maker_chip" />

<androidx.viewpager2.widget.ViewPager2
Copy link
Contributor

Choose a reason for hiding this comment

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

viewPager아니고 RecyclerView!

</data>

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/cl_maker_help_dialog_border"
Copy link
Contributor

Choose a reason for hiding this comment

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

tooltip으로 고쳐봅시다

android:layout_height="match_parent">

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/cl_happy_routine_add_card"
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
Labels
UI ui 관련 작업 호연🐻‍❄️ 호연이가 작업함!
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

[UI] 루틴 추가
2 participants