Olivero: Elements within main content that have a high z-index will appear in front of mobile nav

Did a little looking at this. This is being caused by Flexslider setting a z-index: 2 inline-style within the content. This can be fixed by doing something like

[data-thumb-alt] {
  z-index: auto !important;
}

However IMHO Olivero should be able to support this. If we set z-index: 3 on the .site-header it also fixes the problem. We need to be careful about changing around z-indexes though, because they can be tricky to debug and find problems.

Can't work on this right now, but I think this is something that we can fix within the theme.

Also, thanks for the bug report @Tim Robinson!

yep, @mherchel you are absolutely right.
Also this bug happens also on desktop device if Enable mobile menu at all widths option enabled for Olivero theme. Just add:

  position: relative;
  z-index: 2;

to the any element from main section and try to expand burger menu. So i decided to add z-index directly to .site-header selector. From what i see there was no any side-affects after this change.

Also this bug happens to the .overlay selector, which doesn't have z-index at all (so overlay lives "under" main content if it has z-index defined). As i see from js we use this selector only in terms of navigation, so i have added z-index to the overlay aswell.

This task needs review now in all browsers defined here: https://www.drupal.org/project/drupal/issues/3173877

How to quickly test things:
1. You have to open locally this file https://git.drupalcode.org/project/drupal/-/blob/9.2.x/core/themes/olive... and change lines like this:

    <main role="main">
      <div style="background-color: #98faff; position: relative; z-index: 2;">Custom text with z-index: 2; tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata</div>
      {{ page.content_above }}
      {{ page.content }}
    </main>

^ this is a custom text with highlighted background and long text, with z-index hardcoded -> it needs just to test header (long text here required to be able to test theme with Enable mobile menu at all widths option enabled on desktop).

So after this change you have to clear drupal cache and then you will see your custom text on homepage.

If it's complicated for you to find this twig file and modify, you can go to /node/add/article page, fill the Title field and after for Body field change Text format to Full HTML, then click on Source button in CKEditor and put this content inside:

    <div style="background-color: #98faff; position: relative; z-index: 2;">Custom text with z-index: 2; tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata tratata</div>

Click save and then you can test this task on your newly created node page.

2. Go through the all browsers, test on mobile/desktop and report here if bug is still exist. No need to report thousands screenshots here from all browsers. Just provide 1 or 2 screenshots here if bug exist somewhere still + explanations where problem still exist.

P.S.: you can enable Enable mobile menu at all widths option on this page /admin/appearance/settings/olivero on the bottom.

3. You probably have to pay a lot of attention on header and its animating effects on expanding/collapsing menu. if you will find some "ugly" or/and unexpected/weird behaviors even if no problem anymore with original issue related to z-index - please report it here aswell.

Tagging

Tagging back to `Needs review`, since anyway at least one or two screenshots (or video/s) expected to see if original issue has gone.

Thanks for screenshots

I discussed this with @lauriii earlier today, and we brainstormed a fix for this.

The fix in #4 works, but it conflicts with the z-index of jquery joyride (the tour module), in addition, any elements with z-indexes above 101 within the main content will appear above the nav.

A better way would be to create stacking contexts on each of the main wrapping elements: .site-header, . layout-main-wrapper , and .site-footer. If we set the .site-header's z-index to 10, and the others to 1, it will always ensure that the header appears above the other wrappers (even if they contain elements that have higher z-indexes). We'll also need to move the overlay element into the .site-header wrapper, and if we do that, we should probably change it's name.

For anyone following along with this, z-indexes are more confusing than they appear. A good article on this is https://www.lullabot.com/articles/understanding-debugging-stacking-contexts

New patch incoming shortly.

Patch attached.

Tugboat link: https://3250357-13-z-index-fuoovipqv4lkycp8mbvjgo7cgpqgnkuf.tugboat.qa/

Adding testing steps

This looks pretty good - my main concern was how this would impact dialogs & overlays, but that seems to be handled well (see attached screeshots)

Oh, found an edge case. Having the offcanvas-top dialog open with the mobile menu has some issues.

Looks like this is a pre-existing issue. You can test this out on the latest 9.4.x at https://tugboat-aqrmztryfqsezpvnghut1cszck2wwasr.tugboat.qa/dialog

Opened followup #3251407: Olivero should use Drupal.displace() to place the mobile menu

in that case, moving patch 12 to RTBC per comment 15

in that case, moving patch 12 to RTBC per comment 15

This still conflicts with Tour and Quickedit. For example, Quickedit is still rendered on top of the mobile menu. I think we should change the tour z-index so that there's space between Quickedit and Tour for elements that need to render above Quickedit, but below Tour.

New patch attached for 9.4.x

A couple notes:

• Quickedit has a z-index of 100 and appears at the end of the DOM tree. This means that the header must have a z-index higher. It was changed to 101.
• The contextual link in the site branding needs to be covered up by the navigation. This element is within the DOM structure of the header and has a z-index of 500. This necessitates taking the z-index of the .header-nav to 501.
• Because we increase the z-index of the header-nav, we must now increase the z-index of the mobile navigation button (.mobile-nav-button ) to 505.
• The tour module has two elements at the end of the DOM:
  • .sheperd-element is set to 100 (this is the modal container).
  • .sheperd-modal-overlay-container is set to 100 (this is the overlay).
• I had to separate these and move them slightly so that they'd appear above the navigation.
• .sheperd-element is now at 110
• .sheperd-modal-overlay-container is now at 105

Tugboat URL for 9.4.x: https://3250357-z-index-21-2-avwcbvuimsn13sxldl3daue72nvsr0oh.tugboat.qa/

Patch for 10.0.x attached.

I tested and verified patch #21, Fixes the issue.
Added before patch screenshot and after patch screen recording. Tested with tour module and menu in mobile device.

Can be move to RTBC

Thanks for the review! Setting to RTBC per the comment in #24

Can we get a change record for changing the Tour z-indices? We should also update https://www.drupal.org/node/2823900

Change record: https://www.drupal.org/node/3258948

Updated docs https://www.drupal.org/docs/theming-drupal/z-indexes-in-drupal-8#comment...

Committed baa8211 and pushed to 10.0.x. Also committed the 9.4.x patch to 9.4.x. Thanks!