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

Doesn't work when watching #15

Closed
strarsis opened this issue May 23, 2018 · 3 comments
Closed

Doesn't work when watching #15

strarsis opened this issue May 23, 2018 · 3 comments

Comments

@strarsis
Copy link
Contributor

strarsis commented May 23, 2018

When using watch in sage9 theme using yarn start, the production assets in dist/ are cleaned up,
hence blade-svg-sage can't find the SVGs and the page fails to load.
How can it be configured to look for the SVGs in assets directory instead?

I have to use this workaround while watching:

add_filter('bladesvg_image_path', function () {
    return '/srv/www/web/app/themes/the-theme/resources/assets/images/svg/icon';
});
@strarsis
Copy link
Contributor Author

@Log1x: Related roots discourse discussion: https://discourse.roots.io/t/asset-lookup-while-watching/12640

@Log1x
Copy link
Owner

Log1x commented Jul 17, 2018

I'm going to rewrite and fix a lot of these issues here shortly as I plan on using this in a project I'm currently working on.

I think it went in the wrong direction with a couple of the recent commits. There are a few things to consider:

  1. I merged a commit making it check the manifest for the final file name, but really; an SVG that's being read from the filesystem doesn't need to be cache busted.
  2. I'm unsure if we should read the SVG from dist or keep it defaulted to assets since, like the above, it's being read by the file system. At that point, it'd come down to if the assets folder is being pushed to the production server or not. But related to the roots discourse discussion above, whether the SVG's should actually live with the other assets while being used inline, is another point that needs to be addressed.

@Log1x Log1x mentioned this issue Nov 17, 2018
@Log1x Log1x closed this as completed Nov 17, 2018
@Log1x
Copy link
Owner

Log1x commented Nov 17, 2018

v2.0.0 will take care of this.

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

No branches or pull requests

2 participants