-
Notifications
You must be signed in to change notification settings - Fork 34
feat(#2952): filter chip v2 styling update #3138
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
base: dev
Are you sure you want to change the base?
Conversation
twjeffery
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bdfranck I noticed that the design in Figma had a couple additional properties that were added but not shown explicitly (leading icon and secondary text). I referenced the design templates again from the agency and saw that the chip design was also a bit out of date. I've updated it, and it was faster to update everything in a new commit rather than writing in code suggestions. Feel free to change/remove anything there that's not needed or wanted.
I'll also share a detailed doc with you on Slack explaining all of the changes and we can discuss as needed as it won't upload here.
On a high level:
Design Adjustments:
- Changed border radius from {borderRadius.xl} (8px) to {borderRadius.2xl}
(1rem/16px) - kept V1 value - Changed default border color from greyscale.500 to greyscale.300 (lighter)
- Added hover border color: greyscale.500
- Changed icon size from 24px (medium) to 20px (small) in V2
- Adjusted height from 40px to 36px (changed padding-vertical from 7px to 5px)
- Added hover icon color: greyscale.black
- Removed padding-bottom font alignment fix for V2
Interaction Changes:
- Made only the close icon clickable in V2 (instead of entire chip)
- Wrapped close icon in element with 24x24px minimum touch target
- Changed hover behavior to only trigger on icon button (not whole chip)
- Changed focus to :focus-visible on button (circular focus ring)
- Changed cursor pointer to only show on icon button
New Features Added:
- leadingicon prop - optional icon before text (20px, greyscale.600)
- secondarytext prop - optional label before main text (greyscale.600)
- Added label-container wrapper with 6px gap between icon/secondary/text
Token Structure:
- Simplified padding tokens to single composite token approach
- Added component tokens that reference input.* global tokens for error states
- Added hover-specific tokens for border, icon, and text colors
- Added filter-chip-label-gap token (6px)
Error State Refinements:
- Default text color uses input.color.border.error (lighter)
- Hover text color uses input.color.text.error (darker)
- Icon opacity changed from 0.5 to 1.0 (always fully visible)
- Leading icon and secondary text inherit error colors
See detailed review doc for complete implementation details.
|
@bdfranck This PR will also need it's commits cleaned up |
0e9489e to
3aede37
Compare
3aede37 to
716a9e6
Compare
|
Thanks for the suggestions, @twjeffery! I've amended my commit with the following changes:
|
twjeffery
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates Benji! The structure looks good - using goa-icon-button
for the close action is a nice approach.
I've added a few inline suggestions:
Token naming fixes - A few CSS variables didn't match the generated token names
(e.g., bg-color vs color-bg), which would cause the V2 styles to fall back to
V1 values.
Missing tokens - Added filter-chip-secondary-text-color-error and
filter-chip-close-button-error-hover-bg-color which were referenced in CSS but
didn't exist in the token file.
Error state color - Changed the leading icon error color to use
input.color.text.error (darker) for better visibility.
I've tested locally and it's looking good with these changes.
|
@twjeffery My apologies! I forgot to push my token updates to match my amended commit. I changed |
twjeffery
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.


This PR updates the Filter Chip styling to match the v2 Figma designs.
Changes include:
versionproperty to control which close icon to useleadingiconandsecondarytextproperties