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

In input controls, cursor sporadically jumps to the end of text while typing #220

Closed
cata opened this issue Jul 16, 2021 · 10 comments · Fixed by #223
Closed

In input controls, cursor sporadically jumps to the end of text while typing #220

cata opened this issue Jul 16, 2021 · 10 comments · Fixed by #223

Comments

@cata
Copy link
Contributor

cata commented Jul 16, 2021

Repro steps (successfully reproduced in in Chrome, Edge, FireFox):

  1. Open and run https://try.websharper.com/example/todo-list (handy as it is already available, but any input would do)
  2. Type some text in the input, e.g. END
  3. Move the cursor to the beginning of the text
  4. Type rapidly, with brief breaks (I find that typing random numbers works for me, as I am a slow typist)

Result: The cursor jumps at the end of the text, resulting in incorrect input.

The issue seems to be caused by the logic used to update the input value, which incorrectly assumes that a change in Var value was caused by external factors, when in fact:

  • the Var value changed as result of user input
  • by the time the the view mapping function is called to perform the update check, the input value has also changed due to additional user input.

See video below:

Try.Websharper.and.9.more.pages.-.Personal.-.Microsoft.Edge.2021-07-15.17-32-31.mp4
@granicz
Copy link
Member

granicz commented Jul 18, 2021

@cata Thanks for the report! I have seen this at random as well, hoping that @Tarmil has ideas on how to fix it.

@amieres
Copy link

amieres commented Jul 19, 2021

I have seen this a lot, and not only with Websharper. It happens when there is significant processing happening between keystrokes.
I have a complex solution that works, but there are probably better options.

/// binds an Editor with a Var<string> to avoid annoying jumps to the end when fast typing
/// onChange gets called when the editor changes but not when the var changes
let bindVarEditor setEvent getVal setVal onChange (var:Var<_>) =
    let editorChanged = ref 0L
    let varChanged    = ref 0L
    setEvent(fun _ ->
        let v = getVal() 
        if var.Value <> v then editorChanged := !editorChanged + 1L; var.Value <- v; onChange v 
    )
    var.View |> View.Sink (fun _ ->
        if  !editorChanged > !varChanged then varChanged := !editorChanged
        elif getVal() <> var.Value then setVal var.Value
    )

@Tarmil
Copy link
Member

Tarmil commented Jul 22, 2021

A possible lead would be to add to Var the ability to track the originator (such as a DOM element) of a change that is currently being propagated. This way, the element could see that it is itself the originator of a change, and not set its value in this case.

@cata
Copy link
Contributor Author

cata commented Jul 24, 2021

@Tarmil I think it might be possible to avoid enhancing Var

FWIW, here's a rather inelegant "user land" approach that employs a mutable to keep track of the last known input value (helper functions included for completeness):

module Var = 
    let setIfChanged x v = if x <> Var.Get v then Var.Set v x

module Doc = 

    let Sink f view = Doc.BindView (fun v -> f v |> ignore; Doc.Empty) view

    let Input attrs var =

        let mutable expectedValue = ""

        let onChange el _ =
            expectedValue <- el?value
            var |> Var.setIfChanged expectedValue

        let changeHandlers = [
            on.change onChange
            on.input onChange
            on.keyPress onChange ]

        let input = Elt.input (Seq.append attrs changeHandlers) []

        let updateElement v =
            if v <> expectedValue
            then input.Dom?value <- v

        Doc.Concat [ input; Sink updateElement var.View ]

@cata cata closed this as completed Jul 24, 2021
@cata
Copy link
Contributor Author

cata commented Jul 24, 2021

Sorry, our CI system closed this by mistake when we implemented a temporary workaround in our project. Reopening.

@cata cata reopened this Jul 24, 2021
@Tarmil
Copy link
Member

Tarmil commented Jul 24, 2021

Okay, so here's something interesting. I tried your solution, and it seems to be working correctly; I have never seen the cursor jump with it.

However, when I change it slightly to use Attr.DynamicCustom rather than Doc.BindView like this:

    let Input3 attrs var =

        let mutable expectedValue = ""

        let onChange el _ =
            expectedValue <- el?value
            var |> Var.setIfChanged expectedValue

        let updateElement el v =
            if v <> expectedValue
            then el?value <- v

        let changeHandlers = [
            Attr.DynamicCustom updateElement var.View
            on.change onChange
            on.input onChange
            on.keyPress onChange ]

        Elt.input (Seq.append attrs changeHandlers) []

then I can see the cursor jump again. So it looks like there is actually a bug somewhere in the propagation of Views to Attrs.

@cata
Copy link
Contributor Author

cata commented Jul 24, 2021

@Tarmil That is interesting!

When I implemented the workaround I introduced the check against expectedValue as a blanket measure to address the possibility of el?value having changed between the calls to onChange and updateElement. I did not consider the possibility of a bug in the View -> Attr propagation path.

Your finding made me wonder if the workaround I posted would function without the mutable (since the salient change may have been in fact avoiding the View -> Attr propagation).

So, I tried removing expectedValue and it seems this also works correctly (in that I was not able to reproduce cursor jumps in Chrome/Edge/Firefox):

    let Input attrs var =

        let onChange el _ =
            var |> Var.setIfChanged expectedValue

        let changeHandlers = [
            on.change onChange
            on.input onChange
            on.keyPress onChange ]

        let input = Elt.input (Seq.append attrs changeHandlers) []

        let updateElement v =
            if v <> input.Dom?value 
            then input.Dom?value <- v

        Doc.Concat [ input; Sink updateElement var.View ]

So yes, it looks like the root cause of the issue may indeed be a bug in the propagation of Views to Attrs

@Tarmil
Copy link
Member

Tarmil commented Jul 24, 2021

Yes, and in fact this equality check is already implemented in the library, just in an Attr 🙂

let ApplyValue get set : Apply<'a, 'o1, 'o2> = fun var el cb ->

@cata
Copy link
Contributor Author

cata commented Jul 25, 2021

Oh, I remember something related, from 6 years ago 🙂 : #2

@Tarmil
Copy link
Member

Tarmil commented Jul 27, 2021

Some progress on this, I found that it's not quite a bug in the propagation, but rather the fact that the check-and-set is not done at the same time in both versions. With Doc.BindView, the check is done inside a View.Map, and therefore during the View propagation. Whereas with Attr.DynamicCustom and Doc.Input, it is done at the end of the propagation.

I have a small proof of concept to do the equality check in a View.Map in Attr.Value (and therefore in Doc.Input too) but still sets the DOM element's value at the end of propagation. That seems to be working, I'll continue working on it a push a PR.

granicz added a commit that referenced this issue Jul 31, 2021
Fix #220: in value Attrs, check equality during View propagation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants