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

Gradients in the choices #8

Closed
wants to merge 2 commits into from
Closed

Conversation

davidwebca
Copy link

Gradients get grabbed by the regex right now because of the too general id="(\S+)" regex. Gradients need ids so they can get referenced later and when you have gradients in your "defs", it gets grabbed as well. At first, I wanted to resolve this problem by grabbing only the IDs that aren't between defs with the regex, but that gets complicated, so I went with the more simple strip_tags route by keeping only symbol and g which are the two most popular ways to build svg icon sprites.

Here's an example svg sprite that would cause to have an empty choice because of the gradient in defs :

<svg class="svg-icons-definitions" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <defs>
        <linearGradient x1="33.236%" y1="-36.335%" x2="66.808%" y2="-36.179%" id="0a"><stop stop-color="#EDF0F7" offset="0%"></stop><stop stop-color="#FAFAFA" offset="100%"></stop></linearGradient><linearGradient x1="44.849%" y1="-105.823%" x2="55.16%" y2="-105.541%" id="0b"><stop stop-color="#7045FF" offset="0%"></stop><stop stop-color="#36BFEF" offset="100%"></stop></linearGradient>
    </defs>
    <symbol id="icon-base" viewBox="0 0 180 111">
        <title>base</title><g fill-rule="nonzero" fill="none"><g fill="url(#0a)"><path d="M0 52.7L89.8.6 180 52.9l-89.7 52z"></path><g transform="translate(0 .6)"><path d="M0 52.1L89.8 0 180 52.3l-89.7 52z"></path><path d="M90.3 104.3L0 52.1 89.8 0 180 52.3z"></path></g></g><path d="M180 .2v5.5L90.3 57.8 0 5.6.1 0l90.2 52.2z" transform="translate(0 52.7)" fill="url(#0b)"></path></g>
    </symbol>
</svg>

Gradients get grabbed by the regex right now because of the too general ```id="(\S+)"``` regex. Gradients need ids so they can get referenced later and when you have gradients in your "defs", it gets grabbed as well. At first, I wanted to resolve this problem by grabbing only the IDs that aren't between <defs></defs> with the regex, but that gets complicated, so I went with the more simple strip_tags() route by keeping only <symbols> and <g> which are the two most popular ways to build svg icon sprites.

Here's an example svg sprite that would cause to have an empty choice because of the gradient in <defs> : ```
<svg class="svg-icons-definitions" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
		<defs>
				<linearGradient x1="33.236%" y1="-36.335%" x2="66.808%" y2="-36.179%" id="0a"><stop stop-color="#EDF0F7" offset="0%"></stop><stop stop-color="#FAFAFA" offset="100%"></stop></linearGradient><linearGradient x1="44.849%" y1="-105.823%" x2="55.16%" y2="-105.541%" id="0b"><stop stop-color="#7045FF" offset="0%"></stop><stop stop-color="#36BFEF" offset="100%"></stop></linearGradient>
		</defs>
		<symbol id="icon-base" viewBox="0 0 180 111">
			<title>base</title><g fill-rule="nonzero" fill="none"><g fill="url(#0a)"><path d="M0 52.7L89.8.6 180 52.9l-89.7 52z"></path><g transform="translate(0 .6)"><path d="M0 52.1L89.8 0 180 52.3l-89.7 52z"></path><path d="M90.3 104.3L0 52.1 89.8 0 180 52.3z"></path></g></g><path d="M180 .2v5.5L90.3 57.8 0 5.6.1 0l90.2 52.2z" transform="translate(0 52.7)" fill="url(#0b)"></path></g>
		</symbol>
</svg>
```
@Rahe
Copy link
Member

Rahe commented Apr 12, 2018

Hello,

Thank you for the PR, this is really interesting and can even boost the performances to not preg_Replace the whole text, elegant solution by the way :)
Is there any other SVG tags that can be used for icons ? (symbol and g as you describe them)

Nicolas,

@davidwebca
Copy link
Author

Hi Rahe,

I don't think there are other tags available or commonly used, but I guess that giving users a filter to allow different html tags to pass to strip_tags would prevent edge cases to arise. Like :

$tags = '<symbol><g>';
$tags = apply_filters( 'acf_svg_icon_tags',  $tags);
preg_match_all( '#id="(\S+)"#', strip_tags($contents, $tags), $svg );

What do you think?

@Rahe
Copy link
Member

Rahe commented Apr 13, 2018

Hello David,

This is a great idea, for the filter name I think acf_svg_icon_svg_parse_tags could be more understandable :)

@davidwebca
Copy link
Author

Here you go! I've structured this in an external function to keep it clean and to document what it does properly.

@MaximeCulea
Copy link
Contributor

Hi @david-treblig,
Couldn't merge, but backported into master with credits.
Thx, I close this PR.

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

Successfully merging this pull request may close these issues.

3 participants