Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(tag): web component parity #18774

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

annawen1
Copy link
Member

@annawen1 annawen1 commented Mar 6, 2025

Closes #18653

This updates the cds-tag component with the latest features from React.

New components include:

  • cds-tag-skeleton
  • cds-dismissible-tag
  • cds-selectable-tag
  • cds-operational-tag

NOTE: The tag component incorporates the definition-tooltip that's used when the tag content results in an ellipsis. However since definition-tooltip doesn't exist in Web Components yet, I'm going to create a ticket to add the definition-tooltip to the tag component once completed.

Changelog

New

  • New tag components
    • cds-tag-skeleton
    • cds-dismissible-tag
    • cds-selectable-tag
    • cds-operational-tag

Changed

  • modified the tooltip event methods to accommodate for tag functionality where the tooltip content doesn't appear outside of keyboard interactions

Testing / Reviewing

Review with the tag stories on Storybook and compare with React version

@annawen1 annawen1 requested review from a team as code owners March 6, 2025 13:52
@annawen1 annawen1 requested review from heloiselui and kennylam March 6, 2025 13:52
@annawen1 annawen1 marked this pull request as draft March 6, 2025 13:52
@annawen1 annawen1 added the package: @carbon/web-components @carbon/web-components label Mar 6, 2025
Copy link

netlify bot commented Mar 6, 2025

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit efc9c80
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/67e46e2fa0429e0007b2816d
😎 Deploy Preview https://deploy-preview-18774--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Mar 6, 2025

Deploy Preview for v11-carbon-react ready!

Name Link
🔨 Latest commit efc9c80
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/67e46e2f89d9e3000722fab5
😎 Deploy Preview https://deploy-preview-18774--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

annawen1 added 3 commits March 6, 2025 13:13

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

netlify bot commented Mar 12, 2025

Deploy Preview for v11-carbon-web-components ready!

Name Link
🔨 Latest commit efc9c80
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-web-components/deploys/67e46e2f6e251d000846ab0f
😎 Deploy Preview https://deploy-preview-18774--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Mar 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.85%. Comparing base (6d2c791) to head (efc9c80).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #18774   +/-   ##
=======================================
  Coverage   84.85%   84.85%           
=======================================
  Files         396      396           
  Lines       14491    14491           
  Branches     4743     4775   +32     
=======================================
  Hits        12297    12297           
  Misses       2045     2045           
  Partials      149      149           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

annawen1 added 10 commits March 13, 2025 15:54

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link
Contributor

@heloiselui heloiselui left a comment

Choose a reason for hiding this comment

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

Hey Anna, looks great!
I think we just need to take a look at these things:

For all new components: When I change controls > disable > true, the components are not being disabled. They do not behave like in React, meaning they don’t appear grayed out, and the mouse pointer doesn’t indicate that they are "blocked."

Selectable: By default, React has a tag already selected (all black). Maybe including this behavior in the Web Component would be nice.

kennylam and others added 3 commits March 24, 2025 12:40

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Heloise Lui <[email protected]>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Heloise Lui <[email protected]>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Heloise Lui <[email protected]>
Comment on lines 92 to +93
--cds-layout-size-height-md: 1.5rem;
--cds-layout-size-height-lg: 2rem;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
--cds-layout-size-height-md: 1.5rem;
--cds-layout-size-height-lg: 2rem;
--cds-layout-size-height-md: $spacing-06;
--cds-layout-size-height-lg: $spacing-07;

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually looks like I have to give it a hardcoded value here as the token doesn't get complied it looks like

Screenshot 2025-03-26 at 1 23 04 PM

Copy link
Member

Choose a reason for hiding this comment

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

What about interpolating it? Like #{$spacing-06}.

annawen1 and others added 2 commits March 26, 2025 09:29

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: kennylam <[email protected]>
Comment on lines +259 to +267

// border: 1px solid $border-disabled;
// background-color: $layer;
// color: $text-disabled;

inline-size: rem(60px);
// &:hover {
// background-color: $layer;
// cursor: not-allowed;
// }
Copy link
Member

Choose a reason for hiding this comment

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

These can be removed?

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

React|WC Parity: Tag
3 participants