-
Notifications
You must be signed in to change notification settings - Fork 0
[setting/#4] 네비게이션 세팅 #9
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
Conversation
sonms
left a comment
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.
p1: RouteModel 파일 한 곳에서 관리하고 있는데 각 Feature별 navigation으로 분리하여 관리하는 방식은 어떨까요?
이렇게 한 번에 모든 Route를 중앙에서 관리하게 된다면 예를 들어 아이 기능 쪽에서 굳이 알 필요 없는 부모 Route까지 참조 가능하게 되어 불필요한 의존성이 생기고 캡슐화가 깨질 수 있습니다. 앱 규모가 커질 수록 결합도가 높아질 수 있으니 각 Feature에 navigation으로 라우트를 정의하여 SRP를 지키는 편이 유지보수하기 더 좋을 것 같습니다.
또한 Route를 묶는 방식은 좋으나 Sealed Class -> Sealed interface를 사용하는 것을 추천합니다.
class 같은 경우 단일 상속만 가능하기도 하고 생성자가 존재해서 초기화 비용이 존재합니다, 보통 부모의 상태나 코드, 변수를 자식에게 물려줄 때 사용합니다.
interface로 묶어서 다중 구현이 가능하도록 및 생성 비용이 거의 없도록 하는 것이 좋을 것 같습니다. 보통 단순 같은 그룹을 묶을 때 사용합니다.
Route 정의 시 보통 상태나 변수를 가지지 않고 타입만 정의하는 경우가 많으므로 다중 구현이 가능하고 더 가벼운 인터페이스가 더 적합한 것 같습니다.
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.
p1: RouteModel 파일 한 곳에서 관리하고 있는데 각 Feature별 navigation으로 분리하여 관리하는 방식은 어떨까요?
이렇게 한 번에 모든 Route를 중앙에서 관리하게 된다면 예를 들어 아이 기능 쪽에서 굳이 알 필요 없는 부모 Route까지 참조 가능하게 되어 불필요한 의존성이 생기고 캡슐화가 깨질 수 있습니다. 앱 규모가 커질 수록 결합도가 높아질 수 있으니 각 Feature에 navigation으로 라우트를 정의하여 SRP를 지키는 편이 유지보수하기 더 좋을 것 같습니다.
또한 Route를 묶는 방식은 좋으나 Sealed Class -> Sealed interface를 사용하는 것을 추천합니다. class 같은 경우 단일 상속만 가능하기도 하고 생성자가 존재해서 초기화 비용이 존재합니다, 보통 부모의 상태나 코드, 변수를 자식에게 물려줄 때 사용합니다. interface로 묶어서 다중 구현이 가능하도록 및 생성 비용이 거의 없도록 하는 것이 좋을 것 같습니다. 보통 단순 같은 그룹을 묶을 때 사용합니다. Route 정의 시 보통 상태나 변수를 가지지 않고 타입만 정의하는 경우가 많으므로 다중 구현이 가능하고 더 가벼운 인터페이스가 더 적합한 것 같습니다.
[Type Safe Nested Navigation in Jetpack Compose](https://poulastaa.medium.com/type-safe-nested-navigation-in-jetpack-compose-c8a5bdd2d17d)
[[Kotlinx serialization] Json 직렬화/역직렬화 -Polymorphism #5](https://tourspace.tistory.com/366#google_vignette)
옛날에 해당 블로그들을 보고 공부를 했었는데, 아직 혼동된 개념이 많은 것 같습니다 ㅎ.. ㅜㅜ 좋은 의견 감사합니다.
중첩 네비게이션 사용 시 composable<Auth.Login> 형태로 타입 세이프 네비게이션을 구현하려다 보니, 모든 Route를 하나의 sealed class로 묶어 공통 모델로 관리하려 했습니다. pr에 질문한 대로 고민을 많이했었는데 확실히 말씀하신 것처럼 이 방식은 불필요한 의존성과 높은 결합도가 생길 수 있겠네요..
또 구현하다 보니까 composable에서 사용하려면 sealed class로 상속받아야 한다고 생각했는데, sealed interface로도 충분히 가능할 것 같네요!
Graph에 해당하는 Route만 공통 모듈에서 관리하고, MainTab은 각 presentation 패키지 최상위에, 그리고
세부 화면 Route는 각 Feature의 navigation 패키지 내에서 interface 로 정의하여 관리 하려고 하는데 아래 처럼 관리하는 방식으로 가는게 좋을까요?
package com.kiero.core.navigation
import kotlinx.serialization.Serializable
// core/RouteModel
@Serializable
data object AuthGraph : Route
@Serializable
data object ParentGraph : Route
@Serializable
data object KidGraph : Routepackage com.kiero.presentation.parent.navigation
// Parent 패키지 상위 네비게이션
sealed interface ParentTab : Route
@Serializable
data object Schedule : ParentTab
@Serializable
data object Alarm : ParentTab
fun NavController.navigateToParent(
navOptions: NavOptions? = null,
) {
navigate(ParentGraph, navOptions)
}
fun NavGraphBuilder.parentNavGraph(
navController: NavHostController,
paddingValues: PaddingValues,
navigateUp: () -> Unit,
) {
navigation<ParentGraph>(
startDestination = Schedule
) {
parentScheduleNavGraph(
paddingValues = paddingValues,
navigateUp = navigateUp,
)
parentAlarmNavGraph(
paddingValues = paddingValues,
navigateUp = navigateUp,
)
}
}
// 세부 화면 생성 시 (ex 부모/스케쥴 Tab 내부의 상세화면)
sealed interface ScheduleRoute
@Serializable
data object List : ScheduleRoute 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.
Core에 공통으로 Graph를 두는 방식을 지양해야 한다고 생각해요 여전히 불필요한 의존성이 생기고 캡슐화가 깨질 수 있습니다
Core는 변하지 않는 공통 요소를 담는 것이 목표인데 Graph 를 담게되면 그래프가 추가되면 추가될 수록 변경되겠죠?
또한 의존성에 대해 자세히 살펴볼게요 멀티모듈이라고 가정한다고 했을때
아이 Feature에서 Core의 RouteModel을 dependency에 추가하게 된다면
결국 모든 RouteGraph에 접근이 가능해져요
Core는 그리고 구체적인 저희 앱의 기능에 대해 몰라도 될 것 같은데 Graph가 존재하면 해당 기능 Graph를 알게 되는 것도 결합력이 올라갈 것 같아요
만약 해당 뷰의 네비게이션으로 넘겨주는 데이터가 필요하다면 ?
@Serializable internal data class FeedProfileGraph(val userId: Long, val showLikedDiaries: Boolean = false)
더더욱 그러겠죵?
package com.paw.key.presentation.ui.course.navigation
// WalkCourceRoute파일
sealed interface WalkRoute : MainTabRoute // 중요: 이게 제일 최상위 타입 즉, 이 WalkCourse 계층 구조의 기준
@Serializable
data object WalkPrepare: WalkRoute
@Serializable
data object WalkCourse: WalkRoute
@Serializable
data object WalkComplete: WalkRoutepackage com.paw.key.presentation.ui.course.navigation
@Serializable
data object WalkCourseGraph : WalkRoute // 이건 여러 자식들을 묶는 하나의 구현체 이지 최상위 타입이 아님
fun NavGraphBuilder.walkCourseGraph(
paddingValues: PaddingValues,
navController: NavController,
navigateWalkReview : () -> Unit
) {
navigation<WalkCourseGraph>(
startDestination = WalkPrepare
) {
composable<WalkPrepare> {
WalkPrepareRoute(
paddingValues = paddingValues,
navigateWalkCourse = navController::navigateWalkCourse
)
}
composable<WalkCourse> {
WalkCourseRoute(
paddingValues = paddingValues,
navigateUp = navController::navigateUp,
//navigateWalkComplete = navController::navigateWalkComplete,
navigateReview = navigateWalkReview
)
}
composable<WalkComplete> {
WalkCompleteRoute(
paddingValues = paddingValues,
//navigateReview = navigateWalkReview
)
}
}
}
이런 구조가 좋다고 생각합니다
| navigateToParent: () -> Unit, | ||
| navigateToKid: () -> Unit, | ||
| state: UiState<PersistentList<DummyEntity>>, | ||
| modifier: Modifier = Modifier, |
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.
마찬가지로 Screen에서 굳이 modifier를 뚫어줄 필요는 없다고 생각하는데 주완님 생각은 어떠신가요? 저스트 궁금
app/src/main/java/com/kiero/presentation/main/navigation/MainNavigator.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/kiero/presentation/main/navigation/component/BottomBarTab.kt
Show resolved
Hide resolved
app/src/main/java/com/kiero/presentation/main/navigation/component/MainBottomBar.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/kiero/presentation/main/navigation/KieroNavHost.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/kiero/presentation/auth/navigation/AuthNavigation.kt
Show resolved
Hide resolved
seungjae708
left a comment
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.
네비게이션은 역시 어렵네요... 고생하셨습니다!!
app/src/main/java/com/kiero/presentation/main/navigation/MainTab.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/kiero/presentation/main/navigation/MainTab.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/kiero/presentation/main/navigation/component/MainBottomBar.kt
Outdated
Show resolved
Hide resolved
dmp100
left a comment
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.
두개의 뷰로 나뉘어져 네비게이션 짜기 복잡해했을 것 같은데 고생 많으셨습니다 !
string.xml에 있는 오타정도만 수정 부탁드립니다 !
ISSUE
❗ WORK DESCRIPTION
📢 TO REVIEWERS
📸 SCREENSHOT
-.Clipchamp.11.mp4