Child of #2359331: [Meta] Add missing RTL rules in core CSS files..

The automated script detected 19 issues in core/themes/seven/css/components/tabs.css
I have fixed those issues, and also some rule specifity issues.

I have also attached screenshots for the visible issues.

LTR - 800px



RTL - 800px - Before



RTL - 800px - After



LTR - 320px



RTL - 320px - Before



RTL - 320px - After

CommentFileSizeAuthor
missing-rtl-seven-tabs.patch5.76 KBherom
tabs.missing-rtl.txt2.83 KBherom
rtl-320px-fixed.png9.23 KBherom
rtl-320px-before-annotated.png28.49 KBherom
ltr-320px.png9.33 KBherom
rtl-800px-fixed.png12.31 KBherom
rtl-800px-before-annotated.png18.32 KBherom
ltr-800px.png12.41 KBherom
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

herom’s picture

herom CreditAttribution: herom commented
herom’s picture

herom CreditAttribution: herom commented
Parent issue: » #2359331: [Meta] Add missing RTL rules in core CSS files.
herom’s picture

herom CreditAttribution: herom commented
Issue tags: +D8MI
LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs review » Reviewed & tested by the community

Thank you for making these improvements.

I tested the tabs on RTL and I couldn't find any other issues.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented 
+++ b/core/themes/seven/css/components/tabs.css
@@ -130,7 +147,6 @@ li.tabs__tab a {
-  top: 11px;

This is a non RTL change - how come?

herom’s picture

herom CreditAttribution: herom commented

It is an RTL change. Here's the CSS with some extra context.

 [dir="rtl"] .is-collapse-enabled .tabs__trigger {
   border-right: 0;
   border-left: 1px solid #bfbfbf;
   border-radius: 4px 0 0 0;
   right: auto;
   left: 0;
-  top: 11px;
 }

I don't know why we were even overriding the "top" property in an RTL rule.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented

Doh - thanks @herom - but that leads me another question...

+++ b/core/themes/seven/css/components/tabs.css
@@ -110,14 +127,14 @@ li.tabs__tab a {
+  border-radius: 0 4px 0 0; /* LTR */

@@ -130,7 +147,6 @@ li.tabs__tab a {
   border-radius: 4px 0 0 0;

I don't understand how these rules are the LRT/RTL equivalents in .is-collapse-enabled .tabs__trigger

herom’s picture

herom CreditAttribution: herom commented

Yeah, I always have to look them up myself too.
Here is the order of the values in border-radius:
top-left, top-right, bottom-right, bottom-left
So, to make them RTL, we have to switch first value with the second, and third value with the fourth.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

@herom thanks for answering all my question - this looks good - nice work.

Committed c8d4095 and pushed to 8.0.x. Thanks!

  • alexpott committed c8d4095 on 8.0.x 
    Issue #2360069 by herom: Fixed Add missing RTL rules to Seven tabs.css.

Status: Fixed » Closed (fixed)

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