Skip to content

[Project Solar / Phase 1 / Components] Tag carbonization#3685

Open
dchyun wants to merge 6 commits intoproject-solar/phase-1-main-feature-branchfrom
project-solar/phase-1/dchyun/tag-carbonization
Open

[Project Solar / Phase 1 / Components] Tag carbonization#3685
dchyun wants to merge 6 commits intoproject-solar/phase-1-main-feature-branchfrom
project-solar/phase-1/dchyun/tag-carbonization

Conversation

@dchyun
Copy link
Contributor

@dchyun dchyun commented Mar 9, 2026

📌 Summary

This PR carbonizes the Tag component and adds component tokens for it to the tokens library.

Tag carbonization page

🛠️ Detailed description

Summary of changes:

  • Aligned typography, colors, paddings, other foundational styles
  • Refactored borders to support link specific border colors

🔗 External links

Jira ticket: HDS-6107


👀 Component checklist

  • Percy was checked for any visual regression

💬 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
Copy link

vercel bot commented Mar 9, 2026

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

Project Deployment Actions Updated (UTC)
hds-showcase Ready Ready Preview Mar 18, 2026 8:52pm
hds-website Ready Ready Preview Mar 18, 2026 8:52pm

Request Review

.hds-tag__dismiss,
.hds-tag__text {
color: var(--token-color-foreground-primary);
color: var(--token-tag-foreground-color-default);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is causing the icon in the dismiss button to match the text, which is not how the primary variant appears now. So the x in the dismiss button is matching the color of the text.

Before
Image

After
Image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this style was there before (now gone in this PR) that was controlling the svg icon color:

.hds-tag__dismiss-icon, .hds-tag__text {
    color: var(--token-color-foreground-primary);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been fixed now. I added in some icon color tokens like there are in Figma. I had to separate the link primary and secondary colors for the icon from the plain text color. So I have three new variables

  • tag.icon.color
  • tag.link.icon.color.primary
  • tag.link.icon.color.secondary

Copy link
Contributor

@jorytindall jorytindall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've matched up a couple of things in the Figma component (mainly the focus ring), I think this is looking solid, just left one note about the color of the icon in the dismiss button that is being inherited

@didoo didoo force-pushed the project-solar/phase-1-main-feature-branch branch 3 times, most recently from 395ea90 to 6d0c7de Compare March 18, 2026 18:34
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Note] I added a letter spacing token, because without it the cut off for long text would differ between Carbonized HDS and the Carbon component.

Carbonized HDS w/out letter spacing token

Image

Carbon

Image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I would leave the cutoff different (what is the issue)? I think we're overthinking this letter-spacing thing. For the end-user is not perceivable, it should be considered in the context of a wider, more complex UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants