IssueTracker

Project: platform/frameworks/support
Branch: androidx-main
Author: Louis Pullen-Freilich <lpf@google.com>
Link:      https://android-review.googlesource.com/3328184

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling

Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling 
 
These APIs allow overscroll to have events dispatched to it by one component, and rendered in a separate component. 
 
Fixes: b/266550551 
Fixes: b/204650733 
Fixes: b/255554340 
Fixes: b/229537244 
Test: OverscrollTest 
Relnote: "Adds OverscrollEffect#withoutDrawing and OverscrollEffect#withoutEventHandling APIs - these APIs create a wrapped instance of the provided overscroll effect that doesn't draw / handle events respectively, which allows for rendering  overscroll in a separate component from the component that is dispatching events. For example, disabling drawing the overscroll inside a lazy list, and then drawing the overscroll separately on top / elsewhere." 
Change-Id: Idbb3d91546b49c1987a041f959bce4b2b09a9f61

The following release(s) address this bug.It is possible this bug has only been partially addressed:

• androidx.compose.foundation:foundation:1.8.0-alpha06
• androidx.compose.foundation:foundation-android:1.8.0-alpha06
• androidx.compose.foundation:foundation-jvmstubs:1.8.0-alpha06
• androidx.compose.foundation:foundation-linuxx64stubs:1.8.0-alpha06

It's kind of a crazy convoluted workaround, but this seems to work very well for me:

@Composable
fun AppScreen() {
    val coroutineScope = rememberCoroutineScope()

    var value by remember { mutableStateOf(TextFieldValue()) }
    val scrollState = rememberScrollState()
    var height by remember { mutableStateOf(0) }
    var layoutResult: TextLayoutResult? by remember { mutableStateOf(null) }

    BasicTextField(
        value = value,
        onValueChange = { value = it },
        decorationBox = {
            Box(modifier = Modifier.verticalScroll(scrollState)) {
                it()
            }
        },
        onTextLayout = { layoutResult = it },
        textStyle = LocalTextStyle.current.copy(color = MaterialTheme.colors.onBackground),
        cursorBrush = SolidColor(MaterialTheme.colors.secondary),
        modifier = Modifier
            .fillMaxSize()
            .onSizeChanged {
                coroutineScope.launch {
                    val cursorInView = value.isCursorInView(
                        layoutResult = layoutResult!!,
                        height = it.height.toFloat(),
                        scrollValue = scrollState.value.toFloat()
                    )
                    if (!cursorInView && height > it.height) {
                        scrollState.scrollBy(
                            value.calculateRequiredSizeScroll(
                                layoutResult = layoutResult!!,
                                oldHeight = height.toFloat(),
                                newHeight = it.height.toFloat(),
                                scrollValue = scrollState.value.toFloat()
                            )
                        )
                    }
                    height = it.height
                }
            }
    )

    LaunchedEffect(value.selection) {
        val cursorInView = value.isCursorInView(
            layoutResult = layoutResult!!,
            height = height.toFloat(),
            scrollValue = scrollState.value.toFloat()
        )
        if (!cursorInView) {
            scrollState.scrollBy(
                value.calculateRequiredSelectionScroll(
                    layoutResult = layoutResult!!,
                    height = height.toFloat(),
                    scrollValue = scrollState.value.toFloat()
                )
            )
        }
    }
}

fun TextFieldValue.isCursorInView(
    layoutResult: TextLayoutResult,
    height: Float,
    scrollValue: Float
) = with(layoutResult) {
    val currentLine = try {
        getLineForOffset(selection.min)
    } catch (ex: IllegalArgumentException) {
        System.err.println("Corrected Wrong Offset!")
        getLineForOffset(selection.min - 1)
    }
    val lineBottom = getLineBottom(currentLine)
    val lineTop = getLineTop(currentLine)
    lineBottom <= height + scrollValue && lineTop >= scrollValue
}

fun TextFieldValue.calculateRequiredSelectionScroll(
    layoutResult: TextLayoutResult,
    height: Float,
    scrollValue: Float
) = with(layoutResult) {
    val currentLine = try {
        getLineForOffset(selection.min)
    } catch (ex: IllegalArgumentException) {
        System.err.println("Corrected Wrong Offset!")
        getLineForOffset(selection.min - 1)
    }
    val lineTop = getLineTop(currentLine)
    val lineBottom = getLineBottom(currentLine)
    if (lineTop < scrollValue) -(scrollValue - lineTop)
    else if (lineBottom > height + scrollValue) lineBottom - (height + scrollValue)
    else 0f
}

fun TextFieldValue.calculateRequiredSizeScroll(
    layoutResult: TextLayoutResult,
    oldHeight: Float,
    newHeight: Float,
    scrollValue: Float
) = with(layoutResult) {
    val currentLine = try {
        getLineForOffset(selection.min)
    } catch (ex: IllegalArgumentException) {
        System.err.println("Corrected Wrong Offset!")
        getLineForOffset(selection.min - 1)
    }
    val sizeDifference = oldHeight - newHeight
    val lineBottom = getLineBottom(currentLine)
    if (lineBottom in (newHeight + scrollValue)..(oldHeight + scrollValue))
        sizeDifference - (oldHeight - (lineBottom - scrollValue))
    else 0f
}

I basically wrapped the inner text field Composable in a Box with the verticalScroll modifier, and then using these parameters: TextLayoutResult (to get the current line position), scroll value, and height; all to determine if the cursor is currently visible on the screen, and if it's not, calculate the minimal amount of scroll needed to get it back on the screen, both when the TextField size changes (which is the goal), and also when navigating around the text or editing it (this works fine in Compose by default, but using a scroll modifier breaks it, so I have to manually scroll to follow the cursor).

It works perfectly well for me, with a small annoyance being that sometimes (when typing really fast) the selection I get from the TextFieldValue is higher than what the TextLayoutResult expects, causing a crash. I worked around this by wrapping it in a try/catch, and decrementing the selection value by 1 to get it working. Pretty hacky and lazy, but it works flawlessly now, and those catched exceptions don't seem to affect performance in any noticeable way.

Yet having something like this implemented properly would still be far better than these hacky and convoluted workarounds.

Hi again! Any updates on this? Seems like there's some progress happening, is any version available for us to test with the fixed behavior?

Releases are generally paused during most of December and resume again in January. The release including these fixes should be coming out in the next few days, if everything goes according to plan.

Thanks, I'll be waiting :D

My bad, this particular bug is still not fixed in 1.4.0-alpha04 – the case where the field takes up all available space. The cases where it doesn't are fixed. No timeline to share yet for when this case will be fixed.

Ah that's alright, it's pretty relieving to see progress as issues with TextFields and scrolling have been seeing slow progress for quite some time.

Thanks for all the efforts! Hope 1.4.0-alpha05 fixes this one.

Bug is not solved in 1.4.0-alpha05

Yep, they have no timeline for when it's going to be fixed :p

The above workaround is super inefficient for large texts, as using verticalScroll renders the entirety of the BasicTextField instead of only the visible lines (which i guess is the default behavior of BasicTextField's internal scrolling), causing a lot of lag in scrolling.

At least having something like b/238368987 for now may help making any workaround work efficiently instead of destroying performance.

Yeah it kinda sucks. They seem to be doing some architectural changes to TextFields for 1.5, so fingers crossed for that.

For anyone still experiencing this, the workaround I wrote in #comment4 is working pretty well in two production apps I have on the store, without any issues. You may need to adjust the code a bit for some use-cases, but it generally works out-of-the box, at least until a proper fix is released.

Hi! The new BasicTextField2 in Compose 1.6 alphas seems really promising. For now it seems like the issue of staying on top of the keyboard while typing is fixed; however, on a long text, selecting the text in the area where the keyboard is about to show doesn't scroll the text field to keep the cursor in view.

Once that gets fixed, this issue will be fully resolved and can be effectively closed!

Any updates or ETAs on that?

Any new developments on this issue? I'm using BasicTextField to implement a rich edit window (it can attach images, etc.), but it won't keep the text above the keyboard if I add Modifier.verticalScrollable() :(

which I think is still pretty important

Looks promising! Does that mean this is finally fixed? And when can we expect this to be released so we can test it?

It will be fixed in BasicTextField2 in the next 1.7.0 alpha release, alpha02. We are not planning to fix it in the current BasicTextField.

Great news finally!

This is a bit unrelated, but will the name of the BasicTextField2 API be always like this? Or will it eventually replace BasicTextField?

Because with the 2 it sounds like this "secondary" thing, not like the primary API that everyone should be using.

Other than that, thanks for the amazing work!

Nope. That and other questions answered here: https://www.droidcon.com/2023/10/06/reimagining-text-fields-in-compose-2/

Great to hear! Thanks for sharing the talk, and props to you guys for this amazing work!

#comment26 You should probably file a separate issue for that, as it seems to be unrelated to this one.

Okay I just played with the new BasicTextField... and it seems like this has been only half-fixed.

Inputting anything from the keyboard brings the cursor into focus, which is great. Adding new lines keeps the cursor in view, which is great.

However, tapping the text field (which opens the IME) in an area that will eventually be under the keyboard, does not scroll the cursor to stay in view.

My wacky workaround at the very beginning of this issue does that correctly, and not only for the keyboard, but for any size changes (my app has, in addition to system keyboard, an internal keyboard, and it works the exact same way). If the cursor is in view, I keep it in view no matter what. If the cursor has been scrolled out-of-view, then it's okay to bring it in view only once there's input from the user.

Is this behavior intended? I'm not 100% sure, but I believe that Android's EditText was keeping the cursor in view once the IME is shown.

I can make a video demonstration with all of these (New BasicTextField, legacy BasicTextField, my workaround, and EditText)

#comment28 If you're talking about the cursor not scrolling to stay on top of keyboard when it opens first, I agree, this is relevant here. I just noticed it myself as well.

In terms of the keyboard not showing at all the second time you click the text field, I think that's a different bug, and should be reported as its own issue with all the context of where and how that happens to you.

Also note that BasicTextField2 from 1.6.x releases is outdated, so make sure to use the new BasicTextField in 1.7.0 betas.

To comment #29 I believe you need to handle ime insets for that to work.

#comment31 Yes, I did apply the imePadding() Modifier on the BasicTextField, and it doesn't work even with it.

This isn't even IME-specific; any change in the TextField's size should try to keep the cursor in the viewport (if it was already visible before the size change).

imePadding does exactly just that: add padding, which in turn shrinks the size of the BasicTextField from the bottom. Currently, the new BasicTextField does not react to this at all, thus causing the cursor to stay under the keyboard.

My workaround (from #comment4) does this correctly, making sure that the cursor stays in view on size changes (only if the cursor was already in view, if not, do nothing because something else may have caused the size change).

It is still, however, a workaround (a wacky one to be clear, I'm not even sure I understand my own code, but it works flawlessly in production), and there's nothing than a proper upstream fix for everybody, that would be more performant and just less wacky.

#comment33 Again, the BasicTextField2 that's in Compose 1.6.x is outdated.

If you want to try the latest API, update to Compose 1.7.0-beta03 (which is the latest beta as of now), and use BasicTextField (which now uses the new implementation). This issue is mostly fixed in 1.7.

bringIntoViewRequester is irrelevant here, because we're talking about the internal scroll of the text field (especially in the case when it takes full screen space). It's not about scrolling some LazyList to keep the field in view, it's about scrolling the field itself to keep the cursor in view.

Again, if you update to 1.7.0 and still face the issue of the keyboard not opening the second time, that's a different issue, please report it separately.

#comment35 It's... partially fixed. I guess you still have to do Modifier.imePadding(), but the cursor will still come into view... once you type something.

However, since I still have to use the older BasicTextField sometimes because I need AnnotatedString, and because the current fix doesn't satisfy me, I just use a workaround for now, using BringIntoViewRequester. It's much simpler, cleaner and easier to understand than the abomination I wrote in #comment4, and is generally more flexible.

To keep the cursor in view on text changes (like the current fix), you need to add a bringIntoViewRequester to your BasicTextField, and then get use the TextLayoutResult from onTextLayout to get the bounding of the current cursor position, and call request bringIntoView using that Rect.

To complete the fix, you'll wanna store the TextLayoutResult in a state, and add a LaunchedEffect on TextFieldValue.selection/TextFieldState.selection changes, which does the exact same thing as before (calling bringIntoView with cursor rect).

Do the same on size changes (in Modifier.onPlaced), and you're fully set. The cursor will always be brought to view on size/selection/text changes, while still allowing users to scroll it away when none of these change, works well on desktop, and you can of course choose which triggers to keep.

You can also add some padding to the Rect you pass to bringIntoView, for example to keep some space between the line where the cursor is and the IME/bottom/top of screen.

I generally use this alongside a verticalScroll modifier to apply some internal non-clipping padding, but it should work fine without it, at least on the new BasicTextField.