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.

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

BasicTextField selection collapses to a zero span at the cursor location when focus is lost #2569

Closed
kirill-grouchnikov opened this issue Dec 21, 2022 · 10 comments
Labels
bug Something isn't working desktop

Comments

@kirill-grouchnikov
Copy link

Here is the sample to reproduce this on 1.2.0 and 1.3.0-rc01:

import androidx.compose.foundation.layout.*
import androidx.compose.foundation.text.BasicTextField
import androidx.compose.material.Button
import androidx.compose.material.Text
import androidx.compose.runtime.*
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.TextRange
import androidx.compose.ui.text.input.TextFieldValue
import androidx.compose.ui.unit.DpSize
import androidx.compose.ui.unit.ExperimentalUnitApi
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.*

@ExperimentalUnitApi
fun main() = application {
    val state = rememberWindowState(
        placement = WindowPlacement.Floating,
        position = WindowPosition.Aligned(Alignment.Center),
        size = DpSize(600.dp, 400.dp)
    )

    Window(
        title = "Demo",
        state = state,
        onCloseRequest = ::exitApplication
    ) {
        SampleStyles()
    }
}


@ExperimentalUnitApi
@Composable
fun SampleStyles() {
    var textSelectionRange by remember { mutableStateOf(TextRange.Zero) }
    println("Range $textSelectionRange")

    Column(modifier = Modifier.fillMaxSize()) {
        Row(modifier = Modifier.fillMaxWidth()) {
            Button(onClick = {}) {
                Text("Sample")
            }
        }
        BasicTextField(
            value = TextFieldValue(
                text = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor",
                selection = textSelectionRange
            ),
            onValueChange = {
                println("Changing range to ${it.selection}")
                textSelectionRange = it.selection
            },
            modifier = Modifier.fillMaxWidth().fillMaxHeight(fraction = 1.0f)
        )
    }
}

Run the app. Select a range in the text - it will print the selected range in the console. Now click the button at the top of the window and look at the updated range printed in the console.

JetBrains/compose-multiplatform-core@ca869b1#diff-42592036468eac046f04d9f25ac21f134ed22d3b55e602da3c56bb0c9f206a7c may be the change that caused this behavior change. Note that version 1.1.0 and the Android variant do not lose the selection on focus loss.

In Swing, by the way, selection is not lost on focus loss. It is not drawn by default, and there is a workaround to keep the selection visible:

            textPane.setCaret(new DefaultCaret() {
                @Override
                public void setSelectionVisible(boolean visible) {
                    super.setSelectionVisible(true);
                }
            });

The target use case is a simple text editor that allows toggling bold, italic, underline, strikethrough, font size, font family on the selected span. Think a text pane with a strip of buttons on the side to initiate those actions.

@eymar eymar added desktop bug Something isn't working regression labels Dec 22, 2022
@eymar
Copy link
Collaborator

eymar commented Dec 22, 2022

Reproduced. To be precise, this is the change that causing such a behaviour:
JetBrains/compose-multiplatform-core@ca869b1#diff-42592036468eac046f04d9f25ac21f134ed22d3b55e602da3c56bb0c9f206a7cL563

- .focusableInNonTouchMode(enabled = enabled, interactionSource = interactionSource)
+ .focusable(enabled = enabled, interactionSource = interactionSource)

It was intentional, but apparently it didn't consider the use case like you described. Thank you! We'll need to come up with some better logic.
cc: @igordmn

@igordmn
Copy link
Collaborator

igordmn commented Dec 22, 2022

I checked this code, changing the focus programmatically, not pressing the button:

import androidx.compose.foundation.focusable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxHeight
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.text.BasicTextField
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusRequester
import androidx.compose.ui.focus.focusRequester
import androidx.compose.ui.text.TextRange
import androidx.compose.ui.text.input.TextFieldValue
import androidx.compose.ui.window.Window
import androidx.compose.ui.window.application
import kotlinx.coroutines.delay

fun main() = application {
    Window(onCloseRequest = ::exitApplication) {
        var textSelectionRange by remember { mutableStateOf(TextRange.Zero) }
        println("Range $textSelectionRange")

        val focusRequester = remember { FocusRequester() }
        Box(Modifier.focusRequester(focusRequester).focusable())

        BasicTextField(
            value = TextFieldValue(
                text = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor",
                selection = textSelectionRange
            ),
            onValueChange = {
                println("Changing range to ${it.selection}")
                textSelectionRange = it.selection
            },
            modifier = Modifier.fillMaxWidth().fillMaxHeight(fraction = 1.0f)
        )

        LaunchedEffect(Unit) {
            delay(2000)
            focusRequester.requestFocus()
        }
    }
}

In Compose 1.1.1 and Compose 1.2.2 it works the same - we lose selection in the text field.

So it is not a regression. In Compose 1.2.0 clickable just have a new feature on desktop - clickable components receive focus on click.

It also not a bug, because it programmed intentionally to clear selection when we lose focus.

Maybe it is not right to clear selection from UX perspective or platform guidelines perspective, but we need to investigate that. If we decide to not clear selection - we probably should change the color of the selection (usually it is gray). Also it is not obvious how will 2 text fields behave. Should they have its own selection? Maybe we need to introduce something like focus groups, that can have their own independent focus inside the group.

@igordmn
Copy link
Collaborator

igordmn commented Dec 22, 2022

The target use case is a simple text editor that allows toggling bold, italic, underline, strikethrough, font size, font family on the selected span

It looks like those are icon buttons in a toolbar, and we need to not switch focus to them, when we click on them 🤔. I.e. they shouldn't be focusable. @eymar, I think, we either need to rethink our approach to make all clickable's focusable (make only some widgets), or provide a way to disable focusability.

@igordmn
Copy link
Collaborator

igordmn commented Dec 22, 2022

provide a way to disable focusability

One of the current workarounds - don't use clickable, or IconButton, use Box + onClick

@eymar eymar removed the regression label Dec 22, 2022
@kirill-grouchnikov
Copy link
Author

From the UX perspective, I don't think it would be the right move to disable focus on such toolbar buttons or comboboxes. I'd still want them to be fully traversible.

I would rather prefer to explicitly mark the text field (read only or not) to not lose selection when it loses focus.

@kirill-grouchnikov
Copy link
Author

As an example, see IntelliJ. You can select a block in the editor tab, then start Search / Search & Replace and use tab to traverse the text fields and buttons in that bar without losing the selection in the editor.

@eymar
Copy link
Collaborator

eymar commented Feb 2, 2023

One more report for this issue: #2687

@SalomonBrys
Copy link

SalomonBrys commented Dec 28, 2023

It's been a year since this issue has been reported. Any progress ?
I can confirm this issue is still present in Compose 1.5.11.
This is a real problem for desktop apps that manipulate text inside text fields.

@SalomonBrys
Copy link

SalomonBrys commented Dec 28, 2023

Waiting for a fix, I found the folowing workaround.
I haven't found any undesirable side effect, but that doesn't mean there is none 😅.

var value: TextFieldValue by remember { mutableStateOf(TextFieldValue()) }
var previousSelection by remember { mutableStateOf(TextRange(0)) }
BasicTextField(
    value = value,
    onValueChange = {
        previousSelection = value.selection
        value = it
    },
    modifier = Modifier
        .onFocusChanged {
            if (!previousSelection.collapsed && state.value.selection.max == previousSelection.max) {
                value = value.copy(selection = previousSelection)
            }
        }
)

@okushnikov
Copy link
Collaborator

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working desktop
Projects
None yet
Development

No branches or pull requests

5 participants