Publish Gridicon react component to npm

[gwwar]

This PR mirrors changes done in https://github.com/Automattic/social-logos/pull/41, https://github.com/Automattic/social-logos/pull/42, https://github.com/Automattic/social-logos/pull/43 to catch up to the social-logos build process, and pairs with https://github.com/Automattic/wp-calypso/pull/10721

This is an alternative to #145

Still a work in progress, since we should think about what should be included in the final package. Eg do we need svg-sprites, pdfs, etc? Or should we just publish the generated component/example js files? We've chosen to only publish the gridicon components.

There is also an scss file in Calypso whose rules should be moved inline into the component. https://github.com/Automattic/wp-calypso/blob/master/client/components/gridicon/style.scss

I have published an alpha tag here: https://www.npmjs.com/package/gridicons Which you can download here: https://registry.npmjs.org/gridicons/-/gridicons-1.0.0-alpha.2.tgz

Testing

• npm install
• npm run build
• This generates files that are gitignored, but may be published to npm.
• Make a change to an svg file
• npm run build
• Generated files update
• Calypso usage works: https://github.com/Automattic/wp-calypso/pull/10721

cc @afsheenirani

[gwwar]

Also what's the process for updating install docs? You should now be able to run npm install; npm run build to generate artifacts if you have the font libs installed already.

[gwwar]

Oh docs/index.html is generated too. Where is this being used?

[folletto]

Still a work in progress, since we should think about what should be included in the final package. Eg do we need svg-sprites, pdfs, etc? Or should we just publish the generated component/example js files?

I think on NPM we should have just what's needed to be used in React and no more (svg-sprites is an alternate rendering possibility, and PDF are for mobile native use in iOS).

Also what's the process for updating install docs? You should now be able to run npm install; npm run build to generate artifacts if you have the font libs installed already.

We can update the install docs asap once the changes are in, i.e. the full toolchain works and has no regressions. :)

However: what are the font libs you mention?

Oh docs/index.html is generated too. Where is this being used?

I think it's the "baseline" for http://automattic.github.io/gridicons/ (that however is clunky, as it needs to be updated by hand).

[jasmussen]

I think it's the "baseline" for http://automattic.github.io/gridicons/ (that however is clunky, as it needs to be updated by hand)

That's actually right, but I remember now — we created the new docs folder with index.html, so we could do this:

I should just flip the switch to point to docs right? Then we can delete the gh-pages branch and rely on the automated docs/index.html creation.

[folletto]

I should just flip the switch to point to docs right? Then we can delete the gh-pages branch and rely on the automated docs/index.html creation.

I'm strongly in favour of anything that automates it. :)

[jasmussen]

Done. Gridicons.com now uses the docs/index.html file. Let's see how that feels, then we can delete the gh-pages branch when we're sold on it.

[gwwar]

Thanks for the clarification @folletto @jasmussen !

I updated the package.json to only publish our component files in 33312b2. The structure now looks like:

[gwwar]

I'm finding that the current css styles are tricky to inline, in particular the transform offsets that kick in at certain sizes. https://github.com/Automattic/wp-calypso/blob/master/client/components/gridicon/style.scss#L4

@folletto @jasmussen @aduth

There are a few options here:

1. Finalize the module but still depend on the Calypso styles. This leaves a dangling component/gridicon folder with only astyle.scss in it.
2. Export a css file from this project
3. Attempt to break up <GridIcon /> into smaller components, one per file eg <PencilIcon />. It'd give me a little bit more flexibility to bake in a transform prop into the group svg element.<g transform={ transform } > This also gives us the added benefit of potentially reducing bundle sizes if we aren't using all icons.

[jasmussen]

I'm finding that the current css styles are tricky to inline

Yes, I can imagine that's quite painful.

For now, I think it's okay to publish this as is, and keep the offset hack Calypso only. When it's a good time for it, it seems the best solution is to bundle a stylesheet with Gridicons, or perhaps just output the styles inline?

In any case, it's an added bonus that makes the icons look crisper when used outside of the recommended 24px grid (when used at 18 and 36px). It absolutely doesn't break anything that you don't use that, it just doesn't look as nice.

[gwwar]

For now, I think it's okay to publish this as is, and keep the offset hack Calypso only.

Thanks @jasmussen I'll go ahead and inline fill: currentColor;

When it's a good time for it, it seems the best solution is to bundle a stylesheet with Gridicons, or perhaps just output the styles inline?

This really just depends on how many rules we end up accumulating. As is, I'll plan to inline the transforms when we break apart Gridicon into smaller components.

[jasmussen]

This really just depends on how many rules we end up accumulating. As is, I'll plan to inline the transforms when we break apart Gridicon into smaller components.

The only tricky bit here is that the needs-offset CSS class is applied only to some of the icons, and those icons only when shown at 18 or 36px sizes. Fuzzy logic, I know and if you can make that work, awesome. We just don't want the offset present on 24px sized icons, at any time.

[gwwar]

It'd probably look something like the following. Where the transform line is inserted by the grunt task. Only one would be inserted per component.

Edit: oh hmm the svg viewport might differ, so this would be safer to apply as an inline-style rather than a direct transform attr.

class CalendarIcon extends Component {
    const { size } = this.props;
    const transform = ( size % 18 === 0 ) ? 'translate(1px, 1px)' : 'translate(0, 0)'; // offset
    // const transform = ( size % 18 === 0 ) ? 'translate(1px, 0)' : 'translate(0, 0)'; //x-offset
    // const transform = ( size % 18 === 0 ) ? 'translate(0, 1px)' : 'translate(0, 0)'; //y-offset
    // const transform = 'translate(0, 0)'; // no offset

    const groupStyle = {
        transform
    };

    render() {
        return (
             <svg ... >
                  <g style={ groupStyle } >
                         //...
                  </g>
            </svg>
        );
    }

[jasmussen]

Looks like you've got it right though, but we'll need to test it for sure, that part was gnarly, and I remember having to add the <g> group, because the transform could not be applied directly to the SVG, that wouldn't push the vector subpixels like it needed to.

Here's the HTML document where I experimented with it: https://cloudup.com/cM6ySQJxWko

Note: best tested on a non-retina screen!

Nice work!

[gwwar]

I'll go ahead and inline fill: currentColor;

Turns out the specificity is too high for Calypso in this case. I think it's fine to leave as a Calypso only rule as folks can override this very easily.

Edit: code has been updated

[aduth]

Dunno if this is the right place to make this change, but looking at the output for the React component, I feel like we're wasting a ton of bytes on the svg wrapper which is common to every icon. I feel like the switch should just be generating the path, maybe even just the d prop of the path, and by moving everything else to common return logic, the end result is a much smaller file.

Example:

switch ( icon ) {
    case 'gridicons-add-image':
        d = 'M23 4v2h-3v3h-2V6h-3V4h3V1h2v3h3zm-8.5 7c.828 0 1.5-.672 1.5-1.5S15.328 8 14.5 8 13 8.672 13 9.5s.672 1.5 1.5 1.5zm3.5 3.234l-.513-.57c-.794-.885-2.18-.885-2.976 0l-.655.73L9 9l-3 3.333V6h7V4H6c-1.105 0-2 .895-2 2v12c0 1.105.895 2 2 2h12c1.105 0 2-.895 2-2v-7h-2v3.234z';
        break;
    case 'gridicons-add-outline':
        d = 'M12 4c4.41 0 8 3.59 8 8s-3.59 8-8 8-8-3.59-8-8 3.59-8 8-8m0-2C6.477 2 2 6.477 2 12s4.477 10 10 10 10-4.477 10-10S17.523 2 12 2zm5 9h-4V7h-2v4H7v2h4v4h2v-4h4v-2z';
        break;

    // ...
}

return <svg className={ iconClass } height={ size } width={ size } onClick={ onClick } xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><path d={ d } /></svg>;

[gwwar]

I think we can definitely get the size down and should very soon, but I'd prefer to get this out first.

[gwwar]

Thanks @aduth! Much appreciated for the 👀