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

[#127] 그룹 홈화면 상태에 따른 ui 로직 구현 #134

Merged
merged 13 commits into from
Aug 12, 2024

Conversation

SEO-J17
Copy link
Collaborator

@SEO-J17 SEO-J17 commented Aug 1, 2024

Issue No

Overview (Required)

  • 상태에 따라서 버튼,텍스트 나타나는 로직 적용했습니단

Screenshot

KakaoTalk_Video_2024-08-01-17-44-03.mp4

@SEO-J17 SEO-J17 self-assigned this Aug 1, 2024
Copy link
Collaborator

@JeonK1 JeonK1 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 163 to 173

if (groupInfo.status == GroupStatusType.AFTER_MY_VOTE) {
PicNormalButton(
modifier = Modifier
.padding(top = 16.dp)
.align(Alignment.CenterHorizontally),
iconRes = R.drawable.ic_group_notice,
text = stringResource(R.string.group_home_pic_fcm),
onButtonClicked = onClickSendFcm,
)
}
Copy link
Member

Choose a reason for hiding this comment

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

쿡찌르기만 버튼이 있는건가..? 신기하네

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.

이벤트 생성하기 버튼도 있긴한데 그거는 GroupHomePhotoCard에 들어가 있어..! 버튼이 카드 안에 들어가있는 것도 있고 쿡찌르기는 카드 밖에 있어서 그렇게 해놨어..!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이벤트 생성하기 버튼도 GroupHomePhotoCard에 있습니단~

Copy link
Member

Choose a reason for hiding this comment

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

버튼 종류 더 많은거지? @SEO-J17

@@ -26,6 +27,7 @@ fun NavGraphBuilder.groupHomeNavGraph(
onClickMyPage = onClickMyPage,
onClickGroupMake = onClickGroupMake,
onClickGroupEnter = onClickGroupEnter,
onClickSendFcm = onClickPicFcm,
Copy link
Member

Choose a reason for hiding this comment

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

여기 이름이 안맞는데..? ㅋㅋㅋㅋ
나 궁금한거 있는데 아직 HomeViewModel 없는건가? 이거 로직을 메인 액티비티까지 보내야 하나 싶어서! 그리고 FCM이라는 구현을 UI 에서 알필요는 없다고 생각해서 나는 그룹상세에 쿡찌르기 유스케이스 만들때 PokeOtherUseCase 였나 이렇게 만들었던 것 같은데 어떻개생각해/??

Copy link
Collaborator

Choose a reason for hiding this comment

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

지금은 아니지만 fcm이 쿡찌르기 말고도 쓰이니까 fcm보다는 쿡찌르기 버튼이 눌렸다는 네이밍으로 변경하면 좋긴 하겠당.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋ이름이 안맞네..? 나 급했나봐 ㅋㄷㅋㄷ 아아 지혜야 이해해버렸당..! pokeOthherUsecase를 쓰자는 거지? 나는 좋은거 같아..! 다만 쿡찌르기가 그룹상세에서도 있고 그룹 홈 화면에서도 있고, 그룹상세,그룹홈은 메인 액티비티에서 실행되는 애들이자나..? 그래서 하나의 액티비티 뷰모델에서 처리되면 뭔가 더 일관적이고 좋지 않을까..? 코드도 재사용될 수 있을거라고 생각되어서..! 그래도 각각의 뷰모델에서 처리되는게 나을까..?

Copy link
Member

Choose a reason for hiding this comment

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

오 그러네! 좋다 근데 혹시 그러면 MainViewModel 작업중이야? 나 그룹상세 작업중인데 겹칠거같아서

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@sooziini 나는 저게 쿡찌르기라는게 명확하지 않은거 같아서 어떤 역할을 하는 람다이다를 명시해주는게 더 좋다고 생각했어..! 예를 들면 막 onClickPoke 라고 하면 뭔지 모를거 같아서..! 백버튼이나 넥스트버튼 같은 경우는 버튼 자체가 명확하지만 쿡찌르기는 그렇지 않다고 생각했고든..ㅠ @JeonK1 @oreocube 여러분들은 어떻게 생각하시나유..?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@oreocube 아니아니..! 아직 작업중이 아니야..! 그러면 지혜거 머지 될때까지 기다려도 괜찮을까..?

Copy link
Member

Choose a reason for hiding this comment

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

어..? 내일까지 해보고 안될거같으면 넘겨도 되나? ㅋㅋㅋㅋㅋㅋㅋㅋㅋ

Copy link
Member

@oreocube oreocube Aug 3, 2024

Choose a reason for hiding this comment

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

쿡찌르기가 명확하지 않다는게 먼뜻이지? 아니면 나는 그룹상세에 ActionButton으로 클릭 다 묶어놓고 클릭리스너 내에서 GroupStatusType 가지고 분기처리하려는데 이런건 어때? 약간 이런 느낌

스크린샷 2024-08-04 오전 12 12 29

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋ아아 오키오키 안되면 내가 작업할게 괜차낰ㅋㅋㅋㅋ 아 이게 수진이는 onSendFcm이라는게 뭔가 어떤 버튼이 눌렸다! 라는 걸 명시해주는거를 원하는거 같아서 뭐 onClickPokeButton이런 느낌으로 원하는거 같은데, 나는 이 이름이 어떤역할을 하는지 명확하지 않은거 같아서 sendFcm으로 지은거거든.. 그래서 그런거였엉!

아아 분기처리라는게 이게 타입마다 들어가있는 버튼이 다르니깐 타입마다 로직을 다르게 하겠다는 소리지? 진짜 신박한데...???

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

@sooziini sooziini left a comment

Choose a reason for hiding this comment

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

고생했어 정우~~~

@@ -26,6 +27,7 @@ fun NavGraphBuilder.groupHomeNavGraph(
onClickMyPage = onClickMyPage,
onClickGroupMake = onClickGroupMake,
onClickGroupEnter = onClickGroupEnter,
onClickSendFcm = onClickPicFcm,
Copy link
Collaborator

Choose a reason for hiding this comment

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

지금은 아니지만 fcm이 쿡찌르기 말고도 쓰이니까 fcm보다는 쿡찌르기 버튼이 눌렸다는 네이밍으로 변경하면 좋긴 하겠당.

Comment on lines 163 to 173

if (groupInfo.status == GroupStatusType.AFTER_MY_VOTE) {
PicNormalButton(
modifier = Modifier
.padding(top = 16.dp)
.align(Alignment.CenterHorizontally),
iconRes = R.drawable.ic_group_notice,
text = stringResource(R.string.group_home_pic_fcm),
onButtonClicked = onClickSendFcm,
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

요기 쿡찌르기가 아니라 이벤트 생성하기가 들어가야 할 거 같은데 ..?

@sooziini sooziini self-requested a review August 5, 2024 00:02
@SEO-J17 SEO-J17 requested review from oreocube, sooziini and JeonK1 August 7, 2024 01:30
@SEO-J17
Copy link
Collaborator Author

SEO-J17 commented Aug 7, 2024

KakaoTalk_Video_2024-08-07-14-06-28.mp4

위 영상 처럼 수정 했구요. status가 생각해보니 status에 따라서 카드안에 날짜아니면 무조건 소개 텍스트가 들어가서 when문 삭제했습니다

Copy link
Collaborator

@JeonK1 JeonK1 left a comment

Choose a reason for hiding this comment

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

굿! 어푸완료~~

@SEO-J17 SEO-J17 force-pushed the feature/#127-group-home-screen-state-logic branch from 78e191c to aa96891 Compare August 12, 2024 08:47
@SEO-J17 SEO-J17 merged commit deb811b into develop Aug 12, 2024
1 check passed
@SEO-J17 SEO-J17 deleted the feature/#127-group-home-screen-state-logic branch August 12, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

그룹 홈화면 상태에 따른 ui 로직 구현
4 participants