refactor: composed {} 를 이용한 Custom Modifier 구현 Modifier.Node API 사용 방식으로 변경 #263
Conversation
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
…de API 사용 방식으로 마이그레이션
Walkthroughcomposed 기반의 Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Layer
participant ModifierNode as ClickableSingleNode
participant Pointer as SuspendingPointerInputModifierNode
participant Ripple as RippleNode
participant Semantics as SemanticsModifierNode
participant Cutter as MultipleEventsCutter
UI->>ModifierNode: 클릭 가능 요소 렌더링
ModifierNode->>Semantics: 의미론 등록(onClick, label, enabled)
ModifierNode->>Pointer: 포인터 이벤트 수신 등록
Pointer->>ModifierNode: pointerDown / pointerUp 이벤트
Pointer->>Cutter: 전달(중복 차단)
Cutter->>ModifierNode: onTap 트리거
ModifierNode->>Ripple: 리플 표시 시작
ModifierNode->>Semantics: PressInteraction/State 업데이트
ModifierNode->>UI: onClick 콜백 실행
Ripple->>ModifierNode: 리플 종료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 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 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: 2
🤖 Fix all issues with AI agents
In
`@core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Modifier.kt`:
- Around line 71-88: 현재 Modifier에서 shouldAutoInvalidate = false로 설정해 놓고
update()에서 invalidate()를 호출하지 않아 enabled/label/role 변경 시 포인터 입력 재평가와
applySemantics() 갱신이 누락됩니다; 해결 방법은 (1) shouldAutoInvalidate를 true로 되돌리거나 (2)
update() 내부에서 이전 상태와 현재 상태(enabled, label, role 등)를 비교해 변경이 감지되면 invalidate()를
호출하도록 변경하세요; 관련 식별자: SuspendingPointerInputModifierNode, update(), invalidate(),
shouldAutoInvalidate, applySemantics()을 찾아 적용하고 동일한 수정은 104-110 범위에도 반영하세요.
- Around line 91-97: The semantics onClick lambda always returns true regardless
of enabled state; update SemanticsPropertyReceiver.applySemantics so the
onClick(label = onClickLabel) handler only calls
multipleEventsCutter.processEvent { this@ClickableSingleNode.onClick() } and
returns true when enabled is true, and returns false when enabled is false
(i.e., make the returned boolean reflect enabled), so the accessibility service
receives the correct success/failure result.
core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Modifier.kt
Show resolved
Hide resolved
core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Modifier.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Modifier.kt`:
- Around line 69-89: The pointer input coroutine started by pointerInputNode's
SuspendingPointerInputModifierNode doesn't restart when enabled toggles; modify
the update() implementation to detect changes to the enabled property and call
resetPointerInputHandler() when enabled value differs from previous state so the
detectTapGestures coroutine is restarted; locate pointerInputNode, the
SuspendingPointerInputModifierNode block, and the update() method and ensure you
compare the old and new enabled values and invoke resetPointerInputHandler() on
change.
Comment on lines
+69
to
+89
| @Suppress("unused") | ||
| private val pointerInputNode = delegate( | ||
| SuspendingPointerInputModifierNode { | ||
| if (!enabled) return@SuspendingPointerInputModifierNode | ||
| detectTapGestures( | ||
| onPress = { offset -> | ||
| val press = PressInteraction.Press(offset) | ||
| interactionSource.emit(press) | ||
| val released = tryAwaitRelease() | ||
| if (released) { | ||
| interactionSource.emit(PressInteraction.Release(press)) | ||
| } else { | ||
| interactionSource.emit(PressInteraction.Cancel(press)) | ||
| } | ||
| }, | ||
| onTap = { | ||
| multipleEventsCutter.processEvent { onClick() } | ||
| }, | ||
| ) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
enabled 상태 변경 시 포인터 입력 핸들러가 재시작되지 않습니다.
SuspendingPointerInputModifierNode의 코루틴은 한 번 시작되면 enabled 값이 변경되어도 자동으로 재시작되지 않습니다. enabled가 true에서 false로 변경되어도 이미 실행 중인 detectTapGestures 코루틴이 계속 탭을 처리합니다.
update() 메서드에서 enabled 상태 변경 시 resetPointerInputHandler()를 호출해야 합니다.
🩹 수정 제안
- `@Suppress`("unused")
private val pointerInputNode = delegate(
SuspendingPointerInputModifierNode {
if (!enabled) return@SuspendingPointerInputModifierNode
detectTapGestures(
onPress = { offset ->
val press = PressInteraction.Press(offset)
interactionSource.emit(press)
val released = tryAwaitRelease()
if (released) {
interactionSource.emit(PressInteraction.Release(press))
} else {
interactionSource.emit(PressInteraction.Cancel(press))
}
},
onTap = {
multipleEventsCutter.processEvent { onClick() }
},
)
},
)
fun update(enabled: Boolean, onClickLabel: String?, role: Role?, onClick: () -> Unit) {
+ val enabledChanged = this.enabled != enabled
this.enabled = enabled
this.onClickLabel = onClickLabel
this.role = role
this.onClick = onClick
+ if (enabledChanged) {
+ pointerInputNode.resetPointerInputHandler()
+ }
}Also applies to: 103-108
🤖 Prompt for AI Agents
In
`@core/common/src/main/kotlin/com/ninecraft/booket/core/common/extensions/Modifier.kt`
around lines 69 - 89, The pointer input coroutine started by pointerInputNode's
SuspendingPointerInputModifierNode doesn't restart when enabled toggles; modify
the update() implementation to detect changes to the enabled property and call
resetPointerInputHandler() when enabled value differs from previous state so the
detectTapGestures coroutine is restarted; locate pointerInputNode, the
SuspendingPointerInputModifierNode block, and the update() method and ensure you
compare the old and new enabled values and invoke resetPointerInputHandler() on
change.
easyhooon
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
composed {}는 내부적으로 materialize() 과정을 거쳐 별도의 Composition을 생성합니다. 이를 통해 remember, CompositionLocal 등 Composable API를 사용할 수 있지만,
Modifier.Node가 해결하는 방식
Modifier.Node는 Node 인스턴스가 직접 상태를 보유하므로 remember나 Composition 없이 값을 유지하고, update()를 통해 변경 사항만 반영합니다.
reference)
https://developer.android.com/develop/ui/compose/custom-modifiers?utm_source=android-studio-app&utm_medium=app#implement-custom
coil-kt/coil#2115
https://blog.naver.com/tgyuu_/224143006432
https://kimansu.medium.com/compose-modifier-node-4390861c9e68
Summary by CodeRabbit
새로운 기능
개선사항