fix: Dialog 혹은 BottomSheet를 통해 navigation 하는 경우 해당 컴포넌트가 닫히기 전 화면이 먼저 전환되는 문제 임시 해결#94
Merged
seoyoon513 merged 1 commit intodevelopfrom Aug 1, 2025
Merged
fix: Dialog 혹은 BottomSheet를 통해 navigation 하는 경우 해당 컴포넌트가 닫히기 전 화면이 먼저 전환되는 문제 임시 해결#94seoyoon513 merged 1 commit intodevelopfrom
seoyoon513 merged 1 commit intodevelopfrom
Conversation
WalkthroughDialog나 BottomSheet를 통한 네비게이션 시 화면 전환이 먼저 발생하는 문제를 임시로 해결하기 위해, 네비게이션 함수에 지연(delay)을 주는 확장 함수( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Presenter
participant Navigator
User->>Presenter: Dialog/BottomSheet 이벤트 발생
Presenter->>Presenter: scope.launch { navigator.delayedPop() / delayedGoTo() }
Note right of Presenter: (Coroutine 내에서 delay)
Presenter->>Navigator: delayedPop() 또는 delayedGoTo()
Navigator->>Navigator: delay (200ms)
Navigator->>Navigator: pop() 또는 goTo()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 변경사항 중 범위를 벗어나는 코드 변경은 발견되지 않았습니다.) Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt(3 hunks)feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions.kt(1 hunks)feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegatingNavigator.kt:0-0
Timestamp: 2025-07-16T15:54:19.322Z
Learning: In the Reed-Android project using Circuit architecture, DelegatingNavigator was restored because NavigableCircuitContent can only inject a single navigator, but the dual-navigator architecture (childNavigator for bottom navigation screens, rootNavigator for full-screen screens) requires a delegating component to work within Circuit's constraint.
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegateNavigator.kt:0-0
Timestamp: 2025-07-16T16:11:27.044Z
Learning: In the Reed-Android project's DelegateNavigator implementation, the pop(), peek(), and peekBackStack() methods should always use childNavigator without branching logic, as they operate on the currently active navigation stack. Only goTo() and resetRoot() methods need to route between childNavigator and rootNavigator based on screen type using isMainTabScreen() function.
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegateNavigator.kt:0-0
Timestamp: 2025-07-16T16:11:27.044Z
Learning: In the Reed-Android project's DelegateNavigator implementation, the pop(), peek(), and peekBackStack() methods should always use childNavigator without branching logic, as they operate on the currently active navigation stack. Only goTo() and resetRoot() methods need to route between childNavigator and rootNavigator based on screen type.
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#46
File: feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/component/InfiniteLazyColumn.kt:83-95
Timestamp: 2025-07-14T00:46:03.843Z
Learning: seoyoon513과 팀은 한국어 주석을 선호하며, 한국어 주석을 영어로 번역하라는 제안을 하지 않아야 함
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#75
File: feature/home/src/main/kotlin/com/ninecraft/booket/feature/home/HomeUi.kt:0-0
Timestamp: 2025-07-29T06:07:11.727Z
Learning: seoyoon513 팀에서는 UI 구현 단계에서 더미 데이터를 하드코딩하여 화면을 먼저 구현하고, 이후 서버 연동 시점에 실제 데이터로 교체하는 개발 방식을 사용합니다.
📚 Learning: in the reed-android project's delegatenavigator implementation, the pop(), peek(), and peekbackstack...
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegateNavigator.kt:0-0
Timestamp: 2025-07-16T16:11:27.044Z
Learning: In the Reed-Android project's DelegateNavigator implementation, the pop(), peek(), and peekBackStack() methods should always use childNavigator without branching logic, as they operate on the currently active navigation stack. Only goTo() and resetRoot() methods need to route between childNavigator and rootNavigator based on screen type.
Applied to files:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.ktfeature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions.kt
📚 Learning: in the reed-android project's delegatenavigator implementation, the pop(), peek(), and peekbackstack...
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegateNavigator.kt:0-0
Timestamp: 2025-07-16T16:11:27.044Z
Learning: In the Reed-Android project's DelegateNavigator implementation, the pop(), peek(), and peekBackStack() methods should always use childNavigator without branching logic, as they operate on the currently active navigation stack. Only goTo() and resetRoot() methods need to route between childNavigator and rootNavigator based on screen type using isMainTabScreen() function.
Applied to files:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.ktfeature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions.kt
📚 Learning: in circuit architecture, presenters receive the screen object directly as a constructor parameter (e...
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#88
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt:18-47
Timestamp: 2025-07-31T23:30:37.547Z
Learning: In Circuit architecture, presenters receive the Screen object directly as a constructor parameter (e.g., @Assisted private val screen: RecordDetailScreen), and screen parameters are accessed through this screen object (e.g., screen.recordId). Screen parameters should not be added as separate constructor parameters.
Applied to files:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.ktfeature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
📚 Learning: in the reed-android project using circuit architecture, delegatingnavigator was restored because nav...
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#52
File: feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/screens/DelegatingNavigator.kt:0-0
Timestamp: 2025-07-16T15:54:19.322Z
Learning: In the Reed-Android project using Circuit architecture, DelegatingNavigator was restored because NavigableCircuitContent can only inject a single navigator, but the dual-navigator architecture (childNavigator for bottom navigation screens, rootNavigator for full-screen screens) requires a delegating component to work within Circuit's constraint.
Applied to files:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.ktfeature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.ktfeature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions.kt
📚 Learning: in circuit architecture, screen parameters are not manually passed to presenter constructors. the @c...
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#88
File: feature/detail/src/main/kotlin/com/ninecraft/booket/feature/detail/record/RecordDetailPresenter.kt:18-47
Timestamp: 2025-07-31T23:30:37.547Z
Learning: In Circuit architecture, screen parameters are not manually passed to presenter constructors. The @CircuitInject annotation automatically handles the injection of the entire Screen object, making screen parameters accessible without explicitly adding them to the constructor or factory methods.
Applied to files:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.ktfeature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
📚 Learning: reed-android 프로젝트에서는 `booket.android.feature` convention plugin을 사용하여 feature 모듈들의 공통 의존성을 관리한다. 이 p...
Learnt from: easyhooon
PR: YAPP-Github/Reed-Android#61
File: feature/webview/build.gradle.kts:17-21
Timestamp: 2025-07-20T12:34:23.786Z
Learning: Reed-Android 프로젝트에서는 `booket.android.feature` convention plugin을 사용하여 feature 모듈들의 공통 의존성을 관리한다. 이 plugin은 Circuit, Compose, 그리고 core 모듈들의 의존성을 자동으로 포함하므로, 각 feature 모듈의 build.gradle.kts에서는 특별한 의존성(예: libs.logger, libs.kakao.auth)만 별도로 선언하면 된다.
Applied to files:
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.ktfeature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt
📚 Learning: in the reed-android project's termsagreementscreen.kt, the ontermdetailclick event is intentionally ...
Learnt from: seoyoon513
PR: YAPP-Github/Reed-Android#35
File: feature/login/src/main/kotlin/com/ninecraft/booket/feature/login/TermsAgreementScreen.kt:127-127
Timestamp: 2025-07-09T01:14:29.836Z
Learning: In the Reed-Android project's TermsAgreementScreen.kt, the OnTermDetailClick event is intentionally passed an empty string for the URL parameter because the actual URLs for terms detail pages haven't been decided yet. This is a temporary implementation that will be updated once the URLs are finalized.
Applied to files:
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions.kt
🧬 Code Graph Analysis (1)
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions.kt (1)
feature/main/src/main/kotlin/com/ninecraft/booket/feature/main/bottomnavigation/DelegateNavigator.kt (2)
goTo(17-23)pop(25-27)
⏰ 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)
- GitHub Check: ci-build
🔇 Additional comments (7)
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions.kt (1)
11-14: delayedPop 구현이 올바릅니다.기존 pop() 메서드를 지연 호출하는 구현이 정확합니다. Dialog나 BottomSheet가 닫히는 시간을 확보하여 화면 전환 타이밍 문제를 해결하는데 도움이 될 것입니다.
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/SearchPresenter.kt (2)
20-20: 적절한 import 추가.새로 생성된 delayedGoTo 확장 함수를 올바르게 import했습니다.
195-198: BottomSheet 네비게이션 타이밍 문제 해결.책 등록 성공 후 RecordScreen으로 이동하는 네비게이션을 비동기 지연 호출로 변경하여, BottomSheet가 완전히 닫힌 후 화면 전환이 이루어지도록 개선했습니다. 코루틴 스코프 사용도 적절합니다.
feature/record/src/main/kotlin/com/ninecraft/booket/feature/record/register/RecordRegisterPresenter.kt (4)
21-21: 적절한 import 추가.새로 생성된 delayedPop 확장 함수를 올바르게 import했습니다.
155-157: Dialog 종료 시 네비게이션 타이밍 개선.종료 확인 Dialog가 완전히 닫힌 후 화면이 전환되도록 지연된 pop() 호출을 적용했습니다. 코루틴 스코프 사용도 적절합니다.
222-226: 선택적 적용이 적절합니다.OnRecordSavedDialogConfirm에서는 즉시 네비게이션을 사용하고, OnRecordSavedDialogDismiss에서만 지연된 네비게이션을 사용하는 것이 합리적입니다. 사용자가 명시적으로 확인 버튼을 누른 경우와 단순 닫기의 차이를 잘 구분했습니다.
230-232: Dialog dismiss 시 네비게이션 타이밍 개선.Dialog가 자동으로 닫힐 때 지연된 pop() 호출을 적용하여 화면 전환 타이밍 문제를 해결했습니다.
Comment on lines
+6
to
+9
| suspend fun Navigator.delayedGoTo(screen: ReedScreen, delayMillis: Long = 200L) { | ||
| delay(delayMillis) | ||
| goTo(screen) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
임시 해결책으로는 적절하지만 개선 사항을 고려해보세요.
구현 자체는 올바르지만 다음 사항들을 고려해보시기 바랍니다:
- 200ms 기본 지연 시간이 모든 Dialog/BottomSheet에 적합한지 검증 필요
- 사용자 경험 관점에서 지연이 체감될 수 있음
- Circuit의 Overlay 메커니즘을 활용한 더 나은 해결책 검토 권장
🤖 Prompt for AI Agents
In
feature/screens/src/main/kotlin/com/ninecraft/booket/feature/screens/extensions.kt
around lines 6 to 9, the current delayedGoTo function uses a fixed 200ms delay
which may not suit all Dialogs or BottomSheets and could negatively impact user
experience. Review and validate if the default delay is appropriate for all
cases, and consider replacing this approach by leveraging Circuit's Overlay
mechanism for a more robust and user-friendly navigation delay solution.
easyhooon
approved these changes
Aug 1, 2025
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.
🔗 관련 이슈
📙 작업 설명
feature:screen에 delayedNavigator 확장함수 추가🧪 테스트 내역
📸 스크린샷 또는 시연 영상
Reed_._.mp4
💬 추가 설명 or 리뷰 포인트
Summary by CodeRabbit
신규 기능
버그 수정