Open Oksydan opened 11 months ago
Questions | Answers |
---|---|
Description? | Simple POC replacing plain icon fonts with icons map and simple renderIcon smarty function. It should allow developers to adjust their icons in themes and allow module developers to use icons w/o being aware of what icons are being used by theme. Icons map should be standardized in some way 🥶 |
Type? | POC |
BC breaks? | no |
Deprecations? | no |
Fixed ticket? | https://github.com/PrestaShop/hummingbird/discussions/549 |
Sponsor company | Waynet |
How to test? | N/A |
Looks like svgs are defined in 2 places: icons-sprite.svg and helpers.tpl partial. I'm not sure this is a good idea. There should be 1 source for the icon. Also why don't you let the SVG be compiled and minified? IMO /assets should contain only compiled resources.
Hi @SharakPL It's only for POC. It's not ready solution.
Thank you, @Oksydan. I think this proof of concept is a good idea. 👍
From my point of view, I'm not sure that using an icon map is a good idea. If we want to standardize this, we should not test if the iconName
is part of the iconsMap
, because this implies adding new icons that don’t appear on the map is impossible (for example, if a module dev wants to use a new icon).
For the SVG sprite, the idea is very interesting. This would make the use of SVG sprites much easier.
Finally, I want to point out a problem with this POC that I think can't be solved within the theme. In your example, you change the shopping_cart
icon, and this icon is called inside a template which can be refreshed if we add a product to the cart. So, when we add a product to the cart, the "add to cart" modal doesn't show up because the function defined in Smarty .tpl is not registered globally, I suppose. This triggers an error and breaks the JS. Therefore, we can't use the stuff you propose inside .tpl
files that are refreshed by JS (I think about multiple .tpl files call inside the product page for example). I think this can only be solved by adding a module in the theme that registers the Smarty function, or by adding this to the core in php
. What do you think about that?