Skip to content

Commit c7129ce

Browse files
sophiebitssophiebits
authored
Stopgap fix for element disabling (#8387)
Fix for #8308. This is a bad hack -- EventPluginHub.getListener isn't even DOM-specific -- but this works for now and lets us release 15.4.1.
1 parent 6434c37 commit c7129ce

File tree

5 files changed

+101
-45
lines changed

5 files changed

+101
-45
lines changed

‎scripts/fiber/tests-failing.txt

+9-4
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,19 @@ src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js
7171

7272
src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
7373
* A non-interactive tags clicks bubble when disabled
74-
* should not forward clicks when it starts out disabled
74+
* triggers parent captured click events when target is a child of a disabled elements
75+
* should forward clicks when it becomes not disabled
7576
* should not forward clicks when it becomes disabled
76-
* should not forward clicks when it starts out disabled
77+
* should work correctly if the listener is changed
78+
* should forward clicks when it becomes not disabled
7779
* should not forward clicks when it becomes disabled
78-
* should not forward clicks when it starts out disabled
80+
* should work correctly if the listener is changed
81+
* should forward clicks when it becomes not disabled
7982
* should not forward clicks when it becomes disabled
80-
* should not forward clicks when it starts out disabled
83+
* should work correctly if the listener is changed
84+
* should forward clicks when it becomes not disabled
8185
* should not forward clicks when it becomes disabled
86+
* should work correctly if the listener is changed
8287

8388
src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
8489
* should control a value in reentrant events

‎scripts/fiber/tests-passing.txt

+7-8
Original file line numberDiff line numberDiff line change
@@ -696,18 +696,17 @@ src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js
696696

697697
src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
698698
* A non-interactive tags click when disabled
699+
* does not register a click when clicking a child of a disabled element
700+
* triggers click events for children of disabled elements
701+
* triggers captured click events for children of disabled elements
699702
* should forward clicks when it starts out not disabled
700-
* should forward clicks when it becomes not disabled
701-
* should work correctly if the listener is changed
703+
* should not forward clicks when it starts out disabled
702704
* should forward clicks when it starts out not disabled
703-
* should forward clicks when it becomes not disabled
704-
* should work correctly if the listener is changed
705+
* should not forward clicks when it starts out disabled
705706
* should forward clicks when it starts out not disabled
706-
* should forward clicks when it becomes not disabled
707-
* should work correctly if the listener is changed
707+
* should not forward clicks when it starts out disabled
708708
* should forward clicks when it starts out not disabled
709-
* should forward clicks when it becomes not disabled
710-
* should work correctly if the listener is changed
709+
* should not forward clicks when it starts out disabled
711710
* does not add a local click to interactive elements
712711
* adds a local click listener to non-interactive elements
713712

‎src/renderers/dom/shared/eventPlugins/SimpleEventPlugin.js

+1-23
Original file line numberDiff line numberDiff line change
@@ -138,25 +138,6 @@ var topLevelEventsToDispatchConfig: {[key: TopLevelTypes]: DispatchConfig} = {};
138138
topLevelEventsToDispatchConfig[topEvent] = type;
139139
});
140140

141-
function isInteractive(tag) {
142-
return (
143-
tag === 'button' || tag === 'input' ||
144-
tag === 'select' || tag === 'textarea'
145-
);
146-
}
147-
148-
function shouldPreventMouseEvent(inst) {
149-
if (inst) {
150-
var disabled = inst._currentElement && inst._currentElement.props.disabled;
151-
152-
if (disabled) {
153-
return isInteractive(inst._tag);
154-
}
155-
}
156-
157-
return false;
158-
}
159-
160141
var SimpleEventPlugin: PluginModule<MouseEvent> = {
161142

162143
eventTypes: eventTypes,
@@ -232,10 +213,7 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
232213
case 'topMouseDown':
233214
case 'topMouseMove':
234215
case 'topMouseUp':
235-
// Disabled elements should not respond to mouse events
236-
if (shouldPreventMouseEvent(targetInst)) {
237-
return null;
238-
}
216+
// TODO: Disabled elements should not respond to mouse events
239217
/* falls through */
240218
case 'topMouseOut':
241219
case 'topMouseOver':

‎src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js

+45-8
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,14 @@ describe('SimpleEventPlugin', function() {
1717
var ReactDOM;
1818
var ReactTestUtils;
1919

20-
var onClick = jest.fn();
20+
var onClick;
2121

2222
function expectClickThru(element) {
23-
onClick.mockClear();
2423
ReactTestUtils.SimulateNative.click(ReactDOM.findDOMNode(element));
2524
expect(onClick.mock.calls.length).toBe(1);
2625
}
2726

2827
function expectNoClickThru(element) {
29-
onClick.mockClear();
3028
ReactTestUtils.SimulateNative.click(ReactDOM.findDOMNode(element));
3129
expect(onClick.mock.calls.length).toBe(0);
3230
}
@@ -40,6 +38,8 @@ describe('SimpleEventPlugin', function() {
4038
React = require('React');
4139
ReactDOM = require('ReactDOM');
4240
ReactTestUtils = require('ReactTestUtils');
41+
42+
onClick = jest.fn();
4343
});
4444

4545
it('A non-interactive tags click when disabled', function() {
@@ -53,7 +53,48 @@ describe('SimpleEventPlugin', function() {
5353
);
5454
var child = ReactDOM.findDOMNode(element).firstChild;
5555

56-
onClick.mockClear();
56+
ReactTestUtils.SimulateNative.click(child);
57+
expect(onClick.mock.calls.length).toBe(1);
58+
});
59+
60+
it('does not register a click when clicking a child of a disabled element', function() {
61+
var element = ReactTestUtils.renderIntoDocument(
62+
<button onClick={onClick} disabled={true}><span /></button>
63+
);
64+
var child = ReactDOM.findDOMNode(element).querySelector('span');
65+
66+
ReactTestUtils.SimulateNative.click(child);
67+
expect(onClick.mock.calls.length).toBe(0);
68+
});
69+
70+
it('triggers click events for children of disabled elements', function() {
71+
var element = ReactTestUtils.renderIntoDocument(
72+
<button disabled={true}><span onClick={onClick} /></button>
73+
);
74+
var child = ReactDOM.findDOMNode(element).querySelector('span');
75+
76+
ReactTestUtils.SimulateNative.click(child);
77+
expect(onClick.mock.calls.length).toBe(1);
78+
});
79+
80+
it('triggers parent captured click events when target is a child of a disabled elements', function() {
81+
var element = ReactTestUtils.renderIntoDocument(
82+
<div onClickCapture={onClick}>
83+
<button disabled={true}><span /></button>
84+
</div>
85+
);
86+
var child = ReactDOM.findDOMNode(element).querySelector('span');
87+
88+
ReactTestUtils.SimulateNative.click(child);
89+
expect(onClick.mock.calls.length).toBe(1);
90+
});
91+
92+
it('triggers captured click events for children of disabled elements', function() {
93+
var element = ReactTestUtils.renderIntoDocument(
94+
<button disabled={true}><span onClickCapture={onClick} /></button>
95+
);
96+
var child = ReactDOM.findDOMNode(element).querySelector('span');
97+
5798
ReactTestUtils.SimulateNative.click(child);
5899
expect(onClick.mock.calls.length).toBe(1);
59100
});
@@ -124,10 +165,6 @@ describe('SimpleEventPlugin', function() {
124165
describe('iOS bubbling click fix', function() {
125166
// See http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html
126167

127-
beforeEach(function() {
128-
onClick.mockClear();
129-
});
130-
131168
it('does not add a local click to interactive elements', function() {
132169
var container = document.createElement('div');
133170

‎src/renderers/shared/shared/event/EventPluginHub.js

+39-2
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,31 @@ var executeDispatchesAndReleaseTopLevel = function(e) {
4848
return executeDispatchesAndRelease(e, false);
4949
};
5050

51+
function isInteractive(tag) {
52+
return (
53+
tag === 'button' || tag === 'input' ||
54+
tag === 'select' || tag === 'textarea'
55+
);
56+
}
57+
58+
function shouldPreventMouseEvent(name, type, props) {
59+
switch (name) {
60+
case 'onClick':
61+
case 'onClickCapture':
62+
case 'onDoubleClick':
63+
case 'onDoubleClickCapture':
64+
case 'onMouseDown':
65+
case 'onMouseDownCapture':
66+
case 'onMouseMove':
67+
case 'onMouseMoveCapture':
68+
case 'onMouseUp':
69+
case 'onMouseUpCapture':
70+
return !!(props.disabled && isInteractive(type));
71+
default:
72+
return false;
73+
}
74+
}
75+
5176
/**
5277
* This is a unified interface for event plugins to be installed and configured.
5378
*
@@ -97,13 +122,25 @@ var EventPluginHub = {
97122
*/
98123
getListener: function(inst, registrationName) {
99124
var listener;
125+
126+
// TODO: shouldPreventMouseEvent is DOM-specific and definitely should not
127+
// live here; needs to be moved to a better place soon
100128
if (typeof inst.tag === 'number') {
101129
// TODO: This is not safe because we might want the *other* Fiber's
102130
// props depending on which is the current one.
103-
listener = inst.memoizedProps[registrationName];
131+
const props = inst.memoizedProps;
132+
listener = props[registrationName];
133+
if (shouldPreventMouseEvent(registrationName, inst.type, props)) {
134+
return null;
135+
}
104136
} else {
105-
listener = inst._currentElement.props[registrationName];
137+
const props = inst._currentElement.props;
138+
listener = props[registrationName];
139+
if (shouldPreventMouseEvent(registrationName, inst._currentElement.type, props)) {
140+
return null;
141+
}
106142
}
143+
107144
invariant(
108145
!listener || typeof listener === 'function',
109146
'Expected %s listener to be a function, instead got type %s',

0 commit comments

Comments
 (0)