paper-dialog appears behind backdrop when inside a <app-header-layout>

simonfuhrmann commented 7 years ago

Description

When opening a <paper-dialog> that is nested inside <app-header-layout>, its backdrop (e.g., when using attributes with-backdrop or modal) appears on top of the dialog, rendering the dialog inoperable.

The z-index of the dialog and backdrop element are in the correct order.

Defining all dialogs outside of <app-header-layout> is generally not possible in more complex UIs. Usually the <app-header-layout> appears at a top level component, nesting the main app components. If components further down the DOM need to define dialogs, they will basically end up not working.

Expected outcome

Backdrop should appear behind the dialog.

Actual outcome

Backdrop appears in front of the dialog. The dialog is inoperable. Clicking anywhere closes the dialog. In modal mode, the application is stuck forever as a dialog cannot be closed by clicking on the backdrop.

Steps to reproduce

Create an app layout as follows and add dialogs.

<app-header-layout fullbleed>
  <app-header slot="header">
    <app-toolbar>
      <div main-title>My main title header</div>
    </app-toolbar>
  </app-header>
  <!-- Dialogs here don't work -->
</app-header-layout>
<!-- Dialogs here work -->

Browsers Affected

Only tested on Chrome 59.0.3071.104 (Official Build) (64-bit)

simonfuhrmann commented 7 years ago

In #7, there was a workaround that globally moved all <paper-dialog> elements to a DOM position outside of the app layout (here, app-header-layout, back then paper-header-panel). The workaround is relying on querySelectorAll, and as far as I understand, this workaround doesn't work anymore with shadow dom.

valdrinkoshi commented 7 years ago

Unfortunately we can't fix this problem with the current overlay system, here more details on why.

The best solution you have is to declare dialogs in the right stacking context.

If you can afford loosing style encapsulation, you can reparent the dialogs to document.body before you open them, e.g. http://jsbin.com/vukazer/2/edit?html,output

simonfuhrmann commented 7 years ago

Are you saying it is not possible to have any dialogs with backdrop in an app layout? This seems like a severe limitation, especially when the workaround only works without local styles. Since you marked this with wontfix, will there be no solution to this problem in Polymer 2.0 in the foreseeable future?

In fact, I do have local styles that I want to preserve. Are there other workarounds?

web-padawan commented 7 years ago

@simonfuhrmann you can use app-drawer and app-header and create wrapper aroun them, similar to app-drawer-layout but without setting z-index.

simonfuhrmann commented 7 years ago

@web-padawan, thanks, in fact, not using app-drawer-layout seems to be the easiest solution for me.

valdrinkoshi commented 7 years ago

@simonfuhrmann the issue is not with app-drawer-layout really, but a missing feature of the web platform - we cannot add elements on the Top Layer of the document. Today, any element that creates a new stacking context will "trap" its contents within it. The concept of Blocking Elements stack is in the works, which will finally allow us to add arbitrary elements in the Top Layer. I've been working on possible workarounds for this issue in iron-overlay-behavior (see this PR), and finally opted for a new overlay system, iron-overlay.

Unfortunately there's no silver bullet so far. The safest option is to declare the dialog in <body>.

simonfuhrmann commented 7 years ago

@valdrinkoshi, I understand.

But I can't declare the dialog in the body, because of how the app is structured (i.e., the dialog is defined in a deeply nested element). I also cannot move the dialog using document.body.appendChild(this.$.dialog), as it relies on local styling.

Although this may not be an issue of add-drawer-layout, removing the element and re-implementing what I need is the easiest fix that actually works on my side.

maria-le commented 7 years ago

@simonfuhrmann @valdrinkoshi

I just became aware of a workaround released by IswPolymerElements called <isw-dialog>.

I have not used it yet, so I can not vouch for it. But the demo looks encouraging.

Edit: I tried <isw-dialog> and it did not perform as expected. But I did remove the modal attribute from <paper-dialog> and that workaround will be temporarily sufficient for my purposes for now until the full solution is implemented that will allow me to use the modal feature again.

web-padawan commented 7 years ago

@maria-le any dialog using paper-dialog-behavior should behave the same.

The only exception known to me is <vaadin-dialog> which uses the same approach as prototyped in <iron-overlay>: connecting overlay as direct body child.

rene-lindner-isw commented 7 years ago

@maria-le Can you provide some feedback about the problem you ran into?

Edit: The idea of the <isw-dialog> is to define the dialog somewhere in the right stacking context and control it from inside your view via <isw-dialog-remote>. I know it is uncomfortable, but at least this allows you to control the dialog in the context you use them, without the need for manual controller/mediate communication.

Remote and Dialog are communicating via events on document, like <iron-signals> does.

jukbot commented 6 years ago

any progress on this issue :(

Pietfro commented 6 years ago

@jukbot , to workaround this you could try adding a function that will move dialog's backdrop before dialog itself, like so:

const overlay = document.querySelector('iron-overlay-backdrop');
this.parentNode.insertBefore(overlay, this);

This has to be fired on iron-overlay-opened. Additionally, you should remove the backdrop on iron-overlay-closed:

this.parentNode.removeChild(document.querySelector('iron-overlay-backdrop'))

this refers to element that contains paper-dialog

ergo commented 6 years ago

The problem with this workaround is that when you fire events from your dialog they will end up in different place than someone might expect I believe?

punkratz312 commented 6 years ago

+1

ruckc commented 6 years ago

+1, with the entire goal of webcomponents being isolated and relatively loosely coupled components, having to move the dialog outside of the app-header-layout kinda destroys the intent.

craPkit commented 6 years ago

I have just found a (rather complex) work-around that works with (my setup of) app-header-layout/app-header—as-is, without modifications—in most situations. (i.e. every state except scrolled + revealed).

In a nutshell: remove the z-index from app-header as long as it's not fully scrolled away, and re-apply once it is. Then work around non-clickable contents using CSS' pointer-events property.

The only way I've found for getting the visible height involves observing CSS transform, so the actual code gets rather convoluted:

<template>
  <style>
    app-header-layout {
      pointer-events: none;
    }

    app-header {
      z-index: 0;
      pointer-events: auto;
    }

    app-header.scrolled {
      z-index: 1;
    }

    #content {
      pointer-events: auto;
    }
  </style>

    <app-header-layout id="headerLayout" class="header">
        <app-header id="header" slot="header" reveals condenses effects="waterfall">
          ...
        </app-header>
        <div id="content">
           ...
        </div>
    </app-header-layout>
  </template>
  <script>
    Polymer({
    ...
    attached() {
        const observer = new MutationObserver(this._handleStyleMutation.bind(this));
        observer.observe(this.$.header, {
          attributes: true,
          attributeOldValue: true,
          attributeFilter: ['style']
        });
        // create scroll-eventListener, too, throttled by 100ms
     },
     _handleStyleMutation(mutation) {
        const firstMutation = mutation[0];
        const cssStyleDeclaration = firstMutation.target[firstMutation.attributeName];
        const transform = cssStyleDeclaration.transform;
        if (!transform) {
          return;
        }

        const matches = transform.match(/translate3d\((.+)px, (.+)px, (.+)px\)/);
        if (matches.length <3) {
          return;
        }
        this._headerTop = parseFloat(matches[2], 10);
      },
    _handlePageScrollThrottled(evt) {
        this.$.header.classList.toggle('scrolled', window.scrollY >= this.visibleHeaderHeight);
      },
    get visibleHeaderHeight() {
        const height = this.$.header.getBoundingClientRect().height;
        return height + (this._headerTop || 0);
      }
    ...
  </script>

web-padawan commented 6 years ago

In case someone is interested, we recently published a Material theme preview for Vaadin components, so that <vaadin-dialog> can be now used with a Material Theme, see examples. This component gets teleported as I mentioned in the above comment, so it will bypass the app-layout limitations.

ergo commented 6 years ago

How about overriding app layout elements to make them NOT use shadow dom? Would that solve the issue? Since polymer 2.x this is possible.

carlardron commented 6 years ago

This behaviour seems to (almost) contravene the specifications for Material Design parent-child hierarchical transitions as described here: https://material.io/design/navigation/navigation-transitions.html#hierarchical-transitions. I say almost because I'm not finding anything specifically describing the navigation bar overlay, but I do see many examples of full screen dialog screens.

PolymerElements / paper-dialog