rstudio / DT

R Interface to the jQuery Plug-in DataTables
https://rstudio.github.io/DT/
Other
596 stars 182 forks source link

Bootstrap styles overwritten #517

Open dpmccabe opened 6 years ago

dpmccabe commented 6 years ago

I discovered a perplexing error with the DT's Bootstrap styles. Here's a simple reproducible example with DT 0.4:

library(shiny)
library(DT)

ui <- function(request) {
  fluidPage(
    titlePanel("Test"),
    sidebarPanel(
      actionButton("load_table", "Load")
    ),
    mainPanel(
      DTOutput("faithful_table")
    )
  )
}

server <- function(input, output) {
  is_loaded <- reactiveVal(FALSE)

  observeEvent(input$load_table, {
    is_loaded(TRUE)
  })

  output$faithful_table <- DT::renderDataTable({
    if (!is_loaded()) return(NULL)

    datatable(
      faithful[1:20,],
      style = "bootstrap"
    )
  })
}

shinyApp(ui, server)

After clicking "Load", you get this:

screen shot 2018-03-15 at 2 33 26 pm

Instead of the expected result when you initially set loaded to TRUE:

screen shot 2018-03-15 at 2 33 13 pm

For the correct version, these assets are loaded in this order:

datatables-crosstalk.css
datatables.js
dataTables.bootstrap.min.css
dataTables.bootstrap.extra.css
jquery.dataTables.min.js
dataTables.bootstrap.min.js

But when the table is loaded dynamically via the action button, you get this:

datatables-crosstalk.css
datatables.js
jquery.dataTables.min.css
jquery.dataTables.extra.css
jquery.dataTables.min.js
dataTables.bootstrap.min.css
dataTables.bootstrap.extra.css
jquery.dataTables.min.js
dataTables.bootstrap.min.js

It seems like what's happening is that DT doesn't know you're going to use Bootstrap style tables, so it loads the assets for the default theme even though they will never be needed.

I was able to fix this particular example by changing if (!is_loaded()) return(NULL) to req(is_loaded()), but I'm sure a lot of people are still using the return(NULL) pattern.

Furthermore, due to this same CSS incompatibility, you cannot mix Bootstrap and non-Bootstrap tables in the same app, as this test confirms:

screen shot 2018-03-15 at 2 41 09 pm

One possible solution would be to namespace the two different table styles, but I think a better solution would be to just make the table style a global option.

shrektan commented 6 years ago

Thanks for the great reporting! One of the best.

It should be a long-existed issue because I can remember that I encountered the incompatible of bootstrap style one or two years ago (but I simply choosed not to use the bootstrap style instead of digging into it).

@yihui Any idea of how to fix this easily?

yihui commented 6 years ago

Unfortunately I don't think it is easy to fix. @dpmccabe mentioned two issues:

  1. If you return(NULL), the (delayed) Bootstrap style won't work. The solution is req().

  2. Bootstrap and non-Bootstrap styles cannot be used on the same page. That has been documented for long: https://rstudio.github.io/DT/ (Section 2.2).

So I guess there isn't much we could do here. Sorry.

shrektan commented 4 years ago

It seems like we should have a global style option when DT is used in shiny. Otherwise, if there're two tables one uses the bootstrap style while the other uses the default style, there will be one table displayed strangely.