cfpb / design-system

CFPB's work-in-progress design system
https://cfpb.github.io/design-system
Creative Commons Zero v1.0 Universal
40 stars 13 forks source link

cfpb-icons: make a one-to-one match between icon file and canonical name #1252

Closed anselmbradford closed 1 year ago

anselmbradford commented 3 years ago

Which page is this about? https://cfpb.github.io/design-system/foundation/iconography

What kind of issue is this? Add one or more of the issue labels below to the right-hand sidebar AND issue title. (We strongly encourage you to make content changes yourself where possible!)

Describe your issue Icons have synonyms. In the case of the error icon, there is also delete, close, remove, multiply, multiplication, x, which means there are seven SVG files for that one icon.

Instead, we should choose a canonical name that is descriptive for the icon and have only one SVG file. References to any of the synonyms would need to be updated.

To Reproduce Steps to reproduce the behavior:

  1. Go to https://cfpb.github.io/design-system/foundation/iconography and see the aliases column.
  2. See in https://github.com/cfpb/design-system/tree/master/packages/cfpb-icons/src/icons we have duplicate SVG files for each alias.
Scotchester commented 3 years ago

I'm biased, but just to make the argument for having the aliases: it's realllly nice to be writing code that creates an icon and be able to guess at just about any name you think the icon you see in your mind might have and probably be right.

Yeah, the extra files are a pain, but that could be solved or made less painful with a little extra tooling (starting with maintaining a list of aliases in a machine-readable format like JSON or YAML).

But, if you do go down this road, I would ensure that all of the canonical names are set to something that merely visually describes the icon, if they don't already. The error case you bring up is a good one. It'd be weird to have a modal close button specify an icon of error. That should be changed to just x probably.

anselmbradford commented 3 years ago

Good to hear your input @Scotchester !! Hope all is well in the skies!

I appreciate the ease of being able to choose from a variety of contextual names for the icons, but I'm not entirely sold on the idea. Take svg_icon( 'medical-round' ) for example. Which icon is it?

Screen Shot 2021-06-25 at 9 45 06 AM Screen Shot 2021-06-25 at 9 45 13 AM

These two are health-insurance and healthcare, respectively. medical is an alias of health-insurance. If there was just one mapping for each, however weirdly named, there would be less ambiguity over what name points to what icon. I don't see how the alias helps someone reading the code from needing to look up the icon. In fact, it appears it would make it more likely the reader would need to look it up, since once they've learned what the health-insurance icon looks like, when they encounter medical, they may have to go back and look that one up too.

Then there are the cases of icons where the contextual nuance between the canonical name versus the alias is virtually identical (wifi/wi-fi, pets/pet, phone/telephone, etc.). The problem is, say you needed to find all occurrences of the pets icon in the codebase, to swap it to something else, for example. Now you have to search for all occurrences of pets and then search again for all occurrences of pet. The minor semantic convenience there doesn't seem worth the potential maintenance burden.

I agree describing the icon and making that the canonical name is the best way to go, but for the same searchability-reasons we should probably avoid single letter names (such as x).

If we do want aliases I think that shouldn't be accomplished through duplication of the icon files themselves. The mechanism should instead happen at the point the icon is inserted. It seems like it would be trivial to add a lookup table to the svg_icon helper, for instance.

Scotchester commented 3 years ago

I appreciate the ease of being able to choose from a variety of contextual names for the icons, but I'm not entirely sold on the idea. Take svg_icon( 'medical-round' ) for example. Which icon is it?

These two are health-insurance and healthcare, respectively. medical is an alias of health-insurance. If there was just one mapping for each, however weirdly named, there would be less ambiguity over what name points to what icon. I don't see how the alias helps someone reading the code from needing to look up the icon. In fact, it appears it would make it more likely the reader would need to look it up, since once they've learned what the health-insurance icon looks like, when they encounter medical, they may have to go back and look that one up too.

Yeah, there will be situations no matter what where it will be ambiguous and you have to reference the docs. But just picking one canonical name for each of these won't change that, probably, unless you go with something like shield-with-cross and stethoscope.

Another consideration in the naming of these was that a whole category of icons was added that were intended to be for types of expenses, so the canonical names reflect that context. If those icons start to be used in other places, then the names become more confusing.

Then there are the cases of icons where the contextual nuance between the canonical name versus the alias is virtually identical (wifi/wi-fi, pets/pet, phone/telephone, etc.). The problem is, say you needed to find all occurrences of the pets icon in the codebase, to swap it to something else, for example. Now you have to search for all occurrences of pets and then search again for all occurrences of pet. The minor semantic convenience there doesn't seem worth the potential maintenance burden.

Why would you ever need to do that, though? If the aliases remain, the only updating that needs to be done is to the source SVG files themselves. It's unlikely that you'd need to change every instance of a Wi-Fi icon to some other concept entirely.

If we do want aliases I think that shouldn't be accomplished through duplication of the icon files themselves. The mechanism should instead happen at the point the icon is inserted. It seems like it would be trivial to add a lookup table to the svg_icon helper, for instance.

Yes, I had this thought, as well. The problem comes with other systems that are less controllable than Django, such as this very repo's docs. Jekyll/Liquid doesn't allow us (as far as I know) to create a helper function that can look up a file to be included by multiple references like we could do with the Django helper. Maybe a custom Jekyll plugin could be written do it (get ready for writing some Ruby!) but that wouldn't be usable for GitHub Pages sites where GitHub's Jekyll does the building. But maybe that's not an important use case anymore, either.

anselmbradford commented 3 years ago

Yeah, there will be situations no matter what where it will be ambiguous and you have to reference the docs.

Having one option makes this easier as there's less to look up 😁

The descriptive naming looks good to me. Though I doubt we'd get to it anytime soon.

Why would you ever need to do that, though? If the aliases remain, the only updating that needs to be done is to the source SVG files themselves. It's unlikely that you'd need to change every instance of a Wi-Fi icon to some other concept entirely.

Swapping for another icon is probably unlikely, but knowing where the icon is used seems possible for checking how it appears across multiple contexts. It seems like there are a bunch of aliases we don't even use (though I could be wrong if they're set in Wagtail, which seems like further evidence that the aliases complicate things).

Yes, I had this thought, as well. The problem comes with other systems that are less controllable than Django, such as this very repo's docs. Jekyll/Liquid doesn't allow us (as far as I know) to create a helper function that can look up a file to be included by multiple references like we could do with the Django helper. Maybe a custom Jekyll plugin could be written do it (get ready for writing some Ruby!) but that wouldn't be usable for GitHub Pages sites where GitHub's Jekyll does the building. But maybe that's not an important use case anymore, either.

I think this is okay. That leaves any helpers to the context in which they are used. If it was burdensome to use the canonical icon name in a particular context a workaround could be made for that context, but the burden of looking them up if it's not clear seems pretty low. It doesn't look like we use any aliases for the docs in this repo, for instance. At least not in the iconography table that I see. I found very few aliases used overall outside of the error/delete/close set and some that use chevron-, but maybe there are more inside the wagtail data.

anselmbradford commented 3 years ago

Chatted with @contolini and we'd like to only have one canonical name for the icons and we could leave the "aliases" column on https://cfpb.github.io/design-system/foundation/iconography (maybe renamed as "commonly referred to as" or similar), but make it clear those names don't appear in the code and are just there to help in searching for an icon on the design system.