Fix race condition in textfields

[aforemny]

This is just a reminder for me that I want to start a discussion about this.

edit: As discussion about this started already before I had a chance to motivate it here, please see the discussion below. In particular, have a look at the comment linked by @debois in his first comment which explains the race condition.

[debois]

I take it the proposal is to use defaultValue instead of value in Textfield.input. I think this is a good idea, but I don't understand the ramifications.

There was discussion on this over on elm-dev, which lead to this comment.

[aforemny]

The ramification is that you cannot update defaultValue except for when the DOM node is created.

This is usually what you want, though. And if you do want to update the input later on, you will find out that value is broken in upstream elm anyways.

However, consider the following UI example: say your application is dealing with some resource { name : String } and you do have a screen that displays a list of resources as well as a screen that let's you edit a single resource using an input for name. It is very tempting to have both screens in DOM all the time and conditionally show either of them depending on your model, ie. by providing a default { name = "" } when you are not actually editing some resource. If you follow this approach, the input is not removed from DOM and subsequently not added again to DOM when you want to edit a resource a second time. So defaultValue will not work as expected.

[OvermindDL1]

For note I've exclusively been using Textfield.value and its working for me to update the message as necessary (with Options.nop for when I do not want to).

[aforemny]

The Elm value bug is not observable in smaller examples, but it becomes very apparent in large applications. Are your use cases small and therefore they do not trigger the bug? For example, package.elm-lang.org I would still consider a small example as it is kind of hard to trigger the bug, yet possible. This might vary by the machine also. Try testing your examples under heavy load of your machine, and by adding a few hundred DOM nodes.

Or, are you using intricate adding and removing of event handlers to circumvent the bug? For instance, the bug does not appear if you are removing the input handler on focus and add it back on blur. But you introduce other problems.

I think all test programs that we are considering for this example should be quite large and should not remove the input handler at any time.

edit: Actually, package.elm-lang.org does not seem to set value on the input anymore. So the bug is not observable there anymore.

[OvermindDL1]

It is a fairly large DOM though not particularity complex, so likely not. I'm curious how to replicate the issue for me, seem unable to yet.

I do however utterly destroy the textfield (removing it from the DOM) for a period of time of no less than the server-roundtrip (replaced with an 'in-progress' indicator) before it is re-created and re-focused. That potentially resets the issue I am guessing?

[aforemny]

The bug is observable in this demo.

[OvermindDL1]

Fascinating, and with that I have figured out how to replicate it on mine. (Oh how I miss C++'s determinance rather than random DOM events)

[aforemny]

I think we should add a defaultValue attribute to text inputs. If it is possible, also to textfields.

We should not remove the value function so that we will not break any code. But we should explain in the documentation, that it is broken.

Also, the documentation for defaultValue should explain that this property only changes when the node is added to DOM, so that users are less likely to run into the problem that I described in my second comment.

What do you think?

[debois]

What would be the consequence of just redefining the current value property to set defaultValue?

[vipentti]

The value would not change I assume after the initial rendering ?

Atleast based on http://package.elm-lang.org/packages/elm-lang/html/1.1.0/Html-Attributes#defaultValue

[OvermindDL1]

I very much need access to 'value'. At the very least I need to push a new value update to it to reset the textfield contents (like prepending @somename, surrounding selected text with, say, bold, etc...). I am currently only setting the text when I need to via this:

setTextOnMessageInput : String -> Cmd Msg
setTextOnMessageInput newText =
    Cmd.batch
      [ cmd_from_msg <| UpdateInputMessageText newText
      , cmd_delay <| cmd_from_msg <| ClearInputMessageText
      ]

Which sets/clears a key on the model record for the respective messages, just made this change this morning, works perfectly, and I do not care if the cursor jumps, I also made a new textfield_onInputWithSelection that gives both the entire message as well as the selected text so I know the state of the selection when they click one of the editor buttons.

So yes, I need 'value', defaultValue would also be useful though (since I do clear the textfield element out of the DOM entirely and re-add once I have confirmation the message has successfully sent).

[aforemny]

As @vipentti said, a input with defaultValue does not update its value ever from the Elm side. It just starts out with defaultValue instead of being empty. So things like automatically adding - to a "date" input YYYY-MM-DD wouldn't work, for instance.

[OvermindDL1]

Correct, that is what I've mentioned as to why I still need normal value. :-)

A defaultValue I'd almost never need for this application, though would be quite useful in various forms.

[debois]

Keep in mind that whenever elm re-renders (view is called), you'll set a new default value. I tried changing Textfield to use defaultValue instead of value on a new 161 branch. It seems to work for me on Chrome/Mac OS. Could you try it out? You can try it on the wip demo.

[OvermindDL1]

@debois Tested, and the jump does not happen anymore, however it also makes it so I cannot update it. My functions for highlighting a string of text are changing the backend model (and thus the rendered view) but are not changing what is in the message box, so I need a way to force it to update somehow as this entirely breaks the editor buttons as seen at: https://overminddl1.com/screenshots/work/Textfield-Editor_Broken.gif

I wish there were a way to submit a Cmd via elm-mdl to some specific id of the textfield to force update the textfield with the new data. But as you can see from the image I read the input from the textfield as it changes (markdown), update the preview on the right in real time, have a few editor buttons (this is a college, not everyone is tech savvy enough to know markdown or any formatting at all), and the buttons normally update the model and set a flag and delayed command to unlock it, the flag then causes Textfield.value to be set, then the next message around (the delayed one) resets the flag to False again, which causes Textfield.value to be Options.nop instead.

So yeah, as-is-is that test breaks my editor, have any idea on how to fix. :-)

[OvermindDL1]

Consequently, here it is again after I rollback to 7.3.0: https://overminddl1.com/screenshots/work/Textfield-Editor_Working.gif

[debois]

I'm not sure I follow. I'd expect your Model to have some field str containing the current value of the textfield. In that case, you keep str current with Textfield.onInput. You maintain also the selection state in Model, following the last example in the demo. Then, when the user clicks the Bold icon, you just change the value of str, and you're good.

I don't fully understand your current approach with flags. What is it? Why is it necessary?

[OvermindDL1]

That is precisely what I do, but then I need to update the textfield to show the markdown that I applied. In the second image the markdown appears in the textfield, in the first it does not.

[aforemny]

Textfield should support both value and defaultValue functions. That is the best we can do right know according to my knowledge. Please re-open if I am missing a way to improve the situation in Elm.