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

Implement post render hooks; add hooks to tagList() #267

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

cpsievert
Copy link
Collaborator

@cpsievert cpsievert commented May 21, 2021

(I'm not 100% sure this is an avenue we want to pursue, but it does seem like a decent way to solve shiny::bootstrapLib()'s static rendering issues -- see rstudio/shiny#3402 for motivation)

This main point of this PR is to add a .postRenderHook -- the primary motivation for which is to provide a way to clean-up side-effects that may happen in .renderHook. It also adds the ability to attach hooks to tagList() (this will allow us to fully deprecate tagFunction() since we can now stop using it in shiny::bootstrapLib()). Here's an example:

foo <- tags$body(
  class = "foo",
  .renderHook = function(x) {
    if (isTRUE(getOption("bar"))) tagQuery(x)$addClass("bar")$allTags() else x
  }
)

bar <- tagList(
  .renderHook = function(x) {
    options("bar" = TRUE)
    x
  },
  .postRenderHook = function() {
    options("bar" = NULL)
  }
)

foo
#> <body class="foo"></body>
tagList(bar, foo)
#> <body class="foo bar"></body>

R/tags.R Outdated Show resolved Hide resolved
R/tags.R Outdated Show resolved Hide resolved
R/tags.R Outdated Show resolved Hide resolved
R/tags.R Outdated Show resolved Hide resolved
@cpsievert
Copy link
Collaborator Author

cpsievert commented May 25, 2021

@wch with 71ac6f9, .postRenderHook is now scheduling itself correctly now, but unfortunately I don’t see a way for .postRenderHook to modify its input with a single pass through the tree -- do you think that'll be an issue and/or see a way to do that?

R/tags.R Outdated Show resolved Hide resolved
R/tags.R Outdated Show resolved Hide resolved
@wch
Copy link
Collaborator

wch commented May 27, 2021

We'll need some good unit tests to make sure the pre and post hooks are executed properly.

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

Successfully merging this pull request may close these issues.

2 participants