Move mouse event disabling on interactive elements to SimpleEventPlugin. Related perf tweak to click handlers. (#7642)
* Cull disabled mouse events at plugin level. Remove component level filters * DisabledInputUtils tests are now for SimpleEventPlugin * Add click bubbling test * Add isInteractive function. Use in iOS click exception rules * Invert interactive check in local click listener. Add test coverage * Reduce number of mouse events disabable. Formatting in isIteractive() * Switch isInteractive tag order for alignment * Update formatting of isInteractive method
Nathan Hunzaker committed
Sep 8, 2016 at 14:47 UTC
73c50e7d006856e37a22f3de390cbed859d9dce3
8 files changed
+91
-104
src/renderers/dom/client/eventPlugins/SimpleEventPlugin.js
+29
-4
@@ -146,6 +146,25 @@ function getDictionaryKey(inst: ReactInstance): string {
146
return '.' + inst._rootNodeID;
147
}
148
149
+function isInteractive(tag) {
150
+ return (
151
+ tag === 'button' || tag === 'input' ||
152
+ tag === 'select' || tag === 'textarea'
153
+ );
154
+}
155
+
156
+function shouldPreventMouseEvent(inst) {
157
+ if (inst) {
158
+ var disabled = inst._currentElement && inst._currentElement.props.disabled;
159
+
160
+ if (disabled) {
161
+ return isInteractive(inst._tag);
162
+ }
163
+ }
164
+
165
+ return false;
166
+}
167
+
168
var SimpleEventPlugin: PluginModule<MouseEvent> = {
169
170
eventTypes: eventTypes,
@@ -217,13 +236,18 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
236
return null;
237
}
238
/* falls through */
220
- case 'topContextMenu':
239
case 'topDoubleClick':
240
case 'topMouseDown':
241
case 'topMouseMove':
242
+ case 'topMouseUp':
243
+ // Disabled elements should not respond to mouse events
244
+ if (shouldPreventMouseEvent(targetInst)) {
245
+ return null;
246
+ }
247
+ /* falls through */
248
case 'topMouseOut':
249
case 'topMouseOver':
226
- case 'topMouseUp':
250
+ case 'topContextMenu':
251
EventConstructor = SyntheticMouseEvent;
252
break;
253
case 'topDrag':
@@ -286,7 +310,8 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
310
// non-interactive elements, which means delegated click listeners do not
311
// fire. The workaround for this bug involves attaching an empty click
312
// listener on the target node.
289
- if (registrationName === 'onClick') {
313
+ // http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html
314
+ if (registrationName === 'onClick' && !isInteractive(inst._tag)) {
315
var key = getDictionaryKey(inst);
316
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
317
if (!onClickListeners[key]) {
@@ -303,7 +328,7 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
328
inst: ReactInstance,
329
registrationName: string,
330
): void {
306
- if (registrationName === 'onClick') {
331
+ if (registrationName === 'onClick' && !isInteractive(inst._tag)) {
332
var key = getDictionaryKey(inst);
333
onClickListeners[key].remove();
334
delete onClickListeners[key];
src/renderers/dom/client/eventPlugins/__tests__/SimpleEventPlugin-test.js
renamed
+59
-12
@@ -12,22 +12,22 @@
12
'use strict';
13
14
15
-describe('DisabledInputUtils', () => {
15
+describe('SimpleEventPlugin', function() {
16
var React;
17
var ReactDOM;
18
var ReactTestUtils;
19
20
- var elements = ['button', 'input', 'select', 'textarea'];
20
+ var onClick = jest.fn();
21
22
function expectClickThru(element) {
23
onClick.mockClear();
24
- ReactTestUtils.Simulate.click(ReactDOM.findDOMNode(element));
24
+ ReactTestUtils.SimulateNative.click(ReactDOM.findDOMNode(element));
25
expect(onClick.mock.calls.length).toBe(1);
26
}
27
28
function expectNoClickThru(element) {
29
onClick.mockClear();
30
- ReactTestUtils.Simulate.click(ReactDOM.findDOMNode(element));
30
+ ReactTestUtils.SimulateNative.click(ReactDOM.findDOMNode(element));
31
expect(onClick.mock.calls.length).toBe(0);
32
}
33
@@ -36,17 +36,31 @@ describe('DisabledInputUtils', () => {
36
return element;
37
}
38
39
- var onClick = jest.fn();
39
+ beforeEach(function() {
40
+ React = require('React');
41
+ ReactDOM = require('ReactDOM');
42
+ ReactTestUtils = require('ReactTestUtils');
43
+ });
44
41
- elements.forEach(function(tagName) {
45
+ it('A non-interactive tags click when disabled', function() {
46
+ var element = (<div onClick={ onClick } />);
47
+ expectClickThru(mounted(element));
48
+ });
49
43
- describe(tagName, () => {
50
+ it('A non-interactive tags clicks bubble when disabled', function() {
51
+ var element = ReactTestUtils.renderIntoDocument(
52
+ <div onClick={onClick}><div /></div>
53
+ );
54
+ var child = ReactDOM.findDOMNode(element).firstChild;
55
45
- beforeEach(() => {
46
- React = require('React');
47
- ReactDOM = require('ReactDOM');
48
- ReactTestUtils = require('ReactTestUtils');
49
- });
56
+ onClick.mockClear();
57
+ ReactTestUtils.SimulateNative.click(child);
58
+ expect(onClick.mock.calls.length).toBe(1);
59
+ });
60
+
61
+ ['button', 'input', 'select', 'textarea'].forEach(function(tagName) {
62
+
63
+ describe(tagName, function() {
64
65
it('should forward clicks when it starts out not disabled', () => {
66
var element = React.createElement(tagName, {
@@ -105,4 +119,37 @@ describe('DisabledInputUtils', () => {
119
});
120
});
121
});
122
+
123
+
124
+ describe('iOS bubbling click fix', function() {
125
+ // See http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html
126
+
127
+ beforeEach(function() {
128
+ onClick.mockClear();
129
+ });
130
+
131
+ it ('does not add a local click to interactive elements', function() {
132
+ var container = document.createElement('div');
133
+
134
+ ReactDOM.render(<button onClick={ onClick }></button>, container);
135
+
136
+ var node = container.firstChild;
137
+
138
+ node.dispatchEvent(new MouseEvent('click'));
139
+
140
+ expect(onClick.mock.calls.length).toBe(0);
141
+ });
142
+
143
+ it ('adds a local click listener to non-interactive elements', function() {
144
+ var container = document.createElement('div');
145
+
146
+ ReactDOM.render(<div onClick={ onClick }></div>, container);
147
+
148
+ var node = container.firstChild;
149
+
150
+ node.dispatchEvent(new MouseEvent('click'));
151
+
152
+ expect(onClick.mock.calls.length).toBe(0);
153
+ });
154
+ });
155
});
src/renderers/dom/client/wrappers/DisabledInputUtils.js
deleted
-50
@@ -1,50 +0,0 @@
1
-/**
2
- * Copyright 2013-present, Facebook, Inc.
3
- * All rights reserved.
4
- *
5
- * This source code is licensed under the BSD-style license found in the
6
- * LICENSE file in the root directory of this source tree. An additional grant
7
- * of patent rights can be found in the PATENTS file in the same directory.
8
- *
9
- * @providesModule DisabledInputUtils
10
- */
11
-
12
-'use strict';
13
-
14
-var disableableMouseListenerNames = {
15
- onClick: true,
16
- onDoubleClick: true,
17
- onMouseDown: true,
18
- onMouseMove: true,
19
- onMouseUp: true,
20
-
21
- onClickCapture: true,
22
- onDoubleClickCapture: true,
23
- onMouseDownCapture: true,
24
- onMouseMoveCapture: true,
25
- onMouseUpCapture: true,
26
-};
27
-
28
-/**
29
- * Implements a host component that does not receive mouse events
30
- * when `disabled` is set.
31
- */
32
-var DisabledInputUtils = {
33
- getHostProps: function(inst, props) {
34
- if (!props.disabled) {
35
- return props;
36
- }
37
-
38
- // Copy the props, except the mouse listeners
39
- var hostProps = {};
40
- for (var key in props) {
41
- if (!disableableMouseListenerNames[key] && props.hasOwnProperty(key)) {
42
- hostProps[key] = props[key];
43
- }
44
- }
45
-
46
- return hostProps;
47
- },
48
-};
49
-
50
-module.exports = DisabledInputUtils;
src/renderers/dom/client/wrappers/ReactDOMButton.js
deleted
-24
@@ -1,24 +0,0 @@
1
-/**
2
- * Copyright 2013-present, Facebook, Inc.
3
- * All rights reserved.
4
- *
5
- * This source code is licensed under the BSD-style license found in the
6
- * LICENSE file in the root directory of this source tree. An additional grant
7
- * of patent rights can be found in the PATENTS file in the same directory.
8
- *
9
- * @providesModule ReactDOMButton
10
- */
11
-
12
-'use strict';
13
-
14
-var DisabledInputUtils = require('DisabledInputUtils');
15
-
16
-/**
17
- * Implements a <button> host component that does not receive mouse events
18
- * when `disabled` is set.
19
- */
20
-var ReactDOMButton = {
21
- getHostProps: DisabledInputUtils.getHostProps,
22
-};
23
-
24
-module.exports = ReactDOMButton;
src/renderers/dom/client/wrappers/ReactDOMInput.js
+1
-2
@@ -11,7 +11,6 @@
11
12
'use strict';
13
14
-var DisabledInputUtils = require('DisabledInputUtils');
14
var DOMPropertyOperations = require('DOMPropertyOperations');
15
var LinkedValueUtils = require('LinkedValueUtils');
16
var ReactDOMComponentTree = require('ReactDOMComponentTree');
@@ -71,7 +70,7 @@ var ReactDOMInput = {
70
// in corner cases such as min or max deriving from value, e.g. Issue #7170)
71
min: undefined,
72
max: undefined,
74
- }, DisabledInputUtils.getHostProps(inst, props), {
73
+ }, props, {
74
defaultChecked: undefined,
75
defaultValue: undefined,
76
value: value != null ? value : inst._wrapperState.initialValue,
src/renderers/dom/client/wrappers/ReactDOMSelect.js
+1
-2
@@ -11,7 +11,6 @@
11
12
'use strict';
13
14
-var DisabledInputUtils = require('DisabledInputUtils');
14
var LinkedValueUtils = require('LinkedValueUtils');
15
var ReactDOMComponentTree = require('ReactDOMComponentTree');
16
var ReactUpdates = require('ReactUpdates');
@@ -146,7 +145,7 @@ function updateOptions(inst, multiple, propValue) {
145
*/
146
var ReactDOMSelect = {
147
getHostProps: function(inst, props) {
149
- return Object.assign({}, DisabledInputUtils.getHostProps(inst, props), {
148
+ return Object.assign({}, props, {
149
onChange: inst._wrapperState.onChange,
150
value: undefined,
151
});
src/renderers/dom/client/wrappers/ReactDOMTextarea.js
+1
-2
@@ -11,7 +11,6 @@
11
12
'use strict';
13
14
-var DisabledInputUtils = require('DisabledInputUtils');
14
var LinkedValueUtils = require('LinkedValueUtils');
15
var ReactDOMComponentTree = require('ReactDOMComponentTree');
16
var ReactUpdates = require('ReactUpdates');
@@ -56,7 +55,7 @@ var ReactDOMTextarea = {
55
// to only set the value if/when the value differs from the node value (which would
56
// completely solve this IE9 bug), but Sebastian+Ben seemed to like this solution.
57
// The value can be a boolean or object so that's why it's forced to be a string.
59
- var hostProps = Object.assign({}, DisabledInputUtils.getHostProps(inst, props), {
58
+ var hostProps = Object.assign({}, props, {
59
value: undefined,
60
defaultValue: undefined,
61
children: '' + inst._wrapperState.initialValue,
src/renderers/dom/shared/ReactDOMComponent.js
-8
@@ -22,7 +22,6 @@ var DOMPropertyOperations = require('DOMPropertyOperations');
22
var EventPluginHub = require('EventPluginHub');
23
var EventPluginRegistry = require('EventPluginRegistry');
24
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
25
-var ReactDOMButton = require('ReactDOMButton');
25
var ReactDOMComponentFlags = require('ReactDOMComponentFlags');
26
var ReactDOMComponentTree = require('ReactDOMComponentTree');
27
var ReactDOMInput = require('ReactDOMInput');
@@ -542,9 +541,6 @@ ReactDOMComponent.Mixin = {
541
};
542
transaction.getReactMountReady().enqueue(trapBubbledEventsLocal, this);
543
break;
545
- case 'button':
546
- props = ReactDOMButton.getHostProps(this, props, hostParent);
547
- break;
544
case 'input':
545
ReactDOMInput.mountWrapper(this, props, hostParent);
546
props = ReactDOMInput.getHostProps(this, props);
@@ -887,10 +883,6 @@ ReactDOMComponent.Mixin = {
883
var nextProps = this._currentElement.props;
884
885
switch (this._tag) {
890
- case 'button':
891
- lastProps = ReactDOMButton.getHostProps(this, lastProps);
892
- nextProps = ReactDOMButton.getHostProps(this, nextProps);
893
- break;
886
case 'input':
887
lastProps = ReactDOMInput.getHostProps(this, lastProps);
888
nextProps = ReactDOMInput.getHostProps(this, nextProps);