Material-ui: onTouchTap fires twice on Lists and other components

Created on 25 Aug 2016  ·  11Comments  ·  Source: mui-org/material-ui

I have a List component. Click on an item in the list replaces the original list with another list. On mobile devices, when I tap on the first list, it fires a second tap on the new list when the new list comes up. This ends up clicking the item in the new list too and leads to unpredictable UI behavior. I found a similar issue reported on react-tap-plugin-event page. I'm wondering whether it's that plugin's issue or an MUI issue, and is there a way to resolve it?

Versions

  • Material-UI: 0.15.4
  • React: 15.3
  • Browser: Chrome 52x
bug 🐛

Most helpful comment

In case you didn't see the post in the other issue you linked:

I found a solution so it may help you too.

After the onTouchTap event another onClick event is fired after a delay (~300ms), whether the onTouchTap is handled or not.
The onClick event is triggered on an element at the same position as the onTouchTap event but after the delay.

FIX:
Adding e.preventDefault to the onTouchTap event handler prevented the onClick event for me.

Example:

All 11 comments

How long we can not use react-tap-plugin-event?

In case you didn't see the post in the other issue you linked:

I found a solution so it may help you too.

After the onTouchTap event another onClick event is fired after a delay (~300ms), whether the onTouchTap is handled or not.
The onClick event is triggered on an element at the same position as the onTouchTap event but after the delay.

FIX:
Adding e.preventDefault to the onTouchTap event handler prevented the onClick event for me.

Example:

I'm using React 15.3.1, and while I'm not sure when it was introduced, it looks like TapEventPlugin is now included in React's libs by default, so I've given up using react-tap-plugin-event and have switched to using the following:

require('react/lib/EventPluginHub').injection.injectEventPluginsByName({
  'TapEventPlugin': require('react/lib/TapEventPlugin')
});

I haven't tested it extensively yet, but seems to work well in desktop Chrome and in Chrome for Android. Might be worth a shot to see if it fixes your issue, as the raw TapEventPlugin code is indeed different than react-tap-plugin-event's.

Unfortunately, none of the solutions mentioned above work.

@JamesAlias - when I implement your solution, touch tap on ListItem stops working.

@jamespizzurro - I tried your solution, but the behavior is exactly the same with React's TapEventPlugin too.

I have implemented the workaround for Ghost Clicks posted on react-tap-event-plugin's page and it does help prevent the ghost clicks, but I still get a ripple effect on the list items on the next page.

injectTapEventPlugin({
    shouldRejectClick: function (lastTouchEventTimestamp, clickEventTimestamp) {
        if (lastTouchEventTimestamp && (clickEventTimestamp - lastTouchEventTimestamp) < 2000) {
            console.log("Reject click : " + (clickEventTimestamp - lastTouchEventTimestamp));
            return true;
        }
    }
});

Is there a reason for MUI to have onTouchTap listener instead of onClick now that the 300ms delay has been removed from the browsers?

any news about this ?
How are you dealing with this ?
onClick doesn't work on safari
onTouchTap gets called twice (or you have to reject clicks in injectTapEventPlugin but you still have a ghost ripple effect)
I see that de material-ui docs works fine without shouldRejectClick... How are you doing this ???
@deepfriedbrain did you find a workaround ?

I've finally followed @JamesAlias advice :
calling e.preventDefault() inside the onTouchTap event handler
Is there any drawback to this approach ?

Same here, this seems to happen sporadically. For example when expanding a Card element, it often fires twice so that it will immediately close again.

Wat _does_ seem to help sometimes, but this is on the user side, is to press and hold the CardHeader or Button or whatever element is receiving the onTouchTap, then it will only fire once it seems.

But there are times where I load my page and this does not occur at all, so whether this is an issue with injectTapEventPlugin or using e.preventDefault(), I think it has mostly to do with the implementation of that 300ms delay?

Okay this is called ghost click and documented here with solution.

https://github.com/zilverline/react-tap-event-plugin

Ignoring ghost clicks

When a tap happens, the browser sends a touchstart and touchend, and then 300ms later, a click event. This plugin ignores the click event if it has been immediately preceeded by a touch event (within 750ms of the last touch event).

Occasionally, there may be times when the 750ms threshold is exceeded due to slow rendering or garbage collection, and this causes the dreaded ghost click.

The 750ms threshold is pretty good, but sometimes you might want to override that behaviour. You can do this by supplying your own shouldRejectClick function when you inject the plugin.

The following example will simply reject all click events, which you might want to do if you are always using onTouchTap and only building for touch devices:

var React = require('react'),
injectTapEventPlugin = require("react-tap-event-plugin");
injectTapEventPlugin({
shouldRejectClick: function (lastTouchEventTimestamp, clickEventTimestamp) {
return true;
}
});

what's the progress on this issue? I have a list on one page and touch on one of the items will transit to item's detail page, I can use e.preventDefault to prevent the ghost click when going back to the list page, but still seeing a ripple there. Is there a workaround or universal fixing to deal this issue?

After several attempts at fixing this problem I came to the conclusion that the only solution is to call preventDefault() within the onTouchTap handler.

It would be relatively easy to extend the react-tap-event-plugin in a way that it always prevents the default when firing the onTouchTap event. Unfortunately, that doesn't help much because it breaks functionality for components that do not use onTouchTap but rather "normal" events like onChange. The MUI Checkbox component is one of these.

Having nearly 300 onTouchTap handlers in my project it was not really an option to manually add preventDefault to all of these - especially because it's rather error-prone.

In search of an automated solution and after a day of different attempts the only solution seems to be monkey-patching React.createElement (!) so that it wraps all onTouchTap props on native elements, adding the preventDefault() call.

Ugly, I know, but it works...

import React from 'react';


let orig = React.createElement;

React.createElement = function(type, props, ...children) {

  // only wrap native elements (string-ish `type`) which have an onTouchTap prop
  if (typeof type === "string" && props && props.onTouchTap) {

    let orig = props.onTouchTap;

    props = {
      ...props,
      onTouchTap: function(e) {
        e.preventDefault();
        return orig.apply(this, arguments);
      }
    }

  }

  return orig.call(this, type, props, ...children);

};

Tested on...

  • Chrome Desktop 58
  • Chrome Mobile 57
  • Crosswalk 23
  • Firefox 52
  • Firefox 53
  • Edge Desktop 38
  • Edge Mobile
  • Internet Explorer 11

We have removed the react-tap-event-plugin dependency on the v1-beta branch. People can rely on the onClick event. We should be good with that issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

revskill10 picture revskill10  ·  3Comments

reflog picture reflog  ·  3Comments

newoga picture newoga  ·  3Comments

sys13 picture sys13  ·  3Comments

zabojad picture zabojad  ·  3Comments