-
Notifications
You must be signed in to change notification settings - Fork 860
[EuiBadge] Update styles to match latest design changes #9302
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: feat/eui-badge-updates
Are you sure you want to change the base?
[EuiBadge] Update styles to match latest design changes #9302
Conversation
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.
@ek-so Can you please check this?
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.
Imo, looks good (we'll need to decrease spaces between badges, but that's another story :))
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.
@ek-so Can you please check this?
I don't know if this return symbol can be left as-is right now, but I know we're planning to remove it altogether soon, so maybe that's okay?
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.
Yes, I'd say this is acceptable. And yes, I hope we can get rid of it there
…class name string
weronikaolejniczak
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.
✅ VRT updates are as expected. I ran them locally to double-check all tests pass.
✅ Code changes look as expected.
Thank you for adding the kitchen sink story 🙏🏻 I really like that pattern.
🟢 Good to go (especially because we're merging into a feature branch).
Co-authored-by: Tomasz Kajtoch <[email protected]>
| right: css` | ||
| &:not(:only-child) { | ||
| ${logicalCSS('margin-left', euiTheme.size.xs)} | ||
| ${logicalCSS('margin-left', euiTheme.size.xxs)} |
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.
Hey @tkajtoch, thank you so much for the changes 🙏🏻 Looks perfect, but I have only one question/neat: I know in design it is 8px padding from left/right to the icon. But since here in code the border is counted too, it looks just a liiiittle too much, and I think we could go for 6px from both sider. Would that change be possible?



💚 Build SucceededHistory
cc @tkajtoch |
💚 Build Succeeded
History
cc @tkajtoch |
Summary
Resolves #9299
This PR updates the styles of the
EuiBadgecomponent to apply the latest design changes - rounded corners, updated paddings, and spacing between text and icon.Please note that the VRTs highlight rather important changes that need approval from the design team:
Both of the above could be reverted to the old, less rounded design if necessary, but that would mean more code changes.
Why are we making this change?
Part of the EuiBadge design update epic.
Screenshots #
Impact to users
These style changes are global, and there's no way to opt out of them. The changes themselves are minimal and limited to just the presentation layer. No logic changes have been implemented.
QA
Remove or strikethrough items that do not apply to your PR.
General checklist