googlearchive / paper-ripple

Adds a ripple effect to UI elements à la Material Design
47 stars 18 forks source link

[0.8] Multiple ripple elements in a polymer element don't respect parent node bounds #38

Open peterwmwong opened 9 years ago

peterwmwong commented 9 years ago

http://jsbin.com/yusose/3/edit

ripple

peterwmwong commented 9 years ago

I believe this difference in behavior is due to the following in paper-ripple.html:

      get target () {
        return this.host || this.parentNode;
      }
corbin-r commented 9 years ago

Could this possibly be prevented by just doing:

get target(){
  return this.host;
}

Or would this not be possible?

I will test this when I get the chance to!

tachyon1337 commented 9 years ago

in the current version 1.0.0, if get target() is modified like below, paper-ripple appears to works as expected in either the light dom or within an element definition. Otherwise, unmodified, it does not necessarily work as expected if used within an element definition.

get target(){
   return this.parentNode;
}

Another qualification: the intended bounding container should not be statically positioned

robrez commented 9 years ago

+1

robrez commented 9 years ago

Relating to this, if a ripple is repeated via dom-repeat, all siblings animate at the same time. I assume they all receive the same touch event since a given ripple element does not fit itself within its relative container.

Example:

<template is="dom-repeat" items="{{items}}" index-as="idx">
  <div class="container layout vertical relative">
    <div>{{item}}</div>
    <paper-ripple class="fit"></paper-ripple>
  </div>
</template>

Each ripple will have an additional animating attribute simultaneously.

robrez commented 9 years ago

I worked-around the issue by creating an element:

  <template>
    <content></content>
    <paper-ripple></paper-ripple>
  </template>

and changing my previous markup to use the element:

<template is="dom-repeat" items="{{items}}">
  <inky-element class="container">
    <div>{{item.stuff}}</div>
  </inky-element>
</template>
robrez commented 9 years ago

I can confirm that this has been fixed, for me, over here:

https://github.com/PolymerElements/paper-ripple/issues/2

tachyon1337 commented 9 years ago

Yep, it appears transition from the shadow-dom encapsulation to a "shady-dom" implementation broke the behavior. Looks like the fix checks if a document-fragment before assigning ownerRoot.host, otherwise its simply the parentNode(which is what it should be if not a shadow-dom).

One confusing thing is that "shady-dom" is supposed to employ shadow-dom where natively available

https://www.polymer-project.org/1.0/articles/shadydom.html

but there is no longer any shadow-dom fragments in polymer components, even when testing in chrome. Thus the previous broken behavior of paper-ripple across all browsers.