molecule icon indicating copy to clipboard operation
molecule copied to clipboard

Scheduler breaks keyboard behavior for Compose UI consumers

Open DSteve595 opened this issue 2 years ago • 17 comments

Compose UI's CoreTextField is very picky about getting immediate updates to its values. The common (but useless) example seen in the docs, where the text state is held in a by remember { mutableStateOf("") }, has no problems updating the text field when typing on the keyboard quickly. However, if you provide the text value from something that doesn't provide updates synchronously from onValueChange, the text field starts behaving erratically; characters randomly disappear, the cursor moves around in its own, and other weird stuff.

This issue made us realize that mapping UI state on a background thread will never work. So we've been running our presenter composition on the main thread, using an immediate monotonic frame clock:

private object ImmediateMonotonicFrameClock : MonotonicFrameClock {
  override suspend fun <R> withFrameNanos(onFrame: (frameTimeNanos: Long) -> R): R {
    return onFrame(System.nanoTime())
  }
}

This completely solved the keyboard glitchiness.

We're now looking to migrate to Molecule. But in doing so, we've seen the keyboard glitchiness return! I somewhat narrowed it down to behaviors that depend on the combination of the Molecule's Dispatcher and MonotonicFrameClock:

Dispatcher MonotonicFrameClock Behavior
AndroidUiDispatcher Choreographer Bad
AndroidUiDispatcher ImmediateMonotonicFrameClock Less bad (but still bad)
Dispatchers.Main Choreographer Bad
Dispatchers.Main ImmediateMonotonicFrameClock Good

More specifics on the glitchiness:

Try spamming a character quickly. Occasionally, some will be dropped. When that happens, this appears in logcat:

getSurroundingText on inactive InputConnection
beginBatchEdit on inactive InputConnection
getTextBeforeCursor on inactive InputConnection
getTextAfterCursor on inactive InputConnection
getSelectedText on inactive InputConnection
endBatchEdit on inactive InputConnection

An easy way to check if the issue is happening is to hold the backspace button when there's some text present. With the issue, the backspace will randomly "stop working"; characters will stop being deleted even though you're still pressing backspace.

Honestly, I'm not positive as to whether this is Molecule's or CoreTextField's fault. Input appreciated.

Repro project with those dispatcher/frameclock modes

DSteve595 avatar Nov 13 '21 07:11 DSteve595

Are you using the AndroidUiDispatcher built in to this library or sharing the one used by Compose UI?

JakeWharton avatar Nov 13 '21 22:11 JakeWharton

The one in this library.

DSteve595 avatar Nov 13 '21 22:11 DSteve595

I'll have to write a test or sample and play around

JakeWharton avatar Nov 13 '21 22:11 JakeWharton

I attached a sample project if it helps.

DSteve595 avatar Nov 13 '21 22:11 DSteve595

I'll have to write a test or sample and play around

@JakeWharton, you found something? (Just asking, I wanted to contribute, but don't have enough knowledge of internal working).

firefinchdev avatar Apr 09 '22 16:04 firefinchdev

I have no had a chance to look, no. I can try to look when I'm back working on this library as part of the Kotlin 1.6.20 upgrade to Compose.

JakeWharton avatar Apr 09 '22 23:04 JakeWharton

Depending on the Compose UI version, this can actually be quite nasty. On 1.2.0-rc02 textfields are unusable (the textfield fires two onValueChanged one after the other, the second one being with an empty string). For those running into this, downgrading to 1.1.1 with Steve's tricks above makes it usable... as long as the user doesn't type too fast :)

acristescu avatar Jun 28 '22 21:06 acristescu

Recent updates provide both good-ish and (probably?) bad news on this issue:

  1. With the introduction of RecompositionClock as a param, there's now support within Molecule for the immediate clock used above. Hooray-ish! (Your life is exactly the same as it was 😅 just with better support inside Molecule)
  2. We have used an immediate clock exactly like the one implemented by OP here internally for ages now (for testing). Things work, but every now and then we run into something.... weird. Double emissions, or a scenario where we don't get an emitted value. I discussed this approach with Adam Powell after merging the RecompositionClock changes, and his concerns led to PR #92.

The bad news is that the whole point of #92 is to let the current recomposition finish its business. The new implementation behaves so much better in every other way, but I'm concerned that it may result in the same behavior you're seeing.

jingibus avatar Jul 28 '22 00:07 jingibus

I put together a sample that allows for easy demoing of the issue and switching between the two clock types. RecompositionClock.Immediate does seem to fix the weird behaviour of input fields.

https://user-images.githubusercontent.com/1245751/182053296-14df5da3-e45b-4747-aa10-fd49e74f3a54.mp4

chris-horner avatar Aug 01 '22 00:08 chris-horner

I still got the issue using RecompositionClock.Immediate, but it's way less frequent than using RecompositionClock.ContextClock.

But I have a question: why should we go through launchMolecule method in a full compose app? We could just call our Presenter.present method in the Compose "View" and just use molecule for testing the Presenter.

ganfra avatar Jan 06 '23 09:01 ganfra

But I have a question: why should we go through launchMolecule method in a full compose app? We could just call our Presenter.present method in the Compose "View" and just use molecule for testing the Presenter.

Yep, that is correct.

If you want your business logic to have the lifetime of e.g. your ViewModel, though, and your activity undergoes frequent recreation for configuration changes (e.g. for rotation), you will still need to use Molecule to run composable business logic independently of Compose UI.

Google's guidance is to use Compose to handle rotation changes instead of relying on activity recreation, so you may not need to do that.

jingibus avatar Jan 10 '23 19:01 jingibus

Yes, was my thoughts, thanks for the confirmation!

ganfra avatar Jan 10 '23 19:01 ganfra

Running into this as well.

Our current workaround is to break the UI -> Presenter -> UI loop by keeping a local mutable state to hold the text (we still send an event to the presenter so it can update its internal state):

var localText by remember { mutableStateOf(initialText) }
TextField(
  value = localText,
  onValueChange = {
    localText = it
    presenter.handleEvent(TextUpdateEvent(it)) 
})

Definitely feels architecturally impure though.

prashanOS avatar Mar 08 '23 02:03 prashanOS

Actually, disregard the previous comment. Tested a bit more. Breaking the ui -> presenter -> ui loop is not necessary. Simply having a mutable state that you update within the onValueChange lambda is sufficient:

@Composable fun EditableText(
    text: String,
    onTextChange: (String) -> Unit,
  ) {
  var localText by remember(text) { mutableStateOf(text) }
  TextField(
    value = localText,
    onValueChange = {
      localText = it
      onTextChange(TextUpdateEvent(it)) 
  })
}

prashanOS avatar Mar 10 '23 02:03 prashanOS

I got curious about this behavior, and got inspired by watching "Reimagining text fields in Compose" by Zach Klippenstein https://www.droidcon.com/2023/07/20/reimagining-text-fields-in-compose/ to give it a shot to see how these two would play together.

After doing some trials on top of the repro made by Chris Horner, seems like TextFieldState (from BasicTextField2) is what will solve this problem for molecule without having to use the Immediate RecompositionMode.

Also no need to break the UI -> Presenter -> UI as prashanOS described. Nor do you have to have to have another state in the composable UI itself which would effectively create two sources of truth for that text.

This PR that I made here along with the video here shows that it all seems to work well together, while still allowing you to do other async work which doesn't affect TextFieldState which is also provided to the UI Composable though molecule

The gist is that inside molecule you can do val textFieldState = remember { TextFieldState("") } inside molecule's body composable and provide it inside your exposed Model directly.

StylianosGakis avatar Jul 29 '23 22:07 StylianosGakis

We were seeing this issue using even the immediate recomposition mode. Switching to use BasicTextField2 seems to resolve the weird interaction.

kevincianfarini avatar Apr 19 '24 20:04 kevincianfarini

Upon further investigation, we've settled on a new architecture that slightly disobeys unidirectional data flow to work around this. It turns out that BasicTextField2 encourages consumers of the API to hoist its TextFieldState into the presentation layer. Our team cannot do this since we share our presentation layer between iOS and Android. As a result, we've decided that asynchronously sending text field state to our presenters and then back to the text field is an anti-pattern.

Instead, we'll do a few things differently.

  1. Allow the text field to control its own state at the UI layer. Only once it requires action, for example when a submit button is pressed, do we observe the value of that text field and send it to our presentation layer for processing.
  2. The value of the text field may be continuously streamed to our presentation layer for auxiliary text field concerns only. Examples of this include text field validation and enabling/disabling form buttons based on the contents of the text fields.
  3. Initial values of the text field may be set from a presentation layer model, but once set the text field should maintain its own state in the UI layer until a further action like submitting a form is taken.

For other people encountering this issue in a multiplatform context, I hope our research helps you out.

kevincianfarini avatar Apr 25 '24 14:04 kevincianfarini