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

[ISSUE-61] 약속 공유하기 UI 및 카카오톡 공유 기능을 구현합니다 #98

Merged
merged 13 commits into from
Jul 18, 2022

Conversation

jihee-dev
Copy link
Member

@jihee-dev jihee-dev commented Jul 17, 2022

ISSUE


작업 내용

  • 약속 공유하기 화면 구현
  • 링크 복사 구현
  • 카카오톡 공유하기 기능 구현

실행 화면

Screen Shot Screen Shot
스크린샷 2022-07-18 오전 3 01 15 스크린샷 2022-07-18 오전 3 01 23
스크린샷 2022-07-18 오전 3 05 16 IMG_8815

Check List

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

- UI 구현
- 링크 복사 기능 구현
- 카카오톡 공유하기 임시 구현
- 네비게이션 구현
- 카카오톡 공유하기 intent 처리
- secrets 라이브러리를 통해서 BuildConfig 생성
- 서버로부터 이미지 받아오도록 수정
- 이미지 크기 조정
- 디자인 수정
…hare-plan-ui

# Conflicts:
#	data/src/main/java/com/yapp/growth/data/di/DataModule.kt
#	presentation/build.gradle.kts
#	presentation/src/main/java/com/yapp/growth/presentation/ui/createPlan/CreatePlanScreen.kt
#	presentation/src/main/res/values/strings.xml
@jihee-dev jihee-dev self-assigned this Jul 17, 2022
@KwonDae KwonDae added this to the Sprint #7 milestone Jul 18, 2022
Copy link
Member

@KwonDae KwonDae left a comment

Choose a reason for hiding this comment

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

공유하기 기능 구현하시느라 고생많으셨습니다!
카카오톡으로 공유하기 기능은 완성되었으니 링크로 공유하는 기능만 구현하면 끝나겠네요!
LGTM 👍

@@ -46,6 +47,7 @@ dependencies {
app.ModuleDependencies.materialCalendarView.implement(this)
app.ModuleDependencies.accompanist.implement(this)
app.ModuleDependencies.kotlinDateTime.implement(this)
app.ModuleDependencies.kakaoSdk.implement(this) // Share 부분만 의존성 추가하는 것이 좋은가..?
Copy link
Member

Choose a reason for hiding this comment

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

good, better 차이의 느낌일 것 같은데
웬만하면 분리해서 의존성을 추가하는게 나을 것 같습니다

Copy link
Member Author

@jihee-dev jihee-dev Jul 18, 2022

Choose a reason for hiding this comment

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

좋아여 네이밍도 kakaoOauth, kakaoShare로 바꾸고 나눠서 의존성 추가해놓겠습니당

Copy link
Member

Choose a reason for hiding this comment

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

죠습니당

Comment on lines 192 to 202
runCatching {
KakaoCustomTabsClient.openWithDefault(context, sharerUrl)
}.onFailure {
failToShareWithKakaoTalk()
}

runCatching {
KakaoCustomTabsClient.open(context, sharerUrl)
}.onFailure {
failToShareWithKakaoTalk()
}
Copy link
Member

Choose a reason for hiding this comment

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

try-catch 대신 runCatching 으로 onSuccess, onFailure 사용해서 Result 타입으로 관리하는 부분 정말 잘 하셨어요!
runCaching, mapCatching 모두 많이 쓰이는 Api 이므로 잘 익혀놓으시면 쓸일이 많을겁니다

하나 궁금한건 openWithDefault, open 두 메소드 모두 호출되어야 공유하기 탭이 활성화 되는건가요? 공유하기 로직이 어떻게 되는지 몰라서 궁금합니당

Copy link
Member Author

Choose a reason for hiding this comment

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

openWithDefault()open() 차이가 CustomTabsServiceConnection을 지원하는 브라우저로 여냐 아니냐의 차이인데
아무래도 둘 중 하나의 방법으로만 열어야 할 것 같네요,,,
다시 확인해서 문제 있는 부분 수정해놓겠습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

확인해 보니 확실히 문제가 있었네요 ㅠㅠ 수정해서 다시 커밋했어요!

Copy link
Member

Choose a reason for hiding this comment

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

좋아요! 머지해주시면 됩니다!
LGTM 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

꼼꼼하게 확인해 주셔서 감사합니당!!

@jihee-dev jihee-dev merged commit b8a1e2d into develop Jul 18, 2022
@jihee-dev jihee-dev deleted the feature/issue-61-share-plan-ui branch July 18, 2022 13:30
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.

[Sprint #7] 약속잡기 - 공유하기(P_06) 화면 구현
2 participants