rstudio / DT

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

FixedColumns extension causes invalid background color, with DT 0.20 #946

Open ghost opened 2 years ago

ghost commented 2 years ago

This image shows the difference

    ---
    title: "test"
    output:
      html_document:
        self_contained: false
        theme: 
          version: 4
          bootswatch: superhero
        highlight: tango
    ---

    ```{r setup, include=FALSE}
    knitr::opts_chunk$set(echo = TRUE)

    library(bslib)
    library(DT)

    data(mtcars)
## R Markdown

```{r}
datatable(mtcars,
          extensions = "FixedColumns")

> xfun::session_info('DT')

R version 4.1.2 (2021-11-01) Platform: x86_64-w64-mingw32/x64 (64-bit) Running under: Windows 10 x64 (build 19043), RStudio 1.4.1717

Locale: LC_COLLATE=Greek_Greece.1253 LC_CTYPE=Greek_Greece.1253
LC_MONETARY=Greek_Greece.1253 LC_NUMERIC=C
LC_TIME=Greek_Greece.1253

Package version: base64enc_0.1.3 crosstalk_1.2.0 digest_0.6.28 DT_0.20
fastmap_1.1.0 graphics_4.1.2 grDevices_4.1.2 htmltools_0.5.2
htmlwidgets_1.5.4 jquerylib_0.1.4 jsonlite_1.7.2 later_1.3.0
lazyeval_0.2.2 magrittr_2.0.1 methods_4.1.2 promises_1.2.0.1 R6_2.5.1 Rcpp_1.0.7 rlang_0.4.12 stats_4.1.2
utils_4.1.2 yaml_2.2.1



<!--
Please keep the portion below in your issue. Your issue will be closed if any of the boxes is not checked. In certain (rare) cases, you may be exempted if you give a brief explanation (e.g., you are only making a suggestion for improvement). Thanks!
-->

---

By filing an issue to this repo, I promise that

- [ ] I have fully read the issue guide at https://yihui.name/issue/.
- [ ] I have provided the necessary information about my issue.
    - If I'm asking a question, I have already asked it on Stack Overflow or RStudio Community, waited for at least 24 hours, and included a link to my question there.
    - If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included `xfun::session_info('DT')`. I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: `remotes::install_github('rstudio/DT')`.
    - If I have posted the same issue elsewhere, I have also mentioned it in this issue.
- [ ] I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.
shrektan commented 2 years ago

I believe it's a bug of datatable library itself, as I created two jsfiddle examples, with or without fixedcolumns extension, and found that with fixedcolumns extension's css the theme of the table won't be adjusted to the bootstrap theme automatically.

Without FixedColumns

https://jsfiddle.net/shrektan/cL9artsm/2/

image

With FixedColumns

https://jsfiddle.net/shrektan/0tsoezd2/4/

image

I've filed a bug report in https://datatables.net/forums/discussion/70753/bug-report-the-latest-v4-0-1-fixedcolumns-extension-doesnt-work-with-bootstrap-themes/p1?new=1

shrektan commented 2 years ago

@gd047 However, even in v0.19, the background color of the fixed columns was not correct, either. The differences only happen in the non-fixed columns.

htmltools::browsable({
  shiny::fluidPage(
    theme = bslib::bs_theme(4, "superhero"),
    h1(sprintf("DT version %s", packageVersion("DT"))),
    DT::datatable(
      width = "500px",
      mtcars, 
      style = "bootstrap4",
      options = list(paging = FALSE, scrollY="300px", scrollX=TRUE, fixedColumns=TRUE),
      extensions = "FixedColumns"
    )
  )
})
image image
shrektan commented 2 years ago

Hi @gd047 ,

Based on the reply from https://datatables.net/forums/discussion/comment/200284#Comment_200284 , the hot-fix to this issue would be add a customized css :

htmltools::browsable({
  shiny::fluidPage(
    theme = bslib::bs_theme(4, "superhero"),
###### The customized CSS #########
    htmltools::tags$head(
      htmltools::tags$style(
        type = "text/css",
        "thead th, tr.odd td {
           background-color: #2b3e50 !important;
         }
         tr.even td {
           background-color: #384f66 !important;
         }"
      )
    ),
###### ################ #########
    htmltools::h1(sprintf("DT version %s", packageVersion("DT"))),
    DT::datatable(
      width = "500px",
      mtcars, 
      style = "bootstrap4",
      options = list(paging = FALSE, scrollY="300px", scrollX=TRUE, fixedColumns=TRUE),
      extensions = "FixedColumns"
    )
  )
})
image
ghost commented 2 years ago

@shrektan Thanks. Could you give an example of this hot-fix for the rmarkdown example in OP?

shrektan commented 2 years ago

@gd047

---
title: "test"
output:
  html_document:
    self_contained: false
    theme: 
      version: 4
      bootswatch: superhero
    highlight: tango
---

<style type="text/css">
thead th, tr.odd td {
  background-color: #2b3e50 !important;
}
tr.even td {
  background-color: #384f66 !important;
}
</style>

```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = TRUE)
library(bslib)
library(DT)
data(mtcars)

R Markdown

datatable(mtcars, extensions = "FixedColumns")
ghost commented 2 years ago

@shrektan In this way, however, the color will not be adjusted when changing bootstrap theme. I was thinking of a way to define the color that would make use of a command like the one below: bs_get_variables(bslib::bs_current_theme(), "primary")

Is it possible to use r variables in css code chunk?

Another way could be to use the table options. See for example this answer

shrektan commented 2 years ago

@gd047

Including the following css code should be able to dynamically fix this

thead th, tr td {
  background-color: var(--bs-table-active-bg) !important;
  color: var(--bs-table-active-color) !important;
}

The full rmarkdown code

---
title: "test"
output:
  html_document:
    self_contained: false
    theme: 
      version: 4
      bootswatch: superhero
    highlight: tango
---

```{css, echo=FALSE}
thead th, tr td {
  background-color: var(--bs-table-active-bg) !important;
  color: var(--bs-table-active-color) !important;
}
knitr::opts_chunk$set(echo = TRUE)
library(bslib)
library(DT)
data(mtcars)

R Markdown

datatable(mtcars, extensions = "FixedColumns")
jjfallon commented 2 years ago

These changes to FixedColumns also means that if you use formatStyle to change the row background colour, this is now ignored because styling of the cells from FixedColumns takes precedence. For example,

---
title: "test"
output:
  html_document:
    self_contained: false
---

```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = TRUE)

library(DT)
data(mtcars)
```

## R Markdown

```{r}
datatable(mtcars, extensions = "FixedColumns") %>%
  formatStyle('gear', target = 'row', backgroundColor = styleEqual(3, "coral"))
```

So in DT 0.18 you see rows coloured coral, but you don't in DT 0.20. Is this the same issue as above (it certainly seems to be related as it involves fixed columns and background colours) and will hopefully be fixed up stream, or is it was worth flagging as a separate issue?

shrektan commented 2 years ago

@jjfallon , I can reproduce your issue.

  1. I don't think the "upstream" would fix this. But I suggest you to open an issue with jsfiddle code as I did in https://datatables.net/forums/discussion/comment/200284#Comment_200284 , if you think it's necessary.

  2. The reason of this issue is that FixedColumn extension has to set a background color explicitly for tr.even td and tr.odd td. Otherwise, the fixed column's text would overlap with other columns. I'm not an expert of CSS but I do think the background color should be set in a way that responses to the theme.

  3. The hotfix for your problem is to apply the style on cell instead of row, as I do below.

library(DT)
datatable(mtcars, options = list(scrollX = TRUE, fixedColumns = TRUE), extensions = "FixedColumns", width = "400px") |>
  formatStyle(columns = 0:ncol(mtcars), valueColumns = 'gear', target = 'cell', backgroundColor = styleEqual(4, "coral"))