WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.46k stars 4.17k forks source link

Consider using SVGR to simplify SVG handling #14628

Open gziolo opened 5 years ago

gziolo commented 5 years ago

This idea comes from the chat with @mor10 in #14590 where he shared how one can extend default webpack config with custom loaders. It sparked my interest as we still can improve our workflow around using SVG files. To bridge ReactDOM and React Native we introduce wrapper component for all used SVG tag types in #9685. However, it would be great to be able to use regular SVG files without any manual modification and let Babel (to work with npm packages out of the box in contrast to webpack) do the trick.

My initial reaction from https://github.com/WordPress/gutenberg/pull/14590#issuecomment-476092041:

I was looking into https://github.com/smooth-code/svgr and I wasn't aware it has so many useful packages in addition to CLI tool and webpack loader. It makes me wonder if we shouldn't redefine the way we handle SVG icons and maybe start using files with SVGs and tackle them with custom Babel loader which would convert them to React components on the fly providing a unified format with both React web and React Native can process.

This is what @mor10 shared in https://github.com/WordPress/gutenberg/pull/14590#issuecomment-476300168:

I would back the inclusion of SVGR 100%. It makes adding and using SVGs in blocks significantly easier and removes the need for custom workarounds. Here's how I'm using it to add a block logo as a React component:

import customLogoURL, { ReactComponent as customLogo } from "./logo.svg";

registerBlockType("block/myblock", {
  title: __("A Block", "myblocs"),
  icon: {
    src: customLogo // The React component
  },
  category: "regular",
  attributes: {
    blockImage: {
      type: "string",
      default: customLogoURL // The URL to the original SVG
    }
  },
  (...)
}

I did a quick check on the SVGR monorepo and found some useful packages which could work as a good example of how we could customize things:

gziolo commented 5 years ago

Extracting SVG icons in #14743 might help to explore how this could be achieved as all SVG based icons would be located in their own files making it easier to reason how to convert them to the current form from the commonly known file format.

mkaz commented 5 years ago

@gziolo Any updates or new idea/approaches for this one? How can I help it along?

We're looking at improving how social icons uses and manages SVG, right now it requires duplication on PHP and JS sides, which we would be better if we don't need to.

gziolo commented 5 years ago

@gziolo Any updates or new idea/approaches for this one? How can I help it along?

I didn't perform any further exploration. I documented all my findings above. Feel free to check other options, as well. I'm happy to discuss all the technical options.

We're looking at improving how social icons uses and manages SVG, right now it requires duplication on PHP and JS sides, which we would be better if we don't need to.

I saw those efforts. I think the biggest challenge there is how to store SVG with the save method. I don't quite know why you decide to use render_callback in the first place so if you could share more details it would allow other folks to get involved, too. So to conclude, we need to know all the requirements in regard to working with blocks that use SVG icons.

Originally, I was mostly concerned about all SVG icons we use with UI components rendered in the block editor and I didn't consider putting them in the post content. However, it looks like we should cover both cases to make it work as intended.

mkaz commented 5 years ago

For social links the render_callback was used to allow for SVG to be authored in a post by a user without Admin or Editor rights due to KSES. If the save method was used in Editor/JS side than an author would not be able to use the block.

Also, using render_callback allows for changing the SVG markup as logos change over time without having to reauthor the posts spots that include the blocks.

The downside for using render_callback was all the SVGs had to be loadable via PHP, and at that time, not included as their own svg files, thus the "weird" include and definition within PHP.

Now, @jasmussen is exploring a solution to use CSS background which would allow using individual SVG files. This would actually solve the server side rendering part too, since SVG markup would not be included in the post_content. However, we still need to load the SVG so they can be a block icon, so would be great if we could use webpack svg loader or something similar to import a standard svg file as a react component.

jasmussen commented 5 years ago

I saw those efforts. I think the biggest challenge there is how to store SVG with the save method. I don't quite know why you decide to use render_callback in the first place so if you could share more details it would allow other folks to get involved, too. So to conclude, we need to know all the requirements in regard to working with blocks that use SVG icons.

I took a new approach that would allow us to use completely external SVG files. You can see it here:

https://codepen.io/joen/pen/XWWXvBL

All of that works and is functional, and we can update Social Links to use this method instead, and it would solve the problems Marcus is outlining.

The only missing pieces is icons in the block library:

Screenshot 2019-10-16 at 09 46 06

It would be so nice if those icons could use the same SVG files as we load using the above codepen-outlined method.

gziolo commented 5 years ago

It would be so nice if those icons could use the same SVG files as we load using the above codepen-outlined method.

Can you elaborate what are the benefits of inlining SVG files into JavaScript code rather than offer a way to use CSS-based icons fir blocks as well? The benefit is that you wouldn’t have to download the same icon twice. The drawback is that you probably can’t use HTML based queries for SVG tags.

gziolo commented 5 years ago

There is a related discussion on how to make it possible to inline SVGs and other assets in create-react-app: https://github.com/facebook/create-react-app/issues/3722. They are seeking for the solution which doesn’t require to rewrite Webpack config each time you want to register a new asset type and rather provide ways which scale well with Babel.

There is still no decision made as it’s a complex issue to solve. They are talking Babel macros which I’m exploring in https://github.com/WordPress/gutenberg/pull/16088 for the block.json files. The biggest challenge is that it breaks development workflows because all tooling assumes that you operate on JS files so in both cases JSON and SVG, the inlined files get cached and Babel don’t clear the cache when those files change. This need to be solved on Babel side first, the only known workaround so far is to disable caching for the files which import those assets...

The existing integration for SVG in create-react-app is done with webpack: https://github.com/facebook/create-react-app/pull/3718.

jasmussen commented 5 years ago

Can you elaborate what are the benefits of inlining SVG files into JavaScript code rather than offer a way to use CSS-based icons fir blocks as well? The benefit is that you wouldn’t have to download the same icon twice. The drawback is that you probably can’t use HTML based queries for SVG tags.

To be clear I'm not personally suggesting any specific technical implementation. To me, the goal here isn't SVGR. The specific challenge I'm pondering for Social Links is: how do we reduce duplicate code? If you look at https://codepen.io/joen/pen/XWWXvBL, you'll see that each logo is referenced like so:

background: url(https://cldup.com/w4j97g2E1Y.svg) no-repeat center center;

https://cldup.com/w4j97g2E1Y.svg is an actual URL, and as you see it's just a black SVG. The CSS in the codepen re-colors that SVG to be the color of the specific brand. This already works in the editor and the frontend. The only missing piece is showing those same icons in the block library. And I can't think of a simple way to use an external URL such as https://cldup.com/w4j97g2E1Y.svg to change icons in the block library, and re-color them, without using something like SVGR. But that's the goal and if there are other options, that's cool.

I would personally LOVE if we could do this:

import Icon from './icon.svg';

That would solve it 🤘

gziolo commented 4 years ago

I’m sharing a related article about cross-platform SVG icons: http://nicolasgallagher.com/making-svg-icon-libraries-for-react-apps/

aduth commented 4 years ago

Can you clarify if this is satisfied by #18243 ?

gziolo commented 4 years ago

Can you clarify if this is satisfied by #18243 ?

It depends on how you look at it. It is implemented only for 3rd party projects that use @wordpress/scripts and build or start scripts. However, it doesn't take into account React Native at all but we still don't have a way to register 3rd party blocks for Gutenberg mobile.

Regardless, it would be nice to add an additional layer that transforms SVG files into cross-platform components from @wordpress/primitives. SVG from this package adds some accessibility improvements out of the box.