teal icon indicating copy to clipboard operation
teal copied to clipboard

Fix S3 dispatch for `srv_nested_tabs`

Open asbates opened this issue 2 years ago • 2 comments

It's actually needed here otherwise we get an error. I don't think this is because S3 though. I think it's with how srv_nested_tabs.teal_modules works. It uses the following code (here)

    modules_reactive <- sapply(names(modules$children), USE.NAMES = TRUE, function(id) {
      srv_nested_tabs(id = id, datasets = datasets, modules = modules$children[[id]], reporter = reporter)
    })

In the call to srv_nested_tabs, id is a string. There is no method for this so it reverts back to the generic. Then it hits inherits(reporter, "Reporter") and it fails. There's a recursiveness here generic -> method -> generic that I think is throwing things off.

Originally posted by @asbates in https://github.com/insightsengineering/teal/pull/819#discussion_r1158888447

asbates avatar Apr 05 '23 18:04 asbates

I think the dispatch works as intended, actually. The generic dispatches on the class of modules, not id:

srv_nested_tabs <- function(id, datasets, modules, is_module_specific = FALSE,
                            reporter = teal.reporter::Reporter$new()) {
  checkmate::assert_class(reporter, "Reporter")
  UseMethod("srv_nested_tabs", modules)
}

chlebowa avatar Jul 30 '23 15:07 chlebowa

On the other hand, the default method only raises an error, so in principle it should not need a default value for reporter.

chlebowa avatar Jul 30 '23 15:07 chlebowa

srv_nested_tabs (now srv_teal_module) works exactly as @chlebowa described. Everything's fine - I'm closing and greeting

gogonzo avatar Feb 05 '25 05:02 gogonzo