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] 리스트뷰 구현 #196

Merged
merged 19 commits into from
Feb 25, 2024
Merged

[feat] 리스트뷰 구현 #196

merged 19 commits into from
Feb 25, 2024

Conversation

jihyunniiii
Copy link
Collaborator

Related issue 🛠

Work Description ✏️

  • 리스트뷰를 구현하였습니다. (검색 뷰 관련 논의 사항이 아직 마무리 되지 않아 리스트뷰에 있는 검색 관련 분기처리는 따로 PR 올리겠습니다.)

Screenshot 📸

Screen_recording_20240223_005552.mp4

Uncompleted Tasks 😅

  • N/A

To Reviewers 📢

  • 더미 넣어서 카드 분기처리 모두 확인하였습니다. (리스트에 보이는 순서대로 모집 중 + 참여 신청 X, 모집 중 + 참여 신청 O, 모집 완료 + 참여 신청 X, 모집 완료 + 참여 신청 O, 모집 완료 + MY, 모집 중 + MY)
  • PingleCardTop 에서 모집 완료 배경 색상이 다른 관계로 PingleCardType 만들어 분기처리 진행하였습니다.
  • mainListAdapter과 MapAdapter에 매개변수로 들어가는 함수(핑글 참여, 참여 취소, 삭제, 웹뷰 연결 로직)가 동일한데, 각각의 Fragment에서 따로 작성하는 것이 비효율적이라고 생각하여 카드 관련 로직을 담은 PingleCardUtils를 만들었습니다.
  • 확장함수 정리 하였습니다. (폴더링 및 object화)

Copy link
Collaborator

@HAJIEUN02 HAJIEUN02 left a comment

Choose a reason for hiding this comment

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

이거 머지하면.. 나 바꿀게 쫌 있겠는데.. ㅜㅜ 고생핑!!!!!!💗

import androidx.annotation.StringRes
import org.sopt.pingle.R

enum class MainListOrderType(
Copy link
Collaborator

Choose a reason for hiding this comment

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

type 장인이 되셨군요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

크크

PingleEntity(
id = 2L,
category = "MULTI",
name = "모각공하고 곱창먹자",
Copy link
Collaborator

Choose a reason for hiding this comment

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

헤헷 내 핑글이답ㅋ.ㅋ

override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): MainListViewHolder =
MainListViewHolder(
binding = ItemMainListPingleCardBinding.inflate(
LayoutInflater.from(parent.context),
Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 요기 context를 Adpater 인자에 추가한 뒤 받아와서 쓰는데 이렇게 작성하는 코드가 더 간결하고 괜찮은 것 같네욥!

) : ListAdapter<PingleEntity, MainListViewHolder>(
ItemDiffCallback<PingleEntity>(
onContentsTheSame = { old, new -> old == new },
onItemsTheSame = { old, new -> old.id == new.id }
Copy link
Collaborator

Choose a reason for hiding this comment

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

그냥 old == new 하면 작동 안 되나요?? 왜 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.

오호랏,, ItemDiffCallback 공부 안 하셨나욤?? 이거 숙제 추가요 ~ ㅋ.ㅋ

Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋ ㄷㄷ

)
}
},
navigateToWebViewWithChatLink = { chatLink -> startActivity(navigateToWebView(chatLink)) },
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 Author

Choose a reason for hiding this comment

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

크크 원래는 프래그먼트에 각각을 다 구현하고 넘겨주었는데 리스트뷰랑 지도뷰에 동일한 로직이 쓰이기 때문에 object를 이용해 Util로 만들어두고 받아와서 사용하는 방법으로 변경했슴다. 키득


tvMainListOrderMenuNew.setOnClickListener {
homeViewModel.setMainListOrderType(
MainListOrderType.NEW
Copy link
Collaborator

Choose a reason for hiding this comment

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

근데 type 이름을 MainListOrderType으로 하신 이유가 있나요?? new랑 upcoming이면 MainListStateType 머 이런 다른 직관적인 네이밍이 있지 않을까 싶어서요.. 지금은 딱 뭔지 눈에 안 들어오는 것 같아용

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

서버에 해당 값(정렬 방식)을 order로 넘겨야 하기 때문에 해당 네이밍을 사용하였습니다! 다르게 가져가면 오히려 더 헷갈릴 것 같아서요


fun onBind(pingleEntity: PingleEntity) {
with(binding.pingleCardTopMainListPingleCard) {
initLayout(pingleEntity = pingleEntity, pingleCardType = PingleCardType.MAINLIST)
Copy link
Collaborator

Choose a reason for hiding this comment

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

엥 여기서 initLayout 어케 쓸 수 있죠 인자로 넘겨준 것도 아닌데?? 대혼란•• 얼른 머지해주세요 컨트롤 엔터 절실티비 ㅋㅋ

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pingleCardTopMainListPingleCard 커스텀 뷰 안에 initLayout을 정의해 두었기 때문에 사용할 수 있어요! 카드가 초기화 될 때 필요한 로직을 작성해두었습니다.

@@ -110,11 +105,34 @@ class MapFragment : BindingFragment<FragmentMapBinding>(R.layout.fragment_map),

private fun initLayout() {
mapCardAdapter = MapCardAdapter(
navigateToParticipant = ::navigateToParticipant,
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 Author

Choose a reason for hiding this comment

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

동일한 동작을 하는 함수들이 두 프래그먼트에서 사용되는데 두 프래그먼트에 해당 함수를 각각 작성하는 것보다 util로 관리하는게 추후 유지보수 측면에서 좋을 것 같다고 생각해서 변경하였습니다.
근데 Adapter에 함수를 넣는 부분이 더 길어져서 어떻게 하면 더 간결하게 적을지 고민이 필요할 것 같아요,,,
Util로 빼는 방법 말고 다른 좋은 방법이 있을까요?

Copy link
Member

@DoReMinWoo DoReMinWoo 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 +7 to +14
inline fun <reified T> List<Flow<T>>.combineAll(): Flow<List<T>> {
return when (size) {
0 -> flowOf(emptyList())
else -> combineTransform(this) { flows ->
emit(flows.toList())
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

오오오옹 inline 미쳤네요

Copy link
Member

Choose a reason for hiding this comment

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

diffutil 파일로 따로 뺀건가요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

원래 ViewExt에 있었는데 ViewExt에는 정말로 뷰에 관련된 확장 함수만 작성하는 게 좋을 것 같아 따로 분리했습니다.

Copy link
Collaborator

@Dan2dani Dan2dani left a comment

Choose a reason for hiding this comment

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

LGTM

collectData()
}

override fun onDestroyView() {
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 Author

Choose a reason for hiding this comment

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

오,,, 저는 원래 오버라이드 함수를 젤 위에 쓰는데,,, 요거 담주에 만나서 얘기해보면 좋을 것 같네오

@jihyunniiii jihyunniiii merged commit c032ee5 into develop Feb 25, 2024
1 check passed
@jihyunniiii jihyunniiii deleted the feat-list-view branch February 25, 2024 06:27
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] 리스트뷰 구현
4 participants