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

SVG Icon does not get rendered properly #235

Closed
vaibhav2601 opened this issue Jul 27, 2023 · 2 comments · Fixed by #241
Closed

SVG Icon does not get rendered properly #235

vaibhav2601 opened this issue Jul 27, 2023 · 2 comments · Fixed by #241
Assignees
Labels

Comments

@vaibhav2601
Copy link

@ramboz
Copy link
Collaborator

ramboz commented Jul 31, 2023

When icons use the same id property for various groups in their markup, we actually end up having id clashes in Chrome (at least). This would probably both apply to inlined (styled) SVGs and the sprite version, as both end up adding HTML nodes with clashing id properties.

One possible approach would be to rewrite all identifiers in the SVGs when we inline them, and prefix with the icon name for instance.

ramboz added a commit that referenced this issue Aug 2, 2023
…ns (#241)

Icons that use internal references of the form `url(#…)` or `xlink:href="#…"` to reference groups or specific elements for stuff like gradients or filters have a high risk of clashing when all inlined in the same document. Also, this kind of reference does not work well with the `<use/>` tag.

This PR will thus handle those icons the same way as styled icons and just inline them instead of adding those to the icon sprite, and we also scope the identifiers to the icon name to avoid clashes.


Fix #235

Test URLs:
- Before: https://main--helix-project-boilerplate--adobe.hlx.page/
- After: https://issue235--helix-project-boilerplate--ramboz.hlx.page/
github-actions bot pushed a commit that referenced this issue Nov 30, 2023
# [1.3.0](v1.2.2...v1.3.0) (2023-11-30)

### Bug Fixes

* attempt to fix release ([5bb8f73](5bb8f73))
* **build:** fix org in issues URL ([7226d84](7226d84))
* **build:** no double slashes in issues url ([1191064](1191064))
* **github:** increase default permissions for cleanup action ([f8ed051](f8ed051))
* **github:** restrict running of action to initial commit on main branch ([1a990c2](1a990c2))
* icons no more svgs ([#267](#267)) ([2d4cfa7](2d4cfa7))
* **lib:** update scripts/aem.js to aem.js@1.3.2 ([#266](#266)) ([2d61c6a](2d61c6a))
* rescope icon identifiers to avoid clashing references across icons ([#241](#241)) ([e8dc533](e8dc533)), closes [#235](#235)
* sampleRUM always for checkpoint to be called even if RUM not selected ([fcca39d](fcca39d))
* trigger release ([afbd201](afbd201))

### Features

* add capability to patch block config at runtime ([#246](#246)) ([a774acc](a774acc))
* enable listeners to all RUM events, not just the sampled ones ([871ede4](871ede4))
* use aem.js ([#255](#255)) ([f5ab4df](f5ab4df))
Copy link

🎉 This issue has been resolved in version 1.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment