Skip to content

Updating event handlers to hooks#4

Open
visheshjoshi wants to merge 1 commit into
idobata:masterfrom
visheshjoshi:updateHooks
Open

Updating event handlers to hooks#4
visheshjoshi wants to merge 1 commit into
idobata:masterfrom
visheshjoshi:updateHooks

Conversation

@visheshjoshi

Copy link
Copy Markdown

The handlers
on('activate') and on('deactivate') don't work. Updating them to route hooks instead.

@ursm

ursm commented Mar 17, 2015

Copy link
Copy Markdown
Member

Hi,

Which Ember version do you use? ember-mousetrap uses "Activate and Deactivate Events" that introduced in Ember 1.9.
http://emberjs.com/blog/2014/12/08/ember-1-9-0-released.html#toc_activate-and-deactivate-events
I think on() is more desirable than defining a method because it isn't necessary to call _super() explicitly.

@visheshjoshi

Copy link
Copy Markdown
Author

I currently use 1.8.1 but am planning to upgrade soon. I think your point is pretty valid, but maybe a couple of things can be considered:

  1. Calling hooks gives backward compatibility.
  2. For libraries such as mousetrap, having multiple sources of observation (events) can be tricky to manage, and it might be a good idea to do bindings in hooks which gets called once in a render cycle.

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