Skip to content

Commit

Permalink
Stopgap fix for element disabling (facebook#8387)
Browse files Browse the repository at this point in the history
Fix for facebook#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.
  • Loading branch information
sophiebits authored and acusti committed Mar 15, 2017
1 parent 7ea95a3 commit 0dc8a18
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 45 deletions.
13 changes: 9 additions & 4 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,19 @@ src/renderers/dom/shared/eventPlugins/__tests__/ChangeEventPlugin-test.js

src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
* A non-interactive tags clicks bubble when disabled
* should not forward clicks when it starts out disabled
* triggers parent captured click events when target is a child of a disabled elements
* should forward clicks when it becomes not disabled
* should not forward clicks when it becomes disabled
* should not forward clicks when it starts out disabled
* should work correctly if the listener is changed
* should forward clicks when it becomes not disabled
* should not forward clicks when it becomes disabled
* should not forward clicks when it starts out disabled
* should work correctly if the listener is changed
* should forward clicks when it becomes not disabled
* should not forward clicks when it becomes disabled
* should not forward clicks when it starts out disabled
* should work correctly if the listener is changed
* should forward clicks when it becomes not disabled
* should not forward clicks when it becomes disabled
* should work correctly if the listener is changed

src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should control a value in reentrant events
Expand Down
15 changes: 7 additions & 8 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -696,18 +696,17 @@ src/renderers/dom/shared/eventPlugins/__tests__/SelectEventPlugin-test.js

src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js
* A non-interactive tags click when disabled
* does not register a click when clicking a child of a disabled element
* triggers click events for children of disabled elements
* triggers captured click events for children of disabled elements
* should forward clicks when it starts out not disabled
* should forward clicks when it becomes not disabled
* should work correctly if the listener is changed
* should not forward clicks when it starts out disabled
* should forward clicks when it starts out not disabled
* should forward clicks when it becomes not disabled
* should work correctly if the listener is changed
* should not forward clicks when it starts out disabled
* should forward clicks when it starts out not disabled
* should forward clicks when it becomes not disabled
* should work correctly if the listener is changed
* should not forward clicks when it starts out disabled
* should forward clicks when it starts out not disabled
* should forward clicks when it becomes not disabled
* should work correctly if the listener is changed
* should not forward clicks when it starts out disabled
* does not add a local click to interactive elements
* adds a local click listener to non-interactive elements

Expand Down
24 changes: 1 addition & 23 deletions src/renderers/dom/shared/eventPlugins/SimpleEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,25 +138,6 @@ var topLevelEventsToDispatchConfig: {[key: TopLevelTypes]: DispatchConfig} = {};
topLevelEventsToDispatchConfig[topEvent] = type;
});

function isInteractive(tag) {
return (
tag === 'button' || tag === 'input' ||
tag === 'select' || tag === 'textarea'
);
}

function shouldPreventMouseEvent(inst) {
if (inst) {
var disabled = inst._currentElement && inst._currentElement.props.disabled;

if (disabled) {
return isInteractive(inst._tag);
}
}

return false;
}

var SimpleEventPlugin: PluginModule<MouseEvent> = {

eventTypes: eventTypes,
Expand Down Expand Up @@ -232,10 +213,7 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
case 'topMouseDown':
case 'topMouseMove':
case 'topMouseUp':
// Disabled elements should not respond to mouse events
if (shouldPreventMouseEvent(targetInst)) {
return null;
}
// TODO: Disabled elements should not respond to mouse events
/* falls through */
case 'topMouseOut':
case 'topMouseOver':
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,14 @@ describe('SimpleEventPlugin', function() {
var ReactDOM;
var ReactTestUtils;

var onClick = jest.fn();
var onClick;

function expectClickThru(element) {
onClick.mockClear();
ReactTestUtils.SimulateNative.click(ReactDOM.findDOMNode(element));
expect(onClick.mock.calls.length).toBe(1);
}

function expectNoClickThru(element) {
onClick.mockClear();
ReactTestUtils.SimulateNative.click(ReactDOM.findDOMNode(element));
expect(onClick.mock.calls.length).toBe(0);
}
Expand All @@ -40,6 +38,8 @@ describe('SimpleEventPlugin', function() {
React = require('React');
ReactDOM = require('ReactDOM');
ReactTestUtils = require('ReactTestUtils');

onClick = jest.fn();
});

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

onClick.mockClear();
ReactTestUtils.SimulateNative.click(child);
expect(onClick.mock.calls.length).toBe(1);
});

it('does not register a click when clicking a child of a disabled element', function() {
var element = ReactTestUtils.renderIntoDocument(
<button onClick={onClick} disabled={true}><span /></button>
);
var child = ReactDOM.findDOMNode(element).querySelector('span');

ReactTestUtils.SimulateNative.click(child);
expect(onClick.mock.calls.length).toBe(0);
});

it('triggers click events for children of disabled elements', function() {
var element = ReactTestUtils.renderIntoDocument(
<button disabled={true}><span onClick={onClick} /></button>
);
var child = ReactDOM.findDOMNode(element).querySelector('span');

ReactTestUtils.SimulateNative.click(child);
expect(onClick.mock.calls.length).toBe(1);
});

it('triggers parent captured click events when target is a child of a disabled elements', function() {
var element = ReactTestUtils.renderIntoDocument(
<div onClickCapture={onClick}>
<button disabled={true}><span /></button>
</div>
);
var child = ReactDOM.findDOMNode(element).querySelector('span');

ReactTestUtils.SimulateNative.click(child);
expect(onClick.mock.calls.length).toBe(1);
});

it('triggers captured click events for children of disabled elements', function() {
var element = ReactTestUtils.renderIntoDocument(
<button disabled={true}><span onClickCapture={onClick} /></button>
);
var child = ReactDOM.findDOMNode(element).querySelector('span');

ReactTestUtils.SimulateNative.click(child);
expect(onClick.mock.calls.length).toBe(1);
});
Expand Down Expand Up @@ -124,10 +165,6 @@ describe('SimpleEventPlugin', function() {
describe('iOS bubbling click fix', function() {
// See http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html

beforeEach(function() {
onClick.mockClear();
});

it('does not add a local click to interactive elements', function() {
var container = document.createElement('div');

Expand Down
41 changes: 39 additions & 2 deletions src/renderers/shared/shared/event/EventPluginHub.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,31 @@ var executeDispatchesAndReleaseTopLevel = function(e) {
return executeDispatchesAndRelease(e, false);
};

function isInteractive(tag) {
return (
tag === 'button' || tag === 'input' ||
tag === 'select' || tag === 'textarea'
);
}

function shouldPreventMouseEvent(name, type, props) {
switch (name) {
case 'onClick':
case 'onClickCapture':
case 'onDoubleClick':
case 'onDoubleClickCapture':
case 'onMouseDown':
case 'onMouseDownCapture':
case 'onMouseMove':
case 'onMouseMoveCapture':
case 'onMouseUp':
case 'onMouseUpCapture':
return !!(props.disabled && isInteractive(type));
default:
return false;
}
}

/**
* This is a unified interface for event plugins to be installed and configured.
*
Expand Down Expand Up @@ -97,13 +122,25 @@ var EventPluginHub = {
*/
getListener: function(inst, registrationName) {
var listener;

// TODO: shouldPreventMouseEvent is DOM-specific and definitely should not
// live here; needs to be moved to a better place soon
if (typeof inst.tag === 'number') {
// TODO: This is not safe because we might want the *other* Fiber's
// props depending on which is the current one.
listener = inst.memoizedProps[registrationName];
const props = inst.memoizedProps;
listener = props[registrationName];
if (shouldPreventMouseEvent(registrationName, inst.type, props)) {
return null;
}
} else {
listener = inst._currentElement.props[registrationName];
const props = inst._currentElement.props;
listener = props[registrationName];
if (shouldPreventMouseEvent(registrationName, inst._currentElement.type, props)) {
return null;
}
}

invariant(
!listener || typeof listener === 'function',
'Expected %s listener to be a function, instead got type %s',
Expand Down

0 comments on commit 0dc8a18

Please sign in to comment.