Multiple ripple elements in a polymer element don't respect parent node bounds

[peterwmwong]

http://jsbin.com/dayawe/2/edit?html,output

[ruifung]

On polymer 1.0, with something like

    <template>
        <paper-menu id="genreMenu" attr-for-selected="genreId" selected-values="{{selectedGenres}}" multi>
            <template is="dom-repeat" id="genreRepeat" items="{{genres}}">         
                <paper-item genre-id="[[item.id]]" class="genreItem">
                    <span>[[item.genre]]</span>
                    <iron-icon icon="check-circle"></iron-icon>
                    <paper-ripple recenters></paper-ripple>
                </paper-item>
            </template>
        </paper-menu>
    </template>

the paper ripple still doesn't respect the boundaries properly, and is in fact, interfering with clicking on some of the entries, notably the ones near the top when clicked appear to click the ones at the bottom.

That aside, the ripple effects does leak into any items which are selected at the time, but items which are not selected do not show the leaking effects.

[adalinesimonian]

Can confirm that this bug is still happening on master: http://jsbin.com/donuriyome/

Having dug into it deeper and looked through @peterwmwong's PR, it looks like the problematic code is somewhere within the target getter. The logic, as of the latest commit, is to try and get the owner root using Polymer.dom(this).getOwnerRoot() and then if there if none is found, resort to constraining the ripple to the bounds of this.parentNode.

What is the purpose of this code? Wouldn’t the ripple always want to be stuck to its parent element, or is there some nuance here I’m not catching? Modifying the logic to check for this.parentNode fixes the issue in particular, but I’m not sure if it fixes it for all use cases. I'm afraid to entirely eliminate the check, as I feel like there was a reason for this code in the first place, and that removing the search for the owner root may invalidate a particular use-case, or break functionality under a certain configuration.

I then had a chat with @arthurevans, who suggested:

I'm guessing it's kind of a convenience for people using it at the top-level of a local DOM tree; parentNode is going to be the shadow root, which does't really have a size.

If that is the case, perhaps it would be best to first check if this.parentNode is a shadow root, and if it is, then resort to using ownerRoot.host. Otherwise, it seems that in most cases, the wrong element gets selected when looking for a target.

[adalinesimonian]

Having played around with the element, it looks like indeed, when using the shadow DOM, this.parentNode will return the shadow root as a document fragment, which has no size, and isn't what we want the target to attach to.

It looks like, then, that the logic should first check if this.parentNode is a document fragment, and if it is, resort to ownerRoot.host; if it isn't, use this.parentNode.

I've implemented this logic in #22, which fixes the problem. I have checked that it works in both shady and shadow DOM.

Demo here: http://plnkr.co/edit/2ZlrEBCRVpe9JvxsBlcA?p=preview

[gorakhargosh]

+1

[pvmart]

+1 please apply vsimonian's PR

[cdata]

Thanks for doing all of the heavy lifting guys. The proposed solution seems like a good one.