-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix: change inverse token chip #1309
base: master
Are you sure you want to change the base?
Conversation
Size stats
|
Accessibility report ℹ️ You can run this locally by executing |
Deploy preview for mistica-web ready! ✅ Preview Built with commit 531e929. |
Screenshot tests report ✔️ All passing |
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.
The PR is not aligned with the specs definition :(
The file that you linked in PR description is not correct.
https://www.figma.com/design/512a5Ke4EldL0GYffCXGCJ/%F0%9F%94%B8-Chips-Specs?node-id=0-1
This is the official specs for Chips and you can see with a yellow box what parts are pending to be implemented. Please, read the specs and make the changes according to the specs.
If you have any doubts, please address them before starting work to avoid unnecessary effort or wasted time.
Thanks for your effort with this PR anyways 🙏
OBVIVO-2618 (Chip): including the use of the inverse token (vars.colors.neutralLow) along with some validations to add it if inverse is true. There is also a small change in the text and icon on the chip to make it readable when using inverse.
https://www.figma.com/design/lvpSfY5tWbxfIXDcyOik78/Chips-Theme-Variant-Inverse?node-id=3518-17481&t=FrpMWuiBsl9w9Xks-0