ColinFay/brochure

Brochure adds unnecessary container-fluid

jnolis opened this issue · 4 comments

jnolis commented

When using an HTML template to create the html layout of a Shiny app, the brochure application seems to add an unnecessary <div class="container-fluid"> to the application, potentially causing issues with the site rendering. Thanks for the cool package!

Reproducible example

template.html

<!doctype html>
<html lang="en">
<head>
{{ headContent() }}
</head>
<body>
<p>Hello</p>
</body>
</html>

app.R without brochure that renders correctly:

library(shiny)
ui <- htmlTemplate("template.html", document_=TRUE)
server <- function(input, output, session) {}
shinyApp(ui = ui, server = server)

app.R with brochure that renders incorrectly:

library(shiny)
ui <- htmlTemplate("template.html", document_=TRUE)
server <- function(input, output, session) {}
brochureApp(page("/",ui=ui, server = server))
ColinFay commented

Hey,

thanks for the report :)

this was actually initially thought as a feature: you can define a wrapped arg on your brochure so that you don't have to repeat it on every page.

See https://github.com/ColinFay/brochure/blob/main/R/brochureApp.R#L41

so you can use tagList here for example.

I'm not sure about this feature though, and I'm happy to discuss about adding tagList as the default here (which was the first implementation) 🤔

jnolis commented

Oh I didn't notice that parameter! I would say at a minimum, the default parameters for brochureApp shouldn't affect the UI (so like have wrapped be the identity),

I can see the value of having wrapped as a parameter and having it default to NULL. Although really, brochure is a pretty advance package--I think the type of people who use it might rather handle wrapping outside the package. Like if it were me I would probably use a purrr::map. Maybe other people would use it though!

wmartin-gss commented

Could I second the request for the wrapped parameter to default to NULL/identity, or at least for this to be more clearly documented? It took a long time to diagnose why my custom HTML template wasn't coming out the way I expected - partly it was the addition of the unnecessary container-fluid, but also it was the Bootstrap CSS, which was overriding my own custom CSS.

For those looking for a solution,

app = brochureApp(
  ...,
  wrapped = identity
)

partially fixed it for me.

ColinFay commented

Re-reading this, I do think the default wrapped should indeed not be a fluidPage. Will change that back to tagList

  • 🎉1