Conversation
Walkthrough홈을 시작 화면으로 설정하고 홈 페이지 스캐폴드와 하단 네비게이션(아이템·라우트·리소스)을 추가했으며 기존 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App
participant MainNavHost
participant HomePage
participant BottomNavBar
participant HomeNavHost
participant NavController
App->>MainNavHost: startDestination = /home
MainNavHost->>HomePage: Composable(HomePage)
HomePage->>NavController: rememberNavController()
HomePage->>BottomNavBar: currentRoute, onChangeNavRoute(route)
HomePage->>HomeNavHost: set startDestination = /tables
User->>BottomNavBar: 탭 선택(예: /settings)
BottomNavBar-->>HomePage: onChangeNavRoute("/settings")
HomePage->>NavController: navigate("/settings")
NavController-->>HomeNavHost: route 변경
HomeNavHost-->>User: Settings 화면(플레이스홀더) 표시
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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: 2
🧹 Nitpick comments (12)
app/src/main/res/values/strings.xml (2)
4-5: 리소스 키 네이밍을 더 구체화하세요 (nav_home/nav_settings 권장).향후 다른 컨텍스트의 "home"/"setting"과 충돌을 줄이려면 네이밍을 목적별로 구체화하는 편이 좋습니다.
적용 예:
- <string name="home">홈</string> - <string name="setting">설정</string> + <string name="nav_home">홈</string> + <string name="nav_settings">설정</string>참고: 사용처의 참조(R.string.home/setting)도 함께 변경 필요합니다. 원하시면 일괄 변경 패치 제안해 드립니다.
3-5: 하드코딩 문자열(“안녕”)을 리소스로 이동하세요.HomePage.kt(Line 27)의 텍스트를 리소스로 추출하면 i18n에 유리합니다.
추가 예:
</resources> + <string name="hello">안녕</string> </resources>app/src/main/java/com/debatetimer/app/navigation/BottomNavRoute.kt (1)
6-7: 라우트 문자열의 선행 슬래시 제거로 일관성 유지 (MainNavRoute와 동일 스타일).현재 BottomNavRoute만 "/home", "/setting"처럼 슬래시를 쓰고, MainNavRoute는 "home", "login" 형태입니다. 한쪽으로 통일해 혼동을 줄이세요.
적용 예:
- object Home : BottomNavRoute("/home") - object Setting : BottomNavRoute("/setting") + object Home : BottomNavRoute("home") + object Setting : BottomNavRoute("setting")적용 시 NavHost 및 클릭 네비게이션의 참조도 함께 수정 필요합니다.
app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (3)
21-23: 파라미터 명 오해 소지 — index → route로 변경.타입이 BottomNavRoute인데 변수명이 index입니다. 의미에 맞게 변경하면 가독성이 좋아집니다.
- onChangeNavRoute = { index -> viewModel.setNavRoute(index) } + onChangeNavRoute = { route -> viewModel.setNavRoute(route) }
17-24: 네비게이션은 ViewModel 상태 변경이 아니라 NavController로 수행하세요.현 구조에선 탭 전환 시 UI 선택 상태만 바뀌고 실제 화면 전환은 없습니다. NavController 콜백을 주입하는 형태로 state hoisting을 권장합니다.
예시 시그니처:
@Composable fun HomePage( modifier: Modifier = Modifier, viewModel: HomeViewModel = hiltViewModel(), onChangeBottomTab: (BottomNavRoute) -> Unit ) { ... BottomNavBar( currentNavRoute = viewModel.currentNavRoute.value, onChangeNavRoute = onChangeBottomTab ) }MainNavHost에서 onChangeBottomTab에 navController.navigate(...)를 연결하세요.
26-29: 하드코딩 텍스트를 stringResource로 교체하세요.i18n과 테스트 용이성을 위해 리소스 사용을 권장합니다.
- Text("안녕") + Text(stringResource(R.string.hello))추가 필요 import:
- import androidx.compose.ui.res.stringResource
- import com.debatetimer.app.R
app/src/main/java/com/debatetimer/app/ui/home/HomeViewModel.kt (2)
15-17: 파라미터 명과 불필요한 재설정 방지
- 파라미터명이
index인 것은 오해의 소지가 있습니다.route로 변경 권장.- 동일 값으로의 재설정을 회피해 불필요한 recomposition을 줄일 수 있습니다.
적용 diff:
- fun setNavRoute(index: BottomNavRoute) { - _currentNavRoute.value = index - } + fun setNavRoute(route: BottomNavRoute) { + if (_currentNavRoute.value != route) { + _currentNavRoute.value = route + } + }
3-4: ViewModel에서 Compose State 의존도 축소(옵션)UI 프레임워크와의 결합도를 낮추려면 ViewModel에서는
StateFlow를 노출하고, Compose 쪽에서collectAsState()로 소비하는 패턴을 고려해볼 만합니다. 테스트/비Compose 소비에도 유리합니다.예시 diff:
-import androidx.compose.runtime.State -import androidx.compose.runtime.mutableStateOf +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update @@ - private val _currentNavRoute = mutableStateOf<BottomNavRoute>(BottomNavRoute.Home) - val currentNavRoute: State<BottomNavRoute> = _currentNavRoute + private val _currentNavRoute = MutableStateFlow(BottomNavRoute.Home) + val currentNavRoute: StateFlow<BottomNavRoute> = _currentNavRoute.asStateFlow() @@ - fun setNavRoute(index: BottomNavRoute) { - _currentNavRoute.value = index - } + fun setNavRoute(route: BottomNavRoute) { + _currentNavRoute.update { route } + }Also applies to: 12-17
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt (2)
26-28: 내비게이션 아이콘의 contentDescription은 null 권장
label을 제공하고 있어 스크린리더가 라벨과 아이콘 설명을 이중으로 읽을 수 있습니다. 접근성 중복을 피하려면 아이콘의contentDescription을null로 두는 것이 Material 가이드에 부합합니다.적용 diff:
- contentDescription = stringResource(item.labelId) + contentDescription = null
32-32: 이미 선택된 탭에 대한 중복 클릭 처리이미 선택된 항목을 다시 클릭할 때 콜백을 막아 불필요한 상태 변경/재구성을 줄일 수 있습니다.
적용 diff:
- onClick = { onChangeNavRoute(item.route) } + onClick = { if (!isSelected) onChangeNavRoute(item.route) }app/src/main/java/com/debatetimer/app/navigation/BottomNavItem.kt (2)
3-10: 리소스 ID에 @stringres 애노테이션 추가 권장
labelId에 타입 세이프티를 위해@StringRes를 붙이면 오용을 컴파일 타임에 방지할 수 있습니다.적용 diff:
+import androidx.annotation.StringRes @@ data class BottomNavItem( val route: BottomNavRoute, val selectedIcon: ImageVector, val unselectedIcon: ImageVector, - val labelId: Int + @StringRes val labelId: Int )Also applies to: 11-16
11-16: Compose 안정성 힌트(옵션)재조합 힌트를 위해
@Immutable을 붙이는 것도 고려 가능합니다(빈번한 변경이 없는 단순 값 객체에 적합).예시 diff:
+import androidx.compose.runtime.Immutable @@ - data class BottomNavItem( + @Immutable + data class BottomNavItem(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/src/main/java/com/debatetimer/app/navigation/BottomNavItem.kt(1 hunks)app/src/main/java/com/debatetimer/app/navigation/BottomNavRoute.kt(1 hunks)app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt(1 hunks)app/src/main/java/com/debatetimer/app/navigation/MainNavRoute.kt(1 hunks)app/src/main/java/com/debatetimer/app/navigation/NavItem.kt(0 hunks)app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/HomeViewModel.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/java/com/debatetimer/app/navigation/NavItem.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-02T07:20:08.023Z
Learnt from: i-meant-to-be
PR: debate-timer/debate-timer-fe-android#11
File: app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt:10-14
Timestamp: 2025-08-02T07:20:08.023Z
Learning: In the debate-timer Android project, navigation destinations in MainNavHost.kt are being added incrementally during development, so an empty NavHost block in early stages is intentional and acceptable.
Applied to files:
app/src/main/java/com/debatetimer/app/ui/home/HomePage.ktapp/src/main/java/com/debatetimer/app/navigation/BottomNavRoute.ktapp/src/main/java/com/debatetimer/app/navigation/MainNavHost.ktapp/src/main/java/com/debatetimer/app/navigation/BottomNavItem.ktapp/src/main/java/com/debatetimer/app/ui/home/HomeViewModel.ktapp/src/main/java/com/debatetimer/app/navigation/MainNavRoute.kt
🧬 Code graph analysis (2)
app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (1)
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt (1)
BottomNavBar(13-36)
app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt (1)
app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (1)
HomePage(12-30)
⏰ 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: build_and_test
🔇 Additional comments (5)
app/src/main/java/com/debatetimer/app/navigation/MainNavRoute.kt (1)
6-7: LGTM — 라우트 정의 적절합니다.현 단계에서 Home/Login만 선언된 점 문제없습니다. 이후 화면 추가 시 동일 패턴으로 확장하세요.
app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt (1)
14-15: StartDestination을 Home으로 변경 — 플로우 의도 확인 필요.로그인 게이트가 아직 없거나 비활성화라면 OK입니다. 그렇지 않다면 인증 상태에 따라 startDestination을 분기하도록 고려하세요.
app/src/main/java/com/debatetimer/app/ui/home/HomeViewModel.kt (1)
10-14: 기본 상태 설계 깔끔합니다초기 라우트를 Home으로 두고 읽기 전용 State로 노출한 구조는 Compose에서 사용하기에 적절합니다.
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt (1)
19-35: 전반적으로 구성 적절
NavigationBar/NavigationBarItem활용과 선택 상태 비교 로직이 간결하고 읽기 좋습니다.app/src/main/java/com/debatetimer/app/navigation/BottomNavItem.kt (1)
18-32: 아이템 구성 적절Home/Setting 두 항목 구성과 아이콘 매핑이 명확합니다. 상수 리스트로 노출한 점도 좋습니다.
| navigation( | ||
| route = MainNavRoute.Home.route, | ||
| startDestination = BottomNavRoute.Home.route | ||
| ) { | ||
| composable(BottomNavRoute.Home.route) { | ||
| HomePage() | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
바텀 탭 클릭 시 NavController로 실제 네비게이션을 트리거하세요.
현재 바텀 탭은 ViewModel 상태만 바꾸고 NavHost 백스택은 변경하지 않습니다. 바텀 탭 UX를 완성하려면 navController.navigate(...)를 호출해야 합니다. 일반적인 패턴은 아래와 같습니다.
예시(구조 제안):
// MainNavHost 내부
HomePage(
onChangeBottomTab = { route ->
navController.navigate(route) {
popUpTo(MainNavRoute.Home.route) { saveState = true }
launchSingleTop = true
restoreState = true
}
}
)그리고 HomePage에서 BottomNavBar의 onClick에 이 콜백을 연결하세요. (ViewModel로만 상태 보관 시 NavHost와 분리되어 화면 전환이 이뤄지지 않습니다.)
원하시면 이 패턴으로 MainNavHost/HomePage에 맞춘 구체 패치 드립니다.
🤖 Prompt for AI Agents
In app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt around lines
16-23, the BottomNav tab currently only updates ViewModel state and does not
trigger NavHost navigation; update MainNavHost to pass an onChangeBottomTab
callback into HomePage that calls navController.navigate(targetRoute) with
popUpTo(MainNavRoute.Home.route) { saveState = true }, launchSingleTop = true,
and restoreState = true, and then wire that callback in HomePage so the
BottomNavBar onClick uses it (instead of only updating the ViewModel) so
NavController backstack and UI navigation are properly updated.
🛠️ Refactor suggestion
BottomNav ‘설정(Setting)’ 경로 미정의 — 그래프에 추가하세요.
바텀 탭이 2개(Home/Setting)인데, 그래프에는 Home만 있습니다. 최소한 플레이스홀더 컴포저블이라도 추가해 404 라우팅을 방지하세요.
적용 예:
) {
navigation(
route = MainNavRoute.Home.route,
startDestination = BottomNavRoute.Home.route
) {
composable(BottomNavRoute.Home.route) {
HomePage()
}
+ composable(BottomNavRoute.Setting.route) {
+ Text("설정")
+ }
}
}위 변경에 필요한 import:
+import androidx.compose.material3.Text📝 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.
| navigation( | |
| route = MainNavRoute.Home.route, | |
| startDestination = BottomNavRoute.Home.route | |
| ) { | |
| composable(BottomNavRoute.Home.route) { | |
| HomePage() | |
| } | |
| } | |
| // Add this import at the top of MainNavHost.kt | |
| import androidx.compose.material3.Text | |
| // …inside your NavHost builder… | |
| navigation( | |
| route = MainNavRoute.Home.route, | |
| startDestination = BottomNavRoute.Home.route | |
| ) { | |
| composable(BottomNavRoute.Home.route) { | |
| HomePage() | |
| } | |
| composable(BottomNavRoute.Setting.route) { | |
| Text("설정") | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt around lines
16 to 23, the navigation graph only registers the Home route while the bottom
bar exposes Home and Setting; add a composable entry for
BottomNavRoute.Setting.route (e.g., call a placeholder composable such as
SettingPage() or a simple SettingsPlaceholder composable) inside the same
navigation block to prevent 404s, and add the necessary imports for the
placeholder/Setting composable and BottomNavRoute/Navigation utilities at the
top of the file.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
app/src/main/java/com/debatetimer/app/ui/home/HomeViewModel.kt (1)
7-8: 향후 상태 복원 대비 SavedStateHandle 주입 고려프로세스 데스/딥링크 대비가 필요해지면 SavedStateHandle 주입으로 상태 키를 표준화해 두는 것이 좋습니다.
예시:
-@HiltViewModel -class HomeViewModel @Inject constructor() : ViewModel() +@HiltViewModel +class HomeViewModel @Inject constructor( + private val savedStateHandle: androidx.lifecycle.SavedStateHandle +) : ViewModel()app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavRoute.kt (2)
3-8: 하위 그래프 라우트 문자열에서 선행 ‘/’ 제거 제안 (상위 그래프와 구분 명확화)메인 그래프(MainNavRoute)와 동일한 문자열("/home")을 공유하면 혼동을 야기할 수 있습니다. 하위(홈 내부) 그래프는 슬래시 없는 심볼릭 라우트로 두는 편이 일반적입니다.
sealed class BottomNavRoute( val value: String ) { - object Home : BottomNavRoute("/home") - object Setting : BottomNavRoute("/setting") + object Home : BottomNavRoute("home") + object Setting : BottomNavRoute("setting") }참고: 이 변경 시 해당 라우트를 사용하는 모든 호출부(HomeNavHost, BottomNavBar, HomePage)도 함께 업데이트가 필요합니다.
6-7: 명명 일관성: Setting → Settings 고려영문 표기는 보통 복수형(Settings)을 사용합니다. 리소스/클래스 명칭도 일치시키면 가독성이 좋아집니다.
- object Setting : BottomNavRoute("setting") + object Settings : BottomNavRoute("settings")app/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt (1)
15-29: 플레이스홀더 텍스트도 stringResource 사용 권장국제화 일관성을 위해 임시 텍스트도 stringResource를 사용하면 리소스 기반으로 유지할 수 있습니다.
- composable(BottomNavRoute.Home.value) { - // TODO: Need to replaced with actual view - Text("Home") - } + composable(BottomNavRoute.Home.value) { + // TODO: Need to be replaced with actual view + Text(stringResource(id = com.debatetimer.app.R.string.home)) + } - composable(BottomNavRoute.Setting.value) { - // TODO: Need to replaced with actual view - Text("Setting") - } + composable(BottomNavRoute.Setting.value) { + // TODO: Need to be replaced with actual view + Text(stringResource(id = com.debatetimer.app.R.string.setting)) + }필요 시 다음 import를 추가하세요:
import androidx.compose.ui.res.stringResourceapp/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (1)
21-22: 초기 선택 상태 깜빡임 완화(옵션)초기 컴포지션에서 currentRoute가 null일 수 있습니다. 기본값을 startDestination으로 보정하면 선택 상태 깜빡임을 줄일 수 있습니다.
- val currentRoute = navBackStackEntry?.destination?.route + val currentRoute = navBackStackEntry?.destination?.route + ?: com.debatetimer.app.ui.home.component.BottomNavRoute.Home.valueapp/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt (1)
21-31: 이미 선택된 탭 재클릭 시 불필요한 navigate 방지UI 레벨에서 중복 클릭을 무시하면 재조합/애니메이션 트리거를 줄일 수 있습니다(네비게이션 옵션 적용과 병행하면 더 견고).
- NavigationBarItem( + NavigationBarItem( icon = { Icon( imageVector = if (isSelected) item.selectedIcon else item.unselectedIcon, contentDescription = stringResource(item.labelId) ) }, label = { Text(stringResource(item.labelId)) }, selected = isSelected, - onClick = { onChangeNavRoute(item.route.value) } + onClick = { if (!isSelected) onChangeNavRoute(item.route.value) } )app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavItem.kt (3)
11-16: labelId에 @stringres를 부여해 리소스 타입 안전성 확보정적 분석/IDE 지원을 통해 잘못된 정수 전달을 조기에 방지할 수 있습니다.
적용 diff:
data class BottomNavItem( val route: BottomNavRoute, val selectedIcon: ImageVector, val unselectedIcon: ImageVector, - val labelId: Int + @StringRes val labelId: Int )
3-10: @stringres 추가에 따른 import 보강위 변경을 적용하려면 아래 import가 필요합니다.
적용 diff:
import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Home import androidx.compose.material.icons.filled.Settings import androidx.compose.material.icons.outlined.Home import androidx.compose.material.icons.outlined.Settings import androidx.compose.ui.graphics.vector.ImageVector +import androidx.annotation.StringRes import com.debatetimer.app.R
11-16: API 범위 축소 검토: 내부 모듈 한정이면 internal 권장외부 공개가 필요 없다면 데이터 클래스와 목록을 internal로 좁혀 모듈 경계를 명확히 할 수 있습니다. 동시에 목록에 명시적 타입을 부여하면 가독성과 바이너리 호환성에도 유리합니다.
적용 diff:
-data class BottomNavItem( +internal data class BottomNavItem( val route: BottomNavRoute, val selectedIcon: ImageVector, val unselectedIcon: ImageVector, @StringRes val labelId: Int ) -val bottomNavItems = listOf( +internal val bottomNavItems: List<BottomNavItem> = listOf(Also applies to: 18-19
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt(1 hunks)app/src/main/java/com/debatetimer/app/navigation/MainNavRoute.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/HomeViewModel.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavItem.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavRoute.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt
- app/src/main/java/com/debatetimer/app/navigation/MainNavRoute.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-02T07:20:08.023Z
Learnt from: i-meant-to-be
PR: debate-timer/debate-timer-fe-android#11
File: app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt:10-14
Timestamp: 2025-08-02T07:20:08.023Z
Learning: In the debate-timer Android project, navigation destinations in MainNavHost.kt are being added incrementally during development, so an empty NavHost block in early stages is intentional and acceptable.
Applied to files:
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavRoute.ktapp/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.ktapp/src/main/java/com/debatetimer/app/ui/home/component/BottomNavItem.ktapp/src/main/java/com/debatetimer/app/ui/home/HomePage.kt
🧬 Code graph analysis (1)
app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (2)
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt (1)
BottomNavBar(11-34)app/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt (1)
HomeNavHost(10-30)
⏰ 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: build_and_test
🔇 Additional comments (5)
app/src/main/java/com/debatetimer/app/ui/home/HomeViewModel.kt (1)
7-8: LGTM — Hilt ViewModel 스켈레톤 적절현재 범위(하단 네비게이션 도입)에서 최소 구현으로 충분합니다.
app/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt (1)
15-19: 구조 적합NavHost를 홈 내부 그래프로 분리한 구조는 이후 화면 확장에 유리합니다.
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt (1)
17-33: 구현 무난Material3 NavigationBar/Item 사용과 선택 상태 계산 로직이 간결합니다.
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavItem.kt (2)
18-32: 구성 자체는 명료하고 사용처가 직관적입니다Home/Setting 두 탭의 라벨·아이콘 매핑이 일관적이며, 하단 바의 최소 기능을 충족합니다.
18-32: 네이밍 일관성 확인됨:BottomNavRoute.Setting, 문자열 리소스(setting), 화면 표시("Setting") 모두 단수형으로 일치하며, 아이콘(Settings) 네이밍은 Material Icons 규칙에 따른 예외입니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (1)
31-38: 보텀탭 재선택 중복 누적 방지 옵션 적용 완료popUpTo(findStartDestination) + launchSingleTop + restoreState 적용 확인했습니다. 이전 리뷰 제안이 반영되었습니다.
🧹 Nitpick comments (3)
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavRoute.kt (1)
3-7: value → route로 명명 변경 제안호출부 가독성 향상(.value 대신 .route). 아래처럼 변경 시, 사용처도
.value→.route로 교체 필요합니다.-sealed class BottomNavRoute( - val value: String -) { - object Tables : BottomNavRoute("/tables") - object Settings : BottomNavRoute("/settings") -} +sealed class BottomNavRoute( + val route: String +) { + object Tables : BottomNavRoute("/tables") + object Settings : BottomNavRoute("/settings") +}추가로 사용처 예:
- startDestination = BottomNavRoute.Tables.value + startDestination = BottomNavRoute.Tables.route - composable(BottomNavRoute.Tables.value) { ... } + composable(BottomNavRoute.Tables.route) { ... }app/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt (1)
20-28: 하드코딩 문자열 제거 + TODO 문구 정리로컬라이제이션을 위해 stringResource 사용, TODO 문법도 보정해 두세요.
import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier +import androidx.compose.ui.res.stringResource import androidx.navigation.NavHostController import androidx.navigation.compose.NavHost import androidx.navigation.compose.composable +import com.debatetimer.app.R @@ - composable(BottomNavRoute.Tables.value) { - // TODO: Need to replaced with actual view - Text("Tables") + composable(BottomNavRoute.Tables.value) { + // TODO: Need to be replaced with actual view + Text(stringResource(R.string.table)) } @@ - composable(BottomNavRoute.Settings.value) { - // TODO: Need to replaced with actual view - Text("Settings") + composable(BottomNavRoute.Settings.value) { + // TODO: Need to be replaced with actual view + Text(stringResource(R.string.setting)) }app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (1)
17-20: 사용되지 않는 viewModel 파라미터현재 미사용이라 Lint 경고/불필요한 초기화 가능성이 있습니다. 사용 시점에 도입하거나 주입을 본문으로 옮기길 권장합니다.
옵션 A(간단, 당장 미사용):
-@Composable -fun HomePage( - modifier: Modifier = Modifier, - viewModel: HomeViewModel = hiltViewModel() -) { +@Composable +fun HomePage( + modifier: Modifier = Modifier +) {옵션 B(곧 사용할 예정): 파일/함수에
@Suppress("UNUSED_PARAMETER")추가 후 유지.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavItem.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavRoute.kt(1 hunks)app/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/src/main/res/values/strings.xml
- app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavItem.kt
- app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-02T07:20:08.023Z
Learnt from: i-meant-to-be
PR: debate-timer/debate-timer-fe-android#11
File: app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt:10-14
Timestamp: 2025-08-02T07:20:08.023Z
Learning: In the debate-timer Android project, navigation destinations in MainNavHost.kt are being added incrementally during development, so an empty NavHost block in early stages is intentional and acceptable.
Applied to files:
app/src/main/java/com/debatetimer/app/ui/home/HomePage.ktapp/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt
🧬 Code graph analysis (1)
app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (2)
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavBar.kt (1)
BottomNavBar(11-34)app/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt (1)
HomeNavHost(10-30)
⏰ 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: build_and_test
🔇 Additional comments (4)
app/src/main/java/com/debatetimer/app/ui/home/component/BottomNavRoute.kt (2)
3-8: Sealed route 구성 깔끔합니다단순·타입세이프 구조로 충분히 읽기 좋습니다.
6-7: 경로 앞 슬래시 컨벤션 확인 요청Compose Navigation에서 route는 식별자이므로 슬래시가 필수는 아닙니다. 앱 전반(MainNavRoute 등)과 동일 컨벤션이라면 유지, 아니라면 통일을 권장합니다.
app/src/main/java/com/debatetimer/app/ui/home/component/HomeNavHost.kt (1)
15-19: NavHost 구성 적절startDestination/graph 배치 무난하고 확장 용이합니다.
app/src/main/java/com/debatetimer/app/ui/home/HomePage.kt (1)
21-24: 초기 route 처리 합리적백스택 엔트리 null 시 기본 탭으로 폴백하는 로직 적절합니다.
1 participant
🚩 연관 이슈
closed #10
📝 작업 내용
🏞️ 스크린샷 (선택)
🗣️ 리뷰 요구사항 (선택)
없음
Summary by CodeRabbit