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

Fix multiple events subscription #1529

Merged
merged 1 commit into from
Sep 11, 2017
Merged

Conversation

tadatuta
Copy link
Member

This PR also addresses #1526 for bem-core version without jQuery.

@tadatuta tadatuta requested a review from veged September 10, 2017 20:58
domNode.addEventListener(event, handler, false);
});
}
typeof e === 'string'?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to do this in very first lines of this function — just check if e is a string and contains spaces and do forEach of this.on (we do similar https://github.com/bem/bem-core/blob/v4/common.blocks/i-bem-dom/__events/i-bem-dom__events.js#L226)

}
}.bind(this);

typeof e === 'string'?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same stuff like with on — we can make smaller diff

@tadatuta tadatuta force-pushed the fixEvents@issues/1507 branch 3 times, most recently from dcd772d to 248e7cb Compare September 11, 2017 12:09
@tadatuta
Copy link
Member Author

@veged ⬆️

@@ -52,6 +52,14 @@ var undef,
* @returns {EventManager} this
*/
on : function(e, data, fn, _fnCtx, _isOnce) {
if(typeof e === 'string' && e.indexOf(' ') > -1) {
e.split(' ').forEach(function(evnt) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

evEnt

@tadatuta tadatuta merged commit 1fa037a into issues/1507@v5 Sep 11, 2017
@tadatuta tadatuta deleted the fixEvents@issues/1507 branch September 11, 2017 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants