Add landing popup argument in init and soft deprecate calling it as…

… a normal module (#1284) Changes: 1. Soft deprecates calling the `landing_popup_module` inside `modules` argument of `init` 2. Add a new `landing_popup` argument to `init` to accept this module. 3. Update the `TealAppDriver` to accept this new argument. --------- Signed-off-by: Vedha Viyash <49812166+vedhav@users.noreply.github.com> Co-authored-by: Dawid Kałędkowski <6959016+gogonzo@users.noreply.github.com> Co-authored-by: go_gonzo <dawid.kaledkowski@gmail.com>
insightsengineering · Aug 2, 2024 · ab8ac73 · ab8ac73
1 parent da091b6
commit ab8ac73
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 64 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,8 @@
 # teal 0.15.2.9047
 
+### Breaking changes
+* The `landing_popup_module()` needs to be passed as the `landing_popup` argument of `init` instead of being passed as a module of the `modules` argument of `init`
+
 ### Enhancement
 * Provided progress bar for modules loading and data filtering during teal app startup.
 

diff --git a/R/TealAppDriver.R b/R/TealAppDriver.R
@@ -15,7 +15,7 @@ TealAppDriver <- R6::R6Class( # nolint: object_name.
     #' @description
     #' Initialize a `TealAppDriver` object for testing a `teal` application.
     #'
-    #' @param data,modules,filter,title,header,footer arguments passed to `init`
+    #' @param data,modules,filter,title,header,footer,landing_popup arguments passed to `init`
     #' @param timeout (`numeric`) Default number of milliseconds for any timeout or
     #' timeout_ parameter in the `TealAppDriver` class.
     #' Defaults to 20s.
@@ -37,6 +37,7 @@ TealAppDriver <- R6::R6Class( # nolint: object_name.
                           title = build_app_title(),
                           header = tags$p(),
                           footer = tags$p(),
+                          landing_popup = NULL,
                           timeout = rlang::missing_arg(),
                           load_timeout = rlang::missing_arg(),
                           ...) {
@@ -49,7 +50,8 @@ TealAppDriver <- R6::R6Class( # nolint: object_name.
         filter = filter,
         title = title,
         header = header,
-        footer = footer
+        footer = footer,
+        landing_popup = landing_popup,
       )
 
       # Default timeout is hardcoded to 4s in shinytest2:::resolve_timeout

diff --git a/R/init.R b/R/init.R
@@ -34,6 +34,8 @@
 #' @param id (`character`) optional
 #'   string specifying the `shiny` module id in cases it is used as a `shiny` module
 #'   rather than a standalone `shiny` app. This is a legacy feature.
+#' @param landing_popup (`teal_module`) optional
+#'   A `landing_popup_module` to show up as soon as the teal app is initialized.
 #'
 #' @return Named list containing server and UI functions.
 #'
@@ -99,7 +101,8 @@ init <- function(data,
                  title = build_app_title(),
                  header = tags$p(),
                  footer = tags$p(),
-                 id = character(0)) {
+                 id = character(0),
+                 landing_popup = NULL) {
   logger::log_debug("init initializing teal app with: data ('{ class(data) }').")
 
   # argument checking (independent)
@@ -115,6 +118,7 @@ init <- function(data,
     )
   }
   checkmate::assert_multi_class(data, c("teal_data", "teal_data_module"))
+  checkmate::assert_class(landing_popup, "teal_module_landing", null.ok = TRUE)
 
   ## `modules`
   checkmate::assert(
@@ -159,10 +163,15 @@ init <- function(data,
   # argument transformations
   ## `modules` - landing module
   landing <- extract_module(modules, "teal_module_landing")
-  landing_module <- NULL
   if (length(landing) == 1L) {
-    landing_module <- landing[[1L]]
+    landing_popup <- landing[[1L]]
     modules <- drop_module(modules, "teal_module_landing")
+    # TODO: verify the version before release.
+    lifecycle::deprecate_soft(
+      when = "0.16",
+      what = "landing_popup_module()",
+      details = "Pass `landing_popup_module` to the `landing_popup` argument of the `init` instead of wrapping it into `modules()` and passing to the `modules` argument"
+    )
   } else if (length(landing) > 1L) {
     stop("Only one `landing_popup_module` can be used.")
   }
@@ -242,11 +251,8 @@ init <- function(data,
       )
     },
     server = function(input, output, session) {
-      # todo: remove landing page from here to srv_teal
-      #       - fix srv_data first to not use modal (can't display two in the same time)
-      #       https://github.com/insightsengineering/teal/issues/1244
-      if (!is.null(landing_module)) {
-        do.call(landing_module$server, c(list(id = "landing_module_shiny_id"), landing_module$server_args))
+      if (!is.null(landing_popup)) {
+        do.call(landing_popup$server, c(list(id = "landing_module_shiny_id"), landing_popup$server_args))
       }
       srv_teal(id = ns("teal"), data = data, modules = modules, filter = deep_copy_filter(filter))
     }

diff --git a/R/landing_popup_module.R b/R/landing_popup_module.R
@@ -17,11 +17,11 @@
 #' app1 <- init(
 #'   data = teal_data(iris = iris),
 #'   modules = modules(
-#'     landing_popup_module(
-#'       content = "A place for the welcome message or a disclaimer statement.",
-#'       buttons = modalButton("Proceed")
-#'     ),
 #'     example_module()
+#'   ),
+#'   landing_popup = landing_popup_module(
+#'     content = "A place for the welcome message or a disclaimer statement.",
+#'     buttons = modalButton("Proceed")
 #'   )
 #' )
 #' if (interactive()) {
@@ -31,21 +31,21 @@
 #' app2 <- init(
 #'   data = teal_data(iris = iris),
 #'   modules = modules(
-#'     landing_popup_module(
-#'       title = "Welcome",
-#'       content = tags$b(
-#'         "A place for the welcome message or a disclaimer statement.",
-#'         style = "color: red;"
-#'       ),
-#'       buttons = tagList(
-#'         modalButton("Proceed"),
-#'         actionButton("read", "Read more",
-#'           onclick = "window.open('http://google.com', '_blank')"
-#'         ),
-#'         actionButton("close", "Reject", onclick = "window.close()")
-#'       )
-#'     ),
 #'     example_module()
+#'   ),
+#'   landing_popup = landing_popup_module(
+#'     title = "Welcome",
+#'     content = tags$b(
+#'       "A place for the welcome message or a disclaimer statement.",
+#'       style = "color: red;"
+#'     ),
+#'     buttons = tagList(
+#'       modalButton("Proceed"),
+#'       actionButton("read", "Read more",
+#'                    onclick = "window.open('http://google.com', '_blank')"
+#'       ),
+#'       actionButton("close", "Reject", onclick = "window.close()")
+#'     )
 #'   )
 #' )
 #'

diff --git a/R/module_teal.R b/R/module_teal.R
@@ -5,6 +5,10 @@
 #' Module to create a `teal` app. This module (`ui` and `server`) is called directly by [init()] after
 #' initial argument checking and setting default values. This module can be called directly and
 #' included in your custom application.
+#'
+#' Please note that [init()] adds `reporter_previewer_module` automatically, which is not a case
+#' when calling `ui/srv_teal` directly.
+#'
 #' Module is responsible for creating the main `shiny` app layout and initializing all the necessary
 #' components. This module establishes reactive connection between the input `data` and every other
 #' component in the app. Reactive change of the `data` triggers reload of the app and possibly

diff --git a/man/TealAppDriver.Rd b/man/TealAppDriver.Rd
diff --git a/man/init.Rd b/man/init.Rd
diff --git a/man/landing_popup_module.Rd b/man/landing_popup_module.Rd
diff --git a/man/module_teal.Rd b/man/module_teal.Rd
diff --git a/tests/testthat/test-shinytest2-landing_popup.R b/tests/testthat/test-shinytest2-landing_popup.R
@@ -3,11 +3,11 @@ testthat::test_that("e2e: teal app with landing_popup_module initializes with no
   app <- TealAppDriver$new(
     data = simple_teal_data(),
     modules = modules(
-      landing_popup_module(
-        title = "Welcome",
-        content = tags$b("A welcome message!", style = "color: red;")
-      ),
       example_module()
+    ),
+    landing_popup = landing_popup_module(
+      title = "Welcome",
+      content = tags$b("A welcome message!", style = "color: red;")
     )
   )
 
@@ -23,9 +23,9 @@ testthat::test_that("e2e: app with default landing_popup_module creates modal co
   app <- TealAppDriver$new(
     data = simple_teal_data(),
     modules = modules(
-      landing_popup_module(),
       example_module()
-    )
+    ),
+    landing_popup = landing_popup_module()
   )
 
   testthat::expect_equal(
@@ -41,9 +41,9 @@ testthat::test_that("e2e: when default landing_popup_module is closed, it shows
   app <- TealAppDriver$new(
     data = simple_teal_data(),
     modules = modules(
-      landing_popup_module(),
       example_module()
-    )
+    ),
+    landing_popup = landing_popup_module()
   )
 
   # Button is clicked.
@@ -91,12 +91,12 @@ testthat::test_that(
     app <- TealAppDriver$new(
       data = simple_teal_data(),
       modules = modules(
-        landing_popup_module(
-          title = modal_title,
-          content = modal_content,
-          buttons = modal_buttons
-        ),
         example_module()
+      ),
+      landing_popup = landing_popup_module(
+        title = modal_title,
+        content = modal_content,
+        buttons = modal_buttons
       )
     )
 
@@ -145,10 +145,10 @@ testthat::test_that("e2e: when customized button in landing_popup_module is clic
   app <- TealAppDriver$new(
     data = simple_teal_data(),
     modules = modules(
-      landing_popup_module(
-        buttons = actionButton("read", "Read more", onclick = onclick_text)
-      ),
       example_module()
+    ),
+    landing_popup = landing_popup_module(
+      buttons = actionButton("read", "Read more", onclick = onclick_text)
     )
   )