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-31] 카카오 로그인 SDK 연동 #49

Merged
merged 48 commits into from
Jun 26, 2022

Conversation

KwonDae
Copy link
Member

@KwonDae KwonDae commented Jun 20, 2022

ISSUE


작업 내용

  • �카카오 로그인 SDK 연동
  • 로그인, 로그아웃, AccessToken 가져올 때 에러처리
  • Interceptor 를 통해 헤더에 AccessToken 추가해서 통신
  • 401 에러로 내려오면 refreshToken() 수행 후 통신 재시도

실행 화면

Screen Shot
ezgif com-gif-maker (1)

Check List

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

@KwonDae KwonDae added this to the Sprint #2 milestone Jun 20, 2022
@KwonDae KwonDae self-assigned this Jun 20, 2022
@KwonDae KwonDae removed the request for review from jihee-dev June 24, 2022 17:52
@KwonDae KwonDae requested a review from jihee-dev June 24, 2022 17:52
Copy link
Member

@hoyahozz hoyahozz 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 61 to 63
KakaoLoginButton {
viewModel.requestLogin(this@LoginActivity)
}
Copy link
Member

Choose a reason for hiding this comment

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

요부분에 onClick 파라미터 같이 첨부해줘서 가독성 올려주면 좋을 것 같습니당!

Copy link
Member Author

Choose a reason for hiding this comment

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

164f160
그러게요 저도 이부분이 찜찜했는데 파라미터 추가해주는게 좋을 것 같아요!

Comment on lines +74 to +77
fun startActivity(context: Context) {
val intent = Intent(context, LoginActivity::class.java)
context.startActivity(intent)
}
Copy link
Member

Choose a reason for hiding this comment

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

혹시 LoginActivity 에서 바로 인텐트를 호출하는게 아니라, MainActivityStartActivity 를 사용하는 특별한 이유가 있을까용 ?.?

Copy link
Member Author

Choose a reason for hiding this comment

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

가독성 & 보일러 플레이트 라고 생각합니당

handleEvent(state)
}
}

setContent {
Copy link
Member

Choose a reason for hiding this comment

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

작은 의견인데, 액티비티의 setContent 내에는 ???Screen() 식으로 끝내는건 어떨까요?
UI 관련 작업이 떨어져 있으면 가독성이 떨어질 수도 있을 것 같아요! (의견입니다)

Copy link
Member Author

Choose a reason for hiding this comment

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

b7f2f8f

피드백 감사해용 👍

)

Text(
text = "카카오 로그인",
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 Author

Choose a reason for hiding this comment

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

6c52b10
수정했습니다~

tint = Color.Unspecified
)
Text(
modifier = Modifier.fillMaxWidth().padding(start = 6.dp),
Copy link
Member

Choose a reason for hiding this comment

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

Modifier
    .fillMaxWidth()
    .padding(start = 6.dp)

Modifier는 요런식으로 통일하는건 어떨까용 ?.?

Copy link
Member Author

Choose a reason for hiding this comment

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

a6006ab

수정했습니다~



@Composable
fun TitleIconAndText() {
Copy link
Member

Choose a reason for hiding this comment

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

(의견임니다) 아이콘 + 플랜즈 + 친구들과 함께하는 약속잡기

요부분은 뭔가 앱을 소개하는 느낌인데, fun Introduce() 로 하나로 묶는건 어떨까용 ?.?

Copy link
Member Author

Choose a reason for hiding this comment

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

a6006ab
좋은거 같아요 하나로 수정했습니다~!

Comment on lines +8 to +13
/**
* throws 있는 메소드는
* 사용할 때 에러 처리를 해주어야 합니다.
* ex) runCatching { }, mapCatching { }
*
*/
Copy link
Member

Choose a reason for hiding this comment

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

배워감니다.. 줍줍 👍👍👍👍👍

@KwonDae KwonDae requested a review from hoyahozz June 26, 2022 13:47
Copy link
Member

@hoyahozz hoyahozz left a comment

Choose a reason for hiding this comment

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

좋습니당 :) 👍👍👍👍👍

@KwonDae KwonDae merged commit 05570d0 into develop Jun 26, 2022
@KwonDae KwonDae deleted the feature/issue-031-kakao-login branch July 8, 2022 17:51
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 #3] 카카오 로그인 화면 구현 [Sprint #2] 카카오 로그인 SDK 연동
2 participants