-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
Ripple stays if multiclicked fast #7537
Comments
What browser are you using? I can't reproduce it with Chrome 59. |
Unable to reproduce in:
|
@oliviertassinari @kgregory I'm on Windows 10, Firefox 54.0.1. |
I couldn't reproduce either (Windows 10, Firefox 54.0.1) but in Chrome and with a right (secondary) mouse button it does behave like that. |
@NonameSLdev I'm around 9 clicks per second and can't reproduce it in that version of Firefox. @Dattaya are you repeatedly clicking with the right mouse button to reproduce this in Chrome? |
Nevermind @Dattaya, I reproduced in Chrome by semi-rapidly clicking left and right mouse button simultaneously about ten times. |
I'm wondering should we have a ripple with a right mouse click? I would say no. So maybe the right fix is to disable them. |
@oliviertassinari good thought, but there will probably be some other combination of ripple-worthy user interactions that lead to this behavior. Have you seen anything in the standards about ink ripples on right-clicks (I'm guessing there's nothing there)? In Chrome, there are no ripples on the bookmark bar when a bookmark is right-clicked. Maybe this is enough evidence 😄 I've looked at this for a little while and I'm not exactly sure why these ripples are being stranded in the TouchRipple's state (ripple array). |
At 9cps I found something interesting: On firefox the ripples don't leave, on chrome they leave extremely slowly. BUT, on each click on firefox on a button that already has ripples that didn't leave another ripple shows up then leaves and takes a ripple with it. Odd behaviour indeed |
The issue might have been fixed by #7575. I would say 50/50 as I haven't been able to reproduce |
Just pulled and reinstalled all dependencies, still able to reproduce by clicking left and right mouse buttons simultaneously at a semi-rapid pace. |
@kgregory I guess it's an issue at the ButtonBase level in that case. Thanks for trying it out. |
This issue is driving me crazy, no matter what I try, I can't reproduce. I have added the |
@oliviertassinari I tested it around a few. I can only reproduce it with my laptop's touchpad, even at 3cps. |
Hi guys, at first, nice work. @oliviertassinari we use mui an have some strange errors concerning this issue, maybe this helps to fix this: On a Mac, using chrome you cannot reproduce this issue and everything works just fine. On a Linux machine, with chrome, rippling buttons get darker and darker when you click it at something at 3cps. Clicking it fast does not reproduce this. Remarkable is, that turning of fastclick https://github.com/ftlabs/fastclick, we use, will fix this, and buttons will handle the events right. Pressing the buttons on the mui-doc page also does not have this effect, as i assume, there are no libraries having any side effects on material-ui. Maybe it helps to reproduce or delimit this issue. |
I'm working on a cordova iOS app (with FastClick to avoid 300ms delay) using material-ui@next (1.0 v23) and I'm getting the same behavior. Removing FastClick seems to fix the touch ripples building up, but results in poor UX due to the 300ms delay. @oliviertassinari I noticed you have an app called SplitMe that's using material-ui@next + cordova, do you know of a way to avoid this touch ripple bug when used in tandem with FastClick? Or does SplitMe also have the 300ms delay? P.S. before I had been using [email protected] with FastClick and had no touch ripple issues |
@ssalka Can you notice the delay in the documentation? Is it a Cordova specific issue? I haven't been doing much work on SplitMe for a long time. As far as I know, the delay can be removed with the viewport meta. To be confirmed. |
I think it is a Cordova issue (more specifically, the iOS UIWebView). I actually couldn't find SplitMe in the app store, and unfortunately I have no way to check whether the documentation loads in my project, as CORS is disabled and HTML links to other domains open up in Safari (tried an iframe, no luck). All I can say for certain is that the touch ripples work fine on v0.x, and build up in v1. In general it looks like all the major browsers (even Safari for iOS!) have implemented a fix using the viewport meta tag, like you said, but it is still present in the UIWebView used internally by Cordova. I wouldn't count Cordova as a major browser/platform though, so I'll understand if this is not counted as a priority issue. Thanks for the quick response! |
iOS 11.3.1 - mostly I see it on IconButton components, but also on Fab. |
It looks like it's supposed to:
Steps 2 and 3 for me are not happening on my iPad (11.3.1) an older iPhone (11.4.1) or iPhone 8 (12.1.2). I'll see if I can dig into the code at some point, but can't make any promises about timing. |
@CaptainN Is this in the docs demos, or your own code? I can't reproduce it with the docs on an iPhone 10 with iOS 12.1, or an older iPad with 12.1.3, so I'm wondering if there are confounding factors at play? Not saying it isn't a bug, but there may be more steps required to reproduce it. |
It's in my own app, which is a medium sized app built with Meteor, React and Material-UI. It'll actually be public soon, so I can share a link. I'll both look for the cause in code at some point, and if I can't find it, try to produce a reduction. |
I'm also using SSR - I do get some warnings about mismatched style attributes, usually the server or client doesn't agree on the -webkit- prefix - could that have something do with it? I'm actually not really sure how material-ui applies prefixes (autoprefixer) - are there docs on that? |
I am also experiencing this issue in my own environment using Meteor, React and Material-UI and only on iOS. I've been able to use Chrome's device emulator and only to get this bug to trigger when emulating iOS devices and and not Android devices. |
I'm now seeing this even in Chrome on Windows with mobile mode enabled in Dev Tools. On this app: https://www.pixstoriplus.com/ |
@CaptainN I’ll have a look later. Just for clarity sake can you post your reproduction steps. What Chrome are you using? |
It was doing it quite consistently before, but now that I'm trying to reproduce it in Chrome, nothing. Something must have put it into an error state of some kind. On iOS, you can just load that app up and see the problem pretty easily. Tap back and forth between home and search buttons in the bottom nav for the easiest example. Or tap search, then back to home, and press home a few times. |
I can recreate it on iOS on the bottom navigation. |
I was able to get more consistent behaviour if you reload the page after switching from regular Chrome to device mode. |
@zuus-keith If it's related to the chrome dev tool mobile simulator, it's not something we should particularly fix. Unless it's the same root cause? |
@oliviertassinari I believe it is the same root cause. I initially came across it on an actual iOS device, it so happens that chrome dev tool mobile simulator set to 'iPhone' or 'iPad' views produce the same behaviour. To add on, so far the issue has only been reported on 3 different instances but all using the same tech stack (i.e. Meteor, React and MUI). |
This turned out to be due to another very common Meteor package called "fastclick" - which hasn't really been necessary (if you use the right viewport settings) in Safari for years. If anyone happens across this issue, the solution is to simply remove "fastclick". |
Is there anyone who is affected by this and not in a simulated environment? Or is it okay to close this? |
This was fixed for me in non-simulated environments (on iPad) by removing fastclick package from my meteor app. |
Closing for now, if anyone can reproduce let us know. |
Initially, we had fastclick to remove the touch delay on mobile devices. Mobile browsers have actually removed the delay for click. Only old browsers like UIWebView have kept the behavior. Drive has kept fastclick only for iOS, for this very reason. We had introduced react-fastclick after a peculiar bug on react-select. See 2c187c6 react-fastclick actually brings more problem than necessary. For example, on chrome mobile (in responsive mode with touch events enabled), click would be fired after after a modal was opened and would close it. It would also prevent some time the click on Switches, see https://trello.com/c/eRtVM9Jn/991-probl%C3%A8me-de-clic-un-peu-partout-vu-sur-la-1-23-0-beta-20#action-6009d6d8fe86d00c8927b895 Material UI talks about problems with fastclick mui/material-ui#7537 I am hesitating between re-adding fastclick and completely removing it. I expect that we'll migrate soon to wkwebview. Anyway I hope that these kind of problems forces us to do so. I expect click delay to be increased on mobile cordova iOS.
you can recreate by clicking on a button, and then dragging outside the range of button while holding. |
If someone clicks in about >9 clicks per second for a little while (1-2 seconds) the ripples don't leave the button and it stays with the color. Small demonstration:
![](https://camo.githubusercontent.com/ca8eb5632d81a05e14dcc272b951df706f0a0a57a8bd9f5aedbfee85a049ec0a/68747470733a2f2f6779617a6f2e636f6d2f32323365356232346162343035636163653161636333613535643837646664662e676966)
You're welcome to try it yourself: https://material-ui.com/demos/buttons/#text-buttons
EDIT: This comment describes the issue - there are more mouse downs than ups and the ripple isn't released.
The text was updated successfully, but these errors were encountered: