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

Trigger event on dom object when activated #164

Merged
merged 4 commits into from
Apr 4, 2015
Merged

Trigger event on dom object when activated #164

merged 4 commits into from
Apr 4, 2015

Conversation

mdeboer
Copy link
Contributor

@mdeboer mdeboer commented Mar 7, 2015

Instead of only having a global callback that has the activated element as argument, it's nice to have an event triggered on the element itself. Should avoid cluttering the global callback function with if's.

Also added a jasmine test to check it's behaviour.

emitEvent: (elem, event) ->
if elem.dispatchEvent? # W3C DOM
elem.dispatchEvent(event)
else if elem[event]?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not use else if event of elem here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't that just be confusing? Not sure if it's worth debating and if it helps consistency then it's fine by me. I'll make some changes then.

@attilaolah
Copy link
Collaborator

I like this, @mdeboer. Please have a look at my comments when you have some time, and I'll merge this.

resetAnimation: (event) ->
if event.type.toLowerCase().indexOf('animationend') >= 0
target = event.target || event.srcElement
target.className = target.className.replace(config.animateClass, '').trim()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will leave double spaces if the animateClass is not the first or last class. But then again, who cares. 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not the behaviour what I have seen but I have only tested briefly.

@attilaolah
Copy link
Collaborator

Thanks @mdeboer, I'll merge this tonight.

@mdeboer
Copy link
Contributor Author

mdeboer commented Mar 23, 2015

Cheers mate, appreciate it! I'll fix the things you pointed out when I have the time.

@mdeboer
Copy link
Contributor Author

mdeboer commented Apr 3, 2015

Fixed the things you pointed out @attilaolah 👍 When will it be merged?

@attilaolah
Copy link
Collaborator

Sorry I completely forgot about this.

LGTM, merging.

attilaolah added a commit that referenced this pull request Apr 4, 2015
Trigger event on dom object when activated
@attilaolah attilaolah merged commit d8c2f97 into matthieua:master Apr 4, 2015
@webdizajner
Copy link

How to implement animationend?

@yangzhaohui89
Copy link

config.animateClass undifined

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.

4 participants