Log1x / blade-svg-sage

A simple package to add support for Blade SVG by Adam Wathan to Roots Sage.
MIT License
73 stars 13 forks source link

theme dir URL empty/incorrect #7

Closed strarsis closed 7 years ago

strarsis commented 7 years ago

@Log1x: In latest blade-svg-sage release parse_asset_path function uses \App\config('theme')['dir'] which results in an empty string and not in the actual URL/path to the theme folder. This results in these SVGs not found when using production build in sage9. Apparently \App\config('theme') returns null.

strarsis commented 7 years ago

Workaround:

// Workaround for https://github.com/Log1x/blade-svg-sage/issues/7
add_filter('bladesvg_spritesheet_path', function () {
    return parse_asset_path_fixed('images/icons');
});

add_filter('bladesvg_image_path', function () {
    return parse_asset_path_fixed('images/icons');
});

function parse_asset_path_fixed($asset) {
    return trailingslashit(get_stylesheet_directory_uri()) .
           trailingslashit('dist') .
           \App\sage('assets')->get($asset);
}

But the hash is still not added to filename. - getSvg(...) doesn't take the hash even into account.

strarsis commented 7 years ago

Edit: The sage 9 manifest with the hashed filenames expects a path without 'dist/' while blade-svg-sage expects the path with 'dist/' ('dist/' got a special meaning for sage 9 hence it is always assumed). Hence a lookup using \App\sage('assets')->get($name) cannot find it.

strarsis commented 7 years ago

Final fixed function:

    public function getSvg($name)
    {
        return $this->svgCache->get($name, function () use ($name) {
            $path_raw = sprintf('%s/%s.svg', rtrim($this->svgPath()), $name);

            $path_without_dist = str_replace('/dist/', '', $path_raw);
            $path_sage = \App\sage('assets')->get($path_without_dist);

            $svgpath_without_dist = str_replace('/dist/', '', $this->svgPath());
            $name_sage = str_replace('.svg', '', 
              str_replace($svgpath_without_dist . '/', '', $path_sage)
            );

            $path = get_template_directory() . '/..' . sprintf('%s/%s.svg', rtrim($this->svgPath()), $name_sage);

            return $this->svgCache[$name] = $this->files->exists($path) ? $this->files->get($path) : $this->svgPath().' not found.';
        });
    }
Log1x commented 7 years ago

\App\config('theme')['dir'] works if you are using dev-master of Sage 9 using sage-lib.

var_dump(\App\config('theme'));

Screenshot

strarsis commented 7 years ago

@Log1x: So I have to update my theme to use the latest sage from master?

Log1x commented 7 years ago

In this case, yeah-- or just continue to use your workaround. Otherwise, yes, it will indeed work as intended once you update your theme and utilize the sage-lib dependency.

strarsis commented 7 years ago

@Log1x: There is still an issue with the hash 😢, see https://github.com/Log1x/blade-svg-sage/issues/8.