Skip to content

Commit

Permalink
Fixes problem with sub component listeners being triggered by parent …
Browse files Browse the repository at this point in the history
…components

If an both a component and its child component define inline listeners for the same event type and using the same listener function names, these could conflict with each other. This adds code to make sure that inline events won't be triggered for parent components.
  • Loading branch information
mairatma committed Jun 9, 2015
1 parent de6132b commit 24149af
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
20 changes: 19 additions & 1 deletion src/component/EventsCollector.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class EventsCollector extends Disposable {

if (!this.eventHandles_[selector]) {
var fn = this.getListenerFn_(fnName);
this.eventHandles_[selector] = this.component_.delegate(eventType, selector, fn);
this.eventHandles_[selector] = this.component_.delegate(eventType, selector, this.onEvent_.bind(this, fn));
}
}

Expand Down Expand Up @@ -157,6 +157,24 @@ class EventsCollector extends Disposable {
fnComponent = fnComponent || this.component_;
return fnComponent[fnName].bind(fnComponent);
}

/**
* Fires when an event that was registered by this collector is triggered. Makes
* sure that the event was meant for this component and calls the appropriate
* listener function for it.
* @param {!function(!Object)} fn
* @param {!Object} event
* @return {*} The return value of the call to the listener function, or undefined
* if no function was called.
* @protected
*/
onEvent_(fn, event) {
// This check prevents parent components from handling their child inline listeners.
if (!event.handledByComponent || event.handledByComponent === this.component_) {
event.handledByComponent = this.component_;
return fn(event);
}
}
}

export default EventsCollector;
27 changes: 27 additions & 0 deletions test/src/component/EventsCollector.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,33 @@ describe('EventsCollector', function() {
assert.strictEqual(2, custom.handleClick.callCount);
});

it('should not trigger sub component listeners on parent components', function() {
var SubComponent = createCustomComponent('<div data-onclick="handleClick"></div>');
SubComponent.prototype.handleClick = sinon.stub();

var child;
var CustomComponent = createCustomComponent();
CustomComponent.prototype.renderInternal = function() {
dom.append(this.element, '<div data-onclick="handleClick"></div>');
child = new SubComponent().render(this.element);
};
CustomComponent.prototype.handleClick = sinon.stub();

var custom = new CustomComponent().render();
var collector = new EventsCollector(custom);
collector.attachListeners(custom.element.innerHTML, 'group');
var childCollector = new EventsCollector(child);
childCollector.attachListeners(child.element.innerHTML, 'group');

dom.triggerEvent(child.element.childNodes[0], 'click');
assert.strictEqual(0, custom.handleClick.callCount);
assert.strictEqual(1, child.handleClick.callCount);

dom.triggerEvent(custom.element.childNodes[0], 'click');
assert.strictEqual(1, custom.handleClick.callCount);
assert.strictEqual(1, child.handleClick.callCount);
});

it('should detach listeners that are unused', function() {
var CustomComponent = createCustomComponent(
'<div data-onclick="handleClick" data-onkeydown="handleKeyDown"></div>'
Expand Down

0 comments on commit 24149af

Please sign in to comment.