RTL i18n/l10n support

[aaronjudd]

RTL (right to left) support in internationalization / localizations.

Original discussion/proposal from @danielgindi

Now I want to start working on RTL support...
Which means basically two things:
1. Adding an "rtl" value in the localizations
2. CSS adjustments, base on that rtl attribute

Now Reaction has themes, but I'm guessing a theme is generic and not language-specific, and so a theme must contain the support for rtl languages,

I can think of two ways to do this:
1. Additional CSS files that will load when there's an rtl language loaded
2. Conditionally adding an "rtl" class on the body or html tag, and then having CSS adjustments based on that class

What do you think?
Any other idea?

[aldeed]

I have no experience with RTL on web, but I know that tricky situations can arise with formatting. One simple thing we can do is add/remove rtl class from body element, allowing themes to provide overrides like this:

.rtl button {}

But it's likely there will also be cases where actual HTML elements will need to be reconfigured, which means template overrides. So a theme would also have to have the option to define some override templates that render different markup depending on rtl boolean from currently selected language.

Implementation ideas:

• Add rtl boolean in localization files.
• Define reactive method, ReactionCore.isRTL(), which reacts whenever locale changes.
• Use method to add/remove body class: class="{{#if ReactionCore.isRTL}}rtl{{/if}}"
• Theme packages can define CSS and override templates to handle the rest.

[danielgindi]

So you theoretically support option #2... Another thing is that when we use 3rd party UI libraries, or write UI code in JS, we need to make sure to have it support RTL. Many times it involves detecting "float:right" or "direction:rtl" on a parent element to determine flow direction. I have experience with RTL-enabled UI so I can step in on that

[aaronjudd]

@danielgindi @aldeed now that we have the pricing localization in as well, what do you think remains here? Do either of you want to take a whack at this?

[danielgindi]

Hey guys! The load on me is balanced soon, so I'm stepping in to take care of this. I'll be making sure that there's support for RTL from thr infrastructure, and that UI is fully RTLed too. I'll keep you posted :)

[aaronjudd]

@danielgindi sounds great. I think we've got most of the i18n and currency issues working fairly well. Looking forward to seeing what you come up with.

[danielgindi]

Yay! It will be ready in a few days :)

[danielgindi]

Well it takes a little more time than I thought, as there is a big theme to adjust, and as I got engaged in the past month so I have my hands full. We'll see Reaction in rtl soon!

[aaronjudd]

:+1: Looking forward to it. (and doesn't it always take more time than originally thought?)

[danielgindi]

https://github.com/reactioncommerce/reaction-core-theme/pull/10 https://github.com/reactioncommerce/reaction-core/pull/97

We're ready.

:-)

[aaronjudd]

Looking pretty darn good. However, I'm noticing one glitch. When changing from RTL to LTR not everything is updating for me (it does on refresh though).

Here's what it looks like after it switching back to LTR

I'm merging this into reactioncommerce/reaction-core#96. I'll take a look and see what's going on there.

[danielgindi]

Man that's weird! But after playing with it a little bit - it seems like a WebKit bug with the special columns css. It does not re-layout internally. We can use the standard hacks to trigger a WebKit relayout, file a bugreport, or do both :-)

[aaronjudd]

re: the pr comments, I can update the docs after everything look solid.

Was there a particular reason you used rtl.less as a separate import routine in the build configuration?

In any case, I've updated to use server/buildtools/module-definitions.js and added to the mixins.

Didn't see any difference in behavior (and I think that's how it should be)... but was wondering if you'd encountered some issue I'm not seeing. Still - it looks like it's the same functionally as before ( I just merged my changes into development, if you want to take a look.

[aaronjudd]

Out of curiosity, does the site load in Hebrew by default for you (like it probably should) or English? I am thinking I had tested that locale detection, but at the moment mine keeps defaulting to en-us so I'm not sure how I tested it before. I had some chrome extension I used, but deleted it and now can't find it..

[danielgindi]

Well I just thought it's natural to put it where the original bootstrap is, but the end result is the same!

[danielgindi]

I did have to specify Hebrew, but my Chrome is in English, and Chrome does not take the system locale for that...

[danielgindi]

Something you did in that last PR broke the language menu - it does not load for me

[aaronjudd]

did you do meteor reset? There's new data in the Shops.json that needs to load

[danielgindi]

Yes!

[aaronjudd]

@danielgindi does that mean it worked, or yes, like "duh, I did that" (and it still doesn't work)

[danielgindi]

Oh yes!!! ;-)

[danielgindi]

(Is that clearer?)

[aaronjudd]

Awesome.

[aaronjudd]

re: the refresh issue -> just occurred to me we probably need a Tracker.flush()

[danielgindi]

From my tests it looks like a bug in refreshing CSS, I need to look into it more to say with certainty

[danielgindi]

Yes it is a WebKit bug. Trying to add the class "rtl" to the document live in Chrome, then Ctrl+Z - keeps the bootstrap floats to the right.

These are actually not floats, but "-webkit-column", which I guess is still "-webkit" because it's not complete yet.

Trying to find a webkit-fix

[danielgindi]

OK it seems like Bootstrap is using a very un-finished CSS3 feature there. I wonder why. There are dozens of articles out there blaming the browsers for being buggy and inconsistent about that feature...

[danielgindi]

Update: Found the bug.
Nothing to have with the columns - but these are the main ones affected.
Chrome does not update "direction" after removing the "rtl" class. It still thinks that the HTML element has the direction: "rtl" when it actually supposed to take the default "ltr".
We can fix that by explicitly specifying "ltr" as the default

[danielgindi]

https://github.com/reactioncommerce/reaction-core-theme/pull/11

[aaronjudd]

well that works perfectly! love it when 15 characters of code answers one of life's big mysteries.

I'll update docs and close this issue when it's ready for the next release.

[danielgindi]

Yeah it works great! I'm very satisfied with the result- where everything comes out real clean, and themes can be RTLed with almost zero effort.

Looking forward for the next release :-)