Bartik: Site title lays over the menu block

Are you going to work on this @lucastockmann? Else I'm going to try to fix this.

Went ahead with this. Added 0.375em padding to the bottom of the region header. The same amount that is used as on the top. Did not see any regression of this change when smoke testing various pages.

It's working for me. I've updated Firefox and Chrome screenshots to show it.

Looks good to me.

Assigning to @emma.maria for final sign-off as this is a deign change in Bartik.

Apologies for stalling this issue.

So the change is wonderful for mobile, anything added to the header now has a space below it and the bottom of the section. However the patch in #5 does adds a height change to the header for tablet and desktop widths.

Screenshots.

Tablet

Before

After

Desktop

Before

After

Can a media query + style be added for tablet and up layouts to negate the margin bottom on the region-header please.

Added padding bottom to region header for only devices below 461px using media query.

Now the change only applies for small screens as @emma.maria said. I've tested it and the fix doesn't apply on bigger screens than 461px. Attaching screenshots:

Big

Tablet

Mobile

Thanks @ckrina for the review! RTBC++

Suggested commit message:

git commit -m 'Issue #2574797 by dschenk, zuhair_ak, ckrina, emma.maria, lucastockmann: Bartik: Site title lays over the menu block'

I could be wrong but I think the max-width should be 460 here, not 461. Otherwise at exactly 461px it's inconsistent (a bit too much spacing I think).

For example from bartik.breakpoints.yml:

bartik.narrow:
  label: narrow
  mediaQuery: 'all and (min-width: 560px) and (max-width: 850px)'
  weight: 1
  multipliers:
    - 1x
bartik.wide:
  label: wide
  mediaQuery: 'all and (min-width: 851px)'
  weight: 2
  multipliers:
    - 1x

From css/components/primary-menu.css:

  114: @media all and (min-width: 461px) and (max-width: 900px) {
  115    .region-primary-menu .menu {
  116      margin: 0 5px;
  ...
  161  }
  162  
  163: @media all and (min-width: 901px) {
  164    .region-primary-menu .block-menu .menu {
  165      font-size: 0.929em;

@Cottser, checking

@Cottser, you were right, there was unnecessary padding-bottom on 461px. It should be added on 460. Screens below after my patch

461px:

460px:

@kostyashupenko Thanks for investigating and creating a patch. I've unassigned you from the issue also to free it up for review :-)

I am at mentor sprints here at DrupalCon NOLA and testing this issue.

Confirmed issue before patch and the fix afterwards on a few different pages with lowercase g, q and y in Safari, Chrome, and FireFox. +1 RTBC

We are going to be testing this after lunch.

Confirm patch works in multiple browsers emulating mobile with lowercase g and y.

We are working on posting screenshots showing patch before & after for several browsers.

Attached Images show patch applied to D8 with patch in Firefox showing before and after application of the patch.

Attached Images show patch applied to D8 with patch in Chrome showing before and after application of the patch.

Bumped to normal because it can be a readability/accessibility issue.

Committed d276bb4 and pushed to 8.2.x at DrupalCon New Orleans. Thanks!

We tested this on a Galaxy S7 using Chrome, an iPhone 6S Plus using Safari, and two iPhone 6Ss using Safari and Chrome. The patch worked successfully on all browsers. Attached image is before and after patch on the iPhone 6S Plus and the after patch on the Galaxy S7.

Keithdoyle9, Kay_v, someone else at our table (Sorry I forgot your name!), and I were working on this before the live commit at DrupalCon NOLA 2016. I had issues getting the comment to submit before the live commit, but thought the comment was worth adding for the additional testing on mobile devices.