DropdownMenu offset broken [294103942]

Fixed

Bug

Status Update

No update yet.

Description

ja...@gmail.com

created issue #1

Aug 1, 2023 08:26PM

Jetpack Compose version:
compose-bom = "2023.06.01"
compose-compiler = "1.5.1"

Jetpack Compose component(s) used:
DropdownMenu

Android Studio Build:
Android Studio Giraffe | 2022.3.1

Kotlin version:
1.9.0

Emulator version:
33.1.6

Emulated device:
Pixel 4 (33 api) + google services

Steps to Reproduce or Code Sample to Reproduce:
With i use y offset value from -127.dp to -231.dp DropdownMenu draw in incorrect location
Sample

```
import android.util.Log
import androidx.compose.foundation.clickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.material.DropdownMenu
import androidx.compose.material.DropdownMenuItem
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.DpOffset
import androidx.compose.ui.unit.dp

@Composable
private fun Test() {
var expanded by remember {
mutableStateOf(false)
}
var yOffset by remember {
mutableStateOf((-125).dp)
}
Box(
modifier = Modifier
.fillMaxSize()
.clickable {
yOffset -= 1.dp
expanded = true
},
contentAlignment = Alignment.Center,
) {
Text(text = "Current offset: $yOffset")
val menuOffset = remember(yOffset) {
DpOffset(200.dp, yOffset).also {
Log.d("TestOffset", "OFFSET = $it")
}
}
DropdownMenu(
expanded = expanded,
offset = menuOffset,
onDismissRequest = { expanded = false },
) {
DropdownMenuItem(onClick = { /*TODO*/ }) {
Text(text = "Item 1")
}
DropdownMenuItem(onClick = { /*TODO*/ }) {
Text(text = "Item 2")
}
DropdownMenuItem(onClick = { /*TODO*/ }) {
Text(text = "Item 3")
}
}
}

}

@Preview
@Composable
fun TestPreview() {
Test()
}

```

DropdownMenu issue.webm

1.5 MB

Download

Comments

ti...@google.com <ti...@google.com> Aug 2, 2023 01:10AM

Assigned to je...@google.com.

co...@google.com <co...@google.com> Aug 2, 2023 03:01PM

Reassigned to ma...@google.com.

ma...@google.com <ma...@google.com> Aug 2, 2023 07:31PM

Accepted by ma...@google.com.

ap...@google.com <ap...@google.com> #2Aug 9, 2023 12:38AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ma...@google.com <ma...@google.com> Aug 9, 2023 12:43AM

Accepted by ma...@google.com.

ja...@gmail.com <ja...@gmail.com> #3Aug 9, 2023 09:08AM

This issue also present in androidx.compose.material package

ap...@google.com <ap...@google.com> #4Aug 10, 2023 06:19PM

Project: platform/frameworks/support
Branch: androidx-main

commit cdd989b146a4758f9974b63c2e53e4eb767e961e
Author: Max Alfonso-Ying <maxying@google.com>
Date: Wed Aug 09 19:42:40 2023

[Menu] Fix offset calculations

Same logic as aosp/2681138, ported to the M2 version.

Bug:

b/294103942

b/293137081

Test: updated MenuTest.kt
Relnote: "Fixed DropdownMenu's `offset` calculation so x offsets
depend solely on the local layout direction, and y offsets will no
longer be reversed when the menu is near the bottom of the screen."
Change-Id: Iccc743f3306d9b259f0cc21d1089d9479df203fb

M compose/material/material/src/androidAndroidTest/kotlin/androidx/compose/material/MenuTest.kt
M compose/material/material/src/androidMain/kotlin/androidx/compose/material/AndroidMenu.android.kt
M compose/material/material/src/commonMain/kotlin/androidx/compose/material/Menu.kt
M compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/AndroidMenu.android.kt

https://android-review.googlesource.com/2701713

ma...@google.com <ma...@google.com> Aug 10, 2023 06:21PM

Marked as fixed.

vi...@gmail.com <vi...@gmail.com> #5Aug 18, 2024 08:24AM

Since offset.y is now always added, what is the recommendation for handling the offset when the popup is top aligned? Is there a mechanism to know that so that I can apply a negative offset to shift it up?