-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/#217 세션 디테일 네이버 지도 SDK 연동 #220
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
Conversation
… Add SessionNaverMap composable and update SessionScreen\n- Register :feature:session in settings.gradle.kts\n- Wire navigation in YappNavHost\n- Update app/feature build configs and libs versions\n- Adjust AndroidManifest as needed
- SessionNaverMap에 Preview 모드 지원 추가 - Preview에서 위치 아이콘과 "지도 영역" 텍스트 표시 - 둥근 모서리 테두리로 지도 영역 강조 - Uri.parse() 대신 toUri() 확장 함수 사용 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough로컬 프로퍼티에서 NAVER_MAP_CLIENT_ID를 읽어 매니페스트 플레이스홀더로 주입하고, AndroidManifest에 Naver Map SDK 키 메타데이터를 추가했습니다. 네비게이션 시작 지점을 SessionRoute로 변경하고 sessionNavGraph를 포함했습니다. 디자인 시스템에 외곽선 아이콘 버튼을 추가했으며, 세션 화면에 Naver 지도 컴포저블과 앱/웹 연동 액션을 도입했습니다. Naver Map SDK 저장소/버전/의존성을 설정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App
participant NavHost
participant SessionGraph as sessionNavGraph
User->>App: 앱 실행
App->>NavHost: NavHost 구성(startDestination=SessionRoute)
NavHost->>SessionGraph: sessionNavGraph 등록
Note right of SessionGraph: 세션 관련 화면으로 초기 진입
sequenceDiagram
autonumber
participant Compose as SessionScreen
participant MapComposable as SessionNaverMap
participant MapView
participant NaverSDK as NaverMap SDK
Compose->>MapComposable: center 전달
MapComposable->>MapView: onCreate(null)
MapComposable->>MapView: getMapAsync(callback)
rect rgba(200,235,255,0.3)
Note right of MapView: Lifecycle 연동
MapComposable->>MapView: onStart/onResume (화면 진입)
end
MapView-->>MapComposable: onMapReady(NaverMap)
MapComposable->>NaverSDK: CameraUpdate to center
MapComposable->>NaverSDK: Marker(center) 추가
rect rgba(255,230,200,0.3)
Note right of MapView: 화면 종료 시 정리
MapComposable->>MapView: onPause/onStop/onDestroy
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
|
@coderabbital review |
|
@jinukeu I'll review the PR changes for the Naver Map SDK integration. ✅ Actions performedReview triggered.
|
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: 15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
app/build.gradle.kts(1 hunks)app/src/main/AndroidManifest.xml(2 hunks)app/src/main/java/com/yapp/app/official/navigation/YappNavHost.kt(3 hunks)core/designsystem/src/main/java/com/yapp/core/designsystem/component/button/icons/OutlinedIconButtonDefaults.kt(1 hunks)core/designsystem/src/main/java/com/yapp/core/designsystem/component/button/icons/YappOutlinedIconButton.kt(1 hunks)feature/session/build.gradle.kts(1 hunks)feature/session/src/main/java/com/yapp/feature/session/SessionNaverMap.kt(1 hunks)feature/session/src/main/java/com/yapp/feature/session/SessionScreen.kt(6 hunks)feature/session/src/main/res/drawable/icon_copy.xml(1 hunks)gradle/libs.versions.toml(2 hunks)settings.gradle.kts(1 hunks)
⏰ 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). (2)
- GitHub Check: ci_dev (build, ./gradlew app:assembleDebug) / build_check
- GitHub Check: ci_dev (test, ./gradlew clean testDebugUnitTest) / build_check
🔇 Additional comments (7)
core/designsystem/src/main/java/com/yapp/core/designsystem/component/button/icons/OutlinedIconButtonDefaults.kt (1)
10-16: 디폴트 정의 방향성 LGTM사이즈/컬러 디폴트를 분리한 구조가 재사용과 테마 일관성에 이점이 있습니다.
gradle/libs.versions.toml (2)
58-58: 버전 카탈로그 항목 추가 LGTM버전 카탈로그에 별칭을 추가해 모듈 간 일관성이 좋아졌습니다.
11-11: 맵 SDK 버전 검증 — com.naver.maps:map-sdk 3.22.1이 최신입니다.
gradle/libs.versions.toml (라인 11): mapSdk = "3.22.1"
릴리즈 노트(CHANGELOG.md, NAVER Maps SDK 레퍼런스)에서 주요 변경(키 이름/메타데이터) 및 호환성 노트를 검토하세요.settings.gradle.kts (1)
22-27: 전용 저장소 + group 필터 적용은 적절 — CI/캐시 가용성 확인 필요content { includeGroup("com.naver.maps") }로 POM 충돌 예방 잘됨. gradle/libs.versions.toml에 map-sdk 선언 확인(gradle/libs.versions.toml:58). CI/캐시/미러 환경에서 해당 저장소로 의존성이 정상 해석되는지 확인.
app/build.gradle.kts (1)
31-34: AndroidManifest meta-data 키를com.naver.maps.map.NCP_KEY_ID로 사용하도록 확인/수정Naver Maps Android SDK v3는 meta-data name으로
com.naver.maps.map.NCP_KEY_ID를 사용합니다(com.naver.maps.map.CLIENT_ID는 구식). meta-data 이름이 맞지 않으면 런타임 초기화 실패가 발생합니다.
파일: app/build.gradle.kts (31–34행) — 현재manifestPlaceholders["NAVER_MAP_CLIENT_ID"] = ...로 값이 주입되고 있으니, AndroidManifest의 meta-data가 아래처럼NCP_KEY_ID를 참조하도록 설정되어 있는지 확인하세요:
feature/session/build.gradle.kts (1)
15-18: ProGuard 및 위치 권한 선언 확인 필요
- AndroidManifest.xml에 ACCESS_COARSE_LOCATION 및/또는 ACCESS_FINE_LOCATION 권한 선언하고 런타임 요청 처리했는지 확인
- Minify(R8) 활성화 시 proguard-rules.pro에 아래 규칙이 있는지 확인
-keep class com.naver.maps.** { *; } -dontwarn com.naver.maps.** -keep class com.naver.*.NaverMap* { *; }app/src/main/AndroidManifest.xml (1)
18-21: manifestPlaceholders 설정 확인 — app/build.gradle.ktsapp/build.gradle.kts에서 manifestPlaceholders["NAVER_MAP_CLIENT_ID"] = localProperties.getProperty("NAVER_MAP_CLIENT_ID", "")로 설정되어 local.properties에서 값을 읽습니다. local.properties에 NAVER_MAP_CLIENT_ID를 추가하세요; 현재 값이 없으면 빈 문자열("")이 들어가므로 누락 시 런타임 문제 발생 가능 — 누락 시 빌드 실패 처리로 변경하는 것을 권장합니다.
| NavHost( | ||
| navController = navigator.navController, | ||
| startDestination = navigator.startDestination, | ||
| startDestination = SessionRoute, |
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.
🧩 Analysis chain
startDestination이 고정(SessionRoute)되어 로그인/온보딩 플로우를 우회할 수 있음
이 변경은 앱 구동 시 세션 화면으로 바로 진입시켜 기존 상태 기반(로그인 여부 등) 시작 지점을 무시할 수 있습니다. QA/개발 빌드 한정이 아니라면 게이트(플래그/조건)로 감싸 주세요. 또한 startDestination는 String을 기대합니다. SessionRoute가 객체/타입이라면 .route 등 실제 경로 문자열을 넘기도록 확인이 필요합니다.
다음으로 정의/사용을 확인해 주세요:
🏁 Script executed:
#!/bin/bash
# SessionRoute 실제 타입/프로퍼티 확인
rg -n -C2 -S 'SessionRoute' --type=kt
# sessionNavGraph 시그니처 확인
rg -n -C2 -S 'fun\s+sessionNavGraph\s*\(' --type=kt -PLength of output: 53
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search: SessionRoute usages ==="
rg -n -C2 -S 'SessionRoute' -g '**/*.kt' || true
echo "=== Search: sessionNavGraph signature ==="
rg -n -C2 -S -P 'fun\s+sessionNavGraph\s*\(' -g '**/*.kt' || true
echo "=== Search: startDestination assignments ==="
rg -n -C3 -S 'startDestination\s*=' -g '**/*.kt' || true
echo "=== Search: SessionRoute definition patterns (object/val/const/class) ==="
rg -n -C2 -S -e 'object\s+SessionRoute' -e 'val\s+SessionRoute' -e 'const\s+val\s+SessionRoute' -e 'class\s+SessionRoute' -g '**/*.kt' || true
echo "=== Search: SessionRoute.route usages ==="
rg -n -C2 -S 'SessionRoute\.route' -g '**/*.kt' || true
echo "=== Search: YappNavHost context ==="
rg -n -C3 -S 'YappNavHost' -g '**/*.kt' || trueLength of output: 7769
startDestination(SessionRoute) 고정으로 인증/온보딩 분기 우회 — 수정 필요
SessionRoute는 @serializable data object로 typed NavHost에서 객체를 바로 사용하는 것이 확인되어(.route 변환 불필요) 타입 문제는 없습니다. 그러나 YappNavHost에서 startDestination을 SessionRoute로 하드코딩하면 로그인/온보딩 분기 로직이 무시됩니다 — MainActivity에서 로그인 시 navigator.startDestination = HomeRoute로 설정하는 코드와 충돌합니다. 수정 필요 위치 및 권장 조치:
-
app/src/main/java/com/yapp/app/official/navigation/YappNavHost.kt — line 32: startDestination = SessionRoute
→ navigator.startDestination 또는 로그인/온보딩 상태를 반영하도록 동적으로 설정(예: startDestination = navigator.startDestination ?: SessionRoute). -
feature/session/src/main/java/com/yapp/feature/session/navigation/SessionNavigation.kt — line 11: data object SessionRoute (확인됨)
→ 객체를 startDestination으로 쓰는 것은 정상(.route 변환 불필요). -
app/src/main/java/com/yapp/app/official/MainActivity.kt — line 76: navigator.startDestination = HomeRoute
→ YappNavHost가 이 값을 사용하도록 연결하거나, NavHost 호출 전에 올바른 startDestination을 결정하도록 로직 이동.
🤖 Prompt for AI Agents
In app/src/main/java/com/yapp/app/official/navigation/YappNavHost.kt around line
32, the startDestination is hardcoded to SessionRoute which overrides the
runtime login/onboarding decision (and conflicts with MainActivity setting
navigator.startDestination = HomeRoute); update YappNavHost to use the
navigator-provided start destination when available by setting startDestination
= navigator.startDestination ?: SessionRoute (or otherwise read
navigator.startDestination before building the NavHost) so the NavHost respects
the dynamic startDestination set by MainActivity or authentication flow.
| navigateToLogin = { navigator.navigateLoginScreen(clearBackStackNavOptions) }, | ||
| handleException = handleException, | ||
| ) | ||
| sessionNavGraph() |
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.
sessionNavGraph() 파라미터 누락 가능성
다른 NavGraph들과 달리 예외 핸들링/네비게이션 콜백을 전달하지 않습니다. 세션 그래프 내부에서 예외가 발생하거나 다른 화면으로 이동해야 할 때 현재 NavigatorState와 일관성이 깨질 수 있습니다. 필요한 람다/핸들러가 있다면 연결해 주세요.
🤖 Prompt for AI Agents
app/src/main/java/com/yapp/app/official/navigation/YappNavHost.kt around line
123: sessionNavGraph() is called without the same exception-handling and
navigation callback lambdas passed to the other nav graphs, which can break
NavigatorState consistency; update the call to pass the existing handlers (e.g.,
onError, onNavigateToScreen, navigatorState or the same lambda references used
by other graphs) so the session graph can surface errors and perform navigation
via the shared callbacks, and if sessionNavGraph signature lacks these
parameters, modify its signature to accept them and forward them into its
internal destinations.
...rc/main/java/com/yapp/core/designsystem/component/button/icons/OutlinedIconButtonDefaults.kt
Show resolved
Hide resolved
...em/src/main/java/com/yapp/core/designsystem/component/button/icons/YappOutlinedIconButton.kt
Show resolved
Hide resolved
feature/session/src/main/java/com/yapp/feature/session/SessionScreen.kt
Outdated
Show resolved
Hide resolved
| resourceId = R.drawable.icon_copy, | ||
| contentDescription = "주소 복사하기", | ||
| onClick = {}, | ||
| ) |
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.
주소 복사 기능 구현 필요
복사 버튼의 onClick 이벤트가 빈 람다로 되어 있어 실제 동작하지 않습니다. 클립보드에 주소를 복사하는 기능을 구현해야 합니다.
+ val clipboardManager = context.getSystemService(Context.CLIPBOARD_SERVICE) as android.content.ClipboardManager
+
YappOutlinedIconButtonSmall(
resourceId = R.drawable.icon_copy,
contentDescription = "주소 복사하기",
- onClick = {},
+ onClick = {
+ val clip = android.content.ClipData.newPlainText("address", "서울 마포구 백범로31길 21 서울창업허브 서울복지타운")
+ clipboardManager.setPrimaryClip(clip)
+ // 토스트 메시지 표시 (선택사항)
+ },
)주소 복사 기능의 전체 구현체를 생성해드릴까요?
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In feature/session/src/main/java/com/yapp/feature/session/SessionScreen.kt
around lines 185-188, the copy button's onClick is an empty lambda so it does
nothing; implement it to copy the current address string to the clipboard and
give user feedback. Retrieve the address from the composable state (the same
variable used to display the address), obtain the clipboard manager (e.g.,
LocalClipboardManager.current or Context.getSystemService(CLIPBOARD_SERVICE)),
set the clipboard content to that address, and show a short user feedback (Toast
or Snackbar) indicating the address was copied; add necessary imports and null
checks.
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.
궁금한점 하나를 제외하곤 다른 점은 없습니다 ~~
| .size(32.dp) | ||
| .clip(CircleShape) | ||
| .yappClickable(onClick = { | ||
| openKakaoMap(context, "KT&G상상플래닛", seoulCityHall.latitude, seoulCityHall.longitude) |
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.
Q). 인코딩 될 이름은 상상플래닛인데 위경도는 서울시청으로 된 이유를 알 수 있을까요 ?
상상플래닛의 위경도는 37.5465749, 127.0435001 로 보여지고 있습니다
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.
오 요건 그냥 더미 값으로 대충 넣었습니다! 서버에서 받아온 값으로 대체될 예정이에요!
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.
| Icon( | ||
| modifier = modifier | ||
| .size(iconSize), | ||
| painter = painterResource(resourceId), | ||
| contentDescription = contentDescription, | ||
| ) |
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.
A: YappOutlinedIconButtonBasic에 사용하는 modifier를 사용한 부분은 의도한 것인가요?
- Icon 컴포넌트에 iconTint 파라미터 추가하여 색상 적용 - modifier 중복 제거 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- sessionNavGraph에 navigateToBack, navigateToLogin, handleException 파라미터 추가 - SessionRoute composable에 동일한 파라미터 전달 - YappNavHost에서 다른 NavGraph와 동일한 패턴으로 navigation 처리 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- YappHeaderActionbar를 Box 상단에 고정 배치하여 sticky 효과 구현 - 스크롤 시 타이틀이 가려지면 헤더에 타이틀 표시 - boundsInParent()를 사용하여 타이틀 초기 위치 정확히 측정 - 헤더와 스크롤 영역 사이에 그라데이션 추가 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
- 타이틀의 하단(bottom) 위치를 기준으로 가려짐 판단 - bounds.bottom 값을 사용하여 타이틀이 완전히 스크롤되었는지 확인 - 부분적으로 가려진 상태에서는 헤더에 타이틀 미표시 🤖 Generated with Claude Code Co-Authored-By: Claude <[email protected]>
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.
수고하셨습니다!


💡 Issue
🌱 Key changes
✅ To Reviewers
local.properties 업데이트가 필요해용
https://www.notion.so/yapp-workspace/396002a5eca746e59718d7fd9d44c4d5?source=copy_link
📸 스크린샷
Summary by CodeRabbit