[refactor] #62 composed{} -> Modifier.Node() 마이그레이션

[Ojongseok]

🔗 관련 이슈

• Close [refactor] @Composable Stability 개선 #63

📙 작업 설명

composed {} -> Modifier.Node() 마이그레이션

• noRippleClickable 같은 경우에는 composed{}가 아닌 then/Node로 구현된 Modifier.clickable() 함수가 내장되어 있어 간단하게 변경이 가능했습니다.

---

• 저도 명확하게 이해하지는 못했지만 이해한대로 설명을 드리자면 clickableSingle, noRippleClickableSingle의 경우 각각 ClickableSingleElement이라는 Modifier를 반환하고, ClickableSingleElement는 data class로 ModifierNodeElement<ClickableSingleNode>를 상속받는데 ModifierNodeElement는 결국 Modifier입니다.
• ClickableSingleElement는 ClickableSingleNode를 create()하거나 update()하는데 최초 1회(컴포지션) 시에만 create()되고, 이후에 다시 클릭 이벤트 발생 시 ClickableSingleElement data class의 요소 중 변경사항이 있는지 equals()를 통해 확인하고, 변경사항이 있다면 update()를 한다고 합니다.
• 즉, ClickableSingleElement의 요소인 enabled, onClick, indicationNodeFactory 중 변경사항이 있는지 없는지 확인을 하는데 onClick은 람다이지만 컴포즈 컴파일러가 캡쳐하는 값이 Stable하다면 euquals()의 결과를 true로 반환한다고 합니다...? from. 클코. / 그리고 저희는 ripple을 ripple = ripple()로만 전달하고 있고, ripple()의 내부 구현체를 보면 싱글톤으로 선언된 것을 볼 수 있습니다.
• 그래서 결론은 상위 컴포저블에서 리컴포지션이 발생하더라도 ClickableSingleElement 요소의 변화가 없기 때문에� clickableSingle(), noRippleClickableSingle()을 사용하는 컴포저블 함수의 Modifier를 새로 생성한다거나 업데이트하지 않는다!! 결론입니다. 정확히 이해했는지 모르겠지만요,,

💬 추가 설명 or 리뷰 포인트 (선택)

• 실제 변경되는 파일은 modifier/Click.kt 파일 1건입니다.
• composed{}와 Node의 리컴포지션 관련 동작 방식의 차이를 확인하기에 해당 샘플이 좋은 것 같아 빌드해서 로그 확인용 커밋을 추가했습니다.
  해당 브랜치로 체크아웃 후 로그로 직접 확인해보시면 이해하는데에 도움이 될 것 같습니다!
• 확인이 완료되시면 해당 커밋은 제거 후 병합하겠습니다.

Compose Modifiers deep dive 개인적으로 해당 영상의 11:35 부터 보았을 떄 조금 더 이해가 수월했었는데 한 번 보시고, 코드를 보는 것도 좋을 것 같습니다.

[ikseong00]

참고 영상을 확인하고 코드를 보니 더 잘 이해가 됐습니다!
Compose에서 사용하는 ClickableNode, ClickableElement를 비슷한 형식으로 구현하고 MultipleEventsCutter를 사용하여 Node 내에서 시간정보를 가지고 있어 Throttiling 을 막는 방법 잘 구현하신 것 같습니다!

다만,
ClickableNode는 AbstractClickableNode()를 상속하고 있고, AbstractClickableNode()는 아래 노드들을 상속하고 있습니다.

DelegatingNode(), PointerInputModifierNode, KeyInputModifierNode,
SemanticsModifierNode, TraversableNode, CompositionLocalConsumerModifierNode,
ObserverModifierNode

ClickableSingleNode()는 아래 노드만을 상속하고 있습니다.

DelegatingNode(), PointerInputModifierNode, SemanticsModifierNode

KeyInputModifierNode - 키보드가 필요한 환경에서 사용되는 노드, 현재 디바이스만 타겟이기에 불필요할 것 같습니다.
TraversableNode - 부모 레이아웃 중 ScrollableContainerNode가 있는지 탐색합니다. 스크롤 가능한 레이아웃이라면, delay(TapIndicationDelay) 만큼의 딜레이를 주어 스크롤 이벤트인 경우엔 터치이벤트를 하지 않도록 막습니다.
CompositionLocalConsumerModifierNode - LocalIndication을 읽어옵니다. 현재는 LocalIndication을 따로 정의하지 않기에 필요 없을 것 같습니다.

또한, 현재 PressInteraction.Cancel 를 따로 지정해주지 않고 있습니다.
MutableInteractionSource 에서 Press, Release, Cancel 이벤트를 구독하고 있고 이에 따라 ripple()효과가 진행됩니다.
AbstractClickableNode에서는 onDetach() 에서 노드가 제거될 때 PressInteraction.Cancel를 따로 발행해주고 있습니다. (HoverInteraction.Exit()도 발행합니다.)
ClickableSingleNode에서는 clickableSingle() 에서 ripple()을 주어주고 있으므로 노드가 제거될 때 PressInteraction을 구독하는 InteractionSource도 따라가기 때문에 현 상황에선 문제가 없을 것 같습니다.

fun Modifier.clickableSingle(
    enabled: Boolean = true,
    onClickLabel: String? = null,
    role: Role? = null,
    onClick: () -> Unit,
): Modifier = this.then(
    ClickableSingleElement(
        enabled = enabled,
        onClickLabel = onClickLabel,
        role = role,
        onClick = onClick,
        indicationNodeFactory = ripple(),
    ),
)

따라서 추후에 InteractionSource 를 외부에서 사용해야하는 경우엔 InteractionSource가 노드보다 수명주기가 길어집니다.
이 경우는 드물겠지만 Press 이벤트만 받고 노드가 사라지는 경우에 InteractionSource 에서 Press 만 받은 상태로 유지되는 경우를 해결하기 위해서 onDetach()에서 취소 이벤트를 발행하는 방향으로 수정이 필요할 것 같습니다.

[ikseong00]

enabled 에 따라서 Semantics 처리를 해줘야할 것 같습니다!

    final override fun SemanticsPropertyReceiver.applySemantics() {
        if (this@AbstractClickableNode.role != null) {
            role = this@AbstractClickableNode.role!!
        }
        onClick(
            action = {
                onClick()
                true
            },
            label = onClickLabel,
        )
        if (enabled) {
            with(focusableNode) { applySemantics() }
        } else {
            disabled()
        }
        applyAdditionalSemantics()
    }

AbstractClickableNode 내의 코드입니다.

[Ojongseok]

내부 구현은 위와 같이 되어있지만 저희 앱은 키보드를 통한 조작을 현재로서 지원하지 않아 with(focusableNode) { applySemantics() } 구문 추가가 불필요해 보이고, applyAdditionalSemantics() 구문은 내부적으로 onLongClick()을 지원하기 위해 구현되어 있는 것으로 이해했습니다.

저희는 위와 같은 동작은 지원하지 않는데 해당 구문 추가 시 관련된 코드가 함께 추가되어야 해 불필요하게 길어질 것 같아 if (!enabled) { disabled() } 해당 구문만 추가하였습니다! 98d4881

[Ojongseok]

따라서 추후에 InteractionSource 를 외부에서 사용해야하는 경우엔 InteractionSource가 노드보다 수명주기가 길어집니다. 이 경우는 드물겠지만 Press 이벤트만 받고 노드가 사라지는 경우에 InteractionSource 에서 Press 만 받은 상태로 유지되는 경우를 해결하기 위해서 onDetach()에서 취소 이벤트를 발행하는 방향으로 수정이 필요할 것 같습니다.

로그를 찍어 확인해보니 말씀하신대로 정말 Pressed된 상태에서 화면 이탈 시 Pressed된 상태가 해제되지 않는 현상을 확인했습니다.
onDetach()에서 취소 이벤트를 발생시키는데 기존에 Press나 Cancel을 발생시키는 방식과 유사하게 coroutineScope {}로 처리하려 하니 화면 이탈 시 scope가 같이 종료되어 플래그를 통해 onDetach()에서 해제할 수 있도록 수정했고 로그 통해서 정상적으로 취소되는 것 확인했습니다! be6d597