[Bug]: DT validation errors appear floating

[averissimo]

What happened?

On teal.gallery Exploratory app the validation errors from encodings is floating.

This is a teal-wide problem when rendering DT:datatable with validation errors. (the problem is very obvious on teal.modules.general::tm_missing())

How to reproduce

• Open application
• Navigate to the Missing Data tab
• Select ADLBPCA dataset (inner tabs on main panel)
• Select By Variable Levels sub-tab

Is this behaviour acceptable?

sessionInfo()

R version 4.3.1 (2023-06-16)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.6 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/atlas/libblas.so.3.10.3 
LAPACK: /usr/lib/x86_64-linux-gnu/atlas/liblapack.so.3.10.3;  LAPACK version 3.9.0

locale:
 [1] LC_CTYPE=C.UTF-8       LC_NUMERIC=C           LC_TIME=C.UTF-8       
 [4] LC_COLLATE=C.UTF-8     LC_MONETARY=C.UTF-8    LC_MESSAGES=C.UTF-8   
 [7] LC_PAPER=C.UTF-8       LC_NAME=C              LC_ADDRESS=C          
[10] LC_TELEPHONE=C         LC_MEASUREMENT=C.UTF-8 LC_IDENTIFICATION=C   

time zone: Etc/UTC
tzcode source: system (glibc)

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
 [1] sparkline_2.0                    rtables_0.6.2.9001              
 [3] formatters_0.5.1.9001            rlang_1.1.1                     
 [5] MASS_7.3-60                      lattice_0.21-8                  
 [7] jsonlite_1.8.7                   htmlwidgets_1.6.2               
 [9] gridExtra_2.3                    goftest_1.2-3                   
[11] ggpmisc_0.5.3                    ggpp_0.5.3                      
[13] ggExtra_0.10.0                   colourpicker_1.2.0              
[15] broom_1.0.5                      nestcolor_0.1.2.9001            
[17] tidyr_1.3.0                      dplyr_1.1.2                     
[19] scda.2022_0.1.5.9000             scda_0.1.6.9010                 
[21] teal.modules.general_0.2.15.9033 teal_0.13.0.9023                
[23] teal.transform_0.3.0.9009        magrittr_2.0.3                  
[25] teal.slice_0.3.0.9028            teal.data_0.2.0.9007            
[27] shinyTree_0.2.7                  ggmosaic_0.3.3                  
[29] ggplot2_3.4.2                    shiny_1.7.4.1                   

loaded via a namespace (and not attached):
 [1] tidyselect_1.2.0         viridisLite_0.4.2        farver_2.1.1            
 [4] R.utils_2.12.2           fastmap_1.1.1            lazyeval_0.2.2          
 [7] promises_1.2.0.1         shinyjs_2.1.0            digest_0.6.33           
[10] mime_0.12                lifecycle_1.0.3          ellipsis_0.3.2          
[13] survival_3.5-5           compiler_4.3.1           sass_0.4.7              
[16] tools_4.3.1              utf8_1.2.3               yaml_2.3.7              
[19] data.table_1.14.8        knitr_1.43               labeling_0.4.2          
[22] R.cache_0.16.0           miniUI_0.1.1.1           withr_2.5.0             
[25] purrr_1.0.1              R.oo_1.25.0              shinyWidgets_0.7.6      
[28] grid_4.3.1               fansi_1.0.4              teal.logger_0.1.1.9009  
[31] xtable_1.8-4             colorspace_2.1-0         scales_1.2.1            
[34] cli_3.6.1                rmarkdown_2.23           generics_0.1.3          
[37] httr_1.4.6               polynom_1.4-1            cachem_1.0.8            
[40] stringr_1.5.0            splines_4.3.1            vctrs_0.6.3             
[43] Matrix_1.6-0             SparseM_1.81             ggrepel_0.9.3           
[46] crosstalk_1.2.0          teal.widgets_0.3.0.9007  plotly_4.10.2           
[49] fontawesome_0.5.1        jquerylib_0.1.4          glue_1.6.2              
[52] DT_0.28                  stringi_1.7.12           gtable_0.3.3            
[55] later_1.3.1              shinycssloaders_1.0.0    munsell_0.5.0           
[58] tibble_3.2.1             styler_1.10.1            logger_0.2.2            
[61] pillar_1.9.0             htmltools_0.5.5          quantreg_5.96           
[64] R6_2.5.1                 evaluate_0.21            R.methodsS3_1.8.2       
[67] backports_1.4.1          memoise_2.0.1            teal.reporter_0.1.1.9020
[70] httpuv_1.6.11            bslib_0.5.0              MatrixModels_0.5-2      
[73] Rcpp_1.0.11              teal.code_0.3.0.9009     shinyvalidate_0.1.2     
[76] checkmate_2.2.0          xfun_0.39                forcats_1.0.0           
[79] pkgconfig_2.0.3

Relevant log output

No response

Code of Conduct

• [X] I agree to follow this project's Code of Conduct.

Contribution Guidelines

• [X] I agree to follow this project's Contribution Guidelines.

Security Policy

• [X] I agree to follow this project's Security Policy.

[averissimo]

This can also be seen at teal.modules.general::tm_g_distribution() module on tests table (bottom of the panel widget)

It's not as obvious, but it's rendered outside the parent bounding box.

Why? It takes the height of the output, which for datatables is 0px. This is not seen on plots as it has a non-zero height.

[averissimo]

This is an possible issue that affects any module that renders a datatable

[averissimo]

The CSS code chunk below will solve the issue by using the !important property to overwrite all styling on the element.

It's not a best-practices solution, but the alternatives might add a lot of complexity

.datatables.html-widget + .shiny-output-error.shiny-output-error-validation.htmlwidgets-error {
  /*
  `position`, `height`, `top` and `left` need to be overwritten as they're
  defined dynamically in the element by shiny::validate
  */
  position: relative !important;
  height: auto !important;
  top: 0 !important;
  left: 0 !important;
  padding-top: 1em;
}

Reproducible solution proposal

⊞ Click here to see code on how to reproduce the solution

style_me <- "
.datatables.html-widget + .shiny-output-error.shiny-output-error-validation.htmlwidgets-error {
  /*
  `position`, `height`, `top` and `left` need to be overwritten as they're
  defined dynamically in the element by shiny::validate
  */
  position: relative !important;
  height: auto !important;
  top: 0 !important;
  left: 0 !important;
  padding-top: 1em;
}
"

library(teal.modules.general)
library(scda)
library(scda.2022)
library(dplyr)
library(tidyr)
library(nestcolor)
# optional libraries
library(broom)
library(colourpicker)
library(ggExtra)
library(ggpmisc)
library(ggpp)
library(goftest)
library(gridExtra)
library(htmlwidgets)
library(jsonlite)
library(lattice)
library(MASS)
library(rlang)
library(rtables)
library(sparkline)


options(shiny.useragg = FALSE)

# code>
ADSL <- synthetic_cdisc_data("latest")$adsl
ADLB <- synthetic_cdisc_data("latest")$adlb

ADLBPCA <- ADLB %>%
  dplyr::select(USUBJID, STUDYID, SEX, ARMCD, AVAL, AVISIT, PARAMCD) %>%
  tidyr::pivot_wider(
    values_from = "AVAL",
    names_from = c("PARAMCD", "AVISIT"),
    names_sep = " - "
  )

adsl <- cdisc_dataset("ADSL", ADSL, code = "ADSL <- synthetic_cdisc_data(\"latest\")$adsl")
adlb <- cdisc_dataset("ADLB", ADLB, code = "ADLB <- synthetic_cdisc_data(\"latest\")$adlb")
adlbpca <- cdisc_dataset(
  "ADLBPCA",
  ADLBPCA,
  code = 'ADLBPCA <- ADLB %>%
    dplyr::select(USUBJID, STUDYID, SEX, ARMCD, AVAL, AVISIT, PARAMCD) %>%
    tidyr::pivot_wider(
      values_from = "AVAL",
      names_from = c("PARAMCD", "AVISIT"),
      names_sep = " - "
    )',
  keys = c("STUDYID", "USUBJID"),
  label = "ADLB reshaped",
  vars = list(ADLB = adlb)
)
# <code

app <- init(
  data = cdisc_data(adlbpca, adsl),
  modules = modules(
    tm_missing_data("Missing Data")
  ),
  header = div(
    class = "",
    style = "margin-bottom: 2px;",
    tags$h1("Example App with teal.modules.general modules", tags$span("SPA", class = "pull-right")),
    tags$style(style_me)
  ),
  footer = tags$p(class = "text-muted", "Source: teal.gallery package")
)

shinyApp(app$ui, app$server)

[donyunardi]

Yeah, I am fine with this. This is very similar with the approach the I suggested here.

We do have a vignette that talks about custom teal css: https://insightsengineering.github.io/teal/latest-tag/articles/teal-bs-themes.html#custom-teal-css

Maybe we can add this information there with example to retain the knowledge.

[donyunardi]

I've been pondering this issue. Although it's convenient that we discovered a method to inject the CSS, it doesn't seem practical to repeatedly request users to do this, especially when a module employs DT::datatable.

calling out the @insightsengineering/nest-core-dev to see if there's other suggestion.

[averissimo]

Oh no, my idea is to add this to teal/inst/css/custom.css the PoC is just a way to easily replicate the solution

[chlebowa]

An early version of teal::validate_inputs was able to pass errorClass to shiny::validate. This is now not possible because ... only takes validators but a special argument can be added. Then that CSS will be applied to all such messages.

[averissimo]

@chlebowa I like the explicit nature of adding the class.

It would add some overhead for module developers as they'll need to know if a datatable will be rendered in the output. On the other hand, it doesn't become a "too" generic solution by catching all error elements after a datatable.

I found now that there's an issue with the approach above :-\ ... a min-length CSS/javascript might be the better solution after all (or having a datatables wrapper).

• A datatable is rendered correctly (switching to SEX on group by variable)
• Return to error state (back to USUBJID on group by variable)

The errors appear after the (empty datatable)

[chlebowa]

Funnily enough, when I run the example app from tm_missing_data, the "By Variable Levels" tab does not seem to capture validator messages at all.

[averissimo]

I've created an issue upstream: https://github.com/rstudio/DT/issues/1084

It would be better to have this fixed on DT instead of having to implement a JS hack ourselves.

I have an idea on how to do it, but would like to hold off.

Listen to an event on DT class:

• if height is zero, then add class to the sibling's error element
• if not, make sure it doesn't have a class

This would need to create a resize observer for existing DT elements and account for re-renderings.

OR use a min-height CSS rule and hope that we don't get very tall error messages :sweat_smile:

[averissimo]

@chlebowa that seems to be an alternate solution, embed the message on the data table itself :thinking:

Although a complete message would be preferable