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.

Sign up for GitHub

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

Jump to bottom

Fix text input popup position #339

Merged
merged 2 commits into from
Dec 5, 2022
Merged

Fix text input popup position #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
986c8fb
PlatformTextInputService. Undeprecate notifyFocusedRect
igordmn Dec 1, 2022
c532142
Keep @Deprecated until we don't merget this into AOSP
igordmn Dec 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 33 additions & 3 deletions ...dation/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/CoreTextField.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ internal fun CoreTextField(
textInputService,
state,
value,
imeOptions
imeOptions,
offsetMapping
)

// The focusable modifier itself will request the entire focusable be brought into view
Expand Down Expand Up @@ -392,6 +393,19 @@ internal fun CoreTextField(
state.showCursorHandle =
manager.isSelectionHandleInVisibleBound(isStartHandle = true)
}
state.layoutResult?.let { layoutResult ->
state.inputSession?.let { inputSession ->
TextFieldDelegate.notifyFocusedRect(
value,
state.textDelegate,
layoutResult.value,
it,
inputSession,
state.hasFocus,
offsetMapping
)
}
}
}
state.layoutResult?.innerTextFieldCoordinates = it
}
Expand Down Expand Up @@ -882,11 +896,13 @@ private fun tapToFocus(
}
}

@OptIn(InternalFoundationTextApi::class)
private fun notifyTextInputServiceOnFocusChange(
textInputService: TextInputService,
state: TextFieldState,
value: TextFieldValue,
imeOptions: ImeOptions
imeOptions: ImeOptions,
offsetMapping: OffsetMapping
) {
if (state.hasFocus) {
state.inputSession = TextFieldDelegate.onFocus(
Expand All @@ -896,7 +912,21 @@ private fun notifyTextInputServiceOnFocusChange(
imeOptions,
state.onValueChange,
state.onImeActionPerformed
)
).also { newSession ->
state.layoutCoordinates?.let { coords ->
state.layoutResult?.let { layoutResult ->
TextFieldDelegate.notifyFocusedRect(
value,
state.textDelegate,
layoutResult.value,
coords,
newSession,
state.hasFocus,
offsetMapping
)
}
}
}
} else {
onBlur(state)
}
Expand Down
54 changes: 54 additions & 0 deletions ...on/foundation/src/commonMain/kotlin/androidx/compose/foundation/text/TextFieldDelegate.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@
package androidx.compose.foundation.text

import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.geometry.Rect
import androidx.compose.ui.geometry.Size
import androidx.compose.ui.graphics.Canvas
import androidx.compose.ui.graphics.Paint
import androidx.compose.ui.layout.LayoutCoordinates
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.Paragraph
import androidx.compose.ui.text.SpanStyle
Expand Down Expand Up @@ -126,6 +129,57 @@ internal class TextFieldDelegate {
TextPainter.paint(canvas, textLayoutResult)
}

/**
* Notify system that focused input area.
*
* System is typically scrolled up not to be covered by keyboard.
*
* @param value The editor model
* @param textDelegate The text delegate
* @param layoutCoordinates The layout coordinates
* @param textInputSession The current input session.
* @param hasFocus True if focus is gained.
* @param offsetMapping The mapper from/to editing buffer to/from visible text.
*/
@JvmStatic
internal fun notifyFocusedRect(
value: TextFieldValue,
textDelegate: TextDelegate,
textLayoutResult: TextLayoutResult,
layoutCoordinates: LayoutCoordinates,
textInputSession: TextInputSession,
hasFocus: Boolean,
offsetMapping: OffsetMapping
) {
if (!hasFocus) {
return
}
val focusOffsetInTransformed = offsetMapping.originalToTransformed(value.selection.max)
val bbox = when {
focusOffsetInTransformed < textLayoutResult.layoutInput.text.length -> {
textLayoutResult.getBoundingBox(focusOffsetInTransformed)
}
focusOffsetInTransformed != 0 -> {
textLayoutResult.getBoundingBox(focusOffsetInTransformed - 1)
}
else -> { // empty text.
val defaultSize = computeSizeForDefaultText(
textDelegate.style,
textDelegate.density,
textDelegate.fontFamilyResolver
)
Rect(0f, 0f, 1.0f, defaultSize.height.toFloat())
}
}
val globalLT = layoutCoordinates.localToRoot(Offset(bbox.left, bbox.top))

// TODO remove `Deprecated`, if it is removed in AOSP repository
@Suppress("DEPRECATION")
textInputSession.notifyFocusedRect(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where/how can we observe the focus rect changes? Do we have an API for that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't found such API, but we can write a new one, if we need. Anyway, it seems if we write something like LocalFocusManager.focusedRect it will be not what we want. We want the rect for text input. And here we can get it.

Copy link
Collaborator

@eymar eymar Dec 1, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is that I see a few places which call notifyFocusedRect, but I don't understand which part actually "observers" the changes so input popup position can be set correctly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notifyFocusedRect subscribed on 2 things:

  • Modifier.onGloballyPositioned of the text field, which is called every time its repositioned relative to the window.
  • Modifier.onFocusChanged which triggers when the focus of text field changes.

I realized, that we don't subscribe on cursor changes. We rely on a side effect of onGloballyPositioned, which accidentally is called when we change the cursor position (but it seems, it shouldn't). I will investigate, should we also call notifyFocusedRect somewhere else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call it on relayout too now

Rect(Offset(globalLT.x, globalLT.y), Size(bbox.width, bbox.height))
)
}

/**
* Called when edit operations are passed from TextInputService
*
Expand Down
23 changes: 22 additions & 1 deletion compose/ui/ui-text/src/commonMain/kotlin/androidx/compose/ui/text/input/TextInputService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,20 @@ class TextInputSession(
}
}

@Suppress("DeprecatedCallableAddReplaceWith", "DEPRECATION")
/**
* Notify the focused rectangle to the system.
*
* The system can ignore this information or use it to show additional functionality near this rectangle.
*
* For example, desktop systems show a popup near the focused input area (for some languages).
*
* If the session is not open, no action will be performed.
*
* @param rect the rectangle that describes the boundaries on the screen that requires focus
* @return false if this session expired and no action was performed
*/
// TODO remove `Deprecated`, if it is removed in AOSP repository
@Suppress("DEPRECATION")
@Deprecated("This method should not be called, used BringIntoViewRequester instead.")
fun notifyFocusedRect(rect: Rect): Boolean = ensureOpenSession {
platformTextInputService.notifyFocusedRect(rect)
Expand Down Expand Up @@ -265,6 +278,14 @@ interface PlatformTextInputService {
*/
fun updateState(oldValue: TextFieldValue?, newValue: TextFieldValue)

/**
* Notify the focused rectangle to the system.
*
* The system can ignore this information or use it to show additional functionality near this rectangle.
*
* For example, desktop systems show a popup near the focused input area (for some languages).
*/
// TODO remove `Deprecated`, if it is removed in AOSP repository
@Deprecated("This method should not be called, used BringIntoViewRequester instead.")
fun notifyFocusedRect(rect: Rect) {
}
Expand Down