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

Duplicate attibutes #198

Closed
AidasK opened this issue Sep 12, 2022 · 3 comments
Closed

Duplicate attibutes #198

AidasK opened this issue Sep 12, 2022 · 3 comments
Labels

Comments

@AidasK
Copy link

AidasK commented Sep 12, 2022

Library version

v1.3.1

Laravel version

9.x

PHP version

8.1

Description

Library does not overwrite svg attributes. It's a big deal then using libraries such bootstrap icons as all icons already have class attribute and it's impossible to overwrite it. Example:
https://github.com/twbs/icons/blob/main/icons/1-square-fill.svg?short_path=de579e6

<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-1-square-fill" viewBox="0 0 16 16">
  <path d="M2 0a2 2 0 0 0-2 2v12a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V2a2 2 0 0 0-2-2H2Zm7.283 4.002V12H7.971V5.338h-.065L6.072 6.656V5.385l1.899-1.383h1.312Z"/>
</svg>

Steps to reproduce

Take this svg file for example:

<svg id="a"></svg>

Call this library as svg('file.svg', ['id='b']) or as a component <x-icon-a id="b"/>

The result will be an invalid html element:

<svg id="a" id="b"></svg>

Expected behaviour would be:

<svg id="b"></svg>

How to solve it

The problem lies in https://github.com/blade-ui-kit/blade-icons/blob/1.x/src/Svg.php#L37 file, where each generated attribute is appended instead of replaced. As a solution, svg tag should be parsed with DOMDocument or SimpleXMLElement and attributes should be replaced as needed. Already tried this though and these libraries does not work with xmlns="" attribute and this attribute is always ignored. So we might consider dropping this attribute if we want to go this path.

@AidasK AidasK added the bug label Sep 12, 2022
@driesvints
Copy link
Member

svg's should not have those attributes by default. It's recommended that SVG's are kept clean with any class, id, or similar attributes.

@AidasK
Copy link
Author

AidasK commented Sep 12, 2022

What about other attributes? like width and height? Shouldn't we be able to replace width if it's present in svg file?

@driesvints
Copy link
Member

What about other attributes? like width and height? Shouldn't we be able to replace width if it's present in svg file?

No. It's intended that svg's are cleaned beforehand.

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

No branches or pull requests

2 participants