Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Model paper-dialog not working correctly within a polymer-starter-kit app #154

Closed
rbjarnason opened this issue Jun 11, 2015 · 32 comments
Closed
Assignees
Milestone

Comments

@rbjarnason
Copy link

I have a component that uses paper-dialog to display errors. When I added it to a simple polymer-starter-kit app the modal dialog overlay is also covering the dialog box itself and it is not possible to press the OK button as the overlay is covering it - you can still press return and escape to dismiss the paper-dialog. Strange thing is that z-index looks ok when you debug it in Chrome but still the overlay is on top of the dialog. This does not happen when using exactly the same component code directly outside the polymer-starter-kit.

@Alienuser
Copy link
Contributor

Hi @rbjarnason
I have the same issue and a workaround for that.

I have this button to open the paper-dialog:

<paper-icon-button on-click="feedback" icon="announcement"></paper-icon-button>

This is my "feedback" function in the element: (note: the paper-dialog is in the same element)

feedback: function () {
            var dialog = document.querySelector('#dialogFeedback');
            this.setDialog(true);
            dialog.open();
        }

The feedback function opens the "setDialog" function:

setDialog: function (open) {
            if (open) {
                var node = document.querySelector('#dialogFeedback');
                var textnode = document.querySelector("body");
                textnode.appendChild(node);
            } else {
                var node = document.querySelector('#dialogFeedback');
                var textnode = document.querySelector("my-topnavigation");
                textnode.appendChild(node);
            }
        }

This function is adding the whole paper-dialog to the body-element. Then you can use the paper-dialog as normal.

Every time the paper-dialog should close, you have to call "this.setDialog(false);". Then the paper-dialog will switch back to his old position in the dom.

The error occours, because the "iron-overlay-backdrop"-element - for modal paper-dialogs - is always added to the body-element and overlaps the paper-dialog.

I hope I can help you!

@addyosmani
Copy link
Member

Thanks for posting the workaround @Alienuser.

The error occours, becouse the "iron-overlay-backdrop"-element - for modal paper-dialogs - is always added to the body-element and overlaps the paper-dialog.

Is this a bug with iron-overlay-backdrop? If so, is it worth filing upstream on the relevant paper-/iron- repo so that we can get it fixed there?

@rbjarnason
Copy link
Author

@Alienuser Thanks! I can now use the workaround until a fix is in place :)

@Alienuser
Copy link
Contributor

I don't think this is a bug with iron-overlay-backdrop because the paper-dialog works well if I add it in the index.html.

But I think it would be nice if the iron-overlay-backdrop will add itself in the same node as the paper-dialog. Therefore I think its a feature request? :)

@ghost
Copy link

ghost commented Jun 29, 2015

Hi, same issue here.
@Alienuser is right if overlay is moved in same node as dialog, z-index comparison for both elements works perfectly. Otherwise dialog is considered as a 0 z-index element as is container.

@ConAntonakos
Copy link

I noticed this as well. Echoing @Alienuser's feedback. I added paper-dialog to an inner child other than body, and iron-overlay-backdrop seems to automatically attach itself to body.

By the way, @Alienuser, is your workaround implemented within the Polymer element? Still nascent to Polymer so I apologize!

@vctrl
Copy link

vctrl commented Jul 14, 2015

@Alienuser have same issue. I've followed your instructions: created button which calls feedback button on click and added feedback and setDialog functions to file paper-dialog.html, but nothing gonna happens when I press the button. I am totally beginner, and I really don't know what's the problem. Please, can you explain as detail as you can to help me?

@SayChunKim
Copy link

Is this paper-dialog modal which has been questioned for?

i used

  1. document.querySelector('#dialog').toggle(); for enabling toggle for paper-dialog id '#dialog'
  2. add paper-dialog items under div class="content"

Output:
screenshot from 2015-07-24 20 24 37

Reference from F12 Dev-Tools https://elements.polymer-project.org/elements/paper-dialog?view=demo:demo/index.html

I'll try again put into template and improve again if possible.

@samccone
Copy link
Contributor

samccone commented Jan 6, 2016

@rbjarnason Still an issue?

@robdodson
Copy link
Contributor

Closing but happy to reopen if others are running into this

@Skmsoumya
Copy link

Bump. Same Issue here.

I am using polymer starter kit and opening paper dialog as "modal" makes the overlay cover the whole page even the dialog.

Using the fix for now. Thanks @Alienuser

@samccone samccone reopened this Mar 18, 2016
@samccone
Copy link
Contributor

@Skmsoumya any way you could push a test case for us?

@samccone samccone added this to the 1.4.0 milestone Mar 18, 2016
@nicholaswmin
Copy link

Perhaps it's connected to this: PolymerElements/paper-dialog#79

@RyanEwen
Copy link

RyanEwen commented Apr 3, 2016

I have the same issue as well. I created a custom element which contains an icon button and a dialog, and it's used within a toolbar. Backdrop covers the dialog:(

@timblack1
Copy link

timblack1 commented Apr 24, 2016

Can we just modify paper-dialog-behavior.html to call the following? This works for me. If this direction is acceptable to the devs, I'm happy to submit a patch.

attached: function() {
  if (this.keepWithParent !== true) {
    document.querySelector('body').appendChild(this);
  }
}

See PolymerElements/paper-dialog#7

@Skmsoumya
Copy link

Skmsoumya commented Apr 28, 2016

@polymerteam Sorry for the getting so late. A peculiar thing that I found out is that sometimes the dialog works sometimes it doesn't. I downloaded a fresh Polymer Starter kit and did what I was doing with my production site. It worked there.

@johncvieira
Copy link

+1

1 similar comment
@nicholaswmin
Copy link

+1

@Skmsoumya
Copy link

+1 Great!

@MarcosGalaviz
Copy link

@Alienuser hi, very interesting, I use your solution and works perfect on Chrome but aparently doesnt work on explorer or firefox :(

@robdodson
Copy link
Contributor

@valdrinkoshi is the recommended fix the one advised here PolymerElements/paper-dialog#7 (comment) ?

@valdrinkoshi
Copy link
Member

Hi @robdodson, yes overlays should be appended in a node that is in the same stacking context as document.body.
There are 2 initiatives going on around this problem:

@MarcosGalaviz
Copy link

MarcosGalaviz commented May 4, 2016

Hi, thanks for your comments, but I am just a rookie with polymer and maybe my english is not the best :D , thats why even with your answer I am not sure about how to deal with this problem or how to implement it yet :(, but thanks for your time I am going to try.

@robdodson
Copy link
Contributor

Does it work when you run gulp serve:dist? Any console errors when you open
the firebase version in a browser?

On Jul 23, 2016 7:47 AM, "Chad Baxter" notifications@github.com wrote:

it is fine for me when serving with gulp, but when I deploy it to firebase
this bug appears. I am using the default demo code for the dialog
javascript functions.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#154 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABBFDfQevnFeXJh88fsiEszsyw1fYQbEks5qYimFgaJpZM4E_Gfn
.

@robdodson
Copy link
Contributor

OK so the solution to this problem is to put any paper-dialog elements inside of <body>. Do not put them inside of other custom elements. Explained a bit more here: PolymerElements/app-layout#295

As Valdrin mentioned, there is also work going on to improve the way dialogs work. Honestly, it feels like we're missing some crucial platform primitives and I tried to write about them here: https://robdodson.me/building-better-accessibility-primitives/

Closing this issue as the fix is to put paper-dialog directly inside of body.

@robdodson
Copy link
Contributor

Another approach that I think may also work if you're using the PSK layout.

Add your paper-dialog to my-app.html after your drawer layout
https://gist.github.com/robdodson/d05a9404ac2530b5019d63e07f22e2d6#file-my-app-html-L107-L112

Add a listener for an event to open a dialog. This event will be dispatched by one of your views
https://gist.github.com/robdodson/d05a9404ac2530b5019d63e07f22e2d6#file-my-app-html-L120-L122

In your view, add a button to open a dialog
https://gist.github.com/robdodson/d05a9404ac2530b5019d63e07f22e2d6#file-my-view1-html-L30

When the user clicks the button, dispatch the event which will get picked up by the listener in my-app.html
https://gist.github.com/robdodson/d05a9404ac2530b5019d63e07f22e2d6#file-my-view1-html-L37-L39

@RhysyG
Copy link

RhysyG commented Oct 20, 2016

Sorry for diving back into a closed topic. So for the approach you listed @robdodson, how would one handle all app instances of modals (ie 15+). Would you just list them all there? Or is this of the assumption you would append all the different types of content to that one dialog instance? Cheers.

@robdodson
Copy link
Contributor

@valdrinkoshi do you have any thoughts on how to handle a bunch of modals in an app?

I think I might just see if I could have 1 modal and swap out the content inside of it based on the route somehow

@ketile
Copy link

ketile commented Nov 9, 2016

@robdodson In order to cleanly handle many modals, would it be possible to use an id on the button triggering the action to open a modal paper-dialog of the same name?

Let's say we have two paper-dialogs in my-app.html named dialog1 and dialog2:

    <paper-dialog id="dialog1" modal>
      <h2>Dialog 1</h2>
      <div class="buttons">
        <button dialog-confirm autofocus>Accept</button>
      </div>
    </paper-dialog>  

    <paper-dialog id="dialog2" modal>
      <h2>Dialog 2</h2>
      <div class="buttons">
        <button dialog-confirm autofocus>Accept</button>
      </div>
    </paper-dialog>  

In my-view1.html we have two buttons with corresponding ids:

    <button on-tap="open" id="dialog1">Open dialog 1</button>
    <button on-tap="open" id="dialog2">Open dialog 2</button>

Shadow/Shady/Local/Polymer/WTF DOM should enable us to re-use ids, right?

The open function in my-view.html:

      open: function(e) {
        this.fire('open-dialog');
      }

The event listener in my-app.html:

      listeners: {
        'open-dialog': 'handleOpenDialog'
      },

I've tried the following but not been successful in catching the id of the button:

      handleOpenDialog: function(e) {
        console.info(e.target.id + ' was clicked.'); // was clicked
        console.info(Polymer.dom(e).localTarget.id + ' was clicked.'); // was clicked
        console.info(Polymer.dom(e).path[1].getAttribute('id') + ' was clicked.'); // null was clicked
        console.info(Polymer.dom(e).rootTarget.getAttribute('id') + ' was clicked.'); // null was clicked
        console.info(e.model.get('id') + ' was clicked.'); // cannot read property 'get' of undefined
        // this.$.dialog.open();
      }

What is the proper way of getting the value of the id in this context and how would you use it to trigger a modal dialog of the same name?

Aslo, if you don't see this approach working, could you please give some advice on how to proceed?

@robdodson
Copy link
Contributor

@ketile you'd need to look for the id of the button in your open function and pass that with the event.

open: function(e) {
  var id = e.currentTarget.id;
  this.fire('open-dialog', {id: id})
}

That could work fine on a small site. On a much larger site you might prefer to set the opened property of the modal directly using a top-down data flow structure (maybe something like Redux) or you could use PubSub if you just want to trigger a modal and not go through several layers of event bubbling

@ketile
Copy link

ketile commented Nov 10, 2016

@robdodson Thanks. What would be the syntax for receiving this argument in the event listener and the handleOpenDialog function? Can't find the answer in the documentation.

@robdodson
Copy link
Contributor

handleOpenDialog(event) {
  console.log(event.detail.id);
}

dave-mills added a commit to stats4sd/Stats4SD-Resources-Site that referenced this issue Apr 21, 2017
Included error message in custom div -> hidden by default, shown when needed. 

defaulted to this method, after trying toasts and paper dialogs: 

--- Tried using toasts - couldn't get them to effectively appear where I wanted.

--- Tried using paper-dialog, but they need to go up near the <body> tag, otherwise their overlay appears over the dialog (many github issues, eg: Polymer/polymer-starter-kit#154)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests