vigetlabs / react-ink

A React component for adding material design style ink
https://react-ink.netlify.com/
MIT License
324 stars 36 forks source link

Ripple effect not working in Firefox #25

Open mehrdad-shokri opened 8 years ago

mehrdad-shokri commented 8 years ago

Have you tested ripple effect on firefox.
I have Firefox 48, ripple effect works on chrome but not on Firefox.

mshwery commented 8 years ago

I'm seeing the same issue.

ghost commented 7 years ago

Can confirm, this package does not work on firefox

nhunzaker commented 7 years ago

Hmm. Gross. I'll put in some time tomorrow morning to figure out what's going on.

lourd commented 7 years ago

Anything to report? Do we know what Firefox does/doesn't have that's causing this?

lourd commented 7 years ago

From what I've observed, the root cause is Firefox's handling of the canvas element (or something) inside a button. If I put the ripple component inside a div, it works fine in Firefox.

The reason I say "or something", is because the ripple effect from the Material UI library also doesn't work in Firefox, and that one is implemented with a TransitionGroup component and divs. (Side note — any benchmarks comparing the performance of the two approaches?)

So it's not a deficiency of this library, but rather a bug in Firefox. I've been trying to find a report in Mozilla's tracker of something like this happening in other cases, but nothing yet.

nhunzaker commented 7 years ago

Hey @lourd, thanks for the update! I went down a bit of a rabbit hole on this too.

As far as I can tell, using canvas is significantly faster, but I don't have any hard benchmarks. However I originally did this with SVG and found the paint times were generally way faster in canvas. I performance profiled the current implementation using the timeline tab in Chrome, and through manual testing on devices.

I wish I had a better answer here. My stance on it so far has been that this component is a total enhancement, but I'd really love to sort out what's going on in Firefox.

lourd commented 7 years ago

Here's the corresponding issue from Material UI. Doesn't indicate a fix for this component, they simply workaround it by not putting the ripple in a button.

Wwarrior1 commented 7 years ago

It seems that it works on Firefox but on extra conditions. I just realized that it works at least with 'href' or 'containerElement' attributes. However, it should not have an impact...

Let's see on official page on the label named 'Complex examples' and buttons named 'Choose an image' and 'Github link'.

However, despite it somehow works, it is bugged when screen is scrolled little down. Just like the ripple effect were attached to the point of the page... it is weird.

cobisimo commented 6 years ago

Anyone still having this issue, should reset dom.w3c_touch_events.enabled and dom.w3c_touch_events.expose values in about:config to default values. It looks like it was set to wrong default in past FF versions.

lourd commented 6 years ago

That would only fix the problem for you, not your users.

Regardless, in Firefox 59.0.1, I'm not seeing dom.w3c_touch_events.expose as an option in about:config, and setting dom.w3c_touch_events.enabled to 1 didn't change anything

cobisimo commented 6 years ago

I was investigate and this is not MUI's issue, but Firefox's. dom.w3c_touch_events.enabled default value is 2. If you have issue and this value different from 2 just reset to default, it is last option in right click menu or change to 2 manually.

lourd commented 6 years ago

I just tried using 2 for the value, and that did not change the behavior. And again, that would only fix the problem for myself, not my users.

@cobisimo, did you find or file a bug for this on Mozilla's bugzilla?

cobisimo commented 6 years ago

This issues is fixed by Firefox, but if you used multiple versions and upgrade from one to other in past couple years, as default value changes over time you may have old default value set as custom config (after upgrade). You can check https://developer.mozilla.org/en-US/docs/Web/API/Touch_events for history. Also setting this value to 0 must work, as it disables touch events. 2 is autodetect and it can detect wrong that you have touchscreen monitor. You users can be affected by this issue if they are old Firefox users.

lourd commented 6 years ago

Ah, I'm on a MacBook Pro. The value for that setting doesn't affect the behavior for trackpad or mouse for me. I'm seeing default setting for it is 0. screen shot 2018-03-19 at 4 52 40 pm