Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iOS SingleLayerComposeScene with feature flag #920

Merged
merged 1 commit into from Dec 9, 2023

Conversation

dima-avdeev-jb
Copy link

@dima-avdeev-jb dima-avdeev-jb commented Dec 4, 2023

Added basic iOS implementation for SingleLayerComposeScene.
Added feature flag to turn it on:

@OptIn(ExperimentalComposeApi::class)
ComposeUIViewController(
    configure = {
        platformLayers = true
    }
) {
    Text("Some Compose code")
}

Where are known bug with device rorattion and touch handling.
We need to merge this changes for future cooperative work. This PR contains big architecture changes in iOS ui:ui module.
By default feauture flag is turned off. And stable way to layout Popups and Dialogs will be used.

Also fixes Issue with password TextField (JetBrains/compose-multiplatform#2988)

image

image

@dima-avdeev-jb dima-avdeev-jb changed the title iOS SingleLayerComposeScene by feature flag iOS SingleLayerComposeScene with feature flag Dec 4, 2023
}

internal fun getUITextInputTraits(currentImeOptions: ImeOptions?) =
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied it as is

Comment on lines 114 to 126
when (currentImeOptions?.keyboardType) {
KeyboardType.Password, KeyboardType.NumberPassword -> UITextContentTypePassword
KeyboardType.Email -> UITextContentTypeEmailAddress
KeyboardType.Phone -> UITextContentTypeTelephoneNumber
else -> null
}

override fun isSecureTextEntry(): Boolean =
when (currentImeOptions?.keyboardType) {
KeyboardType.Password, KeyboardType.NumberPassword -> true
else -> false
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fixes password TextField issue JetBrains/compose-multiplatform#2988

Comment on lines 110 to 123
textUIView = IntermediateTextInputUIView(
keyboardEventHandler = keyboardEventHandler,
).also {
rootView.addSubview(it)
it.translatesAutoresizingMaskIntoConstraints = false
NSLayoutConstraint.activateConstraints(
listOf(
it.leftAnchor.constraintEqualToAnchor(rootView.leftAnchor),
it.rightAnchor.constraintEqualToAnchor(rootView.rightAnchor),
it.topAnchor.constraintEqualToAnchor(rootView.topAnchor),
it.bottomAnchor.constraintEqualToAnchor(rootView.bottomAnchor),
).also {
constraints = it
}
)
}
textUIView?.input = createSkikoInput(value)
textUIView?.inputTraits = getUITextInputTraits(imeOptions)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

textUIView it is invisible UIView with userInteraction=false.
But this textUIView can becomeFirstResponder() when Compose TextField comes to focus.

Comment on lines 139 to 141
constraints?.let {
NSLayoutConstraint.deactivateConstraints(it)
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can not deactivateConstraints when view removes from super view?
But I think it is good to have explicit deactivation code.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constraints across the boundaries of detachment point are deactivated automatically.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this code

Comment on lines 122 to 141
_showSoftwareKeyboard()
textUIView?.let {
focusStack.pushAndFocus(it)
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keyboard appearence logic goes to FocusStack

Comment on lines +36 to +37
@ExperimentalComposeApi
var platformLayers: Boolean = false
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Temporary experimental feature flag.
When we fully migrates to SingleLayerComposeScene, this flag will be removed

Comment on lines 84 to 87
internal class IntermediateTextInputUIView(
private val keyboardEventHandler: KeyboardEventHandler,
) : UIView(frame = CGRectZero.readValue()),
UIKeyInputProtocol, UITextInputProtocol {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This IntermediateTextInputUIView helps to solve password TextField problem.
Now when Compose TextField will be focused - this IntermediateTextInputUIView will be created. And removed after unfocus.
So, each new TextField keyboard behavior will be configured from scratch without cahces.

Almost all of implementation just moved from SkikoUIView. (Previously SkikoUIView was implement this behavior)

fun keyboardWillHide(arg: NSNotification)
}

internal class KeyboardVisibilityListenerImpl(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation of this class just copied moved from ComposeWindow.kt

@@ -191,6 +191,7 @@ internal class InflightCommandBuffers(
internal class MetalRedrawer(
private val metalLayer: CAMetalLayer,
private val callbacks: MetalRedrawerCallbacks,
private val transparentBackground: Boolean,
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need transparentBackground for transparent Dialogs (with rounded corners for example) and Popup's like TextFieldSelectionHandle.
But first root ComposeScene will be rendered opaque by default. It will be transparent only with usage of UIKitView as before.

import platform.UIKit.UIView
import platform.UIKit.UIViewController

internal interface RootViewControllerState<RootView, SceneView> {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need generics for smoother migration from UIView as scene view representation to UIViewController

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add TODO/comment that this is temporary for transition

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We decided to remove generic params. And just inlined UIViewController as RootView and UIView as SceneView
And naming also changes to Bridges suffix

@dima-avdeev-jb dima-avdeev-jb marked this pull request as ready for review December 5, 2023 06:35
private fun IntRange.toUITextRange(): UITextRange =
IntermediateTextRange(start = start, end = endInclusive + 1)

private fun NSWritingDirection.directionToStr() =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toString?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is Long type. So better to stay it directionToStr()

var input: IOSSkikoInput? = null
var inputTraits: SkikoUITextInputTraits = object : SkikoUITextInputTraits {}
var onAttachedToWindow: (() -> Unit)? = null
private val _isReadyToShowContent: MutableState<Boolean> = mutableStateOf(false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed - it's better to get rid of compose state primitives inside such classes

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now it is better to leave it here

Comment on lines 98 to 100
// The calculation is split in two instead of
// `(targetTimestamp * 1e9).toLong()`
// to avoid losing precision for fractional part
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extract such things to separate functions. It should not be right in the render function

@MatkovIvan

This comment was marked as resolved.


internal class IOSPlatformContextImpl(
inputServices: PlatformTextInputService,
override val textToolbar: TextToolbar,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it's always the same as the first parameter

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it is different interfaces. So better to pass it as is.

@dima-avdeev-jb dima-avdeev-jb force-pushed the dima.avdeev/ios-single-layer-compose-scene branch from 24334fd to 76cfbe7 Compare December 9, 2023 05:57
@dima-avdeev-jb dima-avdeev-jb merged commit 5b153cc into jb-main Dec 9, 2023
4 checks passed
@dima-avdeev-jb dima-avdeev-jb deleted the dima.avdeev/ios-single-layer-compose-scene branch December 9, 2023 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants