Skip to content
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

RTL Support via Logical CSS Properties #191

Closed
MisterAnmar opened this issue Aug 29, 2020 · 19 comments
Closed

RTL Support via Logical CSS Properties #191

MisterAnmar opened this issue Aug 29, 2020 · 19 comments
Assignees
Labels
feature Feature requests. help wanted Ready for a contributor to tackle. i18n Anything related to internationalization.

Comments

@MisterAnmar
Copy link

Is your feature request related to a problem? Please describe.
Internationalization and the adaptation of UTF-8 became a necessity for Multilanguage apps.
a support for RTL in shoelace in its early stage in development will be great added advantage for developers.

Describe the solution you'd like
is it possible to put it under consideration while the project still in its alpha. As RTL support is lacked from many frameworks including Bootstrap.

Describe alternatives you've considered
using custom css on top of shoelace.
please consider this site https://rtlcss.com/playground/

@MisterAnmar MisterAnmar added the feature Feature requests. label Aug 29, 2020
@claviska
Copy link
Member

This is something I'd love to get right, but I have zero experience with RTL languages. If someone wants to provide guidance, I'd love to make a reasonable effort to accommodate.

@claviska claviska added the help wanted Ready for a contributor to tackle. label Aug 30, 2020
@MisterAnmar
Copy link
Author

I'm not that good with shoelace yet. I have not dived into its core yet. But I'm really hoping for this project to grow. Its neet.
I can assist in RTL SUPPORT.

@MisterAnmar
Copy link
Author

I managed to analyse the code your code is so elegant. It should not be difficult to have an RTL version. However. I never programed with ts.
I think a simple ts function is required to switch between the code RTL scss. Maybe a similar to the theme switcher style,
I will start working on it and show not take long and hopefully you will have the time to code the function for it.
I'm willing to be part of this project if its alright. And I will be the contributer for the RTL support if it alright.

Please let me know what's the procedure...

@claviska
Copy link
Member

It might be good to start off explaining what needs to be done, before jumping into any code.

For example, how is RTL configured? There’s a dir attribute in HTML — do we use that?

Looks like there’s also a :dir selector in CSS. Where would this come into play?

I’d prefer to keep it simple and ensure we’re using native APIs as much as possible rather than introduce our own.

@MisterAnmar
Copy link
Author

MisterAnmar commented Aug 30, 2020

Well, simply there are few steps involved

  1. html doc

<html lang="en">

Will change to

  <html lang="xx" dir="rtl">

This part will change the whole doc onto rtl including flex, grid, text direction,

  1. In the css its not necessary but good practice

html{ 
direction: rtl;
}


  1. Parts of the css files need to change
    Example.
    Margin-left: need to change to margin-right
    You have it in the code in approximately 9 places
    And few other minor things

Generally the issue with rtl is when the coder code specific alignments such as text align left and right
Or over using padding left or right
When I analyzed the code. Most of the code is symmetrical
You hardly used margin left or right. Or padding left or right which makes it pretty easy to achieve the RTL support with few lines of code.

I'm not sure yet. But I think a simple css file to overwrite some of the original css. Again I have not coded in ts before so I'm not sure...

@MisterAnmar
Copy link
Author

MisterAnmar commented Aug 30, 2020

I will show you a sample from your code.

Alert


     .alert__icon {
     ::slotted(*) {
    /*     margin-left: var(--sl-spacing-large); */
     margin-right: var(--sl-spacing-large);
    }
   }



as above this is the only line needs to be modified from the whole component.

so again im not familiar with TS but my humble suggestion would be it could a simple switcher
if(ltr){
'margin-left: var(--sl-spacing-large); '
}else{
'margin-right: var(--sl-spacing-large); '
}

and i could provide you with the list of things that need to be modified for you to analyze, hopefully will benefits you to come up with modular solution. I think its easy for you. God Bless.

the html opening html lang="xx" dir="rtl" handles the conversion of almost all the html and css except the rules that are set explicitly by the developer like the example above.

however, keep in mind this is not necessary everywhere like in tooltip component does not need to be modified.

@staabm
Copy link
Contributor

staabm commented Aug 30, 2020

In modern css you could use margin-inline-start which supports rtl natively

https://css-tricks.com/building-multi-directional-layouts/
https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start

@MisterAnmar
Copy link
Author

MisterAnmar commented Aug 31, 2020

In modern css you could use margin-inline-start which supports rtl natively

https://css-tricks.com/building-multi-directional-layouts/
https://developer.mozilla.org/en-US/docs/Web/CSS/margin-inline-start

Edited: Added screen shots

interesting point @staabm . thanks.
inline style is not a good practice from my experience for who use a lot of dynamic generated data (serverside DB), added inline properties will not change the flex box layout, basically the whole layout design will maintain its direction it will work if you are designing a symmetrical interface and even then you will still have issues with aligning navigation ..etc, also its bit tricky with browsers due to support draft , Also RTL support is not just about content its about design as well, the design will flip to the RTL. see RTL see LTR

but based on the same article logical properties we could use the supported way till the logical properties are fully supported for example

to change the alert component to RTL i added this code


    html[dir="rtl"] sl-alert::part(icon) {
        margin-left: 0;
        margin-right: var(--sl-spacing-large);
    }

sample-screen

so when the user change his language from the doc type the browser will automatically use this properties.
With the above simple code the whole ALERT COMPONENT is fully supported for RTL

so there could be one of these as a solution

  1. Adding a small css file to be included with the framework for RTL support (until Logical Properties fully supported)
  2. can be build with the TS code as a function or something

for option 1 it can be maintained by external developers and point users to it and maintained separately and till the new CSS features are implemented logical properties . However, there will be times where the extension RTL is always late, that's the case with Bootstrap RTL. and the core developers need to keep in mind when designing new components try not to over use margin-left and right, padding-left and right, and text-align:left and right (the current use of right and left is minimum in the current code which makes it so easy to implement RTL support). the rest will be pretty easy to implement.

for option 2 the developers who are helping with the Shoelace core development have to work extra to make sure it works for both directions which presents a possibility of higher chances of bugs (many of the developers never had long experience with RTL).

My humble suggestion would be
lets try and build option 1 and tested for a while as the framework grow then the framework developers would see if it is possible of integrating it into the core or just keep it as simple external css file also see the outcome of the new css drafted feature logical-properties. Please comment on this?

@staabm
Copy link
Contributor

staabm commented Aug 31, 2020

inline style is not a good practice from my experience for who use a lot of dynamic generated data (serverside DB), added inline properties will not change the flex box layout, basically the whole layout design will maintain its direction it will work if you are designing a symmetrical interface and even then you will still have issues with aligning navigation ..etc, also its bit tricky with browsers due to support draft

I never said someone should use inline styles. the property mentioned can be defined like any other css property.
browser support ist pretty good so far: https://caniuse.com/#search=css-logical-props

I don't have opinions on this one - just wanted to share the modern way to solve this problem - which I learned about a few weeks ago.

feel free to solve it in the way you like most

@MisterAnmar
Copy link
Author

inline style is not a good practice from my experience for who use a lot of dynamic generated data (serverside DB), added inline properties will not change the flex box layout, basically the whole layout design will maintain its direction it will work if you are designing a symmetrical interface and even then you will still have issues with aligning navigation ..etc, also its bit tricky with browsers due to support draft

I never said someone should use inline styles. the property mentioned can be defined like any other css property.
browser support ist pretty good so far: https://caniuse.com/#search=css-logical-props

My apologies i miss understood you.

I don't have opinions on this one - just wanted to share the modern way to solve this problem - which I learned about a few weeks ago.

your links led me to find a possible solution. thank you @staabm

@claviska claviska added this to the v2.0.0 release milestone Apr 6, 2021
@claviska claviska mentioned this issue Apr 16, 2021
@claviska claviska added the i18n Anything related to internationalization. label Aug 24, 2021
@claviska claviska pinned this issue Dec 7, 2021
@MisterEdel
Copy link

MisterEdel commented Dec 14, 2021

@edit: Sorry I haven't seen the reply from @staabm

Hi I have some insights, because I'm working on a Design System at my workplace.

We changed from margin-right and margin-left to margin-inline-start and margin-inline-end so we don't need change margin in that regard.

Same for margin-top and margin-bottom with margin-block.

So we're relying on the browser to make the decision for us.

@shaal
Copy link
Contributor

shaal commented Apr 1, 2022

Should we change the issue title, to support css logical properties?

I have a lot of experience with RTL, but fixing it with dir + margin-left/right is the old way. Now that all modern browsers support CSS logical properties it's the way to go, and would support any language / flow direction.

https://elad.medium.com/new-css-logical-properties-bc6945311ce7

@claviska claviska changed the title RTL Support RTL Support via Logical CSS Properties Apr 1, 2022
@claviska
Copy link
Member

claviska commented Apr 1, 2022

I think logical properties are the right solution, but most people will be looking for RTL so I've update the title to reflect both. Any help to accomplish this would be greatly appreciated. My current focus is on the new website, #413, and #520.

As a non-RTL user, how can I test for proper RTL implementations? Any suggestions here?

@shaal
Copy link
Contributor

shaal commented Apr 2, 2022

@claviska To test RTL (even with English content), this seems simple and useful - Add dir="rtl" to the html tag
https://www.w3.org/International/questions/qa-html-dir#rtlsetup

@oliversalzburg
Copy link
Contributor

I also have a small Shoelace application that is available in Hebrew and is suffering from RTL layout issues. It could happily serve as a proofing ground for any RTL changes being tested :D

@shaal
Copy link
Contributor

shaal commented May 18, 2022

Related/unrelated, @claviska is Doxicity going to support translations?
If we added Hebrew translation to Shoelace documentation, those pages will have <html dir="rtl" lang="he"> which will make it very easy to find and fix all RTL issues.

@claviska
Copy link
Member

claviska commented May 18, 2022

Yes, localization is on my list for Doxicity. There’s not much to do to get it there, just need some time to tackle it. Probably through a config option.

@claviska claviska mentioned this issue May 25, 2022
@asmanp2012
Copy link
Contributor

@claviska
Hi, I am ready to help, my mother tongue is Persian and RTL. I can help you please give me more advice to start that.

@claviska
Copy link
Member

claviska commented Jun 1, 2022

The CSS side of things has mostly been implemented in #768, so I'm closing this to focus on any remaining issues (which should be filed as new bugs). See also #773 which could use some feedback.

@claviska claviska closed this as completed Jun 1, 2022
@claviska claviska unpinned this issue Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature requests. help wanted Ready for a contributor to tackle. i18n Anything related to internationalization.
Projects
None yet
Development

No branches or pull requests

7 participants