Add missing RTL rules to Bartik theme CSS

#2375673: Split Bartik's CSS into SMACSS style components landed.

Fixed all in the file except below:

1. /var/www/html/drupal/8/core/themes/bartik/css/style.css
.tabs ul.primary li a

already has:

// core/themes/bartik/css/components/tabs.css:63
  .tabs ul.primary li a {
    float: left; /* not LTR */

Not sure what to do with this.

Good start, but found some typical mistakes. There are quite a few places where all of the LTR marking, the restoration of margins/paddings, etc. is correct, so its surprising that was not consistent.

1. +++ b/core/themes/bartik/color/preview.css
   @@ -17,8 +17,12 @@
   +  padding: 15px 15px 15px 10px; /* LTR */
   ...
   +  padding: 15px 10px 15px 15px;
   
   @@ -172,9 +196,12 @@
   -  padding: 0.8em 2px 0.8em 20px;
   +  padding: 0.8em 2px 0.8em 20px; /* LTR */
      text-indent: -15px;
    }
   +#preview-footer-columns .preview-content li a {
   +  padding: 0.8em 20px 0.8em 2px;
   +}
   
   +++ b/core/themes/bartik/css/components/admin.css
   @@ -45,5 +49,8 @@ div.admin-panel dd {
   -  margin: 0 0 14px 7px;
   +  margin: 0 0 14px 7px; /* LTR */
   +}
   +[dir="rtl"] div.admin-panel .description {
   +  margin: 0 7px 14px 0;
    }
   

   You can override just left and right instead of repeating the whole thing?

2. +++ b/core/themes/bartik/color/preview.css
   @@ -26,9 +30,12 @@
   -  margin-left: 15px;
   +  margin-left: 15px; /* LTR */
      padding-top: 34px;
    }
   +[dir="rtl"] #preview-site-name {
   +  margin-right: 15px;
   +}
   
   @@ -69,17 +76,27 @@
   -  margin-left: 15px;
   +  margin-left: 15px; /* LTR */
      width: 210px;
    }
   +[dir="rtl"] #preview-sidebar {
   +  margin-right: 15px;
   +}
   ...
   -  margin-left: 30px;
   +  margin-left: 30px; /* LTR */
      width: 26.5em;
    }
   +[dir="rtl"] #preview-content {
   +  margin-right: 30px;
   +}
   

   Need to zero out the left margin then, no?

3. +++ b/core/themes/bartik/color/preview.css
   @@ -158,8 +178,12 @@
   -  margin-left: 0;
   -  padding-left: 0;
   +  margin-left: 0; /* LTR */
   +  padding-left: 0; /* LTR */
   +}
   +[dir="rtl"] #preview-footer-columns .preview-content ul {
   +  margin-right: 0;
   +  padding-right: 0;
   

   Need to undo the left margin/padding zeroing no?

4. +++ b/core/themes/bartik/css/base/elements.css
   @@ -73,7 +73,7 @@ del {
   -  border-left: 1px solid #bbb;
   +  border-left: 1px solid #bbb; /* LTR */
   

   No RTL counterpart?

5. +++ b/core/themes/bartik/css/base/elements.css
   @@ -87,11 +87,12 @@ blockquote:before {
   -  margin-right: 0.2em;
   +  margin-right: 0.2em; /* LTR */
      vertical-align: -.4em;
    }
    [dir="rtl"] blockquote:before {
      content: "\201D";
   +  margin-left: 0.2em;
   

   Need to zero out the right margin, no?

6. +++ b/core/themes/bartik/css/components/dropbutton.component.css
   @@ -45,7 +45,7 @@
   -  margin-right: 0;
   +  margin-right: 0; /* LTR */
    }
    [dir="rtl"].js .dropbutton-multiple .dropbutton-widget .dropbutton-action a {
      margin-left: 0;
   

   Does out of view code here restores the right margin to where it should be?

7. +++ b/core/themes/bartik/css/components/form.css
   @@ -26,7 +26,10 @@ fieldset,
   +  margin-right: 10px; /* LTR */
   +}
   +[dir="rtl"] .filter-wrapper .form-item label {
   +  margin-left: 10px;
   

   Undo the right margin in RTL.

8. +++ b/core/themes/bartik/css/components/list.css
   @@ -33,12 +33,18 @@
    .pager__item--previous {
      padding: 10px 10px 10px 0;
    }
   +[dir="rtl"] .pager__item--previous {
   +  padding: 10px 0 10px 10px;
   +}
   

   Mark the original one LTR, only use left/right for override.

9. +++ b/core/themes/bartik/css/components/list.css
   @@ -33,12 +33,18 @@
    .pager__item--last,
    .pager__item--next {
   -  padding: 10px 0 10px 10px;
   +  padding: 10px 0 10px 10px; /* LTR */
   +}
   +[dir="rtl"] .pager__item--next {
   +  padding: 10px 10px 10px 0;
   

   Only use left/right for override. Override should also match to item--last

10. +++ b/core/themes/bartik/css/components/search-results.css
    @@ -1,9 +1,12 @@
     ol.search-results {
    -  padding-left: 0;
    +  padding-left: 0; /* LTR */
       list-style-position: inside;
     }
    +[dir="rtl"] ol.search-results {
    +  padding-right: 0;
    +}
    
    @@ -15,5 +18,8 @@ ol.search-results {
     .search-results .search-snippet-info {
    -  padding-left: 0;
    +  padding-left: 0; /* LTR */
    +}
    +[dir="rtl"] .search-results .search-snippet-info {
    +  padding-right: 0;
     }
    

    Restore left padding

11. +++ b/core/themes/bartik/css/components/skip-link.css
    @@ -2,13 +2,18 @@
    -  left: 50%;
    -  margin-left: -5.25em;
    +  left: 50%; /* LTR */
    +  margin-left: -5.25em; /* LTR */
       margin-top: 0;
       position: absolute !important;
       width: auto;
       z-index: 50;
     }
    +[dir="rtl"] .skip-link,
    +[dir="rtl"] .skip-link.visually-hidden.focusable {
    +  right: 50%;
    +  margin-right: -5.25em;
    +}
    

    left: 0 and restore left margin.

12. +++ b/core/themes/bartik/css/components/table.css
    @@ -62,5 +62,8 @@ table ul.links {
     table ul.links li {
    -  padding: 0 1em 0 0;
    +  padding: 0 1em 0 0; /* LTR */
    +}
    +[dir="rtl"] table ul.links li {
    +  padding: 0 0 0 1em;
    

    Only override left/right.

13. +++ b/core/themes/bartik/css/components/triptych.css
    @@ -28,7 +28,10 @@
    -  padding-left: 0;
    +  padding-left: 0; /* LTR */
    +}
    +[dir="rtl"] #triptych .block ol {
    +  padding-right: 0;
    

    Restore left padding.

14. +++ b/core/themes/bartik/css/components/triptych.css
    @@ -44,6 +47,9 @@
     #triptych-last #block-node-syndicate {
       text-align: right;
     }
    +[dir="rtl"] #triptych-last #block-node-syndicate {
    +  text-align: left;
    +}
    
    @@ -53,3 +59,7 @@
     #triptych-last #block-system-powered-by {
       text-align: right;
     }
    +
    +[dir="rtl"] #triptych-last #block-system-powered-by {
    +  text-align: left;
    +}
    

    Mark LTR.

15. +++ b/core/themes/bartik/css/components/vertical-tabs.component.css
    @@ -1,5 +1,8 @@
     ul.vertical-tabs-list {
    -  margin: -1px 0 -1px -15em;
    +  margin: -1px 0 -1px -15em; /* LTR */
       padding: 0;
     }
    +[dir="rtl"] ul.vertical-tabs-list {
    +  margin: -1px -15em -1px 0;
    +}
    

    Only override left/right.

16. +++ b/core/themes/bartik/css/components/views.css
    @@ -21,7 +21,10 @@
    -  padding-left: 0;
    +  padding-left: 0; /* LTR */
    +}
    +[dir="rtl"] .views-displays .region-content .secondary {
    +  padding-right: 0;
     }
    

    Restore left padding.

17. +++ b/core/themes/bartik/css/components/views.css
    @@ -109,3 +112,6 @@
       margin-right: 0;
       margin-top: 0;
     }
    +[dir="rtl"].views-ui-display-tab-actions .dropbutton input.form-submit {
    +  margin-left: 0;
    +}
    

    Mark LTR counterpart, restore right margin.

18. +++ b/core/themes/bartik/css/maintenance-page.css
    @@ -25,7 +25,10 @@ body.maintenance-page {
    -  padding: 0 0 0 10px;
    +  padding: 0 0 0 10px; /* LTR */
    +}
    +[dir="rtl"] .maintenance-page #content .section {
    +  padding: 0 10px 0 0;
    

    Only override left/right.

Just to let you know we have this META issue for Bartik #1342054: [META] Clean up templates and CSS.
It tries to keep all of the Bartik code cleanup/refactoring work together. Adding this issue to it. Thanks for all the hard work :)

taking a look...

Okay - in #5 almost all looks good and covers's Gábor's observations in #4. Only issue appears to be a missing 0 in the margins of comment inline links.

So, I've added that. Must be my smallest change to date!

Yay, let's get it in then :)

+++ b/core/themes/bartik/css/components/comments.css
@@ -14,40 +19,40 @@
-.comment .attribution {
+.comment__attribution {
...
-[dir="rtl"] .comment .attribution {
+[dir="rtl"] .comment__attribution {
...
-.comment .attribution img {
+.comment__attribution img {
...
-.comment .attribution .username {
+.comment__author {
...
-.comment .submitted p {
+.comment__submitted__data {
...
-.comment .submitted .comment-time {
+.comment__created {
...
-.comment .submitted .comment-permalink {
+.comment__permalink {
...
-.comment .content {
+.comment__content {

These changes are part of #2398461: Clean up the "comments" component in Bartik and should not be included in this patch

I'll revert those bits, then...

Right then - here's another attempt, having reverted the changes as suggested at #13.

+++ b/core/themes/bartik/css/components/comments.css
@@ -1,7 +1,12 @@
-/* ----------------- Comments ----------------- */
+/**
+ * @file
+ * Visual styles for comments in Bartik
+ */
...
-.comment h2.title {
-  margin-bottom: 1em;
+.comment {
+  margin-bottom: 20px;
+  display: table;
+  vertical-align: top;

@@ -47,7 +52,7 @@
-.comment .comment-arrow {
+.comment-arrow {

@@ -57,12 +62,12 @@
-[dir="rtl"] .comment .comment-arrow {
+[dir="rtl"] .comment-arrow {
...
-.comment .comment-text {
+.comment-text {

@@ -95,11 +100,10 @@
-.comment.unpublished .comment-text .comment-arrow {
+.comment.unpublished .comment-arrow {

Same goes for these, sorry :(. These changes are part of #2398461: Clean up the "comments" component in Bartik and should not be included in this patch. I checked the entire patch this time.

I'll update this with Rachel

@DevelCuy: the issue is still a perfect one to review on sprint weekend. :)

Removed SprintWeekend2015Queue by mistake.

Here's the patch again with the reverted comments.css changes as suggested in #16

Patch at #21 applies cleanly and appears to contain all mentioned at #16.

I can't see any missing RTL rules now and I don't *think* we are trying to change anything outside the scope of the issue.

Looks good to me!

Unfortunately a recent commit has broken the latest patch, re-roll please :)

This is why we are trying to have separate issues per component in Bartik as one small change committed for one file somewhere breaks it for everything in this patch.

As I see in the https://www.drupal.org/node/2064379 issue, the ckeditor-iframe.css has been just removed, so I have removed that part from the patch. Hope now it works:

@herom this issue doesn't break other patches, it's other patches in the Bartik queue continually breaking this issue.
We currently have one clean up issue per component which are collected in this issue #1342054: [META] Clean up templates and CSS. RTL fixes are part of these issues and we are massively overhauling the CSS as well. But if we can get this issue in right now than that's fine :)

Does not apply anymore :/

I ran some CSS regression screenshot tests on the areas that are being affected by the patch, just to check what is going on. We ran into some weirdness in a similar RTL issue here #2349517: Fix RTL for Bartik's vertical tabs so I just wanted to be visually sure and check out all the things that have been fixed too. Some nice fixes in there and nothing breaks worse than it already is in Core.

I have attached the huge report from BackstopJS showing all the screenshots at various widths.

Thanks all for the hard work. RTBC++

Also @WigglyKoala++ for your first Core contribution.

Theme CSS is not subject to beta evaluation. Committed dff5688 and pushed to 8.0.x. Thanks!

Yay, thanks!