[refactor] #51 네컷지도 기능 및 UI 고도화#53
Merged
Merged
Conversation
Walkthrough지도 화면을 DragLevel 기반으로 재구성하고 위치 권한 요청/다이얼로그/앱 설정 흐름과 현위치 탐색 칩(MapRefreshChip)을 추가. 토큰 전용 DataStore/리포지토리(TokenRepository/TokenDataStore)를 분리하고 기존 DataStoreRepository는 boolean API로 축소. Brand 명칭을 PhotoBooth로 리네임 및 UI 컴포넌트 보완. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MapScreen
participant MapViewModel
participant DataStore
participant LocationPermissionManager
participant System as "시스템(권한/설정)"
User->>MapScreen: 현 위치 기반 액션 요청 (탐색/길찾기)
MapScreen->>LocationPermissionManager: 권한 보유 여부 확인
alt 권한 없음
LocationPermissionManager-->>MapScreen: false
MapScreen->>MapViewModel: RequestLocationPermission 인텐트
MapViewModel->>DataStore: getBoolean(DataStoreKey.IS_FIRST_LOCATION_PERMISSION)
DataStore-->>MapViewModel: Boolean(초회 여부)
alt 초회 요청
MapViewModel-->>MapScreen: ShowLocationPermissionDialog 효과
MapScreen->>User: 권한 설명 다이얼로그 표시
User->>MapScreen: 동의/거부 선택
MapScreen->>MapViewModel: ConfirmLocationPermissionDialog / DismissLocationPermissionDialog
else 재요청(이미 거부)
MapViewModel-->>MapScreen: RequestLocationPermission 효과
end
MapScreen->>System: 권한 런처 실행
System-->>MapScreen: 권한 결과 콜백
MapScreen->>MapViewModel: 권한 결과 전달
MapViewModel->>DataStore: setBoolean(DataStoreKey.IS_FIRST_LOCATION_PERMISSION, false)
else 권한 있음
LocationPermissionManager-->>MapScreen: true
MapScreen->>MapViewModel: UpdateCurrentLocation or ClickDirection
end
MapViewModel-->>MapScreen: UI 업데이트 / MoveCamera / Navigate 효과
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@core/common/src/main/java/com/neki/android/core/common/permission/PermissionManager.kt`:
- Around line 33-37: The navigateToAppSettings function can crash when called
with a non-Activity Context because startActivity requires
FLAG_ACTIVITY_NEW_TASK outside an Activity; modify the Intent created in
navigateToAppSettings to include Intent.FLAG_ACTIVITY_NEW_TASK (or conditionally
add the flag when context is not an Activity) before calling
context.startActivity(intent) so the settings screen launches safely from any
Context.
In
`@core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt`:
- Around line 48-50: The clearTokens() implementation currently calls
dataStore.edit { it.clear() } which wipes all stored prefs (including
IS_FIRST_LOCATION_PERMISSION); change it to only remove the auth token entries
instead. Inside DataStoreRepositoryImpl.clearTokens(), call dataStore.edit {
prefs -> prefs.remove(ACCESS_TOKEN_KEY); prefs.remove(REFRESH_TOKEN_KEY) } (use
the actual token preference keys/constants defined in your class or file) and do
not call prefs.clear(), so flags like IS_FIRST_LOCATION_PERMISSION remain
intact.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/AnchoredDraggablePanel.kt`:
- Around line 92-111: The AnchoredDraggableState and its anchors are captured in
a keyless remember block so they won't update when screenHeightPx or panel
heights change (e.g., on rotation); change the remember to use explicit keys so
the state and anchors are recomputed when inputs change (e.g.,
remember(screenHeightPx, bottomPanelHeightPx, centerPanelHeightPx, density) {
... }). Recompute the DraggableAnchors and AnchoredDraggableState inside that
keyed remember so values used by DragLevel.FIRST/SECOND/THIRD/INVISIBLE and
positional/velocity/decay calculations are refreshed when configuration or
density changes.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`:
- Around line 61-62: The code force-unwraps LocalActivity.current (val activity
= LocalActivity.current!!) which crashes in previews or non-Activity contexts;
change to a nullable activity (val activity = LocalActivity.current) and update
all uses (e.g., permission rationale checks and functions referenced around
MapScreen, calls at the spots previously at lines ~174-175, 229-233, 263-268,
272-276) to null-safe operations: guard early (if (activity == null) { /* no-op
preview-safe path or fallback UI */ }) or use activity?.let { ... } and compute
rationale/permission logic only when activity is non-null, providing fallback
behavior for preview rendering to prevent NPEs.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapViewModel.kt`:
- Around line 99-116: handleClickDirectionItem uses state.currentLocation
directly and can send (0.0,0.0) as the start point; add a validity check before
posting MapEffect.MoveDirectionApp: after reduce {
copy(isShowDirectionBottomSheet = false) } verify state.currentLocation is a
valid lat/lng (e.g., not Pair(0.0, 0.0) or use an isValidLocation helper) and
only post MoveDirectionApp when valid, otherwise post a side effect to request a
location update or show a location-required message (e.g.,
postSideEffect(MapEffect.RequestLocationUpdate) or
MapEffect.ShowLocationRequired) and return early; keep existing symbols
(handleClickDirectionItem, MapState.currentLocation, MapEffect.MoveDirectionApp,
intent.app, state.selectedBrandInfo) to locate the change.
🧹 Nitpick comments (4)
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/DataStoreRepository.kt (1)
16-17:Preferences.Key를 인터페이스에 노출하면 추상화가 약해집니다.
Preferences.Key<Boolean>을 직접 파라미터로 받으면 소비자 모듈이 DataStore 구현 세부사항에 의존하게 됩니다. 향후 저장소 구현을 교체하기 어려워질 수 있습니다.대안으로 문자열 키나 enum을 사용하고, 키 생성은 내부에서 처리하는 방식을 고려해보세요:
♻️ 추상화 개선 제안
- suspend fun setBoolean(key: Preferences.Key<Boolean>, value: Boolean) - fun getBoolean(key: Preferences.Key<Boolean>): Flow<Boolean> + suspend fun setBoolean(key: String, value: Boolean) + fun getBoolean(key: String): Flow<Boolean>또는 미리 정의된 키를 사용하는 전용 메서드:
suspend fun setFirstLocationPermissionShown(value: Boolean) fun isFirstLocationPermissionShown(): Flow<Boolean>feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/MapRefreshChip.kt (2)
35-38:background()의shape파라미터가 불필요합니다.Line 33에서 이미
clip(CircleShape)을 적용했으므로,background()에서 다시 shape을 지정할 필요가 없습니다.♻️ 중복 제거
.background( - shape = CircleShape, color = NekiTheme.colorScheme.white, )
54-58: 하드코딩된 문자열을 리소스로 분리하는 것을 고려해보세요."현 위치에서 탐색" 문자열이 코드에 직접 작성되어 있습니다. 다국어 지원이나 문자열 관리를 위해
strings.xml리소스로 분리하는 것이 좋습니다.feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt (1)
48-55: 빈 토큰에 대한 명시적 검증을 고려해보세요.
getRefreshToken()이 토큰이 없을 때 빈 문자열("")을 반환합니다. 현재 코드는 빈 토큰을updateAccessToken()에 전달하고 API 실패에 의존하여 에러를 처리합니다.명시적인 검증을 추가하면 의도가 더 명확해지고 불필요한 네트워크 호출을 방지할 수 있습니다:
♻️ 명시적 검증 추가 제안
authRepository.updateAccessToken( - refreshToken = dataStoreRepository.getRefreshToken().first(), - ).onSuccess { + refreshToken = dataStoreRepository.getRefreshToken().first() + .takeIf { it.isNotBlank() } + ?: run { + authEventManager.emitTokenExpired() + return@launch + }, + ).onSuccess { postSideEffect(LoginSideEffect.NavigateToHome)
ikseong00
reviewed
Jan 24, 2026
Contributor
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
고생하셨습니다!
가까운 네컷 사진 브랜드 info icon 팝업 쪽은 아직 디자인이 나오지 않아 다이얼로그로 뜨는 거죠??
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@core/common/src/main/java/com/neki/android/core/common/permission/PermissionUtils.kt`:
- Around line 8-12: The navigateToAppSettings(Context) function can be called
with non-Activity contexts (Application, Service) and will crash if
startActivity is invoked without FLAG_ACTIVITY_NEW_TASK; update the function
(navigateToAppSettings) to set Intent.FLAG_ACTIVITY_NEW_TASK on the constructed
Intent before calling context.startActivity(intent) so the settings Activity can
be launched safely from any Context.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/component/CloseButton.kt`:
- Around line 26-41: The close button's clickable area and accessibility are
insufficient: the Box/Modifier chain in CloseButton.kt (modifier,
.buttonShadow(), .background(..., CircleShape), .clickable(onClick = onClick),
.padding(8.dp)) yields a touch target under 48dp and the Icon has
contentDescription = null; fix by adding a minimum touch target with
Modifier.sizeIn(minWidth = 48.dp, minHeight = 48.dp) in the Box modifier chain
(before padding or clickable as appropriate), and add semantics to expose an
accessibility label (e.g., .semantics { contentDescription = "Close" } or use
role = Role.Button if available) so the Icon can remain decorative while the Box
provides the screen-reader label; keep CircleShape and visual styling intact.
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`:
- Around line 86-88: MapEffect.RefreshPhotoBooth in MapScreen.kt is left as a
TODO so the "현 위치에서 탐색" button does nothing; implement the refresh flow by
resolving the current location and triggering the photo-booth reload: inside the
MapEffect.RefreshPhotoBooth branch call the existing location
provider/getCurrentLocation helper (or viewModel.getCurrentLocation()) and then
invoke the viewModel method that queries nearby booths (e.g.,
viewModel.refreshPhotoBooths(currentLocation) or dispatch
MapIntent.LoadNearbyPhotoBooths with the location). If those helpers don't exist
yet, either implement them or change the effect handling to disable/hide the "현
위치에서 탐색" action until the refresh flow (getCurrentLocation + refreshPhotoBooths)
is implemented.
♻️ Duplicate comments (1)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (1)
60-60:LocalActivity.current!!는 프리뷰/비-Activity 컨텍스트에서 크래시 위험nullable 처리로 NPE를 피하고, rationale 계산/권한 요청은 null-safe하게 처리하는 게 안전합니다.
🐛 수정 제안
- val activity = LocalActivity.current!! + val activity = LocalActivity.current + val shouldShowRationale = + activity?.let(LocationPermissionManager::shouldShowLocationRationale) ?: false ... - viewModel.store.onIntent(MapIntent.RequestLocationPermission(LocationPermissionManager.shouldShowLocationRationale(activity))) + viewModel.store.onIntent(MapIntent.RequestLocationPermission(shouldShowRationale))- val activity = LocalActivity.current!! + val activity = LocalActivity.current + val shouldShowRationale = + activity?.let(LocationPermissionManager::shouldShowLocationRationale) ?: false ... - onIntent(MapIntent.RequestLocationPermission(LocationPermissionManager.shouldShowLocationRationale(activity))) + onIntent(MapIntent.RequestLocationPermission(shouldShowRationale)) ... - onIntent(MapIntent.RequestLocationPermission(LocationPermissionManager.shouldShowLocationRationale(activity))) + onIntent(MapIntent.RequestLocationPermission(shouldShowRationale)) ... - onIntent(MapIntent.RequestLocationPermission(LocationPermissionManager.shouldShowLocationRationale(activity))) + onIntent(MapIntent.RequestLocationPermission(shouldShowRationale))Also applies to: 173-175, 229-233, 262-268, 270-276
Contributor
|
고생하셨습니다! |
ikseong00
approved these changes
Jan 25, 2026
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 77-84: 현재 영구 거부 감지 조건 `!currentShouldShowRationale &&
!previousShouldShowRationale`가 이전 상태에 따라 영구 거부를 놓치므로, 거부 시 현재 상태만 검사하도록 변경하세요:
대신 `if (!currentShouldShowRationale) {
viewModel.store.onIntent(MapIntent.ShowLocationPermissionDialog) }`로 바꿔
`currentShouldShowRationale`가 false인 경우(영구 거부 포함)에 항상 다이얼로그를 표시하도록 수정하고, 관련 식별자
locationTrackingMode, LocationTrackingMode.Follow, currentShouldShowRationale,
previousShouldShowRationale,
viewModel.store.onIntent(MapIntent.ShowLocationPermissionDialog)를 사용해 코드를 찾아
적용하세요.
♻️ Duplicate comments (2)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (2)
61-61: LocalActivity의 강제 언래핑은 Preview에서 크래시 유발
LocalActivity.current!!사용 시 Preview 또는 Activity 없는 컨텍스트에서NullPointerException이 발생합니다. nullable로 처리해 주세요.
96-98: '현 위치에서 탐색' 기능 미구현 상태
MapEffect.RefreshPhotoBooth가 TODO로 비어 있어 버튼 클릭 시 실제 동작이 없습니다. 구현 전이라면 버튼 비활성화 또는 숨김 처리를 고려해 주세요.
🧹 Nitpick comments (1)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (1)
52-52: 사용되지 않는 Timber import
Timber가 import되어 있지만 파일 내에서 사용되지 않습니다. 제거해 주세요.♻️ 수정 제안
-import timber.log.Timber
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 143-150: The call to getPlaceName from MapScreen (used to build
NAVER map URL) can block the main thread on Android <13 because getPlaceName
performs a synchronous geocoder.getFromLocation; move that blocking work to
Dispatchers.IO by making getPlaceName a suspend function that wraps the
synchronous geocoding in withContext(Dispatchers.IO) (or, if you prefer not to
change its signature, call it inside withContext(Dispatchers.IO) where MapScreen
invokes it). Update the MapScreen invocation (the code that computes
startName/destName before DirectionHelper.moveAppOrStore) to run inside a
coroutine and obtain names via withContext(Dispatchers.IO) to avoid main-thread
blocking. Ensure the rest of the call to DirectionHelper.moveAppOrStore remains
on the main thread after names are resolved.
♻️ Duplicate comments (3)
feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt (3)
60-61: LocalActivity의 강제 언래핑 이슈가 아직 해결되지 않았습니다.
LocalActivity.current!!사용은 프리뷰 또는 비Activity 컨텍스트에서NullPointerException을 유발합니다. 이전 리뷰에서 제안된 대로 nullable 처리가 필요합니다.
76-84: 영구 거부 감지 로직이 여전히 일부 시나리오를 놓칠 수 있습니다.이전 리뷰에서 지적된 대로, 첫 번째 거부 후(prev=false → current=true) 두 번째 요청에서 영구 거부(prev=true → current=false)하는 케이스를 현재 조건이 감지하지 못합니다.
!currentShouldShowRationale만 확인하는 방식으로 변경 검토가 필요합니다.
95-97: '현 위치에서 탐색' 동작이 여전히 구현되지 않았습니다.
MapEffect.RefreshPhotoBooth가 TODO로 비어 있어 버튼 클릭 시 실제 동작이 없습니다. 이전 리뷰에서 언급된 대로 구현이 필요합니다.
Comment on lines
143
to
150
| DirectionApp.NAVER_MAP -> { | ||
| val startName = context.getPlaceName(37.5270539, 126.8862648, "출발지") | ||
| val destName = context.getPlaceName(37.5256372, 126.8861924, "도착지") | ||
| val startName = context.getPlaceName(startLatitude, startLongitude, "출발지") | ||
| val destName = context.getPlaceName(endLatitude, endLongitude, "도착지") | ||
| DirectionHelper.moveAppOrStore( | ||
| context = context, | ||
| url = "nmap://route/walk?slat=37.5270539&slng=126.8862648&sname=$startName&dlat=37.5256372&dlng=126.8861924&dname=$destName", | ||
| url = "nmap://route/walk?slat=$startLatitude&slng=$startLongitude&sname=$startName&dlat=$endLatitude&dlng=$endLongitude&dname=$destName", | ||
| packageName = sideEffect.app.packageName, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find getPlaceName function definition
ast-grep --pattern 'fun Context.getPlaceName($$$) {
$$$
}'Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 61
🏁 Script executed:
# Search for getPlaceName function definition using rg
rg "fun.*getPlaceName" -A 15Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 1954
🏁 Script executed:
# Check the context of getPlaceName calls in MapScreen.kt
cat -n feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt | sed -n '120,160p'Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 2393
🏁 Script executed:
# Check broader context to see the function/lambda scope
cat -n feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt | sed -n '80,145p'Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 3385
🏁 Script executed:
# Verify the complete Extension.kt implementation to confirm async pattern
cat -n feature/map/impl/src/main/java/com/neki/android/feature/map/impl/util/Extension.kt | sed -n '1,50p'Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 1558
Android 13(TIRAMISU) 이전 버전에서 메인 스레드 블로킹 문제 발생
getPlaceName 구현에서 Android 13 미만 기기에서는 geocoder.getFromLocation() 동기 호출(Extension.kt 28줄)이 메인 스레드를 차단합니다. 역지오코딩 네트워크 I/O가 완료될 때까지 UI가 멈출 수 있으므로, 해당 호출을 Dispatchers.IO로 옮겨야 합니다.
Android 13 이상에서는 콜백 기반 비동기 패턴이 올바르게 사용되고 있습니다.
🤖 Prompt for AI Agents
In
`@feature/map/impl/src/main/java/com/neki/android/feature/map/impl/MapScreen.kt`
around lines 143 - 150, The call to getPlaceName from MapScreen (used to build
NAVER map URL) can block the main thread on Android <13 because getPlaceName
performs a synchronous geocoder.getFromLocation; move that blocking work to
Dispatchers.IO by making getPlaceName a suspend function that wraps the
synchronous geocoding in withContext(Dispatchers.IO) (or, if you prefer not to
change its signature, call it inside withContext(Dispatchers.IO) where MapScreen
invokes it). Update the MapScreen invocation (the code that computes
startName/destName before DirectionHelper.moveAppOrStore) to run inside a
coroutine and obtain names via withContext(Dispatchers.IO) to avoid main-thread
blocking. Ensure the rest of the call to DirectionHelper.moveAppOrStore remains
on the main thread after names are resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
📙 작업 설명
📸 스크린샷 또는 시연 영상 (선택)
KakaoTalk_Video_2026-01-25-02-17-59.mp4
KakaoTalk_Video_2026-01-25-02-23-21.mp4
KakaoTalk_Video_2026-01-25-02-20-02.mp4
KakaoTalk_Video_2026-01-25-02-23-41.mp4
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
새로운 기능
버그 수정
스타일
리팩터
✏️ Tip: You can customize this high-level summary in your review settings.