Skip to content

ol-chip: use close icon, add border-radius-chip token#12145

Merged
cdrini merged 1 commit intointernetarchive:masterfrom
lokesh:chip-follow-up
Mar 27, 2026
Merged

ol-chip: use close icon, add border-radius-chip token#12145
cdrini merged 1 commit intointernetarchive:masterfrom
lokesh:chip-follow-up

Conversation

@lokesh
Copy link
Copy Markdown
Collaborator

@lokesh lokesh commented Mar 20, 2026

Summary

  • Replace checkmark with close (X) icon on selected chips and remove the icon carousel hover animation
  • Add --border-radius-chip semantic token so multi-line chips don't get an exaggerated pill shape
  • Refactor all semantic border-radius tokens to reference their primitives instead of hardcoded values

Test plan

  • Verify single-line chips render with correct border-radius
  • Verify multi-line chips (long labels) render with correct border-radius
  • Verify selected chips show X icon on the left

Screenshots

Screenshot 2026-03-20 at 3 33 21 PM

'X' icon on left

image

border radius for multi-line chips renders with same value as single line

Stakeholders

@cdrini cc: @mekarpeles

- Replace checkmark with close (X) icon on selected chips
- Remove icon carousel hover animation on selected state
- Add --border-radius-chip semantic token for chips
- Refactor all semantic border-radius tokens to reference primitives

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@lokesh lokesh requested a review from cdrini March 20, 2026 23:09
--border-radius-badge: 2px; /* badges, tags */
--border-radius-notification: 8px; /* notifications, alerts */
--border-radius-avatar: 50%; /* avatars, profile pictures */
--border-radius-button: var(--border-radius-md); /* buttons, tabs */
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I had overlooked this in a previous PR and used hardcoded values for these semantic tokens instead of the the primitives that are defined further up in this same file. Fixed here. No visual impact.

Copy link
Copy Markdown
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Lgtm! Looks great on testing!

@cdrini cdrini merged commit 15df07e into internetarchive:master Mar 27, 2026
4 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.

3 participants