[fix] #190 NekiActionToast 버튼 터치 불가 버그 수정#191
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review Walkthrough기존 Toast 구현을 제거하고 WindowManager 기반의 ComposeView 오버레이로 교체했으며, composable에 Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller (호출자)
participant Neki as NekiToast
participant WM as WindowManager
participant CV as ComposeView
participant CJ as Coroutine (Main)
Caller->>Neki: showToast(...)/showActionToast(...)
Neki->>WM: create LayoutParams, addView(ComposeView)
WM-->>CV: ComposeView attached
Neki->>CV: setContent { toast(dismiss) }
Neki->>CJ: launch delay(duration) -> dismiss()
Caller->>CV: (사용자) 버튼 클릭
CV->>Neki: invoke onClick (wrapped: dismiss() then action)
Neki->>WM: removeView(ComposeView), cancel dismissJob
CJ--xNeki: (if delay expires) dismiss() -> removeView
예상 코드 리뷰 노력🎯 3 (Moderate) | ⏱️ ~25분 시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
core/ui/src/main/java/com/neki/android/core/ui/toast/NekiToast.kt (2)
93-98: 사용하지 않는dismiss파라미터를 명시적으로 무시하세요.
makeToast의 람다는dismiss: () -> Unit파라미터를 받지만,showToast에서는 사용하지 않습니다. 가독성을 위해_로 명시적으로 무시하는 것이 좋습니다.✨ 코드 명확성 개선
- makeToast(duration = duration) { + makeToast(duration = duration) { _ -> NekiToast( iconRes = iconRes, text = text, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/ui/src/main/java/com/neki/android/core/ui/toast/NekiToast.kt` around lines 93 - 98, The lambda passed to makeToast currently ignores the dismiss: () -> Unit parameter; update the call site so the lambda explicitly ignores that parameter (e.g. use { _ -> ... }) to improve clarity. Locate the makeToast invocation in NekiToast.kt (the block that builds NekiToast with iconRes and text) and change its lambda to accept and underscore the dismiss argument so the unused parameter is clearly intentional.
36-86: 중복 토스트 표시 방지 로직 추가를 고려하세요.현재 구현에서는
showToast또는showActionToast가 연속으로 호출되면 기존 토스트를 dismiss하지 않고 새 토스트가 추가됩니다. 이로 인해 여러 토스트가 겹쳐 표시되거나 리소스 누수가 발생할 수 있습니다.♻️ 중복 토스트 방지 로직 제안
class NekiToast( private val context: Context, ) { private val windowManager = context.getSystemService(Context.WINDOW_SERVICE) as WindowManager + private var currentComposeView: ComposeView? = null + private var currentDismissJob: Job? = null private fun makeToast( duration: Int = Toast.LENGTH_SHORT, toast: `@Composable` (dismiss: () -> Unit) -> Unit, ) { val activity = context as ComponentActivity - var dismissJob: Job? = null - lateinit var composeView: ComposeView + + // 기존 토스트 제거 + currentDismissJob?.cancel() + currentComposeView?.let { + if (it.isAttachedToWindow) { + windowManager.removeView(it) + } + } fun dismiss() { - dismissJob?.cancel() - if (composeView.isAttachedToWindow) { - windowManager.removeView(composeView) + currentDismissJob?.cancel() + currentComposeView?.let { + if (it.isAttachedToWindow) { + windowManager.removeView(it) + } } + currentComposeView = null } - composeView = ComposeView(context).apply { + currentComposeView = ComposeView(context).apply { // ... 기존 코드 ... } - windowManager.addView(composeView, params) + windowManager.addView(currentComposeView, params) - dismissJob = activity.lifecycleScope.launch(Dispatchers.Main) { + currentDismissJob = activity.lifecycleScope.launch(Dispatchers.Main) { delay(if (duration == Toast.LENGTH_SHORT) 2500L else 3500L) dismiss() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/ui/src/main/java/com/neki/android/core/ui/toast/NekiToast.kt` around lines 36 - 86, The makeToast currently always creates and adds a new ComposeView causing overlapping toasts; modify makeToast to first check and clear any existing toast by cancelling dismissJob and removing the previous composeView from windowManager (or calling the existing dismiss() helper) before creating a new ComposeView. Persist the active composeView and dismissJob as instance-level properties (e.g., currentComposeView, currentDismissJob) or reuse the existing composeView variable so makeToast can call dismiss() / windowManager.removeView(composeView) and cancel the job prior to windowManager.addView, ensuring you reference makeToast, dismiss(), composeView, dismissJob and windowManager when implementing the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/ui/src/main/java/com/neki/android/core/ui/toast/NekiToast.kt`:
- Around line 31-34: The NekiToast class constructor currently accepts a Context
but later force-casts it to ComponentActivity in makeToast, risking
ClassCastException; change the constructor parameter type from Context to
ComponentActivity (update the constructor signature for class NekiToast and any
callers), initialize windowManager via the provided ComponentActivity (replace
context.getSystemService(...) with activity.getSystemService(...)), and remove
the forced cast inside makeToast so all uses rely on the now type-safe
ComponentActivity instance.
---
Nitpick comments:
In `@core/ui/src/main/java/com/neki/android/core/ui/toast/NekiToast.kt`:
- Around line 93-98: The lambda passed to makeToast currently ignores the
dismiss: () -> Unit parameter; update the call site so the lambda explicitly
ignores that parameter (e.g. use { _ -> ... }) to improve clarity. Locate the
makeToast invocation in NekiToast.kt (the block that builds NekiToast with
iconRes and text) and change its lambda to accept and underscore the dismiss
argument so the unused parameter is clearly intentional.
- Around line 36-86: The makeToast currently always creates and adds a new
ComposeView causing overlapping toasts; modify makeToast to first check and
clear any existing toast by cancelling dismissJob and removing the previous
composeView from windowManager (or calling the existing dismiss() helper) before
creating a new ComposeView. Persist the active composeView and dismissJob as
instance-level properties (e.g., currentComposeView, currentDismissJob) or reuse
the existing composeView variable so makeToast can call dismiss() /
windowManager.removeView(composeView) and cancel the job prior to
windowManager.addView, ensuring you reference makeToast, dismiss(), composeView,
dismissJob and windowManager when implementing the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6dfbfb03-5f57-40fd-b6a1-cd3065484f1c
📒 Files selected for processing (1)
core/ui/src/main/java/com/neki/android/core/ui/toast/NekiToast.kt
Comment on lines
31
to
+34
| class NekiToast( | ||
| private val context: Context, | ||
| ) : Toast(context) { | ||
| private fun makeText( | ||
| duration: Int = LENGTH_SHORT, | ||
| toast: @Composable () -> Unit, | ||
| ) { | ||
| private val windowManager = context.getSystemService(Context.WINDOW_SERVICE) as WindowManager |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Context 타입을 ComponentActivity로 변경하여 타입 안전성을 확보하세요.
현재 생성자에서 Context를 받지만, makeToast 내부(Line 40)에서 ComponentActivity로 강제 캐스팅합니다. 만약 ApplicationContext나 Service 등 다른 Context로 인스턴스를 생성하면 런타임에 ClassCastException이 발생합니다.
🔧 타입 안전성 개선 제안
class NekiToast(
- private val context: Context,
+ private val activity: ComponentActivity,
) {
- private val windowManager = context.getSystemService(Context.WINDOW_SERVICE) as WindowManager
+ private val windowManager = activity.getSystemService(Context.WINDOW_SERVICE) as WindowManager이렇게 변경하면 makeToast 내부의 캐스팅도 제거할 수 있습니다:
- val activity = context as ComponentActivity
+ // activity는 이미 ComponentActivity 타입📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| class NekiToast( | |
| private val context: Context, | |
| ) : Toast(context) { | |
| private fun makeText( | |
| duration: Int = LENGTH_SHORT, | |
| toast: @Composable () -> Unit, | |
| ) { | |
| private val windowManager = context.getSystemService(Context.WINDOW_SERVICE) as WindowManager | |
| class NekiToast( | |
| private val activity: ComponentActivity, | |
| ) { | |
| private val windowManager = activity.getSystemService(Context.WINDOW_SERVICE) as WindowManager |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/ui/src/main/java/com/neki/android/core/ui/toast/NekiToast.kt` around
lines 31 - 34, The NekiToast class constructor currently accepts a Context but
later force-casts it to ComponentActivity in makeToast, risking
ClassCastException; change the constructor parameter type from Context to
ComponentActivity (update the constructor signature for class NekiToast and any
callers), initialize windowManager via the provided ComponentActivity (replace
context.getSystemService(...) with activity.getSystemService(...)), and remove
the forced cast inside makeToast so all uses rely on the now type-safe
ComponentActivity instance.
ikseong00
approved these changes
Apr 15, 2026
Contributor
ikseong00
left a comment
ikseong00
left a comment
There was a problem hiding this comment.
확인했습니다! 리뷰 확인해주시고 바로 머지하셔도 될 것 같아요
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
3 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.
🔗 관련 이슈
📙 작업 설명
NekiToast가 AndroidToast를 상속하는 구조에서WindowManager기반으로 교체ToastWindow는FLAG_NOT_TOUCHABLE이 기본 적용되어ComposeView내 버튼 터치가 불가능했던 문제 수정(토스트 메시지 뒤의 배경이 터치되고 있었음)WindowManager.LayoutParams.TYPE_APPLICATION+FLAG_NOT_FOCUSABLE조합으로 터치 이벤트 정상 전달showActionToast버튼 클릭 시 toast가 즉시 닫히도록dismiss처리 추가🧪 테스트 내역
showToast(),showActionToast()동작 확인 완료📸 스크린샷 또는 시연 영상 (선택)
KakaoTalk_Video_2026-04-15-00-24-55.mp4
Summary by CodeRabbit
토스트 알림 개선
New Features
Improvements