Fix the visual bugs in the Bartik footer.

I'll have a look at it.

Fixed the size and color. The alignment will have to be looked at at this more recent issue: https://www.drupal.org/node/1273052

Fixed the size and color. The alignment will have to be looked at at this more recent issue: https://www.drupal.org/node/1273052

Oops, my bad. It is not related to the linked issue. Here's a new patch.

Ok, sorry, I'm confused with all these issues :-). So, the alignment was actually "right" in beta8? As per: https://www.drupal.org/node/1273052#comment-9699005

If so, ignore the patch from #5, use #4.

Tested patch from #3 and everything looks good.

As far as I can tell, it now looks exactly as before.

On it.

So, here's the status from #3:

As you can see, it aligns to the left, but the padding from the main menu, the content and the footer, are not identical. How is the footer content supposed to be aligned anyway? Do we have some reference somewhere? Because from other issue queues, I get some contradicting info.

This is the status with patch #5:

It is aligned to the left, with no padding. And the footer is slightly wider than the content.

I had no idea reviewing visual bugs was this much of a hazzle.

Anyways, like I have been say, everything looks nice:

I've overlayed the font and placed the two side by side. As you can see they are pretty much identical. On the one site I had some content on the frontpage which makes stuff above the footer look a bit different, but the footer is exactly the same (beta 7 and the patch from #3)

Just without the visual overlays.

Patch #3:

Patch #5:

Sure, I'll take it. It's pretty clear now :-).

Like I said: there are many issues, with diverging screenshots with comments "Looks fine to me". So, I have no idea how it's actually supposed to look. Also, if we have a reference somewhere (design, PSD, JPG, whatever), that would be helpful.

The footer width bug was probably introduced when going for re-usable CSS classes, layout-container to be specific. The obvious fix would be to correct the footer layout-container's width:

/* Override container width. 30px narrower. */
.site-footer .layout-container {
  max-width: 830px;
}
@media all and (min-width: 851px) {
  .site-footer .layout-container {
    max-width: 1260px;
  }
}

However, this is fragile: if we decide to change the layout width, we need to remember to change it in 2 places.

A more robust, but also more verbose, solution would be to adapt the styling a little bit, by putting the top border on .region-footer-fifth instead of .site-footer__bottom. That way, we can add a 15px margin to the left and right of the region, and it will align nicely.

What do you think?

This fixes the width at all breakpoints, as well as the 4 other footer regions, which didn't align properly.

Screenshots!

Ouch. Ok, I'll have a look at it :-)

Actually, I think we should remove the margin on all .block .content elements in the header, not just the .header-region. We want all regions in the header to have as little effect on other elements as possible so as not to mess up the layout (contrary to the content area, which is a lot more lenient).

The header now looks normal (I think):

+++ b/core/themes/bartik/css/components/site-footer.css
@@ -20,6 +25,12 @@
+  .site-footer__top .region:nth-child(2n+1) {
+    padding-right: 10px;
+  }
+  .site-footer__top .region:nth-child(2n) {
+    padding-left: 10px;
+  }

What about rtl?

Ouch, there's a lot more wrong in the footer with RTL. I'll have a look at it.

How's this?

Never mind, I just see now that RTL doesn't line up properly in full width.

Fixed the RTL footer at full width. Unfortunately, we need some pretty specific rule to get this to work...

Refer to #31 for screenshots at other resolutions.

+++ b/core/themes/bartik/css/components/site-footer.css
@@ -27,6 +46,34 @@
+  /**
+   * We need to be very specific for the padding for region-footer-second and
+   * region-footer-third, in order to suppress the padding cancellation rule in
+   * @media all and (min-width: 560px) {
+   *   [dir="rtl"] .site-footer__top .region:nth-child(2n+1) {}
+   *   [dir="rtl"] .site-footer__top .region:nth-child(2n) {}
+   * }.
+   */
+  [dir="rtl"] .site-footer__top .region.region-footer-second,
+  [dir="rtl"] .site-footer__top .region.region-footer-third {
+    padding: 10px;

Instead of having to override the selector, can we use a min-width/max-width media query so it doesn't apply to widths over 851px

I'm working on this @DrupalCon LA

Olright, here it goes, I moved some code around and moved part of the styling into the specific range of 560-850.

Gonna leave the sprinting room soon, so I'll leave it by unassigning it :)

@sxnc Thanks for working on this issue! I found a few issues with manual testing:

1. +++ b/core/themes/bartik/css/components/site-footer.css
   @@ -5,11 +5,27 @@
   -  padding: 0 10px;
   +  padding: 0;
   

   .region has no padding, so this line has no effect.

2. +++ b/core/themes/bartik/css/components/site-footer.css
   @@ -5,11 +5,27 @@
   +/* Temporary fix to align the footer. */
   +.site-footer .layout-container {
   +  padding: 0 15px;
   +}
   

   This line breaks the alignment of the site-footer__top with site-footer__bottom. Perhaps this should be site-footer__bottom .layout-container?

3. +++ b/core/themes/bartik/css/layout.css
   @@ -10,6 +10,7 @@
   +  box-sizing: border-box;
   

   The box-sizing places the padding on the inside of the max-width. This results in footer blocks being places against the viewport for smaller viewports.

Thanks emma, this is looking very good!

1. +++ b/core/themes/bartik/css/components/site-footer.css
   @@ -5,11 +5,16 @@
   -  padding: 35px 15px 30px;
   +  padding: 35px 0 30px;
   ...
   +/* Temporary fix to align the footer. */
   +.site-footer .layout-container {
   +  padding: 0 15px;
    }
   

   Now that .layout-container has moved up, this change is no longer necessary.

2. +++ b/core/themes/bartik/css/components/site-footer.css
   @@ -5,11 +5,16 @@
    .site-footer .region {
      box-sizing: border-box;
   -  padding: 0 10px;
   +  padding: 0;
   

   .region has no padding, so this line does not do anything.

3. +++ b/core/themes/bartik/css/components/site-footer.css
   @@ -27,9 +48,21 @@
    @media all and (min-width: 851px) {
   ...
   +  .site-footer__top .region:first-child {
   +    padding-left: 0; /* LTR */
   +  }
   ...
   +  .site-footer__top .region:last-child {
   +    padding-right: 0; /* LTR */
   +  }
   

   The blocks on the side are now wider than the two in the middle on 1290px+ viewport. Let's try and keep these the same width when possible.

   Screenshot of different block widths:

   

4. +++ b/core/themes/bartik/css/components/site-footer.css
   @@ -27,9 +48,21 @@
      .site-footer__top .region {
   ...
   +    padding: 10px;
   

   The regions in the footer now have additional spacing at the top/bottom. The vertical padding is unnecessary as there is quite a bit of padding already.

5. +++ b/core/themes/bartik/templates/page.html.twig
   @@ -168,19 +168,21 @@
   +        {% if page.footer_fifth %}
   +            <div class="site-footer__bottom">
   +              {{ page.footer_fifth }}
   +            </div>
   +        {% endif %}
   

   Nitpick: the .site-footer_bottom element has 4 spaces indent instead of 2 :)

I manually tested this patch and I can confirm it looks great.

Excellent, thanks Emma! I did another manual test to confirm.

A screenshot for good measure:

Just wanted to say: such awesome collaboration in this last bunch of comments! Kudos to @emma.maria, @LewisNyman and especially @idebr. :)

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed b84088e and pushed to 8.0.x. Thanks!