[NDGL-10] Navigation3 연동 #4
Conversation
둘러보기새로운 디자인 시스템 모듈을 도입하고, AndroidX Navigation 3 기반의 계층적 네비게이션 시스템을 구현하며, 하단 네비게이션 바와 함께 여행 도구, 홈, 여행 기능 모듈을 추가합니다. 패키지 네임스페이스를 간소화하고 의존성을 업데이트합니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant User as 사용자
participant NDGLApp as NDGLApp<br/>(루트)
participant Navigator as Navigator
participant NavigationState as NavigationState
participant EntryProvider as EntryProvider
participant NavDisplay as NavDisplay
User->>NDGLApp: 앱 시작
activate NDGLApp
NDGLApp->>NavigationState: rememberNavigationState<br/>(Home 시작)
activate NavigationState
NavigationState-->>NDGLApp: 상태 반환<br/>(currentKey=Home)
deactivate NavigationState
NDGLApp->>Navigator: Navigator 생성<br/>(NavigationState 포함)
activate Navigator
Navigator-->>NDGLApp: 준비 완료
deactivate Navigator
NDGLApp->>NavDisplay: 현재 경로 표시<br/>(Home)
activate NavDisplay
NavDisplay->>EntryProvider: Home 엔트리 요청
activate EntryProvider
EntryProvider-->>NavDisplay: HomeRoute 반환
deactivate EntryProvider
NavDisplay-->>NDGLApp: Home 화면 렌더링
deactivate NavDisplay
NDGLApp-->>User: UI 표시<br/>(Home + BottomBar)
deactivate NDGLApp
User->>NDGLApp: Travel 탭 클릭
activate NDGLApp
NDGLApp->>Navigator: navigate(Route.Travel)
activate Navigator
Navigator->>NavigationState: currentTopLevelKey<br/>변경(Travel)
activate NavigationState
NavigationState-->>Navigator: 업데이트 완료
deactivate NavigationState
Navigator-->>NDGLApp: 완료
deactivate Navigator
NDGLApp->>NavDisplay: 현재 경로 표시<br/>(Travel)
activate NavDisplay
NavDisplay->>EntryProvider: Travel 엔트리 요청
activate EntryProvider
EntryProvider-->>NavDisplay: TravelRoute 반환
deactivate EntryProvider
NavDisplay-->>NDGLApp: Travel 화면 렌더링
deactivate NavDisplay
NDGLApp-->>User: Travel 화면 업데이트
deactivate NDGLApp
User->>NDGLApp: Travel 상세 이동
activate NDGLApp
NDGLApp->>Navigator: navigate(Route.TravelDetail)
activate Navigator
Navigator->>NavigationState: currentSubStack에 추가
activate NavigationState
NavigationState-->>Navigator: 업데이트 완료
deactivate NavigationState
Navigator-->>NDGLApp: 완료
deactivate Navigator
NDGLApp->>NavDisplay: 현재 경로 표시<br/>(TravelDetail)
activate NavDisplay
NavDisplay->>EntryProvider: TravelDetail 엔트리 요청
activate EntryProvider
EntryProvider-->>NavDisplay: TravelDetailRoute 반환
deactivate EntryProvider
NavDisplay-->>NDGLApp: TravelDetail 화면 렌더링
deactivate NavDisplay
NDGLApp-->>User: TravelDetail 화면 표시<br/>(BottomBar 숨김)
deactivate NDGLApp
예상 코드 리뷰 노력🎯 4 (Complex) | ⏱️ ~60분 시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 7
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/yapp/ndgl/ui/NdglApp.kt`:
- Around line 41-42: NdglApp receives a modifier parameter but ignores it;
update the Scaffold call in NdglApp to use the passed modifier instead of
creating a new one (e.g., pass modifier = modifier.fillMaxSize() or modifier =
modifier.then(Modifier.fillMaxSize()) if you need to combine), ensuring the
incoming Modifier is honored; locate the NdglApp composable and replace the
hardcoded Modifier.fillMaxSize() used in the Scaffold with the passed-in
modifier (merged as needed).
In `@core/designsystem/src/main/java/com/yapp/designsystem/theme/Type.kt`:
- Line 8: Remove the erroneous self-import of
com.yapp.designsystem.theme.Typography from Type.kt; keep only the external
androidx.compose.material3.Typography import so the local Typography variable
(defined in the same package) is not shadowed or causing a self-import compile
error.
In `@feature/auth/src/main/java/com/yapp/auth/AuthScreen.kt`:
- Line 1: AuthScreen.kt declares package com.yapp.auth but uses AuthViewModel
from com.yapp.ndgl.feature.auth causing package inconsistency; fix by making
package declarations consistent across the feature (either change AuthScreen.kt
package to com.yapp.ndgl.feature.auth or move AuthViewModel and related files to
com.yapp.auth), update any imports/usages accordingly (references to
AuthViewModel, related composables, and resource paths), and run a build/IDE
sync to ensure no unresolved references remain.
In `@feature/home/src/main/java/com/yapp/home/navigation/HomeEntry.kt`:
- Around line 9-13: The function homeEntry has an unused navigator parameter;
remove the unused parameter from the signature by changing
EntryProviderScope<NavKey>.homeEntry(navigator: Navigator) to
EntryProviderScope<NavKey>.homeEntry() and update all call sites to stop passing
a Navigator (or, if you cannot update callers immediately, rename the parameter
to _navigator to mark it intentionally unused), ensuring consistency with other
entry functions like TravelEntry.kt that use navigator when needed.
In `@feature/travel/src/main/java/com/yapp/travel/detail/TravelDetailScreen.kt`:
- Line 13: The import for hiltViewModel is using the deprecated package; replace
the import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel with the
non-deprecated androidx.hilt.navigation.compose.hiltViewModel so the
TravelDetailScreen (and any usage of hiltViewModel in this file) uses the
correct Hilt Compose import.
In `@navigation/src/main/java/com/yapp/ndgl/navigation/NavigationState.kt`:
- Around line 39-49: currentTopLevelKey and currentKey call last() inside
derivedStateOf which can throw if topLevelStack or a sub-stack becomes empty;
change these to use lastOrNull() and make their types nullable (e.g., val
currentTopLevelKey: NavKey? by derivedStateOf { topLevelStack.lastOrNull() } and
val currentKey: NavKey? by derivedStateOf { currentSubStack.lastOrNull() }) or
alternatively add defensive logic in goBack() (the method that mutates
topLevelStack/subStacks created by rememberNavBackStack(startRoute)) to never
remove the final entry; update any callers to handle nullable NavKey if you
choose the lastOrNull approach.
In `@navigation/src/main/java/com/yapp/ndgl/navigation/Navigator.kt`:
- Around line 15-23: The goBack() implementation can crash when topLevelStack
becomes empty because state.currentKey reads currentSubStack.last() which relies
on currentTopLevelKey (topLevelStack.last()); change goBack() to guard pops:
before calling state.topLevelStack.removeLastOrNull() ensure
state.topLevelStack.size > 1 (or handle the single-item case explicitly by
returning/triggering app-exit behavior), and likewise avoid assumptions about
currentSubStack.last() by using safe access (lastOrNull) or early-return when
the relevant stack is empty; update the logic around state.currentTopLevelKey,
state.topLevelStack, and state.currentSubStack to prevent accessing .last()
after a pop.
🧹 Nitpick comments (17)
feature/auth/src/main/java/com/yapp/auth/AuthViewModel.kt (1)
4-11: 사용되지 않는 import 제거 필요
viewModelScope,MutableSharedFlow,MutableStateFlow,StateFlow,asSharedFlow,asStateFlow,launch등의 import가 선언되어 있지만 현재 빈 클래스에서 사용되지 않습니다. 향후 구현 시 필요한 import만 추가하는 것이 좋습니다.♻️ 사용되지 않는 import 제거
package com.yapp.ndgl.feature.auth import androidx.lifecycle.ViewModel -import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asSharedFlow -import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.launch import javax.inject.Injectfeature/auth/src/main/java/com/yapp/auth/AuthScreen.kt (2)
15-21: viewModel 파라미터 미사용
AuthRoute에서viewModel파라미터를 받고 있지만AuthScreen에 전달하거나 사용하지 않습니다. 임시 화면이라도 ViewModel 연동 패턴을 올바르게 설정하는 것이 좋습니다.♻️ ViewModel 상태 연동 패턴 예시
`@Composable` fun AuthRoute( viewModel: AuthViewModel = hiltViewModel(), ) { - - AuthScreen() + // TODO: ViewModel의 상태를 수집하여 AuthScreen에 전달 + // val uiState by viewModel.uiState.collectAsStateWithLifecycle() + AuthScreen( + // uiState = uiState, + // onEvent = viewModel::onEvent + ) }
37-41: Preview 함수 가시성Preview 함수는 IDE 도구용이므로
private으로 선언하는 것이 일반적인 관례입니다.♻️ Preview 가시성 수정
`@Preview`(showBackground = true) `@Composable` -fun AuthScreenPreview() { +private fun AuthScreenPreview() { AuthScreen() }feature/travel/src/main/java/com/yapp/travel/travel/TravelScreen.kt (1)
15-23: 사용되지 않는viewModel파라미터
viewModel파라미터가 선언되었지만TravelScreen에 전달되지 않고 있습니다. 현재는 테스트용 임시 화면이므로 의도적일 수 있지만, 추후 실제 구현 시 ViewModel의 상태를 Screen에 전달하도록 수정이 필요합니다.feature/travel-helper/src/main/java/com/yapp/helper/TravelHelperViewModel.kt (1)
7-10: 빈 ViewModel 클래스에 TODO 주석 추가를 권장합니다.PR 설명에 따르면 임시 화면용으로 생성된 것으로 보입니다. 향후 구현이 필요함을 명시하는 TODO 주석을 추가하면 코드베이스의 의도가 더 명확해집니다.
♻️ 제안된 변경사항
`@HiltViewModel` -class TravelHelperViewModel `@Inject` constructor() : ViewModel() { - +class TravelHelperViewModel `@Inject` constructor() : ViewModel() { + // TODO: Implement travel helper business logic }feature/travel-helper/src/main/java/com/yapp/helper/TravelHelperScreen.kt (4)
12-12: Deprecated import 사용 중
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel은 deprecated되었습니다.androidx.hilt.navigation.compose.hiltViewModel을 사용하세요.♻️ 권장 수정 사항
-import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel +import androidx.hilt.navigation.compose.hiltViewModel
11-11: 사용되지 않는 import
dpimport가 선언되어 있지만 코드에서 사용되지 않습니다.♻️ 권장 수정 사항
-import androidx.compose.ui.unit.dp
15-20: 사용되지 않는 viewModel 파라미터
viewModel이 주입되었지만TravelHelperScreen()에 전달되지 않고 사용되지 않습니다. 테스트용 임시 화면이라면 이해되지만, 향후 ViewModel의 상태나 이벤트를 Screen에 전달하는 구조로 발전시켜야 합니다.
36-39: Preview 함수 가시성 불일치
TravelHelperRoute와TravelHelperScreen은internal로 선언되어 있지만,TravelHelperScreenPreview는public입니다. 일관성을 위해private로 변경하는 것을 권장합니다.♻️ 권장 수정 사항
`@Preview`(showBackground = true) `@Composable` -fun TravelHelperScreenPreview() { +private fun TravelHelperScreenPreview() { TravelHelperScreen() }app/src/main/java/com/yapp/ndgl/ui/NdglNavigationBar.kt (1)
14-16: 코드 포맷팅 일관성파라미터 선언에서 콜론 주변 공백이 일관되지 않습니다. Kotlin 컨벤션에 따라 콜론 앞에는 공백 없이, 뒤에만 공백을 두는 것을 권장합니다.
♻️ 권장 수정 사항
`@Composable` internal fun NdglNavigationBar( - currentTab : NavKey, - onTabSelected : (Route) -> Unit, + currentTab: NavKey, + onTabSelected: (Route) -> Unit, ) {app/src/main/java/com/yapp/ndgl/navigation/TopLevelRoute.kt (1)
12-26: 하드코딩된 문자열을 string resource로 변경 검토현재
label이 하드코딩된 한국어 문자열입니다. 향후 다국어 지원을 고려하여 string resource 사용을 권장합니다.♻️ 제안된 변경
enum class TopLevelRoute( `@DrawableRes` val icon: Int, - val label: String, + `@StringRes` val labelRes: Int, val navKey: NavKey, ) { TRAVEL_HELPER( icon = R.drawable.ic_nav_helper, - label = "여행 도구", + labelRes = R.string.nav_travel_helper, navKey = Route.TravelHelper, ), // ... 나머지 항목도 동일하게 변경 }app/src/main/java/com/yapp/ndgl/ui/NdglApp.kt (1)
44-51: 사용되지 않는 애니메이션 import 또는 누락된 애니메이션 적용
slideInVertically와slideOutVertically가 import되었지만AnimatedVisibility에서 사용되지 않습니다. 의도적으로 기본 애니메이션(fade)을 사용하려면 import를 제거하고, bottom bar에 슬라이드 애니메이션을 적용하려면 명시적으로 지정하세요.♻️ 슬라이드 애니메이션 적용 예시
AnimatedVisibility( visible = shouldShowBottomBar, + enter = slideInVertically(initialOffsetY = { it }), + exit = slideOutVertically(targetOffsetY = { it }), ) {feature/home/src/main/java/com/yapp/home/HomeScreen.kt (2)
14-20: 사용되지 않는 viewModel과 import
viewModel파라미터가 주입되지만HomeScreen에 전달되거나 사용되지 않습니다. 또한dpimport(line 11)도 사용되지 않습니다. 임시 화면이므로 향후 구현 시 정리하면 됩니다.
36-40: Preview 함수 가시성
HomeScreenPreview가public으로 선언되어 있습니다. 다른 composable들과 일관성을 위해private으로 변경을 고려하세요.♻️ 수정 제안
`@Preview`(showBackground = true) `@Composable` -fun HomeScreenPreview() { +private fun HomeScreenPreview() { HomeScreen() }navigation/src/main/java/com/yapp/ndgl/navigation/NavigationState.kt (2)
17-32:remember키와 상태 생성 위치 불일치
topLevelStack과subStacks가remember블록 외부에서 생성되지만,remember의 키로startRoute와topLevelKeys가 사용됩니다.topLevelKeys가 변경되면 새로운NavigationState가 생성되지만,rememberNavBackStack으로 생성된 기존 스택 인스턴스들이 재사용될 수 있어 불일치가 발생할 수 있습니다.
topLevelKeys가 런타임에 변경되지 않는다면 문제없지만, 향후 확장성을 위해 명확히 문서화하거나 방어 로직을 추가하는 것이 좋겠습니다.
67-70:toMutableStateList()재생성 시 최적화 검토 필요
toEntries()가 호출될 때마다 새로운SnapshotStateList가 생성됩니다. 현재NdglApp컴포저블에서 래핑된remember없이 직접 호출되고 있어, 리컴포지션마다 새로운 리스트 객체가 생성될 수 있습니다.다만
rememberDecoratedNavEntries()등 내부 컴포저블 함수 호출로 인해 전체 로직을remember로 감싸기는 어려운 상황입니다. 호출 측에서remember로 감싸거나, 또는topLevelStack변경 시에만 재계산되도록 의존성을 명시적으로 관리하는 것을 권장합니다.gradle/libs.versions.toml (1)
64-64: Compose Foundation을 BOM으로 관리하는 것을 권장
androidx-compose-foundation이 BOM과 별도로 버전1.10.1로 고정되어 있습니다. 다른 Compose 라이브러리는 BOM으로 관리되므로 버전 불일치가 발생할 수 있습니다.BOM에서 관리하도록 변경하거나, 별도 버전이 필요한 이유가 있다면 주석으로 문서화해 주세요.
♻️ BOM으로 관리하는 예시
-androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation-layout", version.ref = "androidxComposeFoundation" } +androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation-layout" }그리고
androidxComposeFoundation버전 참조를 제거합니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
app/build.gradle.ktsapp/src/main/java/com/yapp/ndgl/MainActivity.ktapp/src/main/java/com/yapp/ndgl/navigation/TopLevelRoute.ktapp/src/main/java/com/yapp/ndgl/ui/NdglApp.ktapp/src/main/java/com/yapp/ndgl/ui/NdglNavigationBar.ktbuild-logic/src/main/kotlin/NDGLFeaturePlugin.ktbuild-logic/src/main/kotlin/NDGLKotlinLibraryPlugin.ktbuild-logic/src/main/kotlin/convention/Compose.ktbuild-logic/src/main/kotlin/extensions/ProjectExtensions.ktcore/designsystem/.gitignorecore/designsystem/build.gradle.ktscore/designsystem/src/main/AndroidManifest.xmlcore/designsystem/src/main/java/com/yapp/designsystem/theme/Color.ktcore/designsystem/src/main/java/com/yapp/designsystem/theme/Theme.ktcore/designsystem/src/main/java/com/yapp/designsystem/theme/Type.ktcore/designsystem/src/main/res/drawable/ic_nav_helper.xmlcore/designsystem/src/main/res/drawable/ic_nav_home.xmlcore/designsystem/src/main/res/drawable/ic_nav_travel.xmlfeature/auth/src/main/java/com/yapp/auth/AuthScreen.ktfeature/auth/src/main/java/com/yapp/auth/AuthViewModel.ktfeature/home/src/main/java/com/yapp/home/HomeScreen.ktfeature/home/src/main/java/com/yapp/home/HomeViewModel.ktfeature/home/src/main/java/com/yapp/home/navigation/HomeEntry.ktfeature/travel-helper/src/main/java/com/yapp/helper/TravelHelperScreen.ktfeature/travel-helper/src/main/java/com/yapp/helper/TravelHelperViewModel.ktfeature/travel-helper/src/main/java/com/yapp/helper/navigation/TravelHelperEntry.ktfeature/travel/src/main/java/com/yapp/travel/detail/TravelDetailScreen.ktfeature/travel/src/main/java/com/yapp/travel/detail/TravelDetailViewModel.ktfeature/travel/src/main/java/com/yapp/travel/navigation/TravelEntry.ktfeature/travel/src/main/java/com/yapp/travel/travel/TravelScreen.ktfeature/travel/src/main/java/com/yapp/travel/travel/TravelViewModel.ktgradle.propertiesgradle/libs.versions.tomlnavigation/build.gradle.ktsnavigation/src/main/AndroidManifest.xmlnavigation/src/main/java/com/yapp/ndgl/navigation/NavigationState.ktnavigation/src/main/java/com/yapp/ndgl/navigation/Navigator.ktnavigation/src/main/java/com/yapp/ndgl/navigation/Route.ktsettings.gradle.kts
💤 Files with no reviewable changes (4)
- build-logic/src/main/kotlin/NDGLKotlinLibraryPlugin.kt
- build-logic/src/main/kotlin/extensions/ProjectExtensions.kt
- navigation/src/main/AndroidManifest.xml
- gradle.properties
🧰 Additional context used
🧬 Code graph analysis (4)
app/src/main/java/com/yapp/ndgl/ui/NdglApp.kt (5)
navigation/src/main/java/com/yapp/ndgl/navigation/NavigationState.kt (1)
rememberNavigationState(17-32)feature/home/src/main/java/com/yapp/home/navigation/HomeEntry.kt (1)
homeEntry(9-13)feature/travel/src/main/java/com/yapp/travel/navigation/TravelEntry.kt (1)
travelEntry(12-24)feature/travel-helper/src/main/java/com/yapp/helper/navigation/TravelHelperEntry.kt (1)
travelHelperEntry(9-13)app/src/main/java/com/yapp/ndgl/ui/NdglNavigationBar.kt (1)
NdglNavigationBar(13-33)
feature/home/src/main/java/com/yapp/home/navigation/HomeEntry.kt (1)
feature/home/src/main/java/com/yapp/home/HomeScreen.kt (1)
HomeRoute(14-20)
feature/travel/src/main/java/com/yapp/travel/navigation/TravelEntry.kt (2)
feature/travel/src/main/java/com/yapp/travel/travel/TravelScreen.kt (1)
TravelRoute(14-23)feature/travel/src/main/java/com/yapp/travel/detail/TravelDetailScreen.kt (1)
TravelDetailRoute(16-23)
feature/travel-helper/src/main/java/com/yapp/helper/navigation/TravelHelperEntry.kt (1)
feature/travel-helper/src/main/java/com/yapp/helper/TravelHelperScreen.kt (1)
TravelHelperRoute(14-20)
🪛 detekt (1.23.8)
feature/travel/src/main/java/com/yapp/travel/travel/TravelViewModel.kt
[warning] 8-10: The class or object TravelViewModel is empty.
(detekt.empty-blocks.EmptyClassBlock)
feature/travel-helper/src/main/java/com/yapp/helper/TravelHelperViewModel.kt
[warning] 8-10: The class or object TravelHelperViewModel is empty.
(detekt.empty-blocks.EmptyClassBlock)
🔇 Additional comments (33)
feature/auth/src/main/java/com/yapp/auth/AuthViewModel.kt (1)
14-17: Hilt ViewModel 기본 구조 적절함
@HiltViewModel과@Inject constructor()패턴이 올바르게 적용되었습니다. 임시 테스트용 빈 ViewModel로서 적절합니다.feature/auth/src/main/java/com/yapp/auth/AuthScreen.kt (1)
11-12: 불필요한 import 제거
dpimport가 사용되지 않습니다. 제거 필요합니다.androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel은 현재 권장되는 올바른 import 경로입니다 (androidx.hilt.navigation.compose는 deprecated됨).♻️ import 수정
import androidx.compose.ui.tooling.preview.Preview -import androidx.compose.ui.unit.dp import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import com.yapp.ndgl.feature.auth.AuthViewModelLikely an incorrect or invalid review comment.
core/designsystem/src/main/AndroidManifest.xml (1)
1-4: LGTM!Android 라이브러리 모듈에 적합한 최소 manifest 구성입니다.
namespace가build.gradle.kts에서 정의되어 있어 별도의package속성이 필요하지 않습니다.core/designsystem/src/main/java/com/yapp/designsystem/theme/Color.kt (1)
1-1: 패키지명과 namespace 불일치 확인 필요.패키지명
com.yapp.designsystem.theme이build.gradle.kts의 namespace인com.yapp.ndgl.core.designsystem과 일치하지 않습니다. 기술적으로 동작에는 문제없지만, 일관성을 위해 패키지명을com.yapp.ndgl.core.designsystem.theme으로 변경하거나 의도적인 설계인지 확인이 필요합니다.build-logic/src/main/kotlin/NDGLFeaturePlugin.kt (2)
29-29: LGTM!Feature 모듈에 design system 의존성을 추가하여 일관된 UI 컴포넌트 사용이 가능합니다.
36-36: Navigation3 runtime 버전 확인 필요.androidx.navigation3은 2025년 11월 19일에 stable 버전 1.0.0이 정식 릴리스되었습니다.
libs.versions.toml에 정의된 Navigation3 버전이 1.0.0 이상의 stable 버전인지 확인하시기 바랍니다.build-logic/src/main/kotlin/convention/Compose.kt (1)
23-23: LGTM!
androidx-compose-foundation은 레이아웃 컴포넌트(LazyColumn, Box 등)와 제스처 처리를 위해 필수적인 의존성입니다. Navigation UI 구현에 적절한 추가입니다.core/designsystem/build.gradle.kts (1)
1-7: Compose 의존성은 이미 플러그인을 통해 구성되어 있습니다.
ndgl.android.library플러그인이configureComposeAndroid()를 호출하여 Compose를 자동으로 설정합니다. 이 함수는androidx-compose-bom,androidx-compose-material3,androidx-compose-foundation,androidx-compose-ui등 필요한 모든 Compose 의존성을 추가하므로, build.gradle.kts에 별도의 Compose 의존성을 추가할 필요가 없습니다.Likely an incorrect or invalid review comment.
core/designsystem/src/main/res/drawable/ic_nav_home.xml (1)
1-10: LGTM!홈 네비게이션 아이콘이 올바르게 정의되었습니다. 다른 네비게이션 아이콘들(ic_nav_travel, ic_nav_helper)과 동일한 fillColor(
#2C2C2C)를 사용하여 일관성이 유지됩니다.core/designsystem/src/main/res/drawable/ic_nav_travel.xml (1)
1-10: LGTM!여행 네비게이션 아이콘이 올바르게 정의되었습니다. 디자인 시스템 모듈 내 다른 아이콘들과 일관된 스타일을 유지하고 있습니다.
core/designsystem/src/main/java/com/yapp/designsystem/theme/Theme.kt (1)
1-57: LGTM!패키지 네임스페이스가
com.yapp.designsystem.theme으로 올바르게 변경되었으며, Material 3 테마 구성이 표준 패턴을 따르고 있습니다. Dynamic color 지원(Android 12+)도 적절하게 구현되어 있습니다.core/designsystem/src/main/res/drawable/ic_nav_helper.xml (1)
1-19: LGTM!헬퍼 네비게이션 아이콘이 올바르게 정의되었습니다. 여러 path 요소를 사용한 복잡한 아이콘 형태가 적절히 구현되어 있으며, 다른 네비게이션 아이콘들과 일관된 색상(
#2C2C2C)을 사용하고 있습니다.feature/home/src/main/java/com/yapp/home/HomeViewModel.kt (1)
7-10: LGTM!Hilt ViewModel 설정이 올바르게 되어 있습니다. Navigation3 연동을 위한 초기 스캐폴드로 적절합니다.
feature/travel/src/main/java/com/yapp/travel/travel/TravelViewModel.kt (1)
7-10: LGTM!Navigation3 연동을 위한 초기 스캐폴드 ViewModel입니다. detekt의 empty class 경고는 의도된 플레이스홀더이므로 무시해도 됩니다.
feature/travel/src/main/java/com/yapp/travel/detail/TravelDetailScreen.kt (1)
16-23: LGTM!
collectAsStateWithLifecycle을 사용하여 lifecycle-aware하게 StateFlow를 구독하고 있습니다. Route/Screen 분리 패턴도 올바르게 적용되었습니다.feature/travel/src/main/java/com/yapp/travel/detail/TravelDetailViewModel.kt (1)
12-24: LGTM! Assisted Injection 패턴이 올바르게 구현되었습니다.
@HiltViewModel(assistedFactory = ...)와@AssistedInject를 사용하여 navigation argument를 ViewModel에 전달하는 구현이 적절합니다. 프로젝트의 Hilt 버전은 2.57.2로 assistedFactory를 지원하는 2.49+ 버전 요구사항을 충족합니다.feature/travel/src/main/java/com/yapp/travel/travel/TravelScreen.kt (1)
12-12: 이 import은 이미 올바른 경로입니다.
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel은 현재의 올바른 import이며 deprecated되지 않았습니다. 오히려 deprecated된 경로는androidx.hilt.navigation.compose.hiltViewModel입니다. 현재 코드는 이미 최신의 권장 import을 사용 중이므로 변경이 필요 없습니다.Likely an incorrect or invalid review comment.
feature/travel/src/main/java/com/yapp/travel/navigation/TravelEntry.kt (1)
12-24: LGTM! Navigation entry 설정이 잘 구현되었습니다.Navigation3와 Hilt assisted injection 패턴을 적절히 활용하여
travelId를 ViewModel에 전달하고 있습니다. Route별 entry 구분과 콜백 기반 네비게이션 연결이 깔끔합니다.core/designsystem/.gitignore (1)
1-1: LGTM!Gradle 모듈의 표준
.gitignore패턴입니다.settings.gradle.kts (1)
30-30: LGTM!
core:designsystem모듈이 기존 core 모듈들과 일관된 위치에 추가되었습니다.app/build.gradle.kts (1)
18-22: Navigation3 의존성 추가가 적절합니다.
core:designsystem모듈과 Navigation3 관련 라이브러리들이 올바르게 추가되었습니다. Navigation3는 androidx.navigation3:1.0.0으로 안정 버전이 출시되었으므로 프로덕션 환경에서 사용하기에 적합합니다.app/src/main/java/com/yapp/ndgl/MainActivity.kt (1)
11-22: LGTM!Hilt 설정, edge-to-edge 활성화, 테마 적용이 올바르게 구성되어 있습니다.
NdglApp()을 통한 UI 진입점 구조가 깔끔합니다.app/src/main/java/com/yapp/ndgl/ui/NdglNavigationBar.kt (1)
22-22: 캐스팅이 불필요합니다.Route는NavKey인터페이스를 구현하는 sealed interface이고,TopLevelRoute의 모든 enum 값은Route인스턴스만을navKey에 할당합니다.navKey필드를NavKey타입 대신Route타입으로 선언하면 캐스팅을 제거할 수 있습니다.navigation/build.gradle.kts (1)
10-13: Navigation3 의존성이 올바르게 설정되어 있습니다androidx.navigation3.runtime의 안정 버전(1.0.0)이 출시되었으므로 프로덕션 환경에서 안심하고 사용할 수 있습니다.
feature/travel-helper/src/main/java/com/yapp/helper/navigation/TravelHelperEntry.kt (1)
9-13: LGTM - 일관된 네비게이션 엔트리 패턴입니다.
navigator파라미터가 현재는 사용되지 않지만, HomeEntry와 동일한 시그니처를 유지하여 향후 중첩 네비게이션 추가 시 일관성을 보장합니다. TravelEntry처럼 상세 화면 네비게이션이 필요할 때 활용할 수 있습니다.app/src/main/java/com/yapp/ndgl/ui/NdglApp.kt (1)
23-62: 전반적인 네비게이션 구조가 잘 설계되었습니다.Navigation3 패턴을 적절히 활용하여 top-level/nested 네비게이션을 구현했습니다.
rememberNavigationState,Navigator,entryProvider조합이 명확하고 확장 가능한 구조입니다.navigation/src/main/java/com/yapp/ndgl/navigation/Route.kt (2)
6-22: 잘 설계된 타입 안전 네비게이션 라우트 구조입니다.sealed interface와
@Serializable어노테이션을 활용하여 타입 안전하고 직렬화 가능한 라우트 시스템을 구현했습니다. Navigation3의NavKey를 확장하여 프레임워크와 잘 통합됩니다.
8-9:Route.Auth가 정의되었지만 현재 등록되지 않음
Auth라우트가 정의되어 있지만NdglApp의entryProvider에 등록되지 않았습니다. 향후 인증 플로우 구현 시 추가될 예정인지 확인하세요.feature/home/src/main/java/com/yapp/home/HomeScreen.kt (1)
12-12: 문제 없음 - 올바른 import 경로 사용 중
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel은 현재 권장되는 올바른 import 경로입니다.androidx.hilt.navigation.compose.hiltViewModel은 더 이상 사용되지 않는 경로입니다.Likely an incorrect or invalid review comment.
navigation/src/main/java/com/yapp/ndgl/navigation/Navigator.kt (2)
25-30: LGTM!
remove()와add()를 조합한 "bring to front" 패턴이 적절하게 구현되어 있습니다. 기존에 없는 키도 정상적으로 추가됩니다.
39-43: LGTM!
subList(1, size).clear()패턴으로 첫 번째 요소(root)를 유지하면서 나머지를 제거하는 로직이 올바르게 구현되어 있습니다.gradle/libs.versions.toml (2)
113-117: LGTM!테스트 관련 의존성(JUnit, Espresso, AppCompat, Material)이 적절한 버전으로 추가되었습니다.
29-29: Navigation3 1.0.0은 2025년 11월 19일에 공식 안정 버전으로 출시되었으므로 안정성 측면에서는 문제가 없습니다. 다만 최근에 출시된 라이브러리이므로 향후 버전 업데이트 시 변경사항을 주시하기 바랍니다.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
core/designsystem/src/main/java/com/yapp/designsystem/theme/Type.kt
Outdated
Show resolved
Hide resolved
feature/travel/src/main/java/com/yapp/travel/detail/TravelDetailScreen.kt
Show resolved
Hide resolved
| val currentTopLevelKey: NavKey by derivedStateOf { topLevelStack.last() } | ||
|
|
||
| val topLevelKeys | ||
| get() = subStacks.keys | ||
|
|
||
| val currentSubStack: NavBackStack<NavKey> | ||
| get() = subStacks[currentTopLevelKey] | ||
| ?: error("Sub stack for $currentTopLevelKey does not exist") | ||
|
|
||
| val currentKey: NavKey by derivedStateOf { currentSubStack.last() } | ||
| } |
There was a problem hiding this comment.
빈 스택에서 last() 호출 시 예외 가능성
currentTopLevelKey와 currentKey가 derivedStateOf { ...last() }를 사용하는데, 스택이 비어있으면 NoSuchElementException이 발생합니다. rememberNavBackStack(startRoute)로 초기화되므로 일반적으로는 비어있지 않겠지만, goBack()으로 모든 항목이 제거될 경우 크래시가 발생할 수 있습니다.
lastOrNull()과 nullable 타입 사용, 또는 스택이 비지 않도록 goBack()에서 방어 로직 추가를 고려해 주세요.
🛠️ 방어적 접근 예시
- val currentTopLevelKey: NavKey by derivedStateOf { topLevelStack.last() }
+ val currentTopLevelKey: NavKey by derivedStateOf { topLevelStack.lastOrNull() ?: startRoute }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val currentTopLevelKey: NavKey by derivedStateOf { topLevelStack.last() } | |
| val topLevelKeys | |
| get() = subStacks.keys | |
| val currentSubStack: NavBackStack<NavKey> | |
| get() = subStacks[currentTopLevelKey] | |
| ?: error("Sub stack for $currentTopLevelKey does not exist") | |
| val currentKey: NavKey by derivedStateOf { currentSubStack.last() } | |
| } | |
| val currentTopLevelKey: NavKey by derivedStateOf { topLevelStack.lastOrNull() ?: startRoute } | |
| val topLevelKeys | |
| get() = subStacks.keys | |
| val currentSubStack: NavBackStack<NavKey> | |
| get() = subStacks[currentTopLevelKey] | |
| ?: error("Sub stack for $currentTopLevelKey does not exist") | |
| val currentKey: NavKey by derivedStateOf { currentSubStack.last() } | |
| } |
🤖 Prompt for AI Agents
In `@navigation/src/main/java/com/yapp/ndgl/navigation/NavigationState.kt` around
lines 39 - 49, currentTopLevelKey and currentKey call last() inside
derivedStateOf which can throw if topLevelStack or a sub-stack becomes empty;
change these to use lastOrNull() and make their types nullable (e.g., val
currentTopLevelKey: NavKey? by derivedStateOf { topLevelStack.lastOrNull() } and
val currentKey: NavKey? by derivedStateOf { currentSubStack.lastOrNull() }) or
alternatively add defensive logic in goBack() (the method that mutates
topLevelStack/subStacks created by rememberNavBackStack(startRoute)) to never
remove the final entry; update any callers to handle nullable NavKey if you
choose the lastOrNull approach.
| fun goBack() { | ||
| when (state.currentKey) { | ||
| state.currentTopLevelKey -> { | ||
| state.topLevelStack.removeLastOrNull() | ||
| } | ||
|
|
||
| else -> state.currentSubStack.removeLastOrNull() | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for goBack() usages in the codebase
rg -n "goBack\(\)" --type=kt -C 3Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 105
🏁 Script executed:
# Find and examine the Navigator.kt file
fd "Navigator.kt" --type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 141
🏁 Script executed:
# Look for NavigationState to understand the currentKey property
fd "NavigationState" --type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 147
🏁 Script executed:
# Read Navigator.kt to understand the implementation
cat -n navigation/src/main/java/com/yapp/ndgl/navigation/Navigator.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1388
🏁 Script executed:
# Read NavigationState.kt to understand currentKey
cat -n navigation/src/main/java/com/yapp/ndgl/navigation/NavigationState.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2957
🏁 Script executed:
# Search for goBack() usages without file type restriction
rg -n "goBack\(\)" -C 3Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 707
🏁 Script executed:
# Check if there are any callers of goBack outside the definition
rg -n "\.goBack\("Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
# Look for any tests related to Navigator.goBack
fd -e kt | xargs grep -l "goBack" | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 187
🏁 Script executed:
# Check how goBack is used in NdglApp.kt
cat -n app/src/main/java/com/yapp/ndgl/ui/NdglApp.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2608
goBack()에서 빈 topLevelStack 처리 필요
topLevelStack의 마지막 항목을 제거한 후 다시 goBack()을 호출하면 state.currentKey 접근 시 NoSuchElementException이 발생합니다. currentKey는 currentSubStack.last()를 평가하는데, 이는 내부적으로 currentTopLevelKey(즉, topLevelStack.last())를 호출하기 때문입니다. 따라서 removeLastOrNull()이 안전해도 그 이후의 goBack() 호출이 crash를 유발합니다.
마지막 top-level 항목을 pop할 경우 앱 종료를 방지하거나 명시적으로 처리해야 합니다.
🤖 Prompt for AI Agents
In `@navigation/src/main/java/com/yapp/ndgl/navigation/Navigator.kt` around lines
15 - 23, The goBack() implementation can crash when topLevelStack becomes empty
because state.currentKey reads currentSubStack.last() which relies on
currentTopLevelKey (topLevelStack.last()); change goBack() to guard pops: before
calling state.topLevelStack.removeLastOrNull() ensure state.topLevelStack.size >
1 (or handle the single-item case explicitly by returning/triggering app-exit
behavior), and likewise avoid assumptions about currentSubStack.last() by using
safe access (lastOrNull) or early-return when the relevant stack is empty;
update the logic around state.currentTopLevelKey, state.topLevelStack, and
state.currentSubStack to prevent accessing .last() after a pop.
…wModel) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
mj010504
force-pushed
the
feature/NDGL-10
branch
from
820f0f2 to
9203e89
Compare
| @Composable | ||
| internal fun NdglNavigationBar( | ||
| currentTab : NavKey, | ||
| onTabSelected : (Route) -> Unit, | ||
| ) { | ||
| NavigationBar { | ||
| TopLevelRoute.entries.forEach { topLevelRoute -> | ||
| NavigationBarItem( | ||
| selected = currentTab == topLevelRoute.navKey, | ||
| onClick = { onTabSelected(topLevelRoute.navKey as Route) }, | ||
| icon = { | ||
| Icon( | ||
| painter = painterResource(id = topLevelRoute.icon), | ||
| contentDescription = topLevelRoute.label | ||
| ) | ||
| }, | ||
| label = { Text(text = topLevelRoute.label) } | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
요기 내용이 Description에 남겨 주신 추후 수정해야 할 컴포넌트로 보이는데요
이런 상황에서는 TODO 주석이나 FIXME 주석을 남겨두면 히스토리 관리가 용이할 것 같습니다!
추가로 프로젝트 진행하면서 임시로 구현해 둔 코드들이 많이 생길 텐데 임시 코드의 경우 TODO나 FIXME로 표시해 두는 것을 그라운드 룰로 정하면 좋을 것 같습니다
There was a problem hiding this comment.
음 그리고 이 친구는 너무나 디자인 시스템에 있어야 할 컴포넌트인 것 같은데 따로 앱 모듈에 추가하신 이유가 있을까요?
아 이 코멘트를 위쪽 코멘트보다 먼저 썼었는데 툴바 컴포넌트인 줄 알았어요
음.. 요고 고민이긴 하네요 관심사로 보면 디자인 시스템이 맞긴 할 것 같은데,
이쪽에 두신 이유는 메인 탭 쪽에서만 쓰고 그렇다 보니 앱 모듈에서만 사용하니까 이렇게 작성해 주신 것 같네요.. 맞을까요?
There was a problem hiding this comment.
요기 내용이 Description에 남겨 주신 추후 수정해야 할 컴포넌트로 보이는데요 이런 상황에서는 TODO 주석이나 FIXME 주석을 남겨두면 히스토리 관리가 용이할 것 같습니다!
추가로 프로젝트 진행하면서 임시로 구현해 둔 코드들이 많이 생길 텐데 임시 코드의 경우 TODO나 FIXME로 표시해 두는 것을 그라운드 룰로 정하면 좋을 것 같습니다
넵 앞으로 반영하고 수정하겠습니다!
There was a problem hiding this comment.
음 그리고 이 친구는 너무나 디자인 시스템에 있어야 할 컴포넌트인 것 같은데 따로 앱 모듈에 추가하신 이유가 있을까요?아 이 코멘트를 위쪽 코멘트보다 먼저 썼었는데 툴바 컴포넌트인 줄 알았어요 음.. 요고 고민이긴 하네요 관심사로 보면 디자인 시스템이 맞긴 할 것 같은데, 이쪽에 두신 이유는 메인 탭 쪽에서만 쓰고 그렇다 보니 앱 모듈에서만 사용하니까 이렇게 작성해 주신 것 같네요.. 맞을까요?
일단은 앱 모듈 전용으로 사용될 것 같아 우선 여기에 구현해 두었습니다. 디자인 시스템에 넣기에는 재사용될 상황이 있을 것 같진 않은데요.
혹시 다른 관점에서 디자인 시스템으로 옮기는 게 더 나을 부분이 있을까요? 편하게 의견 주시면 반영하겠습니다
| include(":navigation") | ||
| include(":core:ui") | ||
| include(":core:util") | ||
| include(":core:designsystem") |
There was a problem hiding this comment.
모듈을 처음에 설계할 때 디자인 시스템은 :core:ui 모듈 쪽에 designSystem 패키지 쪽으로 모여 있고,
디자인 시스템으로 지정되진 않았지만 개발 편의상 공통 컴포넌트로 사용될 컴포넌트를 모아 둘 생각이었는데
추가 모듈이 필요하다고 생각하신 이유가 있나여?
There was a problem hiding this comment.
보통 :core:designsystem에는 Color, Typography, Theme, 기본 컴포넌트처럼 디자인 가이드에 정의된 순수 자산만 남겨두고, :core:ui에는 Compose 의존성이 필요한 확장 함수(Extension)나 유틸리티성 코드들을 두는 식으로 역할을 다른 레포에서 나누더라고요. 그래서 저도 성격이 달라서 둘을 분리하는게 더 좋다고 생각이 들었습니다.
합쳐도 상관은 없을 것 같기는 합니다. 모듈을 합치는게 더 좋을까요?
| implementation(libs.androidx.navigation3.runtime) | ||
| implementation(libs.androidx.lifecycle.viewmodel.navigation3) |
There was a problem hiding this comment.
이런 거 build-logic에 configureNavigation3 이런 함수 추가해 둔 다음에 사용하도록 해도 괜찮을 것 같네용
| import com.yapp.ndgl.core.designsystem.R | ||
|
|
||
| enum class TopLevelRoute( | ||
| @DrawableRes val icon: Int, |
There was a problem hiding this comment.
The annotation is only applied to the parameter. An explicit annotation use-site target is recommended.
여기 use-site target 지정해 달라는 warning이 뜨는데 붙여 주세요! (get 추천..)
| Icon( | ||
| painter = painterResource(id = topLevelRoute.icon), | ||
| contentDescription = topLevelRoute.label | ||
| ) |
There was a problem hiding this comment.
(nit) 아이콘은 vector만 쓰니까 ImageVector.vectorResource()가 의도가 더 명확할 것 같아요
|
|
||
| @Composable | ||
| fun NdglApp( | ||
| modifier: Modifier = Modifier, |
| import kotlinx.serialization.Serializable | ||
|
|
||
| @Serializable | ||
| sealed interface Route : NavKey { |
There was a problem hiding this comment.
Route의 역할이 무엇인가요?
제가 이해하기로는 저희 서비스 내에서 이동할 수 있는 스크린(혹은 목적지)을 모두 모아서 정의해 두고, 사용하기 위함으로 이해했는데요
그렇다면 새로운 목적지가 추가될 때도 요 룰을 강제하도록 하기 위해서는 NavigationState의 startRoute나 key, stack들에서 NavKey 타입으로 받는 것이 아니라 Route로 받아주어야 하는게 아닌가 싶어서 질문드립니당
There was a problem hiding this comment.
넵, 지희님이 말씀하신 것처럼 navigation 모듈에 Route들을 한곳에 모아두고 싶었습니다. 화면 전환할 때 feature 모듈간의 의존성을 제거하기 위해서요.
근데 Route 타입으로 받으면 기존에 navigation3 라이브러리에서 제공하는 entryProvider나 rememberNavBackstack의 반환 타입이 NavKey라 타입 캐스트가 필요할 것 같은데 이 부분 조금만 더 고민해보겠습니다..
gradle/libs.versions.toml
Outdated
| junit = { group = "junit", name = "junit", version.ref = "junit" } | ||
| androidx-junit = { group = "androidx.test.ext", name = "junit", version.ref = "junitVersion" } | ||
| androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espressoCore" } |
There was a problem hiding this comment.
저희 지금 당장 테스트 코드가 추가되지 않고 있어서 우선 테스트 관련 정의들 제거하는 게 어떨까요?
테스트 코드를 넣는다고 하더라도 junit이랑 espresso를 사용할지 아니면 다른 라이브러리를 사용하게 될지도 모르는 거라고 생각해서요~! ㅠ
지운다고 지웠는데 build-logic 모듈의 convention 패키지에 있는 configureKotlinAndroid() 함수 내부의 testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" 코드도 함께 지워주시면 감사하겠습니다 🙇♀️
gradle/libs.versions.toml
Outdated
| androidx-compose-ui-tooling = { group = "androidx.compose.ui", name = "ui-tooling" } | ||
| androidx-compose-ui-tooling-preview = { group = "androidx.compose.ui", name = "ui-tooling-preview" } | ||
| androidx-compose-material3 = { group = "androidx.compose.material3", name = "material3" } | ||
| androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation-layout", version.ref = "androidxComposeFoundation" } |
There was a problem hiding this comment.
이 친구는 composeBom에 명세되어 있는 버전으로 안 따라가나여?
There was a problem hiding this comment.
앗 실수한 것 같네요. 수정했습니다!
| } | ||
| } | ||
|
|
||
| class NavigationState( |
There was a problem hiding this comment.
NavigationState를 파라미터로 컴포저블 함수에 넘겨주는 상황은 없을까요?
There was a problem hiding this comment.
넵 지금 당장은 없을 것 같습니다. 혹시 생각되는 경우가 있으시면 말씀해주세요!
현재는 Feature별 entry에 navigator 주입해서 네비게이션 로직만 composable 함수에 넘겨주는 상황만 있을 것 같네요.
|
IDE 셋팅에서 EOL(파일 끝에 개행)이 빠져 있는 것 같은데요 |
|
Navigation3를 저도 처음 사용해 보는 거다 보니 새롭네요 ㅎㅎ AssistInject로 넣어줘야 하는 건 확실히 불편하긴 하네요.. 안드로이드 팀에서 의도한 바가 있을 것 같은데.. 아직 공감이 잘 되는 건 아닌 것 같아요 ㅋㅋㅋ |
|
앗 추가로 저희 build.gradle에 정의해놓은 namespace가 있는데, 코드의 package 경로랑 다른 곳들이 있어 보입니다 ex) (현재 코드) com.yapp.home -> (네임스페이스와 동일하게 수정) com.yapp.ndgl.feature.home |
적용했습니다~! |
- namespace 변경 (build.gradle.kts) - core/designsystem: com.yapp.ndgl.core.designsystem → com.yapp.designsystem - core/ui: com.yapp.ndgl.core.ui → com.yapp.ui - feature/auth: com.yapp.ndgl.feature.auth → com.yapp.auth - feature/home: com.yapp.home (유지) - feature/travel: com.yapp.ndgl.feature.travel → com.yapp.travel - feature/travel-helper: com.yapp.ndgl.feature.travelHelper → com.yapp.helper - package 선언 변경 - core/designsystem: com.yapp.designsystem.theme - core/util: com.yapp.ndgl.core.util → com.yapp.util - feature/auth: com.yapp.auth - 디렉토리 구조 변경 - core/util: com/yapp/ndgl/core/util → com/yapp/util 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- NdglNavigationBar → BottomNavigationBar로 변경 - 관련 import 및 참조 업데이트 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
- androidxComposeFoundation 버전 참조 제거 - compose-foundation이 composeBom 버전을 따르도록 변경 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
적용했습니다~! |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt`:
- Around line 15-22: Change the BottomNavigationBar signature so onTabSelected
accepts NavKey instead of Route to avoid unsafe casts: update the parameter type
for onTabSelected in BottomNavigationBar, update all call sites to pass a
NavKey, and remove the cast in NavigationBarItem onClick (replace
onTabSelected(topLevelRoute.navKey as Route) with
onTabSelected(topLevelRoute.navKey)); reference symbols: BottomNavigationBar,
onTabSelected, TopLevelRoute.navKey, NavKey, Route, and the NavigationBarItem
onClick handler.
In `@core/util/src/main/java/com/yapp/util/Extensions.kt`:
- Around line 3-4: The file contains an unused empty class named Extensions
which is dead code; either remove the file entirely or replace the empty class
with actual top-level Kotlin extension functions (do not wrap them in a class).
Locate the class symbol Extensions and either delete the file or implement your
intended extension functions at the file top level (for example declare fun
String.myExtension(...) { ... } etc.) and remove the empty class declaration.
In `@gradle/libs.versions.toml`:
- Around line 63-64: 현재 androidx-compose-foundation 별칭이 foundation-layout 아티팩트로
설정되어 있어 androidx.compose.foundation.isSystemInDarkTheme 같은 Foundation API를 사용할 수
없습니다; gradle/libs.versions.toml에서 androidx-compose-foundation 항목의 name 값을
"foundation-layout"에서 "foundation"으로 변경하여 전체 Foundation 아티팩트를 가리키도록 수정하세요 (참조
심볼: androidx.compose.foundation.isSystemInDarkTheme, 설정 키:
androidx-compose-foundation).
♻️ Duplicate comments (2)
app/src/main/java/com/yapp/ndgl/ui/NdglApp.kt (2)
24-26:modifier파라미터가 사용되지 않고 있습니다.이전 리뷰에서 언급되었던 사항입니다.
modifier파라미터를 삭제하거나, Scaffold에서 사용하도록 수정해 주세요.
39-39: 불필요한 재계산을 줄이기 위해derivedStateOf사용을 고려해 주세요.이전 리뷰에서 언급되었던 사항입니다. 현재 방식은 매 recomposition마다 계산됩니다.
NavigationState내부에서derivedStateOf로 최적화하면 상태가 실제로 변경될 때만 재계산됩니다.
🧹 Nitpick comments (3)
feature/auth/src/main/java/com/yapp/auth/AuthViewModel.kt (1)
3-12: 사용되지 않는 import 제거 필요현재
viewModelScope,MutableSharedFlow,MutableStateFlow,StateFlow,asSharedFlow,asStateFlow,launch등의 import가 선언되어 있지만 클래스 내에서 사용되지 않습니다. 임시 화면이라도 불필요한 import는 제거하는 것이 좋습니다.♻️ 수정 제안
package com.yapp.auth import androidx.lifecycle.ViewModel -import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asSharedFlow -import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.launch import javax.inject.Injectfeature/auth/src/main/java/com/yapp/auth/AuthScreen.kt (1)
15-21: 사용되지 않는 viewModel 파라미터
viewModel파라미터가 선언되었지만AuthRoute함수 내에서 사용되지 않습니다. 임시 구현이라면@Suppress("UNUSED_PARAMETER")어노테이션을 추가하거나, 향후 구현 시 사용할 예정임을 나타내는 TODO 주석을 추가하는 것을 권장합니다.♻️ 수정 제안
+@Suppress("UNUSED_PARAMETER") `@Composable` fun AuthRoute( viewModel: AuthViewModel = hiltViewModel(), ) { - + // TODO: viewModel을 사용하여 상태 및 이벤트 연결 예정 AuthScreen() }app/src/main/java/com/yapp/ndgl/ui/NdglApp.kt (1)
3-5: 사용되지 않는 애니메이션 import가 있습니다.
slideInVertically와slideOutVertically가 import되어 있지만AnimatedVisibility에서 사용되지 않고 있습니다. 의도적으로 bottom bar 애니메이션을 커스터마이징하려 했다면 적용하거나, 그렇지 않다면 제거해 주세요.♻️ 사용하지 않을 경우 제거
import androidx.compose.animation.AnimatedVisibility -import androidx.compose.animation.slideInVertically -import androidx.compose.animation.slideOutVertically import androidx.compose.foundation.layout.fillMaxSize♻️ 또는 AnimatedVisibility에 적용
AnimatedVisibility( visible = shouldShowBottomBar, + enter = slideInVertically { it }, + exit = slideOutVertically { it }, ) {
| currentTab : NavKey, | ||
| onTabSelected : (Route) -> Unit, | ||
| ) { | ||
| NavigationBar { | ||
| TopLevelRoute.entries.forEach { topLevelRoute -> | ||
| NavigationBarItem( | ||
| selected = currentTab == topLevelRoute.navKey, | ||
| onClick = { onTabSelected(topLevelRoute.navKey as Route) }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the BottomNavigationBar.kt file
find . -name "BottomNavigationBar.kt" -type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 138
🏁 Script executed:
# Search for TopLevelRoute definition
rg -n "enum class TopLevelRoute|sealed class TopLevelRoute|class TopLevelRoute|object TopLevelRoute" -g "*.kt" -A 10Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1016
🏁 Script executed:
# Search for NavKey definition
rg -n "enum class NavKey|sealed class NavKey|class NavKey|object NavKey|typealias NavKey" -g "*.kt" -A 5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
# Search for Route definition
rg -n "enum class Route|sealed class Route|class Route|object Route|typealias Route" -g "*.kt" -A 5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
# Search for onTabSelected usage to understand expected type
rg -n "onTabSelected" -g "*.kt" -B 2 -A 2Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1489
🏁 Script executed:
# Read the TopLevelRoute.kt file completely to understand the structure
cat -n app/src/main/java/com/yapp/ndgl/navigation/TopLevelRoute.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 887
🏁 Script executed:
# Search more broadly for NavKey and Route definitions
rg -n "NavKey|Route" -g "*.kt" --type-list | head -20
rg -n "^typealias NavKey|^typealias Route" -g "*.kt"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 468
🏁 Script executed:
# Look at Navigator.navigate signature
rg -n "fun navigate|class Navigator" -g "*.kt" -A 5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 855
🏁 Script executed:
# Check imports in TopLevelRoute.kt
head -20 app/src/main/java/com/yapp/ndgl/navigation/TopLevelRoute.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 563
🏁 Script executed:
# Find Route definition to understand the type hierarchy
rg -n "sealed class Route|class Route|object Route|data class Route" -g "*.kt" -A 10Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
# Check if Route extends NavKey or if NavKey is a typealias for Route
rg -n "class Route|interface Route|sealed" -g "*.kt" -B 2 -A 10 | grep -A 10 "class Route"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
# Look at the complete BottomNavigationBar.kt file
cat -n app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1416
🏁 Script executed:
# Search for Route in all kotlin files more broadly
rg -n "Route\." -g "*.kt" --max-count=5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1769
🏁 Script executed:
# Look for Route file/definition
find . -name "*Route*.kt" -type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 201
🏁 Script executed:
# Check imports of Route in various files
rg -n "import.*Route" -g "*.kt" | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1268
🏁 Script executed:
# Try to find where Route is defined by searching its definition pattern
rg -n "^(sealed |open |abstract )?class Route|^(sealed |open |abstract )?object Route" -g "*.kt" -A 5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
# Read the Route.kt file to understand its definition
cat -n navigation/src/main/java/com/yapp/ndgl/navigation/Route.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 680
🏁 Script executed:
# Verify the type relationship and usage patterns more carefully
rg -n "Navigator\(" -g "*.kt" -B 2 -A 2Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 952
🏁 Script executed:
# Check NdglApp to see the full context of how BottomNavigationBar is called
cat -n app/src/main/java/com/yapp/ndgl/ui/NdglApp.kt | head -60Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2580
onTabSelected 파라미터 타입을 NavKey로 변경하여 불필요한 캐스팅 제거
topLevelRoute.navKey는 NavKey 타입이므로 Route로 강제 캐스팅할 때 런타임 크래시 위험이 있습니다. 더 중요하게는 navigator.navigate(key)가 NavKey를 받으므로, BottomNavigationBar의 onTabSelected 파라미터 타입을 (Route) -> Unit에서 (NavKey) -> Unit으로 변경하면 캐스팅을 완전히 제거할 수 있습니다.
♻️ 권장 변경사항
internal fun BottomNavigationBar(
currentTab : NavKey,
- onTabSelected : (Route) -> Unit,
+ onTabSelected : (NavKey) -> Unit,
) {
NavigationBar {
TopLevelRoute.entries.forEach { topLevelRoute ->
NavigationBarItem(
selected = currentTab == topLevelRoute.navKey,
- onClick = { onTabSelected(topLevelRoute.navKey as Route) },
+ onClick = { onTabSelected(topLevelRoute.navKey) },🤖 Prompt for AI Agents
In `@app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt` around lines 15 -
22, Change the BottomNavigationBar signature so onTabSelected accepts NavKey
instead of Route to avoid unsafe casts: update the parameter type for
onTabSelected in BottomNavigationBar, update all call sites to pass a NavKey,
and remove the cast in NavigationBarItem onClick (replace
onTabSelected(topLevelRoute.navKey as Route) with
onTabSelected(topLevelRoute.navKey)); reference symbols: BottomNavigationBar,
onTabSelected, TopLevelRoute.navKey, NavKey, Route, and the NavigationBarItem
onClick handler.
| androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation-layout" } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
androidx.compose.foundation:foundation vs foundation-layout difference API
💡 Result:
Short answer
- androidx.compose.foundation:foundation is the umbrella "foundation" artifact that contains core foundation APIs (gestures, drawing, text basics, scroll/drag state, basic components and many modifiers).
- androidx.compose.foundation:foundation-layout is the smaller artifact that contains the layout-only implementations (layout composables and layout-related utilities) extracted from foundation so you can depend only on layout pieces without pulling all foundation implementations.
Why it exists
- The layout pieces were split out to reduce artifact size and dependency surface: projects that only need layout composables (Box/Row/Column variations, FlowRow/FlowColumn, layout helpers and related modifiers) can depend on foundation-layout, while projects that need the rest of Foundation (gestures, text basics, overscroll, clickable, etc.) use foundation. See the Compose release notes and artifact pages for the split and which APIs live in each artifact. [1][2]
If you want exact classes or to decide which to add to your Gradle:
- Check the release notes and the specific artifact API list for your Compose version (foundation release notes list what moved/was added). Example references: AndroidX Compose Foundation release notes and the foundation-layout Maven entry. [1][2]
Sources
[1] Android Developers — Compose Foundation release notes.
[2] Maven / artifact page — androidx.compose.foundation:foundation-layout.
🏁 Script executed:
# Search for how androidx-compose-foundation alias is used in the codebase
rg "androidx-compose-foundation" --type toml --type gradle --type kt --type java -A 2 -B 2Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 105
🏁 Script executed:
# Check if there are other files referencing these dependencies
fd -e gradle -e toml -e kt -e java | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1126
🏁 Script executed:
# Search for how the androidx-compose-foundation alias is actually used
rg "androidx-compose-foundation|foundation-layout|androidx\.compose\.foundation" --glob "*.gradle" --glob "*.toml" --glob "*.kts" -A 2 -B 2Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 507
🏁 Script executed:
# Check build.gradle files for dependencies
fd "build\.gradle" -type f | head -10Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 249
🏁 Script executed:
# Search Kotlin files for imports and usage of foundation APIs
rg "androidx\.compose\.foundation" --glob "*.kt" --glob "*.java" | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2171
androidx-compose-foundation 별칭과 아티팩트 불일치 수정 필요
현재 별칭이 foundation-layout을 가리키고 있지만, 코드베이스에서 androidx.compose.foundation.isSystemInDarkTheme을 사용하고 있습니다. 이 API는 레이아웃 전용 아티팩트가 아닌 전체 Foundation 아티팩트에만 포함되어 있으므로, 아티팩트를 foundation으로 변경해야 합니다.
🔧 필수 변경
-androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation-layout" }
+androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation-layout" } | |
| androidx-compose-foundation = { group = "androidx.compose.foundation", name = "foundation" } |
🤖 Prompt for AI Agents
In `@gradle/libs.versions.toml` around lines 63 - 64, 현재
androidx-compose-foundation 별칭이 foundation-layout 아티팩트로 설정되어 있어
androidx.compose.foundation.isSystemInDarkTheme 같은 Foundation API를 사용할 수 없습니다;
gradle/libs.versions.toml에서 androidx-compose-foundation 항목의 name 값을
"foundation-layout"에서 "foundation"으로 변경하여 전체 Foundation 아티팩트를 가리키도록 수정하세요 (참조
심볼: androidx.compose.foundation.isSystemInDarkTheme, 설정 키:
androidx-compose-foundation).
개요
연관 문서
https://developer.android.com/guide/navigation/navigation-3/basics?hl=ko
https://github.com/android/nowinandroid/tree/main/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation - Navigation3 부분 많이 참고했습니다.
스크린샷
변경사항
navigation3 연동
design system 모듈 추가
각 feature 모듈들 임시 화면 제작
Navigation State 추가(네비게이션 상태 관리)
feature 모듈별로 entryProvider 있고 navigator 주입받아서 네비게이션 로직 사용합니다.
Navigation 3에서는 Navigation2 처럼 ViewModel SavedStateHandle에 arguments가 자동 주입되지 않아서, AssistedInject로 Factory를 만들어 직접 전달하는 방식을 사용했습니다. 보일러플레이트가 좀 있는데, 더 나은 방법이 있는지 찾아보겠습니다.
Navigation Bar 디자인도 추후에 수정해야합니다.
테스트 체크 리스트
Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링
✏️ Tip: You can customize this high-level summary in your review settings.