refactor: changed style to minimum support of RTL language

[asmanp2012]

#191
I tried to make a better style with minimum support for RTL language, but many things can't be handled by CSS and I couldn't fix all bugs in RTL language.

bellow components still have bugs:

1. breadcrumb
2. badge
3. tab-group(active-tab-indicator)
4. range
5. progress-bar(indeterminate)
6. rating
7. split-panel

However, by merging this pull request, many style bugs in the RTL language will be fixed.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
shoelace	✅ Ready (Inspect)	Visit Preview	May 28, 2022 at 2:18PM (UTC)

[seyedi]

Awesome, this will make the job much easier in RTL projects.

Thanks in advance
@asmanp2012
@claviska

[alimd]

It will be awesome 👌

[claviska]

Thanks so much for submitting this! I'm going to need a little more time than I have this morning to get it fully reviewed, but it's on my list.

bellow components still have bugs:

breadcrumb
badge
tab-group(active-tab-indicator)
range
progress-bar(indeterminate)
rating
split-panel

However, by merging this pull request, many style bugs in the RTL language will be fixed.

Would you mind elaborating on these ones? What's blocking them from being updated? Should we open up separate tickets to track each one separately?

Lastly, what advice do you have for someone who's only familiar with LTR to develop and test to make sure RTL is done (and remains implemented) properly?

Thanks again!

[asmanp2012]

@claviska
it's very simple I make a demo and put all components on that and we should check them one by one because the problem is functionality on the RTL, not just style. now I made this for myself.

Finally, I should say yes you should make separate tickets per component to fix the functionality there. but before that, we should choose a way to know the components in the RTL mode.

I prefer this will be, a property with the Rtl name that is reflected in dom. because developers should be choosing a way to know Rtl and it's not our business.

the developer put the rtl attribute for the component otherwise we show the component in the ltr mod and direction should be inside the style of the special components.

[claviska]

Finally, I should say yes you should make separate tickets per component to fix the functionality there. but before that, we should choose a way to know the components in the RTL mode.

Can we use the built-in global dir attribute instead? It works for all elements and reflects automatically, so we don't need to do anything special to support it.

Within CSS, this lets us use :dir(). The caveat is that, in JavaScript, we have to use Element.closest('[dir]') or something equivalent. I previously proposed a property called Element.currentDir to make this more efficient — give the issue a 👍 if you find it useful — but I still think the benefits of dir outweigh the cost. In fact, the logic can probably be rolled into a Reactive Controller and reused.

If that's agreeable, I can work that out. Now that this is merged, let's open individual issues to address any remaining RTL issues. This will help us track them more closely and it will give others an opportunity to submit PRs.

Two things I noticed when testing:

• <sl-range>: The tooltip when dragging isn't aligned properly with RTL
• <tag-group>: The indicator isn't aligned properly with RTL

I'll open separate issues for these shortly.

---

Edit: some button group styles had to be reverted, as they were breaking button groups and radio buttons. See 707aeb6 for details.

[claviska]

By the way, if anyone wants to submit a translation: https://github.com/shoelace-style/shoelace/tree/next/src/translations

I believe the only RTL language we currently have at the moment is Hebrew.

[MM25Zamanian]

By the way, if anyone wants to submit a translation: https://github.com/shoelace-style/shoelace/tree/next/src/translations

I believe the only RTL language we currently have at the moment is Hebrew.

@claviska I do this

[claviska]

I decided to treat dir the same way as lang and handle it in the @shoelace-style/localize library. This works pretty well and allows the code to be easily reused in any component.

WIth that, I was able to fix the tooltip's position in <sl-range> in f6d3f79. It's a matter of adding the localization controller and checking for the direction it provides. (See the code in the linked commit for details.)

Unless there are objections to this approach, we can use this approach for solving bugs in other components where logical properties aren't sufficient.

[asmanp2012]

@claviska
I asked @seyedi ( he has more experience in multi-lang web components ), and he suggests we use your way, I too agree with that.

[claviska]

I've updated Tab Group as well. 70c97e2

At this point, the mechanism for detecting RTL in a component has been established, so any outstanding bugs can be filed as new issues. This will allow anyone to track and contribute fixes.

Thanks again for everyone's help with this!

[claviska]

Regarding breadcrumbs, what needs to happen here? Do we need to swap the icons from > to <? I'm taking notes to fix the remaining components, but I'm not sure what else we need to do with this one.

[claviska]

OK, range, badge, progress bar, rating, and tab group have been updated. I believe I've fixed all the issues mentioned above except for three last items:

• split panel resizing is backwards in RTL
• tab group keyboard is backwards in RTL
• breadcrumb icons need to flip

You can try them out by swapping the dir on <html> or on a specific component in the next preview: https://next.shoelace.style/

I'm out of time for today, but I'll work on the last two issues very soon. If you notice anything else, please let me know!

[shaal]

@claviska I thought that for symbols used in breadcrumbs, flipping the icon horizontally with CSS could be a nice solution.

[asmanp2012]

Regarding breadcrumbs, what needs to happen here? Do we need to swap the icons from > to <? I'm taking notes to fix the remaining components, but I'm not sure what else we need to do with this one.

Yes, exactly the icon should be changed or rotated. I don't know which one is better. @claviska

[claviska]

Commit d0ff2fe fixes breadcrumb, tab group, and split panel, including keyboard controls and mouse resizing. Please take a look in next and let me know if you see any other problems.

I believe that's all the RTL bugs that have been identified. If you notice anything else, please post a new issue.

And one last very big THANK YOU for this PR!

[claviska]

I thought that for symbols used in breadcrumbs, flipping the icon horizontally with CSS could be a nice solution.

Yes, exactly the icon should be changed or rotated. I don't know which one is better.

This was a bit more complicated because I'm cloning the separator to improve DX. (Instead of the user having to place a custom separator on each breadcrumb item, they can place one in breadcrumb and the component clones it.)

I was reluctant to simply flip it using CSS. This technique works for simple icons such as chevrons and arrows, but it won't work for custom icons that aren't symmetric or have text or details that require a certain orientation. Instead, I'm using chevron-right for LTR and chevron-left for RTL for the default separators. If you provide a custom separator, it will be up to you to flip or replace it, which seems reasonable since you control it.