Comments

gábor hojtsy’s picture

Issue tags: +RTL
thamas’s picture

Assigned: Unassigned » thamas

Wokin' on it :)

thamas’s picture

Status: Active » Needs review
StatusFileSize
new1.91 KB
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… ;))

rteijeiro’s picture

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 :(

thamas’s picture

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!

thamas’s picture

Rerrolled without the removed drupal.base.css

thamas’s picture

Status: Needs work » Needs review
rteijeiro’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies well and code seems right. Well done!

thamas’s picture

Thanks for the review! :)

tstoeckler’s picture

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.

gábor hojtsy’s picture

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?

thamas’s picture

Status: Needs review » Reviewed & tested by the community

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

gábor hojtsy’s picture

Issue tags: +sprint

Tag on sprint for easier tracking.

plach’s picture

Title: Cotent translation module admin page does not support RTL » Content translation module admin page does not support RTL
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

gábor hojtsy’s picture

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.