search

JohnCoene / waiter

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

support automatically hiding spinner after expression completes; fixes #74 #75

Closed daattali closed 3 years ago

daattali commented 3 years ago

I lied, I just couldn't go to sleep until I get this done 🙈

I believe this is going to be a huge usability improvement.

You'll have to re-run the documentation, I initially did this change locally and ran documentation, but I made the mistake of doing this in your repository and not in a fork. So I would need to re-clone the repo, and it took about 5 minutes to clone and I really want to go to bed so I'll be rude and submit a PR that requires more work 😬

daattali commented 3 years ago

Ok I cheated, I copied the changes from my local waiter.Rd into here. That's extremely bad form, if you don't like it feel free to revert!

JohnCoene commented 3 years ago

Ah, interesting, I had not thought of that.

I reckon we should also add this to the Waiter class, probably in the show method https://github.com/JohnCoene/waiter/blob/2a80b30548218325e2f27406fe7c941e22511c9a/R/waiter.R#L356

What do you think?

JohnCoene commented 3 years ago

On second thought, this is a cool feature but maybe should be implemented in another function.

The show* functions and methods already come with hide_on* arguments to automatically hide the loading screen when element at id re-renders. One could set these to TRUE and pass an expr as well. It would become confusing to understand what exactly is going to hide the loading screen.

I'm not sure I explain this clearly.

It might be better in another function. I'll try to think of something.

daattali commented 3 years ago

Perhaps this functionality already exists and I'm just not familiar with the package to know it. I understand it can be auto hidden when an output is re rendered, but a usecase I commonly see with my clients is that they want a loading screen while running some calculation , but no output is rendering on the screen. That's what this PR was for.

JohnCoene commented 3 years ago

I may take this in the next release, thanks Dean.