-
Notifications
You must be signed in to change notification settings - Fork 154
Add checks for menu position #3290
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
base: jb-main
Are you sure you want to change the base?
Changes from 15 commits
166cb71
0246231
305e513
53f339a
4b94900
b77b423
9f76bf1
88ef9f9
a189f91
b32d072
1186c32
f08eb0b
34414f6
35c4e7b
fae57c5
31a456d
d801335
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,12 +18,18 @@ package androidx.compose.ui.interaction | |
|
|
||
| import androidx.compose.foundation.ComposeFoundationFlags | ||
| import androidx.compose.foundation.ExperimentalFoundationApi | ||
| import androidx.compose.foundation.background | ||
| import androidx.compose.foundation.gestures.awaitEachGesture | ||
| import androidx.compose.foundation.layout.Box | ||
| import androidx.compose.foundation.layout.Column | ||
| import androidx.compose.foundation.layout.fillMaxSize | ||
| import androidx.compose.foundation.layout.height | ||
| import androidx.compose.foundation.layout.padding | ||
| import androidx.compose.foundation.layout.safeDrawingPadding | ||
| import androidx.compose.foundation.layout.size | ||
| import androidx.compose.foundation.layout.width | ||
| import androidx.compose.foundation.text.BasicTextField | ||
| import androidx.compose.foundation.text.KeyboardOptions | ||
| import androidx.compose.foundation.text.contextmenu.builder.item | ||
| import androidx.compose.foundation.text.contextmenu.modifier.appendTextContextMenuComponents | ||
| import androidx.compose.foundation.text.contextmenu.modifier.filterTextContextMenuComponents | ||
|
|
@@ -40,10 +46,13 @@ import androidx.compose.ui.Modifier | |
| import androidx.compose.ui.focus.FocusRequester | ||
| import androidx.compose.ui.focus.focusRequester | ||
| import androidx.compose.ui.focus.onFocusChanged | ||
| import androidx.compose.ui.graphics.Color | ||
| import androidx.compose.ui.input.pointer.PointerEventPass | ||
| import androidx.compose.ui.input.pointer.changedToDown | ||
| import androidx.compose.ui.input.pointer.changedToUp | ||
| import androidx.compose.ui.input.pointer.pointerInput | ||
| import androidx.compose.ui.layout.boundsInWindow | ||
| import androidx.compose.ui.layout.onGloballyPositioned | ||
| import androidx.compose.ui.platform.testTag | ||
| import androidx.compose.ui.test.UIKitInstrumentedTest | ||
| import androidx.compose.ui.test.assertVisibleInContainer | ||
|
|
@@ -53,11 +62,19 @@ import androidx.compose.ui.test.findNodeWithTag | |
| import androidx.compose.ui.test.runUIKitInstrumentedTest | ||
| import androidx.compose.ui.test.tapContextMenuButton | ||
| import androidx.compose.ui.test.utils.findFirstDescendant | ||
| import androidx.compose.ui.test.utils.horizontalDistanceTo | ||
| import androidx.compose.ui.test.utils.isLoupeView | ||
| import androidx.compose.ui.test.utils.union | ||
| import androidx.compose.ui.test.utils.up | ||
| import androidx.compose.ui.test.utils.verticalDistanceTo | ||
| import androidx.compose.ui.test.waitForContextMenu | ||
| import androidx.compose.ui.text.TextLayoutResult | ||
| import androidx.compose.ui.text.TextRange | ||
| import androidx.compose.ui.text.input.PlatformImeOptions | ||
| import androidx.compose.ui.text.input.TextFieldValue | ||
| import androidx.compose.ui.unit.Density | ||
| import androidx.compose.ui.unit.DpRect | ||
| import androidx.compose.ui.unit.toDpRect | ||
| import androidx.compose.ui.unit.dp | ||
| import kotlin.test.Ignore | ||
| import kotlin.test.Test | ||
|
|
@@ -247,6 +264,102 @@ class TextFieldEditMenuTest { | |
| findNodeWithLabel("Paste").assertVisibleInContainer() | ||
| } | ||
|
|
||
| @Test | ||
| @Ignore // CMP-10315: Context menu is positioned far from the caret for BTF1. | ||
| fun testBasicTextFieldContextMenuIsPositionedNearCaret() = | ||
| runTextFieldContextMenuPositionTest(EditableTextFieldKind.BasicTextField) | ||
|
|
||
| @Test | ||
| fun testBasicTextField2ContextMenuIsPositionedNearCaret() = | ||
| runTextFieldContextMenuPositionTest(EditableTextFieldKind.BasicTextField2) | ||
|
|
||
| private fun runTextFieldContextMenuPositionTest(textFieldKind: EditableTextFieldKind) { | ||
| for (newContextMenuEnabled in arrayOf(false, true)) { | ||
| runContextMenuTest(newContextMenuEnabled) { | ||
| UIPasteboard.generalPasteboard().string = "Paste text" | ||
| val layoutInfo = setOffsetTextFieldContent(textFieldKind) | ||
|
|
||
| waitUntil("Text field should be laid out") { | ||
| layoutInfo.textFieldFrame != null && layoutInfo.textLayoutResult != null | ||
| } | ||
|
|
||
| longPressNodeWithTagAndAwaitContextMenu("TextField") | ||
|
|
||
| assertContextMenuNearCaret( | ||
| caretFrame = layoutInfo.caretFrameInWindow(density), | ||
| textFieldKind = textFieldKind, | ||
| newContextMenuEnabled = newContextMenuEnabled | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun UIKitInstrumentedTest.setOffsetTextFieldContent( | ||
| textFieldKind: EditableTextFieldKind, | ||
| ): TextFieldLayoutInfo { | ||
| val text = "I am a TextField" | ||
| val keyboardOptions = KeyboardOptions( | ||
| platformImeOptions = PlatformImeOptions { | ||
| usingNativeTextInput(false) | ||
| } | ||
| ) | ||
| val focusRequester = FocusRequester() | ||
|
|
||
| fun offsetTextFieldModifier(layoutInfo: TextFieldLayoutInfo): Modifier = | ||
| Modifier | ||
| .width(160.dp) | ||
| .height(24.dp) | ||
| .onGloballyPositioned { coordinates -> | ||
| layoutInfo.textFieldFrame = coordinates.boundsInWindow().toDpRect(density) | ||
| } | ||
| .then(textFieldModifier(focusRequester)) | ||
|
|
||
| val initialSelection = TextRange(text.length, text.length) | ||
| val textFieldValue = mutableStateOf(TextFieldValue(text, initialSelection)) | ||
| val textFieldState = TextFieldState(text, initialSelection) | ||
|
|
||
| val layoutInfo = when (textFieldKind) { | ||
| EditableTextFieldKind.BasicTextField -> TextFieldLayoutInfo( | ||
| selectionOffset = { textFieldValue.value.selection.start } | ||
| ) | ||
| EditableTextFieldKind.BasicTextField2 -> TextFieldLayoutInfo( | ||
| selectionOffset = { textFieldState.selection.start } | ||
| ) | ||
| } | ||
|
|
||
| setContent { | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .background(Color.White) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can be omitted
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| .safeDrawingPadding() | ||
| .padding(start = 80.dp, top = 48.dp) | ||
| ) { | ||
| when (textFieldKind) { | ||
| EditableTextFieldKind.BasicTextField -> BasicTextField( | ||
| value = textFieldValue.value, | ||
| onValueChange = { textFieldValue.value = it }, | ||
| modifier = offsetTextFieldModifier(layoutInfo), | ||
| keyboardOptions = keyboardOptions, | ||
| onTextLayout = { layoutInfo.textLayoutResult = it } | ||
| ) | ||
| EditableTextFieldKind.BasicTextField2 -> BasicTextField( | ||
| state = textFieldState, | ||
| modifier = offsetTextFieldModifier(layoutInfo), | ||
| keyboardOptions = keyboardOptions, | ||
| onTextLayout = { getResult -> | ||
| layoutInfo.textLayoutResult = getResult() | ||
| } | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| focusRequester.requestFocus() | ||
| waitForIdle() | ||
| return layoutInfo | ||
| } | ||
|
|
||
| @Test | ||
| fun testBasicTextField2LongPressShowsContextMenu() = runUIKitInstrumentedTest { | ||
| UIPasteboard.generalPasteboard().string = "Paste text" | ||
|
|
@@ -757,6 +870,67 @@ class TextFieldEditMenuTest { | |
| waitForContextMenu() | ||
| } | ||
|
|
||
| private fun UIKitInstrumentedTest.assertContextMenuNearCaret( | ||
| caretFrame: DpRect, | ||
| textFieldKind: EditableTextFieldKind, | ||
| newContextMenuEnabled: Boolean, | ||
| ) { | ||
| val menuFrame = findContextMenuItemsFrame() | ||
| val horizontalDistance = menuFrame.horizontalDistanceTo(caretFrame) | ||
| val verticalDistance = menuFrame.verticalDistanceTo(caretFrame) | ||
| val contextMenu = if (newContextMenuEnabled) "new menu" else "old menu" | ||
| val maxDistance = 16.dp | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: better to extract into constant
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
|
||
| assertTrue( | ||
| horizontalDistance <= maxDistance, | ||
| "Context menu is horizontally more than $maxDistance from caret. " + | ||
| "textFieldKind: $textFieldKind, contextMenu: $contextMenu, " + | ||
| "horizontal distance: $horizontalDistance." | ||
| ) | ||
| assertTrue( | ||
| verticalDistance <= maxDistance, | ||
| "Context menu is vertically more than $maxDistance from caret. " + | ||
| "textFieldKind: $textFieldKind, contextMenu: $contextMenu, " + | ||
| "vertical distance: $verticalDistance." | ||
| ) | ||
| } | ||
|
|
||
| private fun UIKitInstrumentedTest.findContextMenuItemsFrame(): DpRect { | ||
| // Paste is always present in the menu, so its visibility means the menu is shown | ||
| verifyContextMenuItemsVisible(listOf("Paste")) | ||
|
|
||
| val pasteFrame = findNodeWithLabel("Paste").frame!! | ||
| val itemFrames = listOf("Select", "Select All").mapNotNull { label -> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the purpose is to find a menu frame and assert that it is not far away from the caret, why not reuse finding
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| findNodeWithLabelOrNull(label)?.frame | ||
| } | ||
|
|
||
| return pasteFrame.union(itemFrames) | ||
| } | ||
|
|
||
| private fun TextLayoutResult.cursorFrameInWindow( | ||
| textFieldFrame: DpRect, | ||
| offset: Int, | ||
| density: Density, | ||
| ): DpRect { | ||
| val cursorFrame = getCursorRect(offset).toDpRect(density) | ||
| return DpRect( | ||
| left = textFieldFrame.left + cursorFrame.left, | ||
| top = textFieldFrame.top + cursorFrame.top, | ||
| right = textFieldFrame.left + cursorFrame.right, | ||
| bottom = textFieldFrame.top + cursorFrame.bottom | ||
| ) | ||
| } | ||
|
|
||
| private fun TextFieldLayoutInfo.caretFrameInWindow(density: Density): DpRect { | ||
| val textFieldFrame = textFieldFrame ?: error("TextField frame is null") | ||
| val textLayoutResult = textLayoutResult ?: error("TextLayoutResult is null") | ||
| return textLayoutResult.cursorFrameInWindow( | ||
| textFieldFrame = textFieldFrame, | ||
| offset = selectionOffset(), | ||
| density = density | ||
| ) | ||
| } | ||
|
|
||
| private fun UIKitInstrumentedTest.setTextFieldContent( | ||
| textFieldKind: EditableTextFieldKind, | ||
| initialValue: TextFieldValue, | ||
|
|
@@ -800,6 +974,13 @@ class TextFieldEditMenuTest { | |
| BasicTextField2 | ||
| } | ||
|
|
||
| private class TextFieldLayoutInfo( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's possible to wait, then I suggest waiting for that PR to merge, I've made an API for that there It would be also possible to use UIKitInstrumentedTest.setTextFieldContent (line 934)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, thanks, let's wait |
||
| val selectionOffset: () -> Int, | ||
| ) { | ||
| var textFieldFrame: DpRect? = null | ||
| var textLayoutResult: TextLayoutResult? = null | ||
| } | ||
|
|
||
| private companion object { | ||
| private const val PARTIAL_SELECTION_TEXT = "accomplishment extraordinary magnificent establishment" | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: it's default value, not needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done