Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

910 Remove Report Previewer column #919

Merged
merged 6 commits into from
Sep 20, 2023
Merged

910 Remove Report Previewer column #919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bad48f6
drop Report previewer from mapping matrix
Sep 15, 2023
2922610
linter
Sep 15, 2023
8f66b89
amend
Sep 18, 2023
18e5147
reserve module label 'Report previewer'
Sep 19, 2023
19bf60c
improve logical index in mapping matrix
Sep 19, 2023
053405c
circumvent reserved label in reporter_previewer_module
Sep 19, 2023
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
3 changes: 2 additions & 1 deletion NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

### Miscellaneous

* Enhance a `module` validation checks so that it won't throw messages about `data` argument unnecessarily.
* Enhanced a `module` validation checks so that it won't throw messages about `data` argument unnecessarily.
* Removed `Report previewer` module from mapping matrix display in filter manager.

# teal 0.14.0

Expand Down
5 changes: 3 additions & 2 deletions R/module_filter_manager.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,10 @@ filter_manager_srv <- function(id, filtered_data_list, filter) {
rownames(mm) <- ""
}

mm
# Report Previewer will not be displayed.
mm[names(mm) != "Report previewer"]
},
align = paste(c("l", rep("c", length(filtered_data_list))), collapse = ""),
align = paste(c("l", rep("c", sum(names(filtered_data_list) != "Report previewer"))), collapse = ""),
rownames = TRUE
)

Expand Down
13 changes: 12 additions & 1 deletion R/modules.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,18 @@ module <- function(label = "module",
}

if (label == "global_filters") {
stop("Label 'global_filters' is reserved in teal. Please change to something else.")
stop(
sprintf("module(label = \"%s\", ...\n ", label),
"Label 'global_filters' is reserved in teal. Please change to something else.",
call. = FALSE
)
}
if (label == "Report previewer") {
stop(
sprintf("module(label = \"%s\", ...\n ", label),
"Label 'Report previewer' is reserved in teal.",
call. = FALSE
)
Comment on lines +215 to +226
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("module(label = \"%s\", ...\n ", label),
"Label 'global_filters' is reserved in teal. Please change to something else.",
call. = FALSE
)
}
if (label == "Report previewer") {
stop(
sprintf("module(label = \"%s\", ...\n ", label),
"Label 'Report previewer' is reserved in teal.",
call. = FALSE
)
if (label %in% c("global_filters", "Report previewer")) {
stop(
sprintf(
"module(label = %1$s, ...
Label %1$s is reserved in teal. Please change to something else.
",
label
),
call. = FALSE
)
}

Perhaps we can combine the code for optimization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The line break within the fmt string adds loads of white space to the message, you should do

    sprintf(
      "module(label = \"%s\", ...\n  %s",
      label,
      "Label 'global_filters' is reserved in teal. Please change to something else."
    )

The benchmark is exactly the same, down to a microsecond.

Unit: microseconds
    expr    min      lq     mean  median     uq     max neval cld
     new 36.793 41.2085 51.34256 44.1210 52.019 275.206   500   a
 current 36.873 41.2010 51.51580 44.2335 50.337 289.881   500   a

The current is 3 lines less.

}
server_formals <- names(formals(server))
if (!(
Expand Down
3 changes: 2 additions & 1 deletion R/reporter_previewer_module.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ reporter_previewer_module <- function(label = "Report previewer", server_args =
}

module <- module(
label = label,
label = "temporary label",
server = srv, ui = ui,
server_args = server_args, ui_args = list(), datanames = NULL
)
class(module) <- c("teal_module_previewer", class(module))
module$label <- label
module
}