Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move mouse event disabling on interactive elements to SimpleEventPlugin. Related perf tweak to click handlers. #7642

Merged
merged 8 commits into from
Sep 8, 2016
33 changes: 29 additions & 4 deletions src/renderers/dom/client/eventPlugins/SimpleEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,25 @@ function getDictionaryKey(inst: ReactInstance): string {
return '.' + inst._rootNodeID;
}

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 @@ -217,13 +236,18 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
return null;
}
/* falls through */
case 'topContextMenu':
case 'topDoubleClick':
case 'topMouseDown':
case 'topMouseMove':
case 'topMouseUp':
// Disabled elements should not respond to mouse events
if (shouldPreventMouseEvent(targetInst)) {
return null;
}
/* falls through */
case 'topMouseOut':
case 'topMouseOver':
case 'topMouseUp':
case 'topContextMenu':
EventConstructor = SyntheticMouseEvent;
break;
case 'topDrag':
Expand Down Expand Up @@ -286,7 +310,8 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
// non-interactive elements, which means delegated click listeners do not
// fire. The workaround for this bug involves attaching an empty click
// listener on the target node.
if (registrationName === 'onClick') {
// http://www.quirksmode.org/blog/archives/2010/09/click_event_del.html
if (registrationName === 'onClick' && !isInteractive(inst._tag)) {
var key = getDictionaryKey(inst);
var node = ReactDOMComponentTree.getNodeFromInstance(inst);
if (!onClickListeners[key]) {
Expand All @@ -303,7 +328,7 @@ var SimpleEventPlugin: PluginModule<MouseEvent> = {
inst: ReactInstance,
registrationName: string,
): void {
if (registrationName === 'onClick') {
if (registrationName === 'onClick' && !isInteractive(inst._tag)) {
var key = getDictionaryKey(inst);
onClickListeners[key].remove();
delete onClickListeners[key];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@
'use strict';


describe('DisabledInputUtils', () => {
describe('SimpleEventPlugin', function() {
var React;
var ReactDOM;
var ReactTestUtils;

var elements = ['button', 'input', 'select', 'textarea'];
var onClick = jest.fn();

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

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

Expand All @@ -36,17 +36,31 @@ describe('DisabledInputUtils', () => {
return element;
}

var onClick = jest.fn();
beforeEach(function() {
React = require('React');
ReactDOM = require('ReactDOM');
ReactTestUtils = require('ReactTestUtils');
});

elements.forEach(function(tagName) {
it('A non-interactive tags click when disabled', function() {
var element = (<div onClick={ onClick } />);
expectClickThru(mounted(element));
});

describe(tagName, () => {
it('A non-interactive tags clicks bubble when disabled', function() {
var element = ReactTestUtils.renderIntoDocument(
<div onClick={onClick}><div /></div>
);
var child = ReactDOM.findDOMNode(element).firstChild;

beforeEach(() => {
React = require('React');
ReactDOM = require('ReactDOM');
ReactTestUtils = require('ReactTestUtils');
});
onClick.mockClear();
ReactTestUtils.SimulateNative.click(child);
expect(onClick.mock.calls.length).toBe(1);
});

['button', 'input', 'select', 'textarea'].forEach(function(tagName) {

describe(tagName, function() {

it('should forward clicks when it starts out not disabled', () => {
var element = React.createElement(tagName, {
Expand Down Expand Up @@ -105,4 +119,37 @@ describe('DisabledInputUtils', () => {
});
});
});


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');

ReactDOM.render(<button onClick={ onClick }></button>, container);

var node = container.firstChild;

node.dispatchEvent(new MouseEvent('click'));

expect(onClick.mock.calls.length).toBe(0);
});

it ('adds a local click listener to non-interactive elements', function() {
var container = document.createElement('div');

ReactDOM.render(<div onClick={ onClick }></div>, container);

var node = container.firstChild;

node.dispatchEvent(new MouseEvent('click'));

expect(onClick.mock.calls.length).toBe(0);
});
});
});
50 changes: 0 additions & 50 deletions src/renderers/dom/client/wrappers/DisabledInputUtils.js

This file was deleted.

24 changes: 0 additions & 24 deletions src/renderers/dom/client/wrappers/ReactDOMButton.js

This file was deleted.

3 changes: 1 addition & 2 deletions src/renderers/dom/client/wrappers/ReactDOMInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

'use strict';

var DisabledInputUtils = require('DisabledInputUtils');
var DOMPropertyOperations = require('DOMPropertyOperations');
var LinkedValueUtils = require('LinkedValueUtils');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
Expand Down Expand Up @@ -71,7 +70,7 @@ var ReactDOMInput = {
// in corner cases such as min or max deriving from value, e.g. Issue #7170)
min: undefined,
max: undefined,
}, DisabledInputUtils.getHostProps(inst, props), {
}, props, {
defaultChecked: undefined,
defaultValue: undefined,
value: value != null ? value : inst._wrapperState.initialValue,
Expand Down
3 changes: 1 addition & 2 deletions src/renderers/dom/client/wrappers/ReactDOMSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

'use strict';

var DisabledInputUtils = require('DisabledInputUtils');
var LinkedValueUtils = require('LinkedValueUtils');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
var ReactUpdates = require('ReactUpdates');
Expand Down Expand Up @@ -146,7 +145,7 @@ function updateOptions(inst, multiple, propValue) {
*/
var ReactDOMSelect = {
getHostProps: function(inst, props) {
return Object.assign({}, DisabledInputUtils.getHostProps(inst, props), {
return Object.assign({}, props, {
onChange: inst._wrapperState.onChange,
value: undefined,
});
Expand Down
3 changes: 1 addition & 2 deletions src/renderers/dom/client/wrappers/ReactDOMTextarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

'use strict';

var DisabledInputUtils = require('DisabledInputUtils');
var LinkedValueUtils = require('LinkedValueUtils');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
var ReactUpdates = require('ReactUpdates');
Expand Down Expand Up @@ -56,7 +55,7 @@ var ReactDOMTextarea = {
// to only set the value if/when the value differs from the node value (which would
// completely solve this IE9 bug), but Sebastian+Ben seemed to like this solution.
// The value can be a boolean or object so that's why it's forced to be a string.
var hostProps = Object.assign({}, DisabledInputUtils.getHostProps(inst, props), {
var hostProps = Object.assign({}, props, {
value: undefined,
defaultValue: undefined,
children: '' + inst._wrapperState.initialValue,
Expand Down
8 changes: 0 additions & 8 deletions src/renderers/dom/shared/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ var DOMPropertyOperations = require('DOMPropertyOperations');
var EventPluginHub = require('EventPluginHub');
var EventPluginRegistry = require('EventPluginRegistry');
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
var ReactDOMButton = require('ReactDOMButton');
var ReactDOMComponentFlags = require('ReactDOMComponentFlags');
var ReactDOMComponentTree = require('ReactDOMComponentTree');
var ReactDOMInput = require('ReactDOMInput');
Expand Down Expand Up @@ -542,9 +541,6 @@ ReactDOMComponent.Mixin = {
};
transaction.getReactMountReady().enqueue(trapBubbledEventsLocal, this);
break;
case 'button':
props = ReactDOMButton.getHostProps(this, props, hostParent);
break;
case 'input':
ReactDOMInput.mountWrapper(this, props, hostParent);
props = ReactDOMInput.getHostProps(this, props);
Expand Down Expand Up @@ -887,10 +883,6 @@ ReactDOMComponent.Mixin = {
var nextProps = this._currentElement.props;

switch (this._tag) {
case 'button':
lastProps = ReactDOMButton.getHostProps(this, lastProps);
nextProps = ReactDOMButton.getHostProps(this, nextProps);
break;
case 'input':
lastProps = ReactDOMInput.getHostProps(this, lastProps);
nextProps = ReactDOMInput.getHostProps(this, nextProps);
Expand Down