Skip to content

Commit

Permalink
Fix text input popup position (#339)
Browse files Browse the repository at this point in the history
* PlatformTextInputService. Undeprecate notifyFocusedRect

Fixes JetBrains/compose-multiplatform#2040
Fixes JetBrains/compose-multiplatform#2493

notifyFocusedRect was deprecated in https://android-review.googlesource.com/c/platform/frameworks/support/+/1959647

On Android, before deprecation this method was used to scroll to the focused text input.
This functionality was extracted to the text field itself, but we had another functionality on the other platforms.
On Desktop we show text input popup near text input (for some languages):
JetBrains/compose-multiplatform#2040 (comment)

I think that this method is the right choice to implement this functionality, but I am not completely sure. Here my thoughts about alternatives:

1. Use BringIntoViewRequester. Not sure that it is possible, because its purpose - to show the view to the user, not use the passed information to determine the text popup position
2. Get information about focused area from another source that is not related to text input. For example, we can inject FocusManager, and retrieve the focused rect from it. Probably not a good idea, because the rect of arbitrary focused node isn't the same thing as the rect of focused input area.

* Keep @deprecated until we don't merget this into AOSP
  • Loading branch information
igordmn committed Nov 14, 2023
1 parent 33dbf24 commit 291134e
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -293,17 +293,13 @@ internal fun CoreTextField(
state.hasFocus = it.isFocused

if (textInputService != null) {
if (state.hasFocus && enabled && !readOnly) {
startInputSession(
textInputService,
state,
value,
imeOptions,
offsetMapping
)
} else {
endInputSession(state)
}
notifyTextInputServiceOnFocusChange(
textInputService,
state,
value,
imeOptions,
offsetMapping
)

// The focusable modifier itself will request the entire focusable be brought into view
// when it gains focus – in this case, that's the decoration box. However, since text
Expand Down Expand Up @@ -424,7 +420,19 @@ internal fun CoreTextField(
state.showCursorHandle =
manager.isSelectionHandleInVisibleBound(isStartHandle = true)
}
notifyFocusedRect(state, value, offsetMapping)
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 @@ -969,22 +977,39 @@ private fun tapToFocus(
}

@OptIn(InternalFoundationTextApi::class)
private fun startInputSession(
private fun notifyTextInputServiceOnFocusChange(
textInputService: TextInputService,
state: TextFieldState,
value: TextFieldValue,
imeOptions: ImeOptions,
offsetMapping: OffsetMapping
) {
state.inputSession = TextFieldDelegate.onFocus(
textInputService,
value,
state.processor,
imeOptions,
state.onValueChange,
state.onImeActionPerformed
)
notifyFocusedRect(state, value, offsetMapping)
if (state.hasFocus) {
state.inputSession = TextFieldDelegate.onFocus(
textInputService,
value,
state.processor,
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)
}
}

private fun endInputSession(state: TextFieldState) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ internal class TextFieldDelegate {
/**
* 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
Expand Down Expand Up @@ -172,6 +174,8 @@ internal class TextFieldDelegate {
}
val globalLT = layoutCoordinates.localToRoot(Offset(bbox.left, bbox.top))

// TODO remove `Deprecated`, if it is removed in AOSP repository
@Suppress("DEPRECATION")
textInputSession.notifyFocusedRect(
Rect(Offset(globalLT.x, globalLT.y), Size(bbox.width, bbox.height))
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class TextInputSession(
/**
* Notify the focused rectangle to the system.
*
* The system can ignore this information or use it to for additional functionality.
* 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).
*
Expand All @@ -169,6 +169,9 @@ class TextInputSession(
* @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 @@ -278,11 +281,12 @@ interface PlatformTextInputService {
/**
* Notify the focused rectangle to the system.
*
* The system can ignore this information or use it to for additional functionality.
* 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(b/262648050) Try to find a better API.
// TODO remove `Deprecated`, if it is removed in AOSP repository
@Deprecated("This method should not be called, used BringIntoViewRequester instead.")
fun notifyFocusedRect(rect: Rect) {
}
}

0 comments on commit 291134e

Please sign in to comment.