Conversation
WalkthroughBottomNavigationBar를 NavigationBar 기반에서 BoxWithConstraints 기반의 커스텀 레이아웃으로 교체하고, 네비게이션 아이콘 리소스 참조를 새 아이콘으로 변경했으며 Modifier용 dropShadow 유틸리티를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant 사용자 as 사용자
participant UI as BottomNavigationBar
participant 상태 as NavState
participant 애니메이터 as SpringAnimator
사용자->>UI: 탭 클릭
UI->>상태: onTabSelected(newTab)
상태-->>UI: currentTab 업데이트
UI->>UI: 탭 너비 계산 및 targetOffset 결정
UI->>애니메이터: animateTo(targetOffset)
애니메이터-->>UI: 오프셋 애니메이션 프레임
UI->>사용자: 표시기 위치/라벨 변경 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 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
🧪 Generate unit tests (beta)
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: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt`:
- Around line 50-51: selectedIndex can be -1 when no tab route matches
currentTab, which later leads the when branch to hit else and place the
indicator incorrectly; change the selectedIndex computation
(BottomNavTab.entries and selectedIndex) to defensively handle -1 by
clamping/normalizing it (e.g., use .indexOfFirst { it.route == currentTab }.let
{ if (it == -1) 0 else it } or .coerceIn(0, tabs.lastIndex)) so the subsequent
when that maps index -> indicator position always receives a valid index.
🧹 Nitpick comments (2)
app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt (2)
71-80: 탭 개수가 변경될 경우 확장성 부족현재 indicator offset 계산이 정확히 3개의 탭에 맞춰 하드코딩되어 있습니다. 향후 탭이 추가되거나 변경될 경우 이 로직도 수동으로 수정해야 합니다.
♻️ 확장 가능한 계산 방식 제안
val indicatorOffset by animateDpAsState( - targetValue = when (selectedIndex) { - 0 -> 0.dp - 1 -> unselectedWidth - else -> totalWidth - selectedWidth - }, + targetValue = if (selectedIndex == 0) { + 0.dp + } else { + unselectedWidth * selectedIndex + }, animationSpec = spring(dampingRatio = Spring.DampingRatioLowBouncy, stiffness = Spring.StiffnessLow), )
134-138: Preview 함수 추가 👍다양한 탭 상태를 확인할 수 있도록 추가 Preview를 고려해볼 수 있습니다.
♻️ 다양한 상태 Preview 제안
`@Preview`(showBackground = true, name = "Travel Helper Selected") `@Composable` private fun BottomNavigationBarTravelHelperPreview() { BottomNavigationBar(currentTab = BottomNavTab.TRAVEL_HELPER.route, onTabSelected = {}) } `@Preview`(showBackground = true, name = "Travel Selected") `@Composable` private fun BottomNavigationBarTravelPreview() { BottomNavigationBar(currentTab = BottomNavTab.TRAVEL.route, onTabSelected = {}) }
jihee-dev
reviewed
Feb 5, 2026
mj010504
force-pushed
the
design/NDGL-71
branch
from
cc4c132 to
ff13908
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt`:
- Around line 91-127: 현재 탭 너비가 즉시 바뀌어 indicator의 spring 애니메이션과 시각적으로 어긋나므로,
tabs.forEach 블록 안에서 isSelected 기반으로 selectedWidth/unselectedWidth 값을
animateDpAsState로 래핑해 탭의 width에 적용하세요; 예를 들어 val tabWidth =
animateDpAsState(targetValue = if (isSelected) selectedWidth else
unselectedWidth, animationSpec = spring(...)) 형태로 생성하고 기존 Row의
Modifier.width(...)를 .width(tabWidth.value)로 교체해 indicator의 offset 애니메이션과 동기화되도록
만드세요 (참조: tabs.forEach, isSelected, selectedWidth, unselectedWidth,
onTabSelected, AnimatedVisibility).
🧹 Nitpick comments (1)
app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt (1)
74-81:when블록이 탭 3개에 하드코딩되어 있음현재
when(selectedIndex)로직이 0, 1, else로 고정되어 있어, 탭이 추가/삭제되면 indicator offset 계산이 깨집니다. 일반화된 계산으로 대체하면 유지보수가 용이합니다.♻️ 일반화 제안
val indicatorOffset by animateDpAsState( - targetValue = when (selectedIndex) { - 0 -> 0.dp - 1 -> unselectedWidth - else -> totalWidth - selectedWidth - }, + targetValue = unselectedWidth * selectedIndex, animationSpec = spring(dampingRatio = Spring.DampingRatioLowBouncy, stiffness = Spring.StiffnessLow), )
Comment on lines
+91
to
127
| Row(modifier = Modifier.fillMaxSize()) { | ||
| tabs.forEach { tab -> | ||
| val isSelected = currentTab == tab.route | ||
|
|
||
| Row( | ||
| modifier = Modifier | ||
| .width(if (isSelected) selectedWidth else unselectedWidth) | ||
| .fillMaxHeight() | ||
| .clickable( | ||
| interactionSource = remember { MutableInteractionSource() }, | ||
| indication = null, | ||
| ) { onTabSelected(tab.route) }, | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| horizontalArrangement = Arrangement.spacedBy(4.dp, Alignment.CenterHorizontally), | ||
| ) { | ||
| Icon( | ||
| imageVector = ImageVector.vectorResource(id = topLevelRoute.icon), | ||
| contentDescription = topLevelRoute.label, | ||
| modifier = Modifier.size(24.dp), | ||
| imageVector = ImageVector.vectorResource(id = tab.icon), | ||
| contentDescription = tab.label, | ||
| tint = if (isSelected) NDGLTheme.colors.white else NDGLTheme.colors.black600, | ||
| ) | ||
| }, | ||
| label = { Text(text = topLevelRoute.label) }, | ||
| ) | ||
|
|
||
| AnimatedVisibility( | ||
| visible = isSelected, | ||
| enter = fadeIn() + expandHorizontally(), | ||
| exit = fadeOut() + shrinkHorizontally(), | ||
| ) { | ||
| Text( | ||
| text = tab.label, | ||
| color = NDGLTheme.colors.white, | ||
| style = NDGLTheme.typography.bodyLgMedium, | ||
| maxLines = 1, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
탭 너비가 애니메이션 없이 즉시 변경됨 — indicator와 시각적 불일치 가능
Line 74-81에서 indicator의 offset은 spring 애니메이션으로 부드럽게 이동하지만, Line 97에서 각 탭의 width는 if/else로 즉시 전환됩니다. 이로 인해 indicator는 슬라이딩 중인데 탭 콘텐츠(아이콘+라벨)는 갑자기 위치가 바뀌는 현상이 발생할 수 있습니다.
탭 너비도 animateDpAsState로 감싸면 indicator와 동기화됩니다:
♻️ 탭 너비 애니메이션 적용 제안
tabs.forEach { tab ->
val isSelected = currentTab == tab.route
+ val tabWidth by animateDpAsState(
+ targetValue = if (isSelected) selectedWidth else unselectedWidth,
+ animationSpec = spring(dampingRatio = Spring.DampingRatioLowBouncy, stiffness = Spring.StiffnessLow),
+ )
Row(
modifier = Modifier
- .width(if (isSelected) selectedWidth else unselectedWidth)
+ .width(tabWidth)
.fillMaxHeight()📝 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.
Suggested change
| Row(modifier = Modifier.fillMaxSize()) { | |
| tabs.forEach { tab -> | |
| val isSelected = currentTab == tab.route | |
| Row( | |
| modifier = Modifier | |
| .width(if (isSelected) selectedWidth else unselectedWidth) | |
| .fillMaxHeight() | |
| .clickable( | |
| interactionSource = remember { MutableInteractionSource() }, | |
| indication = null, | |
| ) { onTabSelected(tab.route) }, | |
| verticalAlignment = Alignment.CenterVertically, | |
| horizontalArrangement = Arrangement.spacedBy(4.dp, Alignment.CenterHorizontally), | |
| ) { | |
| Icon( | |
| imageVector = ImageVector.vectorResource(id = topLevelRoute.icon), | |
| contentDescription = topLevelRoute.label, | |
| modifier = Modifier.size(24.dp), | |
| imageVector = ImageVector.vectorResource(id = tab.icon), | |
| contentDescription = tab.label, | |
| tint = if (isSelected) NDGLTheme.colors.white else NDGLTheme.colors.black600, | |
| ) | |
| }, | |
| label = { Text(text = topLevelRoute.label) }, | |
| ) | |
| AnimatedVisibility( | |
| visible = isSelected, | |
| enter = fadeIn() + expandHorizontally(), | |
| exit = fadeOut() + shrinkHorizontally(), | |
| ) { | |
| Text( | |
| text = tab.label, | |
| color = NDGLTheme.colors.white, | |
| style = NDGLTheme.typography.bodyLgMedium, | |
| maxLines = 1, | |
| ) | |
| } | |
| } | |
| } | |
| } | |
| Row(modifier = Modifier.fillMaxSize()) { | |
| tabs.forEach { tab -> | |
| val isSelected = currentTab == tab.route | |
| val tabWidth by animateDpAsState( | |
| targetValue = if (isSelected) selectedWidth else unselectedWidth, | |
| animationSpec = spring(dampingRatio = Spring.DampingRatioLowBouncy, stiffness = Spring.StiffnessLow), | |
| ) | |
| Row( | |
| modifier = Modifier | |
| .width(tabWidth) | |
| .fillMaxHeight() | |
| .clickable( | |
| interactionSource = remember { MutableInteractionSource() }, | |
| indication = null, | |
| ) { onTabSelected(tab.route) }, | |
| verticalAlignment = Alignment.CenterVertically, | |
| horizontalArrangement = Arrangement.spacedBy(4.dp, Alignment.CenterHorizontally), | |
| ) { | |
| Icon( | |
| modifier = Modifier.size(24.dp), | |
| imageVector = ImageVector.vectorResource(id = tab.icon), | |
| contentDescription = tab.label, | |
| tint = if (isSelected) NDGLTheme.colors.white else NDGLTheme.colors.black600, | |
| ) | |
| AnimatedVisibility( | |
| visible = isSelected, | |
| enter = fadeIn() + expandHorizontally(), | |
| exit = fadeOut() + shrinkHorizontally(), | |
| ) { | |
| Text( | |
| text = tab.label, | |
| color = NDGLTheme.colors.white, | |
| style = NDGLTheme.typography.bodyLgMedium, | |
| maxLines = 1, | |
| ) | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt` around lines 91 -
127, 현재 탭 너비가 즉시 바뀌어 indicator의 spring 애니메이션과 시각적으로 어긋나므로, tabs.forEach 블록 안에서
isSelected 기반으로 selectedWidth/unselectedWidth 값을 animateDpAsState로 래핑해 탭의 width에
적용하세요; 예를 들어 val tabWidth = animateDpAsState(targetValue = if (isSelected)
selectedWidth else unselectedWidth, animationSpec = spring(...)) 형태로 생성하고 기존
Row의 Modifier.width(...)를 .width(tabWidth.value)로 교체해 indicator의 offset 애니메이션과
동기화되도록 만드세요 (참조: tabs.forEach, isSelected, selectedWidth, unselectedWidth,
onTabSelected, AnimatedVisibility).
jihee-dev
approved these changes
Feb 7, 2026
Contributor
Author
|
@jihee-dev 
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/src/main/java/com/yapp/ndgl/ui/NDGLApp.kt`:
- Around line 57-59: Replace the unsafe cast of
navigationState.currentTopLevelKey to Route in the BottomNavigationBar call with
a safe cast and a fallback: use a safe cast (e.g. currentTopLevelKey as? Route)
and provide a default Route or null-handling before passing to
BottomNavigationBar so it never throws a ClassCastException during
AnimatedVisibility exit recompositions; update the expression used for
currentTab in the BottomNavigationBar invocation and ensure navigator.navigate
handling (onTabSelected) accepts the fallback case.
- Around line 40-51: Scaffold no longer supplies bottom inset because
BottomNavigationBar is overlaid via AnimatedVisibility, so screens rendered by
NavDisplay (used with entries = navigationState.toEntries(entryProvider) and
onBack = navigator::goBack) may be obscured; update the content padding passed
into those screens by adding a bottom padding equal to the navigation bar height
(68.dp + 20.dp) plus system navigationBarsPadding (or use
WindowInsets.navigationBars.asPaddingValues()) so Home, Travel, TravelHelper and
other entry composables receive Modifier.padding(innerPadding) extended with the
extra bottom inset to avoid content being hidden behind the overlaid
BottomNavigationBar.
🧹 Nitpick comments (2)
app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt (2)
72-79: Indicator offset 계산이 3개 탭에 하드코딩되어 있음현재
when블록은 인덱스 0, 1, else만 처리하므로 탭이 추가/삭제될 때마다 수정이 필요합니다. 선택된 탭 앞의 모든 탭이unselectedWidth이므로,selectedIndex * unselectedWidth로 일반화할 수 있습니다.♻️ 일반화된 offset 계산 제안
val indicatorOffset by animateDpAsState( - targetValue = when (selectedIndex) { - 0 -> 0.dp - 1 -> unselectedWidth - else -> totalWidth - selectedWidth - }, + targetValue = unselectedWidth * selectedIndex, animationSpec = spring(dampingRatio = Spring.DampingRatioLowBouncy, stiffness = Spring.StiffnessLow), )
53-67: 하드코딩된 dp 값들을 상수로 추출하는 것을 권장합니다
34.dp,68.dp,24.dp,20.dp,12.dp,6.dp,0.56f등 여러 매직 넘버가 컴포저블 내부에 산재해 있습니다. 파일 상단이나 companion object에 상수로 추출하면 디자인 변경 시 일괄 수정이 용이하고 가독성도 향상됩니다.
Comment on lines
+40
to
+51
| Box(modifier = Modifier.fillMaxSize()) { | ||
| Scaffold( | ||
| modifier = Modifier.fillMaxSize(), | ||
| ) { innerPadding -> | ||
| NavDisplay( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .padding(innerPadding), | ||
| onBack = navigator::goBack, | ||
| entries = navigationState.toEntries(entryProvider), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Searching for bottom padding handling in feature screens ==="
rg -n -C3 'bottomPadding\|navigationBarsPadding\|padding.*bottom\|contentPadding' --type=kotlin -g '*/feature/*' 2>/dev/null | head -100
echo ""
echo "=== Checking NDGLApp.kt file ==="
cat -n app/src/main/java/com/yapp/ndgl/ui/NDGLApp.kt | head -70
echo ""
echo "=== Searching for navigationState implementation ==="
fd -e kt -e java | xargs rg -l 'navigationState' | head -5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2947
🏁 Script executed:
#!/bin/bash
echo "=== Searching for BottomNavigationBar implementation ==="
rg -n 'fun BottomNavigationBar' --type=kotlin -A 15 | head -40
echo ""
echo "=== Searching for currentTopLevelKey implementation ==="
rg -n 'currentTopLevelKey' --type=kotlin -B 2 -A 2 | head -50
echo ""
echo "=== Checking Home feature screen ==="
fd -e kt -path '*/feature/home/*' | xargs cat -n | head -80
echo ""
echo "=== Checking if topLevelKeys contains Route objects ==="
rg -n 'topLevelKeys.*=' --type=kotlin -A 1 | head -30Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 5134
🏁 Script executed:
#!/bin/bash
echo "=== Finding Home feature screen files ==="
find app/src/main -path '*/feature/home/*' -name '*.kt' | head -10
echo ""
echo "=== Finding Travel feature screen files ==="
find app/src/main -path '*/feature/travel/*' -name '*.kt' | head -10
echo ""
echo "=== Searching for Home screen composable ==="
rg -n '@Composable.*Home\|fun Home' --type=kotlin -A 5 | head -40
echo ""
echo "=== Searching for any bottom padding in screens ==="
rg -n 'modifier.*padding.*bottom\|contentPadding' app/src/main --type=kotlin | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 264
🏁 Script executed:
#!/bin/bash
echo "=== Finding all Kotlin files in feature directories ==="
find app/src/main -type d -name 'feature' | head -5
echo ""
echo "=== Listing feature directory structure ==="
find app/src/main -path '*/feature/*' -name '*.kt' | head -20
echo ""
echo "=== Searching for Screen composables ==="
rg -n '@Composable.*Screen\|fun.*Screen' --type=kotlin | head -30
echo ""
echo "=== Getting full BottomNavigationBar.kt to understand padding ==="
cat -n app/src/main/java/com/yapp/ndgl/ui/BottomNavigationBar.kt | sed -n '46,70p'Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1322
BottomNavigationBar 오버레이로 인한 콘텐츠 하단 가림 현상
Scaffold의 bottomBar 파라미터를 제거하고 AnimatedVisibility로 오버레이된 BottomNavigationBar로 변경했으므로, innerPadding에 더 이상 하단 네비게이션 바 높이(68.dp + 20.dp + navigationBarsPadding)가 반영되지 않습니다. 결과적으로 각 화면의 하단 콘텐츠가 네비게이션 바에 의해 가려질 가능성이 있습니다.
Home, Travel, TravelHelper 등의 화면들이 적절한 하단 padding을 적용하여 콘텐츠가 네비게이션 바 뒤에 숨겨지지 않도록 해야 합니다.
🤖 Prompt for AI Agents
In `@app/src/main/java/com/yapp/ndgl/ui/NDGLApp.kt` around lines 40 - 51, Scaffold
no longer supplies bottom inset because BottomNavigationBar is overlaid via
AnimatedVisibility, so screens rendered by NavDisplay (used with entries =
navigationState.toEntries(entryProvider) and onBack = navigator::goBack) may be
obscured; update the content padding passed into those screens by adding a
bottom padding equal to the navigation bar height (68.dp + 20.dp) plus system
navigationBarsPadding (or use WindowInsets.navigationBars.asPaddingValues()) so
Home, Travel, TravelHelper and other entry composables receive
Modifier.padding(innerPadding) extended with the extra bottom inset to avoid
content being hidden behind the overlaid BottomNavigationBar.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
변경사항
테스트 체크 리스트
Summary by CodeRabbit
새로운 기능
스타일