Added setting autoUpdate so we can turn off auto update after events #699
Conversation
|
This could be a really nice feature but I would avoid to use it as global riot.settings I would like to see it more as a specific tag option. Something like |
|
Maybe support both? I would prefer setting it globally. |
|
I don't personally see an issue here. The update() should be something that can be called at any point in time and UI would always reflect the current state. On AJAX calls the UI typically displays a progress bar/spinner etc before the response is available. Curious: what is the actual issue on your case when update() is called too early? |
|
In my case, I have put some code in updated event (e.g. build pagination controls), required to run after the server responds with data. However, with automatic updates, that code ends up running unnecessarily. Or even mounting the components with the wrong states. I support both global and tag-specific settings. In my case the app is entirely data driven and all the pages except the first page depends on some kind of response from the server. |
|
I totally agree with #geordee here. It's a big fail if the updated event triggers too early and then we do also have the second issue I wrote about in my first post. |
|
What about an |
|
Not sure if this will work, just throwing it out there, maybe add the global, and use a mixin instead of the tag level, something like? {
update: function() {
if(this.preventUpdate) {
this.preventUpdate = 0
return
}
Tag.update.apply(this, arguments)
} |
|
That seems like a very complex way to do something very simple and I don't think it's doing it correct either. Why do you want to do it with a mixin? |
|
With global, you have to call update every time. With tag specific, you would have to call it every time for that tag. The mixin is a proof of concept for discussion, so thank you for your input, it was not intended to be a final product or proposed solution in lieu of the ones already presented . Riot likes to keep things minimal, using a mixin would remove it from the core. I am not opposed to a core based solution, this is something that is definitely needed I also am not opposed to a global or tag based solution, but consider the following So if I have a page, the user enters an item in a list, the list updates, calculations based on those list items update, dom items are shown or hidden based on selections and need to be updated, there may be many updates before or after an ajax call is made. When the user saves or loads the list, then the ajax call is made and the update is blocked until the call returns. So I think selectively preventing would be more practical in many cases. So even if both, the global and tag based solutions were added to the core, the selective blocking mixin approach could still be an option. So if you are making an ajax call on every user interaction, global makes perfect sense. I am not sure where tag based would make sense, would love to hear input on this. Selective blocking at least to me also makes sense. |
|
I guess both global and tag level make sense, and leaving autoUpdate "on" by default. Only advanced users need to tweak the settings to suit a data driven application (disable globally), or a workflow driven application (disable selectively, if required). So long the amount of code, and processing to implement these changes remain rather small (just a conditional in this case), I'd recommend it to be part of the core. |
|
Hi guys, the switch of autoUpdate seems nice when making tag libraries, too :) |
|
I like the doSomething(e) {
e.preventUpdate = true
}Feels logical to me. |
|
What is the event object here? The update fires automatically when the tag is mounted. |
|
#tipiirai Sounds good, lets go with that. Shall we drop the global setting? I can live with that. :) #geordee The event object is the first argument sent to the event handler. |
|
👍 better than the mixin, but it was fun to play with anyway |
|
Changes Unknown when pulling 8bc5cc4 on AndreasHeintze:feature/autoUpdate into * on muut:dev*. |
|
#rsbondi Yes, but i have to check out mixins sometime too. Haven't experimented with it yet. |
|
👍 Looks clean and nice! |
|
I like it! |
|
@AndreasHeintze could you add a test as well please? |
|
@GianlucaGuarini I have not yet learned about how to write tests. Can you direct me to some information about how to do it? |
|
@AndreasHeintze that's what you should have done ce9c839 |
|
@GianlucaGuarini Ok, great. Doesn't look that difficult. :) |
In Riot after an event handler is being called el.update() is automatically called, but since this update is sometimes called too early or when we don't want to call update I added this feature to make it possible to turn off automatic update calls. An example is when you make an AJAX request in your event handler, the result right now is that update is called before the AJAX call returns and that is most often not what we want to do. In these cases it is better to call this.update() manually in your event handler.
Also, there is a problem if the event handler method that is called is in a parent or parent.parent tag, like in this example:
http://jsfiddle.net/AndreasHeintze/67nznL8p/
Here the event handler method is in the parent tag and since el.update() is called on the current tag, nothing will get updated. In this case it is better to turn off auto updating and call this.update() manually in the event handler, like in this example:
http://jsfiddle.net/AndreasHeintze/67nznL8p/29/
Of course we can manually call this.update() in our event handlers without this fix, but the result of that is that update is called twice and I think we should avoid that.
Also the added code in this fix is very small.
/Andreas