[TouchRipple] Do not trigger it when scrolling

[manuelpaulo]

I've checked #3407, but I honestly think it's still not fixed, or is it back?!

https://www.dropbox.com/s/jm36onjlml8j9oq/IMG_3636.MOV

Thanks a lot.

[oliviertassinari]

@manuelpaulo The behavior in the video looks correct, the ripple is interpreted as soon as you start scrolling. What I am missing?

[manuelpaulo]

@manuelpaulo The behavior in the video looks correct, the ripple is interpreted as soon as you start scrolling. What I am missing?

Hi, thanks for the fast reply.

What is wrong?
You are correct, problem is ripple effect should be triggered when we touch to select/open and not when we touch to drag. Tapping a list to scroll it is not a select item press gesture. Actual behavior creates a constant flick. Makes one afraid of touching it in order not to trigger it again. Mostly noticed when browsing social networks news feeds.

Why is wrong?
Sometimes it triggers it, sometimes it doesn't. A correct behavior should not be random.

Why now?
Because GPU's are getting faster and faster, and people are using dark mode more often. Many on dark OLED.
https://www.reddit.com/r/RelayForReddit/comments/i05bk5/is_there_a_way_to_disable_this_touch_animation_i/

Is it serious?
Yes. Twitter has disabled it on their Android mobile app after people complaining. This is a very serious issue for people with photosensitive epilepsy spending hours on social networks. I am surprised no one raised this problem before.

[oliviertassinari]

@manuelpaulo Do you have a recording with our components, for instance, the list https://next.material-ui.com/components/lists/#lists? You seem to make references to native Android components.

[manuelpaulo]

@manuelpaulo Do you have a recording with our components, for instance, the list https://next.material-ui.com/components/lists/#lists? You seem to make references to native Android components.

Sure: https://www.dropbox.com/s/p3dm2st04a0qaza/IMG_3644.MOV?dl=0

[oliviertassinari]

@manuelpaulo Ok thanks for the recording. I'm closing for #2165. This is part of the tradeoff.

[eps1lon]

@manuelpaulo Ok thanks for the recording. I'm closing for #2165. This is part of the tradeoff.

Seems like you did fix it temporarily in #8605. That video shows the same interaction as the recorded video in the dropbox. So it's unclear what the "tradeoff" is here.

What this ultimately comes down to is

https://github.com/mui-org/material-ui/blob/5c954b33efdad8e07fd01e780206814eee3fa51b/packages%2Fmaterial-ui%2Fsrc%2FButtonBase%2FTouchRipple.js#L240-L249

in which we purposely trigger the ripple even though we kow it was cancelled. Seems to me we should actually skip it if we haven't started.

[eps1lon]

@manuelpaulo Could you try out https://deploy-preview-23829--material-ui.netlify.app/components/lists/ (might take a few minutes till it's build) and tell me if that feels better to you?

[oliviertassinari]

So it's unclear what the "tradeoff" is here.

I believe there are different aspects in the tradeoff. The most direct one is the amount of time we wait before firing the ripple DELAY_RIPPLE is set at 80ms. In the video of the user, he waits a bit before scrolling. In the video of #8605, I move the finger quickly. However, the higher DELAY_RIPPLE is, the most laggy the UI feels for touch click like interactions.

[eps1lon]

The DELAY_RIPPLE tradeoff I understand. It's a classic fine-tuning issue that will never cover all users properly.

What I'm missing is why we skip the DELAY_RIPPLE if the touch is interrupted (pressumably that's what touchend is for). Though I guess it won't fire if we start scrolling? But if we agree that an interrupt should not "flush" the ripple anyway then we can move to "how do we detect scroll-after-touchstart?".

[oliviertassinari]

What I'm missing is why we skip the DELAY_RIPPLE if the touch is interrupted (pressumably that's what touchend is for).

I think that the idea what to still retain a "smooth" UX, avoiding the UI to flash when interrupting the ripple for a late scroll intent. Instead, we continue on the inertia of the ripple already visible. Making it progressively disappear.

[eps1lon]

Instead, we continue on the inertia of the ripple already visible.

As far as I can tell the linked code is not concerned with an already visible ripple. Instead it will immediately display the ripple that we delayed for DELAY_RIPPLE ms. So the user wouldn't even notice it because we assume that a touch phase shorter than DELAY_RIPPLE is not an intentional touch.

Update:
double-checked with the existing test in #23829 which emphasises that the code in question is not concerned with a visible ripple. It actually causes the ripple to become visible

[oliviertassinari]

double-checked with the existing test in #23829 which emphasises that the code in question is not concerned with a visible ripple. It actually causes the ripple to become visible

Agree 👍 , I was answering on a tangent in #23808 (comment).

[oliviertassinari]

Could you try out https://deploy-preview-23829--material-ui.netlify.app/components/lists/ (might take a few minutes till it's build) and tell me if that feels better to you?

To remove any potential sources of confusion, this approach breaks the ripple for fast interactions, I don't think that we can rely on it. For instance, in the following case, a click event is triggered but no visual feedback is provided (ripple):

[eps1lon]

a click event is triggered but no visual feedback is provided (ripple):

That was my suspicion. A click should always trigger the ripple.

Could you test this on a real device and verify there is actually a click event?

[oliviertassinari]

Could you test this on a real device and verify there is actually a click event?

I have tested on a Xiaomi Mi 9 phone, with Android, MIUI 11, and Chrome latest. The default for me is to have the collapse open/close without ripple. I have to make a "slow" tap to see it. Tested on this demo: https://deploy-preview-23829--material-ui.netlify.app/components/lists/#nested-list.

[manuelpaulo]

Sorry for the late reply. I've tested it and it seems way much better.

Only getting wrongly the effect when scrolling down and then immediately changing my mind and scrolling back up.

[eps1lon]

I'm re-opening for visibility. As a next step we need to figure out why #23829 prevents a ripple for clicks and then check if this still improves the story in this issue. Might very well be that the device @manuelpaulo has actually triggers clicks in which case it becomes not viable to fix the issue from our side.

[oliviertassinari]

figure out why #23829 prevents a ripple for clicks

@eps1lon I think that you are hitting the core of the tradeoff. On touch start, the component schedules the ripple after 80ms. If a touch move is triggered before the end of the 80ms, we unscheduled the ripple. If the touch end triggers before the end of the 80ms, we trigger the ripple.

Why are we using 80ms? It was meant to provide feedback to users under 100ms so it feels like the UI responds instantly, starting from the touch start.

Considering the tradeoff if 4 years old, it makes sense to benchmark what other are doing. Maybe they have found better solutions.

• Angular Material doesn't seem to do anything about it: https://material.angular.io/components/list/overview
• Vuetify seems to use the same tradeoff as us: https://vuetifyjs.com/en/components/lists/#flat
• Gmail, they brutally interrupt the ripple even if it has started when scrolling on the side nav. On the main content, they seem to behave like us.

One this I would like to mention is #19664, which I think is more important than this issue. The current implementation of the ripple doesn't seem aligned anymore with how Google implements it on its product, or on the Material Design spec. Nowadays, the ripple seems to start with a wider surface and to move faster. I have added a screen recorder on the issue to better compare the "feeling".

[manuelpaulo]

check this https://github.com/software-mansion/react-native-gesture-handler
and this: https://reactnative.dev/docs/pressable