Open
Conversation
- iOS 플랫폼을 위한 `PlatformBackGestureContainer` 및 스와이프 제스처 로직 추가 - Android는 별도 처리 없이 콘텐츠만 노출하도록 구현 - `NavDisplay` 애니메이션을 슬라이드에서 페이드 효과로 변경 및 지속 시간 조정 - `MulKkamApp`에 뒤로가기 제스처 컨테이너 적용 및 활성화 조건 설정 (로그인 화면 제외)
github-actions
bot
changed the title
github-actions
bot
changed the title
Contributor
|
Note
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목은 제스처 네비게이션 구현이라는 핵심 변경사항을 명확히 드러내며, 실제 코드 변경(PlatformBackGestureContainer 추가, 제스처 활성화 로직, 애니메이션 개선)과 일치합니다. |
| Description check | ✅ Passed | PR 설명은 템플릿의 모든 필수 섹션을 포함하고 있으며, 관련 이슈, 작업 내용, 주요 변경사항을 체계적으로 설명하고 스크린샷도 첨부했습니다. |
| Linked Issues check | ✅ Passed | PR은 이슈 #1177의 제스처 네비게이션 구현 목표를 충족하며, iOS 제스처 인식(86줄), Android no-op 구조(12줄), 조건부 활성화 로직, 애니메이션 개선을 모두 포함했습니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 제스처 네비게이션 구현과 관련된 범위 내에 있으며, PlatformBackGestureContainer 추상화, 조건부 활성화, 애니메이션 개선은 모두 목표 달성을 위한 필수 변경입니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/1177
📝 Coding Plan
- Generate coding plan for human review comments
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 help to get the list of available commands and usage tips.
Tip
CodeRabbit can enforce grammar and style rules using `languagetool`.
Configure the reviews.tools.languagetool setting to enable/disable rules and categories. Refer to the LanguageTool Community to learn more.
Xcode 프로젝트 파일 내 `DEVELOPMENT_TEAM`의 하드코딩된 값을 `${TEAM_ID}` 변수를 사용하도록 수정하였습니다.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
mobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/NavDisplay.kt (1)
46-52: 매직 넘버 180을 상수로 추출하는 것을 고려해주세요.
transitionDuration = 180이 함수 내부에 하드코딩되어 있어요. 이 값은 다른 곳에서도 재사용되거나 변경될 가능성이 있으므로, 파일 상단에private const val로 선언하면 유지보수성이 높아집니다.제스처 네비게이션과 함께 slide → fade 전환으로 변경한 것은 적절한 선택이에요. 스와이프 제스처 중에 슬라이드 애니메이션이 겹치면 시각적으로 어색할 수 있거든요.
♻️ 상수 추출 제안
+private const val TRANSITION_DURATION_MILLIS = 180 + private fun defaultTransitionSpec(): ContentTransform { - val transitionDuration = 180 - - val enterTransition = fadeIn(animationSpec = tween(durationMillis = transitionDuration)) - val exitTransition = fadeOut(animationSpec = tween(durationMillis = transitionDuration)) + val enterTransition = fadeIn(animationSpec = tween(durationMillis = TRANSITION_DURATION_MILLIS)) + val exitTransition = fadeOut(animationSpec = tween(durationMillis = TRANSITION_DURATION_MILLIS)) return enterTransition togetherWith exitTransition }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/NavDisplay.kt` around lines 46 - 52, defaultTransitionSpec() currently hardcodes transitionDuration = 180; extract this magic number into a top-level constant (e.g. private const val TRANSITION_DURATION_MS = 180) and replace the inline value in defaultTransitionSpec() so the tween(durationMillis = TRANSITION_DURATION_MS) calls use the constant; ensure the constant is private and placed at the file top for reuse and easier maintenance.mobile/shared/src/androidMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.android.kt (1)
5-12: 의도적인 no-op 구현이라면 설명 주석을 추가해주세요.Android에서
enabled와onBack파라미터를 사용하지 않는 이유가 있을 거예요 (Android는 시스템 레벨의 뒤로가기 제스처/버튼을 사용하므로). 하지만 코드만 봐서는 이것이 의도된 것인지, 아직 구현되지 않은 것인지 알기 어려워요.두 가지 개선 방안이 있습니다:
- 설계 의도를 주석으로 명시 - 미래의 개발자가 "왜 파라미터를 무시하지?"라는 의문을 품지 않도록
@Suppress("UNUSED_PARAMETER")- Lint 경고를 명시적으로 억제하여 의도적임을 표시📝 주석 추가 제안
`@Composable` actual fun PlatformBackGestureContainer( + // Android에서는 시스템의 예측 뒤로가기(Predictive Back) 제스처를 사용하므로 + // 별도의 커스텀 제스처 처리가 필요하지 않습니다. enabled: Boolean, onBack: () -> Unit, content: `@Composable` () -> Unit, ) { content() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/shared/src/androidMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.android.kt` around lines 5 - 12, The PlatformBackGestureContainer actual implementation currently ignores its parameters (enabled, onBack) and simply invokes content(), which looks like a deliberate no-op on Android; add a brief explanatory comment above the PlatformBackGestureContainer function stating that Android uses system back gestures/buttons so the parameters are intentionally unused, and also add `@Suppress`("UNUSED_PARAMETER") on the PlatformBackGestureContainer declaration to silence lint and make the intent explicit (or alternatively remove the suppress and keep only the explanatory comment if you prefer).mobile/shared/src/iosMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.ios.kt (1)
47-80:isBackGestureTriggered변수와 마지막 if 블록이 중복되어 보여요.현재 로직 흐름을 보면:
- 제스처가 트리거되면 (Line 66-71):
isBackGestureTriggered = true,shouldFinishGesture = true설정shouldFinishGesture = true이므로 while 루프 종료- Lines 78-80에서
isBackGestureTriggered를 체크하고return@awaitEachGesture하지만
awaitEachGesture블록은 어차피 끝나면 다음 제스처를 기다리므로, 마지막return문이 동작에 영향을 주지 않아요. 이 패턴이 의도적인 것(가독성 향상이나 미래 확장성)이라면 괜찮지만, 그렇지 않다면 제거하여 코드를 간결하게 만들 수 있습니다.♻️ 불필요한 변수 및 조건문 제거 제안
var accumulatedHorizontalDragDistance = 0f var accumulatedVerticalDragDistance = 0f var shouldFinishGesture = false - var isBackGestureTriggered = false while (!shouldFinishGesture) { // ... 중간 로직 동일 ... if (isDraggingToRight && isHorizontalDominant && isDistanceEnough) { pointerEvent.changes.forEach { it.consume() } onBack() - isBackGestureTriggered = true shouldFinishGesture = true } if (!pointerChange.pressed) { shouldFinishGesture = true } } - - if (isBackGestureTriggered) { - return@awaitEachGesture - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/shared/src/iosMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.ios.kt` around lines 47 - 80, The isBackGestureTriggered flag and the final "if (isBackGestureTriggered) return@awaitEachGesture" are redundant; remove the isBackGestureTriggered declaration and all assignments, delete the final if block, and instead exit the awaitEachGesture lambda immediately when the back gesture is detected by invoking return@awaitEachGesture right after calling onBack() inside the branch that currently sets isBackGestureTriggered (use the existing onBack()/shouldFinishGesture logic location to add the early return).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mobile/shared/src/commonMain/kotlin/com/mulkkam/MulKkamApp.kt`:
- Around line 48-53: The enableBackGesture derived state currently enables
gestures on Splash because it only excludes AuthRoute.Login; update the
condition in the derivedStateOf (where navigator.currentRoute, currentTab and
enableBackGesture are used) to exclude the whole AuthRoute hierarchy — e.g.,
change the check from `currentRoute !is AuthRoute.Login && currentTab == null`
to `currentRoute !is AuthRoute && currentTab == null` (or alternatively also
require `navigator.backStack.size > 1` if you prefer guarding against initial
stack); if the original intent was to only exclude Login, add an inline comment
explaining that exception.
---
Nitpick comments:
In
`@mobile/shared/src/androidMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.android.kt`:
- Around line 5-12: The PlatformBackGestureContainer actual implementation
currently ignores its parameters (enabled, onBack) and simply invokes content(),
which looks like a deliberate no-op on Android; add a brief explanatory comment
above the PlatformBackGestureContainer function stating that Android uses system
back gestures/buttons so the parameters are intentionally unused, and also add
`@Suppress`("UNUSED_PARAMETER") on the PlatformBackGestureContainer declaration to
silence lint and make the intent explicit (or alternatively remove the suppress
and keep only the explanatory comment if you prefer).
In `@mobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/NavDisplay.kt`:
- Around line 46-52: defaultTransitionSpec() currently hardcodes
transitionDuration = 180; extract this magic number into a top-level constant
(e.g. private const val TRANSITION_DURATION_MS = 180) and replace the inline
value in defaultTransitionSpec() so the tween(durationMillis =
TRANSITION_DURATION_MS) calls use the constant; ensure the constant is private
and placed at the file top for reuse and easier maintenance.
In
`@mobile/shared/src/iosMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.ios.kt`:
- Around line 47-80: The isBackGestureTriggered flag and the final "if
(isBackGestureTriggered) return@awaitEachGesture" are redundant; remove the
isBackGestureTriggered declaration and all assignments, delete the final if
block, and instead exit the awaitEachGesture lambda immediately when the back
gesture is detected by invoking return@awaitEachGesture right after calling
onBack() inside the branch that currently sets isBackGestureTriggered (use the
existing onBack()/shouldFinishGesture logic location to add the early return).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d269d992-35ec-4c1e-8b02-9410ebb17668
📒 Files selected for processing (5)
mobile/shared/src/androidMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.android.ktmobile/shared/src/commonMain/kotlin/com/mulkkam/MulKkamApp.ktmobile/shared/src/commonMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.ktmobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/NavDisplay.ktmobile/shared/src/iosMain/kotlin/com/mulkkam/ui/main/component/PlatformBackGestureContainer.ios.kt
hwannow
approved these changes
Mar 6, 2026
Contributor
hwannow
left a comment
hwannow
left a comment
There was a problem hiding this comment.
수고하셨습니다 공백 🥔
어푸어푸 🌊🏊♀️🌊
Comment on lines
+20
to
+24
| actual fun PlatformBackGestureContainer( | ||
| enabled: Boolean, | ||
| onBack: () -> Unit, | ||
| content: @Composable () -> Unit, | ||
| ) { |
Contributor
There was a problem hiding this comment.
p3) 혹시 여기에서 previousContent 같이 이전 화면을 받아서...
Swipe 애니메이션을 보여 주는 건 어려울까요?
( 옆으로 화면이 밀려나면서, 이전 화면이 드러나게 )
구현 난이도가 높다면 넘어가도 좋습니당
Contributor
Author
There was a problem hiding this comment.
환노도 저와 같은 말씀을 하셨었네요?! 예리하시군요 👀👀
추가 완료했습니다 :) 기회가 되면 영상도 첨부하도록 하겠습니다!
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/NavDisplay.kt`:
- Around line 29-34: The current isPopTransition in NavDisplay incorrectly
infers "back" vs "reset" by comparing backStack.size; instead, add an explicit
NavigationType enum and a lastNavigationType field to MainNavigator and set it
inside MainNavigator.navigate, MainNavigator.popBackStack, and
MainNavigator.navigateAndClearBackStack (or whatever methods implement
clear/reset) to PUSH, POP, REPLACE respectively; then change NavDisplay to
derive isPopTransition from navigator.lastNavigationType == NavigationType.POP
instead of comparing sizes so tab switches and clears use the correct animation
direction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e8e7a4fa-c7f7-4e71-bc7d-1632990398a2
📒 Files selected for processing (1)
mobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/NavDisplay.kt
Comment on lines
+29
to
+34
| val previousBackStackSizeState = remember { mutableIntStateOf(backStack.size) } | ||
| val isPopTransition = backStack.size < previousBackStackSizeState.intValue | ||
|
|
||
| LaunchedEffect(backStack.size) { | ||
| previousBackStackSizeState.intValue = backStack.size | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check where navigateAndClearBackStack is called to understand impact
echo "=== navigateAndClearBackStack 사용처 확인 ==="
rg -n "navigateAndClearBackStack" --type=kotlin -C 3
echo ""
echo "=== NavDisplay 호출부 확인 ==="
rg -n "NavDisplay\(" --type=kotlin -C 3Repository: woowacourse-teams/2025-mul-kkam
Length of output: 6746
🏁 Script executed:
cat -n mobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/MainNavigator.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 3995
🏁 Script executed:
cat -n mobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/NavDisplay.ktRepository: woowacourse-teams/2025-mul-kkam
Length of output: 4187
백스택 크기 비교만으로는 "뒤로가기"와 "화면 초기화"를 구분할 수 없습니다
현재 isPopTransition 로직이 크기 감소에만 의존하면서, 의미 있는 네비게이션 구분이 불가능합니다. 실제로 코드를 보면 이 문제가 매우 구체적으로 나타나는데요:
문제의 구체적인 영향:
-
하단 탭 네비게이션 (
navigateToHome(),navigateToHistory(),navigateToFriends(),navigateToSetting())- 예: 깊은 중첩 화면(예: Home → Encyclopedia → Detail, 스택 깊이 4)에서 다른 탭(History)으로 이동
- 백스택이 4 → 1로 줄어들어
isPopTransition = true판정 - 하지만 이는 탭 전환이므로 앞으로 가는 애니메이션이 적절함
- 현재는 뒤로 가는 애니메이션(왼쪽에서 슬라이드인)이 재생되어 사용자 기대와 맞지 않음
-
인증 상태 전환 (
navigateToLoginAndClearBackStack(),navigateToSplash())- 로그아웃 후 로그인 화면으로: 백스택 초기화 → 틀린 애니메이션 방향
-
온보딩 플로우 (
navigateToOnboardingTerms())- 동의 화면으로 리셋할 때도 동일한 문제
근본 원인: navigateAndClearBackStack()과 popBackStack()이 모두 백스택 크기를 감소시키므로, 크기만으로는 두 경우를 구분할 수 없습니다.
추천되는 해결 방식:
Option A (권장): MainNavigator에서 네비게이션 의도를 명시적으로 추적
- 장점: 의도가 명확하고 향후 다른 네비게이션 패턴(예: 모달 표시) 추가가 용이
- 단점:
MainNavigator인터페이스 변경 필요
enum class NavigationType { PUSH, POP, REPLACE }
`@Stable`
class MainNavigator internal constructor(
val backStack: SnapshotStateList<Any>,
) {
var lastNavigationType: NavigationType = NavigationType.PUSH
private set
fun popBackStack() {
if (backStack.size > 1) {
lastNavigationType = NavigationType.POP
backStack.removeAt(backStack.lastIndex)
}
}
fun navigate(route: Any) {
lastNavigationType = NavigationType.PUSH
backStack.add(route)
}
fun navigateAndClearBackStack(route: Any) {
lastNavigationType = NavigationType.REPLACE
backStack.clear()
backStack.add(route)
}
}그 후 NavDisplay에서:
val isPopTransition = navigationType == NavigationType.POPOption B: 라우트 정체성으로 판단
- 장점: 구현이 간단함
- 단점: 같은 라우트 타입으로의 전환이나 엣지 케이스를 완벽히 커버하기 어려움
의도를 명시적으로 드러내는 방식이 디버깅과 유지보수 관점에서 더 건강한 패턴입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/shared/src/commonMain/kotlin/com/mulkkam/ui/navigation/NavDisplay.kt`
around lines 29 - 34, The current isPopTransition in NavDisplay incorrectly
infers "back" vs "reset" by comparing backStack.size; instead, add an explicit
NavigationType enum and a lastNavigationType field to MainNavigator and set it
inside MainNavigator.navigate, MainNavigator.popBackStack, and
MainNavigator.navigateAndClearBackStack (or whatever methods implement
clear/reset) to PUSH, POP, REPLACE respectively; then change NavDisplay to
derive isPopTransition from navigator.lastNavigationType == NavigationType.POP
instead of comparing sizes so tab switches and clears use the correct animation
direction.
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.
🔗 관련 이슈
📝 작업 내용
사용자가 앱 내에서 물리적 제스처를 통해 직관적으로 이전 화면으로 돌아갈 수 있도록 하기 위해 플랫폼별 제스처 네비게이션을 구현했습니다. iOS의 네이티브 스와이프-백 동작을 지원하면서, Android에서는 기존 네비게이션 구조를 유지하는 멀티플랫폼 솔루션을 제공합니다.
주요 변경사항
플랫폼별 제스처 핸들러 추상화 및 구현
expect/actual패턴을 이용해PlatformBackGestureContainer컴포저블을 정의조건부 제스처 활성화 로직
MulKkamApp.kt에서enableBackGesture파생 상태를 도입AuthRoute.Login)을 제외한 모든 화면과 탭 선택 전 상태에서만 활성화내비게이션 애니메이션 변경
NavDisplay.kt의 화면 전환 애니메이션을 슬라이드(수평)에서 페이드로 변경구조적 계층 재설정
PlatformBackGestureContainer를Scaffold상위에 배치하여 전역 제스처 인식을 가능하게 함📊 변경 효과 요약
📸 스크린샷 (Optional)
ScreenRecording_03-04-2026.11-20-35_1.mov