ni / nimble

The NI Nimble Design System
https://nimble.ni.dev
MIT License
29 stars 9 forks source link

Add icons for AI, clean, expand, collapse, debug #2165

Closed jattasNI closed 3 weeks ago

jattasNI commented 4 weeks ago

Pull Request

๐Ÿคจ Rationale

New icons are available. One of them is multicolored and we'd like to retain those colors.

Resolves #2138 Resolves #2046

๐Ÿ‘ฉโ€๐Ÿ’ป Implementation

Generally followed CONTRIBUTING instructions for adding icons.

  1. Naming:
    • the expand/collapse icons use Brandon's suggested names which are very free of metaphors
    • the clean icon uses "sparkles" to match FontAwesome
    • the AI icon uses "sparkle-swirls" since we generally want to avoid metaphors but I didn't see a FontAwesome equivalent
    • the debug icon uses "debug" since descriptive names seemed silly ("shield-with-legs"?) and the metaphor seems unique
    • (open to feedback on all the above)
  2. Synonyms: used the ones Brandon suggested
  3. I stripped color from all icons except the AI icon. This doesn't match our CONTRIBUTING guidance and caused a test to fail. I'm proposing an update to the guidance and removing those asserts from the test.
    • I considered establishing a convention where <defs> were allowed in svgs only if you explicitly tag them with a custom attribute that proves you left them in intentionally. But I lean towards just relying on code review to enforce best practices.
    • Note: #327 would be another opportunity for us to improve this if we want themeable multicolored icons.
  4. Minor update to use the new npm run storybook command in a CONTRIBUTING doc

๐Ÿงช Testing

Local storybook inspection

โœ… Checklist

jattasNI commented 4 weeks ago

I'll be OOO most of tomorrow. Someone is welcome to push this through before I'm back if the builds pass (I know one client was hoping for the icons to be available this week). @fredvisser @rajsite