Skip to content
Open
Changes from 1 commit
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
8 changes: 6 additions & 2 deletions R/params.R
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ params_html_head <- function(html_head_style = c(),
#' You must take care to unsure that the URL is absolute.
#' @param html_head_script_link same as above except that these are interpreted as SRC attributes in SCRIPT tags.
#' You must take care to unsure that the URL is absolute.
#' @param disable_bootstrap if true, does not inject boostrap scripts and styles in the HTML HEAD.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what the common formatting is for this package, but you could consider styling with if \code{TRUE}, does not... Probably more important to align with the conventions of the package but just making sure you're aware of the option.

#'
#' @return named list with overridden parameter names and value.
#'
Expand All @@ -263,7 +264,8 @@ knit_params_ask <- function(file = NULL,
html_head_style = c(),
Copy link
Contributor

Choose a reason for hiding this comment

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

One option that won't pollute the argument list so much is to have a single argument that accepts a named list.

html_head = list()

Then, if you want to specify some parts:

html_head = list(style = "body { font-size: 15px; }")

Not sure which would be preferred.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, when I replied in the email, it put the comments in the wrong place.
I initially had the same idea, but as per @trestletech , the long form is preferred.

Copy link
Member

Choose a reason for hiding this comment

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

You could indeed go either way. I agree with @trestletech that R tends to stylistically prefer long argument lists. Even though this isn't normally good form in more well decomposed systems in R where many operations tend to be top level "one button" ones it makes some sense (also forces documentation of the parameters in .Rd as a side benefit).

html_head_script = c(),
html_head_style_link = c(),
html_head_script_link = c()) {
html_head_script_link = c(),
disable_bootstrap = FALSE) {

if (is.null(input_lines)) {
if (is.null(file)) {
Expand Down Expand Up @@ -462,7 +464,9 @@ knit_params_ask <- function(file = NULL,
class = "container-fluid"),
class = "navbar navbar-default navbar-fixed-bottom")

ui <- shiny::tagList(
buildPage <- ifelse((is.null(disable_bootstrap) | !isTruthy(disable_bootstrap)), shiny::bootstrapPage, shiny::tagList)

ui <- buildPage(
params_html_head(
html_head_style = html_head_style,
html_head_script = html_head_script,
Expand Down