search

daattali / shinycssloaders

⌛ Add loading animations to a Shiny output while it's recalculating
https://daattali.com/shiny/shinycssloaders-demo/
Other
395 stars 45 forks source link

Add caption #72

Closed ericnewkirk closed 1 year ago

ericnewkirk commented 1 year ago

Adds caption and caption.color arguments to withSpinner. Captions are implemented as an additional div below the one containing the spinner. Not supported for spinner type 1 because the animation for that spinner causes the caption to move up and down with the animation, which is really goofy looking. Honestly I think 'if you need a caption pick any spinner other than number 1' is fair.

28 68

daattali commented 1 year ago

Hi @ericnewkirk I apologize for taking so long to get to this, but I do like this PR! I prefer to only have a caption parameter without a colour parameter though, only because there is already a very long list of parameters and I don't want there to be too many. Would you mind updating your PR to remove the captioncolour parameter and also to merge with the latest code?

Thanks!

ericnewkirk commented 1 year ago

@daattali sure, here you go. I also bumped the version number and added a bullet to NEWS.md, hope that's alright.

daattali commented 1 year ago

Thanks! I ended up adding another feature that was highly requested, so I'll see if the latest changes I made cause any issues with your new code :)

ericnewkirk commented 1 year ago

Haha, I took a stab at the show/hide feature too. My approach if you're interested.

daattali commented 1 year ago

Thanks, I took a look -- I do prefer a more minimal approach. Even if they both achieve the same result, I think this approach will be a bit harder to debug/understand/maintain. I always strive to have the code as simple and maintainable as possible.

ericnewkirk commented 1 year ago

I don't think the caption tag was being created if no caption was passed (former code):

if (!is.null(caption) && type != 1)
          shiny::div(id = paste0(id, "_caption"), class = "shiny-spinner-custom", caption)

The PR needed to be updated anyway though given more recent changes, and I made an effort to streamline some things. See what you think. The caption object should be NULL here if type 1 was specified or no caption was passed.

daattali commented 1 year ago

Thanks, I've done some extra work on this and will be merging it now.