[Feat] MainActivity Compose 연동 및 하위 Fragment 제거 #40
Conversation
기존 XML 기반의 `BottomNavigationView`와 `FloatingActionButton`으로 구현되었던 메인 화면의 하단 네비게이션 UI를 Jetpack Compose 기반으로 전면 교체했습니다. 이로 인해 XML 레이아웃과 관련된 코드들이 제거되고, Compose 기반의 상태 관리 로직으로 변경되었습니다.
- **`MainActivity.kt` 수정:**
- `BottomNavigationView`와 관련된 기존 리스너(`setOnItemSelectedListener`, `setOnItemReselectedListener`) 및 클릭 핸들러 로직을 제거했습니다.
- `ComposeView`를 추가하고 `FestabookBottomNavigationBar` 컴포저블을 설정했습니다.
- 탭 선택 상태를 관리하기 위해 `MutableState`(`currentTabState`)를 도입하고, 탭 선택 시 `Fragment`를 전환하는 로직을 Compose 콜백 내에서 처리하도록 변경했습니다.
- `handleNavigation`, `navigateToScheduleEvent` 등 네비게이션 관련 로직을 `currentTabState`를 변경하도록 수정했습니다.
- **`activity_main.xml` 수정:**
- 기존의 `CoordinatorLayout`, `BottomAppBar`, `BottomNavigationView`, `FloatingActionButton` 뷰들을 주석 처리하고, `FragmentContainerView`와 하단 네비게이션을 위한 `ComposeView`(`cv_main`)만 남겨두었습니다.
- **`PermissionUtil.kt` & `FragmentUtil.kt` 수정:**
- `Snackbar`의 `anchorView`를 기존 `bab_menu` ID에서 새로운 ComposeView의 ID인 `cv_main`으로 변경하여, 스낵바가 새로운 하단 네비게이션 바 위에 올바르게 표시되도록 수정했습니다.
메인 화면의 각 탭(소식, 장소, 일정, 설정)에 해당하는 `NavGraph`를 별도의 파일로 분리하여 모듈화했습니다. 이 리팩토링을 통해 `NavHost`의 구조를 더 깔끔하게 관리하고, 각 화면의 네비게이션 관련 로직을 독립적으로 구성할 수 있도록 개선했습니다. - **`newsNavGraph` 추가:** `NewsScreen`을 `MainTabRoute.News`와 연결하는 네비게이션 그래프를 정의했습니다. - **`placeMapNavGraph` 추가:** `PlaceMapRoute`를 `MainTabRoute.PlaceMap`과 연결하는 네비게이션 그래프를 정의했습니다. - **`scheduleNavGraph` 추가:** `ScheduleScreen`을 `MainTabRoute.Schedule`과 연결하는 네비게이션 그래프를 정의했습니다. - **`settingNavGraph` 추가:** `SettingRoute`를 `MainTabRoute.Setting`과 연결하는 네비게이션 그래프를 정의했습니다.
`FestabookNavigator`에서 현재 목적지(`currentDestination`)를 찾는 로직을 수정하여 안정성을 높였습니다.
- **`FestabookNavigator.kt` 수정:**
- 기존 `currentBackStackEntryAsState` 대신 `visibleEntries.collectAsState()`를 사용하여 현재 화면에 보이는 `NavEntry` 목록을 수집하도록 변경했습니다.
- `lastOrNull`을 이용해 이 목록에서 유효한 `route`를 가진 마지막 `destination`을 찾도록 하여, 화면 전환 중 발생할 수 있는 잠재적 문제를 방지했습니다.
- `navigateToMainTab` 함수 내에서 사용되던 `defaultNavOptions`를 인라인 `navOptions` 블록으로 대체했습니다.
기존 `MainActivity`에서 Fragment와 Compose를 혼용하던 구조를 완전히 Compose 기반으로 전환했습니다. 이로 인해 Fragment 관련 코드가 대부분 제거되고, 모든 화면이 `NavHost`를 통해 관리됩니다.
- **`MainActivity.kt` 수정:**
- `setContent`를 `Activity`의 최상위 컨텐츠로 설정하고, 모든 UI를 `MainScreen` 컴포저블로 대체했습니다.
- Fragment를 전환하던 로직(`switchFragment`)과 관련 태그(`TAG_HOME_FRAGMENT` 등)를 모두 삭제했습니다.
- `setupBinding`, `setupHomeFragment` 등 기존 View 시스템과 Fragment 초기화 관련 코드를 제거했습니다.
- 지도 화면에 필요한 `FusedLocationSource`를 초기화하는 로직을 추가했습니다.
- 장소 목록 이미지 사전 로딩을 위한 `preloadImages` 함수를 추가했습니다.
- **`main/component/MainScreen.kt` 수정:**
- `NavHost`에 모든 탭(`Home`, `Schedule`, `PlaceMap`, `News`, `Setting`)에 대한 `NavGraph`를 등록했습니다.
- 각 화면에 필요한 ViewModel과 의존성(`logger`, `locationSource`, `notificationPermissionManager` 등)을 주입하도록 구조를 변경했습니다.
- 화면 간 이동(예: `onNavigateToExplore`)과 이미지 프리로딩(`onPreloadImages`) 콜백을 정의하여 `MainActivity`에서 처리할 수 있도록 했습니다.
`NaverMapContent` 컴포저블에서 `DisposableEffect`가 해제될 때 `MapView`의 생명주기 메서드(`onPause`, `onStop`, `onDestroy`)를 수동으로 호출하여 메모리 누수를 해결하려던 코드를 제거했습니다.
이 로직은 `rememberMapViewWithLifecycle`에서 이미 처리되고 있어 중복되었고, 불필요한 복잡성을 야기했습니다.
- **`NaverMapContent.kt` 수정:**
- `DisposableEffect`의 `onDispose` 블록 내에서 `MapView`의 생명주기 메서드를 직접 호출하는 로직을 삭제했습니다.
- `AndroidView`와 오버레이되는 `content`를 `Box` 레이아웃으로 감싸 구조를 명확히 했습니다.
`NaverMapContent` 컴포저블에서 `AndroidView`가 해제될 때 `MapView`의 생명주기 메서드(`onPause`, `onStop`, `onDestroy`)가 호출되도록 `onRelease` 콜백을 추가했습니다. 이를 통해 지도 리소스가 올바르게 해제되도록 하여 메모리 누수를 방지합니다.
- **`NaverMapContent.kt` 수정:**
- `AndroidView`에 `onRelease` 블록을 추가하여 컴포지션에서 제거될 때 `mapView.onPause()`, `mapView.onStop()`, `mapView.onDestroy()`를 순차적으로 호출하도록 구현했습니다.
하단 네비게이션 바의 동일한 탭을 다시 선택했을 때, 각 화면의 데이터를 새로고침하거나 초기 상태로 되돌리는 기능을 추가했습니다.
- **`FestabookBottomNavigationBar.kt` 수정:**
- `FestabookBottomNavigationBar` 컴포저블에 `onTabReSelect` 콜백 파라미터를 추가했습니다.
- 각 네비게이션 아이템(`FestabookNavigationItem`, `PlaceMapNavigationItem`) 클릭 시, 현재 선택된 탭과 클릭된 탭이 동일할 경우 `onTabReSelect` 콜백이 호출되도록 로직을 수정했습니다.
- **`MainScreen.kt` 수정:**
- `FestabookBottomNavigationBar`의 `onTabReSelect` 콜백을 구현하여 각 탭에 맞는 동작을 정의했습니다.
- **SCHEDULE 탭:** 재선택 시 `scheduleViewModel.loadSchedules()`를 호출하여 스케줄 목록을 새로고침합니다.
- **PLACE_MAP 탭:** 재선택 시 `placeMapViewModel`의 관련 함수를 호출하여 선택된 장소를 해제하고 초기 상태로 되돌립니다.
기존에 `NavHost` 내에서 컴포저블로 관리되던 지도 화면(`PlaceMapRoute`)을 `NavHost` 외부로 분리하는 리팩토링을 진행했습니다. 이를 통해 다른 탭으로 이동했다가 다시 지도 탭으로 돌아왔을 때, 지도의 상태(위치, 줌 레벨 등)와 관련 데이터가 초기화되지 않고 그대로 유지되도록 개선했습니다.
- **`MainScreen.kt` 수정:**
- `PlaceMapRoute`를 `NavHost`의 바깥으로 이동시켜 `NavHost`와 동일한 레벨에 배치했습니다.
- `mutableStateOf`를 사용하여 지도 화면의 가시성(`shouldShowPlaceMap`)을 관리하고, 현재 탭이 지도일 때만 화면이 보이도록 `alpha` 값을 조절했습니다.
- `PlaceMapRoute`에 필요한 의존성(ViewModel, locationSource 등)을 직접 전달하도록 구조를 변경했습니다.
- **`placeMap/navigation/PlaceMapNavigation.kt` 수정:**
- `placeMapNavGraph`의 역할을 축소하여, 더 이상 지도 UI를 직접 생성하지 않고 `PlaceMap` 탭이 선택되었을 때 `MainScreen`에 있는 가시성 상태(`mapScreenVisibilityState`)를 `true`로 변경하는 역할만 담당하도록 수정했습니다.
- **`placeMap/component/PlaceMapScreen.kt` 수정:**
- `PlaceMapRoute` 내부에서 `remember`를 사용하여 `MapDelegate`, `MapManagerDelegate`, `MapControlSideEffectHandler`, `PlaceMapSideEffectHandler` 등 지도 관련 객체들을 생성하고 상태를 유지하도록 변경했습니다.
- 이로 인해 `placeMapNavGraph`에서 전달받던 여러 파라미터가 `PlaceMapRoute` 내에서 자체적으로 관리되도록 구조가 단순화되었습니다.
`Scaffold`가 `MainScreen`으로 이동하면서 각 `NavGraph` 함수(`homeNavGraph`, `newsNavGraph`, `scheduleNavGraph`, `settingNavGraph`)에서 더 이상 사용되지 않는 `padding` 파라미터와 관련 `Modifier.padding()` 코드를 제거하여 코드를 정리했습니다.
- **각 `NavGraph` 파일 수정:**
- `homeNavGraph`, `newsNavGraph`, `scheduleNavGraph`, `settingNavGraph` 함수에서 `padding: PaddingValues` 파라미터를 삭제했습니다.
- 각 함수 내부의 컴포저블에서 `Modifier.padding(padding)`을 적용하는 로직을 제거했습니다.
- **`FestabookNavigator.kt` 수정:**
- `navigateToMainTab` 함수의 파라미터 타입을 `FestabookRoute`에서 더 명확한 `FestabookMainTab`으로 변경했습니다.
기존에 `PlaceMap` 화면 위에서 별도의 레이어로 표시되던 장소 상세 화면을 `NavHost` 내의 독립된 컴포저블(`PlaceDetailRoute`)로 전환했습니다. 이를 통해 화면 전환 로직을 표준화하고, 다른 화면으로의 이동 및 상태 관리를 개선했습니다.
- **`placeMap/navigation/PlaceMapNavigation.kt` 수정:**
- `placeMapNavGraph`에 `FestabookRoute.PlaceDetail` 라우트를 추가하여 장소 상세 화면을 `NavHost` 내에서 관리하도록 변경했습니다.
- 화면 전환 시 애니메이션(`slide`, `fade`, `scale`)을 추가하여 사용자 경험을 개선했습니다.
- 복잡한 객체(UI Model)를 네비게이션 인자로 전달하기 위해 `kotlinx.serialization`을 사용한 커스텀 `NavType`을 구현했습니다.
- **`main/component/MainScreen.kt` 수정:**
- `PlaceMapRoute`에서 장소 상세 화면을 시작하는 로직을 `navigator.navigate` 호출로 변경했습니다.
- `NavHost`를 `FestabookNavHost` 컴포저블로 분리하고, 장소 상세 화면에 필요한 `PlaceDetailViewModel.Factory`를 주입하도록 수정했습니다.
- 지도 화면의 가시성(`alpha`)을 `navigator.currentTab` 상태와 연동하여 관리하도록 변경했습니다.
- **`placeDetail/component/PlaceDetailScreen.kt` 추가:**
- `PlaceDetailRoute` 컴포저블을 새로 정의하여 `ViewModel`로부터 UI 상태를 수집하고 `PlaceDetailScreen`에 전달하도록 구현했습니다.
- **`main/FestabookRoute.kt` 수정:**
- `PlaceDetail` 라우트를 `data class`로 변경하여 `placeUiModel`과 `placeDetailUiModel`을 네비게이션 인자로 받을 수 있도록 수정했습니다.
- **`main/MainActivity.kt` 수정:**
- `MainScreen`에 `PlaceDetailViewModel.Factory`를 주입하는 코드를 추가했습니다.
…로직 수정
기존에 Fragment와 Compose를 혼용하던 `MainActivity`의 구조를 완전한 Compose 기반으로 리팩토링했습니다. 이 과정에서 Fragment 관련 코드를 모두 제거하고, 첫 방문 시 알림 설정 다이얼로그를 Compose 컴포넌트로 재구현했습니다. 또한, 이미지 프리로딩 로직을 `MainActivity`에서 `PlaceMap` 화면 내부로 이동시켜 책임을 명확히 분리했습니다.
- **`MainActivity.kt` 수정:**
- Fragment를 관리하던 코드(`switchFragment`, `setupFragmentFactory` 등)를 전부 삭제했습니다.
- `OnBackPressedCallback`을 Compose의 `BackHandler`로 대체하여 뒤로가기 로직을 Compose 내에서 처리하도록 변경했습니다.
- 첫 방문 사용자에게 보여주던 `MaterialAlertDialog`를 `FirstVisitDialog` 컴포저블로 대체했습니다.
- `LiveData`와 `Event` 래퍼를 `StateFlow`, `SharedFlow`로 마이그레이션하여 상태 관리를 개선했습니다.
- `onPreloadImages` 콜백을 제거하고, 이미지 프리로딩 책임을 `PlaceMap` 컴포저블로 이전했습니다.
- **`FirstVisitDialog.kt` 추가:**
- 첫 방문 시 알림 권한을 요청하는 다이얼로그를 Compose 기반의 `FirstVisitDialog` 컴포저블로 새로 구현했습니다.
- **`PlaceMapScreen.kt` 수정:**
- `MainActivity`에서 담당하던 이미지 프리로딩(`preloadImages`) 로직을 `PlaceMapRoute` 컴포저블 내부로 이동시켰습니다.
- **기타 수정:**
- **`PlaceDetailScreen.kt`:** `BackHandler`를 추가하여 뒤로가기 버튼 클릭 시 이전 화면으로 이동하도록 수정했습니다.
- **`MainViewModel.kt`, `SettingViewModel.kt`:** `LiveData`를 `StateFlow` 및 `SharedFlow`로 전환하여 Compose 환경에 더 적합한 방식으로 상태를 관리하도록 수정했습니다.
기존 `getOrAwaitValue`를 사용하던 `MainViewModelTest`의 `LiveData` 이벤트 테스트 방식을 새로운 `observeEvent` 유틸리티 함수를 사용하도록 수정했습니다. 이 변경으로 비동기 이벤트 처리를 더 명확하고 안정적으로 테스트할 수 있게 되었습니다.
- **`main/MainViewModelTest.kt` 수정:**
- `getOrAwaitValue` 대신 `observeEvent`를 사용하여 `backPressEvent` `LiveData`를 관찰하도록 변경했습니다.
- `runCurrent()`와 `await()`를 사용하여 코루틴의 실행 시점을 제어하고 이벤트 발생을 기다리는 로직으로 수정했습니다.
- **`news/NewsTestFixture.kt`, `news/NewsViewModelTest.kt` 수정:**
- 테스트 데이터 생성 시 사용되던 `java.time.LocalDateTime`을 `kotlinx.datetime.LocalDateTime`으로 통일하여 코드 일관성을 개선했습니다.
📝 WalkthroughWalkthrough프로젝트를 Fragment 기반 아키텍처에서 Compose 기반 아키텍처로 마이그레이션하며, LiveData에서 Flow로 상태 관리를 전환하고, java.time에서 kotlinx.datetime으로 날짜/시간 라이브러리를 변경합니다. 새로운 네비게이션 구조와 첫 방문 다이얼로그를 도입합니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 이 PR은 다양한 이질적인 변경사항을 포함합니다: Fragment에서 Compose로의 대규모 마이그레이션, LiveData에서 Flow로의 상태 관리 전환, 날짜/시간 라이브러리 변경, 네비게이션 구조 개편 및 새로운 컴포넌트 도입이 여러 파일에 걸쳐 분산되어 있습니다. 각 변경 영역이 별도의 추론을 요구하며 높은 로직 밀도를 보입니다. Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
oungsi2000
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/com/daedan/festabook/data/model/response/notice/NoticeResponse.kt (1)
22-29:LocalDateTime.parse()에 오류 처리 추가가 필요합니다.현재 코드는 파싱 실패 시 런타임 예외를 발생시킵니다. 동일한 작업을 수행하는
LostItem.kt의toLocalDateTime()확장함수처럼runCatching과getOrElse를 사용하여 오류 처리를 구현하세요. 유효하지 않은 날짜 형식이 서버에서 반환되더라도 안정적으로 처리할 수 있습니다. 같은 문제가PlaceDetailResponse.kt와LineupResponse.kt에도 있습니다.app/src/main/java/com/daedan/festabook/data/model/response/place/PlaceDetailResponse.kt (1)
84-93:LocalDateTime.parse()에 에러 처리 추가 필요현재 코드는
it.createdAt문자열을LocalDateTime.parse()로 직접 파싱하고 있으나, API 응답 형식이 ISO-8601을 벗어나면 런타임 예외가 발생합니다.같은 파일의
toLocalTime()함수(104-107줄)에서 이미runCatching()패턴을 사용하고 있으므로, 동일하게 적용하세요:createdAt = runCatching { LocalDateTime.parse(it.createdAt) }.getOrNull() ?: LocalDateTime(2025, 1, 1, 0, 0, 0)참고:
NoticeResponse.kt28줄도 동일한 문제를 가지고 있습니다.
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt`:
- Around line 103-119: PlaceMapRoute being hidden with alpha(0f) still receives
touch events; change composition so it doesn't accept touches when not active by
either: 1) conditionally compose it only when navigator.currentTab ==
FestabookMainTab.PLACE_MAP (wrap the PlaceMapRoute call in an if) — note this
may reset map state on tab switch; or 2) keep it composed but add a
non-interactive modifier when inactive, e.g. append a
Modifier.pointerInput(Unit) { /* consume events */ } or
Modifier.clickable(enabled = false) / .pointerInput { awaitPointerEventScope {
while(true) { awaitPointerEvent() } } } to consume/block input when
navigator.currentTab != FestabookMainTab.PLACE_MAP; update the modifier chain on
the PlaceMapRoute call accordingly.
In `@app/src/main/java/com/daedan/festabook/presentation/main/FestabookRoute.kt`:
- Around line 3-16: The FestabookRoute.PlaceDetail currently serializes and
passes full PlaceUiModel and PlaceDetailUiModel (which risks
TransactionTooLargeException); change the sealed class to carry only a small
identifier (e.g., placeId: String or Long or minimal primitive key) instead of
PlaceUiModel/PlaceDetailUiModel, remove those model fields from
FestabookRoute.PlaceDetail, and update navigation callers to pass the id; then
fetch the full PlaceUiModel/PlaceDetailUiModel inside the destination via the
Repository or ViewModel (e.g., use the placeId in your ViewModel to load data)
and update any code referencing FestabookRoute.PlaceDetail to use the id-based
payload.
In `@app/src/main/java/com/daedan/festabook/presentation/main/MainActivity.kt`:
- Around line 55-57: The FusedLocationSource instance (locationSource) needs the
permission result forwarded from MainActivity's onRequestPermissionsResult;
update the MainActivity override of onRequestPermissionsResult to call
locationSource.onRequestPermissionsResult(requestCode, permissions,
grantResults) (and keep or call super.onRequestPermissionsResult as appropriate)
so the FusedLocationSource can handle the location permission result.
In `@app/src/main/java/com/daedan/festabook/presentation/main/MainViewModel.kt`:
- Around line 34-39: The MutableSharedFlow _navigateNewsEvent is created with
replay = 1 causing the last event to be re-emitted to new collectors and can
trigger duplicate navigation on recomposition; change its initialization in
MainViewModel (the _navigateNewsEvent declaration) to use replay = 0 (keep
extraBufferCapacity as-is or adjust if needed) so the navigateNewsEvent
asSharedFlow emits one-time navigation events only to active collectors.
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/component/NaverMapContent.kt`:
- Around line 41-53: The composable currently may call mapView.onDestroy() twice
via AndroidView(onRelease) and the lifecycleObserver in RegisterMapLifeCycle; to
fix, choose one place to perform destruction or add a guard flag: either remove
the ON_DESTROY branch from the lifecycleObserver inside RegisterMapLifeCycle so
only AndroidView's onRelease destroys mapView, or add a remembered boolean
(e.g., isDestroyed) that you check/set in AndroidView's onRelease and in the
lifecycleObserver before calling mapView.onDestroy() to ensure
mapView.onDestroy() is executed only once; reference mapView,
AndroidView(onRelease), RegisterMapLifeCycle, lifecycleObserver and the
ON_DESTROY case when applying the change.
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/model/PlaceUiModel.kt`:
- Around line 6-10: PlaceUiModel is annotated with `@Serializable` but its nested
enum type PlaceCategoryUiModel lacks `@Serializable`, causing runtime
serialization failures; open the PlaceCategoryUiModel enum (symbol:
PlaceCategoryUiModel) and add the `@Serializable` annotation to the enum
declaration (or provide a custom KSerializer for it) so that PlaceUiModel can be
fully serialized by kotlinx.serialization.
In
`@app/src/main/java/com/daedan/festabook/presentation/setting/SettingViewModel.kt`:
- Around line 42-46: The code exposes the same MutableSharedFlow twice via val
success and val successFlow; consolidate to a single public Flow name (e.g.,
keep val success = _success.asSharedFlow()) and update all call sites
(SettingScreen, SettingFragment, MainScreen) to use that single property;
optionally add a `@Deprecated` val successFlow that delegates to success for one
release before removal to avoid breaking callers, then remove successFlow once
call sites are migrated.
In `@app/src/main/res/layout/activity_main.xml`:
- Around line 20-26: ComposeView with id cv_main has android:layout_height="0dp"
but no top constraint, so its height collapses; fix by either adding a top
constraint (e.g., set
app:layout_constraintTop_toBottomOf="@id/fcv_fragment_container" on the
ComposeView) or by changing android:layout_height to "wrap_content" to let it
size correctly; update the ComposeView element (id cv_main) accordingly so both
top and bottom constraints exist when height is 0dp or use wrap_content instead.
🧹 Nitpick comments (15)
app/src/main/java/com/daedan/festabook/presentation/schedule/navigation/ScheduleNavigation.kt (1)
9-15: 프로젝트의 네비게이션 패턴을 잘 따르고 있습니다.코드가 깔끔하고 다른 네비게이션 파일(NewsNavigation.kt, PlaceMapNavigation.kt)과 일관된 패턴을 유지하고 있습니다. 타입 안전 네비게이션(
composable<MainTabRoute.Schedule>)을 사용한 점도 적절합니다.선택적 제안:
ScheduleScreen이modifier파라미터를 지원하므로, 향후 패딩이나 스타일링 유연성을 위해 modifier를 전달받아 넘겨주는 것을 고려해볼 수 있습니다.♻️ 선택적 개선안
-fun NavGraphBuilder.scheduleNavGraph(viewModel: ScheduleViewModel) { +fun NavGraphBuilder.scheduleNavGraph( + viewModel: ScheduleViewModel, + modifier: Modifier = Modifier, +) { composable<MainTabRoute.Schedule> { ScheduleScreen( scheduleViewModel = viewModel, + modifier = modifier, ) } }
Modifierimport 추가 필요:import androidx.compose.ui.Modifierapp/src/main/java/com/daedan/festabook/presentation/main/component/FestabookBottomNavigationBar.kt (2)
69-72: 탭 재선택 시onTabSelect중복 호출 검토 필요.현재 로직은 재선택 시에도
onTabSelect를 항상 호출합니다. 이로 인해 불필요한 네비게이션 또는 상태 업데이트가 발생할 수 있습니다. 의도된 동작이라면 무시해도 되지만, 그렇지 않다면 탭이 변경될 때만onTabSelect를 호출하는 것이 좋습니다.♻️ 제안된 수정
onClick = { if (it == currentTab) onTabReSelect(it) - onTabSelect(it) + else onTabSelect(it) },
78-81: 동일한 재선택 로직 중복.위의
FestabookNavigationItem과 동일한 로직이 중복됩니다. 위 코멘트의 수정을 적용한다면 여기에도 일관되게 적용해 주세요.♻️ 제안된 수정 (위와 일관되게)
PlaceMapNavigationItem(onClick = { if (it == currentTab) onTabReSelect(it) - onTabSelect(it) + else onTabSelect(it) })app/src/main/java/com/daedan/festabook/presentation/placeMap/component/NaverMapContent.kt (1)
42-42: TODO 주석에 대한 후속 조치가 필요합니다.AndroidView와 Compose 뷰 혼용으로 인한 컴파일러 경고 해결을 위한 이슈 생성 또는 타임라인 설정을 권장합니다.
이 TODO를 추적하기 위한 GitHub 이슈를 생성해 드릴까요?
app/src/main/res/layout/activity_main.xml (1)
28-64: 주석 처리된 코드 제거 권장마이그레이션 완료 후 주석 처리된 BottomAppBar, BottomNavigationView, FAB 코드는 제거하는 것이 좋습니다. 버전 관리 시스템에서 이전 코드를 참조할 수 있으므로 코드베이스를 깔끔하게 유지하세요.
app/src/main/java/com/daedan/festabook/presentation/setting/component/SettingScreen.kt (1)
96-102: 불필요한 문자열 보간 제거
"${"SettingRoute"}"는 단순히"SettingRoute"로 작성할 수 있습니다.♻️ 수정 제안
onError = { onShowErrorSnackBar(it.throwable) Timber.w( it.throwable, - "${"SettingRoute"}: ${it.throwable.message}", + "SettingRoute: ${it.throwable.message}", ) },app/src/main/java/com/daedan/festabook/presentation/main/component/FirstVisitDialog.kt (1)
156-165: Preview 함수명 불일치Preview 함수명이
UpdateDialogPreview로 되어 있지만, 실제로는FirstVisitDialog를 미리보기합니다. 일관성을 위해FirstVisitDialogPreview로 변경하는 것이 좋습니다.♻️ 수정 제안
`@Preview` `@Composable` -private fun UpdateDialogPreview() { +private fun FirstVisitDialogPreview() { FestabookTheme { FirstVisitDialog( onConfirm = {}, onDecline = {}, ) } }app/src/main/java/com/daedan/festabook/presentation/placeDetail/component/PlaceDetailScreen.kt (1)
184-188: 하드코딩된 contentDescription 문자열 리소스화
"close the popup"문자열이 하드코딩되어 있습니다. 다국어 지원(i18n)을 위해 문자열 리소스로 추출하는 것이 좋습니다.♻️ 수정 제안
Icon( imageVector = Icons.Default.Close, - contentDescription = "close the popup", + contentDescription = stringResource(R.string.content_description_close_popup), tint = FestabookColor.white, )app/src/main/java/com/daedan/festabook/presentation/placeMap/component/PlaceMapScreen.kt (2)
69-97:remember에서 캡처된 의존성이 업데이트되지 않습니다.
mapControlSideEffectHandler와placeMapSideEffectHandler가remember { ... }블록 내에서context,scope,logger,locationSource등을 캡처하고 있습니다. 이 값들이 recomposition 중에 변경되면 handler는 여전히 이전 값을 참조하게 됩니다.
remember에 적절한 key를 추가하거나,rememberUpdatedState를 사용하여 최신 값을 참조하도록 수정하세요.♻️ 제안된 수정
val mapControlSideEffectHandler = - remember { + remember(density, logger, locationSource, placeMapViewModel, mapDelegate, mapManagerDelegate) { MapControlSideEffectHandler( initialPadding = with(density) { 254.dp.toPx() }.toInt(), logger = logger, locationSource = locationSource, viewModel = placeMapViewModel, mapDelegate = mapDelegate, mapManagerDelegate = mapManagerDelegate, ) } val placeMapSideEffectHandler = - remember { + remember(mapManagerDelegate, bottomSheetState, placeMapViewModel, logger, onStartPlaceDetail, context, scope) { PlaceMapSideEffectHandler(
95-95: 빈 람다 대신 SnackBar 처리 로직 연결 필요.
onShowErrorSnackBar = { }가 빈 람다로 설정되어 있어 에러 발생 시 사용자에게 피드백이 제공되지 않습니다. PR 설명에 SnackBar 통합이 다음 작업으로 언급되어 있으니, TODO 주석을 추가하여 추적 가능하도록 해주세요.app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt (1)
171-177: SnackBar 콜백이 빈 람다로 설정되어 있습니다.
settingNavGraph의onShowSnackBar와onShowErrorSnackBar가 빈 람다입니다. PR 설명에 SnackBar 통합이 예정되어 있으니 TODO 주석 추가를 권장합니다.app/src/test/java/com/daedan/festabook/news/NewsViewModelTest.kt (1)
181-182: 완전한 패키지 경로 대신 import 사용 권장
kotlinx.datetime.LocalDateTime을 완전한 패키지 경로로 사용하고 있습니다. 파일 상단에 import를 추가하면 가독성이 향상됩니다.♻️ 제안된 수정
파일 상단 import 섹션에 추가:
import kotlinx.datetime.LocalDateTime그 후 사용부 수정:
- createdAt = kotlinx.datetime.LocalDateTime(2025, 1, 1, 0, 0, 0), + createdAt = LocalDateTime(2025, 1, 1, 0, 0, 0),app/src/test/java/com/daedan/festabook/news/NewsTestFixture.kt (1)
49-50: 테스트 픽스처 내 날짜/시간 라이브러리 불일치
FAKE_NOTICES는kotlinx.datetime.LocalDateTime을,FAKE_LOST_ITEM은java.time.LocalDateTime을 사용합니다. 이는Lost.Item도메인 모델이java.time.LocalDateTime을 사용하기 때문입니다.프로젝트 전반의 일관성을 위해 향후
Lost모델도 kotlinx-datetime으로 마이그레이션하는 것을 고려해보세요.app/src/main/java/com/daedan/festabook/presentation/placeMap/navigation/PlaceMapNavigation.kt (2)
30-31: 빈 Composable에 대한 의도 명시 권장PR 설명에 따르면 PlaceMap은 NavHost 외부에서 visibility로 제어되기 때문에 빈 composable이 의도적인 것으로 보입니다. 명확성을 위해 주석을 추가하는 것이 좋습니다.
💡 제안된 개선
composable<MainTabRoute.PlaceMap> { + // PlaceMap is rendered outside NavHost to preserve map state + // See PlaceMapContent in MainScreen }
72-91: JSON 디코딩 실패 시 예외 처리 권장
Json.decodeFromString은 잘못된 입력에 대해SerializationException을 던질 수 있습니다.parseValue()에서 예외가 발생하면 네비게이션 중 앱이 크래시될 수 있습니다.💡 제안된 개선
+private val navJson = Json { + ignoreUnknownKeys = true + isLenient = true +} + private inline fun <reified T> defaultNavType() = object : NavType<T>(isNullableAllowed = true) { override fun get( bundle: Bundle, key: String, - ): T? = bundle.getString(key)?.let { Json.decodeFromString(it) } + ): T? = bundle.getString(key)?.let { + runCatching { navJson.decodeFromString<T>(it) }.getOrNull() + } override fun parseValue(value: String): T = - Json.decodeFromString( - Uri.decode(value), - ) + navJson.decodeFromString(Uri.decode(value)) override fun put( bundle: Bundle, key: String, value: T, - ) = bundle.putString(key, Json.encodeToString(value)) + ) = bundle.putString(key, navJson.encodeToString(value)) - override fun serializeAsValue(value: T): String = Uri.encode(Json.encodeToString(value)) + override fun serializeAsValue(value: T): String = Uri.encode(navJson.encodeToString(value)) }
| PlaceMapRoute( | ||
| modifier = | ||
| Modifier | ||
| .alpha( | ||
| if (navigator.currentTab == FestabookMainTab.PLACE_MAP) 1f else 0f, | ||
| ).padding(innerPadding), | ||
| placeMapViewModel = placeMapViewModel, | ||
| locationSource = locationSource, | ||
| logger = logger, | ||
| onStartPlaceDetail = { | ||
| navigator.navigate( | ||
| FestabookRoute.PlaceDetail( | ||
| placeDetailUiModel = it.placeDetail.value, | ||
| ), | ||
| ) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
alpha(0f)는 터치 이벤트를 차단하지 않습니다.
PlaceMapRoute가 alpha(0f)로 숨겨져 있을 때도 여전히 터치 이벤트를 수신할 수 있습니다. 다른 탭이 활성화되어 있을 때 지도와의 의도치 않은 상호작용이 발생할 수 있습니다.
🐛 제안된 수정
PlaceMapRoute(
modifier =
Modifier
.alpha(
if (navigator.currentTab == FestabookMainTab.PLACE_MAP) 1f else 0f,
- ).padding(innerPadding),
+ )
+ .then(
+ if (navigator.currentTab != FestabookMainTab.PLACE_MAP) {
+ Modifier.pointerInput(Unit) { }
+ } else {
+ Modifier
+ }
+ )
+ .padding(innerPadding),또는 더 간단하게:
if (navigator.currentTab == FestabookMainTab.PLACE_MAP) {
PlaceMapRoute(...)
}단, 이 경우 지도 상태가 탭 전환 시 초기화될 수 있습니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt`
around lines 103 - 119, PlaceMapRoute being hidden with alpha(0f) still receives
touch events; change composition so it doesn't accept touches when not active by
either: 1) conditionally compose it only when navigator.currentTab ==
FestabookMainTab.PLACE_MAP (wrap the PlaceMapRoute call in an if) — note this
may reset map state on tab switch; or 2) keep it composed but add a
non-interactive modifier when inactive, e.g. append a
Modifier.pointerInput(Unit) { /* consume events */ } or
Modifier.clickable(enabled = false) / .pointerInput { awaitPointerEventScope {
while(true) { awaitPointerEvent() } } } to consume/block input when
navigator.currentTab != FestabookMainTab.PLACE_MAP; update the modifier chain on
the PlaceMapRoute call accordingly.
| import com.daedan.festabook.presentation.placeDetail.model.PlaceDetailUiModel | ||
| import com.daedan.festabook.presentation.placeMap.model.PlaceUiModel | ||
| import kotlinx.serialization.Serializable | ||
|
|
||
| @Serializable | ||
| sealed interface FestabookRoute { | ||
| @Serializable | ||
| data object Splash : FestabookRoute | ||
|
|
||
| // TODO: PlaceUiModel, PlaceDetailUiModel 생성자에 추가 후 UiModel에 @Serializable 어노테이션 필요 | ||
| @Serializable | ||
| data object PlaceDetail : FestabookRoute | ||
| data class PlaceDetail( | ||
| val placeUiModel: PlaceUiModel? = null, | ||
| val placeDetailUiModel: PlaceDetailUiModel? = null, | ||
| ) : FestabookRoute |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# Find PlaceDetailUiModel and PlaceUiModel files
fd 'PlaceDetailUiModel|PlaceUiModel' -t f --extension ktRepository: festabook/android
Length of output: 178
🏁 Script executed:
# Try searching using web for information about Jetpack Compose navigation serialization constraints
# and common issues with navigation route payloads
curl -s "https://developer.android.com/" 2>/dev/null | head -20 || echo "Web access limited"Repository: festabook/android
Length of output: 808
🌐 Web query:
Jetpack Compose Navigation kotlinx.serialization route payload bundle size limits constraints
💡 Result:
In Navigation Compose, “type-safe routes” that use kotlinx.serialization still end up as navigation arguments associated with a NavBackStackEntry (and commonly accessed via toRoute() / SavedStateHandle). (developer.android.com) Those arguments are ultimately stored/transported using Android’s state/Bundles (e.g., SavedStateHandle / SavedState → Bundle). (developer.android.com)
Practical size limits / constraints (what will bite you)
- Binder transaction limit (~1MB, process-wide shared buffer): Any time these arguments/state are sent through Binder (e.g., saving/restoring state, activity/fragment lifecycle handoff, etc.), you can hit
TransactionTooLargeException. Android’s docs state the Binder transaction buffer is currently 1MB and shared across transactions in the process. (developer.android.com) - Saved-state guidance is far smaller than 1MB: In practice you should keep saved state tiny. A commonly cited guideline is < ~50KB for saved instance state, because the system may need to retain it for a long time for back navigation/process death restore. (stackoverflow.com)
- Type constraint (Bundle-compatible) still applies: When using
SavedStateHandlewith navigation, values must be types that can go in aBundle(or you need another mechanism like aViewModel/repository). (developer.android.com)
What this means for “serialized route payloads”
- Serializing a whole object (especially to a string/JSON) and passing it as a nav arg is still constrained by Bundle/Binder limits, and can blow up when state is saved/restored (not necessarily at the moment you call
navigate()). - Best practice is to pass small identifiers (IDs, keys) in the route, and load the heavy object from a repository/DB/shared ViewModel instead of bundling it.
If you share a concrete example of your route payload (types + approximate size), I can suggest a safer pattern and where it should live (route vs SavedStateHandle vs repository).
Citations:
- 1: https://developer.android.com/guide/navigation/design/type-safety?utm_source=openai
- 2: https://developer.android.com/guide/navigation/use-graph/programmatic?utm_source=openai
- 3: https://developer.android.com/reference/android/os/TransactionTooLargeException?utm_source=openai
- 4: https://stackoverflow.com/questions/8552514/is-there-any-limit-of-bundle-in-android?utm_source=openai
- 5: https://developer.android.com/guide/navigation/use-graph/programmatic?utm_source=openai
🌐 Web query:
Android Compose navigation @serializable route performance best practices data classes
💡 Result:
Best practices (performance + reliability) for @Serializable route data classes in Navigation-Compose (type-safe routes)
-
Keep route arguments small and primitive-ish
- Prefer passing IDs, enums, small strings, etc., then load the real data from a repository/ViewModel.
- Type-safe routes are turned into navigation arguments (and can also participate in deep-link URL parameter generation), so large/nested objects (big lists, JSON blobs) increase allocations, encoding/decoding work, and risk hitting size limits. (medium.com)
-
Parse (
toRoute()) once, not on every recomposition- If you do
backStackEntry.toRoute<MyRoute>()inside a composable body, don’t let it re-run on every recomposition—cache it:
composable<Product> { entry -> val route = remember(entry) { entry.toRoute<Product>() } ProductScreen(route.id) }
This follows general Compose guidance to avoid repeated “work” during recomposition. (developer.android.com)
- If you do
-
Better: decode in the ViewModel via
SavedStateHandle.toRoute()- This makes the decode happen once per VM instance and keeps the composable “dumb”:
class ProductViewModel(savedStateHandle: SavedStateHandle) : ViewModel() { val route: Product = savedStateHandle.toRoute() }
Note:
SavedStateHandle.toRoute()currently depends on AndroidBundle(relevant for tests and also hints it’s not “free”). (medium.com) -
Use the right route shape
@Serializable objectfor no-arg destinations,@Serializable data classfor destinations with args. (developer.android.com)
-
If you need custom/complex argument types, use the supported customization points (don’t stringify yourself)
- Navigation added a
typeMapparameter fortoRoute()to support custom argument types. (developer.android.com)
- Navigation added a
-
Baseline profile / macro-level perf
- If navigation + serialization is on a critical path (cold start / first screen), also lean on Compose perf tooling and Baseline Profiles. (developer.android.com)
Citations:
- 1: https://medium.com/androiddevelopers/type-safe-navigation-for-compose-105325a97657
- 2: https://developer.android.com/develop/ui/compose/performance/bestpractices?utm_source=openai
- 3: https://medium.com/androiddevelopers/type-safe-navigation-for-compose-105325a97657
- 4: https://developer.android.com/guide/navigation/design/type-safety
- 5: https://developer.android.com/jetpack/androidx/releases/navigation?utm_source=openai
- 6: https://developer.android.com/develop/ui/compose/performance?utm_source=openai
라우트에서 전체 모델 객체 전달 방식 개선 필수.
PlaceUiModel과 PlaceDetailUiModel 전체 객체를 라우트 페이로드로 전달하는 것은 안드로이드 네비게이션 모범 사례에 위배됩니다. Navigation Compose의 @Serializable 라우트는 Bundle 인자로 처리되며, Bundle은 Binder 트랜잭션 한계(~1MB 공유 버퍼)와 백 스택 저장 시 크기 제한(권장: <50KB)에 제약을 받습니다. 상태 저장/복원 중 TransactionTooLargeException이 발생할 수 있습니다.
개선 방안: PlaceDetail 라우트에는 ID나 최소 필드만 전달하고, 실제 데이터는 Repository나 ViewModel에서 로드하세요.
🤖 Prompt for AI Agents
In `@app/src/main/java/com/daedan/festabook/presentation/main/FestabookRoute.kt`
around lines 3 - 16, The FestabookRoute.PlaceDetail currently serializes and
passes full PlaceUiModel and PlaceDetailUiModel (which risks
TransactionTooLargeException); change the sealed class to carry only a small
identifier (e.g., placeId: String or Long or minimal primitive key) instead of
PlaceUiModel/PlaceDetailUiModel, remove those model fields from
FestabookRoute.PlaceDetail, and update navigation callers to pass the id; then
fetch the full PlaceUiModel/PlaceDetailUiModel inside the destination via the
Repository or ViewModel (e.g., use the placeId in your ViewModel to load data)
and update any code referencing FestabookRoute.PlaceDetail to use the id-based
payload.
| private val locationSource by lazy { | ||
| FusedLocationSource(this, LOCATION_PERMISSION_REQUEST_CODE) | ||
| } |
There was a problem hiding this comment.
FusedLocationSource에 권한 결과 전달 필요.
FusedLocationSource는 onRequestPermissionsResult에서 권한 결과를 전달받아야 합니다. 현재 onRequestPermissionsResult 메서드에서 locationSource.onRequestPermissionsResult()를 호출하지 않고 있습니다.
🐛 제안된 수정
override fun onRequestPermissionsResult(
requestCode: Int,
permissions: Array<out String>,
grantResults: IntArray,
) {
+ if (locationSource.onRequestPermissionsResult(requestCode, permissions, grantResults)) {
+ if (!locationSource.isActivated) {
+ // 권한 거부됨
+ }
+ super.onRequestPermissionsResult(requestCode, permissions, grantResults)
+ return
+ }
grantResults.forEachIndexed { index, result ->🤖 Prompt for AI Agents
In `@app/src/main/java/com/daedan/festabook/presentation/main/MainActivity.kt`
around lines 55 - 57, The FusedLocationSource instance (locationSource) needs
the permission result forwarded from MainActivity's onRequestPermissionsResult;
update the MainActivity override of onRequestPermissionsResult to call
locationSource.onRequestPermissionsResult(requestCode, permissions,
grantResults) (and keep or call super.onRequestPermissionsResult as appropriate)
so the FusedLocationSource can handle the location permission result.
| private val _navigateNewsEvent: MutableSharedFlow<Unit> = | ||
| MutableSharedFlow( | ||
| replay = 1, | ||
| extraBufferCapacity = 1, | ||
| ) | ||
| val navigateNewsEvent = _navigateNewsEvent.asSharedFlow() |
There was a problem hiding this comment.
navigateNewsEvent의 replay = 1 설정 검토 필요
replay = 1은 새로운 collector가 구독할 때 마지막 이벤트를 다시 받게 됩니다. 이로 인해 화면 재구성 시 의도치 않은 중복 네비게이션이 발생할 수 있습니다. 일회성 이벤트의 경우 replay = 0이 더 적합합니다.
💡 제안된 수정
private val _navigateNewsEvent: MutableSharedFlow<Unit> =
MutableSharedFlow(
- replay = 1,
+ replay = 0,
extraBufferCapacity = 1,
)🤖 Prompt for AI Agents
In `@app/src/main/java/com/daedan/festabook/presentation/main/MainViewModel.kt`
around lines 34 - 39, The MutableSharedFlow _navigateNewsEvent is created with
replay = 1 causing the last event to be re-emitted to new collectors and can
trigger duplicate navigation on recomposition; change its initialization in
MainViewModel (the _navigateNewsEvent declaration) to use replay = 0 (keep
extraBufferCapacity as-is or adjust if needed) so the navigateNewsEvent
asSharedFlow emits one-time navigation events only to active collectors.
| Box(modifier = modifier) { | ||
| // TODO AndroidView와 CMP 뷰의 혼용으로 컴파일러 경고 발생중 -> 추후 해결하겠습니다 | ||
| AndroidView( | ||
| factory = { mapView }, | ||
| modifier = Modifier.dragInterceptor(onMapDrag), | ||
| onRelease = { | ||
| mapView.onDestroy() | ||
| }, | ||
| ) | ||
| content(mapDelegate.value) | ||
| } | ||
| RegisterMapLifeCycle(mapView) | ||
| content(mapDelegate.value) | ||
| } |
There was a problem hiding this comment.
onDestroy()가 중복 호출될 가능성이 있습니다.
onRelease 콜백(line 47)과 lifecycleObserver의 ON_DESTROY 이벤트 핸들러(line 140) 모두 mapView.onDestroy()를 호출합니다. Lifecycle이 ON_DESTROY 상태에 도달한 후 composable이 composition에서 제거되면 onDestroy()가 두 번 호출되어 예기치 않은 동작이나 크래시가 발생할 수 있습니다.
onRelease에서 destroy를 처리하려면 lifecycleObserver에서 ON_DESTROY 케이스를 제거하거나, 중복 호출을 방지하는 플래그를 사용하세요.
🐛 중복 호출 방지를 위한 수정 제안
옵션 1: lifecycleObserver에서 ON_DESTROY 제거
private fun MapView.lifecycleObserver(
savedInstanceState: Bundle?,
previousState: MutableState<Lifecycle.Event>,
): LifecycleEventObserver =
LifecycleEventObserver { _, event ->
when (event) {
Lifecycle.Event.ON_CREATE -> this.onCreate(savedInstanceState)
Lifecycle.Event.ON_START -> this.onStart()
Lifecycle.Event.ON_RESUME -> this.onResume()
Lifecycle.Event.ON_PAUSE -> this.onPause()
Lifecycle.Event.ON_STOP -> this.onStop()
- Lifecycle.Event.ON_DESTROY -> this.onDestroy()
- else -> throw IllegalStateException()
+ Lifecycle.Event.ON_DESTROY -> Unit // onRelease에서 처리
+ else -> Unit
}
previousState.value = event
}옵션 2: 플래그를 사용한 중복 호출 방지
val isDestroyed = remember { mutableStateOf(false) }
AndroidView(
factory = { mapView },
modifier = Modifier.dragInterceptor(onMapDrag),
onRelease = {
if (!isDestroyed.value) {
mapView.onDestroy()
isDestroyed.value = true
}
},
)🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/component/NaverMapContent.kt`
around lines 41 - 53, The composable currently may call mapView.onDestroy()
twice via AndroidView(onRelease) and the lifecycleObserver in
RegisterMapLifeCycle; to fix, choose one place to perform destruction or add a
guard flag: either remove the ON_DESTROY branch from the lifecycleObserver
inside RegisterMapLifeCycle so only AndroidView's onRelease destroys mapView, or
add a remembered boolean (e.g., isDestroyed) that you check/set in AndroidView's
onRelease and in the lifecycleObserver before calling mapView.onDestroy() to
ensure mapView.onDestroy() is executed only once; reference mapView,
AndroidView(onRelease), RegisterMapLifeCycle, lifecycleObserver and the
ON_DESTROY case when applying the change.
| import kotlinx.serialization.Serializable | ||
|
|
||
| @Parcelize | ||
| @Serializable | ||
| data class PlaceUiModel( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n app/src/main/java/com/daedan/festabook/presentation/placeMap/model/PlaceUiModel.ktRepository: festabook/android
Length of output: 1132
🏁 Script executed:
fd 'PlaceCategoryUiModel.kt' -t fRepository: festabook/android
Length of output: 151
🏁 Script executed:
rg 'class PlaceCategoryUiModel|data class PlaceCategoryUiModel' -A 5Repository: festabook/android
Length of output: 688
🏁 Script executed:
cat -n app/src/main/java/com/daedan/festabook/presentation/placeMap/model/PlaceCategoryUiModel.ktRepository: festabook/android
Length of output: 7557
🏁 Script executed:
rg 'PlaceCategoryUiModel.*Serializer|KSerializer.*PlaceCategoryUiModel' -t ktRepository: festabook/android
Length of output: 87
🏁 Script executed:
rg '@Serializer.*PlaceCategoryUiModel|object.*:.*Serializer.*PlaceCategoryUiModel' -t ktRepository: festabook/android
Length of output: 87
🏁 Script executed:
rg 'PlaceCategoryUiModel.*Serializer|KSerializer.*PlaceCategoryUiModel'Repository: festabook/android
Length of output: 43
🏁 Script executed:
rg 'object.*Serializer' app/src/main/java/com/daedan/festabook/presentation/placeMap/model/Repository: festabook/android
Length of output: 43
PlaceCategoryUiModel에 @Serializable 어노테이션 추가 필요.
PlaceUiModel이 @Serializable로 표시되어 있지만, 중첩된 타입인 PlaceCategoryUiModel (enum)에는 @Serializable 어노테이션이 없습니다. 커스텀 Serializer도 구현되어 있지 않습니다. 이 상태에서 PlaceUiModel을 직렬화하려고 하면 런타임에 직렬화 실패가 발생합니다.
PlaceCategoryUiModel.kt의 enum 클래스에 @Serializable 어노테이션을 추가하세요:
import kotlinx.serialization.Serializable
`@Serializable`
enum class PlaceCategoryUiModel {
// ...
}🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/model/PlaceUiModel.kt`
around lines 6 - 10, PlaceUiModel is annotated with `@Serializable` but its nested
enum type PlaceCategoryUiModel lacks `@Serializable`, causing runtime
serialization failures; open the PlaceCategoryUiModel enum (symbol:
PlaceCategoryUiModel) and add the `@Serializable` annotation to the enum
declaration (or provide a custom KSerializer for it) so that PlaceUiModel can be
fully serialized by kotlinx.serialization.
| private val _success: MutableSharedFlow<Unit> = | ||
| MutableSharedFlow() | ||
| val success: LiveData<Unit> = _success.asLiveData() | ||
|
|
||
| val success = _success.asSharedFlow() | ||
| val successFlow = _success.asSharedFlow() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the SettingViewModel.kt file
fd "SettingViewModel.kt" --type fRepository: festabook/android
Length of output: 140
🏁 Script executed:
# Search for uses of successFlow and success in the codebase
rg -n "successFlow" --type=ktRepository: festabook/android
Length of output: 87
🏁 Script executed:
# Also search for uses of the success property to understand usage patterns
rg -n "\.success" --type=kt -B2 -A2Repository: festabook/android
Length of output: 87
🏁 Script executed:
# Read the SettingViewModel.kt file to see the actual code
cat -n app/src/main/java/com/daedan/festabook/presentation/setting/SettingViewModel.ktRepository: festabook/android
Length of output: 4643
🏁 Script executed:
# Search for successFlow usage without specifying file type
rg -n "successFlow" app/Repository: festabook/android
Length of output: 491
🏁 Script executed:
# Search for success property usage patterns
rg -n "\.success" app/ -B1 -A1Repository: festabook/android
Length of output: 14681
success 및 successFlow는 동일 Flow를 중복 노출하고 있습니다.
현재 두 속성 모두 실제로 사용 중입니다:
successFlow: SettingScreen, SettingFragment에서 사용success: MainScreen에서 사용
하나의 이름으로 통일하고 모든 사용처를 함께 업데이트하는 것을 권장합니다. 예를 들어 success로 통일한 후, successFlow를 @Deprecated 처리하거나 제거하면서 SettingScreen과 SettingFragment를 동시에 수정해야 합니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/setting/SettingViewModel.kt`
around lines 42 - 46, The code exposes the same MutableSharedFlow twice via val
success and val successFlow; consolidate to a single public Flow name (e.g.,
keep val success = _success.asSharedFlow()) and update all call sites
(SettingScreen, SettingFragment, MainScreen) to use that single property;
optionally add a `@Deprecated` val successFlow that delegates to success for one
release before removal to avoid breaking callers, then remove successFlow once
call sites are migrated.
| <androidx.compose.ui.platform.ComposeView | ||
| android:id="@+id/cv_main" | ||
| app:layout_constraintBottom_toBottomOf="parent" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="0dp" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the full activity_main.xml to understand the layout structure
fd -n "activity_main.xml" --type fRepository: festabook/android
Length of output: 178
🏁 Script executed:
# Search for cv_main usage in the codebase
rg -n "cv_main" --type=kotlin -A 5Repository: festabook/android
Length of output: 2174
🏁 Script executed:
# Also search in Java files
rg -n "cv_main" --type=java -A 5Repository: festabook/android
Length of output: 43
🏁 Script executed:
# Find and examine the activity_main.xml file
fd -n "activity_main.xml" -x cat {}Repository: festabook/android
Length of output: 291
🏁 Script executed:
# Get more context around the ComposeView and related layout elements
cat app/src/main/res/layout/activity_main.xmlRepository: festabook/android
Length of output: 3666
ComposeView 제약조건 수정 필요
cv_main의 android:layout_height="0dp"는 상단(top) 제약조건이 없어 실제로 높이가 0이 됩니다. ConstraintLayout에서 높이를 0으로 설정할 때는 상단과 하단 제약조건이 모두 필요합니다. app:layout_constraintTop_toBottomOf="@id/fcv_fragment_container"를 추가하거나, android:layout_height="wrap_content"로 변경하여 적절한 높이를 설정해야 합니다.
🤖 Prompt for AI Agents
In `@app/src/main/res/layout/activity_main.xml` around lines 20 - 26, ComposeView
with id cv_main has android:layout_height="0dp" but no top constraint, so its
height collapses; fix by either adding a top constraint (e.g., set
app:layout_constraintTop_toBottomOf="@id/fcv_fragment_container" on the
ComposeView) or by changing android:layout_height to "wrap_content" to let it
size correctly; update the ComposeView element (id cv_main) accordingly so both
top and bottom constraints exist when height is 0dp or use wrap_content instead.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/main/java/com/daedan/festabook/presentation/main/MainActivity.kt (1)
105-127:FusedLocationSource에 권한 결과 전달 누락Naver Maps의
FusedLocationSource는 Activity의onRequestPermissionsResult()에서 권한 결과를locationSource.onRequestPermissionsResult()로 반드시 전달해야 위치 추적이 올바르게 작동합니다. 현재 코드는 수동으로 권한을 처리하고만 있어FusedLocationSource가 내부 상태를 갱신할 수 없습니다.🐛 수정 제안
override fun onRequestPermissionsResult( requestCode: Int, permissions: Array<out String>, grantResults: IntArray, ) { + if (locationSource.onRequestPermissionsResult(requestCode, permissions, grantResults)) { + return + } grantResults.forEachIndexed { index, result -> val text = permissions[index] when (text) { Manifest.permission.ACCESS_FINE_LOCATION, Manifest.permission.ACCESS_COARSE_LOCATION, -> { if (!result.isGranted()) { showNotificationDeniedSnackbar( window.decorView.rootView, this, getString(R.string.map_request_location_permission_message), ) } } } } super.onRequestPermissionsResult(requestCode, permissions, grantResults) }app/src/main/java/com/daedan/festabook/presentation/main/FestabookNavigator.kt (1)
16-24:visibleEntries대신currentBackStackEntryAsState()사용 필요
visibleEntries는 화면 전환 중 여러 개의 visible entry를 동시에 포함할 수 있는 StateFlow입니다. 이를.lastOrNull()로 처리하면 애니메이션 도중currentDestination이 불안정하게 변하여 bottom bar 깜빡임이나currentTab상태 불일치가 발생할 수 있습니다.Jetpack Navigation 공식 권장사항에 따르면, "현재 어느 화면에 있는가?"를 판단하는 UI(bottom bar, tab 상태 등)는
currentBackStackEntryAsState()를 사용해야 합니다.visibleEntries는 다이얼로그/오버레이 등 현재 화면에 보이는 모든 항목을 추적할 때만 사용합니다.private val currentDestination `@Composable` get() = navController.currentBackStackEntryAsState().value?.destinationapp/src/test/java/com/daedan/festabook/main/MainViewModelTest.kt (1)
97-110: 테스트 로직 오류: observer가 첫 번째 emission(false)을 캡처하여 테스트가 실패합니다.
observeEvent는 내부적으로flow.first()를 사용하여 첫 번째 emission을 캡처합니다. 현재 테스트 순서:
observeEvent()- observer가 첫 번째 emission 대기- 첫 번째
onBackPressed()-falseemit → observer가 이 값을 캡처- 두 번째
onBackPressed()-trueemit (무시됨)event.await()-false반환따라서
assertThat(actual).isTrue()단언이 실패합니다.🐛 권장 수정안: drop(1)을 사용하여 첫 번째 emission 건너뛰기
`@Test` fun `뒤로 가기를 두 번 빠르게 두 번 클릭했을 때, 종료 이벤트가 발생한다`() = runTest { // given - when - val event = observeEvent(mainViewModel.backPressEvent) + val event = observeEvent(mainViewModel.backPressEvent.drop(1)) mainViewModel.onBackPressed() runCurrent() mainViewModel.onBackPressed() // then val actual = event.await() advanceUntilIdle() assertThat(actual).isTrue() }또는
take(2).toList()를 사용하여 두 emission을 모두 수집하고 두 번째 값을 검증하는 방식도 고려해 보세요.
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/daedan/festabook/data/model/response/place/PlaceDetailResponse.kt`:
- Line 8: The code calls LocalDateTime.parse(it.createdAt) without handling
parse failures in PlaceDetailResponse mapping; wrap the parse in a safe
converter (like LostItemResponse.toLocalDateTime()) or use runCatching around
LocalDateTime.parse to log the exception (Timber.e) and return a safe fallback
(e.g., LocalDateTime.MIN) so malformed createdAt values don't crash; update the
place mapping code that invokes LocalDateTime.parse to use this safe helper or
inline try-catch/runCatching and ensure the log includes the exception and
context.
In
`@app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt`:
- Around line 103-119: The PlaceMapRoute is only visually hidden via
Modifier.alpha(0f) but still receives touch events; update the modifier on
PlaceMapRoute (where Modifier.alpha is applied) to also block pointer input when
navigator.currentTab != FestabookMainTab.PLACE_MAP (e.g., append a conditional
Modifier.pointerInput that consumes all touches) so the hidden map no longer
intercepts taps while preserving its state; keep the alpha for visual hiding and
only apply the pointerInput when the current tab is not PLACE_MAP.
- Around line 157-159: The call in MainScreen's onSubscriptionConfirm passes
navigator.navController.context into
notificationPermissionManager.requestNotificationPermission, but
navController.context can be a non-Activity Context and will break
AlertDialog.Builder in NotificationPermissionManager.showRationaleDialog; change
the call to pass an Activity context (as shown in SettingScreen/SettingFragment
examples) — obtain the hosting Activity (e.g., via LocalContext.current cast to
Activity or navigator-provided Activity/requireActivity()) and pass that
Activity into notificationPermissionManager.requestNotificationPermission
instead of navigator.navController.context.
In `@app/src/main/java/com/daedan/festabook/presentation/main/MainViewModel.kt`:
- Around line 81-83: declineAlert() currently only mutates the in-memory
_isFirstVisit and does not persist the choice, so the dialog reappears after app
restart; add a persistence method to the repository (e.g., add
setIsFirstVisit(Boolean) to FestivalRepository) and implement it in the
repository impl, then update MainViewModel.declineAlert() to call
festivalRepository.setIsFirstVisit(false) before setting _isFirstVisit.value =
false so the user's choice is saved.
In
`@app/src/main/java/com/daedan/festabook/presentation/placeDetail/component/PlaceDetailScreen.kt`:
- Around line 69-84: The Route-level BackHandler in PlaceDetailRoute always
intercepts back presses and causes navigation even when a dialog is open; move
or gate the BackHandler so it respects dialog state: either remove the
BackHandler from PlaceDetailRoute and add it inside PlaceDetailScreen (using
BackHandler(enabled = !uiState.isDialogOpen, onBack = onBackToPreviousClick)),
or keep it in PlaceDetailRoute but wrap its enabled flag with the dialog state
(derive isDialogOpen from viewModel.placeDetail and use BackHandler(enabled =
!isDialogOpen, ...)). Update references: PlaceDetailRoute, PlaceDetailScreen,
BackHandler, and the placeDetail UI state (e.g., uiState.isDialogOpen or
equivalent) so back presses dismiss dialogs first and only navigate when no
dialog is open.
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/component/NaverMapContent.kt`:
- Around line 41-52: There is a risk of calling mapView.onDestroy() twice
because AndroidView's onRelease and the lifecycle observer in
RegisterMapLifeCycle both invoke onDestroy; choose one strategy and remove the
other: either remove the AndroidView onRelease block (the onRelease lambda that
calls mapView.onDestroy()) and let RegisterMapLifeCycle's lifecycleObserver
handle ON_DESTROY, or keep onRelease and modify RegisterMapLifeCycle to stop
calling this.onDestroy() on Lifecycle.Event.ON_DESTROY (or guard it with an
isDestroyed flag) so mapView.onDestroy() is only invoked once.
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/component/PlaceMapScreen.kt`:
- Around line 69-97: The two handlers created with remember
(MapControlSideEffectHandler and PlaceMapSideEffectHandler) lack key parameters
so they won't re-create when their dependencies change; update the remember
calls to include relevant dependency keys (e.g., placeMapViewModel, logger,
locationSource, mapDelegate, mapManagerDelegate for MapControlSideEffectHandler,
and mapManagerDelegate, bottomSheetState, placeMapViewModel, logger,
onStartPlaceDetail, onPreloadImages for PlaceMapSideEffectHandler) so the
handlers are recomposed when any of those referenced values change.
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/navigation/PlaceMapNavigation.kt`:
- Around line 72-91: The NavType created by defaultNavType() marks
isNullableAllowed = true but parseValue (and get) don't handle nullable
payloads: update parseValue to first Uri.decode(value) and if the decoded string
is empty or equals "null" return null as T, otherwise Json.decodeFromString;
likewise change get to treat bundle.getString(key) that is null, empty, or
"null" as null (returning null as T) before calling Json.decodeFromString;
reference the defaultNavType() anonymous NavType<T> and its parseValue and get
methods when applying the fix.
🧹 Nitpick comments (12)
app/src/main/java/com/daedan/festabook/presentation/main/component/FestabookBottomNavigationBar.kt (2)
78-81: 중복 로직 추출 고려.
FestabookNavigationItem(line 69-72)과 동일한 재선택 처리 로직이 반복됩니다. 로컬 함수로 추출하면 유지보수성이 향상됩니다.♻️ 리팩토링 제안
fun FestabookBottomNavigationBar( currentTab: FestabookMainTab?, onTabSelect: (FestabookMainTab) -> Unit, modifier: Modifier = Modifier, onTabReSelect: (FestabookMainTab) -> Unit = {}, ) { + val handleTabClick: (FestabookMainTab) -> Unit = { tab -> + if (tab == currentTab) onTabReSelect(tab) + onTabSelect(tab) + } + Box( // ... 생략 FestabookNavigationItem( tab = item, selected = item == currentTab, - onClick = { - if (it == currentTab) onTabReSelect(it) - onTabSelect(it) - }, + onClick = handleTabClick, ) // ... 생략 - PlaceMapNavigationItem(onClick = { - if (it == currentTab) onTabReSelect(it) - onTabSelect(it) - }) + PlaceMapNavigationItem(onClick = handleTabClick) } }
158-158: Preview에서 Scaffold padding 미적용.
Scaffold의contentPadding이 무시되고 있습니다. Preview용이라 큰 문제는 아니지만, 실제 사용 패턴을 보여주려면 패딩 적용이 권장됩니다.♻️ 수정 제안
- ) { it } + ) { innerPadding -> + Box(modifier = Modifier.padding(innerPadding)) + }app/src/main/java/com/daedan/festabook/presentation/placeMap/component/NaverMapContent.kt (1)
42-42: TODO 코멘트 추적 확인TODO 코멘트가 컴파일러 경고에 대해 남겨져 있습니다. 이 이슈를 추적하기 위한 GitHub 이슈를 생성하시겠습니까?
app/src/main/java/com/daedan/festabook/presentation/main/MainActivity.kt (1)
83-101: Activity와 MainScreen 간 ViewModel 참조 방식 검토 권장Activity에서
mainViewModel을by viewModels()로 생성하고registerDeviceAndFcmToken()과handleNavigation()에서 사용하고 있습니다. 그러나MainScreen에는mainViewModel을 전달하지 않아 내부에서 별도로viewModel()을 통해 참조합니다.동일한
ViewModelStore를 사용하므로 실제로는 같은 인스턴스이지만, 명시적으로 전달하는 것이 의도를 더 명확히 할 수 있습니다.♻️ 선택적 개선안
FestabookTheme { MainScreen( notificationPermissionManager = notificationPermissionManager, logger = appGraph.defaultFirebaseLogger, locationSource = locationSource, placeDetailViewModelFactory = viewModelFactory, onAppFinish = { finish() }, onNavigateToExplore = { startActivity(ExploreActivity.newIntent(this)) }, + mainViewModel = mainViewModel, ) }app/src/main/java/com/daedan/festabook/presentation/setting/SettingViewModel.kt (1)
42-46: success / successFlow 중복 노출은 정리 권장. 동일한 스트림을 두 이름으로 공개하면 호출부 혼동이 생겨요. 하나만 남기고 타입을 명시하면 명확합니다.♻️ 정리 예시
- val success = _success.asSharedFlow() - val successFlow = _success.asSharedFlow() + val success: SharedFlow<Unit> = _success.asSharedFlow()app/src/main/java/com/daedan/festabook/presentation/placeMap/navigation/PlaceMapNavigation.kt (1)
30-31: 빈 composable이 의도된 것인지 확인 필요
MainTabRoute.PlaceMap에 대한 빈 composable이 등록되어 있습니다. PR 설명에 따르면 PlaceMap은 NavHost 외부에서 렌더링되므로 의도적인 것으로 보이나, 빈 composable보다는 주석을 추가하여 의도를 명확히 하는 것이 좋겠습니다.💡 제안: 의도를 명확히 하는 주석 추가
- composable<MainTabRoute.PlaceMap> { - } + // PlaceMap 화면은 NavHost 외부에서 렌더링되므로 빈 composable로 등록 + // (상태 유지를 위해 NavHost와 분리) + composable<MainTabRoute.PlaceMap> { }app/src/main/java/com/daedan/festabook/presentation/placeMap/component/PlaceMapScreen.kt (1)
95-95:onShowErrorSnackBar가 빈 람다로 설정됨에러 스낵바 콜백이 빈 람다로 하드코딩되어 있어 에러가 사용자에게 표시되지 않습니다. PR 설명에 "다음 작업으로 SnackBar 연동 예정"이라고 되어 있으므로, TODO 주석을 추가하거나 파라미터로 전달받는 것을 권장합니다.
💡 제안
- onShowErrorSnackBar = { }, + // TODO: SnackBar 연동 시 구현 필요 + onShowErrorSnackBar = { },SnackBar 연동을 위한 콜백 파라미터 추가 구현을 도와드릴까요?
app/src/main/java/com/daedan/festabook/presentation/main/component/FirstVisitDialog.kt (2)
68-74:onDismissRequest가 호출되지 않는 코드
dismissOnBackPress = false와dismissOnClickOutside = false로 설정되어 있어onDismissRequest가 실제로 호출되지 않습니다. 이 경우onDismissRequest람다는 dead code가 됩니다.의도에 따라 다음 중 하나를 선택해 주세요:
- 백버튼으로 닫기를 허용하려면
dismissOnBackPress = true로 변경- 닫기를 완전히 차단하려면
onDismissRequest = {}로 변경하여 의도를 명확히💡 옵션 2: 닫기 완전 차단 시
Dialog( - onDismissRequest = { onDecline() }, + onDismissRequest = { /* 의도적으로 무시: 버튼을 통해서만 닫을 수 있음 */ }, properties = DialogProperties( dismissOnBackPress = false, dismissOnClickOutside = false, ), ) {
116-151: 버튼 접근성 개선 필요버튼에
contentDescription이 없어 스크린 리더 사용자가 버튼 기능을 파악하기 어려울 수 있습니다.Button컴포저블에 시맨틱 정보를 추가하는 것을 권장합니다.♿ 접근성 개선 제안
+import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics ... Button( onClick = onDecline, modifier = Modifier - .wrapContentWidth(), + .wrapContentWidth() + .semantics { contentDescription = declineButtonText }, ... Button( onClick = onConfirm, modifier = Modifier - .wrapContentWidth(), + .wrapContentWidth() + .semantics { contentDescription = confirmButtonText },app/src/main/res/layout/activity_main.xml (1)
28-64: 주석 처리된 코드 제거 권장BottomAppBar, BottomNavigationView, FAB 관련 주석 처리된 코드가 남아있습니다. Compose로 완전히 전환되었으므로, 버전 관리 시스템에서 이력을 확인할 수 있기 때문에 주석 코드는 제거하는 것이 좋습니다.
app/src/main/java/com/daedan/festabook/presentation/main/MainViewModel.kt (1)
34-39:replay = 1설정이 의도치 않은 이벤트 재생을 유발할 수 있습니다.
navigateNewsEvent에replay = 1이 설정되어 있어, 새로운 collector가 구독을 시작하면 마지막으로 emit된 이벤트가 즉시 재생됩니다. 이는 화면 회전이나 configuration change 시 News 탭으로 의도치 않게 이동하는 현상을 유발할 수 있습니다.일회성 이벤트에는
replay = 0을 사용하는 것이 권장됩니다.♻️ 권장 수정안
private val _navigateNewsEvent: MutableSharedFlow<Unit> = MutableSharedFlow( - replay = 1, + replay = 0, extraBufferCapacity = 1, )app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt (1)
67-70: 비어있는 이벤트 observer가 있습니다.
settingViewModel.success이벤트를 observe하지만 핸들러가 비어있습니다. TODO 주석이 있으므로 추적하고 있다면 문제없지만, 임시로 로깅을 추가하거나 구현 완료까지 observer 자체를 제거하는 것을 고려해 보세요.
| import com.daedan.festabook.domain.model.Place | ||
| import com.daedan.festabook.domain.model.PlaceDetail | ||
| import com.daedan.festabook.domain.model.PlaceDetailImage | ||
| import kotlinx.datetime.LocalDateTime |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find PlaceDetailResponse.kt file
fd PlaceDetailResponse.ktRepository: festabook/android
Length of output: 178
🏁 Script executed:
# Search for createdAt references in the codebase
rg "createdAt" -t kt -t json -B 2 -A 2Repository: festabook/android
Length of output: 87
🏁 Script executed:
# Look for test fixtures or mock data files
fd -e json | head -20Repository: festabook/android
Length of output: 133
🏁 Script executed:
# Search for LocalDateTime parsing and serialization configuration
rg "LocalDateTime" -t kt -B 3 -A 3Repository: festabook/android
Length of output: 87
🏁 Script executed:
# Find PlaceDetailResponse.kt with full path
find . -name "PlaceDetailResponse.kt" -type fRepository: festabook/android
Length of output: 150
🏁 Script executed:
# Search for createdAt in files using correct rg syntax
rg "createdAt" -g "*.kt" -g "*.json" -B 2 -A 2Repository: festabook/android
Length of output: 15496
🏁 Script executed:
# Look for test fixtures or sample responses
find . -name "*fixture*" -o -name "*mock*" -o -name "*sample*" | grep -E "\.(json|kt)$"Repository: festabook/android
Length of output: 43
🏁 Script executed:
# Search for LocalDateTime usage and serialization configuration
rg "LocalDateTime" -g "*.kt" -B 3 -A 3Repository: festabook/android
Length of output: 27862
🏁 Script executed:
# View PlaceDetailResponse.kt file
cat -n "./app/src/main/java/com/daedan/festabook/data/model/response/place/PlaceDetailResponse.kt"Repository: festabook/android
Length of output: 3832
🏁 Script executed:
# Search for PlaceDetailResponse fixtures or test data
rg -n "PlaceDetailResponse\|PlaceDetail" -g "*.kt" --max-count=20Repository: festabook/android
Length of output: 43
🏁 Script executed:
# Search for JSON files containing place or createdAt examples
find . -name "*.json" -type f -exec grep -l "createdAt" {} \;Repository: festabook/android
Length of output: 43
🏁 Script executed:
# Search for custom deserializers for LocalDateTime
rg -n "KSerializer\|JsonDeserializer\|Deserializer" -g "*.kt" | grep -i "localdatetime\|serializ"Repository: festabook/android
Length of output: 43
🏁 Script executed:
# Check for serialization module configuration
fd -e gradle -e kt | xargs grep -l "serialization\|json" 2>/dev/null | head -10Repository: festabook/android
Length of output: 938
🏁 Script executed:
# Search for any JSON configuration or serialization setup
rg -n "@Serializable|Json|Serialization" -g "*.kt" | grep -B 2 -A 2 "LocalDateTime" | head -20Repository: festabook/android
Length of output: 43
🏁 Script executed:
# Check for Http client configuration and JSON serialization setup
rg -n "provideHttpClient\|@Provides\|@Singleton" -g "*.kt" -A 10 | grep -E "@|HttpClient|json|Json" | head -30Repository: festabook/android
Length of output: 43
🏁 Script executed:
# Look for serialization module or JSON configuration
find . -name "*.kt" -path "*/di/*" -o -name "*Module.kt" -o -name "*Configuration.kt" | xargs grep -l "serialization\|json" 2>/dev/nullRepository: festabook/android
Length of output: 123
🏁 Script executed:
# Check build.gradle for serialization settings
find . -name "build.gradle" -o -name "build.gradle.kts" | xargs cat 2>/dev/null | grep -A 5 -B 5 "serialization"Repository: festabook/android
Length of output: 934
🏁 Script executed:
# View NetworkBindings.kt to check HTTP client and serialization configuration
cat -n "./app/src/main/java/com/daedan/festabook/di/NetworkBindings.kt"Repository: festabook/android
Length of output: 3759
🏁 Script executed:
# Search for any custom serializers or format configuration
rg -n "Format\|format\|ISO\|format.*LocalDateTime" -g "*.kt" | head -20Repository: festabook/android
Length of output: 43
LocalDateTime 파싱 시 예외 처리 누락
Line 91에서 LocalDateTime.parse(it.createdAt)를 직접 호출하고 있으나 예외 처리가 없습니다. 서버에서 ISO-8601 형식이 아닌 createdAt을 반환하면 런타임 예외가 발생합니다. LostItemResponse의 toLocalDateTime() 확장 함수처럼 try-catch로 보호하거나, 서버 응답 형식이 ISO-8601임을 명시적으로 문서화하시기 바랍니다.
참고: LostItemResponse의 에러 처리 패턴
fun String.toLocalDateTime(): LocalDateTime =
runCatching {
LocalDateTime.parse(this)
}.onFailure {
Timber.e(it, "LostItem: 날짜 파싱 실패:${it.message}")
}.getOrElse {
LocalDateTime.MIN
}🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/data/model/response/place/PlaceDetailResponse.kt`
at line 8, The code calls LocalDateTime.parse(it.createdAt) without handling
parse failures in PlaceDetailResponse mapping; wrap the parse in a safe
converter (like LostItemResponse.toLocalDateTime()) or use runCatching around
LocalDateTime.parse to log the exception (Timber.e) and return a safe fallback
(e.g., LocalDateTime.MIN) so malformed createdAt values don't crash; update the
place mapping code that invokes LocalDateTime.parse to use this safe helper or
inline try-catch/runCatching and ensure the log includes the exception and
context.
| PlaceMapRoute( | ||
| modifier = | ||
| Modifier | ||
| .alpha( | ||
| if (navigator.currentTab == FestabookMainTab.PLACE_MAP) 1f else 0f, | ||
| ).padding(innerPadding), | ||
| placeMapViewModel = placeMapViewModel, | ||
| locationSource = locationSource, | ||
| logger = logger, | ||
| onStartPlaceDetail = { | ||
| navigator.navigate( | ||
| FestabookRoute.PlaceDetail( | ||
| placeDetailUiModel = it.placeDetail.value, | ||
| ), | ||
| ) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
alpha(0f)는 터치 이벤트를 차단하지 않아 숨겨진 PlaceMapRoute가 터치를 가로챕니다.
Modifier.alpha(0f)는 시각적으로만 투명하게 만들고, 컴포저블은 여전히 터치 이벤트를 받습니다. 다른 탭에 있을 때 사용자가 화면을 터치하면 숨겨진 PlaceMapRoute가 이벤트를 가로채 의도치 않은 동작이 발생할 수 있습니다.
🐛 권장 수정안: 조건부 렌더링 또는 pointerInput 차단 사용
옵션 1: 조건부 렌더링 (상태 유지가 필요 없는 경우)
-PlaceMapRoute(
- modifier =
- Modifier
- .alpha(
- if (navigator.currentTab == FestabookMainTab.PLACE_MAP) 1f else 0f,
- ).padding(innerPadding),
- ...
-)
+if (navigator.currentTab == FestabookMainTab.PLACE_MAP) {
+ PlaceMapRoute(
+ modifier = Modifier.padding(innerPadding),
+ ...
+ )
+}옵션 2: 터치 이벤트 차단 추가 (상태 유지가 필요한 경우)
PlaceMapRoute(
modifier = Modifier
.alpha(if (navigator.currentTab == FestabookMainTab.PLACE_MAP) 1f else 0f)
.then(
if (navigator.currentTab != FestabookMainTab.PLACE_MAP) {
Modifier.pointerInput(Unit) { /* consume all touches */ }
} else Modifier
)
.padding(innerPadding),
...
)PR 목표에서 "PlaceMap(지도) 화면은 NavHost 외부로 분리하여 상태 유지"를 언급했으므로, 옵션 2가 의도에 맞을 것으로 보입니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt`
around lines 103 - 119, The PlaceMapRoute is only visually hidden via
Modifier.alpha(0f) but still receives touch events; update the modifier on
PlaceMapRoute (where Modifier.alpha is applied) to also block pointer input when
navigator.currentTab != FestabookMainTab.PLACE_MAP (e.g., append a conditional
Modifier.pointerInput that consumes all touches) so the hidden map no longer
intercepts taps while preserving its state; keep the alpha for visual hiding and
only apply the pointerInput when the current tab is not PLACE_MAP.
| onSubscriptionConfirm = { | ||
| notificationPermissionManager.requestNotificationPermission(navigator.navController.context) | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find NotificationPermissionManager implementation
find . -type f -name "*NotificationPermissionManager*"Repository: festabook/android
Length of output: 178
🏁 Script executed:
# Search for requestNotificationPermission method across the codebase
rg -n "requestNotificationPermission" --type=kotlin -B 2 -A 10Repository: festabook/android
Length of output: 6485
🏁 Script executed:
# Find the MainScreen.kt file and examine the context around line 157-159
fd "MainScreen.kt" --type fRepository: festabook/android
Length of output: 141
🏁 Script executed:
# Get the full NotificationPermissionManager.kt file
cat app/src/main/java/com/daedan/festabook/presentation/NotificationPermissionManager.ktRepository: festabook/android
Length of output: 3202
🏁 Script executed:
# Search for how navController is created and what type it is
rg -n "navController" app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt -B 2 -A 2Repository: festabook/android
Length of output: 487
navController.context를 사용하면 알림 권한 요청 다이얼로그 표시 실패 위험이 있습니다.
NotificationPermissionManager.requestNotificationPermission()의 showRationaleDialog() 메서드에서 AlertDialog.Builder(context)를 사용하는데, 이는 반드시 Activity context를 요구합니다. navController.context는 navigation 구성에 따라 Application context나 기타 Context 타입을 반환할 수 있어, 다이얼로그 표시가 실패하거나 예기치 않은 동작을 할 수 있습니다.
SettingScreen.kt(line 77)와 SettingFragment.kt(line 96)처럼 Compose/Fragment에서 직접 제공하는 Activity context를 사용하도록 변경하세요.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/main/component/MainScreen.kt`
around lines 157 - 159, The call in MainScreen's onSubscriptionConfirm passes
navigator.navController.context into
notificationPermissionManager.requestNotificationPermission, but
navController.context can be a non-Activity Context and will break
AlertDialog.Builder in NotificationPermissionManager.showRationaleDialog; change
the call to pass an Activity context (as shown in SettingScreen/SettingFragment
examples) — obtain the hosting Activity (e.g., via LocalContext.current cast to
Activity or navigator-provided Activity/requireActivity()) and pass that
Activity into notificationPermissionManager.requestNotificationPermission
instead of navigator.navController.context.
| fun declineAlert() { | ||
| _isFirstVisit.value = false | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the FestivalRepository interface/class
find . -name "*FestivalRepository*" -type fRepository: festabook/android
Length of output: 224
🏁 Script executed:
# Search for setIsFirstVisit or similar methods across the codebase
rg -n "setIsFirstVisit|saveIsFirstVisit|setFirstVisit" --type=kotlin -iRepository: festabook/android
Length of output: 43
🏁 Script executed:
# Let's examine the MainViewModel.kt file to understand the complete flow
find . -name "MainViewModel.kt" -type fRepository: festabook/android
Length of output: 136
🏁 Script executed:
# Read the FestivalRepository interface
cat -n ./app/src/main/java/com/daedan/festabook/domain/repository/FestivalRepository.ktRepository: festabook/android
Length of output: 548
🏁 Script executed:
# Read the FestivalRepositoryImpl implementation
cat -n ./app/src/main/java/com/daedan/festabook/data/repository/FestivalRepositoryImpl.ktRepository: festabook/android
Length of output: 2091
🏁 Script executed:
# Read the MainViewModel to see the complete context around declineAlert()
cat -n ./app/src/main/java/com/daedan/festabook/presentation/main/MainViewModel.ktRepository: festabook/android
Length of output: 4657
declineAlert()가 상태를 영속화하지 않아 앱 재시작 시 다이얼로그가 다시 표시됩니다.
현재 declineAlert()는 _isFirstVisit의 in-memory 값만 변경합니다. isFirstVisit의 초기값이 festivalRepository.getIsFirstVisit()에서 가져오는데, 사용자가 거절한 후에도 repository에 저장하지 않으면 앱 재시작 시 다이얼로그가 다시 표시됩니다.
FestivalRepository 인터페이스에 상태를 영속화할 메서드가 없으므로, 먼저 repository에 저장 메서드(예: setIsFirstVisit())를 추가한 후 declineAlert()에서 호출해야 합니다.
🤖 Prompt for AI Agents
In `@app/src/main/java/com/daedan/festabook/presentation/main/MainViewModel.kt`
around lines 81 - 83, declineAlert() currently only mutates the in-memory
_isFirstVisit and does not persist the choice, so the dialog reappears after app
restart; add a persistence method to the repository (e.g., add
setIsFirstVisit(Boolean) to FestivalRepository) and implement it in the
repository impl, then update MainViewModel.declineAlert() to call
festivalRepository.setIsFirstVisit(false) before setting _isFirstVisit.value =
false so the user's choice is saved.
| @Composable | ||
| fun PlaceDetailRoute( | ||
| viewModel: PlaceDetailViewModel, | ||
| onBackToPreviousClick: () -> Unit, | ||
| modifier: Modifier = Modifier, | ||
| ) { | ||
| val placeDetailUiState by viewModel.placeDetail.collectAsStateWithLifecycle() | ||
| BackHandler { | ||
| onBackToPreviousClick() | ||
| } | ||
| PlaceDetailScreen( | ||
| modifier = modifier, | ||
| uiState = placeDetailUiState, | ||
| onBackToPreviousClick = onBackToPreviousClick, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "PlaceDetailScreen.kt" | head -5Repository: festabook/android
Length of output: 157
🏁 Script executed:
cat -n app/src/main/java/com/daedan/festabook/presentation/placeDetail/component/PlaceDetailScreen.ktRepository: festabook/android
Length of output: 19459
Route 레벨 BackHandler가 다이얼로그 상태를 고려하지 않음
다이얼로그가 열린 상태에서 뒤로가기를 눌렀을 때 Dialog의 onDismissRequest와 Route의 BackHandler가 모두 실행되어, 다이얼로그가 닫히는 동시에 화면이 전환되는 문제가 발생합니다. 다이얼로그가 열린 상태에서는 Route를 벗어나지 않도록 제어해야 합니다.
PlaceDetailScreen 내부에서 BackHandler(enabled = !isDialogOpen) 형태로 조건부 제어하거나, Route에서 BackHandler를 제거하고 Screen 내부로 이동하는 것을 권장합니다.
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/placeDetail/component/PlaceDetailScreen.kt`
around lines 69 - 84, The Route-level BackHandler in PlaceDetailRoute always
intercepts back presses and causes navigation even when a dialog is open; move
or gate the BackHandler so it respects dialog state: either remove the
BackHandler from PlaceDetailRoute and add it inside PlaceDetailScreen (using
BackHandler(enabled = !uiState.isDialogOpen, onBack = onBackToPreviousClick)),
or keep it in PlaceDetailRoute but wrap its enabled flag with the dialog state
(derive isDialogOpen from viewModel.placeDetail and use BackHandler(enabled =
!isDialogOpen, ...)). Update references: PlaceDetailRoute, PlaceDetailScreen,
BackHandler, and the placeDetail UI state (e.g., uiState.isDialogOpen or
equivalent) so back presses dismiss dialogs first and only navigate when no
dialog is open.
| Box(modifier = modifier) { | ||
| // TODO AndroidView와 CMP 뷰의 혼용으로 컴파일러 경고 발생중 -> 추후 해결하겠습니다 | ||
| AndroidView( | ||
| factory = { mapView }, | ||
| modifier = Modifier.dragInterceptor(onMapDrag), | ||
| onRelease = { | ||
| mapView.onDestroy() | ||
| }, | ||
| ) | ||
| content(mapDelegate.value) | ||
| } | ||
| RegisterMapLifeCycle(mapView) |
There was a problem hiding this comment.
onDestroy() 중복 호출 가능성 확인 필요
onRelease 콜백(line 47)에서 mapView.onDestroy()를 호출하고 있지만, lifecycleObserver(line 140)에서도 ON_DESTROY 이벤트 시 동일하게 this.onDestroy()를 호출합니다.
AndroidView가 컴포지션에서 제거될 때 onRelease가 호출되고, Activity/Fragment의 생명주기가 DESTROY될 때 lifecycle observer도 onDestroy()를 호출할 수 있어 중복 호출이 발생할 수 있습니다.
🔧 제안: lifecycle observer에서 ON_DESTROY 처리 제거 또는 onRelease 제거
옵션 1: lifecycle observer에서 ON_DESTROY 제거 (onRelease에 위임)
private fun MapView.lifecycleObserver(
savedInstanceState: Bundle?,
previousState: MutableState<Lifecycle.Event>,
): LifecycleEventObserver =
LifecycleEventObserver { _, event ->
when (event) {
Lifecycle.Event.ON_CREATE -> this.onCreate(savedInstanceState)
Lifecycle.Event.ON_START -> this.onStart()
Lifecycle.Event.ON_RESUME -> this.onResume()
Lifecycle.Event.ON_PAUSE -> this.onPause()
Lifecycle.Event.ON_STOP -> this.onStop()
- Lifecycle.Event.ON_DESTROY -> this.onDestroy()
- else -> throw IllegalStateException()
+ Lifecycle.Event.ON_DESTROY -> Unit // onRelease에서 처리
+ else -> Unit
}
previousState.value = event
}옵션 2: onRelease 제거하고 lifecycle observer에 위임
AndroidView(
factory = { mapView },
modifier = Modifier.dragInterceptor(onMapDrag),
- onRelease = {
- mapView.onDestroy()
- },
)📝 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.
| Box(modifier = modifier) { | |
| // TODO AndroidView와 CMP 뷰의 혼용으로 컴파일러 경고 발생중 -> 추후 해결하겠습니다 | |
| AndroidView( | |
| factory = { mapView }, | |
| modifier = Modifier.dragInterceptor(onMapDrag), | |
| onRelease = { | |
| mapView.onDestroy() | |
| }, | |
| ) | |
| content(mapDelegate.value) | |
| } | |
| RegisterMapLifeCycle(mapView) | |
| Box(modifier = modifier) { | |
| // TODO AndroidView와 CMP 뷰의 혼용으로 컴파일러 경고 발생중 -> 추후 해결하겠습니다 | |
| AndroidView( | |
| factory = { mapView }, | |
| modifier = Modifier.dragInterceptor(onMapDrag), | |
| ) | |
| content(mapDelegate.value) | |
| } | |
| RegisterMapLifeCycle(mapView) |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/component/NaverMapContent.kt`
around lines 41 - 52, There is a risk of calling mapView.onDestroy() twice
because AndroidView's onRelease and the lifecycle observer in
RegisterMapLifeCycle both invoke onDestroy; choose one strategy and remove the
other: either remove the AndroidView onRelease block (the onRelease lambda that
calls mapView.onDestroy()) and let RegisterMapLifeCycle's lifecycleObserver
handle ON_DESTROY, or keep onRelease and modify RegisterMapLifeCycle to stop
calling this.onDestroy() on Lifecycle.Event.ON_DESTROY (or guard it with an
isDestroyed flag) so mapView.onDestroy() is only invoked once.
| val mapControlSideEffectHandler = | ||
| remember { | ||
| MapControlSideEffectHandler( | ||
| initialPadding = with(density) { 254.dp.toPx() }.toInt(), | ||
| logger = logger, | ||
| locationSource = locationSource, | ||
| viewModel = placeMapViewModel, | ||
| mapDelegate = mapDelegate, | ||
| mapManagerDelegate = mapManagerDelegate, | ||
| ) | ||
| } | ||
| val placeMapSideEffectHandler = | ||
| remember { | ||
| PlaceMapSideEffectHandler( | ||
| mapManagerDelegate = mapManagerDelegate, | ||
| bottomSheetState = bottomSheetState, | ||
| viewModel = placeMapViewModel, | ||
| logger = logger, | ||
| onStartPlaceDetail = onStartPlaceDetail, | ||
| onPreloadImages = { | ||
| preloadImages( | ||
| context = context, | ||
| scope = scope, | ||
| places = it.places, | ||
| ) | ||
| }, | ||
| onShowErrorSnackBar = { }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
remember 블록에 key 파라미터 누락으로 의존성 변경 시 핸들러가 갱신되지 않음
MapControlSideEffectHandler와 PlaceMapSideEffectHandler가 remember로 생성되지만 key가 없어서, placeMapViewModel, logger, locationSource 등의 의존성이 변경되어도 핸들러가 재생성되지 않습니다.
♻️ 수정 제안
val mapControlSideEffectHandler =
- remember {
+ remember(placeMapViewModel, logger, locationSource, mapDelegate, mapManagerDelegate) {
MapControlSideEffectHandler(
initialPadding = with(density) { 254.dp.toPx() }.toInt(),
logger = logger,
locationSource = locationSource,
viewModel = placeMapViewModel,
mapDelegate = mapDelegate,
mapManagerDelegate = mapManagerDelegate,
)
}
val placeMapSideEffectHandler =
- remember {
+ remember(placeMapViewModel, logger, bottomSheetState, mapManagerDelegate, onStartPlaceDetail, context, scope) {
PlaceMapSideEffectHandler(
mapManagerDelegate = mapManagerDelegate,
bottomSheetState = bottomSheetState,
viewModel = placeMapViewModel,
logger = logger,
onStartPlaceDetail = onStartPlaceDetail,
onPreloadImages = {
preloadImages(
context = context,
scope = scope,
places = it.places,
)
},
onShowErrorSnackBar = { },
)
}📝 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 mapControlSideEffectHandler = | |
| remember { | |
| MapControlSideEffectHandler( | |
| initialPadding = with(density) { 254.dp.toPx() }.toInt(), | |
| logger = logger, | |
| locationSource = locationSource, | |
| viewModel = placeMapViewModel, | |
| mapDelegate = mapDelegate, | |
| mapManagerDelegate = mapManagerDelegate, | |
| ) | |
| } | |
| val placeMapSideEffectHandler = | |
| remember { | |
| PlaceMapSideEffectHandler( | |
| mapManagerDelegate = mapManagerDelegate, | |
| bottomSheetState = bottomSheetState, | |
| viewModel = placeMapViewModel, | |
| logger = logger, | |
| onStartPlaceDetail = onStartPlaceDetail, | |
| onPreloadImages = { | |
| preloadImages( | |
| context = context, | |
| scope = scope, | |
| places = it.places, | |
| ) | |
| }, | |
| onShowErrorSnackBar = { }, | |
| ) | |
| } | |
| val mapControlSideEffectHandler = | |
| remember(placeMapViewModel, logger, locationSource, mapDelegate, mapManagerDelegate) { | |
| MapControlSideEffectHandler( | |
| initialPadding = with(density) { 254.dp.toPx() }.toInt(), | |
| logger = logger, | |
| locationSource = locationSource, | |
| viewModel = placeMapViewModel, | |
| mapDelegate = mapDelegate, | |
| mapManagerDelegate = mapManagerDelegate, | |
| ) | |
| } | |
| val placeMapSideEffectHandler = | |
| remember(placeMapViewModel, logger, bottomSheetState, mapManagerDelegate, onStartPlaceDetail, context, scope) { | |
| PlaceMapSideEffectHandler( | |
| mapManagerDelegate = mapManagerDelegate, | |
| bottomSheetState = bottomSheetState, | |
| viewModel = placeMapViewModel, | |
| logger = logger, | |
| onStartPlaceDetail = onStartPlaceDetail, | |
| onPreloadImages = { | |
| preloadImages( | |
| context = context, | |
| scope = scope, | |
| places = it.places, | |
| ) | |
| }, | |
| onShowErrorSnackBar = { }, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/component/PlaceMapScreen.kt`
around lines 69 - 97, The two handlers created with remember
(MapControlSideEffectHandler and PlaceMapSideEffectHandler) lack key parameters
so they won't re-create when their dependencies change; update the remember
calls to include relevant dependency keys (e.g., placeMapViewModel, logger,
locationSource, mapDelegate, mapManagerDelegate for MapControlSideEffectHandler,
and mapManagerDelegate, bottomSheetState, placeMapViewModel, logger,
onStartPlaceDetail, onPreloadImages for PlaceMapSideEffectHandler) so the
handlers are recomposed when any of those referenced values change.
| private inline fun <reified T> defaultNavType() = | ||
| object : NavType<T>(isNullableAllowed = true) { | ||
| override fun get( | ||
| bundle: Bundle, | ||
| key: String, | ||
| ): T? = bundle.getString(key)?.let { Json.decodeFromString(it) } | ||
|
|
||
| override fun parseValue(value: String): T = | ||
| Json.decodeFromString( | ||
| Uri.decode(value), | ||
| ) | ||
|
|
||
| override fun put( | ||
| bundle: Bundle, | ||
| key: String, | ||
| value: T, | ||
| ) = bundle.putString(key, Json.encodeToString(value)) | ||
|
|
||
| override fun serializeAsValue(value: T): String = Uri.encode(Json.encodeToString(value)) | ||
| } |
There was a problem hiding this comment.
parseValue에서 null 처리 누락
isNullableAllowed = true로 설정되어 있지만, parseValue는 nullable 타입에 대해 빈 문자열이나 "null" 문자열을 적절히 처리하지 않습니다. 이로 인해 nullable 파라미터 전달 시 런타임 예외가 발생할 수 있습니다.
🐛 수정 제안
private inline fun <reified T> defaultNavType() =
object : NavType<T>(isNullableAllowed = true) {
override fun get(
bundle: Bundle,
key: String,
): T? = bundle.getString(key)?.let { Json.decodeFromString(it) }
- override fun parseValue(value: String): T =
- Json.decodeFromString(
- Uri.decode(value),
- )
+ override fun parseValue(value: String): T {
+ val decoded = Uri.decode(value)
+ `@Suppress`("UNCHECKED_CAST")
+ if (decoded == "null" || decoded.isEmpty()) {
+ return null as T
+ }
+ return Json.decodeFromString(decoded)
+ }
override fun put(
bundle: Bundle,
key: String,
value: T,
- ) = bundle.putString(key, Json.encodeToString(value))
+ ) {
+ bundle.putString(key, value?.let { Json.encodeToString(it) })
+ }
- override fun serializeAsValue(value: T): String = Uri.encode(Json.encodeToString(value))
+ override fun serializeAsValue(value: T): String =
+ value?.let { Uri.encode(Json.encodeToString(it)) } ?: "null"
}🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/presentation/placeMap/navigation/PlaceMapNavigation.kt`
around lines 72 - 91, The NavType created by defaultNavType() marks
isNullableAllowed = true but parseValue (and get) don't handle nullable
payloads: update parseValue to first Uri.decode(value) and if the decoded string
is empty or equals "null" return null as T, otherwise Json.decodeFromString;
likewise change get to treat bundle.getString(key) that is null, empty, or
"null" as null (returning null as T) before calling Json.decodeFromString;
reference the defaultNavType() anonymous NavType<T> and its parseValue and get
methods when applying the fix.
#️⃣ 이슈 번호
🛠️ 작업 내용
🙇🏻 중점 리뷰 요청
📸 이미지 첨부 (Optional)
mobizen_20260123_180316.mp4
Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링
의존성
✏️ Tip: You can customize this high-level summary in your review settings.