PolymerElements / paper-dropdown-menu

A Material Design browser select element
https://www.webcomponents.org/element/PolymerElements/paper-dropdown-menu
61 stars 107 forks source link

`iron-overlay-closed` event bubbles up unexpectedly #87

Closed morficus closed 8 years ago

morficus commented 8 years ago

Use case: when using a <paper-dropdown-menu> element is contained with in a <paper-dialog> element, the iron-overlay-closed listener gets triggered when the <paper-dropdown-menu> is closed.

Impact: the function registered to the listener gets triggered at unexpected times

Demo: https://jsbin.com/sugocaguva/edit?html,output

kriscooke commented 8 years ago

+1 Not sure why/whether it's supposed to bubble up... If so, then we need a more specific 'closed' event from paper-dialog. Particularly hazardous for common 'closed' listeners like form.reset()!

In my case, making a selection from the paper-dropdown causes the form to be unexpectedly reset.

cdata commented 8 years ago

This sounds reasonable. These events probably shouldn't bubble.

@valdrinkoshi do you have any opinions about this?

valdrinkoshi commented 8 years ago

Yeah it makes sense, the event shouldn't bubble. But I don't know if this should be fixed in iron-overlay-behavior to fire an event that doesn't bubble, or handle it in the paper-dropdown-menu..

fakiolinho commented 8 years ago

Maybe an idea would be the option to attach the event to an emitter. So if an emitter's id is provided we can understand who emits it and respond accordingly. If the emitter's id is omitted the event can be caught by everyone as usual:

listeners: {
    'dialogID.iron-overlay-closed': '_modalClosed',
    'dropdownMenuID.iron-overlay-closed': '_menuClosed'
},
_modalClosed: function(e) {

},
_menuClosed: function(e) {

}

So i could respond on those 2 using separate methods. At the very moment in order to tackle the event bubbling and respond only if the event is triggered by the paper-dialog element i am doing this:

...
_modalClosed: function(e) {
    if (e.srcElement.id == 'dialogID') {
        // do sth here
    }
},
...
valdrinkoshi commented 8 years ago

related to https://github.com/PolymerElements/iron-overlay-behavior/issues/70, see [proposed solution](https://github.com/PolymerElements/iron-overlay-behavior/issues/70#issuecomment-172035455]. Changing the event to non-bubble would be a breaking change, so I'd suggest to use the proposed solutions & close this issue.

valdrinkoshi commented 8 years ago

Will move this issue to paper-menu-button as it has already 2 other events for opening/closing, and it should be the one stopping the propagation of iron-overlay-opened/closed/canceled

valdrinkoshi commented 8 years ago

This issue was moved to PolymerElements/paper-menu-button#86