[feat] #52 네컷지도 API 연동 및 UX 개선#60
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`:
- Around line 365-374: The loading dialog can trap users when isLoadingLocation
(computed from locationTrackingMode == LocationTrackingMode.Follow &&
uiState.currentLocLatLng == null) never resolves; add a timeout and a
user-cancel path: start a timer/coroutine when isLoadingLocation becomes true
that resets locationTrackingMode (or updates uiState.isLoading) after a
configurable timeout and surface an error state, and update the LoadingDialog
usage to expose a cancel action (or set DialogProperties.dismissOnBackPress =
true) that calls the same cancel handler; reference the isLoadingLocation
expression, LocationTrackingMode.Follow, uiState.currentLocLatLng, LoadingDialog
and DialogProperties to locate where to add the timer, cancel callback and state
update.
🧹 Nitpick comments (2)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt (2)
207-222: 사용되지 않는state파라미터
loadBrands함수에서state파라미터가 전달되지만 함수 내부에서 사용되지 않습니다. 불필요한 파라미터는 제거하는 것이 좋습니다.♻️ 리팩터링 제안
- private fun loadBrands( - state: MapState, - reduce: (MapState.() -> MapState) -> Unit, - ) { + private fun loadBrands( + reduce: (MapState.() -> MapState) -> Unit, + ) {호출부(line 32)도 함께 수정:
- MapIntent.EnterMapScreen -> loadBrands(state, reduce) + MapIntent.EnterMapScreen -> loadBrands(reduce)
178-196: 디버그 로그 제거 고려
Timber.d호출이 프로덕션 코드에 남아있습니다. 개발 완료 후 제거하거나, 필요시 더 낮은 레벨의 로깅을 사용하는 것이 좋습니다.또한,
currentLocLatLng가 null일 때distance를 0으로 설정하면 UI에서 "0m"로 표시될 수 있습니다. 위치를 알 수 없는 경우와 실제로 0m인 경우를 구분할 수 없으므로,distance필드를 nullable로 처리하거나 -1 같은 sentinel 값 사용을 고려해 보세요.
| val isLoadingLocation = locationTrackingMode == LocationTrackingMode.Follow && uiState.currentLocLatLng == null | ||
|
|
||
| if (isLoadingLocation || uiState.isLoading) { | ||
| LoadingDialog( | ||
| properties = DialogProperties( | ||
| dismissOnBackPress = false, | ||
| dismissOnClickOutside = false, | ||
| ), | ||
| ) | ||
| } |
There was a problem hiding this comment.
위치 로딩 타임아웃 고려
isLoadingLocation 상태에서 위치 획득이 실패하거나 오래 걸리면 사용자가 로딩 다이얼로그에 갇힐 수 있습니다. dismissOnBackPress = false로 설정되어 있어 취소도 불가능합니다.
타임아웃 처리나 취소 버튼 추가를 고려해 보세요.
🤖 Prompt for AI Agents
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`
around lines 365 - 374, The loading dialog can trap users when isLoadingLocation
(computed from locationTrackingMode == LocationTrackingMode.Follow &&
uiState.currentLocLatLng == null) never resolves; add a timeout and a
user-cancel path: start a timer/coroutine when isLoadingLocation becomes true
that resets locationTrackingMode (or updates uiState.isLoading) after a
configurable timeout and surface an error state, and update the LoadingDialog
usage to expose a cancel action (or set DialogProperties.dismissOnBackPress =
true) that calls the same cancel handler; reference the isLoadingLocation
expression, LocationTrackingMode.Follow, uiState.currentLocLatLng, LoadingDialog
and DialogProperties to locate where to add the timer, cancel callback and state
update.
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
고생하셨습니다!
카톡으로 말씀하셨던 것처럼 수정사항 이후 재 리뷰 하겠습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (1)
109-118: 권한 거부 처리 주석/로직 불일치Line 115의 “2회 이상 거부” 주석과 달리, 현재 조건은
previous/current가 모두false일 때만 다이얼로그가 뜹니다. 2회 이상 거부를 의도했다면 거부 횟수 추적이 필요하고, 그렇지 않다면 주석을 의도에 맞게 정리해 주세요.✏️ 주석 정리 예시
- // 2회 이상 거부 + // "다시 묻지 않음" 등으로 rationale이 더 이상 표시되지 않을 때
🤖 Fix all issues with AI agents
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/PhotoBoothDetailContent.kt`:
- Around line 123-137: The Icon inside the Box currently has contentDescription
= null which breaks accessibility; update the Icon in PhotoBoothDetailContent
(the Box with .noRippleClickableSingle(onClick = onClickDirection)) to provide a
descriptive contentDescription such as "길찾기" (or a translatable string resource)
so screen readers announce the button purpose; ensure you set the
contentDescription on the Icon and keep tint and other properties unchanged.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`:
- Around line 212-218: The onMapLoaded handler currently skips dispatching
MapIntent.NaverMapLoaded when permissions are granted, preventing initial data
loading; modify the onMapLoaded block to always call
viewModel.store.onIntent(MapIntent.NaverMapLoaded) even when
LocationPermissionManager.isGrantedLocationPermission(context) is true (i.e.,
call viewModel.store.onIntent(MapIntent.NaverMapLoaded) before or after invoking
setFollowMode()), so the ViewModel receives the NaverMapLoaded intent and can
trigger MapEffect.LoadInitialPhotoBooths.
🧹 Nitpick comments (1)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/PhotoBoothCard.kt (1)
34-119: 주석 처리된 PhotoBoothCard는 사용되지 않으므로 삭제 권장코드베이스 전체에서
PhotoBoothCard()함수 호출이 없습니다. 현재MapScreen.kt에서는PhotoBoothDetailContent로 대체되어 있으며,ClickPhotoBoothCard등의 Intent 객체는 존재하지만 이는 composable 함수와 무관합니다. 주석 처리된 코드 블록 전체를 제거하는 것이 안전합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt`:
- Around line 88-91: Remove the hardcoded JWT string from the BearerTokens
construction and make the lambda return the computed token value from the
existing conditional token-loading logic (so the last expression is the
conditional result rather than the literal); specifically update the
BearerTokens(accessToken = ..., refreshToken =
tokenRepository.getRefreshToken().first()) usage to obtain accessToken from the
tokenRepository/getAccessToken flow used earlier (or the conditional that checks
stored token presence) instead of the hardcoded string, ensuring the lambda's
final expression is the conditional/tokenRepository call so the stored token is
actually returned.
Ojongseok
force-pushed
the
feat/#52-map-api
branch
from
1eec767 to
4c8f5b1
Compare
[refactor] #65 네컷지도 관련 로직 개선
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`:
- Around line 85-96: cameraPositionState.contentBounds can be null (e.g.,
permission denied or timing), so update MapScreen to fall back to a predefined
MapBounds when contentBounds is null instead of skipping the load; in the code
around cameraPositionState.contentBounds and the
MapIntent.LoadPhotoBoothsByBounds call, compute a default MapBounds (e.g.,
centered on 강남역 with reasonable bbox) and call
viewModel.store.onIntent(MapIntent.LoadPhotoBoothsByBounds(defaultBounds)) so
the behavior matches the GrantedLocationPermission path and avoids an empty map
when contentBounds is unavailable.
🧹 Nitpick comments (3)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/PhotoBoothMarker.kt (1)
183-196: Placeholder 이미지 선택 검토 필요
cachedBitmap이 null일 때R.drawable.icon_info_gray_stroke를 placeholder로 사용하고 있습니다. 이 아이콘이 브랜드 로고 placeholder로 적절한지 확인이 필요합니다. 브랜드 이미지 로딩 전용 placeholder 이미지 사용을 고려해 보세요.feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt (2)
294-323: ImageLoader 인스턴스 재사용 권장
cacheBrandImages함수가 호출될 때마다 새로운ImageLoader인스턴스를 생성합니다. Coil의ImageLoader는 캐시와 스레드 풀을 관리하므로, 매번 새로 생성하면 리소스 낭비가 발생합니다.ViewModel 클래스 레벨에서 한 번만 생성하거나, Coil의 싱글톤
ImageLoader를 사용하세요.♻️ 리팩토링 제안
`@HiltViewModel` class MapViewModel `@Inject` constructor( `@ApplicationContext` private val context: Context, private val mapRepository: MapRepository, ) : ViewModel() { + private val imageLoader = ImageLoader(context) // ... private fun cacheBrandImages( brands: List<Brand>, reduce: (MapState.() -> MapState) -> Unit, ) { viewModelScope.launch { - val imageLoader = ImageLoader(context) val cache = brands
157-165: 불필요한 LocLatLng 생성
state.currentLocLatLng은 이미LocLatLng타입이므로, 새로운LocLatLng인스턴스를 생성할 필요가 없습니다.♻️ 리팩토링 제안
if (state.currentLocLatLng != null) { reduce { copy(isCameraOnCurrentLocation = true, isVisibleRefreshButton = false) } postSideEffect( - MapEffect.MoveCameraToPosition( - LocLatLng(state.currentLocLatLng.latitude, state.currentLocLatLng.longitude), - isRequiredLoadPhotoBooths = true, - ), + MapEffect.MoveCameraToPosition(state.currentLocLatLng, isRequiredLoadPhotoBooths = true), ) }
| cameraPositionState.contentBounds?.let { bounds -> | ||
| viewModel.store.onIntent( | ||
| MapIntent.LoadPhotoBoothsByBounds( | ||
| MapBounds( | ||
| southWest = LocLatLng(bounds.southWest.latitude, bounds.southWest.longitude), | ||
| northWest = LocLatLng(bounds.northWest.latitude, bounds.northWest.longitude), | ||
| northEast = LocLatLng(bounds.northEast.latitude, bounds.northEast.longitude), | ||
| southEast = LocLatLng(bounds.southEast.latitude, bounds.southEast.longitude), | ||
| ), | ||
| ), | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "LoadPhotoBoothsByBounds\|contentBounds" --type=kotlin -C 3 feature/map/impl/Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 61
🏁 Script executed:
git ls-files | grep -i mapscreen | head -20Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 156
🏁 Script executed:
git ls-files | grep -E "feature.*map" | head -20Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 1656
🏁 Script executed:
wc -l feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 160
🏁 Script executed:
cat -n feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 17000
🏁 Script executed:
cat -n feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapContract.kt | head -100Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 4168
🏁 Script executed:
cat -n feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt | head -150Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 8096
🏁 Script executed:
sed -n '150,250p' feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 3854
🏁 Script executed:
rg -n "RequestLocationPermission|LoadPhotoBoothsByBounds" feature/map/impl/ -A 2 -B 2Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 5267
🏁 Script executed:
sed -n '250,332p' feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 3018
🏁 Script executed:
rg -n "loadBrands\|loadPhotoBoothsByPolygon" feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt -A 15Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 61
🏁 Script executed:
sed -n '1,332p' feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt | tail -150Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 5613
🏁 Script executed:
rg -n "loadPhotoBoothsByPolygon" feature/map/impl/ -A 25Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 9111
🏁 Script executed:
sed -n '354,400p' feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 1782
권한 거부 시 contentBounds null일 때 포토부스 로딩 실패 발생 가능
권한 결과 콜백에서 cameraPositionState.contentBounds가 null일 경우, 현재 코드는 ?.let 블록을 건너뛰므로 포토부스 데이터가 로드되지 않습니다. 권한 허용 시는 GrantedLocationPermission 인텐트를 통해 현재 위치 조회 후 포토부스를 로드하는 반면, 권한 거부 시는 contentBounds 존재 여부에만 의존하므로 차이가 있습니다.
지도가 완전히 초기화되지 않았거나 타이밍 문제로 contentBounds가 null이면 빈 지도가 표시될 수 있습니다. 기본 좌표(강남역 등)를 사용한 대체 로직이 필요합니다.
🤖 Prompt for AI Agents
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`
around lines 85 - 96, cameraPositionState.contentBounds can be null (e.g.,
permission denied or timing), so update MapScreen to fall back to a predefined
MapBounds when contentBounds is null instead of skipping the load; in the code
around cameraPositionState.contentBounds and the
MapIntent.LoadPhotoBoothsByBounds call, compute a default MapBounds (e.g.,
centered on 강남역 with reasonable bbox) and call
viewModel.store.onIntent(MapIntent.LoadPhotoBoothsByBounds(defaultBounds)) so
the behavior matches the GrantedLocationPermission path and avoids an empty map
when contentBounds is unavailable.
2 participants
🔗 관련 이슈
📙 작업 설명
API 연동
각종 UX 개선
📸 스크린샷 또는 시연 영상 (선택)
KakaoTalk_Video_2026-01-28-00-56-26.mp4
KakaoTalk_Video_2026-01-28-01-05-42.mp4
KakaoTalk_Video_2026-01-28-00-59-13.mp4
KakaoTalk_Video_2026-01-28-01-01-20.mp4
💬 추가 설명 or 리뷰 포인트 (선택)
마커 이미지 렌더링 관련
MarkerComposable내부의rememberComposeOverlayImage()가 Composable을 동기적으로 Bitmap으로 캡처합니다rememberCachedBrandImage()로 브랜드 이미지를 사전에 ImageBitmap으로 변환 후 Image Composable에 전달하는 방식을 사용했습니다.위치 권한 관련 질문
Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링
✏️ Tip: You can customize this high-level summary in your review settings.
Summary by CodeRabbit
릴리즈 노트
새로운 기능
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.