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

Add new sass_html_dependencies() function for more convenient htmlDependency bundling #46

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cpsievert
Copy link
Contributor

@cpsievert cpsievert commented Sep 2, 2020

No description provided.

@@ -0,0 +1,58 @@
#' Compile Sass to CSS using LibSass, then return an HTML Dependencies
#'
#' Calls `sass(input, ...)` and wraps the output into a [htmltools::htmlDependency()].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
#' Calls `sass(input, ...)` and wraps the output into a [htmltools::htmlDependency()].
#' Calls `sass(input, ...)` and wraps the output into a list of [htmltools::htmlDependency()]s.

#' stylesheet = "my-style.css"
#' ))
sass_html_dependencies <- function(
input, name, version, stylesheet, cache_key = NULL,
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's kind of confusing how the htmlDependency args are interleaved with the sass args, and that the order of the htmlDependency args aren't even the same as in the htmlDependency function.

One possibility: maybe all of the htmlDependency args should go after ...? We can also discuss other ways to go about this.

@cpsievert cpsievert marked this pull request as draft September 8, 2020 15:08
@cpsievert
Copy link
Contributor Author

Update: this idea might be worth returning to if we decide to return to the idea of "run-time" html dependencies. sass_html_dependencies could return some sort of closure that remembers the mapping from sass input to htmlDependency (that could then receive new input at a later time)

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