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: Refactor Pill component to use new styling utilities and tokens #3104

Merged
merged 63 commits into from
Mar 17, 2025

Conversation

mannycarrera4
Copy link
Contributor

@mannycarrera4 mannycarrera4 commented Jan 23, 2025

Summary

Fixes: #2394, #2968, #2049

Release Category

Components

Release Note

We've updated ExternalHyperlink to use our new styling utilities and tokens.

  • The border color on hover has been updated from licorice400 to licorice500 to match our design specs.
  • We've removed extra elements and leverage [flex box}(https://developer.mozilla.org/en-US/docs/Learn_web_development/Core/CSS_layout/Flexbox) to ensure only the label receives overflow styles. When maxWidth is set, it is set on the parent <Pill/> element and the child elements will be styled accordingly. Before v13, maxWidth wasn't calculating the width of all its elements and wasn't a true pixel value.

BREAKING CHANGES

  • maxWidth has been removed from the usePillModel. This config was used to style sub components. With the refactor to use data-part and stencils, it is no longer needed on the model. You can still apply maxWidth on the parent <Pill> element and the child elements will be styled accordingly.
  • Pill.Icon no longer has a default aria-labe="add". You must provide an aria-label for Pill.Icon to ensure proper accessibility. Our examples have been updated to reflect this change.
  • Pill.IconButton no longer has a default aria-label="remove". You must provide an aria-label for Pill.IconButton to ensure proper accessibility. Our examples have been updated to reflect this change.
  • Pill.Label is a required element when using other sub components like Pill.Icon to ensure that the label truncates correctly.

Checklist

For the Reviewer

  • PR title is short and descriptive
  • PR summary describes the change (Fixes/Resolves linked correctly)
  • PR Release Notes describes additional information useful to call out in a release message or removed if not applicable
  • Breaking Changes provides useful information to upgrade to this code or removed if not applicable

Where Should the Reviewer Start?

Areas for Feedback? (optional)

  • Code
  • Documentation
  • Testing
  • Codemods

Testing Manually

Screenshots or GIFs (if applicable)

Thank You Gif (optional)

Copy link

cypress bot commented Jan 23, 2025

Workday/canvas-kit    Run #8507

Run Properties:  status check passed Passed #8507  •  git commit 8901021d95 ℹ️: Merge 4b5cd4fcda79914253e9c0bd84848e1ad8f37e71 into 7575839a5da478211d9ab816e5c7...
Project Workday/canvas-kit
Branch Review mc-refactor-pills
Run status status check passed Passed #8507
Run duration 02m 46s
Commit git commit 8901021d95 ℹ️: Merge 4b5cd4fcda79914253e9c0bd84848e1ad8f37e71 into 7575839a5da478211d9ab816e5c7...
Committer Manuel Carrera
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 21
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 936
View all changes introduced in this branch ↗︎
UI Coverage  21.31%
  Untested elements 1530  
  Tested elements 412  
Accessibility  99.27%
  Failed rules  6 critical   5 serious   0 moderate   2 minor
  Failed elements 98  

@mannycarrera4 mannycarrera4 marked this pull request as ready for review January 27, 2025 21:27
@mannycarrera4 mannycarrera4 added the ready for review Code is ready for review label Jan 27, 2025
@mannycarrera4 mannycarrera4 added automerge and removed ready for review Code is ready for review labels Mar 17, 2025
@alanbsmith alanbsmith merged commit dcac6a3 into Workday:prerelease/major Mar 17, 2025
18 of 20 checks passed
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.

4 participants