Icon exploration v2 #3432

zamoore · 2025-12-09T20:05:37Z

📌 Summary

DO NOT MERGE - This PR is a spike with the goal of figuring out the best way to import Carbon icons in to HDS

🛠️ Detailed description

📸 Screenshots

🔗 External links

Jira ticket: HDS-XXX
Figma file: [if it applies]

👀 Component checklist

Percy was checked for any visual regression
A changelog entry was added via Changesets if needed (see templates here)

💬 Please consider using conventional comments when reviewing this PR.

📋 PCI review checklist

If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've worked with GRC to document the impact of any changes to security controls.
Examples of changes to controls include access controls, encryption, logging, etc.
If applicable, I've worked with GRC to ensure compliance due to a significant change to the in-scope PCI environment.
Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.

vercel · 2025-12-09T20:05:44Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
hds-showcase	Ready	Preview	Dec 20, 2025 0:12am
hds-website	Ready	Preview	Dec 20, 2025 0:12am

didoo

Did a general review of the PR. We're definitely in the right direction, there are still a few things I want to discuss with you to better understand (when you're back).

packages/flight-icons/scripts/build-parts/generateBundleSVGJS.ts

packages/components/src/components/hds/icon/index.ts

packages/components/src/components/hds/icon/index.gts

zamoore · 2025-12-16T22:16:08Z

Did a general review of the PR. We're definitely in the right direction, there are still a few things I want to discuss with you to better understand (when you're back).

What do you think of the changes? I believe I've addressed everything.

didoo

I think the overall architecture and implementation clicks now. There are a few small things to tweak/fix here and there, but nothing major. We're in a very good spot now, I think.

The next steps for me, before opening a proper PR (or a set of stacked PRs, to ease code reviewing/approval) are:

address the latest comments
stress-test a bit more the implementation (I remember in the other PR you had this huge grid of all the HDS icon with a switcher on top, we could add it to the component/icon page)
stress-test in a couple of consumer app (HCP/Cloud UI and TFC/Atlas, I would say)

When we're sure that everything works (there may be things we didn't think of) we can start to write down the proposed solution in the RFC, so we can gather feedback from our consumers (I would also ask the FEF team and @meirish for feedback)

What do you think? It's a plan?

didoo · 2025-12-17T12:43:21Z

hds-carbon-icon-map.json

FYI @jorytindall is working on a script that generates a JSON with these mappsings, later we can use his JSON as "source of truth" so design and engineering are in sync

packages/components/src/components/hds/icon/index.ts

packages/components/src/components/hds/icon/index.gts

packages/flight-icons/scripts/build-parts/generateBundleSymbolJS.ts

packages/flight-icons/scripts/build-parts/generateBundleSVGJS.ts

didoo · 2025-12-17T14:25:20Z

packages/flight-icons/scripts/build-parts/generateBundleSymbolJS.ts

+        }
+    }
+
+    // Generate Registry (JS + Types)


Not sure if this comment I've found in Carbon is relevant for us, but I wanted to share it with you just in case: https://github.com/carbon-design-system/carbon/blob/main/packages/icon-build-helpers/src/builders/react/builder.js#L75-L84

packages/flight-icons/scripts/build-parts/generateBundleSVGJS.ts

packages/flight-icons/scripts/build-parts/generateBundleSymbolJS.ts

packages/flight-icons/scripts/build-parts/generateBundleSVGJS.ts

didoo

I haven't fully reviewed generateBundleSymbolJS.ts but everything else yes, but everything is great 👍 and I think at this point we can start to think how to open the "official" PR (or PRs, if you think you can split it somehow). Branching from the PR #3237 so you can use directly the hdsTheming service instead of the hdsCarbon service

didoo · 2025-12-18T16:47:48Z

packages/components/src/components/hds/icon/index.ts

+    const { name } = this.args;
+    const registryEntry = IconRegistry[name as HdsIconName];
+
+    if (this.hdsCarbon.carbonModeEnabled && registryEntry?.carbon) {


[note] below it's registryEntry.carbon (without ?), which one is correct?

They're both correct. The second instance just has an assertion above it.

@zamoore OK, this lead me to another observation: you dont' need at all to set the viewport to 32px: because of how SVG works, the important thing is that the <symbol> has the correct viewport (32px) for the Carbon icons, here you can leave the standard viewport; which means you can get rid of this whole getter and just use 0 0 ${this.size} ${this.size} directly in the template as we did before

didoo · 2025-12-18T16:52:14Z

packages/components/src/components/hds/icon/index.ts

+import type { SafeString } from '@ember/template';
 import type { IconName } from '@hashicorp/flight-icons/svg';
-import type Owner from '@ember/owner';
+import type { HdsIconModule, HdsIconName } from '@hashicorp/flight-icons/symbol-js/registry.ts';


[note] The fact that we have IconName and HdsIconName may lead to confusion, what other name can we think of, for the HdsIconName imported from the registry?

or could we use a Partial of the IconName for the IconRegistry type?

We can use IconName directly which cleans up some of the typing. Good suggestion.

packages/components/src/components/hds/icon/index.ts

didoo · 2025-12-18T16:55:52Z

packages/components/src/components/hds/icon/index.ts

+
+      const loader = shouldUseCarbon 
+        ? registryEntry.carbon 
+        : registryEntry.flight[size];


[praise] Neat! ❤️

didoo · 2025-12-18T16:57:56Z

packages/components/src/components/hds/icon/index.ts

+      if (loader) {
+        this.iconModule = await loader();
+      } else {
+        assert(`Size "${size}" not found for icon "${name}"`);


Mmm, are we sure this is the correct error message for this condition? Probably we can add a bit of logic here to differentiate the different messages for the different cases/errors/fails?

didoo · 2025-12-18T18:34:59Z

packages/flight-icons/symbol-js/registry.d.ts

+    carbon: (() => Promise<HdsIconModule>) | null;
+}
+
+export declare const IconRegistry: Record<string, HdsIconRegistryEntry>;


[praise] so much simpler than the previous version! 👏

didoo · 2025-12-18T21:36:06Z

@zamoore FYI this will be soon merged: #3444 and then I'll rebase #3237 on main so if you branch and start to work on the final PR(s) for this work, you can do it directly in .gts format

alex-ju

Some general consideration on my end – started a few days ago, but took me a while to re-assess today as many of my suggestions were no longer relevant/valid.

The refactored registry structure is a major improvement (grouping icons by base name with nested size loaders reduces registry size in half and creates much clearer separation between size-specific Flight icons and universal Carbon icons. Moving from bundled files to separate /symbol-js/flight/ and /symbol-js/carbon/ folders completely addresses my initial network overhead concern. With HTTP/2 multiplexing and individual file caching, this dynamic loading approach should actually be more efficient than bundled sprites. The simplified regex-based SVG processing and universal 32px Carbon icons with dynamic viewBox adjustment are both elegant solutions.

I think this is now production-ready pending stress testing (ideally with some basic performance metrics). I would personally look for any potential FOUC during async icon loads.

I would also recommend explicitly documenting the fallback behaviour (e.g. when Carbon mode is enabled but no Carbon equivalent exists).

Overall, the architecture is sound, backward compatibility is maintained, and the approach is well-suited for modern HTTP/2 environments. Great work @zamoore and @didoo!

hashibot-hds added the packages/flight-icons label Dec 9, 2025

vercel bot deployed to Preview – hds-showcase December 9, 2025 20:06 View deployment

vercel bot deployed to Preview – hds-showcase December 9, 2025 20:10 View deployment

vercel bot deployed to Preview – hds-showcase December 9, 2025 20:15 View deployment

vercel bot deployed to Preview – hds-website December 9, 2025 20:17 View deployment

hashibot-hds added packages/components showcase labels Dec 10, 2025

vercel bot deployed to Preview – hds-showcase December 10, 2025 21:40 View deployment

vercel bot deployed to Preview – hds-website December 10, 2025 21:42 View deployment

didoo reviewed Dec 11, 2025

View reviewed changes

zamoore force-pushed the icon-exploration-v2 branch from 3ff6b1b to 94f6d16 Compare December 16, 2025 16:09

vercel bot deployed to Preview – hds-showcase December 16, 2025 16:11 View deployment

vercel bot deployed to Preview – hds-website December 16, 2025 16:14 View deployment

vercel bot had a problem deploying to Preview – hds-website December 16, 2025 20:57 Failure

vercel bot had a problem deploying to Preview – hds-showcase December 16, 2025 20:57 Failure

vercel bot deployed to Preview – hds-showcase December 16, 2025 22:10 View deployment

vercel bot deployed to Preview – hds-website December 16, 2025 22:13 View deployment

zamoore requested a review from didoo December 16, 2025 22:16

didoo reviewed Dec 17, 2025

View reviewed changes

vercel bot had a problem deploying to Preview – hds-website December 18, 2025 00:27 Failure

vercel bot had a problem deploying to Preview – hds-showcase December 18, 2025 00:27 Failure

vercel bot deployed to Preview – hds-showcase December 18, 2025 01:03 View deployment

vercel bot deployed to Preview – hds-website December 18, 2025 01:06 View deployment

vercel bot deployed to Preview – hds-showcase December 18, 2025 01:44 View deployment

vercel bot deployed to Preview – hds-website December 18, 2025 01:46 View deployment

didoo reviewed Dec 18, 2025

View reviewed changes

vercel bot deployed to Preview – hds-showcase December 19, 2025 11:58 View deployment

vercel bot deployed to Preview – hds-website December 19, 2025 12:01 View deployment

vercel bot deployed to Preview – hds-showcase December 19, 2025 12:42 View deployment

vercel bot deployed to Preview – hds-website December 19, 2025 12:45 View deployment

vercel bot deployed to Preview – hds-showcase December 19, 2025 13:20 View deployment

vercel bot deployed to Preview – hds-website December 19, 2025 13:23 View deployment

vercel bot deployed to Preview – hds-showcase December 19, 2025 14:11 View deployment

vercel bot deployed to Preview – hds-website December 19, 2025 14:14 View deployment

vercel bot deployed to Preview – hds-showcase December 19, 2025 22:44 View deployment

vercel bot deployed to Preview – hds-website December 19, 2025 22:47 View deployment

zamoore force-pushed the icon-exploration-v2 branch from d5e2a40 to 84700b5 Compare December 19, 2025 23:59

vercel bot deployed to Preview – hds-showcase December 20, 2025 00:02 View deployment

zamoore added 2 commits December 19, 2025 19:03

icon exploration work

ed500f9

built icons

370edf0

zamoore force-pushed the icon-exploration-v2 branch from 9d863da to 370edf0 Compare December 20, 2025 00:03

vercel bot deployed to Preview – hds-showcase December 20, 2025 00:05 View deployment

vercel bot deployed to Preview – hds-website December 20, 2025 00:12 View deployment

alex-ju reviewed Dec 22, 2025

View reviewed changes

alex-ju mentioned this pull request Dec 22, 2025

Draft: Carbon Icon exploration spike - Do not merge #3382

Draft

5 tasks

Icon exploration v2 #3432

Are you sure you want to change the base?

Icon exploration v2 #3432

Uh oh!

Conversation

zamoore commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 Summary

🛠️ Detailed description

📸 Screenshots

🔗 External links

👀 Component checklist

Uh oh!

vercel bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

didoo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zamoore commented Dec 16, 2025

Uh oh!

didoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

didoo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

didoo commented Dec 18, 2025

Uh oh!

alex-ju left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zamoore commented Dec 9, 2025 •

edited

Loading

vercel bot commented Dec 9, 2025 •

edited

Loading