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 18, 2025 1:46am
hds-website	Ready	Preview	Dec 18, 2025 1:46am

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

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/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.

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?

didoo · 2025-12-18T16:53:43Z

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

+      const shouldUseCarbon = carbonModeEnabled && registryEntry.carbon;
+
+      const loader = shouldUseCarbon 


no need to define a temporary variable, we can use directly the evaluation in the ternary

Suggested change

const shouldUseCarbon = carbonModeEnabled && registryEntry.carbon;

const loader = shouldUseCarbon

const loader = carbonModeEnabled && registryEntry.carbon

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

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 added 5 commits December 16, 2025 11:00

added the icon mapping to the root dir

057263b

added built icons

0167397

revert unneeded changes

938c340

changing copyright

e95dae6

building icon paths

94f6d16

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

makign script adjustments

283e64c

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

addressed feedback

ac2aef4

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

responding to PR feedback

56d04d7

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

finished addressing PR feedback

38afacc

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

added showcase examples

17f3102

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

		const shouldUseCarbon = carbonModeEnabled && registryEntry.carbon;

		const loader = shouldUseCarbon

Icon exploration v2 #3432

Are you sure you want to change the base?

Icon exploration v2 #3432

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zamoore commented Dec 9, 2025 •

edited

Loading

vercel bot commented Dec 9, 2025 •

edited

Loading