Breaking in combination with Material UI

[linde12]

Hello!

I am trying to migrate a project from that uses React to Preact.

The project uses the Material UI library by CallEmAll and it doesn't seem to play nice with Preact: Some of their components(like EnhancedButton), call event.persist() on a SyntheticEvent. Instead in Preact this event is a FocusEvent and does not have the .persist() method on it. The result is that Material UI buttons(and probably other components) don't work.

Does anyone know how this could be fixed? Since out whole application is built using Material-UI it is crucial for it to work, otherwise we're unable to migrate.

The SyntheticEvent docs doesn't say much about .persist()(see: https://facebook.github.io/react/docs/events.html) so it's difficult to understand why it's needed and how it could be fixed.

Otherwise, so far, everything else in the migration seems to be working. The components i can see without pressing buttons seem to be properly rendered and work the same way as they would with React.

[developit]

Strange, I've never heard of that method. You could try patching it in:

import { options } from 'preact';
options.event = e => {
  e.persist = () => {};
};

[linde12]

Ah, yes, i did something like similar to that and thought it should work because by looking at the React source it seems to have something to do with persisting the event from their event pool. So i thought it shouldn't affect Preact and it seems like it doesn't. However the button is still not clickable. I think this has to do with FloatingActionButton or any EnhancedButton in material-ui not working with onClick. You have to use onTouchTap or something like onMouseUp.

FloatingActionButton is subscribing to the following events:

• onMouseDown
  

• onMouseEnter
  

• onMouseLeave
  

• onMouseUp
  

• onTouchEnd
  

• onTouchStart
  

I wasn't able to find something similar to react-tap-event-plugin for Preact. Do you have any information about this? As in if it was implemented and should work or if that's something that hasn't been done?

Because the current application(with your fix for the .persist() method) almost everything seems to render properly. I have one problem with a component in the application which causes a crash but i'll narrow that down later. The important thing now is getting all the buttons to work.

Thanks for your help so far @developit

[developit]

This is really good, I've been hoping to get proper support for Material UI in for some time now. I think we can definitely add the .persist() modification to preact-compat. As for tap-event-plugin, I've actually never used it - I assume that's what is adding support for onTouchTap? Any pointers here would be good, or if you have a repo where you're trying to get this functioning I can play around with that.

[linde12]

Yeah, this combined with #236 will hopefully make MUI run smooth(er) The application i tried this out on was pretty complex and other than the click-events(touch-tap) everything seemed fine.

As for the tap-plugin, yeah it's to make onTouchTap work. This nicely sums up why. As for how to implement it i haven't really looked at it in detail. I know that react-lite uses Fastclick but i'm not entirely sure if this fixes the issue.

No, i don't have any public repo unfortunately but i can set up a small repo which will have the same symptoms. I'm kind of interested in what react-tap-event-plugin does so i will try to look into this as well!

[linde12]

Alright, so after some snooping around it seems like React listens for events on the document which are then passed to the event system which in turn passes it to plugins which in their turn either return null or a SyntheticEvent. Then i'm guessing React just looks at what type of event that is and calls the callback.

So as i understand it:

If a touchstart and a touchend event are < 10px away from eachother and are fired within a small amount of time(a few ms) then it is, to react, counted as a touchTap and if there is a onTouchTap callback it will be called.

We could try one of these approaches:

• Have a special case for onTouchTap(something like what #237 did to onChange) that remaps onTouchTap -> click event and then apply fastclick(get rid of 300ms delay on phones). This is what react-lite does. I'll just have to verify that it actually works.
• Implement a similar event-system as to what react offers. Maybe we can avoid using synthetic events and keep to the real ones?

[developit]

Definitely have to go with #1 - synthetic events is one of the things that makes React larger than Preact. I think your first option is pretty good, and the radius detection should be pretty quick to implement. I actually believe I have a demo somewhere of this technique as a preact plugin...

[linde12]

The problem with option 1 however is that components that listen to both onTouchTap and onClick events(see this example, used by all MUI buttons)

1. Component listens to onClick, name gets mapped to click event in the _listeners object
2. Component listens to onTouchTap, name gets mapped to click which is already in use so a new listener will not be added
3. Only the onClick callback is called

React is able to differentiate between these because the underlying events are not click but touchstart, touchend and click

I really think we have to implement onTouchTap using touchstart and touchend to make it work well on both phones and computers.

[developit]

Likely will have to use a proxy handler that does the type negotiation when the event is triggered. I agree though, my thinking was that a VNode Hook would work for massaging the props to add those multiple listeners to fire the meta-event.

[linde12]

Ah yeah, i was thinking about something similar with the multiple listeners but i'm not too familiar with the code yet. If you solve it i'd like to have a look :+1:

[developit]

Will do, I'll loop you in on the PR!

[stevewillard]

Any progress on this front? Looking to use preact with material-ui as well.

[linde12]

I have, with a hack, successfully gotten Material UI to work without(afaik) any issues. However i'm hoping developit has a better solution since mine was just a hack to make it work. But to me it seems that we are very close to being compatible with MUI

[developit]

I've been pondering this. I think actually all that is needed is to move this line into the if (opts!==NO_RENDER) block here. Crazy?

[linde12]

@developit

I think you had the wrong issue. This one is about touchtap. Anyway thats what i tried to make refs work but i think i still had problems after doing so because then some refs were not fired. I will come back with results and tag you in the other issue.

Maybe, since there were s few other issues making MUI break, i should just change the title to something about touchtap! :-)

[cia48621793]

@linde12 Do you had any news? I can get AppBar running by monkeypatching event.persist but I can't get through onTouchTap

[developit]

@cia48621793 I just pushed the persist fix, but I'm wondering if adding onTouchTap will still not fix the issue - won't there still be a broken react-dom/lib/ import?

[harshmaur]

Didnt know there is an active discussion going on in this. These are still an issue. https://cl.ly/2u2M1x183E2p

[developit]

@harshmaur the issue is that inject-tap-event-plugin is ridiculously unnecessary with preact-compat, but it relies on 5 internal libs from the react-dom package. We can fake out all the APIs as noops, but it's getting a little silly at that point. Perhaps a replacement for inject-tap-event-plugin would be better, since that's just a single dependency?

[linde12]

@harshmaur @cia48621793 I think it's as @developit says that we need a replacement inject-tap-event-plugin

I don't think it would be difficult for a user to replace that plugin with a preact counterpart, let's say preact-tap

I'm not entirely sure how the API for the event system in preact in terms of hooks looks today but maybe we will need some changes to allow this sort of behavior? What needs to happen is that preact-tap has to listen for touchstart/mousedown & touchend/mouseup and if they're within a certain interval and their positions are close to eachother then it's considered a touchTap so then the plugin would have to trigger a callback in preact

[harshmaur]

@linde12 I am not sure how replacing would work with material-ui as it tightly integrates with onTouchTap events. The source code also contains their own modifiers like onActionTouchTap events and so on. If there is a plugin that gives onTouchTap support then probably, material-ui would not complain.

[linde12]

@harshmaur Material-UI has a peerDependency towards react-tap-event-plugin which provides the onTouchTap functionality. What i mean is that we could provide the same functionality(without SyntheticEvent) with a new plugin because unfortunately react-tap-event-plugin doesn't integrate well with Preact(the event systems are different). So we would have to make our own replacement plugin.

And onActionTouchTap is just using onTouchTap internally AFAIK so just implementing onTouchTap should solve the issues with MUI.

tl;dr MUI isn't dependent on react-tap-event-plugin per se, but it's dependent on a onTouchTap implementation so we could make one that works for Preact.

[harshmaur]

@linde12 Totally agreed with your point.

[developit]

Wouldn't the require('react-tap-event-plugin') throw though? or are they specifying it as a peerDependency just to indicate that you (as the library consumer) need to have installed it?

[harshmaur]

@developit the latter. material-ui works without react-tap-event-plugin but it wants it to be installed separately.

If there is a custom lib that provides for onTouchTap, IMO material-ui should work fine.

BTW, material-ui@next branch is supposed to work without onTouchTap, however the release is not going to happen any soon.

[developit]

I wonder - would it suffice to have a package that could be aliased in for react-tap-event-plugin? It could look something like this:

import { options } from 'preact';
const opts = {
  // # of pixels after which to discount as drag op
  threshold: 5
};

let oldHook = options.vnode;
options.vnode = vnode => {
  let attrs = vnode.attributes;
  if (attrs) {
    let map = {};
    for (let i in attrs) map[i.toLowerCase()] = i;
    if (map.ontouchtap) proxy(attrs, map);
  }
  if (oldHook) oldHook(vnode);
};

function proxy(attrs, map) {
  let start = attrs[map.ontouchstart],
    end = attrs[map.ontouchend],
    tap = attrs[map.ontouchtap],
    coords = e => ({ x: e.touches[0].pageX, y: e.touches[0].pageY });
    down;
  delete attrs[map.ontouchtap];
  attrs[map.ontouchstart || 'onTouchStart'] = e => {
    down = coords(e);
    return start(e);
  };
  attrs[map.ontouchend || 'onTouchEnd'] = e => {
    let up = coords(e),
      ret = end && end(e),
      dist = Math.sqrt( Math.pow(up.x-down.x) +Math.pow(up.y-down.y) );
    if (down && ret!==false && dist<opts.threshold) tap(e);  // or createEvent('touchtap')
    return ret;
  };
}

// in case someone calls inject():
export default (newOpts) => {
  Object.assign(opts, newOpts);
};

[linde12]

@developit

This looks promising! I don't even think you would have to use an alias if unless you don't want to. It's a separate peerDependency so all that would happen is that npm would complain that the dependency isn't satisfied(which it wouldn't be with an alias anyway?)

[harshmaur]

npm complaining wouldnt be a problem if aliasing works.

[developit]

Plus this can be a separate package that might actually be useful beyond getting MUI working - anyone who wants basic FastClick style tap events could use it. Not as elegant as a compositional component, but simple once installed.

[cia48621793]

@developit React-Lite had their solution of touchtap with @fastclick but it won't work with Preact.

[harshmaur]

Hi @developit, I created it into a file and imported it in my webpack config and aliased it.

I get an error like this

ERROR in ./src/index.js
Module not found: Error: Cannot resolve module 'function _default(newOpts) {
  Object.assign(opts, newOpts);
}' in /Users/harshmaur/react/mobilenew/src
 @ ./src/index.js 19:27-60

So I am not sure where I am going wrong.