Icon library structure (ui-icons)

[cooper-joe]

I'd like to look at the requirements from a ui-icons library. Most core apps are using Material-UI icons and we'd like to replace all these with icons provided by ui.

The icon library will contain icons that are available in different variants based on certain attributes.

For example, let's look at the icons provided for alert:

Taking alert as a single icon/entity/concept, you could say it has the following properties:

• alert
  • color: inherit
  • variant: outlined
  • size: 12 / 16 / 24 / 32
  • variant: filled
  • size: 12 / 16 / 24 / 32

Graphically, this looks like:

and exported as SVG files, it looks like:

icon-outlined-12.svg
icon-outlined-16.svg
icon-outlined-24.svg
icon-outlined-32.svg
icon-filled-12.svg
icon-filled-16.svg
...

Scaling a single SVG is not an option because icons are often made specifically for a set size (more detail in large icons, less detail in smaller is the usual reason). This ensures we have clear icons of all sizes. Not all icons will include all sizes.

---

@varl and I discussed how we could bundle a single <Icon> component and include variant, color and size as props. The idea was that any icon could be used like so:

<Icon name="alert" variant="outlined" size="12" color="red700"/>

This is a nice, clean way to call any icon. However, I couldn't figure out a way of creating this <Icon> component without including an import for every single variation of all SVG files available. This would result in a component with potentially thousands of import statements.

Suggestion

Rather than having an <Icon> component that requires all these imports, maybe it best to just have all the icons exported as components. This requires the work of setting up/exporting a lot of components, but it seems like the simplest solution.

A single icon component, IconInfoFilled32.js would look like:

const IconInfoFilled32 = ({color}) => (
  <svg fill={color}>
    ...
  </svg>
)

Feedback on this idea would be great, @dhis2/team-apps. I am planning to have enough icons finished so that all Material-UI icon usage can be replaced in 2.35.

Here are a few points I'm wondering about:

• if we set color: inherit; on the SVG fill, is there any way of setting a default fill color? I'm thinking it would be useful to set a reasonable default, but I'm not sure how that interferes with the inherit rule.
• not all icons will have an outlined/filled variant, sometimes an icon might have a single variant, usually outlined. Does that mean that 'outlined' should be omitted and considered the default style? Then 'filled' or 'bold' or 'invert' would simply be add-on styles.

[ismay]

Rather than having an component that requires all these imports, maybe it best to just have all the icons exported as components. This requires the work of setting up/exporting a lot of components, but it seems like the simplest solution.

What would make sense to me is to use what you specified in the name prop as the name of the component, and use props for the rest:

// instead of
import { Icon } from '@dhis2/ui'

<Icon name="alert" variant="outlined" size="12" color="red700"/>

// we could do this
import { AlertIcon } from '@dhis2/ui'

<AlertIcon variant="outlined" size="12" color="red700"/>

Scaling a single SVG is not an option because icons are often made specifically for a set size (more detail in large icons, less detail in smaller is the usual reason). This ensures we have clear icons of all sizes. Not all icons will include all sizes.

I do think scaling can be nice to have though, I've always found that a nice bonus of using vector instead of raster graphics. I see your point though, and it does fit with our approach in other areas. I could also see an intermediate solution, with min and max-widths, so it can at least scale a bit. But that's just an idea.

if we set color: inherit; on the SVG fill, is there any way of setting a default fill color? I'm thinking it would be useful to set a reasonable default, but I'm not sure how that interferes with the inherit rule.

Since the icons are react components anyway, we could also handle this in js. Set a default and accept a fill or color prop to override it.

not all icons will have an outlined/filled variant, sometimes an icon might have a single variant, usually outlined. Does that mean that 'outlined' should be omitted and considered the default style? Then 'filled' or 'bold' or 'invert' would simply be add-on styles.

Yeah, we could have a variant prop like in your example, that accepts a string that specifies what style the icon will be rendered as (if these styles are mutually exclusive). And then we could set a default for that prop, say outlined. So <AlertIcon variant="filled" /> or <AlertIcon variant="outlined" />, etc.

[Mohammer5]

I'd like to add an opinion I've state here already:

I think it'd be good to give the icons unique names, they don't need to be easily "human" readable, as long as they're unique (e. g. D2ICON1, D2ICON2, etc).

This way we can add as many "arrow" as we want and re-use very specific types in our components, just like we re-use the colors.500 as theme.error color (red500 -> error -> Field uses error for displaying an error). Then we can easily switch out icons in the intermediate stage while not having to repeat ourselves (otherwise an icon library is kind of senseless?): D2ICON1->core/Node/Arrow->core/Node. Changing the arrow of the Node won't impact any other component and D2ICON1` can safely be re-used, as long as we never modify but deprecate existing icons and add new ones if we want to entirely replace an icon. . . .

@Mohammer5 Interesting. Icons have been requested to be externally available too, e.g. from Apps.

Does that change anything or do you still think the non-human readable icon names is the way to go?

I'm used to generic names so people don't interpret too much, but rather look at the styleguide for when to use which icon (i. e. when looking for an arrow pointing right and there are several available but the styleguide has rules for them as they should only be used in a certain context)

I still think that this is applicable. It has already happened to us: There are different arrow icons in the DropdownButton and in the Node compnents, but as they were shared in the past, when the DropdownButton's icon changed, the Node's icon change unintentionally as well and even broke the cypress tests (which is why we noticed it, otherwise it would've gone unnoticed for quite a while)

---

I also agree with what @ismay wrote, it'd work with the above approach as well:

import { AlertIcon } from '@dhis2/ui'

<DHIS2ArrowA1
  variant="outlined"
  size="12"
  color="red700"
/>

[cooper-joe]

Thanks for the feedback!

I do think scaling can be nice to have though, I've always found that a nice bonus of using vector instead of raster graphics

Agreed, I do think we should adapt to situations where icons are used at the 'wrong' size, but in general iconography should be used at intended sizes to communicate efficiently, so I want to focus on that use case.

There are different arrow icons in the DropdownButton and in the Node compnents, but as they were shared in the past, when the DropdownButton's icon changed, the Node's icon change unintentionally

I think the main purpose of a DHIS2 icon library is to provide icons for use in apps for all developers (internal and external) rather than providing icons for core components. I'd argue that we should potentially bundle important/critical icons directly with components, rather than add a dependency to ui-icons that could change for reasons not related to a component.

External developers especially will not always have a styleguide/designed spec to work with, so I think reasonable names could help correct icon usage. That said, there are certainly ambiguous icons that could be used for multiple purposes - so naming those is a challenge.

What would make sense to me is to use what you specified in the name prop as the name of the component, and use props for the rest:

// instead of
import { Icon } from '@dhis2/ui'

<Icon name="alert" variant="outlined" size="12" color="red700"/>

// we could do this
import { AlertIcon } from '@dhis2/ui'

<AlertIcon variant="outlined" size="12" color="red700"/>

I think this is much more elegant, but does this not also run into the issue of loading multiple, unnecessary SVG files if only a single variant/size is used?

I did some reading about lazy-loading/code-splitting, but this is still unclear to me. If <AlertIcon> contains 12 different icons, that means 12 different SVG files. When someone does import { AlertIcon }..., with the intention of only using variant="outlined" size="12", do they not also load the all the SVG code for the 11 other SVGs? If an app used 20 icons in a single variant/size, this would be 220 unnecessary loads.

[Mohammer5]

Actually, I wouldn't mind having individual components.

The variant prop kind of means that a different component will be displayed and if the size will have an impact on the underlying svg, then the size also indicates different components.

[ismay]

I think this is much more elegant, but does this not also run into the issue of loading multiple, unnecessary SVG files if only a single variant/size is used?

I wouldn't worry about that too much. Optimising that shouldn't block a nice DX. My priority would be to settle on a nice api first, and from there we could potentially optimise. Svg's are not that big, and if there's a lot of duplication gzip would also be able to compress it quite well.

[HendrikThePendric]

One important aspect that has come up a lot before, but has not been mentioned yet, is the padded VS unpadded SVG discussion. In principle I think that SVGs should not have any padding, but @cooper-joe provided a very valid explanation why he thinks they should:

[...] including padding means there is more flexibility in the icon composition (e.g. you can design to 18x16 if the extra 2 pixels help. Otherwise you’d need to go down to 16x14 to get that ratio. been testing this 12, 16, 24, 32 icon size grid system but yeah, the padding thing can be annoying sometimes and produce unexpected results

The reason why I suggested having unpadded SVGs was mainly because with padded SVGs the alignment can look a bit funky.

Do you think we need a padded prop (or alternatively dense)?

---

In the past I worked on a dashboard application for over a year that was basically just a bunch of animated SVGs. In that project we were doing a lot of manual manipulation to SVG shapes, and I was quite amazed with how much is possible. So I am going to suggest a completely different approach below. I don't necessarily suggest that we should definitely go this way for our icon library, but I think it's good to at least highlight an alternative approach.

An SVG has the following the characteristics:

• If exported as individual shapes, an SVG is often pretty much human-readable
• Like HTML, each shape can get a className and/or be conditionally rendered
• SVGs can be styled using CSS reasonably well
• All things that can't be set via HTML can be set via inline attributes

So, as an alternative to producing a shedload of individual SVGs that will carry a lot of duplications, we could also go the complete opposite direction and produce a much smaller set of hand-crafted SVGs that are tweaked using various techniques:

• We could cater for the variant (I guess default | filled | outlined) prop by always including some background shape and adjusting fill and stroke colors depending on the variant.
• The size prop could be catered for by using the height and width prop on the svg tag. If the various sizes don't have the correct visual weight we can first try to fix this by adjusting properties such as strokeWidth. If this fails too we can conditionally render individual shapes depending on size.
• The color prop can obviously be linked too fill and stroke attributes
• And if we want to also be able to have padded and unpadded versions of the icons, we could control this with the viewBox attribute

We are actually already using the same / a similar approach as the above for some of the icons we have already, such as the Switch.

The advantage of this type of approach is that you end up with a much more manageable set of components. The downside would be that every icon component would require a lot more attention than just exporting an SVG and putting it in a component.

[cooper-joe]

So, as an alternative to producing a shedload of individual SVGs that will carry a lot of duplications, we could also go the complete opposite direction and produce a much smaller set of hand-crafted SVGs that are tweaked using various techniques

The advantage of this type of approach is that you end up with a much more manageable set of components. The downside would be that every icon component would require a lot more attention than just exporting an SVG and putting it in a component.

This is not my area of expertise, but I think that in practice we are going to have too many icons and too few devs to make this work. Often I'll make an icon on-demand for an app that needs it, so I think we might end up with a queue of 'waiting for processing' icons.

---

Do you think we need a padded prop (or alternatively dense)?

This one is still keeping me up at night! I'm working through icons in the library, seeing how padded vs. not-padded would work. Right now padded is still my gut because of the reasoning you posted. I hope that we don't need a padded prop. Perhaps certain components that use icons might have some kind of padding built-in (Chip, MenuItem).

---

@amcgee — what do you think of this thread? (Not just the above two points, but the entire 'how should we structure icons in ui?' question 😉 )

[HendrikThePendric]

This is not my area of expertise, but I think that in practice we are going to have too many icons and too few devs to make this work.

Yeah that is my gut feeling too. I think the only way for my suggestion to work is if we would work out a single workflow for all icons. I'll provide an imaginary example:

• Each icon is created via a single template file in Sketch
• As a result, each exported SVG has the same structure of shapes with specific class-names, i.e. one path for the icon, and one path for the background, which would be used for the various variants
• The vast majority of the styling is done via a single shared stylesheet, and/or implemented in a single component. We only "hand-code" things in exceptional cases.

I guess the only way to find out if something like the above would be possible is to just start producing SVGs, and once we've got a bunch of them, see if there are enough commonalities to start working like that.

[HendrikThePendric]

I've had a look at some approaches and tooling for producing consistent and optimised SVGs:

• Here's an interesting article about Designing for code. They mentioned to always make sure you work with an artboard with a fixed background to prevent auto-cropping. This fits nicely with the background we'd need anyway for the variants.
• There are also node.js tools and plugins for Sketch around that could help us:
  • SVGO: a nodejs tool that can do lots of things and is used under the hood in the following Sketch plugins
  • svgo-compressor: basically SVGO as a Sketch plugin
  • svg-bem: Also a Sketch plugin powered by SVGO. This one is interesting because it replaces IDs with classes, which is something we would want. It uses BEM styled class-names which is probably not so great for us.

[cooper-joe]

I have designed the first ~100 icons that can replace the various icons that are in use across DHIS2 apps today with the aim of consistent icon use. Ideally we could get ui-icons up and running with this initial batch.

There are some modifications to the original spec I discussed above, the icons now only need 2 props: size and color. I think the -filled modifier can instead be separate icons - there are a very small number of icons that need this modifier, so I think it's simpler to just split them. See below for a visual overview of the icons ready for inclusion. Only sizes S and M are ready right now, XS and L will be added as needed.

@amcgee, what do you think about how this library could be structured, and could we get a first draft of it to include in any apps for 2.35?

[HendrikThePendric]

Good stuff @cooper-joe. Let's wait for @amcgee before we take any action, but in the meanwhile I'd like to share my thoughts:

• There is a already an icons package in dhis2/ui, I think we should put these there and we don't need a separate repo.
• Before we proceed we need to decide what we want to do with the existing icons in that package. I've talked to @varl about this in the past, and if I remember that conversation correctly, I think he would prefer it if every ui component has his own icons colocated with the component. This way we eliminate the chance of getting unexpected changes in the components.
• I'm pretty sure the color can just be a CSS thing, and I expect the style declaration could be the same for each component.
• However, since every size variation has its own path/vector we'll need iconCount * sizeCount SVGs. So we're dealing with a lot of SVGs files here, but since we want code-splitting to work effectively we need each icon to be an isolated component.....

So I'm thinking that we can probably use some code generating script in this project to avoid us from manually editing 100s of files. Provided the SVGs are exported in a consistent manner, and with consistent file names, i.e. iconName_size, and all/most SVGs need the same styles, we can probably write a script what wraps these SVGs into a small template so they are available as a react component with the right props....