Multi-color icon support

[fredvisser]

🤨 Rationale

Adds opt-in multi-color icon support to Nimble. Single-color icons remain unchanged.

👩‍💻 Implementation

Architecture:

• Multi-color icons extend MultiColorIcon base class
• Icons are manually defined in src/icons-multicolor/ (not auto-generated)
• Static styles define layer colors using CSS custom properties (--ni-nimble-icon-layer-N-color)
• registerMultiColorIcon helper composes the base icon styles with the specific multi-color styles
• No factory functions or runtime style generation—fully declarative

Icon Creation:

• Multi-color icons manually created in src/icons-multicolor/ directory
• Icons listed in src/icon-base/tests/icon-multicolor-metadata.ts
• Build scripts (Angular/React wrappers) detect multi-color icons via metadata and use correct import paths
• Build-time validation ensures consistency and layer count limits

Styling:

• SVG classes cls-1 through cls-6 map to design tokens (max 6 layers supported)
• Layer colors defined statically in the component file
• Multi-color icons don't support severity attribute—each layer has fixed theme color

Example: See nimble-icon-circle-partial-broken (Chromatic)

🧪 Testing

• Unit tests for multi-color functionality
• Visual regression tests in Storybook/Chromatic
• Build-time validation script enforces layer count limits and metadata consistency

✅ Checklist

• Documentation updated (CONTRIBUTING.md in nimble-components and nimble-tokens)

[rajsite]

I'm skeptical about the implementation direction of the PR. API-wise as custom element authors we shouldn't be using data attributes, those are for users for user use-cases.

And I don't think the single attribute handles the use cases I've seen for multicolor icons. I think there are both multi-tone icons (which need multi theme aware colors configured) and like multi-tone configurable icons (icons that have like a configuration api to show both theme aware tones and have an api for configuring part of an icon like a separate status).

Haven't thought it through completely but maybe these icons with extra features should actually be separate files that extend Icon. i.e. they are treated more like web components and can have additional capabilities / apis instead of wedging everything into an Icon class and overly generic generated code behaviors.

[fredvisser]

we shouldn't be using data attributes, those are for users for user use-cases.

Is that right? Copilot tells me that:

Use data-attributes (data-*) when:

• You are storing custom, private, or application-specific data on standard HTML elements.
• The attribute is not part of the HTML specification or a Web Component’s published API.

…and that's the rationale here - data-multicolor is not intended to part of the published API. Is there an alternative approach?

I talked it through with Brandon, and we don't currently have use cases for the multi-color configurable icons, so I have no interest in supporting it at this time.

[rajsite]

Is there an alternative approach?

Did you ask copilot? Would be interesting what it finds. Maybe I'll keep it secret and you tell me. Yes there are alternative approaches to storing state on custom elements to data attributes. They are used ubiquitously in practically every web component.

Also ask it for an approach that does not rely on code generation. It can look through PR history where lots of effort was put in to avoid excessive code generation. Another hint is we have patterns for "extending" components for specialized behavior. Goal is pretty much all source should be in source, almost none in build scripts.

There is probably also an exercise needed in how one opens up a PR proposing larger pattern changes vs incremental feature changes designed by AI.

Edit: the above is framed as an exploration of how to try and get the AI to generate code and patterns expected in nimble. That also may not be the goal of your questions? I'm not actually sure what the intentions of this PR is. Try and make code that owners would happily approve? Then we need more refinement because the direction here seems out of line of normal in nimble and my hunch is that it is unnecessary done so. Maybe we would want to encourage asking in the issue about implementation directions before starting or opening PRs in draft to get input in direction. Use AI to generate anything that seems functional and accept that? That's the intention of the ok packages, not core nimble.

[jattasNI]

I don't think the latest update generated with stricter architectural prompting has succeeded in creating something that meets the architectural bar of the Nimble package. It reduced the amount of code generation and stopped using data attributes, which are steps in the right direction. But it also started setting styles from code rather than stylesheets, broke the architectural boundaries of our packages which are intended to be standalone, created build scripts that don't report an error when input files are malformed, and added a lot of code that still seems verbose and complex to human eyes (and those are just the issues I noticed in an initial scan).

We should talk about what direction to go next. I can think of a few:

1. make minimal changes to nimble-tokens and nimble-components to allow this approach to go into the "ok" packages
2. have a conversation about the architecture we want and incorporate that into the prompts and see if we're happier with the resulting code
3. keep refining prompts to capture each flaw that owners find in Copilot's attempts in the hopes that eventually it can discover an architecture we're happy with

1 sounds like the quickest if the goal is simply to add this capability ASAP
2 sounds like a quicker way to get to a good architecture but doesn't teach the prompts much about best practices
3 sounds tedious but possibly valuable if the end result is an AI that can more productively contribute to the core nimble packages with less owner input.

[fredvisser]

I'm game for trying option 3 a little longer. Take a look at this latest approach and see if it's converging on an acceptable solution. I've updated the review requirements to reflect this feedback, and iterated a bit more.

(The architectural boundaries issue was definitely me doing a poor job trying to reduce code duplication from the initial copilot solution)

[jattasNI]

The creation of a new base class for multicolor icons feels like a good direction but incompletely done:

• there are multicolor styles in the Icon base class. Instead we could use our existing patterns for composing styles and have separate styles files for multicolor and single color icons
• the template is identical to the single color icon template. Instead we could share the template.
• places that try to distinguish between single and multi color icons do so via a mix of heuristics. Instead we could put the icons in separate folders

These are just a few examples, but I think this is a case where sitting down and talking about it rather than letting the LLM make guesses at architecture would be more efficient.

[jattasNI]

While this build script is somewhat better since it's no longer relying on log/warning for errors, it's still pretty far from clean code. It uses a mix of error handling mechanisms and it's still reaching out to other packages in strange ways. If the goal is validating contents in nimble-tokens, should this just be a script in that package?