search

blade-ui-kit / blade-icons

A package to easily make use of SVG icons in your Laravel Blade views.
https://blade-ui-kit.com/blade-icons
MIT License
2.22k stars 148 forks source link

Blade helper renaming proposal #17

Closed giovannipds closed 7 years ago

giovannipds commented 7 years ago

Hello. I was thinking about the package, when we're talking about SVG (scalable vector graphics), not every SVG is an "icon", it could be just a simple graphic or whatever. Perhaps shouldn't we consider renaming the helper @icon to @svg? Wouldn't it be better and more semantic flexible to the package? (I know we go the svg_icon as well, but my complaining is about the word "icon". What do you guys thinks? Cheers!

adamwathan commented 7 years ago

Yeah not opposed to this, let's start with some aliases maybe?

adamwathan commented 7 years ago

Just merged #18 which attempts to switch everything to SVG. Care to test our master and let me know if you run into any issues?

giovannipds commented 7 years ago

@adamwathan not at all, I'll test it as soon as I can. I wasn't being able to use it because of something (as I've mentioned on the other issue #16).

giovannipds commented 7 years ago

@adamwathan I'm testing it, seems to be working fine. =) I'm already using @svg (I'm using it with inline config set to true. Glad you were so receptive and helpful, thank you! o/

giovannipds commented 7 years ago

@adamwathan phpunit tests are passing too.

giovannipds commented 7 years ago

@adamwathan I think I may have found a bug. When using inline svgs, this happened:

<svg class="C:\path\to\my\project svg svg-wine-mark" ...>
    ...
</svg>

Where svg is my optional config class and svg-wine-mark is my inline svg's class. That C:\path\to\my\project\ came out of nowhere. I've tested with the older version and this didn't happen. I'll be using the older version until there. Do you suspect where is that happening? Regards.

adamwathan commented 7 years ago

Hmm just tried to replicate quickly but can't seem to. Can you create a repository that reproduces it and share it with me?

giovannipds commented 7 years ago

@adamwathan Did you test it with the master branch (it doesn't happen on the release 0.1.1)?

adamwathan commented 7 years ago

Yeah I tested it on master by adding a test that simulated what you described. Can you push a repository that reproduces it? That's the fastest way for me to diagnose without wasting time 👍

LasseRafn commented 7 years ago

It's because ALL configs was mapped with base_path in BladeSvgServiceProvider.php. I've pushed a PR: #19

giovannipds commented 7 years ago

Can we close the issue? =)