Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abort paper ripple when scrolling on touch devices #66

Open
MajorBreakfast opened this issue Mar 10, 2016 · 7 comments
Open

Abort paper ripple when scrolling on touch devices #66

MajorBreakfast opened this issue Mar 10, 2016 · 7 comments

Comments

@MajorBreakfast
Copy link

Scrolling on touch devices triggers the ripple effect which is really distracting. It'd be great if the ripple could start as normal but aborts if a scroll is detected. Maybe using requestAnimationFrame and element.getBoundingClientRect() to detect if the element was moved?

@keanulee
Copy link
Contributor

@MajorBreakfast Calling getBoundingClientRect() on every frame seems expensive. A better way would be to install a scroll listener and then cancelling the ripple in there:

document.addEventListener('scroll', function() {
  this.getRipple().uiUpAction();
});

Of course, this gets complicated if the element is inside an element scroller (i.e. not the document scroller), so this behavior is probably more suitable at the user-level.

@MajorBreakfast
Copy link
Author

@keanulee Using getBoundingClientRect() was just a quick idea. There might be a better solution.

About document vs element scroller: I've never built a web app that is a document scroller. That's too constrained for me. I'm always scrolling inside elements. I'm using Polymer mostly because it lets me build great looking apps. I don't want my app to be recognized as a web app too easily. Scroll indicators on top of the toolbar would be a giveaway. So, never document scroller :)

About the performance cost of getBoundingClientRect(). This solution requires the loop to run only on pressed buttons. That's (unless you nest buttons) just a single button at a time. I can't come up with a more reliable solution to detect scrolls.

I think what's really needed here is different events like press-start, press-cancel and press-end in the iron behavior. Where press-end is fired after successful presses and press-cancel if the press turned out to be a scroll. Then, it's easy to modify the ripple behavior.

I've added a quick-fix in my app:

listeners: {
  'touchmove': '_onTouchMove'
},
_onTouchMove: function (event) {
  var rippleEl = this.getRipple()
  rippleEl.ripples.map(function (r) { rippleEl.removeRipple(r) })
}

That's not a good general solution though, because it detects false positives. I'm not using uiUpAction() action because it's not instant.

@MajorBreakfast
Copy link
Author

I've played around some more and I come to the conclusion that it'd be best if the ripple only appeared once a tap is detected. That's the way it is done in android, e.g. the settings app. So, buttons should behave differently depending on whether they're inside a scrollable area or not:

  • Inside scrollable area (e.g. a list item): Play ripple on tap event
  • Outside scrollable area (e.g. a toolbar button): Play ripple on mouse down (sooner)

Maybe an attribute to switch between the behaviors could be introduced: ripple-on="mousdown", ripple-on="tap"

@imolorhe
Copy link

imolorhe commented Oct 4, 2016

@keanulee @MajorBreakfast what is the best approach for this now? I currently have the same issue.

@MajorBreakfast
Copy link
Author

MajorBreakfast commented Oct 5, 2016

@imolorhe I placed a <paper-ripple> inside a <div> with pointer-events: none and trigger the ripples manually when appropriate.

...
<dom-module id="my-element">
  <template>
    <style>
      :host { position: relative; }
      #rippleContainer { pointer-events: none; }
    </style>

    ...

    <div id="rippleContainer">
      <paper-ripple id="ripple"></paper-ripple>
    </div>
  </template>

  <script>
    Polymer({
      is: 'my-element',

      listeners: {
        'tap': '_onTap'
      },

      _onTap: function (event) {
        this.$.ripple.uiDownAction(event)
        this.$.ripple.uiUpAction(event)
        ...
      }
    })
  </script>
</dom-module>

Just a workaround, though.

(It's been a while. Come the think about it, the wrapping <div> might not be needed. Setting pointer-events: none on the ripple probably works also. You can test that if you want.)

@imolorhe
Copy link

imolorhe commented Oct 5, 2016

@MajorBreakfast Thanks! This helped a lot! I had to reuse it for my purpose though, since mine was a list component with several ripples in it.

@MajorBreakfast
Copy link
Author

@imolorhe Glad I could help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants