palantir / blueprint

A React-based UI toolkit for the web
https://blueprintjs.com/
Apache License 2.0
20.64k stars 2.17k forks source link

Add support for SVG icons #365

Closed adidahiya closed 6 years ago

adidahiya commented 7 years ago

And possibly deprecate the icon font.

See https://github.com/blog/2112-delivering-octicons-with-svg

For the same reasons Github states there, the only option for us would be inline SVGs (we need to color them on-the-fly).

This approach would fix:

Caveats:

Other notes:

allankikkas commented 7 years ago

If you are switching, please provide a method to add your own SVG to icon set.

For now I rewrote all your CSS framework to fit my design needs (and fix many bugs) and also to include inlined SVG icons (using https://github.com/peeter-tomberg/react-svg-icon-generator/) and I have to say it sure is a breaking change and requires a lot of work to re-implement. Outcome looks better and is way more flexible than current iconfont approach (which does not even allow your own icons).

giladgray commented 7 years ago

boy that sounds like a lot of work!

orecus commented 7 years ago

+1 for this and being able to use our own SVG or font icon set down the road. :)

llorca commented 7 years ago

Saw this great presentation by Sara Soueidan on SVG in general, with a good chunk about icon systems: http://slides.com/sarasoueidan/building-better-interfaces-with-svg#/51

Really interesting stuff.

bsr203 commented 7 years ago

Please also look at https://github.com/gorangajic/react-icons could use the scripts from https://github.com/gorangajic/react-icon-base to generate a more maintained version of icons within blueprint

dnch commented 7 years ago

Has anyone started on this? I've got a private fork with nascent support for SVG-based icons; happy to work on improving it in order to add it?

llorca commented 7 years ago

@dnch we're starting to investigate. Would love to hear what you're doing in your fork, mind detailing your approach?

dnch commented 7 years ago

@llorca Basically, I'm treating each icon as a pure component that inlines the SVG (edited for brevity):

import React from 'react'

export const Add = (props) => {
  return (
    <svg
      viewBox='0 0 20 20'
      className='icon icon-Add'>
      <path fillRule='evenodd' clipRule='evenodd' d='M10 0C4.48 0 0' />
    </svg>
  )
}

These files are generated via a script helper that runs the original .svg through SVGO, then generates the actual JSX / TSX file, then re-builds icons/index.js in the repo. The generated files are committed directly and then just treated like any other component.

When using them, it's simply a matter of passing a reference to the icon component as a prop:

<Button icon={Icons.Airplane} />

...then rendering it out:

export class Button extends React.PureComponent {
  render () {
    const { Icon, text } = this.props
    return (<button type='button' {...removeNonHTMLProps(this.props)}>
      {Icon ? <Icon /> : null}
      {text ? <span>{text}</span> : null}
    </button>)
  }
}

Styling is then handled via CSS. The upshot is that classes can potentially be added to the individual path components in each SVG to enable multi-coloured icons (where practicable).

I've also re-written checkboxes, radios and switches to use SVG directly:

export const CheckboxIndicator = (props) => {
  return (<svg className='control-indicator check' viewBox='0 0 16 16'>
    <rect className='indicator-bg' width='16' height='16' rx='1' />
    <path className='indicator-check' d='M12.1666667 4c-.2333334 0-.4416667.09777778-.5916667.25777778l-5.24166667 5.6L4.425 7.81333333c-.15-.16-.35833333-.25777777-.59166667-.25777777-.45833333 0-.83333333.4-.83333333.88888888 0 .2488889.09166667.47111112.24166667.63111112l2.5 2.66666664c.15.16.35833333.2577778.59166666.2577778.23333334 0 .44166667-.0977778.59166667-.2577778L12.7583333 5.52c.15-.16.2416667-.38222222.2416667-.6311111C13 4.4 12.625 4 12.1666667 4z' />
  </svg>)
}
export const RadioIndicator = (props) => {
  return (<svg className='control-indicator radio' viewBox='0 0 16 16'>
    <circle className='indicator-bg' cx='8' cy='8' r='8' />
    <circle className='indicator-check' cx='8' cy='8' r='3' />
  </svg>)
}
export const SwitchIndicator = (props) => {
  return (<svg className='control-indicator switch' viewBox='0 0 32 16'>
    <rect className='indicator-bg' width='32' height='16' rx='8' />
    <circle className='indicator-check' cx='8' cy='8' r='6' />
  </svg>)
}
adidahiya commented 7 years ago

We shouldn't maintain multiple copies of the SVG in this repository. There are a number of automated solutions that can help us turn the SVG resources we already have checked in into React components (I haven't looked closely into all of these yet):

dnch commented 7 years ago

@adidahiya oh, absolutely.

I should have clarified—the only reason I commit the generated SVG-as-Components to my repo is because it suits my need.

I'd imagine that implementing it in Blueprint would be done as part of the build-step; instead / in addition to generating the icon-font, you'd generate the components and include them as part of the bundle.

The reason I chose to do it this way (instead of using a loader/inliner—I tried all 4 of those suggestions, btw) was specifically to get around the need to have a run-time dependency to manage loading or inlining SVG.

In modern browsers, SVG is given equal status with HTML; given that—and the fact that JSX provides a nice interface to generate HTML/DOM, why bother with dealing with individual SVG files at run-time? Treat icons as components directly and "pre-inline" them, then they can be easily distributed as a separate package; this would then enable people to replace the default set with their own—use the build tools on correctly-named SVG files, et voila.

bsr203 commented 7 years ago

+1 for SVG components. I suggested this above, but please see the demo.

Full Material Design Icons: http://gorangajic.github.io/react-icons/md.html Font Awesome: http://gorangajic.github.io/react-icons/fa.html

brsanthu commented 7 years ago

while we are thinking about using SVGs in blueprint, can you please consider designing such that we can plugin our own icons along with default ones and use them? May be with some namespace like "fa:arrow"?

I'm looking forward to Font Awesome 2 svg icons and hoping there would be a way to use those as required instead of default ones.

tgreenwatts commented 7 years ago

@ryanmcnamara mentioned that he knows of several icons that are misaligned by a few pixels, possibly related?

llorca commented 7 years ago

Most likely. Referring to the first comment here, we've experienced blurriness since the beginning:

Blurriness of some icons at certain browser widths in Chrome & Safari

ericanderson commented 7 years ago

If you are following STIGs, you can't have webfonts. We need a fix.

brsanthu commented 7 years ago

@ericanderson can you please elaborate more about STIGs?

ericanderson commented 7 years ago

Way behind on this, my apologies. The STIGs say that you must disable font loading in IE for anything not on your intranet / whitelisted. It can take a long time to get a URL whitelisted by IT departments.

AlexLandau commented 7 years ago

Here's a link for the STIG mentioned earlier, including how to configure IE to reproduce the problem: https://www.stigviewer.com/stig/microsoft_internet_explorer_11/2015-03-26/finding/V-46505