Skip to content

Commit

Permalink
Merge branch 'jb-main' into dima.avdeev/COMPOSE-478-iOS-SelectionCont…
Browse files Browse the repository at this point in the history
…ainer

# Conflicts:
#	compose/mpp/demo/src/uikitMain/kotlin/bugs/IosBugs.kt
  • Loading branch information
dima-avdeev-jb committed Oct 12, 2023
2 parents af70a0f + d0a8fe4 commit a92e0e4
Show file tree
Hide file tree
Showing 14 changed files with 212 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.State
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.drawWithCache
import androidx.compose.ui.geometry.Offset
Expand All @@ -35,6 +36,7 @@ import androidx.compose.ui.unit.LayoutDirection
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.Popup
import androidx.compose.ui.window.PopupPositionProvider
import androidx.compose.ui.window.PopupProperties
import kotlin.math.roundToInt

/**
Expand All @@ -52,6 +54,7 @@ private val RADIUS = 6.dp
*/
private val THICKNESS = 2.dp

@OptIn(ExperimentalComposeUiApi::class)
@Composable
internal actual fun SelectionHandle(
position: Offset,
Expand Down Expand Up @@ -90,7 +93,10 @@ internal actual fun SelectionHandle(
y = anchorBounds.top + positionState.value.y
)
}
}
},
properties = PopupProperties(
clippingEnabled = false,
),
) {
Spacer(
modifier.size((PADDING + RADIUS) * 2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ import androidx.compose.ui.window.ComposeUIViewController
// TODO This module is just a proxy to run the demo from mpp:demo. Figure out how to get rid of it.
// If it is removed, there is no available configuration in IDE
fun getViewControllerWithCompose() = ComposeUIViewController {
IosDemo()
IosDemo("")
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package androidx.compose.mpp.demo.textfield

import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.text.BasicTextField
import androidx.compose.material.OutlinedTextField
import androidx.compose.material.Text
import androidx.compose.material.TextField
Expand Down Expand Up @@ -67,6 +68,10 @@ val TextFields = Screen.Selection(
label = { Text("OutlinedTextField Label") },
)
}
},
Screen.Example("BasicTextField") {
var text by remember { mutableStateOf("usage of BasicTextField") }
BasicTextField(text, { text = it })
}
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@ import androidx.compose.runtime.*
import androidx.compose.ui.main.defaultUIKitMain
import androidx.compose.ui.window.ComposeUIViewController
import bugs.IosBugs
import bugs.StartRecompositionCheck


fun main() {
fun main(vararg args: String) {
val arg = args.firstOrNull() ?: ""
defaultUIKitMain("ComposeDemo", ComposeUIViewController {
IosDemo()
IosDemo(arg)
})
}

@Composable
fun IosDemo() {
fun IosDemo(arg: String) {
val app = remember {
App(
extraScreens = listOf(
Expand All @@ -24,5 +26,10 @@ fun IosDemo() {
)
)
}
app.Content()
when (arg) {
"demo=StartRecompositionCheck" ->
// The issue tested by this demo can be properly reproduced/tested only right after app start
StartRecompositionCheck.content()
else -> app.Content()
}
}
1 change: 1 addition & 0 deletions compose/mpp/demo/src/uikitMain/kotlin/bugs/IosBugs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ val IosBugs = Screen.Selection(
KeyboardPasswordType,
UIKitRenderSync,
DispatchersMainDelayCheck,
StartRecompositionCheck,
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright 2023 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package bugs

import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.height
import androidx.compose.material.Text
import androidx.compose.mpp.demo.Screen
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.unit.dp

private var counter = 0

// https://github.com/JetBrains/compose-multiplatform/issues/3778
//TODO: We should write autotest to check this sample: https://youtrack.jetbrains.com/issue/COMPOSE-488/iOS-test-on-CI-to-check-recompositions-count
val StartRecompositionCheck = Screen.Example("Start Recomposition Check") {
val value = remember(LocalDensity.current) {
(1..100).random()
}
println("Density: ${LocalDensity.current}")
println("value is $value")

Column {
Spacer(Modifier.height(64.dp))
Text("This demo should be launched using app arguments: `demo=StartRecompositionCheck`\n")
Text("Recompositions count = ${++counter} (should be 1)")
Text("Density = ${LocalDensity.current}")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package androidx.compose.ui.platform

import androidx.compose.ui.node.LayoutNode
import androidx.compose.ui.semantics.ProgressBarRangeInfo
import androidx.compose.ui.semantics.SemanticsNode
import androidx.compose.ui.semantics.SemanticsOwner
import androidx.compose.ui.semantics.SemanticsProperties
Expand Down Expand Up @@ -55,12 +56,13 @@ internal class AccessibilityControllerImpl(
}

@Suppress("UNUSED_PARAMETER")
fun fireNewNodeEvent(accessible: ComposeAccessible) {}
private fun onNodeAdded(accessible: ComposeAccessible) {}

@Suppress("UNUSED_PARAMETER")
fun fireRemovedNodeEvent(accessible: ComposeAccessible) {}
private fun onNodeRemoved(accessible: ComposeAccessible) {
accessible.removed = true
}

fun fireChangedNodeEvent(
private fun onNodeChanged(
component: ComposeAccessible,
previousSemanticsNode: SemanticsNode,
newSemanticsNode: SemanticsNode
Expand Down Expand Up @@ -119,6 +121,15 @@ internal class AccessibilityControllerImpl(
)
}
}

SemanticsProperties.ProgressBarRangeInfo -> {
val value = entry.value as ProgressBarRangeInfo
component.composeAccessibleContext.firePropertyChange(
ACCESSIBLE_VALUE_PROPERTY,
prev,
value.current
)
}
}
}
}
Expand Down Expand Up @@ -160,10 +171,10 @@ internal class AccessibilityControllerImpl(
nodes[currentNode.id] = previous[currentNode.id]?.let {
val prevSemanticsNode = it.semanticsNode
it.semanticsNode = currentNode
fireChangedNodeEvent(it, prevSemanticsNode, currentNode)
onNodeChanged(it, prevSemanticsNode, currentNode)
it
} ?: ComposeAccessible(currentNode, this).also {
fireNewNodeEvent(it)
onNodeAdded(it)
}

// TODO fake nodes?
Expand All @@ -178,7 +189,7 @@ internal class AccessibilityControllerImpl(
findAllSemanticNodesRecursive(rootSemanticNode)
for ((id, prevNode) in previous.entries) {
if (nodes[id] == null) {
fireRemovedNodeEvent(prevNode)
onNodeRemoved(prevNode)
}
}
_currentNodes = nodes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,15 @@ internal class ComposeAccessible(

val composeAccessibleContext: ComposeAccessibleComponent by lazy { ComposeAccessibleComponent() }

override fun getAccessibleContext(): AccessibleContext {
var removed = false

override fun getAccessibleContext(): AccessibleContext? {
if (removed) {
// The accessibility system keeps calling functions on the context even after the node
// has been removed. We return null so it doesn't do that.
return null
}

// see doc for [nativeInitializeAccessible] for details, why this initialization is needed
if (isNativelyInitialized.compareAndSet(false, true)) {
nativeInitializeAccessible(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ package androidx.compose.ui.window

import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.ui.node.Ref
import androidx.compose.ui.util.UpdateEffect
import kotlinx.coroutines.DelicateCoroutinesApi
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.launch
import org.jetbrains.skiko.MainUIDispatcher
import java.awt.Window
Expand All @@ -41,8 +39,8 @@ import java.awt.Window
*
* The [update] block can be run multiple times (on the UI thread as well) due to recomposition,
* and it is the right place to set [Window] properties depending on state.
* When state changes, the block will be reexecuted to set the new properties.
* Note the block will also be ran once right after the [create] block completes.
* When state changes, the block will be re-executed to set the new properties.
* Note the block will also be run once right after the [create] block completes.
*
* [AwtWindow] is needed for creating window's / dialog's that still can't be created with
* the default Compose functions [androidx.compose.ui.window.Window] or
Expand All @@ -65,8 +63,6 @@ fun <T : Window> AwtWindow(
dispose: (T) -> Unit,
update: (T) -> Unit = {}
) {
val currentVisible by rememberUpdatedState(visible)

val windowRef = remember { Ref<T>() }
fun window() = windowRef.value!!

Expand All @@ -82,45 +78,38 @@ fun <T : Window> AwtWindow(
update(window)
}

val showJob = Ref<Job?>()

SideEffect {
DisposableEffect(visible) {
// Why we dispatch showing in the next AWT tick:
//
// 1.
// window.isVisible = true can be a blocking operation.
// So we have to schedule it when we will be outside of Compose render frame.
// So we have to schedule it outside the Compose render frame.
//
// This happens in the when we create a modal dialog.
// When we call `window.isVisible = true`, internally will be created a new AWT event loop,
// This happens in when we create a modal dialog.
// When we call `window.isVisible = true`, internally a new AWT event loop will be created,
// which will handle all the future Swing events while dialog is visible.
//
// We can't use LaunchedEffect or rememberCoroutineScope, because they have a dispatcher
// which is controlled by the Compose rendering loop (ComposeScene.dispatcher) and we
// will block coroutine.
// We can't show the window directly inside LaunchedEffect or rememberCoroutineScope because
// their dispatcher is controlled by the Compose rendering loop (ComposeScene.dispatcher)
// and we will block the coroutine.
//
// 2.
// We achieve the correct order when we open nested
// window at the same time when we open the parent window. If we would show the window
// immediately we will have this sequence in case of nested Window's:
// We achieve the correct order when we open nested window at the same time when we open the
// parent window. If we had shown the window immediately we would have had this sequence in
// case of nested windows:
//
// 1. window1.setContent
// 2. window2.setContent
// 3. window2.isVisible = true
// 4. window1.isVisible = true
//
// So we will have a wrong active window (window1).

showJob.value?.cancel()
showJob.value = GlobalScope.launch(MainUIDispatcher) {
window().isVisible = currentVisible
// So we will have the wrong window active (window1).
val showJob = GlobalScope.launch(MainUIDispatcher) {
window().isVisible = visible
}
}

DisposableEffect(Unit) {
onDispose {
showJob.value?.cancel()
window().isVisible = false
showJob.cancel()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class AccessibilityTest {
}

val node = rule.onNodeWithTag("text").fetchSemanticsNode()
val accessibleContext = ComposeAccessible(node).accessibleContext
val accessibleContext = ComposeAccessible(node).accessibleContext!!
val accessibleText = accessibleContext.accessibleText!!
assertEquals(22, accessibleText.charCount)

Expand Down Expand Up @@ -115,7 +115,7 @@ class AccessibilityTest {
}

rule.onNodeWithTag("progressbar").apply {
val context = ComposeAccessible(fetchSemanticsNode()).accessibleContext
val context = ComposeAccessible(fetchSemanticsNode()).accessibleContext!!
val value = context.accessibleValue
?: fail("No accessibleValue on LinearProgressIndicator")

Expand All @@ -124,11 +124,10 @@ class AccessibilityTest {
assertThat(value.maximumAccessibleValue).isEqualTo(1f)
assertThat(value.currentAccessibleValue).isEqualTo(0.2f)
}

}

private fun SemanticsNodeInteraction.assertHasAccessibleRole(role: AccessibleRole) {
val accessibleContext = ComposeAccessible(fetchSemanticsNode()).accessibleContext
val accessibleContext = ComposeAccessible(fetchSemanticsNode()).accessibleContext!!
assertThat(accessibleContext.accessibleRole).isEqualTo(role)
}

Expand Down
Loading

0 comments on commit a92e0e4

Please sign in to comment.