-
Notifications
You must be signed in to change notification settings - Fork 0
[feat] #13 네이버지도 API 연동 #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughGradle에서 local.properties의 NAVER_MAP_CLIENT_ID를 BuildConfig로 주입하고, 앱 시작 시 해당 키로 NaverMapSdk를 초기화합니다. Compose 기반 MapRoute/MapScreen와 AnchoredDraggablePanel 컴포저블이 추가되며, 맵 SDK 의존성과 Maven 리포지토리가 등록됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Gradle as Gradle (build)
participant BuildConfig as BuildConfig (generated)
participant App as Application.onCreate
participant NaverSDK as NaverMapSdk
participant UI as Compose MapScreen
Gradle->>BuildConfig: inject NAVER_MAP_CLIENT_ID from local.properties
App->>BuildConfig: access NAVER_MAP_CLIENT_ID
App->>NaverSDK: initialize NcpKeyClient(BuildConfig.NAVER_MAP_CLIENT_ID)
UI->>NaverSDK: render NaverMap (MapRoute/MapScreen)
UI->>UI: AnchoredDraggablePanel handles drag anchors & animations
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @app/build.gradle.kts:
- Around line 16-20: The BuildConfig string value is missing surrounding quotes
causing invalid generated Java; update the buildConfigField call for
"NAVER_MAP_CLIENT_ID" to wrap the returned value from
getLocalPropertiesValue("NAVER_MAP_CLIENT_ID") in quotes so the generated
BuildConfig field is a valid Java string literal (i.e., ensure the third
argument passed to buildConfigField is a quoted string expression).
- Around line 48-50: getLocalPropertiesValue currently calls
gradleLocalProperties(...).getProperty(propertyKey) which can return null;
update getLocalPropertiesValue to handle missing properties by either throwing a
clear exception (e.g., IllegalStateException with a message including the
propertyKey) or returning a safe default after logging a warning, so callers
don’t get a null; locate the call in function getLocalPropertiesValue and add
the null-check/handling around gradleLocalProperties(rootDir,
providers).getProperty(propertyKey).
- Around line 15-21: Ensure the Naver map client setup is safe and will
authenticate: register your Android package names (up to 10) in Naver Cloud
Platform to match the app package used at runtime; confirm the buildConfigField
call that injects NAVER_MAP_CLIENT_ID via
getLocalPropertiesValue("NAVER_MAP_CLIENT_ID") is sourcing a value from a
local.properties file that is listed in .gitignore so it is not committed; and
never put any client secret into buildConfig or local.properties—if a secret is
required, remove it from the app (do not add it to NAVER_MAP_CLIENT_ID or
getLocalPropertiesValue) and implement server-side handling for secret-based
calls instead.
In @app/src/main/java/com/neki/android/app/NekiApplication.kt:
- Around line 24-25: Validate BuildConfig.NAVER_MAP_CLIENT_ID before calling
NaverMapSdk.getInstance(this).client = NaverMapSdk.NcpKeyClient(...): check for
null/empty/invalid value and skip initialization if invalid, and wrap the
assignment in a try-catch around NaverMapSdk.getInstance(...) /
NaverMapSdk.NcpKeyClient(...) to log the exception (using your app logger) and
avoid crashing; update the NekiApplication initialization so that
NaverMapSdk.getInstance, NcpKeyClient and BuildConfig.NAVER_MAP_CLIENT_ID are
guarded and failures are handled gracefully (log error and disable map features
or provide a safe fallback).
In @feature/map/impl/build.gradle.kts:
- Around line 11-12: The module currently exposes map SDK dependencies with
api(libs.map.sdk) and api(libs.naver.map.compose) which leak them to consumers;
change both to implementation(libs.map.sdk) and
implementation(libs.naver.map.compose) in feature/map/impl/build.gradle.kts
because NaverMap and navermap.compose are only used inside MapScreen() and are
not part of the public API, ensuring the feature/map module encapsulates the
SDK.
In
@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt:
- Around line 16-19: MapRoute declares a modifier parameter but never uses it
when calling MapScreen, preventing callers from controlling layout; update
MapRoute to forward the modifier to MapScreen (i.e., call MapScreen(modifier =
modifier)) so the incoming Modifier is applied, or if MapScreen has a
differently named parameter, pass modifier to that parameter; remove the unused
parameter only if intentional.
- Around line 24-26: The MapScreen function declares a modifier parameter but
ignores it by using Modifier.fillMaxSize() directly; update the Box call inside
MapScreen to chain the incoming modifier with the full-size modifier (e.g., use
modifier.fillMaxSize() or modifier.then(Modifier.fillMaxSize())) so the caller's
modifier is respected; locate the MapScreen function and the Box(...) usage and
replace Modifier.fillMaxSize() with the chained form.
🧹 Nitpick comments (6)
gradle/libs.versions.toml (1)
75-76: 비공식 Compose 래퍼 라이브러리 사용에 주의하세요.
naver-map-compose는 커뮤니티에서 제작한 비공식 래퍼 라이브러리입니다. 널리 사용되지만, 공식 지원이 아니므로 향후 유지보수나 호환성 문제가 발생할 수 있습니다.네이버에서 공식 Compose 지원을 제공하는지 확인하고, 가능하다면 공식 라이브러리로 마이그레이션하는 것을 고려하세요.
Does Naver Maps SDK for Android provide official Jetpack Compose support?feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/AnchoredDraggablePanel.kt (3)
28-28: 컴포저블의 재사용성을 위한 매개변수 추가를 고려해보세요.현재
AnchoredDraggablePanel은 매개변수가 없어 재사용이 어렵습니다. 콘텐츠, 초기 상태, anchor 비율 등을 커스터마이징할 수 있도록 매개변수를 추가하면 좋습니다.♻️ 매개변수 추가 제안
@Composable -fun AnchoredDraggablePanel() { +fun AnchoredDraggablePanel( + modifier: Modifier = Modifier, + initialValue: DragValue = DragValue.Start, + content: @Composable () -> Unit, +) { val density = LocalDensity.current val configuration = LocalConfiguration.current // ... state setup ... Box( - modifier = Modifier + modifier = modifier .fillMaxSize() .offset { IntOffset(0, state.requireOffset().roundToInt()) } .anchoredDraggable( state = state, orientation = Orientation.Vertical, ), ) { Box( modifier = Modifier .fillMaxSize() .background( color = Color.White, shape = RoundedCornerShape(topStart = 20.dp, topEnd = 20.dp), ) .padding(20.dp), ) { - Text( - text = "시트의 내용", - color = Color.Black, - ) + content() } } }
73-74: 하드코딩된 텍스트를 문자열 리소스로 이동하세요."시트의 내용"이 하드코딩되어 있습니다. 다국어 지원과 유지보수를 위해 문자열 리소스로 관리하는 것이 좋습니다.
36-40: Anchor 비율을 상수 또는 매개변수로 추출하는 것을 고려하세요.0.8f, 0.5f, 0.2f와 같은 매직 넘버가 사용되고 있습니다. 가독성과 유지보수를 위해 의미 있는 이름의 상수로 정의하거나 매개변수로 받는 것이 좋습니다.
♻️ 상수 추출 예시
private object AnchorRatios { const val START = 0.8f // Panel mostly hidden const val CENTER = 0.5f // Panel half visible const val END = 0.2f // Panel mostly visible } // Usage in DraggableAnchors: val anchors = DraggableAnchors { DragValue.Start at screenHeightPx * AnchorRatios.START DragValue.Center at screenHeightPx * AnchorRatios.CENTER DragValue.End at screenHeightPx * AnchorRatios.END }feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (2)
13-13: DragValue enum을 공유 위치로 이동하는 것을 고려하세요.
DragValue가MapScreen.kt에 정의되어 있지만AnchoredDraggablePanel.kt에서도 사용됩니다. enum을 별도 파일이나 공통 패키지로 이동하면 모듈 간 결합도를 낮출 수 있습니다.♻️ 별도 파일로 분리 제안
새 파일
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/DragValue.kt생성:package com.neki.android.feature.map.impl enum class DragValue { Start, Center, End }그런 다음 두 파일 모두에서 import하여 사용합니다.
22-22: 실험적 API 사용에 대한 문서화를 추가하세요.
@OptIn(ExperimentalNaverMapApi::class)를 사용하는MapScreen()함수에 주석을 추가하여 왜 이 실험적 API를 사용하는지, 그리고 언제까지 모니터링이 필요한지 명시해야 합니다. Naver Map Compose의 해당 API는 아직 안정화되지 않았으며 향후 릴리스에서 변경될 수 있으므로, 호환성 문제에 대비하기 위해 의도적 사용임을 코드에 문서화하는 것이 중요합니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/build.gradle.ktsapp/src/main/java/com/neki/android/app/NekiApplication.ktfeature/map/impl/build.gradle.ktsfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapEntryProvider.ktfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.ktfeature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/AnchoredDraggablePanel.ktgradle/libs.versions.tomlsettings.gradle.kts
🧰 Additional context used
🧬 Code graph analysis (2)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (2)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/AnchoredDraggablePanel.kt (1)
AnchoredDraggablePanel(27-79)core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(34-56)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapEntryProvider.kt (1)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (1)
MapRoute(15-20)
🔇 Additional comments (6)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapEntryProvider.kt (1)
27-27: 네비게이션 엔트리가 올바르게 구성되었습니다.MapRoute()로의 변경이 적절하며, MapScreen.kt에 정의된 컴포저블과 올바르게 연결되어 있습니다.
app/src/main/java/com/neki/android/app/NekiApplication.kt (1)
25-25: BuildConfig.NAVER_MAP_CLIENT_ID 필드가 올바르게 생성되고 있으며, 보안 설정도 적절합니다. app/build.gradle.kts에서 buildConfigField로 정의되어 있고, local.properties는 .gitignore에 포함되어 있으며 Git에 추적되지 않습니다.settings.gradle.kts (1)
22-22: 네이버 Maven 저장소 URL은 공식 문서에서 권장되는 올바른 주소입니다.
https://repository.map.naver.com/archive/maven은 네이버 공식 문서(https://navermaps.github.io/android-map-sdk/guide-en/1.html)에서 권장하는 저장소 URL로 확인됩니다. 프로젝트에서com.naver.maps:map-sdk(3.23.0)와io.github.fornewid:naver-map-compose(1.9.0)를 정상적으로 사용 중입니다. 빌드 시 의존성 해석이 성공적으로 이루어지는지 확인하여 저장소 접근성을 최종 검증하세요.gradle/libs.versions.toml (1)
22-23: 라이브러리 버전이 이미 최신입니다.두 라이브러리 모두 최신 버전으로 선언되어 있으며 알려진 보안 취약점이 없습니다:
mapSdk = "3.23.0"✓ (현재 최신)naverMapCompose = "1.9.0"✓ (현재 최신)feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (2)
30-34: 지도와 패널의 상호작용을 검증하세요.
AnchoredDraggablePanel이 전체 화면을 차지하고 지도 위에 오버레이되어 있습니다. 패널이 확장되었을 때 지도의 터치 이벤트가 올바르게 처리되는지 확인이 필요합니다.다음 시나리오를 테스트하여 상호작용을 확인해 주세요:
- 패널이 Start 위치(80%)일 때 지도 제스처(핀치 줌, 패닝)가 정상 작동하는지
- 패널이 End 위치(20%)일 때 패널 영역과 지도 영역의 터치 이벤트가 올바르게 구분되는지
- 패널 드래그 중 지도가 의도치 않게 반응하지 않는지
15-20: MapRoute 구현이 잘 되어 있습니다.Navigation 진입점으로서 역할이 명확하며, 향후 ViewModel이나 추가 로직을 주입하기 좋은 구조입니다.
...map/impl/src/main/java/com/neki/android/feature/map/impl/component/AnchoredDraggablePanel.kt
Show resolved
Hide resolved
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt
Show resolved
Hide resolved
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt
Show resolved
Hide resolved
ikseong00
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BottomSheet나 BottomDialog 대신에 DraggableAnchor 를 사용한 이유가 있으실까요??
| dependencies { | ||
| implementation(projects.feature.map.api) | ||
| } | ||
| api(libs.map.sdk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기서 api 로 선언하신 이유가 있으실까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app 모듈의 Application 클래스에서 NaverMap Sdk를 초기화하기 위함입니다!
app모듈에 별도 의존성을 추가하는 것 보다 :map:impl을 임포트하고 있기 때문에 impl/build.gradle에 api로 추가를 했습니다
우선 가장 큰 이유는 BottomSheet와 BottomDialog 모두 디바이스 최하단(?), BottomNavigation 영역 위에서 올라오기 때문이었습니다. 제가 이해하기로는 이부분을 제어하는 것이 불가능해 슬라이딩 패널 역할을 할 수 있는 DraggableAnchor를 활용하려 했습니다. 상/하단 스와이프됨에 따라 2~3분할로 영역을 자연스럽게 조절하기 편리함도 있구요. |
🔗 관련 이슈
#13 의 이슈를 개발하려 했으나 다른 feature 우선으로 인해 이슈를 닫지는 않고 병합만 진행하려 합니다.
📙 작업 설명
📸 스크린샷 또는 시연 영상 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
새로운 기능
설정
✏️ Tip: You can customize this high-level summary in your review settings.