-
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
[EuiText] Fix multiple missing CSS logical properties #6059
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_6059/ |
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.
LGTM! 🎉
How did we miss that? 😄
There's also one text-align: center
that can be converted to use ${logicalTextAlignCSS('center')};
.
eui/src/components/text/text.styles.ts
Line 193 in e6c461f
text-align: center; |
|
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.
LGTM after the snapshot update
it's also maybe useful to include to remind us to use more logical properties in the future
Yeah we have the util function so might as well use it. If it ever does change then we can update one place.
Preview documentation changes for this PR: https://eui.elastic.co/pr_6059/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_6059/ |
Summary
I missed converting a few of these properties in #5895 and #6049 🙈
Before
After
Checklist
direction: rtl
andwriting-mode: vertical-rl