Fix menu closing on soft keyboard input
A change in API 31+ means that the x and y of ACTION_OUTSIDE
MotionEvents are incorrectly reported as (-width, -height)
instead of (0, 0) for touches in other windows. We switch to
using rawX and rawY instead, which are still correct.
Also cleans up some unused fields in PopupLayout.
Bug: b/238331998
Test: Added in ExposedDropdownMenuTest
Change-Id: Ie5d465fd98797df5870d18f2c450e5be22bfb55b
diff --git a/compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/ExposedDropdownMenuTest.kt b/compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/ExposedDropdownMenuTest.kt
index 24bde2e..95357cf 100644
--- a/compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/ExposedDropdownMenuTest.kt
+++ b/compose/material3/material3/src/androidAndroidTest/kotlin/androidx/compose/material3/ExposedDropdownMenuTest.kt
@@ -54,8 +54,11 @@
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.MediumTest
import androidx.test.platform.app.InstrumentationRegistry
+import androidx.test.uiautomator.By
import androidx.test.uiautomator.UiDevice
import com.google.common.truth.Truth.assertThat
+import org.junit.Assume.assumeNotNull
+import org.junit.Ignore
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
@@ -75,7 +78,7 @@
private val OptionName = "Option 1"
@Test
- fun expandedBehaviour_expandsOnClickAndCollapsesOnOutside() {
+ fun edm_expandsOnClick_andCollapsesOnClickOutside() {
var textFieldBounds = Rect.Zero
rule.setMaterialContent(lightColorScheme()) {
var expanded by remember { mutableStateOf(false) }
@@ -106,7 +109,7 @@
}
@Test
- fun expandedBehaviour_collapseOnTextFieldClick() {
+ fun edm_collapsesOnTextFieldClick() {
rule.setMaterialContent(lightColorScheme()) {
var expanded by remember { mutableStateOf(true) }
ExposedDropdownMenuForTest(
@@ -126,7 +129,39 @@
}
@Test
- fun expandedBehaviour_expandsAndFocusesTextFieldOnTrailingIconClick() {
+ fun edm_doesNotCollapse_whenTypingOnSoftKeyboard() {
+ rule.setMaterialContent(lightColorScheme()) {
+ var expanded by remember { mutableStateOf(false) }
+ ExposedDropdownMenuForTest(
+ expanded = expanded,
+ onExpandChange = { expanded = it }
+ )
+ }
+
+ rule.onNodeWithTag(TFTag).performClick()
+
+ rule.onNodeWithTag(TFTag).assertIsDisplayed()
+ rule.onNodeWithTag(TFTag).assertIsFocused()
+ rule.onNodeWithTag(EDMTag).assertIsDisplayed()
+ rule.onNodeWithTag(MenuItemTag).assertIsDisplayed()
+
+ val device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation())
+ val zKey = device.findObject(By.desc("z")) ?: device.findObject(By.text("z"))
+ // Only run the test if we can find a key to type, which might fail for any number of
+ // reasons (keyboard doesn't appear, unexpected locale, etc.)
+ assumeNotNull(zKey)
+
+ repeat(3) {
+ zKey.click()
+ rule.waitForIdle()
+ }
+
+ rule.onNodeWithTag(TFTag).assertTextContains("zzz")
+ rule.onNodeWithTag(MenuItemTag).assertIsDisplayed()
+ }
+
+ @Test
+ fun edm_expandsAndFocusesTextField_whenTrailingIconClicked() {
rule.setMaterialContent(lightColorScheme()) {
var expanded by remember { mutableStateOf(false) }
ExposedDropdownMenuForTest(
@@ -146,7 +181,7 @@
}
@Test
- fun expandedBehaviour_doesNotExpandIfTouchEndsOutsideBounds() {
+ fun edm_doesNotExpand_ifTouchEndsOutsideBounds() {
var textFieldBounds = Rect.Zero
rule.setMaterialContent(lightColorScheme()) {
var expanded by remember { mutableStateOf(false) }
@@ -184,7 +219,7 @@
}
@Test
- fun expandedBehaviour_doesNotExpandIfTouchIsPartOfScroll() {
+ fun edm_doesNotExpand_ifTouchIsPartOfScroll() {
val testIndex = 2
var textFieldSize = IntSize.Zero
rule.setMaterialContent(lightColorScheme()) {
@@ -268,7 +303,7 @@
}
@Test
- fun uiProperties_menuMatchesTextWidth() {
+ fun edm_widthMatchesTextFieldWidth() {
var textFieldBounds by mutableStateOf(Rect.Zero)
var menuBounds by mutableStateOf(Rect.Zero)
rule.setMaterialContent(lightColorScheme()) {
@@ -294,7 +329,7 @@
}
@Test
- fun EDMBehaviour_rightOptionIsChosen() {
+ fun edm_collapsesWithSelection_whenMenuItemClicked() {
rule.setMaterialContent(lightColorScheme()) {
var expanded by remember { mutableStateOf(true) }
ExposedDropdownMenuForTest(
@@ -314,8 +349,9 @@
rule.onNodeWithTag(TFTag).assertTextContains(OptionName)
}
+ @Ignore("b/266109857")
@Test
- fun doesNotCrashWhenAnchorDetachedFirst() {
+ fun edm_doesNotCrash_whenAnchorDetachedFirst() {
var parent: FrameLayout? = null
rule.setContent {
AndroidView(
@@ -323,12 +359,20 @@
FrameLayout(context).apply {
addView(ComposeView(context).apply {
setContent {
- Box {
- ExposedDropdownMenuBox(expanded = true, onExpandedChange = {}) {
- Box(
- Modifier
- .menuAnchor()
- .size(20.dp))
+ ExposedDropdownMenuBox(expanded = true, onExpandedChange = {}) {
+ TextField(
+ value = "Text",
+ onValueChange = {},
+ modifier = Modifier.menuAnchor().size(20.dp),
+ )
+ ExposedDropdownMenu(
+ expanded = true,
+ onDismissRequest = {},
+ ) {
+ DropdownMenuItem(
+ text = { Text(OptionName) },
+ onClick = {},
+ )
}
}
}
@@ -349,7 +393,7 @@
@OptIn(ExperimentalMaterial3Api::class)
@Test
- fun withScrolledContent() {
+ fun edm_withScrolledContent() {
rule.setMaterialContent(lightColorScheme()) {
Box(Modifier.fillMaxSize()) {
ExposedDropdownMenuBox(
diff --git a/compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/internal/ExposedDropdownMenuPopup.kt b/compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/internal/ExposedDropdownMenuPopup.kt
index d18f90d..6900df5 100644
--- a/compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/internal/ExposedDropdownMenuPopup.kt
+++ b/compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/internal/ExposedDropdownMenuPopup.kt
@@ -32,7 +32,6 @@
import androidx.compose.runtime.CompositionContext
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.SideEffect
-import androidx.compose.runtime.compositionLocalOf
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
@@ -85,7 +84,6 @@
) {
val view = LocalView.current
val density = LocalDensity.current
- val testTag = LocalPopupTestTag.current
val layoutDirection = LocalLayoutDirection.current
val parentComposition = rememberCompositionContext()
val currentContent by rememberUpdatedState(content)
@@ -93,10 +91,9 @@
val popupLayout = remember {
PopupLayout(
onDismissRequest = onDismissRequest,
- testTag = testTag,
composeView = view,
+ positionProvider = popupPositionProvider,
density = density,
- initialPositionProvider = popupPositionProvider,
popupId = popupId
).apply {
setContent(parentComposition) {
@@ -121,7 +118,6 @@
popupLayout.show()
popupLayout.updateParameters(
onDismissRequest = onDismissRequest,
- testTag = testTag,
layoutDirection = layoutDirection
)
onDispose {
@@ -134,17 +130,10 @@
SideEffect {
popupLayout.updateParameters(
onDismissRequest = onDismissRequest,
- testTag = testTag,
layoutDirection = layoutDirection
)
}
- DisposableEffect(popupPositionProvider) {
- popupLayout.positionProvider = popupPositionProvider
- popupLayout.updatePosition()
- onDispose {}
- }
-
// TODO(soboleva): Look at module arrangement so that Box can be
// used instead of this custom Layout
// Get the parent's position, size and layout direction
@@ -167,11 +156,6 @@
}
}
-// TODO(b/139861182): This is a hack to work around Popups not using Semantics for test tags
-// We should either remove it, or come up with an abstracted general solution that isn't specific
-// to Popup
-internal val LocalPopupTestTag = compositionLocalOf { "DEFAULT_TEST_TAG" }
-
// TODO(soboleva): Look at module dependencies so that we can get code reuse between
// Popup's SimpleStack and Box.
@Suppress("NOTHING_TO_INLINE")
@@ -214,21 +198,18 @@
@SuppressLint("ViewConstructor")
private class PopupLayout(
private var onDismissRequest: (() -> Unit)?,
- var testTag: String,
private val composeView: View,
+ private val positionProvider: PopupPositionProvider,
density: Density,
- initialPositionProvider: PopupPositionProvider,
popupId: UUID
) : AbstractComposeView(composeView.context),
ViewRootForInspector,
ViewTreeObserver.OnGlobalLayoutListener {
+
private val windowManager =
composeView.context.getSystemService(Context.WINDOW_SERVICE) as WindowManager
private val params = createLayoutParams()
- /** The logic of positioning the popup relative to its parent. */
- var positionProvider = initialPositionProvider
-
// Position params
var parentLayoutDirection: LayoutDirection = LayoutDirection.Ltr
var parentBounds: IntRect? by mutableStateOf(null)
@@ -247,15 +228,6 @@
override val subCompositionView: AbstractComposeView get() = this
- // Specific to exposed dropdown menus.
- private val dismissOnOutsideClick = { offset: Offset?, bounds: IntRect ->
- if (offset == null) false
- else {
- offset.x < bounds.left || offset.x > bounds.right ||
- offset.y < bounds.top || offset.y > bounds.bottom
- }
- }
-
init {
id = android.R.id.content
setViewTreeLifecycleOwner(composeView.findViewTreeLifecycleOwner())
@@ -304,9 +276,7 @@
content()
}
- /**
- * Taken from PopupWindow
- */
+ // Taken from PopupWindow. Calls [onDismissRequest] when back button is pressed.
override fun dispatchKeyEvent(event: KeyEvent): Boolean {
if (event.keyCode == KeyEvent.KEYCODE_BACK) {
if (keyDispatcherState == null) {
@@ -329,11 +299,9 @@
fun updateParameters(
onDismissRequest: (() -> Unit)?,
- testTag: String,
layoutDirection: LayoutDirection
) {
this.onDismissRequest = onDismissRequest
- this.testTag = testTag
superSetLayoutDirection(layoutDirection)
}
@@ -383,28 +351,18 @@
// matter whether we return true or false as some upper layer decides on whether the
// event is propagated to other windows or not. So for focusable the event is consumed but
// for not focusable it is propagated to other windows.
- if (
- (
- (event.action == MotionEvent.ACTION_DOWN) &&
- (
- (event.x < 0) ||
- (event.x >= width) ||
- (event.y < 0) ||
- (event.y >= height)
- )
- ) ||
- event.action == MotionEvent.ACTION_OUTSIDE
+ if (event.action == MotionEvent.ACTION_OUTSIDE ||
+ (event.action == MotionEvent.ACTION_DOWN &&
+ (event.x < 0 || event.x >= width || event.y < 0 || event.y >= height))
) {
val parentBounds = parentBounds
val shouldDismiss = parentBounds == null || dismissOnOutsideClick(
- if (event.x != 0f || event.y != 0f) {
- Offset(
- params.x + event.x,
- params.y + event.y
- )
- } else null,
+ // Keep menu open if ACTION_OUTSIDE event is reported as raw coordinates of (0, 0).
+ // This means it belongs to another owner, e.g., the soft keyboard or other window.
+ if (event.rawX != 0f && event.rawY != 0f) Offset(event.rawX, event.rawY) else null,
parentBounds
)
+
if (shouldDismiss) {
onDismissRequest?.invoke()
return true
@@ -427,6 +385,15 @@
super.setLayoutDirection(direction)
}
+ // Specific to exposed dropdown menus.
+ private fun dismissOnOutsideClick(offset: Offset?, bounds: IntRect): Boolean =
+ if (offset == null) {
+ false
+ } else {
+ offset.x < bounds.left || offset.x > bounds.right ||
+ offset.y < bounds.top || offset.y > bounds.bottom
+ }
+
/**
* Initialize the LayoutParams specific to [android.widget.PopupWindow].
*/
