-
Notifications
You must be signed in to change notification settings - Fork 0
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
[TNT-176] 트레이너, 트레이니 메인 네비게이션 기초 세팅 #64
Conversation
* 모든 화면에서 Scaffold 를 사용하는 것으로 협의되었으므로, Main 의 innerPadding 제거 처리
* NavHost 에 한 번이라도 등록된 경우, 다시 재사용이 불가능. (viewmodelStore should be set before setGraph call) * 이에 따라 Main 화면이 생성될 때마다 NavController 인스턴스를 생성하도록 수정
* 세션 유효 여부에 따라 startDestination 이 변경될 수 있으므로, 로직 수정 처리 * Note : https://medium.com/@banmarkovic/jetpack-compose-clear-back-stack-popbackstack-inclusive-explained-14ee73a29df5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아이고 진짜 고생하셨습니다..! 👍👍 쵝오
궁금한거 몇 개 리뷰 남겨놨습니다 :-) 머지되면 홈화면 작업 바로 시작할게요!
private fun SignUpRequest.toRequestBody(json: Json): RequestBody { | ||
private fun SignUpRequest.toRequestBody(): RequestBody { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗,, 감사합니다
launchSingleTop = true | ||
restoreState = true | ||
} | ||
navController.navigateToTraineeMain(clearBackStack = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 navigateToTrainerMain
은 아직 추가 안 된건가용?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
어라 이거 API 반영 됐었군요!.. 이따 집가서 수정해두겠습니다 '-'
trainerMainScreen( | ||
navigateToConnect = navController::navigateToTrainerConnect, | ||
navigateToWebView = navController::navigateToWebView, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
연결 화면으로 이동하기 위해
trainerMainScreen
에서는 함수 참조 방식(::)을 사용하고
traineeMainScreen
에서는 람다({})를 사용하는 방식인데
둘이 다른 방식을 선택한 이유와, 두 방식으로 나눠 사용하는 기준이 있는지 궁금합니다!!
통일성을 위해 함수 참조를 우선적으로 사용하도록 수정하는게 좋을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우선 일전에 말씀드렸듯이 람다 타입을 넘길 경우 함수 객체의 인스턴스를 넘겨주는 개념이므로 리컴포지션 카운트가 증가합니다!
참조를 사용할 경우 동일한 인스턴스를 지속적으로 재사용하는 개념이므로, 리컴포지션이 최적화됩니다 !_!
그래서 가능하다면 참조를 사용하는 것이 가장 좋구요..!
그리고 동일한 connect
지만 넘겨주는 방식이 달랐던 이유는, trainerConnect
에서는 (boolean, boolean) -> Unit
형태로 함수 타입을 넘겨주고 있어 참조가 가능하지만 traineeConnect
에서는 () -> Unit
형태로 전달하고 있기 때문에 참조가 불가능합니다 '-'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아하 이해했습니다! 머지되면 기존 코드들 함수 타입을 넘겨주도록 수정해보겠습니다 👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserType 다 찾아서 고치시느라 고생하셨습니다!! 😢👍
fun fromDomain(type: UserType): DefaultUserProfile { | ||
fun fromDomain(type: User): DefaultUserProfile { | ||
return when (type) { | ||
is UserType.Trainee -> Trainer | ||
is UserType.Trainer -> Trainee | ||
is User.Trainee -> Trainer | ||
is User.Trainer -> Trainee | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿굿굿 감사합니다👍
@@ -22,4 +25,5 @@ fun LoginResponse.toDomain(): LoginResult = | |||
email = socialEmail, | |||
authType = socialType, | |||
isSignUp = isSignUp, | |||
userType = runCatching(memberType::toDomain).getOrNull(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😯👍
companion object { | ||
val EMPTY = Trainer( | ||
id = "", | ||
name = "", | ||
image = null, | ||
) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 이런 방법이 있었군요..!
📝 작업 내용
closes [TNT-176] 로그인 시 유저 타입에 따라 홈 화면 분기 처리 #39
트레이너, 트레이니 메인 화면의 네비게이션 관련 기초 세팅을 진행하였습니다.
navigation
을 활용한Single NavGraph
구조와, 현재의Multiple NavGraph
구조를 되게 많이 고민했는데, 우선Multiple NavGraph
구조를 선택했습니다.Main
전까지만 동일(비슷)한 플로우를 지니고,Main
부터는 완전히 다른 앱이라고 봐야 합니다.Main
을 기준으로NavGraph
를 한 번 더 분리하면, 직군별로 유연하게Navigation
구조를 구성할 수 있겠다 생각했습니다.TrainerMain
,TraineeMain
이 명확히 분리되어 가독성이 좋다는 느낌을 받았어요.다만
Multiple NavGraph
는 딥링크와 같은 부분을 핸들링하기 까다롭다는 부분때문에 권장되는 형태는 아니라는 점인데요, 이 점이 마음에 걸려 자체적으로 딥링크 관련 테스트를 진행해보았는데 일단 동작 시키는 데엔 성공했습니다.물론 더 복잡한 네비게이션 로직이 요구되는 경우 추후 변경을 해야할 수도 있겠지만, 우리 앱에 맞는 형태는 일단 이 네비게이션 구조라고 판단내렸습니다.
고민을 더 하고 싶었는데 시간 여건상 우선 이 상태로 PR 올립니다 '-'
📸 실행 화면
TnT_deeplink.mp4
TnT_navigation.mp4
🙆🏻 리뷰 요청 사항
BottomNavigationView
는 아직 미구현 상태입니다! 어떻게 동작하는지만 봐주시면 좋을 것 같아요!NoticationScreen
머지되면 컨플릭 해결 +Navigation
연동까지 진행한 후 머지하도록 하겠습니다!👀 레퍼런스