[EuiButtonDisplay] Fix missing text-align CSS #8057
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Note
This doesn't fundamentally affect any actual visuals, because flex CSS was already handling centering button content for us. But it's nice not to have buggy CSS-in-JS in our codebase 😅
This was missed during EuiButton's original Emotion conversion. Just for funsies, I'm going to dive into what was causing the bug:
Our
logical*Style
utilities output objects of CSS key/values, e.g.{ color: red }
Our
logical*CSS
utilities output strings of CSS, e.g.'color: red;'
Emotion CSS that uses the
css``
template literal can parse both.Regular template literals (
``
), like the one being used ineuiButtonBaseCSS
(to avoid generating an extra emotion label, presumably), cannot parse objects, only strings. So the previous code was generating this:Huge thank you/shout out to @tsullivan for logging and noticing this bug!
Changing our internal EUI code to use the string output instead of object output solves the issue. We now have
text-align: center
in our CSS again, where it was previously missing.Bonus: another approach I could have tried to resolve this was potentially using the
css``
template literal and renaming the util to_euiButtonBaseCSS
. Emotion generally seems to respect and not add extra classNames to underscore-prefixed utilities, although this is not always the case.QA
text-align: center
is now present on our buttons, where it is not on prodGeneral checklist
[ ] A changelog entry exists and is marked appropriately.I'm opting to skip a changelog for this since it's an internal-only component / tech debt and doesn't affect any consumers or end-users.