@samitouri / QOS-React / commits / c7129ce1f0

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.

Ben Alpert committed Nov 22, 2016 at 14:39 UTC c7129ce1f0bba7d04e9d5fce806aaeed40730b17
5 files changed +101 -45
scripts/fiber/tests-failing.txt
+9 -4
@@ -71,14 +71,19 @@ src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js
71
72 src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
73 * 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
76 * 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
79 * 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
82 * 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
85 * should not forward clicks when it becomes disabled
86 +* should work correctly if the listener is changed
87
88 src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
89 * should control a value in reentrant events
scripts/fiber/tests-passing.txt
+7 -8
@@ -696,18 +696,17 @@ src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js
696
697 src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
698 * 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
702 * 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
704 * 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
706 * 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
708 * 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
710 * does not add a local click to interactive elements
711 * adds a local click listener to non-interactive elements
712
src/renderers/dom/shared/eventPlugins/SimpleEventPlugin.js
+1 -23
@@ -138,25 +138,6 @@ var topLevelEventsToDispatchConfig: {[key: TopLevelTypes]: DispatchConfig} = {};
138 topLevelEventsToDispatchConfig[topEvent] = type;
139 });
140
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 -
141 var SimpleEventPlugin: PluginModule<MouseEvent> = {
142
143 eventTypes: eventTypes,
@@ -232,10 +213,7 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
213 case 'topMouseDown':
214 case 'topMouseMove':
215 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
217 /* falls through */
218 case 'topMouseOut':
219 case 'topMouseOver':
src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
+45 -8
@@ -17,16 +17,14 @@ describe('SimpleEventPlugin', function() {
17 var ReactDOM;
18 var ReactTestUtils;
19
20 - var onClick = jest.fn();
20 + var onClick;
21
22 function expectClickThru(element) {
23 - onClick.mockClear();
23 ReactTestUtils.SimulateNative.click(ReactDOM.findDOMNode(element));
24 expect(onClick.mock.calls.length).toBe(1);
25 }
26
27 function expectNoClickThru(element) {
29 - onClick.mockClear();
28 ReactTestUtils.SimulateNative.click(ReactDOM.findDOMNode(element));
29 expect(onClick.mock.calls.length).toBe(0);
30 }
@@ -40,6 +38,8 @@ describe('SimpleEventPlugin', function() {
38 React = require('React');
39 ReactDOM = require('ReactDOM');
40 ReactTestUtils = require('ReactTestUtils');
41 +
42 + onClick = jest.fn();
43 });
44
45 it('A non-interactive tags click when disabled', function() {
@@ -53,7 +53,48 @@ describe('SimpleEventPlugin', function() {
53 );
54 var child = ReactDOM.findDOMNode(element).firstChild;
55
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 +
98 ReactTestUtils.SimulateNative.click(child);
99 expect(onClick.mock.calls.length).toBe(1);
100 });
@@ -124,10 +165,6 @@ describe('SimpleEventPlugin', function() {
165 describe('iOS bubbling click fix', function() {
166 // See http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html
167
127 - beforeEach(function() {
128 - onClick.mockClear();
129 - });
130 -
168 it('does not add a local click to interactive elements', function() {
169 var container = document.createElement('div');
170
src/renderers/shared/shared/event/EventPluginHub.js
+39 -2
@@ -48,6 +48,31 @@ var executeDispatchesAndReleaseTopLevel = function(e) {
48 return executeDispatchesAndRelease(e, false);
49 };
50
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 +
76 /**
77 * This is a unified interface for event plugins to be installed and configured.
78 *
@@ -97,13 +122,25 @@ var EventPluginHub = {
122 */
123 getListener: function(inst, registrationName) {
124 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
128 if (typeof inst.tag === 'number') {
129 // TODO: This is not safe because we might want the *other* Fiber's
130 // 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 + }
136 } 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 + }
142 }
143 +
144 invariant(
145 !listener || typeof listener === 'function',
146 'Expected %s listener to be a function, instead got type %s',