Patch #6012

RTL layout

Added by Orgad Shaneh over 7 years ago. Updated over 7 years ago.

Status:ClosedStart date:2010-08-01
Priority:NormalDue date:
Assignee:Eric Davis% Done:

100%

Category:UI
Target version:1.0.1

Description

Hello,

Following #5972 I thought of a better way to implement RTL support.

Instead of using a theme, which affects all users (unless installing theme changer, see #5993), I moved the css file to the standard stylesheet directory, named rtl.css, and added a link tag if current language is Hebrew (suppose Arabic support will be added, it will need to be added to this patch as well).

Another downside that is handled with this method is the dependency between the language and the selected theme - the rtl theme uses 'alternate' as its base theme, so if the admin wants to use it, he must either use the alternate theme or manually edit the css file to include some other theme.

With this new method, this dependency is removed, and any theme should work with it.

rtl.css should be added to public/stylesheets, and the rtl theme may be removed

rtl-layout.patch Magnifier - This should be applied to app/views/layouts/base.rhtml (359 Bytes) Orgad Shaneh, 2010-08-01 14:00

rtl.css Magnifier - This is combined from application.css and context-menu.css from rtl theme (1.96 KB) Orgad Shaneh, 2010-08-01 14:00

rtl-layout.patch Magnifier - Use l(:direction) instead of hardcoding language (343 Bytes) Orgad Shaneh, 2010-08-01 14:29

he.yml Magnifier - Updated yml file that includes direction (42.4 KB) Orgad Shaneh, 2010-08-01 14:29

rtl.patch Magnifier - Adds RTL layout support and updates all locale files accordingly (14.9 KB) Ebrahim Mohammadi, 2010-08-08 00:38

rtl.css Magnifier (2.01 KB) Orgad Shaneh, 2010-08-08 07:56

he.yml Magnifier (42.4 KB) Orgad Shaneh, 2010-08-08 07:56

context_menu_rtl.patch Magnifier (2.27 KB) Azamat Hackimov, 2010-08-19 20:27

rtl-error.png (6.37 KB) Eric Davis, 2010-08-21 00:13


Related issues

Duplicated by Redmine - Patch #962: Hebrew translation and RTL Layout Closed 2008-03-30
Duplicated by Redmine - Patch #2318: RTL Theme Closed 2008-12-11

Associated revisions

Revision 4005
Added by Azamat Hackimov over 7 years ago

Real RTL-theme support in locales (#6012)
Now each locale-file have direction string (ltr - left-to-right - by default).

Revision 4014
Added by Eric Davis over 7 years ago

Add RTL support to the context menu. #6012

History

#1 Updated by Orgad Shaneh over 7 years ago

This approach is similar to Qt translation. It requires no changes to base.rhtml when adding a new RTL language. All the translator needs to do is define direction: RTL in the yml file.

#2 Updated by Ebrahim Mohammadi over 7 years ago

  • File rtl.patchMagnifier added
  • Assignee set to Holger Just

Find a patch against trunk in attachment that implements RTL support, and adds direction string to all locale files. I've used rtl.css of Orgad Shaneh.

#3 Updated by Holger Just over 7 years ago

  • Category set to Translations
  • Assignee changed from Holger Just to Azamat Hackimov

The patch looks good to me. Looks like a clean approach. I haven't tried it though.

Could you please check and confirm that it works with all distributed themes? Once that's done, I vote for inclusion / replacement of the RTL theme.

#4 Updated by Orgad Shaneh over 7 years ago

I wouldn't submit it without checking :)

I added another entry for <pre> in the css, and made a few changes in the translation file (can I have svn account for the translation, instead of posting it here with each change?)

#5 Updated by Holger Just over 7 years ago

Orgad,

  1. Please keep this ticket for the RTL changes only, for updated of the Hebrew translation, please use a new ticket.
  2. Please post patches instead of whole files.

The question for further checking was not meant to insult you. Sorry if that may have sounded so. I just want to make sure that the patches are of the known good quality. As I am unable to check it myself due to my lack of RTL language knowledge, I just ask the community :)

#6 Updated by Orgad Shaneh over 7 years ago

Regarding Mohammad's patch, the rtl stylesheet must be referenced after all other stylesheets (ses the difference between rtl-layout.patch and rtl.patch)

#7 Updated by Holger Just over 7 years ago

This is a rather difficult question (with larger boundaries as only this patch).
The question is: which parts of the stylesheets should be evaluated last (and thus has the greatest power to overwrite stuff without having to resort to !important.

In my opinion, the order should be:
  1. application style
  2. specific module and plugin styles
  3. themes

The RTL patch then should be inserted after the module styles.

Unfortunately the theme is currently loaded first which makes it very hard to overwrite default plugin styles. However, I think the RTL style provide defaults which should be easily overwritable by plugins. If this is a problem with current core styles, I think those should be changes instead. In this sense, I consider the rtl.css as a part of application.css which should be evaluated in the same context.

#8 Updated by Orgad Shaneh over 7 years ago

It should at least appear after context menu (which it overrides)

#9 Updated by Azamat Hackimov over 7 years ago

I commited RTL layout. For context menu I think better create own rtl css file, see patch for my idea (patch not good, need more work from ruby-guru).

#10 Updated by Eric Davis over 7 years ago

  • File rtl-error.png added
  • Status changed from New to 7
  • Assignee changed from Azamat Hackimov to Eric Davis
  • Target version set to 1.0.1

Azamat sent me a style bug for me to look at with the context menu before the 1.0.1 release. The context menu arrows are on the right instead of the left.

#11 Updated by Orgad Shaneh over 7 years ago

See note 6 (Is there a normal way to reference a note?)

#12 Updated by Eric Davis over 7 years ago

Holger Just and Orgad Shaneh:

I think it would be good to discuss the stylesheet load order in the forums. Based on Redmine's limited theme support, there are several edge cases that we would need to think about in order to support application, theme, rtl, and plugin styles.

#13 Updated by Eric Davis over 7 years ago

  • Category changed from Translations to UI
  • Status changed from 7 to Resolved
  • % Done changed from 0 to 100

Updated the context menu to support RTL using Azamat's patch. I also fixed the icon's css, the context menu added it's own styles that override the rtl.css. (r4014)

#14 Updated by Eric Davis over 7 years ago

  • Status changed from Resolved to Closed

Merged to 1.0-stable.

Also available in: Atom PDF