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

Icon implementation strategy #217

Open
lastquestion opened this issue Jun 21, 2018 · 3 comments
Open

Icon implementation strategy #217

lastquestion opened this issue Jun 21, 2018 · 3 comments

Comments

@lastquestion
Copy link
Contributor

Let's do icons, better.

The approach I want to take is inspired from (private repo) https://github.com/opentable/buffet/pull/74

The principal things I want to enable are:

  • only one parse/source of each icon, no matter how many are on a page - this resolves cases of 500+ star icons on a review page
  • maintain the ids of the paths, so that they can be targeted via CSS - to enable dynamic coloring of icons, e.g. inline SVG

Some choices (or limitations) I want to adhere by:

  • there is only one size of icon, and it's consistent across all icons, e.g. we're not going to try and read and transport the metadata on a per icon basis

In summary, the idea is:

  • transport icons as complete full SVGs - optimize, but don't strip out ids, colors, etc.
  • transport the width x height as number constants

I want to make sure we hit everything we want in React, and then have some level of fallbacks that are less performant and/or less functional.

So, in React, we'll:

  • use CommonJS export of the icons.
  • every use of a icon on the page pulls the icon in and we dump it to the top of the page
  • we reference the icon using svg use to actually display it
  • for those clients using pure clientside rendering, people seem to be using https://github.com/kisenka/svg-sprite-loader, so build a canonical integration path

For pure CSS fallback,

  • let's also generate the entire SVG as a data-uri encoded value
  • this can be used directly in CSS (or CSS module) as background-uri

Here's what I think we need to do in this repo:

  • Simplify the svggo code to optimize only what we want
  • Change the output yaml code to generate the entire SVG including viewbox
  • Implement react icon use in the styleguide using https://github.com/kisenka/svg-sprite-loader if possible (it might require isomorphic)

@jrolfs 's PR #200 is a good start, which I might base off of.

@lastquestion
Copy link
Contributor Author

cc @aselbie

@aselbie
Copy link

aselbie commented Jun 25, 2018

Okay, finally getting around to this

What I like

  • Principals seem 👍 . I would also add that supporting some form of bundling/spriting is also a requirement, but I believe that is already covered in this implementation.
  • Icon size limitation seems good, just have to make sure that design doesn't try to get fancy.
  • Distributing the full icons as svg seems likely to support many usages.

What I don't understand

  • It is not clear to me what "use CommonJS export of the icons" means. Could you provide an example of what would be provided, and how one might import?

What like less

  • My experience with SVGO is that it works really well 97% of the time, but that the other 3% of the time it subtly mangles an icon to a sufficient degree that I feel iffy about using it as an automated solution. An alternative would be something manual like: https://jakearchibald.github.io/svgomg/

@lastquestion
Copy link
Contributor Author

So further work:

  • enable some form of fallback for CSS modules by generating data-uri only for CSS modules and keep the full SVG for commonJS
  • write some example code for loading this in react in the styleguide - which currently just plonks each down without using use

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

No branches or pull requests

2 participants