-
Notifications
You must be signed in to change notification settings - Fork 838
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
[EuiSearchBar] Clear button hides RTL text #3960
Comments
We'll likely need to address RTL concerns at a higher level than this case. Even if we want to support/fix RTL issues case-by-case (i.e. when they are reported), we need an official way to know when to apply relevant changes. A quick search did not find a way to programatically detect the applied direction for a given element - we'd likely need to walk the parent tree up until a Another RTL-support question is if those languages should affect other presentation order such as table/grid columns. |
@chandlerprall thanks, yes, deciding the overall strategy would be needed. As for computing -- you don't need to walk the tree -- Lastly, the order of the columns is already automatically reversed as soon as you set the |
For columns, there are places - like datagrid - where we programmatically handle the column display order. |
I just tried adding P.S. I think the text would look correct if it was written in an RTL language. The actual strings are in LTR, and that's why I think we are viewing the trailing part. Right-To-LeftOriginal |
Didn't expect you to go test it right away 😅 (but thank you, it's good validation!) - the virtualization work for rendering data grid is going to more explicitly set the column order. Currently, your RTL direction is modifying how the browser interprets flexbox ordering. Virtualization techniques rely on the app generating absolute positioning coordinates, which won't natively be aware of the direction. It's possible the virtualization librar(y|ies) apply this understanding, however. We'll find out soon enough! |
When showing a button in RTL, margin-left/right shows up incorrectly. The needed CSS seems to be `margin-inline-start/end`. Partially relates to #3960 Those values do not exist in IE. I tested these changes with a search bar's filters and `EuiHeaderLink` with icons. english | current RTL | desired RTL ---|---|--- ![image](https://user-images.githubusercontent.com/1641515/91629484-7629b600-e997-11ea-8a29-85a7871443d4.png) | ![image](https://user-images.githubusercontent.com/1641515/91629493-95c0de80-e997-11ea-92ed-68c6af62c4c3.png) | ![image](https://user-images.githubusercontent.com/1641515/91629489-8b9ee000-e997-11ea-87c7-367749d876e2.png) Co-authored-by: Caroline Horn <[email protected]>
I just discovered a very well documented plugin that does CSS post-processing -- essentially converting logical back into physical properties but with proper fallbacks, etc.: postcss-logical Example: .banner {
color: #222222;
inset: logical 0 5px 10px;
padding-inline: 20px 40px;
resize: block;
transition: color 200ms;
}
/* becomes */
.banner:dir(ltr) {
padding-left: 20px; padding-right: 40px;
}
.banner:dir(rtl) {
padding-right: 20px; padding-left: 40px;
}
.banner {
resize: vertical;
transition: color 200ms;
} |
As mentioned in Slack, the plugin sounds interesting but it would be better if we could find one that does the opposite so that consumers can decide if they need to support RTL languages. The main reason being that this will likely double our already bloated compiled CSS in the dist. Increasing the load for most consumers, since most won't be translating. Even Elastic products, at the moment, do not support RTL display. It would also cause a huge update that means vigorous testing of all downstream effects that the EUI team doesn't have capacity to support at the moment including any subsequent windfall from consumer usage. Dear @nyurik, you've definitely enlightened us on this topic and even introduced us to these "new" properties. Had we known 2 years ago, we most likely would have built around this concept. But retro-fitting now with all of EUI's usages is scary. My ideal scenario:
We can certainly update single components here are there as you come across ones that egregiously break in RTL languages. Individual components are easier than trying to do a blanket update. |
As part of our Emotion conversion (#5685), we now have several logical property helpers (https://github.com/elastic/eui/blob/main/src/global_styling/functions/logicals.ts) that will automatically fix this issue once our form components are converted to Emotion. I'm closing this issue for now as it's not a bug, it's a feature/functionality request that we currently do not support but will support OOTB as part of our Emotion migration. |
When
<EuiSearchBar>
is in a RTL direction, the text is hidden behind the clear button. Also, both search and clear icons stay in the same location as for LTR, but should be reversed.Example -- open sandbox and start typing in the search bar -- observe that the clear button appeared on top of the text.
The text was updated successfully, but these errors were encountered: