googlearchive / paper-menu

A Material Design menu
https://www.webcomponents.org/element/PolymerElements/paper-menu
27 stars 48 forks source link

opened-changed overservers gets triggered twice #99

Open ktiedt opened 8 years ago

ktiedt commented 8 years ago

Due to iron-collapse also generating the same event, it bubbles up to the paper-submenu and triggers twice, and I am guessing due to event retargetting the target is set to paper-submenu each time...

We have to evaluate the event.path property to make sure we only listen to the actual submenu event. Below is the handler we use for on-opened-changed:

  /**
   * An observer that intercepts submenu open events and forces a subsequent
   * iron-collapse updateSize call to bypass a race condition in iron-collapse.
   * @param {!Object} evt The opened-changed event object.
   * @param {!Object} change The change record for the opened property.
   */
  ensureMenuOpens_: function(evt, change) {
    if (change.value) {
      var menu = evt.target;
      // event from iron-collapse will bubble up and trigger this twice, so if
      // path[0].tagName is not paper-submenu, we ignore this.
      if (evt.path[0].tagName.toLowerCase() == 'paper-submenu') {
        menu.async(function() {
          this.set('opened', true);
          this.$.collapse.updateSize('auto', true);
        }, 1);
      }
    }
  },

Note: Our need for this handler could be a side-effect of #88, after reading it, but I am not positive.

bicknellr commented 8 years ago

@ktiedt, in our slack chat the other day you said this only occurs in Shadow DOM (not Shady), right?

ktiedt commented 8 years ago

That sounds correct, I'm not able to test it today easily though to confirm. On Apr 14, 2016 11:30, "Russell Bicknell" notifications@github.com wrote:

@ktiedt https://github.com/ktiedt, in our slack chat the other day you said this only occurs in Shadow DOM (not Shady), right?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/PolymerElements/paper-menu/issues/99#issuecomment-210090115

krumware commented 7 years ago

I wanted to expand on this. I'm seeing a similar issue when the menu is set to open. , the menu requires two clicks to close. The first click appears to adjust the container's scroll position, then the second click causes the close.

danbeam commented 7 years ago

@krumware see https://github.com/PolymerElements/paper-menu/issues/88

@ktiedt it seems like there's a localTarget check in the event listener now? is this still a valid issue or shall we close?

ktiedt commented 7 years ago

I've been out the past week, let me find where that code was used again and also verify that we have finally updated to a point where the fix is in our local code base, I can do this tomorrow.

ktiedt commented 7 years ago

Sorry for the delay, was unable to reach Github to compare the code in question last Friday. Long story short, both of my problems still exist.

1) in ShadyDOM paper-submenu will not render open the first time its clicked 2) in ShadowDOM paper-submenu receives 2 on-open-changed events.