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

merging rmarkdown branch with master #51

Closed xiangnandang closed 1 year ago

xiangnandang commented 4 years ago

merge rmarkdown branch with master to allow further development of the package to support rmarkdown documents with shiny runtime

daattali commented 4 years ago

Thanks. That code looks like it was written in a hurry (understandably - Andrew was just writing it as a quick proof of concept), I don't feel comfortable merging it at its state. The code style is poor, I see there's a paramter header_file with a default value but does that default value work? There is no documentation saying how to use this function, and the rmd_in_header function is using the old style of manually adding all the CSS styles in the code (see this refactor).

I don't want to add any bad code into the repository because then it becomes a maintenance liability. If you have some time you can start going through and cleaning this up. When I find time to devote to this package again I can do it too.

daattali commented 4 years ago

Related to https://github.com/daattali/shinycssloaders/issues/8

xiangnandang commented 4 years ago

thanks, that makes sense. I'll go through and clean up at a slight later time.

daattali commented 2 years ago

Hi @xiangnandang would you like to come back to this PR and fix it up?

daattali commented 1 year ago

@xiangnandang I want to do some cleanup of old PRs - do you think you'll have time in 2023 to resolve this PR, or should we abandon it?

daattali commented 1 year ago

Closing due to inactivity. I'd be happy to accept a PR if someone wants to take this up in the future.