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

The return value of bootstrapPage() has changed in an unfortunate way #3235

Closed
cpsievert opened this issue Jan 4, 2021 · 3 comments · Fixed by #3236
Closed

The return value of bootstrapPage() has changed in an unfortunate way #3235

cpsievert opened this issue Jan 4, 2021 · 3 comments · Fixed by #3236
Assignees
Milestone

Comments

@cpsievert
Copy link
Collaborator

cpsievert commented Jan 4, 2021

With the current CRAN version of shiny (1.5.0):

> str(bootstrapPage())
List of 3
 $ : NULL
 $ : NULL
 $ : list()
 - attr(*, "class")= chr [1:2] "shiny.tag.list" "list"
 - attr(*, "html_dependencies")=List of 1
  ..$ :List of 10
  .. ..$ name      : chr "bootstrap"
...

With the development version (> 1.5.0):

> str(bootstrapPage())
List of 4
 $ :function ()  
  ..- attr(*, "srcref")= 'srcref' int [1:8] 83 15 126 3 15 3 83 126
  .. ..- attr(*, "srcfile")=Classes 'srcfilecopy', 'srcfile' <environment: 0x7fbefe65d950> 
  ..- attr(*, "class")= chr "shiny.tag.function"
 $ : NULL
 $ : NULL
 $ : list()
 - attr(*, "class")= chr [1:2] "shiny.tag.list" "list"

This means, user code that does something like this will now break:

x <- bootstrapPage()
tagAppendAttribute(x[[3]], class = "navbar")

We can do better to not break this code if {bslib} isn't being used, but the structure will likely have to change if {bslib} is being used.

Some relevant discussion/use-case radiant-rstats/radiant.data#26 (comment)

@cpsievert cpsievert self-assigned this Jan 4, 2021
@cpsievert cpsievert added this to the 1.6 milestone Jan 4, 2021
cpsievert added a commit that referenced this issue Jan 4, 2021
…b isn't relevant

Also, improved error message if theme is a character vector with 2 or more elements
@vnijs
Copy link
Contributor

vnijs commented Jan 5, 2021

Question: Does this mean that if bslib is used, tagAppendAttribute() likely won't work? I'm definitely interested in bslib but would still need a way to dynamically update the right-navbar to show directory and/or Rstudio project information.

@cpsievert
Copy link
Collaborator Author

Does this mean that if bslib is used, tagAppendAttribute() likely won't work?

No, but it does means that the way in which you're using tagAppendAttribute() will have to be different when the bootstrapPage() (or fluidPage(), navbarPage(), etc) has a theme set to a bslib::bs_theme() object. In particular, the contents of the page will be included in the fourth index instead of the third (which is why I've proposed this change to radiant)

cpsievert added a commit that referenced this issue Jan 5, 2021
…ant (#3236)

* Close #3235: Don't change the return value of bootstrapPage() if bslib isn't relevant

Also, improved error message if theme is a character vector with 2 or more elements

* yarn build (GitHub Actions)

* bump version

* yarn build (GitHub Actions)

* Don't add an additional level to the returned tree structure

* More straightforward use of do.call()

Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
Co-authored-by: Winston Chang <winston@stdout.org>
@vnijs
Copy link
Contributor

vnijs commented Jan 5, 2021

Ah. So I could check the options setting you suggested and then use either the 3rd or 4th element as needed. Thanks @cpsievert!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants