FE-1207: add chip to ds - #9130
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview The component supports sizes, colors, variants ( Ladle stories cover variants, shapes, sizes, and affix interactions. Separately, disabled form error text color mixing in Reviewed by Cursor Bugbot for commit 94e5ed9. Bugbot is set up for automated code reviews on this repo. Configure here. |
853cecf to
79cd366
Compare
64d6807 to
94e5ed9
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 94e5ed9. Configure here.
lunelson
left a comment
There was a problem hiding this comment.
Looks good, my only nitpick is that I believe the remove/dismiss box for chips and tabs should always be on the left or rather at block-start, never the right (block-end). It's widely done on the right (maybe a bad convention inherited from microsoft windows) but once you need to remove several chips of different lengths in a row, in a flex-flow ltr layout, it's clear why this should be at block-start
I agree with you, but I also think it reads better/the intention of the chips are clearer when remove is at the end. Personally adding an X at the beginning would confuse me into thinking that the chip represents the item not being present in the list. Instead I think a better pattern is a) in combo boxes allowing the whole lozenge to be clicked to remove it (instead of just the button) and b) allowing users to remove items with backspace, which solve the issue but keep readability better. |

🌟 What is the purpose of this PR?
Adds a chip component to the DS
Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR: