Move Safari onClick hack into ReactDOMComponent (#8189)
This is already where we trap non-bubbling events. We also already branch on the tag type so we know if it is interactive or not. This will let us get rid of the didPutListener and willDeleteListener abstractions. I use a simple onclick = emptyFunction to avoid the need for a bookkeeping map.
Sebastian Markbåge committed
Nov 3, 2016 at 15:26 UTC
b20e3afbf5487d7208eb53bfca6eb48af8fd807c
2 files changed
+31
-45
src/renderers/dom/shared/eventPlugins/SimpleEventPlugin.js
-45
@@ -12,9 +12,7 @@
12
13
'use strict';
14
15
-var EventListener = require('EventListener');
15
var EventPropagators = require('EventPropagators');
17
-var ReactDOMComponentTree = require('ReactDOMComponentTree');
16
var SyntheticAnimationEvent = require('SyntheticAnimationEvent');
17
var SyntheticClipboardEvent = require('SyntheticClipboardEvent');
18
var SyntheticEvent = require('SyntheticEvent');
@@ -27,7 +25,6 @@ var SyntheticTransitionEvent = require('SyntheticTransitionEvent');
25
var SyntheticUIEvent = require('SyntheticUIEvent');
26
var SyntheticWheelEvent = require('SyntheticWheelEvent');
27
30
-var emptyFunction = require('emptyFunction');
28
var getEventCharCode = require('getEventCharCode');
29
var invariant = require('invariant');
30
@@ -141,14 +138,6 @@ var topLevelEventsToDispatchConfig: {[key: TopLevelTypes]: DispatchConfig} = {};
138
topLevelEventsToDispatchConfig[topEvent] = type;
139
});
140
144
-var onClickListeners = {};
145
-
146
-function getDictionaryKey(inst: ReactInstance): string {
147
- // Prevents V8 performance issue:
148
- // https://github.com/facebook/react/pull/7232
149
- return '.' + inst._rootNodeID;
150
-}
151
-
141
function isInteractive(tag) {
142
return (
143
tag === 'button' || tag === 'input' ||
@@ -304,40 +293,6 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
293
return event;
294
},
295
307
- didPutListener: function(
308
- inst: ReactInstance,
309
- registrationName: string,
310
- listener: () => void,
311
- ): void {
312
- // Mobile Safari does not fire properly bubble click events on
313
- // non-interactive elements, which means delegated click listeners do not
314
- // fire. The workaround for this bug involves attaching an empty click
315
- // listener on the target node.
316
- // http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html
317
- if (registrationName === 'onClick' && !isInteractive(inst._tag)) {
318
- var key = getDictionaryKey(inst);
319
- var node = ReactDOMComponentTree.getNodeFromInstance(inst);
320
- if (!onClickListeners[key]) {
321
- onClickListeners[key] = EventListener.listen(
322
- node,
323
- 'click',
324
- emptyFunction
325
- );
326
- }
327
- }
328
- },
329
-
330
- willDeleteListener: function(
331
- inst: ReactInstance,
332
- registrationName: string,
333
- ): void {
334
- if (registrationName === 'onClick' && !isInteractive(inst._tag)) {
335
- var key = getDictionaryKey(inst);
336
- onClickListeners[key].remove();
337
- delete onClickListeners[key];
338
- }
339
- },
340
-
296
};
297
298
module.exports = SimpleEventPlugin;
src/renderers/dom/stack/client/ReactDOMComponent.js
+31
@@ -313,6 +313,20 @@ function trackInputValue() {
313
inputValueTracking.track(this);
314
}
315
316
+function trapClickOnNonInteractiveElement() {
317
+ // Mobile Safari does not fire properly bubble click events on
318
+ // non-interactive elements, which means delegated click listeners do not
319
+ // fire. The workaround for this bug involves attaching an empty click
320
+ // listener on the target node.
321
+ // http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html
322
+ // Just set it using the onclick property so that we don't have to manage any
323
+ // bookkeeping for it. Not sure if we need to clear it when the listener is
324
+ // removed.
325
+ // TODO: Only do this for the relevant Safaris maybe?
326
+ var node = getNode(this);
327
+ node.onclick = emptyFunction;
328
+}
329
+
330
function trapBubbledEventsLocal() {
331
var inst = this;
332
// If a component renders to null or if another component fatals and causes
@@ -711,6 +725,14 @@ ReactDOMComponent.Mixin = {
725
this
726
);
727
break;
728
+ default:
729
+ if (typeof props.onClick === 'function') {
730
+ transaction.getReactMountReady().enqueue(
731
+ trapClickOnNonInteractiveElement,
732
+ this
733
+ );
734
+ }
735
+ break;
736
}
737
738
return mountImage;
@@ -911,6 +933,15 @@ ReactDOMComponent.Mixin = {
933
lastProps = ReactDOMTextarea.getHostProps(this, lastProps);
934
nextProps = ReactDOMTextarea.getHostProps(this, nextProps);
935
break;
936
+ default:
937
+ if (typeof lastProps.onClick !== 'function' &&
938
+ typeof nextProps.onClick === 'function') {
939
+ transaction.getReactMountReady().enqueue(
940
+ trapClickOnNonInteractiveElement,
941
+ this
942
+ );
943
+ }
944
+ break;
945
}
946
947
assertValidProps(this, nextProps);