Skip to content

Commit

Permalink
1032 fix ellipsis documentation for teal::modules() (#1053)
Browse files Browse the repository at this point in the history
Close #1032 

Issue appeared because there is multiple function having the same
parameter being documented in one documentation page. The last
appearance of `@param` took precedence and overwritten any other text/

The idea is to have one entry for `@param ...` explaining usage for
different functions.

<img width="574" alt="image"
src="https://github.com/insightsengineering/teal/assets/133694481/079cfb70-7733-4951-b14c-305f2bd36c2a">


The drawback of the solution is that `@param ...` is not not documented
near `print` nor `toString` functions.
We could either 
- leave it as it is (with this PR fixing documentation)
- move all parameters to one place
- leave `@param ...` entries next to `print` and `toString` functions
but without `@` tag, so they are treated as comments.

---------

Signed-off-by: Marcin <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Aleksander Chlebowski <[email protected]>
  • Loading branch information
3 people authored Jan 25, 2024
1 parent c1e5fc8 commit f216a46
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 23 deletions.
4 changes: 2 additions & 2 deletions NAMESPACE
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Generated by roxygen2: do not edit by hand

S3method(c,teal_slices)
S3method(format,teal_module)
S3method(format,teal_modules)
S3method(get_metadata,default)
S3method(get_metadata,tdata)
S3method(join_keys,tdata)
Expand All @@ -9,8 +11,6 @@ S3method(print,teal_modules)
S3method(srv_nested_tabs,default)
S3method(srv_nested_tabs,teal_module)
S3method(srv_nested_tabs,teal_modules)
S3method(toString,teal_module)
S3method(toString,teal_modules)
S3method(ui_nested_tabs,default)
S3method(ui_nested_tabs,teal_module)
S3method(ui_nested_tabs,teal_modules)
Expand Down
24 changes: 11 additions & 13 deletions R/modules.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
#' This function dictates what modules are included in a `teal` application. The internal structure of `teal_modules`
#' shapes the navigation panel of a `teal` application.
#'
#' @param ... (`teal_module` or `teal_modules`) see [module()] and [modules()] for more details
#' @param ...
#' - For `modules()`: (`teal_module` or `teal_modules`) see [module()] and [modules()] for more details.
#' - For `format()` and `print()`: arguments passed to other methods.
#' @param label (`character(1)`) label of modules collection (default `"root"`).
#' If using the `label` argument then it must be explicitly named.
#' For example `modules("lab", ...)` should be converted to `modules(label = "lab", ...)`
Expand Down Expand Up @@ -414,44 +416,40 @@ module_labels <- function(modules) {
#'
#' @param x (`teal_modules`) to print
#' @param indent (`integer`) indent level;
#' each `submodule` is indented one level more
#' @param ... (optional) additional parameters to pass to recursive calls of `toString`
#' each nested `teal_modules` or `teal_module` is indented one level more
#' @return (`character`)
#' @export
#' @rdname modules
toString.teal_modules <- function(x, indent = 0, ...) { # nolint
format.teal_modules <- function(x, indent = 0, ...) { # nolint
# argument must be `x` to be consistent with base method
paste(c(
paste0(rep(" ", indent), "+ ", x$label),
unlist(lapply(x$children, toString, indent = indent + 1, ...))
unlist(lapply(x$children, format, indent = indent + 1, ...))
), collapse = "\n")
}

#' Converts `teal_module` to a string
#'
#' @inheritParams toString.teal_modules
#' @inheritParams format.teal_modules
#' @param x (`teal_module`)
#' @param ... ignored
#' @export
#' @rdname module
toString.teal_module <- function(x, indent = 0, ...) { # nolint
format.teal_module <- function(x, indent = 0, ...) { # nolint
paste0(paste(rep(" ", indent), collapse = ""), "+ ", x$label, collapse = "")
}

#' Prints `teal_modules`
#' @param x (`teal_modules`)
#' @param ... parameters passed to `toString`
#' @export
#' @rdname modules
print.teal_modules <- function(x, ...) {
s <- toString(x, ...)
cat(s)
return(invisible(s))
cat(format(x, ...))
invisible(x)
}

#' Prints `teal_module`
#' @param x (`teal_module`)
#' @param ... parameters passed to `toString`
#' @param ... arguments passed to other methods.
#' @export
#' @rdname module
print.teal_module <- print.teal_modules
8 changes: 4 additions & 4 deletions man/module.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 7 additions & 4 deletions man/modules.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions tests/testthat/test-modules.R
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,20 @@ testthat::test_that("append_module produces teal_modules with unique named child
mod_names <- names(appended_mods$children)
testthat::expect_equal(mod_names, unique(mod_names))
})


# format ----------------------------------------------------------------------------------------------------------

testthat::test_that("format.teal_modules returns proper structure", {
mod <- module(label = "a")
mod2 <- module(label = "c")
mods <- modules(label = "c", mod, mod2)
mod3 <- module(label = "c")

appended_mods <- append_module(mods, mod3)

testthat::expect_equal(
format(appended_mods),
"+ c\n + a\n + c\n + c"
)
})

0 comments on commit f216a46

Please sign in to comment.