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

Fix up url clauses for CSS files in sub-directories #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eclecticdave
Copy link

Hi,

This is a fix for an issue I encountered where the CSS file is in a sub-directory relative to the html file that references it e.g.

    <link rel="stylesheet" href="styles/main.css" data-hitch-interpret>

and the css file itself contains e.g. a background property with a url parameter such as:

    background: url("icons.png") no-repeat;

The problem is, when Hitch picks up the css file and translates it into an embedded <style> section, it needs to fix up the url parameter - in this case it needs to change it to styles/icons.png

* Add the path to the css file to any embedded urls
  - note: currently assumes the embedded url is a relative one!
* Fix URL matching in order to try to match all relative URLs
  i.e. any urls not beginning with a '/' or a protocol
@bkardell
Copy link
Owner

bkardell commented Apr 9, 2014

We are still debating whether to do a minor release before switching over to prollyfill the actual CSS aliases proposal which has some significant differences. We'll definitely take this into consideration on the new branch, but my question to you is: Do you feel like you would benefit from a minor patch in the meantime? + @clintjhill for comment.

@eclecticdave
Copy link
Author

Hi, thanks for the reply. I don't really need a release as I'm not currently doing anything "serious" with Hitch - I'm just experimenting with it and I'm happy just tracking "master". So don't go to any extra effort just for little old me :-)

The stuff in the CSS Aliases spec looks really cool BTW :-)

@bkardell
Copy link
Owner

You're right though, it is a bug. Clint and I have written a few similar systems and they always take that into account and at the recent Extensible Web Summit I specifically brought it up, so I'm not sure why we left it out here. We'll make sure we get it fixed. We'd love any feedback, issues or ideas so we can keep them in mind as we drive forward with V2.

One thing we are seriously considering is making the parsing possible via grunt/sass plugins so it doesn't have to happen in the browser with an extra request, effort to parse and all -- that's sucky early latency and it is easy to play with but not ideal for a production environment. Those systems already have watchers and pre-processing in place and much more robust parsers, etc.

WDYT?

@eclecticdave
Copy link
Author

+1 for the grunt plugin. A sass plugin also sounds cool, although I happen to prefer stylus ;-)

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