[스터디 관리하기 뷰 시작 후] 스터디 관리하기 뷰 기능 구현 #140

RightHennessy · 2023-07-30T13:22:41Z

😋 작업한 내용

ViewPager 연결
스터디의 선택 투두 RecyclerView 구현
스터디의 멤버 RecyclerView 구현
이전 회차, 다음 회차 버튼에 대한 로직 구현

🙏 PR Point

뷰페이저 어댑터의 리스트에 데이터를 넣는 과정이 회차별 정보를 각각 따로 서버 통신을 해야해서
이에 관한 로직은 서버 연결할 때 완성될 것 같습니다 ~

👍 관련 이슈

Resolved : [스터디 관리하기 뷰 시작 후] 서버 연결을 제외한 기능 구현 #120

github-actions · 2023-07-30T13:29:05Z

Test Results

13 tests 13 ✔️ 3s ⏱️
  7 suites   0 💤
  7 files   0 ❌

Results for commit c9a9439.

♻️ This comment has been updated with latest results.

inseonyun

어떤 로직은 도메인에 있고 어떤 로직은 UI에 있고 관심사 분리가 조금 덜 된 거 같네용
매직 넘버가 너무 많아요!!!!
피드백 남겨드린거 참고해서 반영 부탁드립니다

inseonyun · 2023-07-31T02:14:35Z

...pp/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementActivity.kt

+        val studyId = intent.getLongExtra(KEY_STUDY_ID, KEY_ERROR)
+        val roundId = intent.getLongExtra(KEY_ROUND_ID, KEY_ERROR)
+        studyManagementViewModel.getStudyRounds(studyId, roundId)


KEY_ERROR 가 되었을 때는 어떤 플로우를 타게 되나요?

아직 이부분에 대한 코드가 없습니다..
서버 연결을 하면서 오류처리를 어떻게 할지 결정할 것 같습니다.

inseonyun · 2023-07-31T02:25:39Z

...pp/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementActivity.kt

+
+    private fun setPageChangeButtonEnabled() {
+        binding.ivStudyManagementPreviousButton.isEnabled =
+            studyManagementViewModel.currentRound.value != 1


매직 넘버

inseonyun · 2023-07-31T02:26:30Z

...pp/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementActivity.kt

+        binding.ivStudyManagementPreviousButton.setOnClickListener {
+            val page = (binding.vpStudyManagement.currentItem - 1).coerceAtLeast(0)
+            binding.vpStudyManagement.setCurrentItem(page, true)
+            studyManagementViewModel.fetchRoundDetail(page)
+        }
+        binding.ivStudyManagementNextButton.setOnClickListener {
+            val page =
+                (binding.vpStudyManagement.currentItem + 1).coerceAtMost(studyManagementAdapter.itemCount - 1)
+            binding.vpStudyManagement.setCurrentItem(page, true)
+            studyManagementViewModel.fetchRoundDetail(page)
+        }


페이지 계산 로직이 뷰에 있어야하는게 맞을까요?

PageIndex에서 관리하도록 분리함슴

inseonyun · 2023-07-31T02:28:46Z

...p/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementViewModel.kt

+        val studyDetails = studyRounds.value ?: throw IllegalStateException()
+        val currentStudy = studyRounds.value?.get(currentItemId) ?: throw IllegalStateException()


업데이트하지 못하면 터진다..?
무시무시한 앱이군요...
의도한 흐름인가요?

무시무시하죠?

inseonyun · 2023-07-31T02:29:08Z

...p/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementViewModel.kt

+        when (isNecessary) {
+            true -> {
+                updateNecessaryTodoCheck(studyDetails, todoId, isDone)
+                study = studyDetails.find { it.necessaryTodo.todoId == todoId }!!


!!
해결할 방법이 없을까요?

산군코드라 생략하겠습니다.

궁금한점이 가짜 study를 생성해서 넣어주는게 더 좋다고 생각하나요?

아뇨 그냥 그 뷰 안 띄우고, 토스트로 안내 해주는 것이 전 더 좋다고 봅니다

스터디 하면서 먼말인지 알아들었습니다

inseonyun · 2023-07-31T02:34:06Z

...m/created/team201/presentation/studyManagement/adapter/StudyManagementOptionalTodoAdapter.kt

+    override fun onBindViewHolder(holder: StudyOptionalTodoViewHolder, position: Int) {
+        when (getItemViewType(position)) {
+            1 -> (holder as StudyManagementOptionalTodoAddViewHolder).bind(getItem(position))
+            else -> (holder as StudyManagementOptionalTodoViewHolder).bind(getItem(position))


분기처리 else에서 뷰홀더 연결보다는 1 -> StudyManagementOptionalAdd...ViewHolder 처럼
2 -> StudyManagementOptionalViewHolder 이렇게 처리하고 else에서는 예외를 발생하는 것이 명시적이고 좋을 거 같아요

OptionalTodoViewType에서 예외처리 했습니다.

inseonyun · 2023-07-31T02:36:42Z

...ted/team201/presentation/studyManagement/adapter/StudyManagementOptionalTodoAddViewHolder.kt

+    override fun bind(optionalTodoUiModel: OptionalTodoUiModel) {
+    }


추후에 쓰일 함수인가여?

아니라면 상위 클래스에 bind 함수를 굳이 둬서 하위 클래스에서 이러한 불필요한 함수 구현부를 계속 두게 돼서 불편할 거 같아요

제거하였습니다

inseonyun · 2023-07-31T02:38:27Z

...p/src/main/java/com/created/team201/presentation/studyManagement/model/StudyMemberUiModel.kt

+    val profileImageUrl: String,
+    val isDone: Boolean,
+) {
+    fun progressPercentage(): Int = if (isDone) 100 else 0


ui model에 있어야할 로직인가요?

커스텀 뷰의 경우 프로그레스 바의 기본 색이 필수투두를 완료한 색상으로 지정되어 있습니다.
또한 프로그레스가 0이라면 필수투두를 미완료한 색상으로 지정되어있습니다.
다시 말해 티어를 지정해주지 않으면 필수투두를 완료 했냐 안했냐를 표현해줄 수 있습니다.
때문에 isDone으로 투두를 완료했으면 progress가 100 미완료라면 0으로 표현해주는 겁니다.
뷰의 관심사지만 액티비티 또는 xml에 위치해 있는거도 자연스럽지 않다 생각해서 그외에 가장 어울리는 파일에 작성한 것입니다.
액티비티에 따로 로직을 나누고 적용시킨다면 databinding expression을 사용하는 의미가 약간 퇴색되는거 같습니다.
저는 일단 그렇게 생각하는데 혹시 다른 의견이 있으신가요?

뷰 로직이라고 생각된다면, uiModel에 있어도 될 것 같습니다!
근데 왜 Activity에서 뷰로직을 작성하면, dataBinding의 의미가 퇴색된다고 생각했나요?

뷰 로직이다. 해당 퍼센테이지의 수치가 필수투두 완료 =10000으로 바뀐다면, 뷰모델까지 바뀌는가?

UiModel, BindingAdapter, Activity에 위치해도 될 것 같다.

해당 UiModel이 뷰에 의존하고 있는가? 뷰라고 생각하는가?

해당 UiModel을 혹시 뷰모델에서 생성하고 있는가? 그럼 뷰모델은 뷰를 참조하고 있는가?

inseonyun · 2023-07-31T02:39:10Z

android/app/src/main/res/layout/item_study_management.xml

@@ -36,7 +44,7 @@
            android:textAppearance="@style/text_r12"
            android:textColor="@color/white"
            app:drawableStartCompat="@drawable/ic_edit_12"
-            app:isVisible="@{isMaster}"
+            app:isVisible="@{role.Companion.isMaster(studyManagement.role)}"


오,,신기

뷰에 도메인 임포트.. 어떻게 생각하시나요?

inseonyun · 2023-07-31T02:41:47Z

android/app/src/main/res/layout/item_study_management_study_member.xml

+
+        <variable
+            name="onClickMember"
+            type="com.created.team201.presentation.studyManagement.StudyMemberClickListener" />


그냥 갑자기 든 생각인데, xml에서 이렇게 개발자가 만든 listener라는 함수만을 받도록 만드는 것과
kotlin.jvm.functions... 을 받는 것과 어떤게 더 나을까욤
저는 이왕 추상화하는 거 후자가 더 나을 거 같단 생각이 드네요

저는 생각했을 때, 이미 onClickMember 자체는 추상화된거잖아요?
근데 그 특정 추상화 중에 어떤 것을 선택하는지 정도는 오히려 명시해주는 것이
어떤 기능을 사용하고 있는지 읽기 쉽다고 생각이 들었습니다.
특히나 xml은 코드를 왔다갔다 해야하는 번거로움이 있기 때문에 더 그렇게 느껴지네용

흐음음음

inseonyun · 2023-07-31T03:33:48Z

menu 파일에 studyManagement 메뉴 파일 언제 추가 된건지 모르겠는데 네이밍 확인 부탁드립니당 study_list라고 되어있길래

s9hn

졸린데 남기다보니 16개나 되네요... 고생하셨습니다!! 늦어서 미안해용!

s9hn · 2023-07-31T10:39:12Z

...pp/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementActivity.kt

+        binding.ivStudyManagementPreviousButton.setOnClickListener {
+            val page = (binding.vpStudyManagement.currentItem - 1).coerceAtLeast(0)
+            binding.vpStudyManagement.setCurrentItem(page, true)
+            studyManagementViewModel.fetchRoundDetail(page)
+        }
+        binding.ivStudyManagementNextButton.setOnClickListener {
+            val page =
+                (binding.vpStudyManagement.currentItem + 1).coerceAtMost(studyManagementAdapter.itemCount - 1)
+            binding.vpStudyManagement.setCurrentItem(page, true)
+            studyManagementViewModel.fetchRoundDetail(page)
+        }


s9hn · 2023-07-31T16:50:35Z

...d/app/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementState.kt

+        R.string.study_management_app_bar_title_is_study_participant,
+        R.string.study_management_end_button_is_study_participant,
+    )
+}


enum class StudyManagementState(@StringRes val title: Int, @StringRes val buttonText: Int) { MASTER(R.string.study_management_app_bar_title_is_study_master, R.string.study_management_end_button_is_study_master,), MEMBER(R.string.study_management_app_bar_title_is_study_participant, R.string.study_management_end_button_is_study_participant,) ; }

sealed Class와 오브젝트를 사용한 이유가 궁금합니다(진짜모름)

s9hn · 2023-07-31T16:56:04Z

...p/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementViewModel.kt

+                    optionalTodos = studyDetailUiModel.optionalTodos.map {
+                        it.takeUnless { it.todo.todoId == id }
+                            ?: it.copy(it.todo.copy(isDone = isDone))
+                    },


뭐죠 이 혼란의 코드는!?!?!?!!?!?!? ㅎ내꺼군

s9hn · 2023-07-31T17:01:48Z

.../com/created/team201/presentation/studyManagement/adapter/StudyManagementMemberViewHolder.kt

+            if (item.isDone) {
+                binding.root.context.getString(R.string.item_study_management_todo_done)
+            } else {
+                binding.root.context.getString(R.string.item_study_management_todo_undone)
+            }


dataBinding과 viewBinding을 혼용하신 연유가 무엇인지 여쭈어 봐도 되겠습니까를 질문해도 괜찮겠습니까?

s9hn · 2023-07-31T17:01:58Z

...m/created/team201/presentation/studyManagement/adapter/StudyManagementOptionalTodoAdapter.kt

+
+    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): StudyOptionalTodoViewHolder {
+        return when (viewType) {
+            1 -> StudyManagementOptionalTodoAddViewHolder.from(parent, studyManagementClickListener)


블랙 맘바

s9hn · 2023-07-31T17:14:39Z

...p/src/main/java/com/created/team201/presentation/studyManagement/model/StudyMemberUiModel.kt

+    val profileImageUrl: String,
+    val isDone: Boolean,
+) {
+    fun progressPercentage(): Int = if (isDone) 100 else 0


뷰 로직이라고 생각된다면, uiModel에 있어도 될 것 같습니다!
근데 왜 Activity에서 뷰로직을 작성하면, dataBinding의 의미가 퇴색된다고 생각했나요?

뷰 로직이다. 해당 퍼센테이지의 수치가 필수투두 완료 =10000으로 바뀐다면, 뷰모델까지 바뀌는가?

UiModel, BindingAdapter, Activity에 위치해도 될 것 같다.

해당 UiModel이 뷰에 의존하고 있는가? 뷰라고 생각하는가?

해당 UiModel을 혹시 뷰모델에서 생성하고 있는가? 그럼 뷰모델은 뷰를 참조하고 있는가?

s9hn · 2023-07-31T17:15:48Z

android/app/src/main/res/layout/item_study_management.xml

@@ -36,7 +44,7 @@
            android:textAppearance="@style/text_r12"
            android:textColor="@color/white"
            app:drawableStartCompat="@drawable/ic_edit_12"
-            app:isVisible="@{isMaster}"
+            app:isVisible="@{role.Companion.isMaster(studyManagement.role)}"


뷰에 도메인 임포트.. 어떻게 생각하시나요?

android/app/src/main/res/layout/item_study_management_optional_todo_add.xml

s9hn · 2023-07-31T17:17:12Z

android/app/src/main/res/layout/item_study_management_optional_todo_add.xml

+        <TextView
+            android:id="@+id/tv_item_study_management_optional_todo_add_button"
+            android:layout_width="60dp"
+            android:layout_height="40dp"


고정dp 최선인가요!?

s9hn · 2023-07-31T17:20:09Z

...p/src/main/java/com/created/team201/presentation/studyManagement/StudyManagementViewModel.kt

+        // 서버 통신으로 round 정보 가져옴
+    }
+
+    fun updateCurrentPage(pageIndex: Int) {


Round라는 도메인의 책임으로 보긴 힘들까요?

round와 page의 개념이 모두 있는데, 이 부분은 Page의 개념이어서 기존에 만들어두었던 PageIndex 도메인을 사용하도록 코드를 변경하였습니다.

…ub.com/woowacourse-teams/2023-yigongil into AN/feature/120-studymanagement-inflate

…igongil into AN/feature/120-studymanagement-inflate

s9hn

LGTM~

RightHennessy and others added 20 commits July 27, 2023 11:22

feat: StudyManagement 의 intent 생성

942691b

feat: 스터디 관리하기 뷰의 UiModel 작성

dd94c59

feat: 스터디 라운드를 조회하는 기능 구현

9a19b54

feat: 스터디 관리하기 뷰 홀더 구현

58a5511

feat: 스터디 관리하기 뷰페이저 어댑터 구현

220aaa1

refactor: id를 long으로 받도록 변경

7dd275f

feat: todo클릭리스너 생성

2e0e529

feat: 스터디 관리하기 뷰 내의 클릭리스너 구현

1ac0895

feat: view pager 어댑터 연결

2efaa29

feat: 스터디 멤버 어댑터와 뷰홀더 구현

9e55c26

feat: 스터디 멤버 어댑터 연결

a5a64fb

feat: 리사이클러뷰 최적화

e7dca89

feat: 선택 투두 뷰홀더와 어댑터 생성

24d62f8

feat: 선택 투두 완료 기능 구현

5aa1d63

fix: 선택 투두 참조 오류 수정

8b18bfa

style: 코드 정리

1dae7ea

fix: 기존의 이름이 중복되는 title 커스텀 attribute 수정

f8278da

feat: 스터디장과 스터디 멤버에 따라 달라지는 뷰 분기처리 구현

ecc6817

feat: xml에 투두 클릭리스너 연결

0c9802e

feat: 버튼 클릭으로 뷰페이저 이동 구현

e1414ac

RightHennessy added android💚 안드 feat 기능 추가 ringring🤙 링링 ßändäl🌗 반달 labels Jul 30, 2023

RightHennessy requested review from s9hn and inseonyun July 30, 2023 13:22

RightHennessy assigned no1msh and RightHennessy Jul 30, 2023

RightHennessy changed the title ~~An/feature/120 studymanagement inflate~~ [스터디 관리하기 뷰 시작 후] 스터디 관리하기 뷰 기능 구현 Jul 30, 2023

rename: 파일 이름 변경

b07d953

inseonyun approved these changes Jul 31, 2023

View reviewed changes

RightHennessy and others added 5 commits July 31, 2023 17:24

refactor: PageIndex 계산 로직 분리

e84eee7

refactor: OptionalTodoViewType으로 로직 개선

fdbfbf6

refactor: 불필요한 bind 함수 제거

c16418e

Merge branch 'develop' into AN/feature/120-studymanagement-inflate

6d7d055

feat: 선택투두 4개까지한 허용하도록 기능 추가

ebf2c6e

s9hn requested changes Jul 31, 2023

View reviewed changes

RightHennessy added 7 commits August 1, 2023 14:06

Merge branch 'AN/feature/120-studymanagement-inflate' of https://gith…

14e8f80

…ub.com/woowacourse-teams/2023-yigongil into AN/feature/120-studymanagement-inflate

fix: conflict 해결

164dad2

Merge branch 'develop' of https://github.com/woowacourse-teams/2023-y…

d0346f2

…igongil into AN/feature/120-studymanagement-inflate

refactor: viewHolder 초기화 시 clickListener 등록

ead8181

refactor: 뷰페이저의 index를 PageIndex로 관리

9a04724

refactor: isMaster판단 로직을 StudyRoundDetailUiModel에서 처리

dff9731

feat: 선택 투두 추가 글자 수 제한 및 디자인 개선

3275a4d

RightHennessy requested a review from s9hn August 1, 2023 08:23

RightHennessy added 2 commits August 1, 2023 17:28

fix: 키보드가 화면을 가리는 현상 수정

a994e81

style: 코드 가독성 개선

c9a9439

s9hn approved these changes Aug 2, 2023

View reviewed changes

RightHennessy merged commit f695277 into develop Aug 2, 2023

RightHennessy deleted the AN/feature/120-studymanagement-inflate branch August 2, 2023 02:02

		val studyDetails = studyRounds.value ?: throw IllegalStateException()
		val currentStudy = studyRounds.value?.get(currentItemId) ?: throw IllegalStateException()

		override fun bind(optionalTodoUiModel: OptionalTodoUiModel) {
		}

[스터디 관리하기 뷰 시작 후] 스터디 관리하기 뷰 기능 구현 #140

[스터디 관리하기 뷰 시작 후] 스터디 관리하기 뷰 기능 구현 #140

Conversation

RightHennessy commented Jul 30, 2023

😋 작업한 내용

🙏 PR Point

👍 관련 이슈

github-actions bot commented Jul 30, 2023 • edited Loading

Test Results

inseonyun left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inseonyun commented Jul 31, 2023

s9hn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

s9hn left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 30, 2023 •

edited

Loading

inseonyun left a comment •

edited

Loading