Revisit iOS interop API #1494

elijah-semyonov · 2024-08-13T13:44:04Z

Changes

Revisit iOS interop API to build upon new implementation of InteropView.
Main functional additions are:
Support of onReset, fine-grain touches strategy control (cooperative with explicit time delay, eager, none)ю

Fixes 5719, 5876

Testing

TBD, new/updated demo pages.

This should be tested by QA

Release Notes

Breaking changes - iOS

UIKitView and UIKitViewController in package androidx.compose.ui.interop are deprecated. New API are mentioned in deprecation message. Deprecated invocations should work fine unless custom onResize is used, it is disallowed now and will print a warning.

Features - iOS

New UIKitView and UIKitViewController API in package androidx.compose.ui.viewinterop. Support of onReset to reuse the interop composable emitted node and avoid excessive native views reallocations, fine-grain touches strategy control (cooperative with explicit time delay, non-cooperative where no touches are received by Compose, ignoring touches).

MatkovIvan · 2024-08-13T14:19:10Z

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitView.uikit.kt

- * @see Modifier.semantics
- */
+@Deprecated(
+    message = "Use androidx.compose.ui.viewinterop.UIKitView instead"


Restore the doc and add replaceWith as separate parameter instead of writing it in the message

MatkovIvan · 2024-08-13T14:20:00Z

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/interop/UIKitView.uikit.kt

+    // Despite the name, onResize actually contains the logic for default resizing strategy.
+    // Since this strategy is already implied, changes to this argument can't be processed in a
+    // sane manner
+    require(onResize == DefaultViewResize) {


It seems like a breaking change if it was used previously

Yes, but it didn't make sense because it basically required user to set a proper frame of interop view, which was already laid out by compose.

It doesn't matter. It can be used in libraries that the user doesn't control, so it will become a blocker for Compose update

When did we stabilise this API in the first place?
I mean, I just don't want to carry a trail of compatibility code on API, that was experimental due to target being experimental.

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropCallbacks.kt

MatkovIvan · 2024-08-13T14:24:10Z

...i/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropElementHolder.uikit.kt

+                callbacks?.run {
+                    newUserComponentSize?.let {


Nitpick: just regular if? double lambda wrapping seems weird

Yeah, it's a side effect of my experimentation with reassignable callbacks/properties and callbacks being var. Changed it.

MatkovIvan · 2024-08-13T14:25:57Z

...i/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropElementHolder.uikit.kt

+    /**
+     * Check that [group] doesn't entirely clip a child view with a [cgRect]
+     */
+    private fun isCgRectEntirelyClippedByGroup(cgRect: CValue<CGRect>): Boolean =


It seems semantic of this is simpler than name isVisible probably?

Yes, refactored it.

MatkovIvan · 2024-08-13T14:29:19Z

...e/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropProperties.uikit.kt

+@Immutable
+data class UIKitInteropProperties @ExperimentalComposeUiApi constructor(
+    internal val interactionMode: UIKitInteropInteractionMode? = UIKitInteropInteractionMode.Cooperative(),
+    internal val isNativeAccessibilityEnabled: Boolean = false


Why are getters internal?

Because it's only needed for us.

I'm not so sure. This is kind of an object where you just wrap a few booleans/enums. The expectation here is full creating + reading for the ability to create a new instance based on the some values from the existing one

Okay, let's make it public

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitView.uikit.kt

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitViewController.uikit.kt

MatkovIvan · 2024-08-13T14:34:06Z

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropCallbacks.kt

+ * All the changes are executed synchronously right after the state change on a main thread
+ * in sync with a CATransaction batching UIKit objects changes to sync it with Compose rendering.
+ */
+interface UIKitInteropCallbacks<T> {


UIKitInteropListener?

Sure, sounds better

MatkovIvan · 2024-08-13T14:36:22Z

...e/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropProperties.uikit.kt

+         * - View receives touches with 150ms delay, allowing compose to intercept them.
+         * - Native accessibility resolution is disabled
+         */
+        val Default = UIKitInteropProperties()


Not sure that we want this as part of a public API

What would you like to do here?
I think we'd still expand this constructor/argument types, so want to keep at least a stable part without having a constructor overloads combinatory explosion

I mean this particular default value. Let's make it internal for now (still can be used as default arg)

Made it internal

MatkovIvan

Also, such new APIs require a second reviewer

MatkovIvan · 2024-08-15T13:39:19Z

...se/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropInteractionMode.kt

+     * @property delay Indicates how much time in milliseconds is given for Compose to intercept
+     * the touches before delivering them to the interop view. The default value is [DEFAULT_DELAY].
+     */
+    class Cooperative(


Let's explicitly mark it as @ExperimentalComposeUiApi

...se/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropInteractionMode.kt

MatkovIvan · 2024-08-15T13:41:51Z

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropListener.kt

+ * main thread in sync with a CATransaction batching UIKit objects changes to sync it with Compose
+ * rendering.
+ */
+interface UIKitInteropListener<T> {


Is this the final shape of this API? public-stable is intended here?

Actually, I've requested some insight from our dev advocates, so it could be not

MatkovIvan · 2024-08-15T13:42:14Z

...e/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropProperties.uikit.kt

+@Immutable
+data class UIKitInteropProperties @ExperimentalComposeUiApi constructor(
+    internal val interactionMode: UIKitInteropInteractionMode? = UIKitInteropInteractionMode.Cooperative(),
+    internal val isNativeAccessibilityEnabled: Boolean = false


...e/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropProperties.uikit.kt

compose/mpp/demo/src/uikitMain/kotlin/androidx/compose/mpp/demo/UIKitInteropExample.kt

elijah-semyonov · 2024-08-20T07:30:58Z

I rolled back additional APIs, only onReset is included.

Gaubee · 2024-08-21T01:07:20Z

Hope you can review and merge this PR as soon as possible. The change in touch strategy is having a big impact on our project, and we really need to push out a version that behaves correctly for our clients.

Thanks! 😊

elijah-semyonov · 2024-08-21T08:00:47Z

Hope you can review and merge this PR as soon as possible. The change in touch strategy is having a big impact on our project, and we really need to push out a version that behaves correctly for our clients.

Thanks! 😊

You mean new touches strategy brought in recent alpha?

kingsword09 · 2024-08-21T08:30:21Z

Hope you can review and merge this PR as soon as possible. The change in touch strategy is having a big impact on our project, and we really need to push out a version that behaves correctly for our clients.
Thanks! 😊

You mean new touches strategy brought in recent alpha?

Yes, in youtrack is this.

elijah-semyonov · 2024-08-21T08:39:46Z

No worries, it's in its presumably final state, so it will be merged after reviewed.
But please, don't use alpha and dev builds for production.

Gaubee · 2024-08-21T10:10:33Z

No worries, it's in its presumably final state, so it will be merged after reviewed.
But please, don't use alpha and dev builds for production.

Understand, but because we are happy to conduct active exploration, we will provide customers with some experimental versions.

ASalavei · 2024-08-21T11:20:12Z

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/viewinterop/InteropViewHolder.skiko.kt

            .trackInteropPlacement(this)
            .onGloballyPositioned { layoutCoordinates ->
                layoutAccordingTo(layoutCoordinates)
                // TODO: Should be the same as [Owner.onInteropViewLayoutChange]?
-//                container.onInteropViewLayoutChange(this)
+                // container.onInteropViewLayoutChange(this)


It's a reminder for investigation after upstream merge, ask @MatkovIvan for more details.

...i/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropElementHolder.uikit.kt

...ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropInteractionMode.uikit.kt

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/InteractionUIView.uikit.kt

MatkovIvan

I like the current shape of it, thanks!
(aside of InteropPlatformDetails in shared code 🙃 )

Also, could you please update PR description/release notes (no breaking changes here)

MatkovIvan · 2024-08-23T06:08:21Z

...e/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/viewinterop/InteropPlatformDetails.skiko.kt

+ * chain needs to be changed within ComposeNode Updater
+ */
+internal interface InteropPlatformDetails {
+    fun platformModifier(holder: InteropViewHolder): Modifier


~~Why isn't it a part of InteropContainer? Those 2 booleans don't seem as platform-specific and can be mentioned in the common interface as parameters in this function.~~

or. it might be converted to something like

internal fun interface InteropViewModifierFactory { fun create(holder: InteropViewHolder): Modifier }

and probably make some default implementation(s) in shared code (outside of this PR).

Native platform accessibility resolution is absolutely platform specific, touch interaction mode is also platform specific.
The point of InteropPlatformDetails is an ability to change something in the implementation of specific InteropViewHolder when it's reused. It can cover basically anything what happens here (in case of iOS - not only change of a modifier, but also - reconfiguration of InteropWrappingView which serves as a group. In future it could be used for expanding gesture recogniser failure requirements behavior and forwarding it to the user.

MatkovIvan · 2024-08-23T06:30:24Z

...i/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropElementHolder.uikit.kt

+            interopWrappingView.interactionMode = null
+        } else {
+            val uiKitInteropPlatformDetails =
+                checkNotNull(platformDetails as? UIKitInteropPlatformDetails) {


Resolving the previous comment should remove this cast, I guess

…re#1494]

vickyleu · 2024-08-31T09:51:24Z

1.7.0-dev1783
In a full-screen WebView, to ensure smooth scrolling, you must set

properties = UIKitInteropProperties(
                    interactionMode= UIKitInteropInteractionMode.Cooperative(
                        delayMillis = 1,
                    ),
                    isNativeAccessibilityEnabled = false
)

I still don't quite understand the purpose of delaying touch events. Why is delay necessary? In most cases, delayMillis doesn't make much sense, as in scrollable scenarios, no one would hold down and wait for the page to become scrollable.

…re#1494]

Gaubee · 2024-09-01T04:47:41Z

1.7.0-dev1783 In a full-screen WebView, to ensure smooth scrolling, you must set
properties = UIKitInteropProperties(
                    interactionMode= UIKitInteropInteractionMode.Cooperative(
                        delayMillis = 1,
                    ),
                    isNativeAccessibilityEnabled = false
)
I still don't quite understand the purpose of delaying touch events. Why is delay necessary? In most cases, delayMillis doesn't make much sense, as in scrollable scenarios, no one would hold down and wait for the page to become scrollable.

Why no use interactionMode = UIKitInteropInteractionMode.NonCooperative?
In most cases, a lightweight UIView just needs a tap event, and WKWebView is a heavyweight UIView, so it has its own scroll event. Scroll delay is the problem that scrolling gestures are not consumed by UIView in a long list of mixed rendered UIViews.

elijah-semyonov · 2024-09-02T06:33:56Z

@vickyleu
No, you need to set NonCooperative. @Gaubee explained the rationale well

elijah-semyonov requested review from MatkovIvan and mazunin-v-jb August 13, 2024 13:44

elijah-semyonov self-assigned this Aug 13, 2024

MatkovIvan reviewed Aug 13, 2024

View reviewed changes

elijah-semyonov force-pushed the es/revisit-ios-interop-api branch from 17081a3 to cdd5ae3 Compare August 14, 2024 08:39

elijah-semyonov marked this pull request as ready for review August 15, 2024 07:01

elijah-semyonov mentioned this pull request Aug 15, 2024

iOS - Scroll does not work at all when touch starts on a UIKitView inside eg a LazyColumn JetBrains/compose-multiplatform#4818

Closed

vickyleu mentioned this pull request Aug 15, 2024

IOS 无法滚动页面内容 KevinnZou/compose-webview-multiplatform#212

Open

elijah-semyonov force-pushed the es/revisit-ios-interop-api branch from 0f4508e to bdae9c4 Compare August 15, 2024 11:20

MatkovIvan reviewed Aug 15, 2024

View reviewed changes

MatkovIvan added the changes in API label Aug 15, 2024

SebastianAigner reviewed Aug 16, 2024

View reviewed changes

compose/mpp/demo/src/uikitMain/kotlin/androidx/compose/mpp/demo/UIKitInteropExample.kt Outdated Show resolved Hide resolved

elijah-semyonov marked this pull request as draft August 19, 2024 07:48

elijah-semyonov force-pushed the es/revisit-ios-interop-api branch from beea29e to a80338b Compare August 19, 2024 09:58

elijah-semyonov marked this pull request as ready for review August 20, 2024 07:20

ASalavei reviewed Aug 21, 2024

View reviewed changes

...i/ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropElementHolder.uikit.kt Show resolved Hide resolved

JetBrains deleted a comment from ASalavei Aug 21, 2024

ASalavei reviewed Aug 21, 2024

View reviewed changes

...ui/src/uikitMain/kotlin/androidx/compose/ui/viewinterop/UIKitInteropInteractionMode.uikit.kt Outdated Show resolved Hide resolved

mazunin-v-jb reviewed Aug 21, 2024

View reviewed changes

compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/InteractionUIView.uikit.kt Outdated Show resolved Hide resolved

mazunin-v-jb approved these changes Aug 21, 2024

View reviewed changes

elijah-semyonov added 3 commits August 21, 2024 14:26

Iterate

12a5615

Rewire API

d82ed6a

Update example

146a32e

elijah-semyonov added 5 commits August 21, 2024 14:26

Modify comment

bd23b5b

Add doc

87ed4fa

Add ms postfix

58ffded

Rename private implementation delay to delayInMilliseconds

45a7ae1

Rename

8bbcb64

elijah-semyonov force-pushed the es/revisit-ios-interop-api branch from 71b6e10 to 8bbcb64 Compare August 21, 2024 12:26

elijah-semyonov added 3 commits August 22, 2024 12:05

Update documentation.

8bdd155

Update documentation.

0dd0d24

Update documentation.

85fa511

MatkovIvan reviewed Aug 23, 2024

View reviewed changes

elijah-semyonov added 10 commits August 26, 2024 13:29

Ditch platform details

b1e93d5

Fix compilation and add an example with new properties behavior

d5f1f3e

Modify example

6ca7a72

Modify example

1926a7c

Early exit from unplace

baa3c2e

Early exit from unplace

9e85c4d

Move early unplace exit

187e95c

Modify examples, remove debug prints

670e007

Modify examples

971e2ac

Remove debug println

e9f520c

MatkovIvan approved these changes Aug 27, 2024

View reviewed changes

elijah-semyonov merged commit 538f5ce into jb-main Aug 28, 2024
6 checks passed

elijah-semyonov deleted the es/revisit-ios-interop-api branch August 28, 2024 08:13

vickyleu added a commit to vickyleu/compose-webview-multiplatform that referenced this pull request Aug 31, 2024

1.7.0 iOS interop API (#pull/1494)[JetBrains/compose-multiplatform-co…

3de0999

…re#1494]

vickyleu added a commit to vickyleu/compose-webview-multiplatform that referenced this pull request Aug 31, 2024

1.7.0 iOS interop API (#pull/1494)[JetBrains/compose-multiplatform-co…

f193a21

…re#1494]

belgianwaves mentioned this pull request Sep 6, 2024

Not working on compose multiplatform 1.7.0-beta01 KevinnZou/compose-webview-multiplatform#219

Open

shageldi-dev mentioned this pull request Sep 16, 2024

Function 'UIKitView' can not be called: No function found for symbol 'androidx.compose.ui.interop/UIKitView|UIKitView Chaintech-Network/ComposeMultiplatformMediaPlayer#20

Closed

Revisit iOS interop API #1494

Revisit iOS interop API #1494

Conversation

elijah-semyonov commented Aug 13, 2024 • edited Loading

Changes

Testing

Release Notes

Breaking changes - iOS

Features - iOS

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elijah-semyonov Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MatkovIvan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elijah-semyonov Aug 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elijah-semyonov commented Aug 20, 2024

Gaubee commented Aug 21, 2024

elijah-semyonov commented Aug 21, 2024

kingsword09 commented Aug 21, 2024

elijah-semyonov commented Aug 21, 2024

Gaubee commented Aug 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MatkovIvan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elijah-semyonov Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vickyleu commented Aug 31, 2024

Gaubee commented Sep 1, 2024 • edited Loading

elijah-semyonov commented Sep 2, 2024 • edited Loading

elijah-semyonov commented Aug 13, 2024 •

edited

Loading

elijah-semyonov Aug 14, 2024 •

edited

Loading

elijah-semyonov Aug 19, 2024 •

edited

Loading

elijah-semyonov Aug 23, 2024 •

edited

Loading

Gaubee commented Sep 1, 2024 •

edited

Loading

elijah-semyonov commented Sep 2, 2024 •

edited

Loading