Riot: Seeking community consensus on how updates to input expressions should function

[rsbondi]

I am posting this to get feedback on how input expressions on update should be handled. See further discussion at #1612 & #1638

<input value={expression} name="myInput" onchange={changed}>
this.expression = "initial value"

The riot functionality has been to evaluate {expression} with "initial value". On additional calls to update, {expression} will only be evaluated if the value of this.expression has changed. The user can type freely into the input field but riot will not evaluate the expression on update as long as the value for this.expression has not changed. A change of this.expression will then overwrite any user input on update. Additionally, you can manually sync the two like

changed() {
  this.expression = myInput.value
}

Interest has been shown suggesting that a different approach may be desired. That would be to force a synchronized state by always evaluating the expression on update. So the user types in values, but the expression will be evaluated on update returning the input field to "initial value" or whatever the current value is for this.expression The dom updates would be limited by comparing the dom value to this.expression. The above manual sync would be required to preserve the input value on update. See more here

Please help us decide if we should keep the existing functionality in the 3.0 release or implement the alternate method

[ilblog]

Well RIOT is nice because it is tiny and fast. As @GianlucaGuarini said, it is very close to native JS. Making easier life for developers, means more code and less speed. Current RIOT solution seems to be right for me personally. This is what I use.

    <div if={ renaming }>
        <input type="text" onblur={ blur } onkeyup={ changeName } value={ fav.name }>
    </div>  

    this.blur = function() {
        self.renaming = false;
        // Save new name to DB;
    }

    this.changeName = function(e) {

        if(e.keyCode === 13)  // Enter
            self.blur()
        else 
            newName = e.target.value; 
    }

Guys, RIOT is not Angular JS or some big fat framework. Think of RIOT JS lake Rapsbery Pi. Small tiny toy for sophisticated programmer.

[dogada]

IMO, Riot should NEVER discard user input. Input value should be treated as
initial only and after DOM-node for the input is constructed it should
ignore any expression updates.

When I need to change an input value I should do it implicitly
this.myInputName.value = newValue.

Regards,
Dima.

On Fri, Mar 4, 2016 at 3:52 AM, Richard Bondi [email protected]
wrote:

I am posting this to get feedback on how input expressions on update
should be handled. See further discussion at #1612
https://github.com/riot/riot/issues/1612 & #1638
https://github.com/riot/riot/issues/1638

this.expression = "initial value"

The riot functionality has been to evaluate {expression} with "initial
value". On additional calls to update, {expression} will only be
evaluated if the value of this.expression has changed. The user can type
freely into the input field but riot will not evaluate the expression on
update as long as the value for this.expression has not changed. A change
of this.expression will then overwrite any user input on update.
Additionally, you can manually sync the two like

changed() {
this.expression = myInput.value
}

Interest has been shown suggesting that a different approach may be
desired. That would be to force a synchronized state by always evaluating
the expression on update. So the user types in values, but the expression
will be evaluated on update returning the input field to "initial value"
or whatever the current value is for this.expression The dom updates
would be limited by comparing the dom value to this.expression. The above
manual sync would be required to preserve the input value on update. See
more here
http://blog.iansinnott.com/managing-state-and-controlled-form-fields-with-react/

Please help us decide if we should keep the existing functionality in the
3.0 release or implement the alternate method

—
Reply to this email directly or view it on GitHub
https://github.com/riot/riot/issues/1642.

[sylvainpolletvillard]

What you are describing by forcing the state looks like an <output>, not an <input>. I don't see any good reason to prevent user input by default on an input tag.

[gregorypratt]

Is this what you're talking about?

http://plnkr.co/edit/uIqFsVKfCef1lHdKsRAa?p=preview

If you make changes, the template reverts to the initial value on blur.

[GianlucaGuarini]

@gregorypratt exactly, the next [email protected] will fix this issue but we don't know if this is a good behavior to introduce in [email protected] so we want to know what you think guys

[gregorypratt]

From the example at the top:

<input value={expression} name="myInput" onchange={changed}>
this.expression = "initial value"

this bit is important: value={expression} this tells Riot to apply the value of expression to the input.

That's all you need to know really. And unless you actually change expression why would you expect the input to change?

If you were to use this.myInput.value everywhere however you wouldn't have an issue. It would always be in-sync because you're in _direct control of the template_. You're not asking Riot to do anything automatically for you.

This behaviour is what I'd expect.

[aMarCruz]

We are going to nowhere.
I think we can set a flag in riot.options to enable the new behavior, for those guys coming from React or for special requirements.

BTW:
people used to working with data-driven apps (like me) usually use "disconnected" fields in edit mode. Allow user edition in forms linked directly to the database or data store is risky (crazy?).
That's why I see the change proposed by @GianlucaGuarini as something natural; the current behavior has always confused me.

[yuretz]

@rsbondi wrote

The riot functionality has been to evaluate {expression} with "initial value". On additional calls to update, {expression} will only be evaluated if the value of this.expression has changed. The user can type freely into the input field but riot will not evaluate the expression on update as long as the value for this.expression has not changed.

Well, is it still true for inputs inside a loop without no-reorder tag? If not much has changed regarding loop rendering since the version I use (2.3.1), then any user edits are discarded whenever looped input triggers an event, because the whole loop is re-rendered on any looped child update.

My personal vote goes for consistency. As for me, I can live with any solution, as long as the behavior/logic is the same in all the situations (also in loops!).

Having a flag in riot.options is of course a possibility, but that means increasing the complexity, which is just a way to more bugs and inconsistencies. So I would prefer if you could choose one true riot way (hopefully, the simplest one, with the least "magic" involved) and stick to it, even though some users might disagree.

[yuretz]

Thinking a little bit more about the topic in question, the issue #1319 might also be relevant for this discussion, because besides value, things like input focus and cursor position are another part of an input state, and it looks like it gives some weird results if we allow riot resetting those on each update.

[rsbondi]

@yuretz I think the issue is "with" no-reorder, it is fine without but point taken about consistency.

[jmk2142]

After some mild initial whiplash between the prior behavior and newer behavior, I think the most consistent behavior with what I understand as the "riot way" is the original. this.data is the source of truth and unless that's updated (explicitly) the expression shouldn't be re-computed and updated. Having an onchange listener to explicitly set state isn't a difficult extra step and makes the intent very clear. A short blurb in the docs about how input values work, to reemphasize this one directional state driven behavior would hopefully solve any confusion.

I suppose there are ways that both behaviors could be supported, like options or perhaps a special attribute to indicate that the particular input will be synced with the source data but part of the beauty of riot is that it doesn't have a lot of special directives and such.

[mattjcowan]

2.3.17 seems to be working again like 2.3.15 was, in these regards, nice! The 2.3.17 implementation works nicely for me.The problem with 2.3.16 is very apparent when using onkeyup events without updating the expression.
I'd agree with @jmk2142, and also close enough to React imo ...
It would be nice to see some documentation/example on editing rows for example in a grid context (inline-editing), and discussing in further detail when it would be good to update an existing entity vs a copy of the entity, and how to "reset" fields to their original values, for newer folks, more of a design pattern. Just a thought, as I'm trying to help others with the framework, and the general concepts, and figure out best patterns myself.
Btw, the plunker from @gregorypratt is now pointing to 2.3.17, so got me confused at first, just a thought that using script tags to specific versions of Riot (ie: cloudflare) can help understand the intent of the plunker better over time, for late-comers (6 days, lol) like me :-) ...

[gregorypratt]

Looking at the plunker (that is now using 2.3.17 as pointed out by @mattjcowan) I think I prefer this behaviour...

[ghost]

I'm sorry for the confusion I may have caused with #1612. As I've noted in a later comment ( https://github.com/riot/riot/issues/1612#issuecomment-187387623 ) the previous riot behavior actually made much more sense and a special "refresh"-method could be a better option to implement this behavior. Thanks at @rsbondi for explaining it to me.

[aMarCruz]

An additional option in update should be enough, very useful to restore the data in a form.

update([data,] true) --> true forces an inconditional update of the UI.

EDIT: I mean, updates the UI if it value is different from the expression.

[jmk2142]

@whatda No need to apologize. Issues are how discussions are raised that bring about consensus as to where to go right? I've delved deeper into this and in the end, came out understanding Riot even better. :-)

[GianlucaGuarini]

Ok here I would like to let our user voting democratically what's the best behavior for riot in this case, I would like to avoid new api methods, flags or whatsoever attributes. I will make 2 posts here and the one receiving more :+1: will be the way to go

[GianlucaGuarini]

_Please vote here_ for a version where riot must always update the input/select tags giving priority to the data coming from the update events discarding in this case the user input events
example

[GianlucaGuarini]

_Please vote here_ for a version where riot should update the input/select tags only when the original value passed initially to them changes independently from the user input
example

[ilearnio]

I think that updating of the input tag while the user is typing, is a really bad idea. Especially if it would be textarea where he might enter a lot of text and then suddenly lose all the changes. So my vote is for the second variant

[willemmulder]

So how does this work now? If we update the original input value, the input field will be updated? If so, how would we handle the case where the original input value remains the same, but we still want to discard user input?

[GianlucaGuarini]

@willemmulder The next riot major release will update the input values only if the original value ( of the model/object/store bound to it ) will be changed according to our community feedback https://github.com/riot/riot/issues/1642#issuecomment-198789169

[willemmulder]

@GianlucaGuarini Yes thanks, I saw that. So for me, the question remains how we can reset an input field to its bound value, discarding user populated values, _without_ actually changing the bound value. Or should I simply do a quick "backup bound value Y", then "change bound value to X", then "change bound value back to Y" to simulate a change in the bound value?

[GianlucaGuarini]

@willemmulder oh sorry I assumed it was clear http://plnkr.co/edit/9qrn6Mb6zzpIj9WawGSU?p=preview
Of course you don't handle all this logic directly in a tag, but you do it somewhere else using redux/flux/whathever script

[GianlucaGuarini]

@willemmulder I have tried to emulate a kind of simpler redux logic here http://plnkr.co/edit/VRkTQdrctcd8Vn1ilxfj?p=preview
EDIT:
I have also added all the possible features you can handle in the form, reset included

[willemmulder]

@GianlucaGuarini Ah that makes sense! On update, Riot compares the bound values to see if they have changed, and so we need to wrap our values in a String() object for example, so the pointer-comparison will be false. The easiest that I could come up with is this, which seems to work: http://plnkr.co/edit/Va0d4DMWLRNEJpbOIXIM?p=preview. Is my thinking correct? Thanks again!

[GianlucaGuarini]

@willemmulder yes your demo looks fine to me you got it ;)

[willemmulder]

@GianlucaGuarini Thanks! :-)

[bradleypeabody]

Has anyone looked at the idea of adding a "formRefresh" (or other appropriate name) method to a Tag which would just explicitly cause all value={} expressions to copy from the source data newly? From looking at the thread above and contemplating some different use cases, it seems to me that when/if you want to force this kind of update is dependent upon the use case, and if you do want it, there is usually a pretty clear place to put that call - you just don't want to have to list out a bunch of this.input1.value = ''. this.input2.value = ''; etc. and keep that up to date as your form evolves. Having a simple call that just explicitly says "yes, update my form values from the underlying data, dammit!" would seem to be very useful.

[nodefish]

I feel like Vue.js has a good solution for this, by supporting an initial bind feature, written as {{* expression }}

(i.e. asterisk at the start of the mustache)

A common use case is calling a handler on the oninput event in order to generate an autocomplete / suggestions list while the user is typing. In riot's current behavior, using oninput causes the user input to be lost, as handlers trigger updates (and if you use e.preventUpdate=true, then the autocomplete list won't be updated).

[aMarCruz]

update needs a flag, as commented here.

[brauliobo]

both proposals (https://github.com/riot/riot/issues/1642#issuecomment-198788275 and https://github.com/riot/riot/issues/1642#issuecomment-198789169) don't handle the updates made by the user on the input to sync values with the component variables. how this can or will be done? (as @bradleypeabody described)

[ghost]

Is there any workaround for a form refresh right now? Because I really need to solve this issue in my app. :S

Example: I have a modal for both editing and creating entities. After editing an entity, saving it and then choosing to create a new one, the input values are still filled with the values of the edit...

I think an update flag would be the best idea, because this is apparently not just some trivial "nice to have"-feature, but a serious restraint when working with forms.

[willemmulder]

@whatda yes we have the exact same problem. An attribute like < form refresh={isFresh}> would be perfect. Then if we set isFresh=true and subsequently call update(), all the containing input-fields would be refreshed from their bind-expressions.

[seanzer]

It should be up to the developer to update value. There is a placeholder attribute that should be used for placeholders. Value should always be updated.

[willemmulder]

@rsbondi I see you closed the issue, but I'm still unclear to what I should do to update a form-field to use it's current bind-expression and get its new value from there... My mistake probably, but what is the suggested way of doing it, right now?

[GianlucaGuarini]

@willemmulder the way you (users) have decided https://github.com/riot/riot/issues/1642#issuecomment-198789169

[willemmulder]

@GianlucaGuarini Okay, so if we change the value on the model, the form-field will refresh if we call .update(). That's fine. But if we want to to force a refresh (without changing the model), we'd have to go over the model and change all the properties to e.g. model.value = new String(model.value); so that pointer comparison reports a change, even though the value is the same.

There is no escape from doing that, right?

[GianlucaGuarini]

we'd have to go over the model and change all the properties to e.g. model.value = new String(model.value); so that pointer comparison reports a change, even though the value is the same.

You can also sync your data simply listening the onchange event http://plnkr.co/edit/aDEaHlykV4iXtXPEIa9M?p=preview . If you will use a mixin for it it would be everything easier

[willemmulder]

@GianlucaGuarini Yes good point. That's what I usually do.

The case where I needed a 'hard reset' of the form to its input-expressions is after some strange behaviour where if I switch the model of a field, somehow the old values of the field in the form are preserved. But I should actually try to find and fix that issue instead of working around it. If I find it, I'll post back here.

Thanks!

[ghost]

@GianlucaGuarini I don't intend to be rude here, but imo the survey was a bit flawed: There was no option for an extra update-flag or method, just update behavior in general. Given that, it's no wonder people voted for the old style, because a flag or extra method wasn't up for debate (maybe it would be better if it would have been up for debate).