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.

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

edit card name #172

Merged
merged 14 commits into from
Nov 25, 2022
15 changes: 13 additions & 2 deletions R/AddCardModule.R
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,13 @@ add_card_button_srv <- function(id, reporter, card_fun) {
shiny::modalDialog(
easyClose = TRUE,
shiny::tags$h3("Add a Card to the Report"),
shiny::tags$hr(),
shiny::textInput(
mhallal1 marked this conversation as resolved.
Show resolved Hide resolved
ns("label"),
"Card Name",
value = "",
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels like the modules should be able to give a default to here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this requires passing label as an argument to add_card_button_srv and simple_reporter_srv.

Copy link
Contributor

@Polkas Polkas Nov 22, 2022

Choose a reason for hiding this comment

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

Now I think it could be a good move as we are not hiding some value internally. The label argument can by "" by default.

Copy link
Contributor

@Polkas Polkas Nov 22, 2022

Choose a reason for hiding this comment

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

Another way can be to not have any default for label then to have empty name if not specified.when card is added.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup that's definitely a better way of adding it - though we'd still need to pass the label into the module args - might be best to not do that right now though and wait until teal is reconfigured to make that not needed...

Copy link
Contributor

Choose a reason for hiding this comment

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

Another way can be to not have any default for label then to have empty name if not specified.when card is added.

Or to have the name the module developer has set for it (rather than the app developer).

Copy link
Contributor Author

@mhallal1 mhallal1 Nov 22, 2022

Choose a reason for hiding this comment

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

so the latest update is:

  1. the module developer can set a default card label in the card function (it can be unnamed also)
  2. the module user can change the card label with the UI input
  3. No support for passing the module name by default to the reporter cards till the teal refactor is accomplished as it requires passing the label argument to the server function of all modules.

placeholder = "Add the card title here",
width = "100%"
),
shiny::textAreaInput(
ns("comment"),
"Comment",
Expand Down Expand Up @@ -140,7 +146,7 @@ add_card_button_srv <- function(id, reporter, card_fun) {
card_fun_args_nams <- names(formals(card_fun))
has_card_arg <- "card" %in% card_fun_args_nams
has_comment_arg <- "comment" %in% card_fun_args_nams

has_label_arg <- "label" %in% card_fun_args_nams
mhallal1 marked this conversation as resolved.
Show resolved Hide resolved

arg_list <- list()

Expand Down Expand Up @@ -180,6 +186,11 @@ add_card_button_srv <- function(id, reporter, card_fun) {
card$append_text("Comment", "header3")
card$append_text(input$comment)
}

if (length(input$label) > 0 && input$label != "") {
mhallal1 marked this conversation as resolved.
Show resolved Hide resolved
card$set_name(input$label)
}

reporter$append_cards(list(card))
shiny::showNotification(sprintf("The card added successfully."), type = "message")
shiny::removeModal()
Expand Down
2 changes: 1 addition & 1 deletion R/SimpleReporter.R
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ simple_reporter_ui <- function(id) {
#' @param id `character(1)` this `shiny` module's id.
#' @param reporter [`Reporter`] instance.
#' @param card_fun `function` which returns a [`ReportCard`] instance,
#' the function have at`card`argument and optional `comment`.
#' the function has a `card` argument and an optional `comment` argument.
#' @return `shiny::moduleServer`
#' @export
simple_reporter_srv <- function(id, reporter, card_fun) {
Expand Down
2 changes: 1 addition & 1 deletion man/simple_reporter_srv.Rd

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