fix: rework assets sprite building

thibault-mahe commented 1 year ago

Changes description

Remove all useless attributes and tags in svg sprite causing bugs in some browsers

Context

When using the the svg element from an external reference (in our case the sprite/assets.svg#some-flag), the original svg is cloned inside a shadow root to keep import in isolation. It seems that sometimes Chrome doesn't know how to handle a local fetch with url() inside a remote file and do not honor the original defs element.

This PR suggests to remove all url() since they seems useless and only bring bugs...

See more comment on the affiliated issue: https://github.com/Decathlon/vitamin-web/issues/1313

Also, the way browsers are differently handling <defs>content in external svgs & omitting many definitions like clip paths, gradients, masks or styles, ... is a known bug, at least since 2012. See for instance:

Stack overflow issue:
- https://stackoverflow.com/questions/74128326/svg-with-defs-from-external-symbol-sheet-not-loading-in-firefox-or-safari/
- https://stackoverflow.com/questions/74292618/svg-use-ignores-gradient-styling
Chromium bug trackers : issue 1385248, issue 1094613, issue 951265, issue 729599, issue 229081, ...

Checklist

[ ] Make sure you are requesting to pull a topic/feature/bugfix branch. Please, don't request directly from your main!
[ ] Check commits & PR names matches our requested structure. It must follow the https://www.conventionalcommits.org pattern.
[ ] Check your code additions will fail neither code linting checks.
[ ] I have reviewed the submitted code.
[ ] I have tested on related showcases.
[ ] If it includes design changes, please ask for a review with a core team designer.

Does this introduce a breaking change?

TO BE TESTED

Yes
No

Other information

thibault-mahe commented 1 year ago

There is still an issue with some flags using a combination of fill and url(). And I can't just remove the fill attribute in this case since it is used with hard values most of the times.

i'll try with a regex to remove fill=url(...

thibault-mahe commented 1 year ago

New update: fill=url(... cannot be removed, too many flags are depending on it and linear gradient... I'll look into alternative solutions...

thibault-mahe commented 1 year ago

Update: still in draft, waiting for https://github.com/Decathlon/vitamin-design/issues/135

thibault-mahe commented 1 year ago

Closing this, the icons will be reworked on the design part so we shouldn't have to rework them and our build process on our side.

Decathlon / vitamin-web