Closed (fixed)
Project:
Localization server
Version:
6.x-2.x-dev
Component:
Code
Priority:
Minor
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
23 Oct 2010 at 11:06 UTC
Updated:
17 May 2011 at 23:01 UTC
Jump to comment: Most recent file
Comments
Comment #1
gábor hojtsyOh. I was a vocal supporter of RTL support in Drupal 6, but that will not work without some additional work in the module and theme. Do you think we need theme modifications or just module changes? (I don't know what exactly needs to be RTL, so concrete code suggestions would do wonders, thanks - I can separate the required changes between module and theme in fact).
Comment #2
alaa commentedwell it needs to be in module because there are potentially 3 languages involved.
the interface language, the source language (English) and the target language.
the theme should take care of the interface language. but if you are browsing or translating Arabic translations the translations/suggestions should be displayed in RTL regardless of the UI's direction.
same if you are browsing in Arabic the original English strings should be displayed LTR.
Comment #3
gábor hojtsySo basically we need something like the following?
Comment #4
alaa commentedyes that should do it.
Comment #5
gábor hojtsyOk, let's move this to the right place then. Committing and deploying this change.
Comment #6
gábor hojtsyOh, in fact, looks like we need to actually improve on what we have :) See #798192: l10n_community rtl css files Committing the attached patch.
Comment #7
gábor hojtsyRolled out to localize.drupal.org, hope it helps.
Comment #8
alaa commentedtext areas for arabic are still LTR
Comment #9
good_man commentedI can't see it too, is it a cache issue?
Comment #10
gábor hojtsyCan you help by firebugging / providing a patch? We can continue with me doing hit and miss, but you see exactly what is there and what should be changed. Thanks!
Comment #11
good_man commentedI checked this patch twice in firebug.
Comment #12
good_man commentedThe patch in #11 is not valid, in fact I found all changes I made in #11 are already done in editor-rtl.css
Any reason why this file editor-rtl.css is not included in css files in l.d.o for RTL languages? I didn't find any rule from editor-rtl.css in firebug!
The included patch is just a claned version of css files (added white spaces after : ) and add direction: rtl to li.translation.
Comment #13
gábor hojtsyWell editor-rtl.css is supposedly included by Drupal when it detects an RTL language. Other RTL CSS files are properly added?
Comment #14
good_man commentedNo I can only see non rtl files editor.css and l10n_community.css (in cached format), how the language of l.d.o is setup? I think Drupal handles Arabic and other languages as well, as content in l.d.o not as language.
Comment #15
alaa commenteddrupal loads rtl css files when an rtl language is the active content or ui language right? but why would it do that automatically when translating to an rtl language in l.d.o ?
Comment #16
gábor hojtsyIn fact, some language teams set their "team language" to their language as well, which caused them to have some UI elements translated. While others are not. I think we should instead look into just adding the RTL files ourselves in l10n_server if an RTL language translation page is shown. Let's add code for that.
Comment #17
alaa commentedI assume you mean changing the content language by editing the group. I did that and it RTLs pages like user and role management but it does not RTL the translation page.
until this is fixed l.d.o is unusable with any string including variables, markup, filenames, urls, emails, domains or any other text that does not get translated/transliterated
Comment #18
gábor hojtsyUnderstood. I'm planning to look into this soon, cannot provide an ETA.
Comment #19
good_man commentedis it l10n_server issue or core?
Comment #20
gábor hojtsyI think this should be "hacked" in the l10n_server. Basically, the l10n_server translation pages should either add their RTL sheets themselves or should pretend the page language is RTL, so Drupal adds the RTL sheets.
Comment #21
good_man commentedThe previous patch (css organizing) + l10n_server and l10n_community handling of RTL languages.
Comment #22
gábor hojtsyLooks very good. Let me do a bit of testing on this before committing and deploying. Thanks!
Comment #23
gábor hojtsyAll right, committed and deployed. This is how it looks now. Not sure this is entirely what you intended. Ie. the filters should be reorganized as well?! Please reopen for followups if this is not 100% as you wanted it to look. Of course if you have bigger unrelated improvements, let's open new issues.
Update: had issues uploading image, here it is: https://skitch.com/gabor.hojtsy/rdca4/rtluserinterface
Comment #24
good_man commentedIt's looking good, thanks Gabor.
Comment #25
shadysamir commentedWhen translating a RTL language using a LTR interface there is no need to override the following CSS rules with the following values. It causes the filters form to go RTL when it doesnt need to:
Comment #26
shadysamir commentedI am attaching screenshots of what the form looks like now (RTL-filters) and what it should look like (LRT-filters) since UI language should be independent from the translation language I'm working on
Comment #27
shadysamir commentedLet me also add rules:
that should not be overridden by RTL in this case
Comment #28
good_man commentedYeah I think the filter widget shouldn't be RTL now as it's fully in English. Maybe if it's translated we'll go back to the RTL code in #25.
Comment #29
good_man commentedCan you post a patch for the changes you'd like to add?
Comment #30
shadysamir commentedIf I can find the rules to patch. The file
l10n_community/editor-rtl.css
is only found in branch DRUPAL-6--2 and it does not have those rules!
Comment #31
good_man commentedI think let's translate l10n_server and l10n_community modules, or just leave the filters RTL-ed instead of this hacky way to add-then-remove the rules after translating the strings. The current layout doesn't break the UI of l.d.o also.
Comment #32
shadysamir commentedIt doesnt (hence minor) but it's still a bug. I dont think we will ever put it back. As long as the UI is LTR these rules need to be removed.
Comment #33
good_man commentedActually I don't see it like a bug now, it's the way RTL is written for many projects in core and in contribe, unless there is a real bug with a broken UI elements, it won't be considered as a bug.
Removing these rules won't solve the problem (problem is not the right word here), translating this project will absolutely do, so I encourage you to begin translating it as an important step to make l.d.o better for RTL languages.
Comment #34
gábor hojtsyYeah, well, as I posted above: "Not sure this is entirely what you intended. Ie. the filters should be reorganized as well?! Please reopen for followups if this is not 100% as you wanted it to look.". I thought filters in RTL while they are English look odd. The username and search box is probably tricky, because you can either enter English or RTL text, depending on what you are looking up.
Anyway, I think we can easily solve this problem if we just separate the translation UI and the rest CSS files into two and only include RTL for the translation UI (not the filtering for example), if the interface is not RTL but the language being translated is RTL. You can help by separating the rules you'd not like to see so we can have the CSS in two. Thanks!
Comment #35
shadysamir commentedThe rules I mentioned are the ones overriding filters direction, and they should be separated. I cant find them in the CSS in cvs
Comment #36
shadysamir commentedThe rules I mentioned are the ones overriding filters direction, and they should be separated. I cant find them in the CSS in cvs
Comment #37
good_man commentedshadysamir: which tag you are checking out from cvs?
Gábor Hojtsy: I love this D7 translation layering style :)
Comment #38
shadysamir commentedDRUPAL-6--2 is the only one with editor-rtl.css
Comment #39
good_man commented@shadysamir: It's ported to 3.x as well, marking this as fixed now, only reopen if there is a real broken things in RTL UI.