JohnCoene / waiter

🕰️ Loading screens for Shiny
https://waiter.john-coene.com/
Other
499 stars 25 forks source link

Using 'hide_on_render=TRUE' on a plot, does not work #94

Closed sigmapi closed 3 years ago

sigmapi commented 3 years ago

Hi, using hide_on_render=TRUE on a ggplot fails to hide the waiter after the value(or error) Shiny events.

I noticed some errors in the javascript code (waiter.js):

// remove when output receives value
$(document).on('shiny:value', function(event) {
  if(waiter_to_hide.indexOf(event.name) > 0){
    hide_waiter(event.name);
  }
});

Javascript arrays are 0 based. So the comparison above will naturally become false, even if the correct element is in the array with index 0. This is the correct way:

// remove when output receives value
$(document).on('shiny:value', function(event) {
  if(waiter_to_hide.indexOf(event.name) > -1){
    hide_waiter(event.name);
  }
});

This should be changed for operations in all three arrays: waiter_to_hide, waiter_to_hide_on_error, waiter_to_hide_on_silent_error. And perhaps on other javascript files too?

JohnCoene commented 3 years ago

Thanks, well spotted. I just changed it to use includes instead. It should work with the dev version.

sigmapi commented 3 years ago

Thanks for the quick response. Be aware that includes is not fully supported method, I suggest sticking to indexOf. https://caniuse.com/?search=Array.prototype.includes

On the same topic, items are added to these three arrays but they are never removed. Shouldn't they?

Should I call waiter_show every time a plot is regenerated on the server? Or once per session?

sigmapi commented 3 years ago

One more thing: how do we specify from server side the hide_on_error, waiter_to_hide_on_error? Currently when I use waiter_show, only the to_hide flag is set.

Perhaps there is no need for three separate arrays. Is there a scenario where a user would want the spinner (in this auto-hide case) to continue in case of an error?

sigmapi commented 3 years ago

And another thing (sorry, I just write what I notice, when I use the waiter_show on a plot render): In hide_recalculate a style is added to the document head and it never gets removed. After recalculating a plot several times in a session, the head is full of these repeating styles.

JohnCoene commented 3 years ago

Thanks for this!

I'll be honest I have not taken care of the waiter_show function too much because I want to push for the use of the reference class. I initially had both the functional and class APIs thinking it would make it easier but it does not, on the contrary.

You can specify the arguments you ask about here: https://github.com/JohnCoene/waiter/blob/8cdb0e259c75e55d856dd1bc68b5dca4aac9589d/R/waiter.R#L373