Skip to content

Conversation

@oungsi2000
Copy link
Contributor

@oungsi2000 oungsi2000 commented Jan 16, 2026

#️⃣ 이슈 번호

#31


🛠️ 작업 내용

한꺼번에 마이그레이션 하면 File Changes가 폭발할 것 같아서 작업을 나눠서 가져가려고 합니다.

  • BottomNavigationBar를 Compose로 마이그레이션 하였습니다.
    아직 MainActivity는 연동 전이기 때문에 기존 바텀네비게이션 뷰가 작동하고 있습니다.
    먼저 새롭게 추가된 Composable 함수 검토 + NavHost의 컨벤션 등을 정하고 난 후, 나머지 작업을 마저 하겠습니다.

추후 SnackBarHost 추가 + 미뤄왔던 SnackBar 연동 작업 또한 연속으로 가져가면 좋을 것 같습니다.


🙇🏻 중점 리뷰 요청

  • HomeFragment 부분 코드는 제가 생각한 예제 코드입니다.
  • 다음 작업을 진행하게 될 때 FestabookNavigator, MainScreen의 경우 큰 변화가 일어날 것으로 예상됩니다.
    파일의 내용을 중점적으로 보기보다는 어떤 파일이 추가되었는지 를 위주로, 설계 부분을 리뷰해주시면 감사하겠습니다.

📸 이미지 첨부 (Optional)

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능
    • 앱의 주요 네비게이션 구조를 구현했습니다. 하단에 5개의 탭(홈, 일정, 지도, 뉴스, 설정)이 있는 탭 기반 네비게이션 인터페이스가 추가되었습니다.
    • 하단 네비게이션 바에 중앙 위치의 PlaceMap 버튼이 추가되었습니다.
    • 화면 간 이동을 위한 라우팅 시스템이 구현되었습니다.

✏️ Tip: You can customize this high-level summary in your review settings.

Navigation Compose 라이브러리를 프로젝트에 추가하여 Jetpack Compose 환경에서의 화면 전환 및 탐색 기능을 구현할 수 있도록 준비했습니다.

- **`gradle/libs.versions.toml` 수정:**
    - `navigationCompose` 버전 변수(`2.9.6`)를 추가했습니다.
    - `androidx.navigation:navigation-compose` 라이브러리를 `androidx-navigation-compose`라는 이름으로 등록했습니다.

- **`app/build.gradle.kts` 수정:**
    - `dependencies` 블록에 `implementation(libs.androidx.navigation.compose)`를 추가하여 Navigation Compose 라이브러리를 모듈에 적용했습니다.
메인 화면의 하단 네비게이션 바 UI와 관련된 컴포저블 및 라우팅 구조를 추가했습니다.

- **`FestabookMainTab.kt` 추가:**
    - `HOME`, `SCHEDULE`, `PLACE_MAP`, `NEWS`, `SETTING` 5개의 탭을 정의하는 `enum class`를 추가했습니다.
    - 각 탭은 아이콘, 라벨, 라우트 정보 등을 포함합니다.

- **`component/FestabookBottomNavigationBar.kt` 추가:**
    - `FestabookBottomNavigationBar` 컴포저블을 구현하여 하단 탭 UI를 구성했습니다.
    - 중앙의 `PLACE_MAP` 탭은 Floating Action Button(FAB) 스타일로 돌출되도록 별도 처리했습니다.

- **`FestabookRoute.kt` 추가:**
    - 앱 전반의 화면 전환을 관리하기 위한 `FestabookRoute`와 메인 탭에 해당하는 `FestabookMainRoute` sealed interface를 정의했습니다.
    - `kotlinx.serialization`을 사용하여 각 라우트를 직렬화 가능하도록 설정했습니다.
Compose `NavHost`를 기반으로 한 메인 화면의 네비게이션 구조를 구현하고, 하단 네비게이션 바와 각 탭의 화면을 연결했습니다. `FestabookNavigator` 클래스를 도입하여 화면 간 이동 로직을 중앙에서 관리하도록 했습니다.

- **`FestabookNavigator.kt` 추가:**
    - `NavHostController`를 감싸는 `FestabookNavigator` 클래스를 추가하여 네비게이션 로직을 캡슐화했습니다.
    - 현재 경로에 따라 현재 탭(`currentTab`)과 하단 바 표시 여부(`shouldShowBottomBar`)를 결정하는 로직을 구현했습니다.
    - `rememberFestabookNavigator` 컴포저블을 추가하여 네비게이터 인스턴스를 쉽게 생성하고 기억할 수 있도록 했습니다.

- **`MainScreen.kt` 추가:**
    - `Scaffold`와 `NavHost`를 사용하여 메인 화면의 전체적인 레이아웃을 구성했습니다.
    - `FestabookBottomNavigationBar`와 `NavHost`를 `FestabookNavigator`와 연동하여 탭 선택 시 화면이 전환되도록 구현했습니다.

- **`homeNavGraph.kt` 추가:**
    - 홈 화면(`HomeScreen`)에 대한 `NavGraph` 확장 함수를 추가하여, 메인 `NavHost`에서 홈 그래프를 모듈화하여 관리할 수 있도록 했습니다.

- **`FestabookMainTab.kt` 수정:**
    - `find` 컴패니언 함수를 추가하여, 주어진 조건에 맞는 `FestabookMainTab`을 찾을 수 있도록 유틸리티 기능을 제공했습니다.
    - 이 함수는 `FestabookNavigator`에서 현재 활성화된 탭을 찾는 데 사용됩니다.
@oungsi2000 oungsi2000 self-assigned this Jan 16, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 16, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

네비게이션 인프라를 위해 androidx.navigation.compose 라이브러리를 추가하고, 직렬화된 경로 정의, 탭 열거형, 네비게이터 래퍼, 하단 네비게이션 바 컴포넌트, 홈 네비게이션 그래프를 구현한 새로운 Compose 기반 네비게이션 시스템을 도입합니다.

Changes

Cohort / File(s) 변경 사항
의존성 및 라이브러리
app/build.gradle.kts, gradle/libs.versions.toml
androidx.navigation.compose v2.9.6 의존성 추가
경로 정의
app/src/main/java/com/daedan/festabook/presentation/main/FestabookRoute.kt
FestabookRouteFestabookMainRoute 직렬화 가능 sealed 인터페이스 도입 (Splash, PlaceDetail, Explore, Home, Schedule, PlaceMap, News, Setting 포함)
탭 구조 및 네비게이션 논리
app/src/main/java/com/daedan/festabook/presentation/main/FestabookMainTab.kt, app/src/main/java/com/daedan/festabook/presentation/main/FestabookNavigator.kt
FestabookMainTab 열거형 (아이콘, 레이블, 경로 메타데이터 포함) 및 NavHostController를 감싸는 FestabookNavigator 클래스 추가
UI 컴포넌트
app/src/main/java/com/daedan/festabook/presentation/main/component/FestabookBottomNavigationBar.kt, app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt
FestabookBottomNavigationBar (PlaceMap FAB 오버레이 포함) 및 NavHost가 통합된 MainScreen 컴포넌트 구현
홈 네비게이션 그래프
app/src/main/java/com/daedan/festabook/presentation/home/navigation/HomeNavigation.kt
NavGraphBuilder.extension 함수 homeNavGraph 추가로 Home 경로 등록 및 HomeScreen 렌더링

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

  • [Feat] 홈화면 Compose 마이그레이션 #22: 메인 PR에서 네비게이션, 경로, HomeScreen 컴포저블을 등록하고 네비게이션하는 MainScreen NavHost를 추가하며, 해당 PR은 HomeScreen 및 관련 Compose 홈 컴포넌트를 구현하므로 코드 수준에서 직접적으로 연관됩니다.

Suggested reviewers

  • etama123
  • parkjiminnnn

Poem

🐰 탭들이 춤을 추고 경로가 노래하네,
네비게이션 새 옷으로 갈아입고 반짝거려,
BottomBar 친구 PlaceMap과 함께하며,
Compose의 마법으로 화면 넘나드는,
우리의 Festabook 더 멋져졌어! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 BottomNavigationBar를 Jetpack Compose로 마이그레이션한다는 주요 변경사항을 명확하게 설명합니다.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@oungsi2000
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Jan 16, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/daedan/festabook/presentation/main/component/FestabookBottomNavigationBar.kt`:
- Around line 115-134: PlaceMapNavigationItem uses contentDescription = null and
interactionSource = null which harms accessibility and is discouraged; update
the Image clickable invocation in PlaceMapNavigationItem to provide a meaningful
contentDescription (use an existing string resource or add one like "Open place
map") and supply a remembered interaction source via remember {
MutableInteractionSource() } (pass it as the interactionSource parameter to
clickable). Keep the onClick behavior (onClick(FestabookMainTab.PLACE_MAP))
intact and ensure the description string is localized if you add a resource.

In
`@app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt`:
- Around line 20-27: MainScreen currently always renders
FestabookBottomNavigationBar; change the Scaffold's bottomBar to render
conditionally using navigator.shouldShowBottomBar (from FestabookNavigator) so
the bar is shown only on main tabs (e.g., Splash or PlaceDetail will hide it);
implement bottomBar = if (navigator.shouldShowBottomBar) {
FestabookBottomNavigationBar(currentTab = navigator.currentTab, onTabSelect = {
navigator.navigateToMainTab(it.route) }) } else null, keeping references to
FestabookBottomNavigationBar, navigator.currentTab and
navigator.navigateToMainTab.
🧹 Nitpick comments (5)
app/src/main/java/com/daedan/festabook/presentation/main/FestabookRoute.kt (1)

10-12: TODO 추적: PlaceDetail 라우트에 UiModel 직렬화 필요

PlaceDetail 라우트에 파라미터를 추가할 때 @Serializable 어노테이션이 적용된 UiModel이 필요합니다.

이 작업을 추적하기 위한 이슈를 생성해 드릴까요?

app/src/main/java/com/daedan/festabook/presentation/main/FestabookMainTab.kt (2)

16-45: contentDescription에 하드코딩된 문자열 대신 문자열 리소스 사용을 권장합니다.

labelResId는 문자열 리소스를 사용하고 있지만 contentDescription은 하드코딩된 한국어 문자열을 사용하고 있습니다. 일관성과 국제화(i18n) 지원을 위해 문자열 리소스를 사용하는 것이 좋습니다.

♻️ 권장 수정안
 enum class FestabookMainTab(
     `@DrawableRes` val iconResId: Int,
     `@StringRes` val labelResId: Int,
-    val contentDescription: String,
+    `@StringRes` val contentDescriptionResId: Int,
     val route: FestabookMainRoute,
 ) {
     HOME(
         iconResId = R.drawable.ic_home,
         labelResId = R.string.menu_home_title,
-        contentDescription = "홈",
+        contentDescriptionResId = R.string.menu_home_content_description,
         route = FestabookMainRoute.Home,
     ),
     // ... 나머지 항목들도 동일하게 수정
 }

49-53: find 함수의 @Composable predicate 사용을 재고해 주세요.

@Composable predicate를 사용하면 불필요한 리컴포지션이 발생할 수 있습니다. 현재 사용처인 FestabookNavigator에서는 hasRoute 체크만 수행하므로, 일반 람다로 충분합니다.

♻️ 단순화된 구현
-        `@Composable`
-        fun find(predicate: `@Composable` (FestabookRoute) -> Boolean) =
+        fun find(predicate: (FestabookRoute) -> Boolean): FestabookMainTab? =
             entries.find {
                 predicate(it.route)
             }
app/src/main/java/com/daedan/festabook/presentation/main/FestabookNavigator.kt (1)

40-45: shouldShowBottomBarcurrentTab 로직을 중복합니다.

shouldShowBottomBarcurrentTab != null로 간단히 표현할 수 있습니다.

♻️ 간소화된 구현
     val shouldShowBottomBar
         `@Composable`
-        get() =
-            FestabookMainTab.find {
-                currentDestination?.hasRoute(it::class) == true
-            } != null
+        get() = currentTab != null
app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt (1)

14-17: ViewModel 스코핑 패턴 검토

HomeViewModel이 외부에서 주입되고 있습니다. 향후 Navigation Compose로 완전히 마이그레이션 시, hiltViewModel() 또는 viewModel()을 사용하여 navigation graph 내에서 ViewModel을 스코핑하는 것이 일반적인 패턴입니다. 현재는 예시 코드라 괜찮지만, 추후 리팩토링 시 고려해 주세요.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e8cc52 and 75d7655.

📒 Files selected for processing (8)
  • app/build.gradle.kts
  • app/src/main/java/com/daedan/festabook/presentation/home/navigation/HomeNavigation.kt
  • app/src/main/java/com/daedan/festabook/presentation/main/FestabookMainTab.kt
  • app/src/main/java/com/daedan/festabook/presentation/main/FestabookNavigator.kt
  • app/src/main/java/com/daedan/festabook/presentation/main/FestabookRoute.kt
  • app/src/main/java/com/daedan/festabook/presentation/main/component/FestabookBottomNavigationBar.kt
  • app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt
  • gradle/libs.versions.toml
🧰 Additional context used
🧬 Code graph analysis (3)
app/src/main/java/com/daedan/festabook/presentation/home/navigation/HomeNavigation.kt (1)
app/src/main/java/com/daedan/festabook/presentation/home/component/HomeScreen.kt (1)
  • HomeScreen (35-68)
app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt (3)
app/src/main/java/com/daedan/festabook/presentation/main/FestabookNavigator.kt (1)
  • rememberFestabookNavigator (67-73)
app/src/main/java/com/daedan/festabook/presentation/main/component/FestabookBottomNavigationBar.kt (1)
  • FestabookBottomNavigationBar (41-73)
app/src/main/java/com/daedan/festabook/presentation/home/navigation/HomeNavigation.kt (1)
  • homeNavGraph (12-24)
app/src/main/java/com/daedan/festabook/presentation/main/component/FestabookBottomNavigationBar.kt (1)
app/src/main/java/com/daedan/festabook/presentation/theme/FestabookTheme.kt (1)
  • FestabookTheme (15-34)
⏰ 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: Run-PR-Test
🔇 Additional comments (10)
app/src/main/java/com/daedan/festabook/presentation/main/FestabookRoute.kt (1)

5-16: LGTM! 라우트 구조가 잘 설계되어 있습니다.

FestabookRouteFestabookMainRoute의 계층 구조가 명확하고, kotlinx.serialization을 사용한 타입 안전한 네비게이션 패턴을 올바르게 적용했습니다.

app/src/main/java/com/daedan/festabook/presentation/main/FestabookNavigator.kt (2)

67-73: LGTM! rememberFestabookNavigator 구현이 적절합니다.

rememberNavController()가 안정적인 인스턴스를 반환하므로 현재 구현이 올바르게 동작합니다. 네비게이션 상태 관리가 잘 캡슐화되어 있습니다.


47-47: TODO 확인: Splash 및 Explore 연동 시 startRoute 변경 필요

현재 startRouteHome으로 하드코딩되어 있습니다. 향후 Splash 화면 연동 시 조건부 로직이 필요할 수 있습니다.

app/build.gradle.kts (1)

153-153: 의존성이 올바르게 추가되었습니다. 버전 2.9.6은 최신 안정 버전입니다.

androidx.navigation.compose 2.9.6(2025년 11월 5일 릴리스)은 현재 최신 안정 버전이며, Compose 네비게이션 인프라에 필수적인 의존성입니다.

app/src/main/java/com/daedan/festabook/presentation/home/navigation/HomeNavigation.kt (1)

12-24: 이 패턴은 이 프로젝트의 아키텍처에 맞는 적절한 방식입니다.

homeViewModel은 MainActivity에서 by viewModels() 위임을 통해 생성되며, Activity 생명주기로 스코핑됩니다. Activity 스코프를 가진 ViewModel을 Compose 네비게이션 계층을 통해 전달하는 것은 이 하이브리드 아키텍처(Fragment 기반 네비게이션 + Compose 컴포넌트)에서는 타당한 패턴입니다. Metro DI의 MetroViewModelFactory가 올바르게 ViewModel 생명주기를 관리하고 있으므로, 상태 복원도 자동으로 이루어집니다.

Likely an incorrect or invalid review comment.

app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt (1)

30-44: NavHost 설정 LGTM

startDestinationnavController가 올바르게 연결되어 있고, homeNavGraph가 적절히 등록되어 있습니다. TODO 주석으로 향후 작업이 명확히 표시되어 있습니다.

app/src/main/java/com/daedan/festabook/presentation/main/component/FestabookBottomNavigationBar.kt (3)

51-70: 하단 네비게이션 바 구현 LGTM

중앙 FAB 스타일의 PLACE_MAP 탭을 위한 레이아웃 구조가 적절합니다. Spacer를 사용하여 중앙 공간을 확보하고, 나머지 탭들을 균등하게 배치하는 방식이 잘 설계되어 있습니다.


75-113: FestabookNavigationItem 구현 LGTM

selectable modifier와 조건부 색상 적용이 올바르게 구현되어 있습니다. interactionSource가 적절히 remember되어 있고, 아이콘에 contentDescription이 설정되어 접근성도 고려되어 있습니다.


136-150: Preview 구현 LGTM

FestabookTheme으로 감싸고 로컬 상태를 사용한 인터랙티브 프리뷰가 잘 구현되어 있습니다.

gradle/libs.versions.toml (1)

31-31: Navigation Compose 버전 2.9.6은 안정 버전입니다

네, navigationCompose = "2.9.6"은 2025년 11월 5일에 릴리스된 androidx.navigation:navigation-compose의 최신 안정 버전입니다. 이 버전 지정은 정상이며 승인됩니다.

Also applies to: 52-52

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Compose `NavHost`를 기반으로 한 메인 화면의 네비게이션 구조를 구현하고, 하단 네비게이션 바와 각 탭의 화면을 연결했습니다. `FestabookNavigator` 클래스를 도입하여 화면 간 이동 로직을 중앙에서 관리하도록 했습니다.

- **`FestabookNavigator.kt` 추가:**
    - `NavHostController`를 감싸는 `FestabookNavigator` 클래스를 추가하여 네비게이션 로직을 캡슐화했습니다.
    - 현재 경로에 따라 현재 탭(`currentTab`)과 하단 바 표시 여부(`shouldShowBottomBar`)를 결정하는 로직을 구현했습니다.
    - `rememberFestabookNavigator` 컴포저블을 추가하여 네비게이터 인스턴스를 쉽게 생성하고 기억할 수 있도록 했습니다.

- **`MainScreen.kt` 추가:**
    - `Scaffold`와 `NavHost`를 사용하여 메인 화면의 전체적인 레이아웃을 구성했습니다.
    - `FestabookBottomNavigationBar`와 `NavHost`를 `FestabookNavigator`와 연동하여 탭 선택 시 화면이 전환되도록 구현했습니다.

- **`homeNavGraph.kt` 추가:**
    - 홈 화면(`HomeScreen`)에 대한 `NavGraph` 확장 함수를 추가하여, 메인 `NavHost`에서 홈 그래프를 모듈화하여 관리할 수 있도록 했습니다.

- **`FestabookMainTab.kt` 수정:**
    - `find` 컴패니언 함수를 추가하여, 주어진 조건에 맞는 `FestabookMainTab`을 찾을 수 있도록 유틸리티 기능을 제공했습니다.
    - 이 함수는 `FestabookNavigator`에서 현재 활성화된 탭을 찾는 데 사용됩니다.
`FestabookBottomNavigationBar`의 플로팅 액션 버튼(FAB)의 접근성과 사용자 인터랙션을 개선했습니다.

- **`FestabookBottomNavigationBar.kt` 수정:**
    - `interactionSource`에 `null` 대신 `MutableInteractionSource`를 명시적으로 전달하여 클릭 시 시각적 피드백(리플 효과)이 표시되도록 수정했습니다.
    - 접근성 향상을 위해 `contentDescription`에 `null` 대신 `PLACE_MAP` 탭에 정의된 설명을 사용하도록 변경했습니다.
`FestabookBottomNavigationBar`의 플로팅 액션 버튼(FAB)의 접근성과 사용자 인터랙션을 개선했습니다.

- **`FestabookBottomNavigationBar.kt` 수정:**
    - `interactionSource`에 `null` 대신 `MutableInteractionSource`를 명시적으로 전달하여 클릭 시 시각적 피드백(리플 효과)이 표시되도록 수정했습니다.
    - 접근성 향상을 위해 `contentDescription`에 `null` 대신 `PLACE_MAP` 탭에 정의된 설명을 사용하도록 변경했습니다.
Copy link
Contributor

@parkjiminnnn parkjiminnnn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생했어요 밀러~
코멘트 확인해주세요!

Comment on lines 16 to 45
HOME(
iconResId = R.drawable.ic_home,
labelResId = R.string.menu_home_title,
contentDescription = "",
route = FestabookMainRoute.Home,
),
SCHEDULE(
iconResId = R.drawable.ic_schedule,
labelResId = R.string.menu_schedule_title,
contentDescription = "일정",
route = FestabookMainRoute.Schedule,
),
PLACE_MAP(
iconResId = R.drawable.ic_map,
labelResId = R.string.menu_map_title,
contentDescription = "지도",
route = FestabookMainRoute.PlaceMap,
),
NEWS(
iconResId = R.drawable.ic_news,
labelResId = R.string.menu_news_title,
contentDescription = "뉴스",
route = FestabookMainRoute.News,
),
SETTING(
iconResId = R.drawable.ic_setting,
labelResId = R.string.menu_setting_title,
contentDescription = "설정",
route = FestabookMainRoute.Setting,
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contentDescriptionstring.xml에 정의된 걸로 변경 부탁드려요!

Copy link
Contributor Author

@oungsi2000 oungsi2000 Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contentDescription이 따�로 정의된게 없어서 labelResId를 사용하도록 변경했습니다 !
refactor(Main): 접근성 강화를 위해 contentDescription 속성 변경

Comment on lines 11 to 12
@DrawableRes val iconResId: Int,
@StringRes val labelResId: Int,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

param: 붙여주시면 감사하겠습니당

Comment on lines +67 to +73
@Composable
fun rememberFestabookNavigator(): FestabookNavigator {
val navController = rememberNavController()
return remember {
FestabookNavigator(navController = navController)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수로 따로 빼신 이유가 궁금합니다 !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

navController와 생명주기를 동일하게 가져가도록 보장하고 싶었기 때문입니다 !

Comment on lines 5 to 34
@Serializable
sealed interface FestabookRoute {
@Serializable
data object Splash : FestabookRoute

// TODO: PlaceUiModel, PlaceDetailUiModel 생성자에 추가 후 UiModel에 @Serializable 어노테이션 필요
@Serializable
data object PlaceDetail : FestabookRoute

@Serializable
data object Explore : FestabookRoute
}

@Serializable
sealed interface FestabookMainRoute : FestabookRoute {
@Serializable
data object Home : FestabookMainRoute

@Serializable
data object Schedule : FestabookMainRoute

@Serializable
data object PlaceMap : FestabookMainRoute

@Serializable
data object News : FestabookMainRoute

@Serializable
data object Setting : FestabookMainRoute
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum말고 sealed interface로 하신 이유가 궁금해요! 원래 바텀네비게이션은 인터페이스 타입으로 하나요?(단순 궁금)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum은 모든 요소에 동일한 프로퍼티를 담아야 하지만, sealed interface로 하면 각 라우트 별로 별개의 데이터를 담을 수 있어서 채택했습니다.

일례로 PlaceDetail의 경우 UiModel을 넘겨받습니다. 이런 상황에 유연하게 대응하려면 sealed interface 로 하는것이 좋다고 생각했어요.

@@ -0,0 +1,24 @@
package com.daedan.festabook.presentation.home.navigation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HomeNavigation과 FestabookNavigator의 패키지 위치를 지금처럼 설정한 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

navGraph의 경우 각 패키지 별로 설정이 다 다르기 때문에 한 곳에서 모아서 관리하는 것보다, 각 패키지 별로 관리하는 것이 더 가독성이 높다고 판단했습니다.

Comment on lines 35 to 37
get() =
FestabookMainTab.find {
currentDestination?.hasRoute(it::class) == true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

중복되는 계산을 한 번만 하도록 수정하면 어떨까요?

Copy link
Contributor Author

@oungsi2000 oungsi2000 Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currentDestination가 null인 경우 false를 반환하도록 설정했습니다.
refactor(Main): FestabookNavigator의 중복된 boolean 처리 개선

Comment on lines +24 to +31
private val defaultNavOptions =
navOptions {
popUpTo(navController.graph.findStartDestination().id) {
saveState = true
}
launchSingleTop = true
restoreState = true
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 네비게이션 옵션을 val 로 설정한 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

공통적으로 쓰일 확률이 높아 옵션을 재사용하기 위해 이렇게 구성했습니다.

)
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

뒤로가기 기능을 하는 함수도 만들어두면 좋을 것 같아요 ~

Copy link
Contributor Author

@oungsi2000 oungsi2000 Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contentDescription = "일정",
route = FestabookMainRoute.Schedule,
),
PLACE_MAP(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 메뉴는 MAP으로 네이밍해도 괜찮지 않을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 패키지 네이밍을 따라가는 것이 더 직관적이라고 생각했습니다..!

}

@Serializable
sealed interface FestabookMainRoute : FestabookRoute {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FestabookMainRoute이 FestabookRoute와 비교해서 어떤 화면을 보여주고 있는 지 모호하게 느껴져요😥
MainTabRoute 혹은 BottomNavRoute로 네이밍을 변경하는 건 어떤지 조심스레 제안해봅니다,,!

Copy link
Contributor Author

@oungsi2000 oungsi2000 Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MainTabRoute가 더 좋아보이네요 ! 반영했습니다
refactor(main): FestabookMainRoute 이름을 MainTabRoute로 변경

@etama123 etama123 self-requested a review January 18, 2026 17:38
Copy link
Contributor

@etama123 etama123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!
몇 가지 리뷰를 남겼으니 확인 부탁드립니다☺️

하단 네비게이션 바 컴포넌트의 접근성을 개선했습니다.

- **`FestabookMainTab.kt` 수정:**
    - 각 탭에 하드코딩되어 있던 `contentDescription` 문자열 속성을 제거했습니다.

- **`FestabookBottomNavigationBar.kt` 수정:**
    - 각 탭 아이콘과 플로팅 액션 버튼(FAB)의 `contentDescription`에 기존 문자열 대신 `stringResource`를 사용하도록 변경했습니다. 이를 통해 다국어 지원 및 접근성 관리가 용이해졌습니다.
`FestabookNavigator`에서 `currentDestination`이 `null`일 경우 발생할 수 있는 잠재적 `NullPointerException`을 방지하기 위해 코드 안정성을 개선했습니다.

- **`FestabookNavigator.kt` 수정:**
    - `currentTab`과 `shouldShowBottomBar`의 `get()` 메서드 내에서 `currentDestination?.hasRoute(...)`의 결과가 `null`일 경우 `false`를 반환하도록 엘비스 연산자(`?: false`)를 추가했습니다.
`FestabookNavigator` 클래스에 `popBackStack` 메서드를 추가하여 외부에서 화면 스택을 조작할 수 있도록 기능을 확장했습니다.

- **`FestabookNavigator.kt` 수정:**
    - `navController.popBackStack()`를 호출하는 `popBackStack()` 함수를 추가했습니다.
    - 특정 라우트까지 스택을 제거할 수 있도록 `route`와 `inclusive` 파라미터를 받는 오버로딩된 `popBackStack()` 함수를 추가했습니다.
`FestabookMainRoute`라는 `sealed interface`의 이름을 `MainTabRoute`로 변경하여, 메인 화면의 탭 라우트를 나타내는 역할임을 더 명확하게 표현했습니다. 이 변경사항을 관련된 모든 파일에 적용했습니다.

- **`FestabookRoute.kt` 수정:**
    - `FestabookMainRoute`를 `MainTabRoute`로 이름 변경했습니다.
- **`FestabookMainTab.kt` 수정:**
    - 각 탭(`enum`)이 참조하는 라우트 타입을 `MainTabRoute`로 업데이트했습니다.
- **`HomeNavigation.kt` 수정:**
    - `homeNavGraph`에서 사용하는 라우트 타입을 `MainTabRoute.Home`으로 변경했습니다.
- **`FestabookNavigator.kt` 수정:**
    - `startRoute`의 타입을 `MainTabRoute.Home`으로 업데이트했습니다.
@oungsi2000 oungsi2000 requested a review from etama123 January 19, 2026 07:59
@oungsi2000 oungsi2000 merged commit 6bc11a6 into develop Jan 21, 2026
5 checks passed
@oungsi2000 oungsi2000 deleted the feat/31 branch January 21, 2026 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants