[ISSUE-56] 약속 관리 UI 구현 #80

jihee-dev · 2022-07-04T15:01:10Z

ISSUE

resolved [Sprint #5] 약속잡기 - 약속관리(P_00) 화면 구현 #56

작업 내용

화살표 아이콘 추가
- 기존의 캘린더에서 사용하던 화살표 아이콘의 네이밍을 ic_arrow_box_{방향}으로 변경
  -> @hoyahozz 이 의견에 대해 어떻게 생각하시는지 궁금합니당 그냥 화살표 모양의 아이콘이 있어서 이름 짓기가 조금 어려워서요..!
- 새로 추가된 아이콘의 이름을 ic_arrow_{방향}으로 설정
Pager 추가
약속 관리 UI 구현
- Tab 및 Pager 추가
Plan 도메인 모델 추가
- WaitingPlan / FixedPlan 으로 가져가되, Plan을 상속받아 공통 부분을 가져갈 수 있도록 구현했는데 요 부분 같이 봐주셨으면 좋겠습니다
  -> 이렇게 구현한 이유는 UI상에서 동일한 속성만 사용하여 그려주는 곳이 있기도 하고 연관된 부분이라서 이렇게 구현해 봤어요
- 더 좋은 방법이 있다면 알려주세요 ㅜㅡㅜ
약속 관리 화면 네비게이션 연결
- 다른 화면으로 이동하는 부분은 주석으로 TODO 처리해 두었습니당 추후 연결만 하면 될 것 같아용

실행 화면

Screen Shot	Screen Shot

Check List

PR 제목은 [ISSUE-{N}] PR 제목으로 작성
CI/CD 통과 여부
1명 이상의 Approve 확인 후 Merge
관련 이슈 연결

KwonDae

고생하셨습니다 👍
코멘트 드린 것만 설명해 주시면 좋을 것 같아요! 💯

KwonDae · 2022-07-04T16:22:44Z

presentation/src/main/java/com/yapp/growth/presentation/ui/main/manage/ManageScreen.kt

+ Column(
+ modifier = Modifier.padding(horizontal = 20.dp), /* TODO: 스크롤 범위 수정 */
+ verticalArrangement = Arrangement.spacedBy(12.dp)
+ ) {


요기 LazyColumn 대신 Column 으로 하신 이유가 있을까요?

위에서 언급드린 부분에서 이미 LazyColumn으로 구성하였고 그 안에 아이템으로 호출되는 아이들입니다
 중첩 LazyColumn 구성은 불가능하며, 이미 LazyColumn 스코프 안에 있는 컴포저블 함수들입니당

KwonDae · 2022-07-04T16:22:57Z

presentation/src/main/java/com/yapp/growth/presentation/ui/main/manage/ManageScreen.kt

+ waitingPlans: List<Plan.WaitingPlan>,
+ fixedPlans: List<Plan.FixedPlan>,


여기 네이밍을 List 를 붙이는게 어떨까요? 조금 더 명시적일거 같아요!

아 이거 항상 고민인 부분인데
제가 책을 읽으면서 네이밍 관련해서 읽은 내용 중에,
변수명에 해당 타입이 무엇인지 명시하는 것이 좋지 못하다는 내용을 봤습니다
거기서 말하는 것 중에 하나가 OOOString, OOOList가 있었어요
굳이 타입을 명시해 주지 않아도 요즘 IDE는 해당 타입이 무엇인지 알려주고 있으며,
해당 변수가 나중에 어떤 타입으로 변경될지 모른다,
그렇게 타입이 변경되었을 때 변수명이 같이 변경되는 것이 잊혀지고 결국 혼란을 낳을 수 있다
이런 내용을 읽은 적이 있습니당..
그리고 생각보다 코드를 읽는 사람들이 앞의 OOO에 집중하지, String이니 List니까지는 잘 안 읽게 된다라고 하더라고요..
그래서 복수형으로 명시해 주라고 했던 것 같아요
그래서 저도 가능하면 리스트라는 네이밍을 사용하지 않고 복수형으로 작성해 주려고 노력 중이긴 해용,,
그런데 저런 리스트에 OOOList라고 명시해 주는 경우가 많더라고요(약간 관례적인 것 같기도 하고..?)
위에서 말씀드린 책 내용에 공감하면서도, List라고 명시해 주는 것이 더 명확해 보이기도 하고,
특히 한국어 문법에는 복수형이 없기 때문에 한국인들이 뒤의 s를 잘 무시하는 경향이 있다?
요런 내용을 토익 준비하면서 들었던 게 생각이 나기도 하네여..
대원 님은 어떻게 생각하시나용? 저도 이거 쓸 때마다 항상 고민입니당 ㅠㅡㅠ

아 사실 맞습니다..
회사 서비스에서도 List 대신 menuContents, menuCategories 이런식으로 가져가는데 제가 이부분은 습관적..관례?로 생각이 짧았네요

사실 개발하면서 변수명 제일 뒤에 list 붙이는게 흔하게 보이는 케이스인데 이부분은 우리가 채택해서 가져가도 좋을 것 같습니다

오 회사 서비스에서도 List 대신 복수형으로 작성하는군요..!
항상 궁금했어요 ㅠㅠ
딱 저거 쓸 때도 List로 썼다 s로 썼다 두세 번은 바꿨던 것 같아요 ㅋㅋㅋ
이게 맞나 싶기도 하고 s가 너무 쪼끄매서 안 보이는 것 같기도 하고..
과연 List가 다른 타입으로 바뀔 일이 있을까 싶기도 하고.. 🥲

그러면 요 친구는 지금처럼 List를 붙이지 않는 방식으로 채택해도 괜찮을까용?

네 복수형을 붙이는 방식으로 채택해도 좋을 것 같아요.
정호님이랑 같이 얘기해보고 방향 한번더 정하시죠!
@hoyahozz

저도 이번에 작업하면서 계속 고민하다가, HomeMonthlyPlanList로 할까 HomeMonthlyPlans로 할까 너무 고민했거든요 ㅠ

원래 우리가 네이밍을 정할 때 참고하기로 했던 레파지토리에서도 s 를 강조하고 있기는 한데, 제 화면상에서는 HomeMonthlyPlan -> HomeMonthPlanList 형식으로 이어지는지라 HomeMonthlyPlans 으로 네이밍을 하면 오히려 가독성을 해치는 것 같아서 우선 List를 붙이는 식으로 했거든요.. 나중에 저도 이야기 나눠보자고 생각했는데 깜빡하고 있었네요..ㅠ

아무튼 결론은 모두 s로 두는 것에 동의하신다면 저도 동의합니다요 :)

화면에 대한 거는 리스트로 가져가되,
변수명에 타입을 명시하지 않고 복수형으로 짓는 것으로 결정!

KwonDae · 2022-07-04T16:27:18Z

presentation/src/main/java/com/yapp/growth/presentation/ui/main/manage/ManageScreen.kt

+ plans.forEach { plan ->
+ ManagePlansItem(plan = plan, type = type, onItemClick = onItemClick)


Suggested change

plans.forEach { plan ->

ManagePlansItem(plan = plan, type = type, onItemClick = onItemClick)

items(plans.size) { index ->

ManagePlansItem(plan = plansList[index], type = type, onItemClick = onItemClick)

or

itemsIndexed(plansList) { index, _ ->

ManagePlansItem(plan = plansList[index], type = type, onItemClick = onItemClick)

LazyColumn 사용 가능해 보이느데 혹시 Column 을 사용하신 이유가 있다면 알려주시면 감사하겠습니다 ㅎㅎ

위의 코멘트와 동일해요! 
인덱스를 굳이 사용하지 않아도 되기 때문에 forEachIndex보다는 forEach가 적절하다고 판단하였고, 이미 LazyColumn 스코프 안에서 호출되는 컴포저블 함수입니당

음 요 부분이 조금 혼란을 줄 수 있는 부분인 것 같은데,, 고러면 아예 저 친구를 아이템째로(item{ // }) 빼서 LazyColoumScope로 명시해 주는 게 좋을지 고민이네요..!

HorizontalPager 의 contents 부분을 Column 로 구성하고 Spacer 준 다음

when (index) { ManageTapMenu.WAITING_PLAN.ordinal -> { ManagePlansList( plans = waitingPlans, type = ManageTapMenu.WAITING_PLAN, onItemClick = onWaitingItemClick ) } ManageTapMenu.FIXED_PLAN.ordinal -> { ManagePlansList( plans = fixedPlans, type = ManageTapMenu.FIXED_PLAN, onItemClick = onFixedItemClick ) } }

이게 들어가고, ManagePlansList 에서 LazyColumn 으로 구성하는건 어떤가요?

개별 레이지 컬럼으로 구성될 수 있도록 수정해 보겠습니당!

jihee-dev · 2022-07-04T17:25:01Z

presentation/src/main/java/com/yapp/growth/presentation/ui/main/manage/ManageScreen.kt

+ HorizontalPager(
+ state = pagerState,
+ modifier = Modifier.fillMaxSize()
+ ) { index ->
+ LazyColumn(
+ modifier = Modifier.fillMaxSize(),
+ verticalArrangement = Arrangement.spacedBy(12.dp)
+ ) {
+ item {
+ Spacer(modifier = Modifier.height(24.dp))
+ }
+
+ item {
+ when (index) {
+ ManageTapMenu.WAITING_PLAN.ordinal -> {
+ ManagePlansList(
+ plans = waitingPlans,
+ type = ManageTapMenu.WAITING_PLAN,
+ onItemClick = onWaitingItemClick
+ )
+ }
+ ManageTapMenu.FIXED_PLAN.ordinal -> {
+ ManagePlansList(
+ plans = fixedPlans,
+ type = ManageTapMenu.FIXED_PLAN,
+ onItemClick = onFixedItemClick
+ )
+ }


@KwonDae 여기입니당!

KwonDae

LGTM 👍
고생하셨습니다

hoyahozz

고생 많으셨어요!!!

hoyahozz · 2022-07-06T06:40:46Z

domain/src/main/java/com/yapp/growth/domain/entity/Plan.kt

+sealed class Plan(
+ open val id: Int,
+ open val title: String,
+ open val isLeader: Boolean,
+ open val category: String, // type?
+ open val members: List<String>,


이렇게 도메인 모델 구축한거 되게 좋은 것 같아요..!
저였으면 대기중 약속, 확정 약속 다 따로 만들었을듯....

…anage-plan # Conflicts: # presentation/build.gradle.kts # presentation/src/main/res/drawable/ic_arrow_box_left.xml # presentation/src/main/res/drawable/ic_arrow_box_right.xml # presentation/src/main/res/drawable/ic_arrow_right.xml

jihee-dev added 5 commits July 2, 2022 02:11

fix - 패딩 속성이 잘못 적용된 부분 수정

dc5d37b

design - 아이콘 추가 및 변경

85f6ed2

chore - accompanist pager 의존성 추가

5a4417b

feat - 약속 관리 UI 구현

201d52e

feat - 네비게이션 추가

da91c87

jihee-dev self-assigned this Jul 4, 2022

jihee-dev requested review from hoyahozz and KwonDae July 4, 2022 15:01

KwonDae requested changes Jul 4, 2022

View reviewed changes

jihee-dev commented Jul 4, 2022

View reviewed changes

refactor - 리스트를 LazyColumn으로 구성

77e2ad0

KwonDae approved these changes Jul 5, 2022

View reviewed changes

hoyahozz approved these changes Jul 6, 2022

View reviewed changes

jihee-dev merged commit 826c235 into develop Jul 6, 2022

jihee-dev deleted the feature/issue-56-manage-plan branch July 6, 2022 12:34

KwonDae added this to the Sprint #5 milestone Jul 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISSUE-56] 약속 관리 UI 구현 #80

[ISSUE-56] 약속 관리 UI 구현 #80

jihee-dev commented Jul 4, 2022 •

edited by hoyahozz

Loading

KwonDae left a comment

KwonDae Jul 4, 2022

jihee-dev Jul 4, 2022 •

edited

Loading

KwonDae Jul 4, 2022

jihee-dev Jul 4, 2022

KwonDae Jul 4, 2022

jihee-dev Jul 4, 2022

KwonDae Jul 4, 2022

jihee-dev Jul 4, 2022

hoyahozz Jul 5, 2022 •

edited

Loading

hoyahozz Jul 5, 2022

jihee-dev Jul 5, 2022

KwonDae Jul 4, 2022

jihee-dev Jul 4, 2022 •

edited

Loading

jihee-dev Jul 4, 2022

KwonDae Jul 4, 2022 •

edited

Loading

jihee-dev Jul 5, 2022

jihee-dev Jul 4, 2022

KwonDae left a comment

hoyahozz left a comment

hoyahozz Jul 6, 2022

		waitingPlans: List<Plan.WaitingPlan>,
		fixedPlans: List<Plan.FixedPlan>,

		plans.forEach { plan ->
		ManagePlansItem(plan = plan, type = type, onItemClick = onItemClick)

- plans.forEach { plan ->
- ManagePlansItem(plan = plan, type = type, onItemClick = onItemClick)
+ items(plans.size) { index ->
+ ManagePlansItem(plan = plansList[index], type = type, onItemClick = onItemClick)
+ or
+ itemsIndexed(plansList) { index, _ ->
+ ManagePlansItem(plan = plansList[index], type = type, onItemClick = onItemClick)

[ISSUE-56] 약속 관리 UI 구현 #80

[ISSUE-56] 약속 관리 UI 구현 #80

Conversation

jihee-dev commented Jul 4, 2022 • edited by hoyahozz Loading

ISSUE

작업 내용

실행 화면

Check List

KwonDae left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihee-dev Jul 4, 2022 • 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

hoyahozz Jul 5, 2022 • 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

jihee-dev Jul 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KwonDae Jul 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KwonDae left a comment

Choose a reason for hiding this comment

hoyahozz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihee-dev commented Jul 4, 2022 •

edited by hoyahozz

Loading

jihee-dev Jul 4, 2022 •

edited

Loading

hoyahozz Jul 5, 2022 •

edited

Loading

jihee-dev Jul 4, 2022 •

edited

Loading

KwonDae Jul 4, 2022 •

edited

Loading