Fix overflow visibility for wrapper content in navigation CSS

The Chromium issue above is slightly different. It deals with setting display: contents directly on the <button>, which is not what we're doing.

From what I see, we're only applying it in one place on .admin-toolbar__scroll-wrapper

I'll investigate a bit more.

Opened an MR that wholesale removes it, not sure the consequences

@mherchel, you're right. I took https://cdpn.io/aardrian/debug/NWqbggX and but the button inside of a div with display: contents. Maybe it's a bug in Chromedriver itself?

I found https://github.com/SeleniumHQ/selenium/issues/6977 and their fix https://github.com/SeleniumHQ/selenium/commit/bbdf7c28a14645c3f5c53f2492...

      if (goog.string.startsWith(containerDisplay, 'inline') ||
          (containerDisplay == 'contents')) {

I don't exactly know the way forward, but it shows the fix is not removing display: contents but investigating the test issue

I've also found https://github.com/angular/protractor/issues/4951. There is no solution but some details to the problem.

Finally getting a chance to come back to this, and I was able to chisel it down quite a bit, and I've found that it's a combination of two style declarations: display: contents with overflow: hidden.

We're in that situation with

  overflow-x: hidden;
  overflow-y: auto;

This failed

.admin-toolbar__scroll-wrapper {
  display: flex;
  /*overflow-x: hidden;*/
  overflow-y: auto;

@media (min-width: 64rem) {
  .admin-toolbar__scroll-wrapper {
    display: contents;
    background: none;

This also failed

.admin-toolbar__scroll-wrapper {
  display: flex;
  overflow-x: hidden;
  /*overflow-y: auto;*/

@media (min-width: 64rem) {
  .admin-toolbar__scroll-wrapper {
    display: contents;
    background: none;

This passed.

.admin-toolbar__scroll-wrapper {
  display: flex;
  /*overflow-x: hidden;*/
  /*overflow-y: auto;*/

@media (min-width: 64rem) {
  .admin-toolbar__scroll-wrapper {
    display: contents;
    background: none;

So it is not display: contents;. Only in combination with overflow

Marking for review, I have no idea the impact of the overflow. But from I've read, it isn't needed.

Removing overflow-y: auto; is having impact on design in mobile devices. See screenshot for reference.

Found another report of this combo failing tests with Cypress: https://github.com/cypress-io/cypress/issues/25199

@Gauravvvv thanks for adding a fix! I'll copy it to a test I have to verify it makes WebDriver happy

I've tested. seems all fine.

@finnsky did you re-run the linked Nightwatch test? Or manually test.

Nope. I just checked for regressions in tugboat instance

Okay, going to move to NW because this all kicked off since it's not accessible when testing with WebDriver and that needs to be verified. I'll be at a computer soon

🙌 the Nightwatch test passed locally for me with these changes. I copied them over to ✨ Evaluate adding Nightwatch tests Active , waiting to double verify with pipeline https://git.drupalcode.org/issue/drupal-3393400/-/pipelines/169765

@mglaman should this be Needs work per #19?

The test passed with this added before the a11y test was added there. It's a horrible chicken and egg problem of "What do we fix where?" We could fix this with the tests, but it seems like it should be its own fix. As far as I can tell, this issue fixes the visibility bug and can be RTBC'd and unblock testing the new Navigation module.

Sounds good.

updating title

nod_ → closed merge request !8008

Committed and pushed cdc8135c56 to 11.x and 87ae449bf4 to 11.0.x and 8ac8832201 to 10.4.x and 2fffa8f31d to 10.3.x. Thanks!

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