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

Run juice even without a stylesheet so <style> tag CSS can be inlined. #37

Closed
wants to merge 1 commit into from

Conversation

trevordixon
Copy link

I have the same request as #35.

What I've done here allows me to do something like this:

<style>
  <% include ../shared-styles.css %>
</style>

so that multiple email templates share CSS that gets inlined by juice. I implemented this in a weird, lazy way, so I understand you probably won't want to merge it without some cleaning up.

Are you willing to support something like this or come up with a better way to share CSS?

@niftylettuce
Copy link
Collaborator

@trevordixon is this fix still wanted? i can pull in

@trevordixon
Copy link
Author

I haven't worked on it for a while. It doesn't affect me much right now.

@ragulka
Copy link

ragulka commented Oct 23, 2013

Ah, I would very much like to see this merged. At the moment, I am including a css file, but it is not inlined.

@niftylettuce
Copy link
Collaborator

See #39 (comment)

@andrewjensen
Copy link

I wanted to add my support for this pull request. I fought with the email-templates module for an hour or two to have shared stylesheets before just requiring juice and coding around it. And like @ragulka said, this isn't about EJS, it's about the internal implementation of juice.

@niftylettuce
Copy link
Collaborator

@andrewjensen have you tested the commit trevordixon@1852c99 by @trevordixon? just want to be sure everything works and nothing breaks from it before I accept the PR and push to NPM

@andrewjensen
Copy link

@niftylettuce I haven't yet but I'll be able to test it on Monday.

@niftylettuce
Copy link
Collaborator

@jasonsims has this already been resolved?

@jasonsims
Copy link
Contributor

It seems like there are two requests here:

  1. Always run juice even if no style.less file is provided within the template
  2. Allow for shared style sheets between templates

The first currently does not happen. On #L82, we only run juice if a stylesheet file was found. I'm not clear on why this would be useful.

The second should work as long as you include a style.less (or any supported CSS preprocessor extension) file within the template directory which includes the base stylesheet. An example setup would be:

| myProject/
|--  templates/
|----  baseStyle.less
|----  template01/
|------  html.hbs
|------  style.less  # Include the base css file: @import "../baseStyle.less";
|----  template02/
|------  html.hbs
|------  style.less  

@niftylettuce Do you think there is any action to take here? If so, we should close this and open as a bug since this PR has become stale.

@custa1200
Copy link

Any chance this will ever be resolved?

@jeduan jeduan closed this Jan 11, 2016
@jeduan
Copy link
Contributor

jeduan commented Jan 11, 2016

@custa1200 this PR was for a very old version of this project. I'll take a PR if you want to take a stab at it

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

Successfully merging this pull request may close these issues.

None yet

7 participants