[Init] #3 Nav3 셋업#7
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughNavigation3 기반의 다층 네비게이션(core/navigation: NavigationState, Navigator/Impl), 피처별 API/impl 모듈(pose/archive/map)과 EntryProvider 등록, 앱 MainActivity에 Hilt 주입된 Navigator 연결 및 Compose 바텀 네비게이션, 빌드 로직·버전 카탈로그 확장 추가. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant MainActivity
participant NavigatorImpl
participant NavigationState
participant EntryProviderScope
participant FeatureModule as "Feature\n(Pose/Archive/Map)"
User->>MainActivity: 앱 실행 / 탭 선택
MainActivity->>NavigatorImpl: navigate(key)
alt key가 탑레벨일 때
NavigatorImpl->>NavigationState: goToTopLevel(key)
else 서브 키일 때
NavigatorImpl->>NavigationState: goToKey(key)
end
NavigationState-->>MainActivity: 상태(현재 키 및 스택) 업데이트
MainActivity->>EntryProviderScope: entryProvider 구성 (Set<EntryProviderInstaller>)
EntryProviderScope->>FeatureModule: 등록된 entry 빌드 요청
FeatureModule-->>MainActivity: NavEntry 반환
MainActivity->>User: NavDisplay로 화면 렌더링
sequenceDiagram
autonumber
participant User
participant BottomNavBar
participant NavigatorImpl
participant NavigationState
User->>BottomNavBar: 탭 클릭
BottomNavBar->>NavigatorImpl: onTabSelected(navKey)
NavigatorImpl->>NavigationState: goToTopLevel(navKey)
NavigationState-->>BottomNavBar: UI 상태(선택/비선택) 업데이트
User->>System: Back 버튼
System->>NavigatorImpl: goBack()
alt 현재가 startKey
NavigatorImpl->>NavigatorImpl: 에러 처리 / 종료 시그널
else top-level 스택에서 pop
NavigatorImpl->>NavigationState: pop topLevelStack
else sub-stack에서 pop
NavigatorImpl->>NavigationState: pop currentSubStack
end
NavigationState-->>User: 이전 화면으로 복귀
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (10)
build-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidFeatureApiConventionPlugin.kt (1)
1-5: 패키지 선언이 누락되었습니다.파일 경로가
com/neki/android/buildlogic/plugins인데 패키지 선언이 없습니다. 다른 컨벤션 플러그인과의 일관성을 위해 패키지 선언을 추가하는 것이 좋습니다.🔎 제안하는 수정
+package com.neki.android.buildlogic.plugins + import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.kotlin.dsl.apply import org.gradle.kotlin.dsl.dependenciesfeature/pose/impl/build.gradle.kts (1)
9-14: 의존성 구성 확인impl 모듈의 의존성 구성이 적절합니다. pose.api 모듈에 대한 의존성이 올바르게 선언되어 있습니다.
참고:
androidx.appcompat의존성이 Compose 기반 구현에 실제로 필요한지 확인해보세요. Compose만 사용한다면 불필요할 수 있습니다.feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleScreen.kt (1)
9-14:modifier파라미터가 적용되지 않았습니다.
modifier파라미터가 선언되었지만Textcomposable에 전달되지 않아 호출자가 레이아웃을 제어할 수 없습니다.🔎 수정 제안
@Composable fun SampleScreen( modifier: Modifier = Modifier, viewModel: SampleViewModel = hiltViewModel(), ) { - Text("Sample") + Text("Sample", modifier = modifier) }app/src/main/res/drawable/ic_nav_map_unselected.xml (1)
8-8: 하드코딩된 색상 값 사용
#B7B9C3색상이 직접 하드코딩되어 있습니다. 디자인 시스템 통합 시 테마 색상 리소스(@color/...또는?attr/...)로 변경하면 유지보수성이 향상됩니다. PR 설명에 임시 구현이라고 언급되어 있으므로 현재는 허용 가능합니다.Also applies to: 14-14
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapEntryProvider.kt (2)
25-27: Entry 구현 내용을 추가해야 합니다.
entry<MapNavKey.Map>블록이 비어 있습니다. Map 화면의 Composable 콘텐츠를 추가해야 합니다.Map 화면 구현 코드를 생성하거나 이를 추적할 이슈를 생성하시겠습니까?
29-29: 파일 끝의 불필요한 빈 줄을 제거하세요.🔎 제안된 수정사항
private fun EntryProviderScope<NavKey>.mapEntry(navigator: Navigator) { entry<MapNavKey.Map> {} } - -feature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/navigation/PoseEntryProvider.kt (1)
25-27: 빈 entry 본문 및 미사용 파라미터 확인 필요.
entry<PoseNavKey.Pose> { }가 비어 있어 이 경로로 이동 시 아무 UI도 렌더링되지 않습니다. 또한navigator파라미터가 사용되지 않고 있습니다. 의도된 플레이스홀더라면 TODO 주석을 추가하거나, 실제 UI 구현이 필요하다면PoseScreen컴포저블을 추가해 주세요.🔎 사용하지 않는 파라미터 제거 제안
-private fun EntryProviderScope<NavKey>.poseEntry(navigator: Navigator) { - entry<PoseNavKey.Pose> { } +private fun EntryProviderScope<NavKey>.poseEntry() { + entry<PoseNavKey.Pose> { + // TODO: PoseScreen 구현 필요 + } }그리고 호출부도 수정:
fun providePoseEntryBuilder(navigator: Navigator): EntryProviderInstaller = { - poseEntry(navigator) + poseEntry() }app/src/main/java/com/neki/android/app/MainActivity.kt (1)
27-28: 구체적 구현체 대신 인터페이스 주입을 권장합니다.
NavigatorImpl을 직접 주입하는 것보다Navigator인터페이스를 주입하는 것이 DI 원칙에 부합합니다. 이렇게 하면 테스트 시 mock 교체가 용이하고 구현 변경에 유연하게 대응할 수 있습니다.🔎 인터페이스 주입으로 변경
@Inject -lateinit var navigator: NavigatorImpl +lateinit var navigator: Navigator단,
Navigator인터페이스에state프로퍼티가 노출되어 있어야 합니다.app/src/main/java/com/neki/android/app/ui/BottomNavigationBar.kt (1)
69-99: 하드코딩된 컬러값은 디자인 시스템으로 이전이 필요합니다.PR 설명에 디자인 시스템 폰트/컬러 적용이 보류 중이라고 명시되어 있습니다. 현재 하드코딩된 컬러값(
0xFF3C3E48,0xFFB7B9C3)을 추후NekiTheme의 컬러 시스템으로 교체하시기 바랍니다.추가로,
BottomNavigationBarItem이 public으로 선언되어 있는데, 이 컴포넌트 내부에서만 사용된다면private으로 변경하는 것을 고려해 주세요.core/navigation/src/main/java/com/neki/android/core/navigation/NavigationState.kt (1)
31-50:toMutableStateList()호출이 매 리컴포지션마다 새 리스트를 생성합니다.Line 49의
toMutableStateList()는remember로 감싸지 않으면 매 리컴포지션마다 새로운SnapshotStateList인스턴스를 생성합니다. 이로 인해NavDisplay와 같은 소비자에서 참조 안정성이 깨져 불필요한 리컴포지션이 발생할 수 있습니다.
remember로 결과를 캐싱하여 참조 안정성을 확보하세요:@Composable fun NavigationState.toEntries( entryProvider: (NavKey) -> NavEntry<NavKey>, ): SnapshotStateList<NavEntry<NavKey>> { val decoratedEntries = subStacks.mapValues { (_, stack) -> val decorators = listOf( rememberSaveableStateHolderNavEntryDecorator<NavKey>(), rememberViewModelStoreNavEntryDecorator<NavKey>(), ) rememberDecoratedNavEntries( backStack = stack, entryDecorators = decorators, entryProvider = entryProvider, ) } - return topLevelStack - .flatMap { decoratedEntries[it] ?: emptyList() } - .toMutableStateList() + return remember(topLevelStack.toList(), decoratedEntries) { + topLevelStack + .flatMap { decoratedEntries[it] ?: emptyList() } + .toMutableStateList() + } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (54)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/com/neki/android/app/MainActivity.ktapp/src/main/java/com/neki/android/app/navigation/TopLevelNavItem.ktapp/src/main/java/com/neki/android/app/navigation/di/AppModule.ktapp/src/main/java/com/neki/android/app/navigation/di/NavigationModule.ktapp/src/main/java/com/neki/android/app/navigation/keys/Keys.ktapp/src/main/java/com/neki/android/app/ui/BottomNavigationBar.ktapp/src/main/res/drawable/ic_nav_archive_selected.xmlapp/src/main/res/drawable/ic_nav_archive_unselected.xmlapp/src/main/res/drawable/ic_nav_map_selected.xmlapp/src/main/res/drawable/ic_nav_map_unselected.xmlapp/src/main/res/drawable/ic_nav_pose_selected.xmlapp/src/main/res/drawable/ic_nav_pose_unselected.xmlapp/src/main/res/values/strings.xmlbuild-logic/build.gradle.ktsbuild-logic/src/main/java/com/neki/android/buildlogic/const/BuildConst.ktbuild-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidFeatureApiConventionPlugin.ktbuild-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidFeatureImplConventionPlugin.ktbuild.gradle.ktscore/navigation/.gitignorecore/navigation/build.gradle.ktscore/navigation/src/main/java/com/neki/android/core/navigation/NavigationState.ktcore/navigation/src/main/java/com/neki/android/core/navigation/Navigator.ktcore/navigation/src/main/java/com/neki/android/core/navigation/NavigatorImpl.ktfeature/archive/api/.gitignorefeature/archive/api/build.gradle.ktsfeature/archive/api/src/main/kotlin/com/neki/android/feature/archive/api/ArchiveNavKey.ktfeature/archive/impl/.gitignorefeature/archive/impl/build.gradle.ktsfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/navigation/ArchiveEntryProvider.ktfeature/map/api/.gitignorefeature/map/api/build.gradle.ktsfeature/map/api/src/main/java/com/neki/android/feature/map/api/MapNavKey.ktfeature/map/impl/.gitignorefeature/map/impl/build.gradle.ktsfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapEntryProvider.ktfeature/pose/api/.gitignorefeature/pose/api/build.gradle.ktsfeature/pose/api/src/main/java/com/neki/android/feature/pose/api/PoseNavKey.ktfeature/pose/impl/.gitignorefeature/pose/impl/build.gradle.ktsfeature/pose/impl/src/main/java/com/neki/android/feature/pose/impl/navigation/PoseEntryProvider.ktfeature/sample/api/build.gradle.ktsfeature/sample/api/src/main/java/com/neki/android/feature/sample/api/MyClass.ktfeature/sample/api/src/main/java/com/neki/android/feature/sample/api/SampleNavKey.ktfeature/sample/impl/build.gradle.ktsfeature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/MainActivity.ktfeature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/MainViewModel.ktfeature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleEntryProvider.ktfeature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleScreen.ktfeature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleViewModel.ktgradle/libs.versions.tomlsettings.gradle.kts
💤 Files with no reviewable changes (3)
- feature/sample/api/src/main/java/com/neki/android/feature/sample/api/MyClass.kt
- feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/MainViewModel.kt
- feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/MainActivity.kt
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/com/neki/android/app/MainActivity.kt (2)
core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(35-57)app/src/main/java/com/neki/android/app/ui/BottomNavigationBar.kt (1)
BottomNavigationBar(32-67)
app/src/main/java/com/neki/android/app/ui/BottomNavigationBar.kt (1)
core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(35-57)
feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleEntryProvider.kt (1)
feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleScreen.kt (1)
SampleScreen(8-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (43)
feature/pose/impl/.gitignore (1)
1-1: 표준 빌드 무시 규칙이 올바르게 추가되었습니다.모듈별 빌드 아티팩트를 버전 관리에서 제외하는 표준 관행입니다.
feature/pose/api/.gitignore (1)
1-1: LGTM!표준 빌드 디렉토리 무시 규칙입니다.
feature/map/impl/.gitignore (1)
1-1: LGTM!올바른 빌드 아티팩트 무시 규칙입니다.
feature/map/api/.gitignore (1)
1-1: LGTM!표준 .gitignore 규칙이 올바르게 적용되었습니다.
core/navigation/.gitignore (1)
1-1: LGTM!core/navigation 모듈의 빌드 디렉토리가 올바르게 무시 처리되었습니다.
feature/archive/impl/.gitignore (1)
1-1: LGTM!표준 빌드 무시 규칙입니다.
feature/archive/api/.gitignore (1)
1-1: LGTM!올바른 빌드 아티팩트 무시 규칙입니다.
build-logic/src/main/java/com/neki/android/buildlogic/const/BuildConst.kt (1)
12-13: Android SDK 36은 2025년 3월에 공식 출시된 안정적인 플랫폼입니다. 프로젝트의 현재 의존성 버전들이 모두 SDK 36과 호환됩니다:
- AGP 8.13.1: API 36 공식 지원 (최소 요구사항: AGP 8.9.1)
- Kotlin 2.1.0: AGP 8.13.1과 호환
- Hilt 2.54: AGP 8.1 이상 지원, KSP 기반 컴파일러 사용
- Navigation3 1.0.0: 현재 Kotlin/AGP 버전과 호환
- KSP 2.1.0-1.0.29: Kotlin 2.1.0과 일치
TARGET_SDK와 COMPILE_SDK를 36으로 업데이트하는 것은 안전하고 호환성 문제가 없습니다.
feature/sample/api/build.gradle.kts (1)
2-2: 새로운 feature.api 플러그인으로 마이그레이션이 잘 되었습니다.기존 library 플러그인에서 feature.api 플러그인으로의 전환이 적절합니다. 이를 통해 feature API 모듈에 필요한 의존성과 설정이 자동으로 적용됩니다.
build.gradle.kts (1)
3-3: android.library 플러그인 추가 확인라이브러리 모듈 지원을 위한 표준적인 플러그인 추가입니다.
feature/archive/api/build.gradle.kts (1)
1-7: Archive API 모듈 설정이 올바릅니다.feature API 컨벤션 플러그인을 사용한 표준적인 설정입니다. 네임스페이스도 적절하게 지정되었습니다.
feature/map/api/build.gradle.kts (1)
1-7: Map API 모듈 설정이 올바릅니다.Archive API 모듈과 동일한 패턴을 따르고 있으며, feature API 컨벤션 플러그인이 적절하게 적용되었습니다.
build-logic/src/main/java/com/neki/android/buildlogic/plugins/AndroidFeatureApiConventionPlugin.kt (1)
6-19: 플러그인 구현이 올바르게 설정되어 있습니다."neki.android.library" 플러그인이 build-logic에 제대로 등록되어 있으며, AndroidLibraryConventionPlugin도 존재합니다. API 모듈에 적합한 구성입니다.
api()의존성으로 navigation을 노출하는 것이 올바른 접근이며, 직렬화 플러그인도 네비게이션 데이터 처리에 적합합니다.feature/map/impl/build.gradle.kts (1)
1-14: LGTM!표준 feature 구현 모듈 구성이 올바르게 적용되었습니다. 플러그인, 네임스페이스, 의존성 선언이 일관성 있게 구성되어 있습니다.
app/src/main/res/drawable/ic_nav_map_selected.xml (1)
1-16: LGTM!하단 네비게이션 아이콘 리소스가 올바르게 정의되었습니다.
app/src/main/res/drawable/ic_nav_archive_unselected.xml (1)
1-9: LGTM!미선택 상태의 아카이브 아이콘이 올바르게 정의되었습니다.
feature/pose/api/build.gradle.kts (1)
1-7: LGTM!feature api 모듈의 표준 구성이 올바르게 적용되었습니다.
app/src/main/res/drawable/ic_nav_pose_selected.xml (1)
1-9: LGTM!선택 상태의 포즈 아이콘이 올바르게 정의되었습니다.
app/src/main/res/values/strings.xml (1)
4-6: LGTM!하단 네비게이션 라벨에 필요한 문자열 리소스가 올바르게 추가되었습니다.
app/src/main/java/com/neki/android/app/navigation/keys/Keys.kt (1)
6-7: LGTM!네비게이션 키 상수 정의가 올바르게 구성되었습니다. (import 오류 수정 필요)
feature/archive/impl/build.gradle.kts (1)
1-14: LGTM!표준 feature 구현 모듈 구성이 일관성 있게 적용되었습니다.
feature/sample/impl/build.gradle.kts (1)
2-3: LGTM!플러그인 별칭이
neki.android.feature.impl로 변경되고feature.sample.api의존성이 추가되어 feature 모듈 분리 패턴(api/impl)과 일관성 있게 구성되었습니다.Also applies to: 13-13
app/src/main/AndroidManifest.xml (1)
17-24: LGTM!
MainActivity가 app 모듈로 이동됨에 따라 manifest에서 상대 경로(.MainActivity)를 올바르게 사용하고 있습니다. 기존.NekiApplication참조와 일관성 있는 패턴입니다.app/build.gradle.kts (1)
14-34: LGTM!Navigation 3 설정을 위한 의존성이 잘 구성되었습니다:
- Core navigation 모듈 추가
- Feature 모듈들(pose, archive, map)이 api/impl 패턴으로 일관되게 추가됨
- 필요한 Compose 및 Navigation UI 라이브러리 포함
feature/map/api/src/main/java/com/neki/android/feature/map/api/MapNavKey.kt (1)
7-15: LGTM!Navigation key 패턴이 잘 구현되었습니다:
sealed interface로 타입 안전성 확보@Serializable로 상태 직렬화 지원Navigator확장 함수로 편리한 네비게이션 API 제공다른 feature 모듈(pose, archive)과 일관된 패턴입니다.
feature/pose/api/src/main/java/com/neki/android/feature/pose/api/PoseNavKey.kt (1)
7-15: LGTM!
MapNavKey와 동일한 패턴으로 일관성 있게 구현되었습니다. Sealed interface와@Serializabledata object를 사용한 Navigation key 정의가 적절합니다.feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleScreen.kt (1)
6-6: import 경로는 정확합니다현재 코드에서 사용하는
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel은 Hilt Compose 통합의 현재 표준입니다. 이전에 사용되던androidx.hilt.navigation.compose.hiltViewModel은 이미 deprecated되었으며, API가 lifecycle.viewmodel.compose로 이동되었습니다.Likely an incorrect or invalid review comment.
feature/sample/api/src/main/java/com/neki/android/feature/sample/api/SampleNavKey.kt (1)
1-15: LGTM! 잘 구현된 네비게이션 키입니다.파라미터를 받는
Sample데이터 클래스와 네비게이션 확장 함수가 올바르게 구현되었습니다. 다른 feature 모듈들의 NavKey 패턴과 일관성 있게 구성되어 있습니다.app/src/main/java/com/neki/android/app/navigation/di/AppModule.kt (1)
10-18: LGTM! Navigator DI 바인딩이 올바릅니다.
ActivityRetainedComponent스코프에서Navigator인터페이스를NavigatorImpl에 바인딩하는 것이 적절합니다.app/src/main/java/com/neki/android/app/navigation/di/NavigationModule.kt (1)
11-22: LGTM! NavigationState 제공 로직이 올바릅니다.
START_NAV_KEY와TOP_LEVEL_NAV_KEYS를 사용한NavigationState초기화가 적절하게 구현되었습니다.core/navigation/src/main/java/com/neki/android/core/navigation/Navigator.kt (1)
1-11: LGTM! 깔끔한 네비게이션 인터페이스입니다.
Navigator인터페이스와EntryProviderInstallertypealias가 명확하고 간결하게 정의되었습니다.settings.gradle.kts (1)
35-41: LGTM! 새 모듈이 올바르게 추가되었습니다.
core:navigation모듈과 feature 모듈들(pose, archive, map의 api/impl)이 프로젝트 구조에 맞게 적절히 추가되었습니다.core/navigation/build.gradle.kts (1)
11-14: Navigation3 라이브러리 버전이 유효하게 정의되어 있습니다.gradle/libs.versions.toml에서 버전이 명시적으로 관리되고 있으며, 사용 중인 버전들이 유효합니다:
- androidxNavigation3 = "1.0.0"
- androidxLifecycleViewModelNavigation3 = "2.10.0"
이 버전들은 안정 버전이며 알려진 보안 취약점이 없습니다.
feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleEntryProvider.kt (1)
1-35: LGTM!Assisted injection을 사용한
hiltViewModel패턴이 올바르게 구현되었습니다. 이 패턴은 런타임에navKey를 ViewModel에 전달하는 좋은 예시입니다.feature/sample/impl/src/main/java/com/neki/android/feature/sample/impl/SampleViewModel.kt (1)
1-21: LGTM!
@HiltViewModel과@AssistedFactory를 함께 사용한 패턴이 올바르게 구현되었습니다. Nav3에서 네비게이션 인자를 ViewModel에 전달하는 좋은 참조 구현입니다.build-logic/build.gradle.kts (1)
34-41: LGTM!feature 모듈의 api/impl 분리에 맞춰 플러그인을 두 개로 분리한 것이 적절합니다. 이 구조는 feature 모듈 간 의존성 관리를 개선합니다.
core/navigation/src/main/java/com/neki/android/core/navigation/NavigatorImpl.kt (2)
44-48:subList().clear()사용이 올바릅니다.
subList(1, size).clear()는 원본 리스트를 직접 수정하여 첫 번째 요소만 남기는 올바른 방법입니다.
18-27: startKey가topLevelKeys에 포함되어 있는지 확인하세요.
goBack()에서topLevelStack이 완전히 비워지는 상황은 방지됩니다. 다만,startKey가topLevelKeys에 없으면 더 심각한 문제가 발생합니다.topLevelStack이[startKey]로 축소되었을 때currentSubStack에 접근하면,subStacks에startKey의 항목이 없어 (NavigationState 라인 21) 예외가 발생합니다.topLevelStack이[startKey]상태에서currentSubStack이나currentKey에 접근하면 "Sub stack for startKey does not exist" 에러가 나올 수 있습니다.app/src/main/java/com/neki/android/app/navigation/TopLevelNavItem.kt (1)
11-38: Enum 구조가 잘 설계되었습니다.
@DrawableRes,@StringRes어노테이션을 활용하여 리소스 타입 안전성을 확보했고, 각 항목에navKey를 연결하여 네비게이션과 UI의 결합이 명확합니다.app/src/main/java/com/neki/android/app/ui/BottomNavigationBar.kt (2)
32-67:currentKey와currentTab파라미터의 역할이 혼동될 수 있습니다.
currentKey는 현재 네비게이션 위치(서브스택 포함)를,currentTab은 현재 선택된 탭을 나타내는 것으로 보입니다. 그러나:
- Line 59:
selected판단에는currentTab사용- Line 61: 클릭 방지에는
currentKey사용이 로직이 의도된 것인지 확인이 필요합니다. 만약 같은 탭 내에서 다른 화면에 있을 때 탭을 다시 클릭하면 어떤 동작을 원하시나요? 현재 로직상 같은 탭이라도
currentKey가 다르면onTabSelected가 호출됩니다.
101-113: Preview 구현이 적절합니다.
mutableStateOf를 사용하여 탭 전환을 인터랙티브하게 테스트할 수 있도록 구현되어 있습니다.gradle/libs.versions.toml (2)
43-50: 라이브러리 추가 및 버전 관리가 잘 구성되어 있습니다.Navigation3 관련 의존성들이 버전 카탈로그에 체계적으로 추가되었습니다. 그러나
hilt-lifecycle-viewmodel-compose가 알파 버전(1.3.0-alpha02)인 점은 주의가 필요합니다. 안정 버전이 출시되면 업그레이드를 고려해 주세요.
76-78: Feature 플러그인 분리가 적절합니다.
neki-android-feature를neki-android-feature-impl과neki-android-feature-api로 분리하여 feature 모듈의 api/impl 구조에 맞게 구성한 것은 좋은 접근입니다.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/neki/android/app/navigation/TopLevelNavItem.ktapp/src/main/java/com/neki/android/app/navigation/keys/Keys.ktfeature/archive/api/src/main/kotlin/com/neki/android/feature/archive/api/ArchiveNavKey.ktfeature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/navigation/ArchiveEntryProvider.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/neki/android/app/navigation/keys/Keys.kt
- app/src/main/java/com/neki/android/app/navigation/TopLevelNavItem.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (3)
feature/archive/api/src/main/kotlin/com/neki/android/feature/archive/api/ArchiveNavKey.kt (2)
1-11: 이전 이슈가 해결되었으며 구현이 올바릅니다.패키지 선언의 중복 문제가 수정되었습니다.
ArchiveNavKeysealed interface와Archivedata object의 구조가 Navigation3 패턴을 올바르게 따르고 있으며,@Serializable어노테이션도 적절히 적용되었습니다.
13-15: 편리한 확장 함수입니다.
Navigator에 대한 확장 함수로 Archive 화면으로의 네비게이션을 간편하게 제공합니다. 타입 안전성을 유지하면서 호출 코드를 간결하게 만듭니다.feature/archive/impl/src/main/kotlin/com/neki/android/feature/archive/impl/navigation/ArchiveEntryProvider.kt (1)
14-23: 이전 이슈가 모두 해결되었으며 Hilt 모듈 구성이 올바릅니다.패키지 선언, import 경로, 모듈 이름이 모두 올바르게 수정되었습니다.
ActivityRetainedComponent에 설치하는 것이 네비게이션 엔트리 제공자로서 적절하며,@IntoSet과@Provides를 통한 멀티바인딩 패턴이 올바르게 구현되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
app/src/main/java/com/neki/android/app/navigation/di/NavigationModule.kt (2)
15-16: provider 함수를 internal로 변경하는 것을 고려하세요.모듈 객체는 이미
internal로 선언되어 있지만,providesNavigationState()함수는 public입니다. 이전 리뷰 코멘트에서 언급된 것처럼, DI provider 함수를internal로 선언하면 모듈 외부에서의 직접 접근을 방지하여 캡슐화를 강화할 수 있습니다.🔎 제안하는 수정
@Provides - fun providesNavigationState(): NavigationState { + internal fun providesNavigationState(): NavigationState { return NavigationState( startKey = START_NAV_KEY, topLevelKeys = TOP_LEVEL_NAV_KEYS.toSet(), ) }
19-19: toSet() 변환이 매번 수행됩니다.
TOP_LEVEL_NAV_KEYS.toSet()이 provider 함수 내부에서 호출되므로, 스코프가 추가되더라도 초기화 시 변환이 발생합니다. 만약TOP_LEVEL_NAV_KEYS가 이미 List라면, Keys.kt에서 미리 Set으로 정의하면 약간의 성능 개선이 가능합니다.core/navigation/src/main/java/com/neki/android/core/navigation/NavigatorImpl.kt (1)
7-10: 생성자 스타일을 통일하는 것을 고려하세요.이전 리뷰 코멘트에서 언급된 것처럼, 생성자 주입 시 파라미터를 개행하여 가독성을 높이는 것이 권장됩니다.
🔎 제안하는 코드 스타일
@ActivityRetainedScoped -class NavigatorImpl @Inject constructor( - val state: NavigationState, -) : Navigator { +class NavigatorImpl @Inject constructor( + val state: NavigationState +): Navigator {또는:
@ActivityRetainedScoped class NavigatorImpl @Inject constructor( - val state: NavigationState, + val state: NavigationState ) : Navigator {
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/neki/android/app/navigation/di/AppModule.ktapp/src/main/java/com/neki/android/app/navigation/di/NavigationModule.ktcore/navigation/src/main/java/com/neki/android/core/navigation/NavigatorImpl.ktfeature/archive/impl/build.gradle.ktsfeature/map/impl/build.gradle.ktsfeature/pose/impl/build.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/neki/android/app/navigation/di/AppModule.kt
- feature/archive/impl/build.gradle.kts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (9)
feature/map/impl/build.gradle.kts (2)
1-7: 플러그인과 네임스페이스 설정이 올바릅니다.
neki.android.feature.impl플러그인 사용과 네임스페이스 명명이 feature 모듈의 api/impl 분리 컨벤션에 정확히 부합합니다. 버전 카탈로그 alias 사용도 좋습니다.
9-11: 의존성 설정이 올바릅니다.api 모듈에 대한 의존성이 올바르게 선언되었고, type-safe project accessor(
projects.feature.map.api) 사용은 모범 사례입니다. 컨벤션 플러그인이 Hilt, Compose 등 공통 의존성을 제공할 것으로 예상됩니다.feature/pose/impl/build.gradle.kts (3)
5-7: 네임스페이스가 명명 규칙을 올바르게 따르고 있습니다.
com.neki.android.feature.pose.impl네임스페이스는 api/impl 분리 구조의 명명 규칙을 정확히 따르고 있습니다.
9-11: 의존성 선언이 올바릅니다.
pose.api모듈에 대한 의존성이 정확하게 선언되어 있으며, type-safe project accessor를 사용하여 컴파일 타임 안전성을 확보하고 있습니다. 컨벤션 플러그인이 Compose, Hilt 등의 공통 의존성을 제공하므로 이 선언만으로 충분합니다.
1-3: 컨벤션 플러그인 적용이 올바릅니다.새로 생성된
AndroidFeatureImplConventionPlugin을 올바르게 사용하고 있습니다. 컨벤션 플러그인은neki.android.library.compose와neki.hilt를 적용하고, Compose, Hilt, Navigation3 등 필요한 의존성을 제공합니다.core/navigation/src/main/java/com/neki/android/core/navigation/NavigatorImpl.kt (4)
12-18: navigate() 로직을 확인했습니다.현재 key에 따라 clearSubStack, goToTopLevel, goToKey를 호출하는 분기 로직이 명확합니다. 각 케이스가 적절히 처리되고 있습니다.
31-36: goToKey() 로직을 확인했습니다.키를 제거한 후 다시 추가하여 스택의 끝으로 이동시키는 로직이 명확합니다. 중복 방지 및 순서 관리가 적절히 구현되어 있습니다.
38-44: goToTopLevel() 로직을 확인했습니다.
startKey인 경우 스택을 초기화하고, 그렇지 않으면 기존 키를 제거한 후 추가하는 로직이 명확합니다. Top-level 네비게이션의 의도가 잘 반영되어 있습니다.
46-50: clearSubStack() 로직을 확인했습니다.
subList(1, size).clear()를 사용하여 첫 번째 요소만 남기고 나머지를 제거하는 구현이 효율적이고 명확합니다. 현재 top-level key로 이동 시 sub-stack을 초기화하는 의도가 잘 구현되어 있습니다.
2 participants
🔗 관련 이슈
📙 작업 설명
컨벤션 플러그인
feature모듈을api,impl로 분리함에 맞춰서 컨벤션 플러그인 또한AndroidFeatureApiConventionPlugin,AndroidFeatureImplConventionPlugin로 분리했습니다.의존성
NavigationStateNavKeyNavKey의Substack들TopLevelKey등을 가지고 있는 클래스입니다.NavigatorEntryProvider를Hilt로 주입하고 있고,Navigator의navigate함수와goBack함수를 사용해야 하기 때문에 해당EntryProvider에 주입하기 위해서 인터페이스와 구현체로 나누었고,app모듈에서 의존성을 주입합니다.바텀 탭 모듈 생성 (Pose, Archive, Map)
바텀 네비게이션 바 임시 구현
💬 추가 설명 or 리뷰 포인트 (선택)
Screen에서 전달받고 뷰모델로 넘기는 것 대신에,entry Scope에서 바로ViewModel로 전달하는 것을 사용하려면Sample모듈을 확인하면 될 것 같습니다!viewModelStoreOwner을nav3에서도 적용이 가능한 지 확인해봐야할 것 같습니다.Summary by CodeRabbit
새로운 기능
UI/UX 개선
✏️ Tip: You can customize this high-level summary in your review settings.