[Core] Implements warning for 'react-tap-event-plugin' if not registered #3064
Conversation
|
Wow! Great job 👍 👍 👍 👍 I didn; know it was possible 😅 😅 |
|
@oliviertassinari @alitaheri @mbrookes This is probably low risk, but take a look at it. Also let me know if there's a better place to run the warning other than Okay, bed time. 😪 |
Neither did I! But I read the tap-event-plugin's source, then React's source, then soon enough it was done 😄 |
|
@newoga Thanks for starting this 😄! |
| @@ -12,6 +13,8 @@ import zIndex from './zIndex'; | |||
| * theme will be deeply merged with the second argument. | |||
| */ | |||
| export default function getMuiTheme(baseTheme, muiTheme) { | |||
| if (process.env.NODE_ENV !== 'production') checkTapEventPlugin(); | |||
There was a problem hiding this comment.
I have tested this on my project. Unfortunately, getMuiTheme is called before injectTapEventPlugin.
That won't always work.
What about moving the check to the function that applies the style? We will be sure that the check is made during the rendering.
oliviertassinari
added
PR: needs revision
|
@newoga There is also one tiny edge case. When 2 instances of react are loaded and one of them reside in the |
|
stupid |
| The 'react-tap-event-plugin' has not been registered! | ||
|
|
||
| Material-UI requires that you load 'react-tap-event-plugin' and call 'injectTapEventPlugin()' for | ||
| components receive touch events and work properly. |
There was a problem hiding this comment.
"to receive". Edit: also it isn't just touch events, right?
|
Yeah, sorry @newoga - I felt bad posting that suggestion right after you've made this PR! 😰 |
Never feel bad about proposing an better solution (at least to me)! 😆 We're all here for what's best, I really mean that 🚀. If anything, feel bad for my table! (╯°□°)╯︵ ┻━┻ I'll put my thoughts on #3068 there! |
LOL 😆 |
Almost gave me a heart attack 😆 😆 😆 |
|
@mbrookes @oliviertassinari @alitaheri I'll leave this branch open but close this PR while we consider #3079 (and #3068) |
|
Thank you. I was gonna rush here to stop you from deleting the branch :D :D :D |
Closes #2553.
This check should help a lot of new material-ui developers catch the infamous "forgot to call
injectTapEventPlugin()" issue.Probably related to a number of issues.