[refactor] 리스트뷰 <-> 지도뷰 ViewPager로 리팩토링 #187

jihyunniiii · 2024-02-16T15:59:15Z

Related issue 🛠

closed [refactor] 리스트뷰 <-> 지도뷰 ViewPager로 리팩토링 #186

Work Description ✏️

리스트뷰 <-> 지도뷰를 ViewPager를 사용하여 리팩토링 하였습니다.

Screenshot 📸

Screen_recording_20240218_010040.mp4

Uncompleted Tasks 😅

N/A

To Reviewers 📢

리스트 뷰와 지도 뷰에 동일하게 사용되는 로직(핑글 신청, 삭제, 취소 등 서버 통신 로직 등) 및 겹치는 뷰(상단 로고 및 검색 버튼, 칩, FAB)가 많고, 리스트뷰 와 지도뷰에 공통적으로 저장되어야 하는 정보(칩 선택 여부, 검색어)가 많아 ViewPager를 이용하여 리팩토링을 진행하였습니다. (전체적으로 핑글 개최 프로세스와 비슷한 플로우라고 생각하면 이해가 편할 것 같습니다.)
두 FAB 모두 HomeFragment에 두고 관리하려 하였으나, 현위치 FAB는 지도를 활용한 로직이라 전환 FAB는 HomeFragment에 현위치 FAB는 MapFragment에 두었습니다. (현위치 FAB를 HomeFragment에 두면 해당 FAB를 클릭할 때 카메라 이동을 위해 MapFragment에 있는 지도 객체에 접근해주어야 하는데, 지도 객체는 뷰와 관련된 정보라 뷰모델로 옮기는 건 적절하지 않은 것 같아보이고,, 고차함수로 넘기자니 두 Fragment 간의 결합도를 높이는 것 같아 현위치 FAB를 MapFragment로 옮기는 것이 가장 적합하다고 생각하였습니다. 다른 좋은 방법이 있다면 알려주세여 ㅠ)
HomeFragment로 꼭 옮겨야 하는 로직들만 옮겨두었습니다. 리스트뷰 + 검색 로직까지 모두 작성한 후 로직 분리 더 정확히 진행하도록 하겠습니다. (고민이 더 필요합니다요 ㅋ.ㅋ)

Dan2dani

저도 현재 FAB 방식이 옳다고 생각합니다 ~ 현위치의 책임은 온전히 Map에만 있는거니까요!
잘 구현한거 같습니당~

Dan2dani · 2024-02-17T06:27:57Z

app/src/main/java/org/sopt/pingle/presentation/ui/main/home/HomeFragment.kt

+                vpHome.setCurrentItem(
+                    when (vpHome.currentItem) {
+                        MAP_INDEX -> MAIN_LIST_INDEX
+                        else -> MAP_INDEX


개취이긴 하지만! 전 뭔가 else보다 명확하게 명시해주는게 더 가독성 측면에서 좋은거 같기두 하네여 ~~

else를 무조건 넣으라 해서,,, else로 했습니다요 ㅠㅠ

아항 그럼 else문을 따로 둘 수도 이ㅛ지 않을까염

그러명

MAP_INDEX -> MAIN_LIST_INDEX MAIN_LIST_INDEX -> MAP_INDEX else -> MAP_INDEX

이런 식으로 되어야 할 것 같은데 이게 더 나을까욤? 두 개의 반환 값이 같아서 하나로 쓰는 게 더 낫지 않을까 생각했거든요

아 MAP_INDEX가 아닌 경우도 고려해서 else문에 MAP_INDEX로 가는건가여,,?? 이거 뷰페이저 index 두 개 아닌가염?

이게 fragmentStateAdapter에 리스트를 2개 넣어주긴 하는데 vpHome.currentItem 값이 무조건 2개는 아니라 그런 듯요???? 몇 개 넣어주냐에 따라 다르니까,,?

[다은] 지정된 인덱스가 아닐 때는 아무 일도 안 일어나도록 고쳐줘욤 ~

HAJIEUN02

머싯다 고생핑🍕🩵🍒

HAJIEUN02 · 2024-02-17T12:23:00Z

app/src/main/java/org/sopt/pingle/presentation/ui/main/home/HomeFragment.kt

+            cgHomeCategory.setOnCheckedStateChangeListener { group, checkedIds ->
+                homeViewModel.setCategory(
+                    category = checkedIds.getOrNull(SINGLE_SELECTION)
+                        ?.let { group.findViewById<PingleChip>(it).categoryType }


여기는 꼭 findViewById를 사용해야만 하나욤? 궁금핑..

cgHomeCategory 그룹에서 id가 checkedIds인 값을 받아와야 하기 때문에 findViewById 사용하였숩니다.
찾아보니까 칩그룹에서 선택된 값을 가져오려면 무조건 저렇게 해야하는 것 같더라구요?
다른 방법을 아직 못 찾았어요 ㅜ

HAJIEUN02 · 2024-02-17T12:24:33Z

app/src/main/java/org/sopt/pingle/presentation/ui/main/home/HomeFragment.kt

+
+    private fun collectData() {
+        homeViewModel.category.flowWithLifecycle(viewLifecycleOwner.lifecycle)
+            .distinctUntilChanged()


우리 그때 피드백 받을 때 stateflow에는 사실상 distinctUntilChanged()가 포함되어 있으니까 써줄 필요가 없다고 했자나요.. 이걸 빼면 제대로 작동이 안되나요 여기도?!

넹 여긴 없으면 안 되네요 ㅜㅜ
distinctUntilChanged 이거 관련 리팩은 로직 다 짜구 하려구욤 ㅠㅠ

어떻게 그럴까,,, stateFlow는 내부적으로 저게 처리가 돼있는데,, 원인을 찾아봐야겠네요 구독 시점의 여부일 수 있어요

HAJIEUN02 · 2024-02-17T12:26:00Z

app/src/main/java/org/sopt/pingle/presentation/ui/main/home/HomeViewModel.kt

@@ -55,17 +55,17 @@ class MapViewModel @Inject constructor(
    private val _pingleDeleteState = MutableSharedFlow<UiState<Unit?>>()
    val pingleDeleteState get() = _pingleDeleteState.asSharedFlow()

+    fun setCategory(category: CategoryType?) {


별거 아닌 질문이긴 한데욥ㅋ.ㅋ 이 코드를 굳이 상단으로 올린 이유는 먼가요?
근데 진짜 궁금핑....

원래 변수 순서에 맞춰서 함수도 배열하는 편인데요,, 얘가 혼자 아래 가있더라구요? 그래서 위로 올렸어용

HAJIEUN02 · 2024-02-17T12:26:32Z

app/src/main/java/org/sopt/pingle/presentation/ui/main/home/mainlist/MainListFragment.kt

얘가 이제 머지 후에 작업해야 하는 파일인가부네용

맞슴다,,

HAJIEUN02 · 2024-02-17T12:28:06Z

app/src/main/java/org/sopt/pingle/presentation/ui/main/home/map/MapFragment.kt

-            .distinctUntilChanged()
-            .onEach {
-                mapViewModel.getPinListWithoutFilter()
+        homeViewModel.markerModelData.flowWithLifecycle(viewLifecycleOwner.lifecycle)


여기서는 또 distinctUntilChanged()를 지우셨네용 그러면 위에 남긴 부분도 지워도 잘 작동하는 것이 아닌지!!

여기는 지워도 되는뎅 저기는 지우면 안 돼요 ㅜ

HAJIEUN02 · 2024-02-17T12:28:23Z

app/src/main/res/layout/fragment_home.xml

+            app:chipSpacingHorizontal="@dimen/chip_spacing"
+            app:layout_constraintEnd_toEndOf="parent"
+            app:layout_constraintStart_toStartOf="parent"
+            app:layout_constraintTop_toBottomOf="@+id/iv_home_search"


@+id ㅋㅋ

DoReMinWoo

고생하셨습니다~!

DoReMinWoo · 2024-02-20T10:46:19Z

app/src/main/java/org/sopt/pingle/presentation/ui/main/home/HomeFragment.kt

+                    with(binding) {
+                        fabHomeChange.visibility =
+                            if (isMarkerUnselected) View.VISIBLE else View.INVISIBLE
+                    }


binding이라는 변수가 한번 사용되는거 같은데 스코프함수를 사용해야하는 건가요??

아 이게 원래 2개였는데 하나로 변경돼서 ㅜㅜ 반영할게요 ~

DoReMinWoo · 2024-02-20T10:51:15Z

app/src/main/res/layout/fragment_home.xml

+            app:layout_constraintBottom_toBottomOf="parent"
+            app:layout_constraintEnd_toEndOf="parent"
+            app:layout_constraintHorizontal_bias="1.0"
+            app:layout_constraintStart_toStartOf="parent"
+            app:layout_constraintTop_toTopOf="parent"
+            app:layout_constraintVertical_bias="0.0" />


제약을 4개 다 주시고 0dp를 주셨는데
layout_constraintHorizontal_bias
layout_constraintVertical_bias
이 두개의 속성이 필요한건가요?!

아놔,, 이게 왜 있지,,, 당장 빼겠습니다

# Conflicts: # app/src/main/java/org/sopt/pingle/presentation/ui/main/home/map/MapFragment.kt

jihyunniiii added 12 commits February 16, 2024 09:09

[feat] #186 HomeFragment 생성

5bb22dc

[chore] #186 바텀 네비에 HomeFragment 연결

4f12e32

[chore] #186 fragmentStateAdapter Util로 이동

0df7a93

[feat] #186 HomeFragment UI 구현

afd1511

[chore] #186 지도 및 리스트뷰 UI 수정 (HomeFragment와 공통된 부분 제거)

345f9c0

[feat] #186 HomeFragment 뷰 전환 로직 구현

6b4e4b7

[chore] #186 MainListFragment clickListener 제거

815f898

[chore] #186 네이밍 수정 및 fab visibility 조절

6d12b27

[chore] #186 현위치 버튼 MapFragment 이동

b4b71db

[feat] #186 Map, Home 로직 분리

2640a1a

[chore] #186 ktlintFormat

c12b571

Merge branch 'develop' into refactor-list-map

845e8d0

jihyunniiii added 🍥 refactor 🍥 🐸 지현 🐸 labels Feb 16, 2024

jihyunniiii added this to the 1차, 2차 스프린트 리팩토링 milestone Feb 16, 2024

jihyunniiii self-assigned this Feb 16, 2024

jihyunniiii requested a review from a team as a code owner February 16, 2024 15:59

Dan2dani approved these changes Feb 17, 2024

View reviewed changes

HAJIEUN02 approved these changes Feb 17, 2024

View reviewed changes

jihyunniiii added 3 commits February 17, 2024 22:04

[chore] #186 @+id -> @id

80cfc2f

[feat] #186 프래그먼트 재생성 시 선택된 칩 유지

8297f48

[feat] #186 HomeFragment 상단에 접속 중인 단체명 뜨도록 구현

c1b93e8

DoReMinWoo approved these changes Feb 20, 2024

View reviewed changes

jihyunniiii added 4 commits February 20, 2024 20:04

[chore] #186 불필요한 scope 함수 제거

df84411

[chore] #186 불필요한 제약 제거

1ecaa4d

[chore] #186 vpHome 전환 시 지정된 인덱스가 아닐 경우 아무런 변화 없도록 수정

4862e92

Merge branch 'develop' into refactor-list-map

fba57ec

# Conflicts: # app/src/main/java/org/sopt/pingle/presentation/ui/main/home/map/MapFragment.kt

jihyunniiii merged commit 26ef6a3 into develop Feb 21, 2024
1 check passed

jihyunniiii deleted the refactor-list-map branch February 21, 2024 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[refactor] 리스트뷰 <-> 지도뷰 ViewPager로 리팩토링 #187

[refactor] 리스트뷰 <-> 지도뷰 ViewPager로 리팩토링 #187

jihyunniiii commented Feb 16, 2024 •

edited

Loading

Dan2dani left a comment

Dan2dani Feb 17, 2024

jihyunniiii Feb 17, 2024

Dan2dani Feb 17, 2024

jihyunniiii Feb 17, 2024

Dan2dani Feb 20, 2024

jihyunniiii Feb 20, 2024

jihyunniiii Feb 20, 2024

HAJIEUN02 left a comment

HAJIEUN02 Feb 17, 2024

jihyunniiii Feb 17, 2024

HAJIEUN02 Feb 17, 2024

jihyunniiii Feb 17, 2024

Dan2dani Feb 20, 2024

HAJIEUN02 Feb 17, 2024

jihyunniiii Feb 17, 2024

HAJIEUN02 Feb 17, 2024

jihyunniiii Feb 17, 2024

HAJIEUN02 Feb 17, 2024

jihyunniiii Feb 17, 2024

HAJIEUN02 Feb 17, 2024

jihyunniiii Feb 17, 2024

DoReMinWoo left a comment

DoReMinWoo Feb 20, 2024

jihyunniiii Feb 20, 2024

DoReMinWoo Feb 20, 2024

jihyunniiii Feb 20, 2024

[refactor] 리스트뷰 <-> 지도뷰 ViewPager로 리팩토링 #187

[refactor] 리스트뷰 <-> 지도뷰 ViewPager로 리팩토링 #187

Conversation

jihyunniiii commented Feb 16, 2024 • edited Loading

Related issue 🛠

Work Description ✏️

Screenshot 📸

Uncompleted Tasks 😅

To Reviewers 📢

Dan2dani 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

HAJIEUN02 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DoReMinWoo 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

jihyunniiii commented Feb 16, 2024 •

edited

Loading