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

Dialog ime inset transaction fix #1099

Draft
wants to merge 3 commits into
base: jb-main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ actual val WindowInsets.Companion.displayCutout: WindowInsets
/**
* An insets type representing the window of an "input method",
* for iOS IME representing the software keyboard.
*
* TODO: Animation doesn't work on iOS yet
*/
actual val WindowInsets.Companion.ime: WindowInsets
@Composable
Expand Down
5 changes: 3 additions & 2 deletions compose/ui/ui/api/desktop/ui.api
Original file line number Diff line number Diff line change
Expand Up @@ -3853,8 +3853,8 @@ public final class androidx/compose/ui/window/DialogProperties {
public static final field $stable I
public fun <init> (ZZZ)V
public synthetic fun <init> (ZZZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (ZZZZJILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (ZZZZJLkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (ZZZZZJILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (ZZZZZJLkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (ZZ[Ljava/lang/Void;Z)V
public synthetic fun <init> (ZZ[Ljava/lang/Void;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
Expand All @@ -3863,6 +3863,7 @@ public final class androidx/compose/ui/window/DialogProperties {
public final fun getScrimColor-0d7_KjU ()J
public final fun getUsePlatformDefaultWidth ()Z
public final fun getUsePlatformInsets ()Z
public final fun getUseSoftwareKeyboardInset ()Z
public fun hashCode ()I
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ class PlatformInsets(
}
}

internal fun PlatformInsets.union(insets: PlatformInsets) = PlatformInsets(
left = maxOf(left, insets.left),
top = maxOf(top, insets.top),
right = maxOf(right, insets.right),
bottom = maxOf(bottom, insets.bottom)
)

internal fun PlatformInsets.exclude(insets: PlatformInsets) = PlatformInsets(
left = (left - insets.left).coerceAtLeast(0.dp),
top = (top - insets.top).coerceAtLeast(0.dp),
Expand All @@ -80,18 +87,32 @@ internal interface InsetsConfig {
val safeInsets: PlatformInsets
@Composable get

val ime: PlatformInsets
@Composable get

// Don't make it public, it should be implementation details for creating new root layout nodes.
// TODO: Ensure encapsulation and proper control flow during refactoring [Owner]s
@Composable
fun excludeSafeInsets(content: @Composable () -> Unit)
fun excludeInsets(
safeInsets: Boolean,
ime: Boolean,
content: @Composable () -> Unit
)
}

internal object ZeroInsetsConfig : InsetsConfig {
override val safeInsets: PlatformInsets
@Composable get() = PlatformInsets.Zero

override val ime: PlatformInsets
@Composable get() = PlatformInsets.Zero

@Composable
override fun excludeSafeInsets(content: @Composable () -> Unit) {
override fun excludeInsets(
safeInsets: Boolean,
ime: Boolean,
content: @Composable () -> Unit
) {
content()
}
}
Expand All @@ -103,6 +124,6 @@ internal object ZeroInsetsConfig : InsetsConfig {
* different on each platform.
*
* TODO: Stabilize and make the window paddings in the foundation-layout module depend on it.
* There is a plan to potentially move this variable into the [Platform] interface.
* There is a plan to potentially move this variable into the [PlatformContext] interface.
*/
internal expect var PlatformInsetsConfig: InsetsConfig
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,18 @@ import androidx.compose.ui.input.key.key
import androidx.compose.ui.input.key.type
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.layout.Layout
import androidx.compose.ui.platform.InsetsConfig
import androidx.compose.ui.platform.LocalWindowInfo
import androidx.compose.ui.platform.PlatformInsets
import androidx.compose.ui.platform.PlatformInsetsConfig
import androidx.compose.ui.platform.ZeroInsetsConfig
import androidx.compose.ui.platform.union
import androidx.compose.ui.scene.ComposeSceneLayer
import androidx.compose.ui.scene.rememberComposeSceneLayer
import androidx.compose.ui.semantics.dialog
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.unit.IntRect
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.center
import androidx.compose.ui.unit.dp

/**
* The default scrim opacity.
Expand All @@ -58,8 +58,10 @@ private val DefaultScrimColor = Color.Black.copy(alpha = DefaultScrimOpacity)
* dialog's bounds. If true, clicking outside the dialog will call onDismissRequest.
* @property usePlatformDefaultWidth Whether the width of the dialog's content should be limited to
* the platform default, which is smaller than the screen width.
* @property usePlatformInsets Whether the width of the popup's content should be limited by
* @property usePlatformInsets Whether the size of the dialog's content should be limited by
* platform insets.
* @property useSoftwareKeyboardInset Whether the size of the dialog's content should be limited by
* software keyboard inset.
* @property scrimColor Color of background fill.
*/
@Immutable
Expand All @@ -68,6 +70,7 @@ actual class DialogProperties @ExperimentalComposeUiApi constructor(
actual val dismissOnClickOutside: Boolean = true,
actual val usePlatformDefaultWidth: Boolean = true,
val usePlatformInsets: Boolean = true,
val useSoftwareKeyboardInset: Boolean = true,
val scrimColor: Color = DefaultScrimColor,
) {
// Constructor with all non-experimental arguments.
Expand All @@ -80,6 +83,7 @@ actual class DialogProperties @ExperimentalComposeUiApi constructor(
dismissOnClickOutside = dismissOnClickOutside,
usePlatformDefaultWidth = usePlatformDefaultWidth,
usePlatformInsets = true,
useSoftwareKeyboardInset = true,
scrimColor = DefaultScrimColor,
)

Expand All @@ -102,6 +106,7 @@ actual class DialogProperties @ExperimentalComposeUiApi constructor(
dismissOnClickOutside = dismissOnClickOutside,
usePlatformDefaultWidth = usePlatformDefaultWidth,
usePlatformInsets = true,
useSoftwareKeyboardInset = true,
scrimColor = DefaultScrimColor,
)

Expand All @@ -113,6 +118,7 @@ actual class DialogProperties @ExperimentalComposeUiApi constructor(
if (dismissOnClickOutside != other.dismissOnClickOutside) return false
if (usePlatformDefaultWidth != other.usePlatformDefaultWidth) return false
if (usePlatformInsets != other.usePlatformInsets) return false
if (useSoftwareKeyboardInset != other.useSoftwareKeyboardInset) return false
if (scrimColor != other.scrimColor) return false

return true
Expand All @@ -123,6 +129,7 @@ actual class DialogProperties @ExperimentalComposeUiApi constructor(
result = 31 * result + dismissOnClickOutside.hashCode()
result = 31 * result + usePlatformDefaultWidth.hashCode()
result = 31 * result + usePlatformInsets.hashCode()
result = 31 * result + useSoftwareKeyboardInset.hashCode()
result = 31 * result + scrimColor.hashCode()
return result
}
Expand Down Expand Up @@ -173,7 +180,7 @@ private fun DialogLayout(
onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)? = null,
content: @Composable () -> Unit
) {
val platformInsets = properties.insetsConfig.safeInsets
val platformInsets = properties.platformInsets
val layer = rememberComposeSceneLayer(
focusable = true
)
Expand All @@ -188,7 +195,10 @@ private fun DialogLayout(
containerSize = containerSize,
platformInsets = platformInsets
)
properties.insetsConfig.excludeSafeInsets {
PlatformInsetsConfig.excludeInsets(
safeInsets = properties.usePlatformInsets,
ime = properties.useSoftwareKeyboardInset,
) {
Layout(
content = content,
modifier = modifier,
Expand All @@ -198,16 +208,28 @@ private fun DialogLayout(
}
}

private val DialogProperties.platformInsets: PlatformInsets
@Composable get() {
val safeInsets = if (usePlatformInsets) {
PlatformInsetsConfig.safeInsets
} else {
PlatformInsets.Zero
}
val ime = if (useSoftwareKeyboardInset) {
PlatformInsetsConfig.ime
} else {
PlatformInsets.Zero
}
return safeInsets.union(ime)
}

@Composable
private fun rememberLayerContent(layer: ComposeSceneLayer, content: @Composable () -> Unit) {
remember(layer, content) {
layer.setContent(content)
}
}

private val DialogProperties.insetsConfig: InsetsConfig
get() = if (usePlatformInsets) PlatformInsetsConfig else ZeroInsetsConfig

@Composable
private fun rememberDialogMeasurePolicy(
layer: ComposeSceneLayer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import androidx.compose.ui.platform.LocalWindowInfo
import androidx.compose.ui.platform.PlatformInsets
import androidx.compose.ui.platform.PlatformInsetsConfig
import androidx.compose.ui.platform.ZeroInsetsConfig
import androidx.compose.ui.platform.union
import androidx.compose.ui.scene.ComposeSceneLayer
import androidx.compose.ui.scene.rememberComposeSceneLayer
import androidx.compose.ui.semantics.popup
Expand Down Expand Up @@ -433,7 +434,7 @@ private fun PopupLayout(
onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)? = null,
content: @Composable () -> Unit
) {
val platformInsets = properties.insetsConfig.safeInsets
val platformInsets = properties.platformInsets
var layoutParentBoundsInWindow: IntRect? by remember { mutableStateOf(null) }
EmptyLayout(Modifier.parentBoundsInWindow { layoutParentBoundsInWindow = it })
val layer = rememberComposeSceneLayer(
Expand All @@ -454,7 +455,10 @@ private fun PopupLayout(
layoutDirection = layoutDirection,
parentBoundsInWindow = parentBoundsInWindow
)
properties.insetsConfig.excludeSafeInsets {
PlatformInsetsConfig.excludeInsets(
safeInsets = properties.usePlatformInsets,
ime = false,
) {
Layout(
content = content,
modifier = modifier,
Expand All @@ -464,6 +468,13 @@ private fun PopupLayout(
}
}

private val PopupProperties.platformInsets: PlatformInsets
@Composable get() = if (usePlatformInsets) {
PlatformInsetsConfig.safeInsets
} else {
PlatformInsets.Zero
}

@Composable
private fun rememberLayerContent(layer: ComposeSceneLayer, content: @Composable () -> Unit) {
remember(layer, content) {
Expand All @@ -480,9 +491,6 @@ private fun EmptyLayout(modifier: Modifier = Modifier) = Layout(
}
)

private val PopupProperties.insetsConfig: InsetsConfig
get() = if (usePlatformInsets) PlatformInsetsConfig else ZeroInsetsConfig

private fun Modifier.parentBoundsInWindow(
onBoundsChanged: (IntRect) -> Unit
) = this.onGloballyPositioned { childCoordinates ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal enum class UIKitInteropState {
BEGAN, UNCHANGED, ENDED
}

internal enum class UIKitInteropViewHierarchyChange {
internal enum class UIKitInteropEvent {
VIEW_ADDED,
VIEW_REMOVED
}
Expand All @@ -38,7 +38,7 @@ internal interface UIKitInteropTransaction {
val state: UIKitInteropState

companion object {
val empty = object : UIKitInteropTransaction {
val empty: UIKitInteropTransaction = object : UIKitInteropTransaction {
override val actions: List<UIKitInteropAction>
get() = emptyList()

Expand Down Expand Up @@ -77,7 +77,7 @@ private class UIKitInteropMutableTransaction: UIKitInteropTransaction {

/**
* Class which can be used to add actions related to UIKit objects to be executed in sync with compose rendering,
* Addding deferred actions is threadsafe, but they will be executed in the order of their submission, and on the main thread.
* Adding deferred actions is thread-safe, but they will be executed in the order of their submission, and on the main thread.
*/
internal class UIKitInteropContext(
val requestRedraw: () -> Unit
Expand All @@ -86,9 +86,9 @@ internal class UIKitInteropContext(
private var transaction = UIKitInteropMutableTransaction()

/**
* Number of views, created by interop API and present in current view hierarchy
* Number of views whose changes are being synchronized with Compose rendering
*/
private var viewsCount = 0
private var synchronizedViewsCount = 0
set(value) {
require(value >= 0)

Expand All @@ -98,22 +98,22 @@ internal class UIKitInteropContext(
/**
* Add lambda to a list of commands which will be executed later in the same CATransaction, when the next rendered Compose frame is presented
*/
fun deferAction(hierarchyChange: UIKitInteropViewHierarchyChange? = null, action: () -> Unit) {
fun deferAction(event: UIKitInteropEvent? = null, action: () -> Unit) {
requestRedraw()

lock.doLocked {
if (hierarchyChange == UIKitInteropViewHierarchyChange.VIEW_ADDED) {
if (viewsCount == 0) {
if (event == UIKitInteropEvent.VIEW_ADDED) {
if (synchronizedViewsCount == 0) {
transaction.state = UIKitInteropState.BEGAN
}
viewsCount += 1
synchronizedViewsCount += 1
}

transaction.actions.add(action)

if (hierarchyChange == UIKitInteropViewHierarchyChange.VIEW_REMOVED) {
viewsCount -= 1
if (viewsCount == 0) {
if (event == UIKitInteropEvent.VIEW_REMOVED) {
synchronizedViewsCount -= 1
if (synchronizedViewsCount == 0) {
transaction.state = UIKitInteropState.ENDED
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,12 @@ fun <T : UIView> UIKitView(
interopContext.deferAction(action = it)
}

interopContext.deferAction(UIKitInteropViewHierarchyChange.VIEW_ADDED) {
interopContext.deferAction(UIKitInteropEvent.VIEW_ADDED) {
embeddedInteropComponent.addToHierarchy()
}

onDispose {
interopContext.deferAction(UIKitInteropViewHierarchyChange.VIEW_REMOVED) {
interopContext.deferAction(UIKitInteropEvent.VIEW_REMOVED) {
embeddedInteropComponent.removeFromHierarchy()
}
}
Expand Down Expand Up @@ -231,12 +231,12 @@ fun <T : UIViewController> UIKitViewController(
interopContext.deferAction(action = it)
}

interopContext.deferAction(UIKitInteropViewHierarchyChange.VIEW_ADDED) {
interopContext.deferAction(UIKitInteropEvent.VIEW_ADDED) {
embeddedInteropComponent.addToHierarchy()
}

onDispose {
interopContext.deferAction(UIKitInteropViewHierarchyChange.VIEW_REMOVED) {
interopContext.deferAction(UIKitInteropEvent.VIEW_REMOVED) {
embeddedInteropComponent.removeFromHierarchy()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.InternalComposeApi
import androidx.compose.runtime.staticCompositionLocalOf
import androidx.compose.ui.uikit.LocalKeyboardOverlapHeight
import androidx.compose.ui.unit.dp

/**
* Composition local for SafeArea of ComposeUIViewController
Expand All @@ -38,13 +40,22 @@ private object SafeAreaInsetsConfig : InsetsConfig {
override val safeInsets: PlatformInsets
@Composable get() = LocalSafeArea.current

override val ime: PlatformInsets
@Composable get() = PlatformInsets(bottom = LocalKeyboardOverlapHeight.current.dp)

@Composable
override fun excludeSafeInsets(content: @Composable () -> Unit) {
override fun excludeInsets(
safeInsets: Boolean,
ime: Boolean,
content: @Composable () -> Unit
) {
val safeArea = LocalSafeArea.current
val layoutMargins = LocalLayoutMargins.current
val keyboardOverlapHeight = LocalKeyboardOverlapHeight.current
CompositionLocalProvider(
LocalSafeArea provides PlatformInsets(),
LocalLayoutMargins provides layoutMargins.exclude(safeArea),
LocalSafeArea provides if (safeInsets) PlatformInsets() else safeArea,
LocalLayoutMargins provides if (safeInsets) layoutMargins.exclude(safeArea) else layoutMargins,
LocalKeyboardOverlapHeight provides if (ime) 0f else keyboardOverlapHeight,
content = content
)
}
Expand Down
Loading
Loading