Skip to content
Open
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 69 additions & 11 deletions R/TealAppDriver.R
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ TealAppDriver <- R6::R6Class( # nolint: object_name.
)
# end od check

private$set_active_ns()
self$wait_for_idle()
private$set_active_ns()
},
#' @description
#' Append parent [`shinytest2::AppDriver`] `click` method with a call to `waif_for_idle()` method.
Expand Down Expand Up @@ -234,7 +234,7 @@ TealAppDriver <- R6::R6Class( # nolint: object_name.
get_active_module_plot_output = function(plot_id) {
checkmate::check_string(plot_id)
self$get_attr(
self$namespaces()$module(sprintf("%s-plot_main > img", plot_id)),
self$namespaces(TRUE)$module(sprintf("%s-plot_main > img", plot_id)),
"src"
)
},
Expand All @@ -249,7 +249,6 @@ TealAppDriver <- R6::R6Class( # nolint: object_name.
#' @return The `TealAppDriver` object invisibly.
set_active_module_input = function(input_id, value, ...) {
checkmate::check_string(input_id)
checkmate::check_string(value)
self$set_input(
self$namespaces()$module(input_id),
value,
Expand Down Expand Up @@ -554,18 +553,77 @@ TealAppDriver <- R6::R6Class( # nolint: object_name.
filter_panel = character(0)
),
# private methods ----
set_active_ns = function() {
# Helper function to extract wrapper ID from selector and take first match if multiple found
extract_wrapper_id = function(selector) {
wrapper_id <- sub(
"^#",
"",
self$get_attr(selector = selector, attribute = "href")
)
# Take first match if multiple found
if (length(wrapper_id) > 1) {
wrapper_id <- wrapper_id[1]
}
wrapper_id
},
# Helper function to check if wrapper ID is valid
is_valid_wrapper_id = function(wrapper_id) {
length(wrapper_id) == 1 && wrapper_id != "" && !is.na(wrapper_id)
},
set_active_ns = function(sleep_time = 0.5) {
# Although wait_for_idle() is called before set_active_ns(), it only ensures Shiny is not processing.
# wait_for_page_stability() is needed here to ensure the DOM/UI is fully rendered and stable
# before trying to extract the namespace.
private$wait_for_page_stability()

all_inputs <- self$get_values()$input
active_tab_inputs <- all_inputs[grepl("-active_module_id$", names(all_inputs))]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use AppDriver$wait_for_value to make sure we have a value and then discard the rest of the added complexity.

The ignore = list(NULL, "") argument will allow us to avoid any of the initialization

spoiler... same can be done with javascript code and AppDriver$wait_for_js (let's say for is_visible())... I'm trying it out in widgets.


active_wrapper_id <- sub(
"^#",
"",
self$get_attr(
selector = sprintf(".teal-modules-tree li a.module-button[data-value='%s']", active_tab_inputs),
attribute = "href"
# If no active_module_id input found, find the selected/active tab button directly
Copy link
Contributor

@averissimo averissimo Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get why would DOM have the input id if the input does not exist in shiny.

How would that work?

  • Is it because of bslib doing some magic? This creates additional overhead when/if the navigation selectors are changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I follow your thought. Note that self$get_attr is also used by extract_wrapper_id

if (!length(active_tab_inputs) || active_tab_inputs == "") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clause makes more sense if we start with the positive case when active_tab_inputs is available

active_wrapper_id <- private$extract_wrapper_id(
".teal-modules-tree li a.module-button.active, .teal-modules-tree li a.module-button[aria-selected='true']"
)
)
# If still not found, try any module button with a wrapper href
if (!private$is_valid_wrapper_id(active_wrapper_id)) {
active_wrapper_id <- private$extract_wrapper_id(
".teal-modules-tree li a.module-button[href*='-wrapper']:not([href='#'])"
)
}
} else {
active_wrapper_id <- private$extract_wrapper_id(
sprintf(".teal-modules-tree li a.module-button[data-value='%s']", active_tab_inputs)
)
}

# Ensure we have a valid wrapper ID
# get_attr returns character(0) when no elements found, or NA_character_ for missing attributes
if (!private$is_valid_wrapper_id(active_wrapper_id)) {
# Try one more time after a short wait - the page might still be loading
Sys.sleep(sleep_time)
active_wrapper_id <- private$extract_wrapper_id(
".teal-modules-tree li a.module-button[href*='-wrapper']:not([href='#'])"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to make more sense to sleep and call the method again with sleep_time = sleep_time - 1 that skips this part if time is negative (or NULL to simply skip it).

}

# Final check - if still not found, throw error with diagnostic information
if (!private$is_valid_wrapper_id(active_wrapper_id)) {
found_ids <- paste(
self$get_attr(
selector = ".teal-modules-tree li a.module-button[href*='-wrapper']",
attribute = "href"
),
collapse = ", "
)
stop(sprintf(
paste0(
"Could not determine active module namespace. ",
"Make sure a module tab is selected and the page has finished loading. Found wrapper IDs: %s"
),
if (length(found_ids) > 0) found_ids else "none"
))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not good clear and simple code, please refactor and take advantage that stop pastes together all its elements.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
stop(sprintf(
paste0(
"Could not determine active module namespace. ",
"Make sure a module tab is selected and the page has finished loading. Found wrapper IDs: %s"
),
if (length(found_ids) > 0) found_ids else "none"
))
stop(
"Could not determine active module namespace. ",
"Make sure a module tab is selected and the page has finished loading. Found wrapper IDs: "
if (length(found_ids) > 0) found_ids else "none"
))

}

active_base_id <- sub("-wrapper$", "", active_wrapper_id)

private$ns$base_id <- active_base_id
Expand Down
Loading