Problem

Looking at the CSS in #2059719: Content translation admin CSS file not renamed from translation_entity.admin.css I realized there is no RTL support included for paddings. That sounds like a mistake.

Proposal

Add RTL styles.

Files: 
CommentFileSizeAuthor
#6 drupal-content-translation-admin-page-rtl-fix-2059719-6.patch1.28 KBthamas
PASSED: [[SimpleTest]]: [MySQL] 58,265 pass(es).
[ View ]
#3 content-lang-settings-rtl.png45.79 KBthamas
#3 content-lang-settings-rtl-fixed.png40.21 KBthamas
#3 drupal-content-translation-admin-page-rtl-fix-2059719-3.patch1.91 KBthamas
PASSED: [[SimpleTest]]: [MySQL] 58,792 pass(es).
[ View ]

Comments

Issue tags:+RTL

Assigned:Unassigned» thamas

Wokin' on it :)

Status:Active» Needs review
StatusFileSize
new1.91 KB
PASSED: [[SimpleTest]]: [MySQL] 58,792 pass(es).
[ View ]
new40.21 KB
new45.79 KB

Here is the patch which adds the missing padding and removes some unneeded lines. It (hopefully) also fixes some inconsistencines in drupal.base.css I found during working on the original issue.

Before:

content-lang-settings-rtl.png

After:

content-lang-settings-rtl-fixed.png

(Sorry for sending it so much later I assigned it to myself but DrupalCon catched me… ;))

Status:Needs review» Needs work

diff --git a/core/misc/drupal.base.css b/core/misc/drupal.base.css
index 2f081e9..77e55a2 100644
--- a/core/misc/drupal.base.css
+++ b/core/misc/drupal.base.css
@@ -54,20 +54,18 @@ table {
   border-collapse: collapse;

This file doesn't exists, so patch doesn't apply :(

Issue tags:+Needs reroll

Ohh, it was removed two days later: https://drupal.org/node/1839318#comment-7912709

I'll reroll the patch… Thanks for the review!

Issue tags:-Needs reroll
StatusFileSize
new1.28 KB
PASSED: [[SimpleTest]]: [MySQL] 58,265 pass(es).
[ View ]

Rerrolled without the removed drupal.base.css

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

Patch applies well and code seems right. Well done!

Thanks for the review! :)

Status:Reviewed & tested by the community» Needs review

Sorry if this is obvious, but:

+++ b/core/modules/content_translation/css/content_translation.admin.css
@@ -6,30 +6,31 @@
+  padding-left: 1em;
...
+  padding-left: 1em;

Any reason to add this additional padding? I don't see the equivalent being added in ltr-mode.

I think that would be the standard table cell padding that is overridden in the LTR version, so needs to set back again? @thamas, can you confirm?

Status:Needs review» Reviewed & tested by the community

We are overriding the 3em and 5em left padding which are defined earlier in LTR settings.

Issue tags:+sprint

Tag on sprint for easier tracking.

Title:Cotent translation module admin page does not support RTLContent translation module admin page does not support RTL

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Issue tags:-sprint

Yay! Thanks!

Maybe you noticed the random distracting labels right near the checkboxes on the screenshots in #3, those are *not supposed to be there*. Simple RTBC issue at #2112303: Random extra text around translatability configuration is confusing.

Automatically closed -- issue fixed for 2 weeks with no activity.