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

fix: rescope icon identifiers to avoid clashing references across icons #240

Closed
wants to merge 1 commit into from

Conversation

ramboz
Copy link
Collaborator

@ramboz ramboz commented Aug 1, 2023

Icons that use internal references of the form url(#…) 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:

@aem-code-sync
Copy link

aem-code-sync bot commented Aug 1, 2023

Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@aem-code-sync
Copy link

aem-code-sync bot commented Aug 1, 2023

Page Scores Audits Google
/untitled-document PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@ramboz
Copy link
Collaborator Author

ramboz commented Aug 1, 2023

Based on the discussion in https://cq-dev.slack.com/archives/C9KD0TT6G/p1690463563902089, I re-use the same offending icon and worked with 3 variants that are rewritten on the fly.

Preview of the fix:
Screenshot 2023-08-01 at 9 33 14 AM

@ramboz ramboz closed this Aug 1, 2023
Copy link

@vaibhav2601 vaibhav2601 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ramboz

We will have to update the xlink:href in tag as well.

svg file before:

<?xml version="1.0" encoding="utf-8"?>
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="120" height="120" viewBox="0 0 120 120">
    <style class=""></style>
    <defs>
        <linearGradient id="b" x1="50%" x2="50%" y1="0%" y2="100%">
            <stop offset="0%" stop-color="#90BFDC"/>
            <stop offset="100%" stop-color="#47ABD5"/>
        </linearGradient>
        <path id="a" d="M9.094 69.563l3.627 10.702a.88.88 0 0 0 .845.82l3.18.15a.887.887 0 0 0 .912-.714c.34-1.748 1.614-6.083 6.044-7.635a10.882 10.882 0 0 1 9.672 1.897s2.441 1.457 3.168 6.035c0 .071.048.315.071.487 0 .185.054.37.072.56a1.19 1.19 0 0 0 1.233 1.081h3.114a.489.489 0 0 0 .334-.13 9.238 9.238 0 0 1 3.365-2.165c1.417-.505 27.322-1.974 31.074 2.117.093.108.227.17.37.172h4.585a1.22 1.22 0 0 0 1.233-1.064c.036-.368.107-.826.208-1.326a.594.594 0 0 0 .054-.142c.595-3.128 2.638-8.003 9.07-8.003 0 0 8.992-.333 9.66 9.406.034.67.59 1.193 1.262 1.189h4.425a.495.495 0 0 0 .369-.16c2.054-2.331 4.169-8.01 3.942-11.297a15.058 15.058 0 0 0-4.109-3.122s-.047-.035-.077-.047c-.371-.234-.755-.446-1.15-.636a52.32 52.32 0 0 0-20.724-5.078h-.887S73.018 56.994 65.4 53.914l1.102-.97s-8.075-3.9-36.66-.653c0 0-3.651.214-5.771.315v1.01l3.305 1.868-5.187 4.239a8.904 8.904 0 0 1-4.098 1.867l-4.27.755a1.71 1.71 0 0 0-1.113.707l-3.395 4.93c-.319.462-.4 1.048-.22 1.58zm73.465-3.698c-4.662.123-9.326-.075-13.96-.595-2.126-.214-8.337-.85-15.18-1.611a.75.75 0 0 1-.667-.666l-.875-9.37c3.275 0 5.36.19 5.36.19 7.926.464 25.286 10.03 25.286 10.03 3.597 1.665.036 2.022.036 2.022zm-53.556-5.708a11.486 11.486 0 0 1 3.221-3.776c1.966-1.658 5.158-1.878 5.158-1.878a98.42 98.42 0 0 1 12.649-.88l.834 9.15a.487.487 0 0 1-.548.523c-10.6-1.207-21.618-2.598-21.314-3.14z"/>
    </defs>
    <g fill="none" fill-rule="evenodd">
        <mask id="c" fill="#fff">
            <use xlink:href="#a"/>
        </mask>
        <use fill="url(#b)" fill-rule="nonzero" xlink:href="#a"/>
        <path fill="#01577D" fill-rule="nonzero" d="M4 45.398h60c-2.667 2.232-4 5.92-4 11.062 0 7.714 7.365 8.762 5.927 15.951C64.97 77.204 59.66 82.548 50 88.444H4V45.398z" mask="url(#c)"/>
    </g>
</svg>


After :


<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="120" height="120" viewBox="0 0 120 120">
    <style class=""></style>
    <defs>
        <linearGradient id="auto-paint-shop1-b" x1="50%" x2="50%" y1="0%" y2="100%">
            <stop offset="0%" stop-color="#90BFDC"></stop>
            <stop offset="100%" stop-color="#47ABD5"></stop>
        </linearGradient>
        <path id="auto-paint-shop1-a" d="M9.094 69.563l3.627 10.702a.88.88 0 0 0 .845.82l3.18.15a.887.887 0 0 0 .912-.714c.34-1.748 1.614-6.083 6.044-7.635a10.882 10.882 0 0 1 9.672 1.897s2.441 1.457 3.168 6.035c0 .071.048.315.071.487 0 .185.054.37.072.56a1.19 1.19 0 0 0 1.233 1.081h3.114a.489.489 0 0 0 .334-.13 9.238 9.238 0 0 1 3.365-2.165c1.417-.505 27.322-1.974 31.074 2.117.093.108.227.17.37.172h4.585a1.22 1.22 0 0 0 1.233-1.064c.036-.368.107-.826.208-1.326a.594.594 0 0 0 .054-.142c.595-3.128 2.638-8.003 9.07-8.003 0 0 8.992-.333 9.66 9.406.034.67.59 1.193 1.262 1.189h4.425a.495.495 0 0 0 .369-.16c2.054-2.331 4.169-8.01 3.942-11.297a15.058 15.058 0 0 0-4.109-3.122s-.047-.035-.077-.047c-.371-.234-.755-.446-1.15-.636a52.32 52.32 0 0 0-20.724-5.078h-.887S73.018 56.994 65.4 53.914l1.102-.97s-8.075-3.9-36.66-.653c0 0-3.651.214-5.771.315v1.01l3.305 1.868-5.187 4.239a8.904 8.904 0 0 1-4.098 1.867l-4.27.755a1.71 1.71 0 0 0-1.113.707l-3.395 4.93c-.319.462-.4 1.048-.22 1.58zm73.465-3.698c-4.662.123-9.326-.075-13.96-.595-2.126-.214-8.337-.85-15.18-1.611a.75.75 0 0 1-.667-.666l-.875-9.37c3.275 0 5.36.19 5.36.19 7.926.464 25.286 10.03 25.286 10.03 3.597 1.665.036 2.022.036 2.022zm-53.556-5.708a11.486 11.486 0 0 1 3.221-3.776c1.966-1.658 5.158-1.878 5.158-1.878a98.42 98.42 0 0 1 12.649-.88l.834 9.15a.487.487 0 0 1-.548.523c-10.6-1.207-21.618-2.598-21.314-3.14z"></path>
    </defs>
    <g fill="none" fill-rule="evenodd">
        <mask id="auto-paint-shop1-c" fill="#fff">
            <use xlink:href="#a"></use>
        </mask>
        <use fill="url(#auto-paint-shop1-b)" fill-rule="nonzero" xlink:href="#a"></use>
        <path fill="#01577D" fill-rule="nonzero" d="M4 45.398h60c-2.667 2.232-4 5.92-4 11.062 0 7.714 7.365 8.762 5.927 15.951C64.97 77.204 59.66 82.548 50 88.444H4V45.398z" mask="url(#auto-paint-shop1-c)"></path>
    </g>
</svg>



Here you can see that `xlink:href` are not updated so svg file will not be displayed properly. 



@ramboz
Copy link
Collaborator Author

ramboz commented Aug 1, 2023

Good catch. I'll push this to #241

@ramboz
Copy link
Collaborator Author

ramboz commented Aug 1, 2023

Closing this as it doesn't update commits for some reason. Superseded by #241

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.

SVG Icon does not get rendered properly
2 participants