Skip to content

Conversation

@mklucsarits
Copy link
Contributor

@mklucsarits mklucsarits commented Oct 10, 2016

The way in which we're using SVG sprites on Fandom Stories exposed a bug in Chrome. I don't entirely understand the issue, except that we had two SVGs not displaying properly as a result. However, the two files shared a pattern: They had both had group (<g>) tags containing masks separate from the main paths. This allowed me to propose a fix. I asked Derek H. to edit the files in Sketch, and to move the paths in separate groups into the main layer with the other paths. This resolved the issue.

This bug was not occurring on the Mediawiki stack. The difference being that on Wikia/app, each entire SVG is included inside the <svg> tag, whereas on Fandom stories we use <use> like Mercury. The separate transform groups in the problematic symbols also had their own <use> tags linking to paths in the combined/inlined <defs>. I would guess these multiple <use> links play a part in the underlying issue, although I'm not sure why.

package.json Outdated
{
"name": "design-system",
"version": "0.14.4",
"version": "0.14.5”,
Copy link

Choose a reason for hiding this comment

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

This is causing build to fail:

npm ERR! Failed to parse json
npm ERR! Unexpected token '\n' at 3:24
npm ERR!   "version": "0.14.5�,

http://jenkins.wikia-prod:8080/job/design-system-reference-page-pr-check/705/console

Copy link

Choose a reason for hiding this comment

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

After you fix it please check the VRT results to make sure there is no visual difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are visual differences. The reference page is completely different even though all I did was update the SVGs.

<path fill="#0A0B09" d="M118.147 30.86c0 1.85-1.51 3.35-3.38 3.35H4.17c-1.87 0-3.385-1.5-3.385-3.35V4.134C.785 2.284 2.3.78 4.17.78h110.596c1.87 0 3.38 1.504 3.38 3.354V30.86"/>
<path fill="#FFF" d="M26.557 17.31c-.025-2.82 2.327-4.19 2.434-4.256-1.33-1.928-3.39-2.19-4.12-2.21-1.73-.182-3.41 1.03-4.3 1.03-.9 0-2.26-1.014-3.72-.984-1.88.03-3.65 1.113-4.62 2.797-1.99 3.432-.5 8.477 1.41 11.25.96 1.36 2.08 2.877 3.54 2.824 1.44-.05 1.97-.9 3.7-.9 1.72 0 2.22.91 3.71.88 1.53-.03 2.5-1.37 3.43-2.74 1.11-1.55 1.55-3.09 1.57-3.17-.03-.01-2.98-1.13-3.01-4.51m-2.82-8.3c.77-.95 1.3-2.26 1.15-3.58-1.11.05-2.51.77-3.31 1.7-.71.83-1.34 2.18-1.18 3.45 1.26.09 2.54-.63 3.35-1.57m20.12 13.69l-.99-3.04c-.1-.31-.3-1.04-.59-2.19h-.03c-.12.5-.3 1.23-.56 2.2l-.97 3.04h3.15zm3.43 4.86H45.3l-1.1-3.42h-3.81l-1.04 3.42H37.4l3.78-11.64h2.33l3.79 11.65zm7.8-4.23c0-.81-.18-1.49-.56-2.02-.41-.55-.95-.83-1.64-.83-.46 0-.89.16-1.26.46-.38.31-.63.71-.74 1.21-.06.23-.09.42-.09.57v1.4c0 .61.19 1.13.56 1.55.38.42.87.63 1.47.63.71 0 1.26-.27 1.66-.81.4-.54.6-1.26.6-2.14zm1.91-.07c0 1.43-.39 2.56-1.16 3.39-.7.74-1.56 1.11-2.59 1.11-1.11 0-1.91-.39-2.4-1.19h-.04V31h-1.88v-9.06c0-.898-.02-1.82-.07-2.765h1.66l.11 1.33h.04C51.28 19.5 52.23 19 53.5 19c1 0 1.83.39 2.5 1.173.666.786 1 1.817 1 3.096zm7.82.07c0-.82-.18-1.49-.55-2.02-.41-.55-.95-.83-1.64-.83-.46 0-.88.15-1.26.46-.38.31-.63.71-.74 1.21-.06.23-.09.42-.09.57v1.4c0 .61.19 1.13.56 1.55.38.42.87.63 1.47.63.71 0 1.26-.27 1.65-.81.39-.54.59-1.25.59-2.14zm1.92-.07c0 1.43-.39 2.56-1.17 3.39-.7.74-1.56 1.11-2.59 1.11-1.12 0-1.92-.39-2.4-1.19h-.04V31h-1.88v-9.06c0-.898-.03-1.82-.07-2.765h1.65l.1 1.33h.04C61 19.5 61.95 19 63.23 19c.996 0 1.83.39 2.497 1.173.667.786 1 1.817 1 3.096zM77.6 24.3c0 .99-.347 1.797-1.042 2.42-.763.68-1.827 1.018-3.195 1.018-1.26 0-2.274-.24-3.04-.725l.436-1.555c.82.495 1.73.743 2.71.743.7 0 1.25-.15 1.65-.47.39-.31.59-.74.59-1.27 0-.47-.16-.87-.49-1.19-.32-.32-.87-.62-1.62-.9-2.06-.76-3.08-1.87-3.08-3.34 0-.95.36-1.74 1.08-2.35.71-.61 1.67-.91 2.87-.91 1.06 0 1.95.18 2.66.55l-.47 1.52c-.66-.35-1.41-.53-2.25-.53-.66 0-1.18.16-1.55.49-.31.29-.47.64-.47 1.06 0 .46.18.84.54 1.14.31.28.88.58 1.7.9 1.01.41 1.75.88 2.22 1.42.47.54.71 1.22.71 2.02m6.21-3.73H81.7v4.08c0 1.04.36 1.56 1.093 1.56.332 0 .612-.03.832-.08l.05 1.42c-.37.14-.854.21-1.46.21-.74 0-1.32-.22-1.74-.67-.415-.45-.626-1.2-.626-2.26V20.6h-1.23v-1.4h1.24v-1.54l1.85-.555v2.09h2.07v1.4m7.39 2.79c0-.78-.17-1.44-.51-2-.39-.67-.956-1.005-1.685-1.005-.76 0-1.33.336-1.73 1.007-.34.55-.51 1.23-.51 2.02 0 .77.166 1.44.5 1.99.41.67.977 1.007 1.71 1.007.715 0 1.28-.34 1.685-1.02.348-.565.52-1.237.52-2.01zm1.95-.06c0 1.29-.37 2.35-1.11 3.17-.77.853-1.81 1.28-3.09 1.28-1.24 0-2.23-.41-2.97-1.228-.74-.82-1.11-1.85-1.11-3.094 0-1.3.38-2.365 1.14-3.194.76-.83 1.79-1.24 3.07-1.24 1.25 0 2.24.41 3 1.23.72.79 1.08 1.82 1.08 3.07zm6.11-2.49a2.82 2.82 0 0 0-.59-.05c-.66 0-1.17.24-1.53.74-.31.44-.47.99-.47 1.66v4.4H94.8l.017-5.75c0-.97-.023-1.85-.07-2.647h1.638l.068 1.61h.052c.198-.55.512-.994.94-1.33.418-.3.87-.45 1.358-.45.174 0 .33.016.47.04v1.78m6.613 1.676c.013-.55-.11-1.03-.365-1.43-.32-.52-.82-.777-1.49-.777-.61 0-1.11.25-1.49.76-.31.4-.5.887-.55 1.45h3.92zm1.792.488c0 .335-.022.617-.07.847h-5.636c.022.83.294 1.46.817 1.9.47.39 1.09.58 1.84.58.83 0 1.59-.13 2.28-.4l.29 1.3c-.8.35-1.75.52-2.84.52-1.31 0-2.34-.38-3.09-1.15-.75-.76-1.13-1.79-1.13-3.08 0-1.27.34-2.32 1.04-3.16.73-.9 1.71-1.35 2.95-1.35 1.22 0 2.14.45 2.77 1.35.49.71.74 1.59.74 2.64zM42.263 8.78c0-.666-.178-1.18-.535-1.535-.357-.356-.877-.535-1.56-.535-.292 0-.54.02-.745.06v4.277c.114.01.322.02.624.02.707 0 1.253-.2 1.638-.59.385-.39.578-.96.578-1.71zm.974-.024c0 1.03-.31 1.804-.932 2.325-.577.48-1.394.72-2.454.72-.52 0-.97-.02-1.35-.06V6.11c.49-.08 1.02-.12 1.59-.12 1.01 0 1.77.22 2.29.655.58.492.87 1.196.87 2.113zm4.206.93c0-.38-.083-.708-.248-.98-.194-.33-.47-.496-.83-.496-.37 0-.652.165-.846.495-.17.27-.25.602-.25.993 0 .38.08.707.25.98.2.328.47.492.83.492.35 0 .63-.167.83-.5.17-.28.25-.608.25-.99zm.958-.03c0 .633-.18 1.153-.54 1.56-.38.42-.89.627-1.52.627-.61 0-1.09-.2-1.46-.602-.36-.4-.54-.91-.54-1.52 0-.64.19-1.16.56-1.57.38-.4.88-.61 1.51-.61.61 0 1.1.2 1.47.61.36.39.53.9.53 1.51zm6.93-2.03l-1.3 4.125h-.84l-.54-1.79c-.13-.44-.24-.89-.33-1.33h-.02c-.08.45-.19.9-.33 1.33l-.57 1.79h-.85l-1.23-4.12h.95l.47 1.96c.11.46.21.9.28 1.32h.01c.07-.35.19-.79.35-1.32l.59-1.97h.75l.57 1.93c.14.47.25.92.33 1.36h.03c.06-.42.16-.88.28-1.36l.51-1.92h.9m4.79 4.12h-.93V9.39c0-.727-.27-1.09-.83-1.09-.27 0-.49.1-.66.3-.17.2-.26.436-.26.707v2.447h-.92V8.81c0-.364-.01-.757-.03-1.18h.81l.05.644h.02c.11-.2.27-.367.48-.498.25-.155.53-.232.84-.232.39 0 .71.125.97.374.32.305.48.76.48 1.366v2.47m1.63-6.017h.93v6.017h-.925m5.4-2.067c0-.38-.082-.706-.25-.978-.193-.33-.47-.5-.83-.5-.37 0-.653.16-.85.49-.164.27-.25.6-.25.99 0 .38.086.7.25.98.2.33.48.49.84.49.35 0 .63-.17.83-.5.17-.28.25-.61.25-.99zm.96-.03c0 .635-.183 1.155-.55 1.562-.382.41-.89.62-1.522.62-.61 0-1.092-.2-1.453-.6-.37-.4-.55-.91-.55-1.52 0-.64.19-1.16.56-1.57.37-.41.88-.61 1.51-.61.61 0 1.1.2 1.47.6.35.39.53.89.53 1.51zm3.51.68V9.7c-1.02-.017-1.53.26-1.53.832 0 .215.058.376.177.483.12.108.27.16.45.16a.925.925 0 0 0 .568-.19c.18-.126.29-.29.32-.487.01-.045.02-.1.02-.162zm.96 1.417h-.83l-.07-.475h-.024c-.286.37-.69.56-1.216.56-.39 0-.71-.13-.95-.38a1.183 1.183 0 0 1-.325-.84c0-.51.215-.89.64-1.16.427-.27 1.025-.4 1.795-.39V9c0-.543-.29-.814-.862-.814-.41 0-.77.1-1.08.305l-.19-.6c.39-.23.868-.35 1.43-.35 1.09 0 1.63.57 1.63 1.71v1.52c0 .41.02.74.06.99zm4.29-1.74v-.687c0-.12-.01-.216-.03-.29-.05-.22-.16-.405-.33-.555a.906.906 0 0 0-.615-.225.96.96 0 0 0-.82.407c-.194.272-.293.62-.293 1.044 0 .408.09.74.28.993.2.28.47.41.81.41.3 0 .55-.11.73-.34.18-.21.27-.46.27-.75zm.958 1.74H77l-.04-.662h-.026c-.262.503-.707.755-1.334.755-.5 0-.92-.195-1.25-.585-.33-.39-.494-.897-.494-1.52 0-.667.18-1.208.54-1.62a1.657 1.657 0 0 1 1.282-.578c.555 0 .946.187 1.17.56h.015V5.737h.926v4.906c0 .4.01.77.03 1.11zm6.94-2.067c0-.38-.085-.706-.25-.978-.194-.33-.47-.5-.83-.5-.37 0-.65.16-.85.49-.162.27-.25.6-.25.99 0 .38.088.7.25.98.2.33.48.49.84.49.356 0 .63-.17.83-.5.17-.28.26-.61.26-.99zm.955-.03c0 .635-.18 1.155-.546 1.562-.383.41-.89.62-1.522.62-.61 0-1.09-.2-1.454-.6-.362-.4-.542-.91-.542-1.52 0-.64.188-1.16.56-1.57.374-.41.877-.61 1.51-.61.61 0 1.1.2 1.47.6.354.39.53.89.53 1.51zm4.97 2.097H89.8V9.39c0-.727-.28-1.09-.84-1.09-.27 0-.49.1-.66.3-.17.2-.255.436-.255.707v2.447h-.93V8.81c0-.364-.01-.757-.03-1.18h.81l.047.644h.03c.106-.2.27-.367.476-.498.256-.155.53-.232.84-.232.39 0 .71.125.97.374.32.305.48.76.48 1.366v2.47m6.215-3.438H95.9v2.004c0 .51.186.764.54.764.17 0 .307-.014.416-.043l.02.7c-.18.07-.42.1-.72.1-.365 0-.65-.11-.853-.33-.207-.22-.31-.59-.31-1.11V8.32h-.607v-.69h.604v-.757l.905-.27V7.63h1.02v.685m4.89 3.44h-.923V9.41c0-.74-.28-1.11-.837-1.11-.43 0-.72.214-.88.643-.027.09-.04.2-.04.33v2.48h-.92V5.738h.92v2.485h.02c.29-.452.71-.678 1.25-.678.383 0 .7.125.95.374.314.31.47.77.47 1.38v2.45m4.165-2.5c.01-.27-.05-.51-.18-.71-.16-.26-.4-.38-.73-.38-.3 0-.55.12-.736.37-.15.2-.24.43-.27.71h1.92zm.886.24c0 .16-.016.3-.04.41h-2.77c.015.41.147.72.4.93.238.19.54.29.91.29.41 0 .785-.07 1.12-.2l.147.63c-.392.17-.855.25-1.393.25-.64 0-1.15-.19-1.514-.57-.37-.38-.55-.88-.55-1.52 0-.62.17-1.14.51-1.55.356-.45.84-.67 1.45-.67.6 0 1.054.22 1.36.66.246.35.37.78.37 1.3z"/>
</g>
<svg width="119px" height="35px" viewBox="0 0 119 35" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
Copy link

Choose a reason for hiding this comment

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

In https://github.com/Wikia/upstream/pull/733 I see that you had an exception for google play and yet, here you modify the app store icon. Is that right?

<?php // todo: fix svg min FAN-985 ?>
<?php if ( $link['image'] == 'wds-company-store-googleplay' ): ?>
    <img class="logo-fandom-tagline" src="<?php echo get_site_url(); ?>/wp-content/themes/upstream/dist/svg/store-googleplay.svg">

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct. The problem with the Google Play image was the gradient was not displaying, due to id conflicts in the inlined SVG. https://github.com/Wikia/upstream/pull/733 resolves that. The Apple store image then had the problems described in this PR. So they are two separate issues.

<use xlink:href="#logo-fandom-powered-by-wikia-two-lines-path-1"/>
</mask>
<path d="M24.028 2.343C22.668.821 20.748.016 18.476.016c-2.314 0-4.21 1.106-5.377 3.125C11.933 1.122 10.036.016 7.723.016 5.451.016 3.53.82 2.17 2.343.902 3.763.204 5.753.204 7.947v9.086H4.49V7.947c0-2.454 1.178-3.862 3.233-3.862s3.233 1.408 3.233 3.862v9.086h4.286V7.947c0-2.454 1.179-3.862 3.234-3.862 2.054 0 3.233 1.408 3.233 3.862v9.086h4.286V7.947c0-2.193-.699-4.184-1.967-5.604" fill="#FFF" mask="url(#logo-fandom-powered-by-wikia-two-lines-mask-1)"/>
<svg width="184px" height="44px" viewBox="0 0 184 44" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
Copy link

Choose a reason for hiding this comment

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

Please use width and height without px to be consistent with the rest of the files.
Also, run npm run svgo to optimize the source files. There is a lot of ids, transforms, paths and other unnecessary cruft here.

@mklucsarits
Copy link
Contributor Author

@rogatty Can you take another look at this please?

Copy link

@rogatty rogatty left a comment

Choose a reason for hiding this comment

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

@mklucsarits I've found that we have a bug in the SVGO configuration (https://wikia-inc.atlassian.net/browse/XW-2206) and manually optimized the assets you've updated.

I had to update the VRT baseline because in another PR were some changes in CSS but there was no index.html build done. This PR doesn't change anything visually.

LGTM

@mklucsarits
Copy link
Contributor Author

Thanks, @rogatty !

@mklucsarits mklucsarits merged commit 527a917 into master Oct 13, 2016
@mklucsarits mklucsarits deleted the FAN-985 branch October 13, 2016 16:31
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