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

Input is losing focus on re-draw when using Bind.el #92

Open
MangelMaxime opened this issue Sep 20, 2024 · 4 comments
Open

Input is losing focus on re-draw when using Bind.el #92

MangelMaxime opened this issue Sep 20, 2024 · 4 comments

Comments

@MangelMaxime
Copy link
Contributor

The introduction is a bit long, but at the end there is a small reproduction code. I felt like explaining by context could help to understand the why

The way Fable.Form works is, by composing onChange functions it is able to handle any number of inputs via a single onChang function.

In a pure React application, the code looks like that:

[<ReactComponent>]
let View () =
    let formState, setFormState =
        {
            Email = ""
            Password = ""
            RememberMe = false
        }
        |> Form.View.idle
        |> React.useState

    let onSubmit =
        fun (_formResult: FormResult) ->
            { formState with
                State = Form.View.Success "You have been logged in successfully"
            }
            |> setFormState

    Form.View.asHtml
        {
            OnChange = setFormState
            OnSubmit = onSubmit
            Action = Form.View.Action.SubmitOnly "Sign in"
            Validation = Form.View.ValidateOnSubmit
        }
        form
        formState

With Elmish:

let view (model: Model) (dispatch: Dispatch<Msg>) =
    Form.View.asHtml
        {
            OnChange = FormChanged >> dispatch
            OnSubmit = dispatch
            Action = Form.View.Action.SubmitOnly "Sign in"
            Validation = Form.View.ValidateOnSubmit
        }
        form
        model

So I tried to do something similar with Sutil, and naively I used Bind.el which seems to allows to depends on a store (to me it looked similar to React hooks):

let app () =
    let stateStore =
        {
            Email = ""
            Password = ""
            RememberMe = false
        }
        |> Form.View.idle
        |> State.Filling
        |> Store.make

    bulma.section [

        Bind.el(stateStore, fun state ->
            match state with
            | State.Filled (email, password, rememberMe) ->
                Html.div [
                    Html.h1 [
                        prop.text "Form filled"
                    ]
                    Html.p [
                        prop.text $"Email: %s{email}"
                    ]
                ]

            | State.Filling formValues ->
                Form.View.asHtml
                    {
                        OnChange = fun values ->
                            printfn "Form values changed: %A" values
                            State.Filling values |> Store.set stateStore
                        OnSubmit = fun result -> State.Filled result |> Store.set stateStore
                        Action = Form.View.Action.SubmitOnly "Sign in"
                        Validation = Form.View.ValidateOnSubmit
                    }
                    form
                    formValues
        )
    ]

The problem is that when the stateStore is updated, Sutil re-draw the DOM and the input lose focus.

CleanShot.2024-09-20.at.22.17.16.mp4

Looking at the documentation, I found that Bind.attr ("value", ...) should be probably be used. I could perhaps, use Store.map in order to mimic how Fable.Form compose function internally but it would require quite a lot of work and means that I can't share as much logic between the different renderer as I would like.

So I am asking if it is possible to keep the focus, without using Bind.attr or is it the only way and I should look into trying to compose/map the store internally directly?


Below is a simplify code to reproduce the situation:

let inputStore = Store.make ""

Bind.el (inputStore, fun input ->
    Html.div [
        Html.div [
            prop.text $"Input value: %s{input}"
        ]

        Html.input [
            Ev.onInput (fun ev ->
                let value: string = ev.target?value
                Store.set inputStore value
            )
            prop.value input
        ]
    ]
)
@davedawkins
Copy link
Owner

I have not looked at the example in detail yet, I'll do that from my desk tomorrow. I can tell you though that if the input is re-rendered, the input will be lost - Sutil (as yet) does not have a vdom. You will need to project the input value into the value field as you say.

Let me look properly tomorrow though

@MangelMaxime
Copy link
Contributor Author

What if instead of always re-creating the DOM element Sutil used something like that:

  1. If the element to render and the one existing at that place in the DOM are of different type then Sutil remove the DOM and rebuilt it
  2. If they are of the same type, then Sutil checks each properties that it wants to set for a different and set it. I think, not patching all the props but just the one that changes was better in performance in the past. I don't know if this is still the case.
  3. Addition rules can applies like key, etc.

To me it looks similar to what Svelete, React or DOM patching libraries does in general.

The main complexity it will add is that, Sutil will need to walkthrough the DOM tree to compare the different Nodes, but I don't think this should be too difficult because this is a recursive functions.

@davedawkins
Copy link
Owner

Absolutely, but it will need a VDOM to do that. The SutilElement type is a factory function that operates on the current context, and it can't be examined. What we can do is have it generate a virtual DOM element and then compare that with the real DOM. I will try this, but it won't be quick

@MangelMaxime
Copy link
Contributor Author

Ah that's where the Sutil don't have a VDOM comes from.

I didn't know that SutilElement or Feliz.Engine were factories and didn't contains the information required to apply the patch / do some comparaisons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants