insightsengineering / teal

Exploratory Web Apps for Analyzing Clinical Trial Data
https://insightsengineering.github.io/teal/
Other
167 stars 32 forks source link

modify `hideSidebar` and `showSidebar` funcitons to work in embedded apps #1239

Closed chlebowa closed 2 weeks ago

chlebowa commented 4 weeks ago

BACKGROUND

I am trying to embed a teal app in a shiny app (shiny is parent and teal is child). I want to be able to go back and forth between configuring the teal app and inspecting it.

The teal app is created by calling module_teal_with_splash, with ui_teal_with_splash placed in a renderUI call and shinyjs::hidden. When configutraion is ready, an action button runs shonyjs::show on the module UI and calls the module server function. When the user wants to go back to configuration, the child app is hidden again. A new child is run with a different, incremented id.

PROBLEM

I am having some trouble with the filter panel that results from the current implementation of showSidebar and hideSidebar.

Over a single session of the parent app, when the child is rendered for the first time, the filter panel can be toggled normally. However, when I generate the child app for the second time, the JS console throws an error and toggling is no longer possible. The filter panel is expanded but invisible because it retains display: none.

NOTE

The bug is only triggered by modules that contain plots. Modules with tables or simple prints (like example_module or tm_t_crosstable) work just fine but once the bug is triggerred, those too are affected.

SOLUTION

I modified the script that toggles the filter panel to get around the issue.

  1. The resize method throws an error (even thought the resizing does occur) so showSidebar exits early and fadeIn does not kick in. I moved resize to the end of the function.
  2. There seems to be a conflict between $(".teal_secondary_col").fadeIn(50) and $(".teal_primary_col").resize() that prevents resizing. I replaced fadeOut/In with changing the display to none/block, respectively, and used setTimeout because delay didn't seem to work.
averissimo commented 2 weeks ago

Thanks for the PR @chlebowa the JS code seems equivalent.

Can you share a simple reproducible example?

I've tried to create an example to understand the issue but couldn't.

My attempt using {teal.modules.general} scatterplot and a custom module: (screencast at the end)

Example code ```r library(ggplot2) mods <- teal::modules( teal.modules.general::tm_g_scatterplot( x = teal.transform::data_extract_spec( dataname = "iris", select = teal.transform::select_spec( label = "Select variable:", choices = teal.transform::variable_choices("iris", c("Sepal.Length", "Sepal.Width", "Petal.Length", "Petal.Width")), selected = "Sepal.Length", multiple = FALSE, fixed = FALSE ) ), y = teal.transform::data_extract_spec( dataname = "iris", select = teal.transform::select_spec( label = "Select variable:", choices = teal.transform::variable_choices("iris", c("Sepal.Length", "Sepal.Width", "Petal.Length", "Petal.Width")), selected = "Sepal.Width", multiple = FALSE, fixed = FALSE ) ), ), teal::module( label = "Custom", ui = function(id) { ns <- shiny::NS(id) shiny::tags$div( shiny::selectInput(ns("species"), "Species", choices = character(0L)), shiny::plotOutput(ns("plot")) ) }, server = function(id, data) { shiny::moduleServer(id, function(input, output, session) { iris <- shiny::reactive(data()[["iris"]]) species <- shiny::reactive(input$species) shiny::updateSelectInput(session, "species", choices = unique(iris()$Species)) output$plot <- renderPlot({ shiny::req(species()) iris() |> dplyr::filter(Species == species()) |> ggplot2::ggplot(ggplot2::aes(Sepal.Length, Sepal.Width)) + ggplot2::geom_point() }) }) } ) ) # Shiny app with modular integration of teal ui <- shiny::fluidPage( shiny::fluidPage( shiny::h3("There is a hidden teal app here"), shiny::actionButton("show", "Show teal app"), shiny::uiOutput("app1_wrapper") ) ) server <- function(input, output, session) { shiny::observe({ shinyjs::show(id = "app1_wrapper") output$app1_wrapper <- shiny::renderUI({ shiny::tagList( shinyjs::hidden(numericInput("n1", "n1", value = 0)), teal::ui_teal_with_splash( id = "app1", data = teal.data::teal_data() ) ) }) }) |> shiny::bindEvent(input$show, ignoreInit = TRUE, ignoreNULL = TRUE) observe({ req(input$n1) shinyjs::disable(id = "show") teal::srv_teal_with_splash( id = "app1", data = teal_data(iris = iris), modules = mods ) }) } shiny::shinyApp(ui, server) ```

Screencast from 2024-06-18 16-11-32.webm

chlebowa commented 2 weeks ago

I am unable to provide a screencast but here is an example app.

exapmle app ``` library(shiny) library(shinyjs) devtools::load_all("./teal") library(teal.modules.general) library(teal.widgets) rm(list = ls()) # generate incremented id for teal module call teal_module_id <- function(counter) { sprintf("teal_module_%i", counter) } # generate teal module ui and plac eit in a hidden div teal_app_ui <- function(counter) { div( id = "teal_app_container", ui_teal_with_splash( id = teal_module_id(counter), data = teal_data() ) ) |> hidden() } ui <- fluidPage( title = "teal app generator", useShinyjs(), # this panel switches between teal app configuration and inspection uiOutput("control_panel"), uiOutput("app_configuration"), div(id = "teal_app_inspection", teal_app_ui(0L)), NULL ) server <- function(input, output, session) { # buttons to alternate between configuration and inspection screens output[["control_panel"]] <- renderUI({ actionButton("master_button", label = "generate app") #|> disabled() }) # configuration screen output[["app_configuration"]] <- renderUI({ dataset_choices <- c("iris", "mtcars") module_choices <- c("tm_g_distribution", "tm_t_crosstable") div( id = "teal_app_controls", selectInput("datasets", "choose datasets", choices = dataset_choices, multiple = TRUE, selected = dataset_choices), selectizeInput("modules", "choose modules", choices = module_choices, multiple = TRUE, selected = module_choices) ) }) # disable master button if not ready to build app observe({ toggleState("master_button", condition = isTruthy(input[["datasets"]]) && isTruthy(input[["modules"]])) }) # inspection screen # app ui is wrapped in a div so that it has an id that can be toggled # teal module id is incremented on every inspection to avoid reusing observers teal_app_counter <- reactiveVal(0L) # build teal_data object for teal app # note this re-runs every time datasets change so it may cause performance problems as dataset code must be evaluated teal_app_data <- reactive({ validate(need(input[["datasets"]], "select some datasets")) validate(need(input[["modules"]], "select some modules")) datasets <- mget(input$datasets, envir = as.environment("package:datasets")) datasets_code <- do.call(expression, lapply(sprintf("%s <- %s", input$datasets, input$datasets), str2lang)) ans <- do.call("teal_data", c(datasets, list(code = datasets_code))) |> verify() ans <- within(ans, { for (v in c("cyl", "vs", "am", "gear")) { mtcars[[v]] <- as.factor(mtcars[[v]]) } mtcars[["primary_key"]] <- seq_len(nrow(mtcars)) }) join_keys(ans) <- join_keys(join_key("mtcars", "mtcars", "primary_key")) ans }) # build teal_modules object for teal app teal_app_modules <- reactive({ req(input[["modules"]]) module_funs <- mget(input$modules, mode = "function", inherits = TRUE) module_args <- lapply(input$modules, function(x) { switch(x, "example_module" = list(), "tm_g_distribution" = list( dist_var = data_extract_spec( dataname = "iris", select = select_spec(variable_choices("iris"), "Petal.Length") ) ), "tm_g_response" = list( response = data_extract_spec( dataname = "mtcars", select = select_spec( label = "Select variable:", choices = variable_choices("mtcars", c("cyl", "gear")), selected = "cyl", multiple = FALSE, fixed = FALSE ) ), x = data_extract_spec( dataname = "mtcars", select = select_spec( label = "Select variable:", choices = variable_choices("mtcars", c("vs", "am")), selected = "vs", multiple = FALSE, fixed = FALSE ) ) ), "tm_t_crosstable" = list( label = "Cross Table", x = data_extract_spec( dataname = "mtcars", select = select_spec( label = "Select variable:", choices = variable_choices("mtcars", c("cyl", "vs", "am", "gear")), selected = c("cyl", "gear"), multiple = TRUE, ordered = TRUE, fixed = FALSE ) ), y = data_extract_spec( dataname = "mtcars", select = select_spec( label = "Select variable:", choices = variable_choices("mtcars", c("cyl", "vs", "am", "gear")), selected = "vs", multiple = FALSE, fixed = FALSE ) ), basic_table_args = basic_table_args( subtitles = "Table generated by Crosstable Module" ) ) ) }) module_objs <- mapply(FUN = do.call, what = module_funs, args = module_args, SIMPLIFY = FALSE) do.call("modules", module_objs) }) # track display state areweinspecting <- reactiveVal(FALSE) # switch between app configuration and inspection observeEvent(input[["master_button"]], { areweinspecting(!areweinspecting()) if (!areweinspecting()) { teal_app_counter(teal_app_counter() + 1L) removeUI("#teal_app_container") insertUI( "#teal_app_inspection", "afterBegin", teal_app_ui(teal_app_counter()) ) } if (areweinspecting()) { srv_teal_with_splash( id = teal_module_id(teal_app_counter()), data = teal_app_data(), modules = teal_app_modules() ) } toggleElement("teal_app_container", condition = isTRUE(areweinspecting())) toggleElement("teal_app_controls", condition = isFALSE(areweinspecting())) updateActionButton(inputId = "master_button", label = if (areweinspecting()) "configure app" else "generate app") }) } shinyApp(ui, server, options = list("launch.browser" = TRUE)) ```

Generate an app, go back to configuration, then generate the same app again, and try clicking the hamburger button.

This one uses insertUI/removeUI to replace teal apps. I have another one that uses renderUI and it works pretty much the same (both problem and solution).

m7pr commented 2 weeks ago

Hey @chlebowa something is off with one of the tests https://github.com/insightsengineering/teal/actions/runs/9566206096/job/26373606743?pr=1239#step:41:152

chlebowa commented 2 weeks ago

It is but I am having trouble running that one locally. I will try again.

chlebowa commented 2 weeks ago

@m7pr done

averissimo commented 2 weeks ago

It seems that as soon as the teal UI is removed $(window).trigger("resize") starts to give out errors.

Edit: There's one error with the re-declaration of const of the JS functions

However, even if this is resolved, the resize event will always fail (just as resizing the window)

This is a problem with the example though, so I guess we can merge this PR as it improves the JS code.

chlebowa commented 2 weeks ago

plots no longer resize

It seems that as soon as the teal UI is removed $(window).trigger("resize") starts to give out errors.

Yes, I've seen that but I decided it's something I can live with or perhaps solve later.

Edit: There's one error with the re-declaration of const of the JS functions

I only see the clientWidth error. In any case, re-declarations fail and the consts stay in place, so the whole script works as expected. That is also something I can live with.

Thank you, I applied your suggestions.

m7pr commented 2 weeks ago

@averissimo you need to merge, as @chlebowa does not have priviledges :)