-
Notifications
You must be signed in to change notification settings - Fork 841
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
[EuiCode] Fix bug where padding is not being contained in the inline box #5629
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_5629/ |
Hmm, so I totally noticed what you noticed as well, but I don't think The main reason being that with However, with Using I'm wondering if there's a better solution? |
Thanks, @cchaos totally forgot those use cases. Let me try a different solution. 🙌🏽 |
inline-block
@@ -5,6 +5,7 @@ | |||
@include euiCodeFont; | |||
font-size: .9em; /* 1 */ | |||
padding: .2em .5em; /* 1 */ | |||
line-height: 1.72; // Adds a line height so that the padding is contained in the inline box |
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.
@cchaos here's the new approach. I decided to use a unitless line-height
which preserves the font-size / line-height
ratio when the font size changes.
But I have to say that I found this 1.72
by trying different numbers until it looked good. Probably there is a formula based on the current font-size
and padding
that gives the perfect number. But I couldn't find it. 😬
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.
I'm still worried that the increase in line-height, especially when nested in a EuiText will offset that paragraph's line-height. I created a sandbox to test out some possible options and to compare the differences: https://codesandbox.io/s/stoic-kalam-vtprj?file=/demo.js
Preview documentation changes for this PR: https://eui.elastic.co/pr_5629/ |
Summary
The padding for the EuiCode component was not being taken into consideration because it uses a
<code />
element that is inline by default.This PR adds a line-height so that the padding is contained in the inline box.
Checklist
[ ] Checked in mobile[ ] Props have proper autodocs and playground toggles[ ] Added documentation[ ] Checked Code Sandbox works for any docs examples[ ] Added or updated jest and cypress tests[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes