Supply smarter theming defaults if a {bslib} theme is active

[cpsievert]

This PR allows reactable() to supply smarter reactableTheme() defaults when a {bslib} theme is active at render time. For example:

library(shiny)
library(reactable)
library(bslib)

theme <- bs_theme(
  bg = "#202123", fg = "#B8BCC2", primary = "#EA80FC",
  base_font = font_google("Grandstander")
)

ui <- fluidPage(
  theme = theme,
  reactableOutput("table")
)

server <- function(input, output, session) {
  output$table <- renderReactable({
    reactable(
      mtcars, searchable = TRUE, 
      resizable = TRUE, filterable = TRUE,
      highlight = TRUE, striped = TRUE, 
      outlined = TRUE, bordered = TRUE, 
      # Can still override theme defaults, if you want 
      #theme = reactableTheme(pageButtonStyle = list(color = "orange"))
    )
  })
}

shinyApp(ui, server)

Since the widget calls bslib::bs_current_theme() inside it's preRenderHook, it'll automatically re-render with new theming defaults whenever the {bslib} theme changes via bslib::bs_themer() (or more generally, whenever the new session$setCurrentTheme() is called). This generally just works as long the reactable() passed through renderReactable():

library(shiny)
library(reactable)
library(bslib)

ui <- fluidPage(
  theme = bs_theme(),
  reactableOutput("table")
)

x <- reactable(mtcars)

server <- function(input, output, session) {
  bslib::bs_themer()
  output$table <- renderReactable(x)
}

shinyApp(ui, server)

This approach will also generally 'just work' in {rmarkdown} (when {bslib} is being used to provide Bootstrap CSS) once rstudio/rmarkdown#1706 lands

Outside of Shiny and R Markdown, this can also work in static HTML sites via global {bslib} themes:

library(htmltools)
theme <- bs_theme(bootswatch = "darkly")
old_theme <- bs_global_set(theme)
browsable(tags$body(
  bs_theme_dependencies(bs_global_get()),
  reactable(mtcars)
))
bs_global_set(old_theme)

[codecov-io]

Codecov Report

Merging #96 (f1b6a9a) into master (33255ab) will decrease coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
- Coverage   99.17%   99.02%   -0.16%     
==========================================
  Files          15        9       -6     
  Lines        1823     1125     -698     
  Branches      335      335              
==========================================
- Hits         1808     1114     -694     
+ Misses         15       11       -4     

Impacted Files	Coverage Δ
R/shiny.R
R/columns.R
R/language.R
R/theme.R

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33255ab...f1b6a9a. Read the comment docs.

[glin]

Sorry for the delay - this is awesome! I've wanted to create a Bootstrap theme for reactable before, but this is much easier and more flexible than what I was thinking of.

My only hesitation is with applying the bslib theme by default, since reactable isn't a Bootstrap themed table. How about making the bslib theme opt-in, like having the theme argument optionally take a bslib theme:

library(bslib)

theme <- bs_theme(
  bg = "#202123", fg = "#B8BCC2", primary = "#EA80FC",
  base_font = font_google("Grandstander")
)

# Theme an individual table
reactable(mtcars, theme = theme)

# Or all tables
option(reactable.theme = bslib::bs_current_theme())

You wouldn't be able to combine this with a reactableTheme(), but I think that would be ok for now. bs_theme() can theme most of the table already, and a future addition could make it possible to combine or merge themes.

[glin]

Is there a specific reason to set the theme in preRenderHook instead of reactable()? I guess I'm just generally wondering what preRenderHook is used for, since there's not much documentation about it out there.

[cpsievert]

Is there a specific reason to set the theme in preRenderHook instead of reactable()?

Yes, preRenderHook gets called at print-time, just before the widget is serialized into JSON. When calling bs_current_theme() at print-time, we can have some guarantee of knowing when a {bslib} theme has been provided to a page layout function like fluidPage():

library(shiny)

ui <- fluidPage(
  theme = bslib::bs_theme(),
  verbatimTextOutput("txt")
)

server <- function(input, output, session) {
  output$txt <- renderPrint(bslib::bs_current_theme())
}

shinyApp(ui, server)

[glin]

Ahh gotcha, that makes sense.

[cpsievert]

My only hesitation is with applying the bslib theme by default, since reactable isn't a Bootstrap themed table.

I understand the hesitation, but keep in mind that this implementation (i.e., retrieving bslib::bs_current_theme() inside preRenderHook) means that these new styles should only kick-in when {bslib} is being used with Shiny and/or R Markdown (and thus, we have some guarantee that Bootstrap will be on the page). Although, there is one exception (when a global theme is set via bslib::bs_global_set()), but hopefully those cases are few and far between (and limited to power users).

How about, as some sort of compromise, we have theme default to "auto" (i.e., the current behavior), but we'd also accept a sentinel value (maybe NULL?) to opt-out and also accommodate bslib::bs_theme() objects? This also has the benefit of being similar to how the new {DT}+{bslib} behavior will work (i.e., by default, the styles will respect the {bslib} theme when relevant, but users may also opt-out)

reactable <- function(..., theme = getOption("reactable.theme", "auto")) {}

[glin]

I like the idea of an opt-out, but am still on the fence about applying the {bslib} theme by default. I'm just worried there'll be enough cases to opt out that it doesn't justify the defaulting. It'll also be different from how it is now, where reactable doesn't use Bootstrap even though Shiny and R Markdown put Bootstrap on the page by default.

For now, I'd rather keep it opt-in and wait for user feedback. If the vast majority of users opt in and it becomes a pain, then we could change it to apply by default. Setting the {bslib} theme explicitly might be inconvenient, but at least it's still a huge benefit over having to do the Bootstrap styling manually.

[cpsievert]

I like the idea of an opt-out, but am still on the fence about applying the {bslib} theme by default. I'm just worried there'll be enough cases to opt out that it doesn't justify the defaulting. It'll also be different from how it is now, where reactable doesn't use Bootstrap even though Shiny and R Markdown put Bootstrap on the page by default.

Keep in mind that existing code, as well as non-{bslib} use cases, will continue to work as before. So, this is still an 'opt-in' from the perspective that users have opt-in to a {bslib} theme; and when they do, we'd like to minimize the amount of effort required for everything on the page to reflect that theme. Of course I'll respect whatever decision you make, but just know we're pretty heavily invested in this idea by now; and in the near future, {DT}, {flexdashboard}, {gt} {shinyWidgets}, and hopefully many more packages will all be using this same approach to {bslib} integration.

[januz]

{bslib} integration seems like a great improvement! Will this be merged any time soon?

[cpsievert]

Hi @glin, would you have the time and interest to resurrect this if we made this an opt-in feature (similar to what you originally proposed)?

[glin]

Hey @cpsievert - yep, still interested, and I still think it'd be safer as an opt-in. For example, I'm now using the BS5 theme in the pkgdown docs and wouldn't want to opt out in every example. A middle ground might be to provide a simple reactable() wrapper that has the Bootstrap theme enabled, like bsreactable() or something.

I don't think I'll have time to work on this any time soon though, because of general lack of time and a few other priorities to work on right now.