Replacement input elements injecting Ref function to fix jumping cursor

[chadunit]

This is a followup to https://github.com/elmish/react/issues/12#issuecomment-446163748 regarding handling jumping cursor at the library level.

Here is a prototype idea for replacement input elements which automatically fix the cursor, see input' and textarea' below. They do this by altering the prop list to inject a Ref that conditionally updates the value (like Elmish valueOrDefault behavior). If a Ref already exists it grabs the existing Ref function and chains it after the conditional update.

I guess these would result in extra unnecessary processing in many cases (e.g. non-Elmish usage?) but if the impact is small then it may be worth avoiding the trap of premature optimization to ensure correct UI behavior out of the box.

If users did encounter problems with it they could revert to the original functions and the manual valueOrDefault workarounds; this would confine the workarounds to the select few who really need them.

open Fable.Core
open Fable.Core.JsInterop
open Fable.Import.React
open Fable.Helpers.React
open Fable.Helpers.React.Props

// Check if prop is HTMLAttr.Value and return the value.
let tryValueProp (prop:IHTMLProp) =
  match prop with
  | :? HTMLAttr as attr ->
    match attr with
    | HTMLAttr.Value v -> Some v
    | _ -> None
  | _ -> None

// Check if prop is Prop.Ref and return the value.
let tryRefProp (prop:IHTMLProp) =
  match prop with
  | :? Prop as p ->
    match p with
    | Prop.Ref refFn -> Some refFn
    | _ -> None
  | _ -> None

// Update value of an element if different, then call chained function.
let updateValueIfDifferent value (chainFn:Fable.Import.Browser.Element -> unit) =
  fun (e:Fable.Import.Browser.Element) ->
    if e |> isNull |> not && !!e?value <> !!value then e?value <- !!value
    chainFn e

// Replaces Value prop with Ref prop that sets the value only if different, to avoid cursor jump.
// If Ref prop already exists, chain the existing function after the one we are adding.
let convertValueToRef (props: IHTMLProp seq) =
  let tryValue = props |> Seq.tryPick tryValueProp
  match tryValue with
  | None -> props
  | Some v ->
    let tryRefFn = props |> Seq.tryPick tryRefProp
    let chainFn = tryRefFn |> Option.defaultValue ignore
    let newRefFn = updateValueIfDifferent v chainFn
    props
    |> Seq.filter (tryValueProp >> Option.isNone)
    |> Seq.filter (tryRefProp >> Option.isNone)
    |> Seq.append (Seq.singleton (Prop.Ref newRefFn :> IHTMLProp))

let input' props =
  createElement("input", props |> convertValueToRef |> keyValueList CaseRules.LowerFirst, [])
let textarea' props children =
  createElement("textarea", props |> convertValueToRef |> keyValueList CaseRules.LowerFirst, children)

[MangelMaxime]

Future version of Elmish will have a better naming for withReact so I don't think adding these specials handlers is needed.

See https://github.com/elmish/react/pull/31/files

Also, if people are using Prop.Ref to add a special behavior to their input or textarea your code will override this behavior and make it hard for them to understand the source of the bug.

[chadunit]

What bug do you mean? The user's Ref callback is still called. Plus this is a quick prototype to explore the idea, code can still be improved or warnings/hints added, etc.

I think this wouldn't add any more user difficulty than the status quo which has been accepted up to present.

Compare:

1. Some number of users encounter common issue with cursor.
2. Search web site/github for cursor issues.
3. Learn about performance optimization affecting cursor in withReactBatched, and get alternatives.

vs:

1. Presumably much smaller number of users encounter unknown edge case issue with Ref.
2. Search web site/github for Ref issues.
3. Learn about usability optimization using Ref internally in the input helper, and get alternatives.

I support the new names but they mostly improve understanding of the issue vs. avoiding the issue in the first place. Especially if templates/samples/etc. just switch over to withReactBatched, which the obsolete tag for withReact will recommend, then new user experience will be essentially the same but with improved names.

[MangelMaxime]

Hum sorry I didn't saw the updateValueIfDifferent and was thing you were loosing the user ref function.

I am not sure what to think about this but as long as it's provide as input' and textarea' (not made the default) I am ok for the change.

[chadunit]

I don't think there is much value adding them as non-default. Then they just become yet another workaround in a list of several workarounds already. I want to eliminate the need for workarounds for the majority of users, not add another one to their decision tree.

[alfonsogarciacaro]

This is great work @chadunit, thank you! Only thing is, as Maxime says, we're trying to introduce as less magic as possible in the bindings. We do it for example with the prop lists to have more idiomatic F# code, but this already confuses some users (see #132). When the function doesn't correspond 1:1 to React, I like to change the name to avoid this (example).

I'd wait a bit to see if the new names in Elmish.React help people be aware of the error. If it's still happening we can revisit this and implement the patches :)

[chadunit]

If changed names are the only option then perhaps this:

/// An input element with an automatic internal Ref prop to maintain cursor position when Value matches DOM state.
let inputRefCursor props =
  createElement("input", props |> convertValueToRef |> keyValueList CaseRules.LowerFirst, [])

/// A textarea element with an automatic internal Ref prop to maintain cursor position when Value matches DOM state.
let textareaRefCursor props children =
  createElement("textarea", props |> convertValueToRef |> keyValueList CaseRules.LowerFirst, children)

But anyway will go ahead and close for now, to await resurrection if revisted later.