bslib icon indicating copy to clipboard operation
bslib copied to clipboard

Should `theme` argument in `page_*()` functions accept a string?

Open daattali opened this issue 1 year ago • 6 comments

In shiny's page functions, the theme argument can accept either a bs_theme() object or a path to a CSS file.

In bslib's page functions, the documentation says that the only acceptable value is a bs_theme() object. However, it seems that providing a string path to a CSS file also works. Should this be accepted or no? If yes, it should be documented. If no, an error should get thrown.

daattali avatar Jun 03 '24 07:06 daattali

I think the ship has probably sailed at this point for accepting a string. We also do throw an informative error:

> bslib::page("bar", theme = "foo")
Error in assert_bs_theme(theme) : `theme` must be a `bs_theme()` object

cpsievert avatar Jun 03 '24 13:06 cpsievert

Strange, I tested on both the CRAN version and the latest github version of {bslib}, and ui <- page(theme = "style.css", "test") does work for me. No error message, and it applies the stylesheet.

daattali avatar Jun 03 '24 15:06 daattali

Ahh, right, that error only gets thrown when rendered statically. Otherwise, it gets passed to shiny::bootstrapPage(), which doesn't error. Maybe we should do more to throw earlier on.

cpsievert avatar Jun 03 '24 15:06 cpsievert

The other side of this would be to allow a string value for theme, in which case theme should point to a static CSS file, i.e. a pre-compiled Bootstrap theme. This would be in line with how the theme argument works in Shiny for Python page functions. But it would require more thinking (and also probably improved utilities for saving a bs_theme() to a set of static files, which of course brings additional issues with Shiny's built-in components that do use bs_dependency()).

Clearly throwing an error would be easier, but I think we should contemplate alternatives before implementing.

gadenbuie avatar Jul 11 '24 18:07 gadenbuie

What is meant by "static rendering"? What is the dynamic rendering version of the code I gave above ui <- page(theme = "style.css", "test")?

daattali avatar Jul 11 '24 21:07 daattali

What is meant by "static rendering"?

I think Carson means printing ui or otherwise rendering it to html, where dynamic rendering would be using ui in a Shiny app.

gadenbuie avatar Jul 11 '24 22:07 gadenbuie