Skip to content

Commit

Permalink
Experiment: load gridicons from external SVG file.
Browse files Browse the repository at this point in the history
Gridicons are currently being wrapped in JSX and included as a separate
JS bundle. This is suboptimal for a number of reasons, among which:
- JS is byte-for-byte much more CPU-demanding than SVG, a concern on
  mobile devices. The fewer JS bytes the browser needs to handle, the
  better.
- Encoding image data as JS invalidates any optimizations the browser
  may be able to otherwise perform, such as download prioritization.

This PR experiments with an alternate approach: SVG external content.

SVG data is kept in a static file, with each icon being assigned an ID.
We then have a small component which simply references the correct
ID in this external file, leaving it to the browser to fetch the data at
an appropriate time during bootup.

Since the SVG data is all kept in a single file, we strike a good
balance between cacheability and number of requests, we avoid having
large amounts of procedurally-generated JSX, and we delegate all image
fetching to the browser, which can apply the usual optimizations as it
sees fit.

Potential caveats that appear to be handled correctly in this PR:
- IE11 and older versions of modern browsers work correctly, through
  the svg4everybody polyfill (~1KB). The polyfill is not included in
  the evergreen bundle.
- Icons are aligned correctly by copyign existing alignment logic.
  Some minor changes to existing CSS rules were needed.
- A blank icon is shown when the icon name is unknown.
  • Loading branch information
sgomes committed Apr 10, 2019
1 parent dc2f942 commit 044df83
Show file tree
Hide file tree
Showing 12 changed files with 1,222 additions and 119 deletions.
6 changes: 3 additions & 3 deletions assets/stylesheets/_vendor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
.gridicon {
fill: currentColor;

&.needs-offset g {
&.needs-offset use {
transform: translate( 1px, 1px ); /* translates to .5px because it's in a child element */
}

&.needs-offset-x g {
&.needs-offset-x use {
transform: translate( 1px, 0 ); /* only nudges horizontally */
}

&.needs-offset-y g {
&.needs-offset-y use {
transform: translate( 0, 1px ); /* only nudges vertically */
}
}
Expand Down
2 changes: 1 addition & 1 deletion client/blocks/inline-help/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
height: 36px;
width: 36px;

g {
use {
transform: none;
}
}
Expand Down
23 changes: 0 additions & 23 deletions client/components/async-gridicons/fallback.jsx

This file was deleted.

63 changes: 0 additions & 63 deletions client/components/async-gridicons/index.jsx

This file was deleted.

83 changes: 83 additions & 0 deletions client/components/external-gridicons/icons-offset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
export const iconsThatNeedOffset = [
'gridicons-add-outline',
'gridicons-add',
'gridicons-align-image-center',
'gridicons-align-image-left',
'gridicons-align-image-none',
'gridicons-align-image-right',
'gridicons-attachment',
'gridicons-bold',
'gridicons-bookmark-outline',
'gridicons-bookmark',
'gridicons-calendar',
'gridicons-cart',
'gridicons-create',
'gridicons-custom-post-type',
'gridicons-external',
'gridicons-folder',
'gridicons-heading',
'gridicons-help-outline',
'gridicons-help',
'gridicons-history',
'gridicons-info-outline',
'gridicons-info',
'gridicons-italic',
'gridicons-layout-blocks',
'gridicons-link-break',
'gridicons-link',
'gridicons-list-checkmark',
'gridicons-list-ordered',
'gridicons-list-unordered',
'gridicons-menus',
'gridicons-minus',
'gridicons-my-sites',
'gridicons-notice-outline',
'gridicons-notice',
'gridicons-plus-small',
'gridicons-plus',
'gridicons-popout',
'gridicons-posts',
'gridicons-scheduled',
'gridicons-share-ios',
'gridicons-star-outline',
'gridicons-star',
'gridicons-stats',
'gridicons-status',
'gridicons-thumbs-up',
'gridicons-textcolor',
'gridicons-time',
'gridicons-trophy',
'gridicons-user-circle',
'gridicons-reader-follow',
'gridicons-reader-following',
];

export const iconsThatNeedOffsetY = [
'gridicons-align-center',
'gridicons-align-justify',
'gridicons-align-left',
'gridicons-align-right',
'gridicons-arrow-left',
'gridicons-arrow-right',
'gridicons-house',
'gridicons-indent-left',
'gridicons-indent-right',
'gridicons-minus-small',
'gridicons-print',
'gridicons-sign-out',
'gridicons-stats-alt',
'gridicons-trash',
'gridicons-underline',
'gridicons-video-camera',
];

export const iconsThatNeedOffsetX = [
'gridicons-arrow-down',
'gridicons-arrow-up',
'gridicons-comment',
'gridicons-clear-formatting',
'gridicons-flag',
'gridicons-menu',
'gridicons-reader',
'gridicons-strikethrough',
];
65 changes: 65 additions & 0 deletions client/components/external-gridicons/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/** @format */

/**
* External dependencies
*/
import React from 'react';
import PropTypes from 'prop-types';
import svg4everybody from 'svg4everybody';

/**
* Internal dependencies
*/
import { iconsThatNeedOffset, iconsThatNeedOffsetX, iconsThatNeedOffsetY } from './icons-offset';

function needsOffset( name, icons ) {
return icons.indexOf( name ) >= 0;
}

const isBrowser = typeof window !== 'undefined';
if ( isBrowser ) {
// Polyfill SVG external content support. Noop in the evergreen build.
svg4everybody();
}

function ExternalGridicons( props ) {
const { size = 24, icon, onClick, className, ...otherProps } = props;
const isModulo18 = size % 18 === 0;

// Using a missing icon doesn't produce any errors, just a blank icon, which is the exact intended behaviour.
// This means we don't need to perform any checks on the icon name.
const iconName = `gridicons-${ icon }`;
const offsetClasses = isModulo18
? [
needsOffset( iconName, iconsThatNeedOffset ) ? 'needs-offset' : false,
needsOffset( iconName, iconsThatNeedOffsetX ) ? 'needs-offset-x' : false,
needsOffset( iconName, iconsThatNeedOffsetY ) ? 'needs-offset-y' : false,
]
: [];
const iconClass = [ 'gridicon', iconName, className, ...offsetClasses ]
.filter( Boolean ) // Remove all falsy values.
.join( ' ' );

return (
<svg
xmlns="http://www.w3.org/2000/svg"
viewBox="0 0 24 24"
className={ iconClass }
height={ size }
width={ size }
onClick={ onClick }
{ ...otherProps }
>
<use xlinkHref={ `/calypso/images/gridicons/sprite.svg#${ iconName }` } />
</svg>
);
}

ExternalGridicons.propTypes = {
icon: PropTypes.string.isRequired,
size: PropTypes.number,
onClick: PropTypes.func,
className: PropTypes.string,
};

export default React.memo( ExternalGridicons );
2 changes: 1 addition & 1 deletion client/my-sites/plugins/plugin-action/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
display: block;
}

.gridicons-info-outline g {
.gridicons-info-outline use {
// revert the translate(1px,1px) done by needs-offset
transform: none;
}
Expand Down
6 changes: 3 additions & 3 deletions client/my-sites/plugins/plugin-automated-transfer/style.scss
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
.plugin-automated-transfer__notice {
.gridicons-sync g {
.gridicons-sync use {
animation: spinning-sync-icon linear 2s infinite;
transform-origin: center;
}
.gridicons-checkmark g,
.gridicons-notice g {
.gridicons-checkmark use,
.gridicons-notice use {
transform: rotate( 0deg );
}
}
Expand Down
Loading

0 comments on commit 044df83

Please sign in to comment.