gistfile1.diff

From 53d61784fafa380fb051b908ac946502a31e0b7d Mon Sep 17 00:00:00 2001
From: Jo Liss <[email protected]>
Date: Thu, 31 May 2012 23:23:53 +0200
Subject: [PATCH] Call preventDefault and stopPropagation on events handled
 through {{action}}

The idea is that we'd like to be able to use arbitrary view methods as
action handlers. As it is, they need to `return false` or call
`e.preventDefault(); e.stopPropagation();` to be usable as handlers.

For the (presumably few) cases where propagation or default handling is
desired, we can still register our own handlers/delegates with jQuery,
or use event handling on views, which does not have automatic
preventDefault/stopPropagation.
---
 packages/ember-handlebars/lib/helpers/action.js    |   10 ++++++++++
 .../ember-handlebars/tests/helpers/action_test.js  |    4 ++--
 .../ember-views/lib/system/event_dispatcher.js     |    5 ++++-
 packages/ember-views/lib/views/states/in_dom.js    |    2 ++
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/packages/ember-handlebars/lib/helpers/action.js b/packages/ember-handlebars/lib/helpers/action.js
index 6f20f48..c3c109c 100644
--- a/packages/ember-handlebars/lib/helpers/action.js
+++ b/packages/ember-handlebars/lib/helpers/action.js
@@ -67,8 +67,18 @@ ActionHelper.registerAction = function(actionName, eventName, target, view, cont
   `jQuery.Event` object as its argument. The `jQuery.Event` object will be extended to include
   a `view` property that is set to the original view interacted with (in this case the `aView` object).
 
+  ### Event propagation
+
+  Events triggered through the action helper will automatically have
+  `.preventDefault()` and `.stopPropagation()` called on them. You do not need
+  to do so in your event handlers, and you do not need to return `false`.
+
+  If you need the default handler to trigger, or if you need propagation,
+  you should either register your own event handler, or use event methods on
+  your view class.
 
   ### Specifying an Action Target
+
   A `target` option can be provided to change which object will receive the method call. This option must be
   a string representing a path to an object:
 
diff --git a/packages/ember-handlebars/tests/helpers/action_test.js b/packages/ember-handlebars/tests/helpers/action_test.js
index 152cee8..856ef75 100644
--- a/packages/ember-handlebars/tests/helpers/action_test.js
+++ b/packages/ember-handlebars/tests/helpers/action_test.js
@@ -250,7 +250,7 @@ test("should properly capture events on child elements of a container with an ac
   ok(eventHandlerWasCalled, "Event on a child element triggered the action of it's parent");
 });
 
-test("should allow bubbling of events from action helper to original parent event", function() {
+test("events from action helper should not bubble to original parent event", function() {
   var eventHandlerWasCalled = false,
       originalEventHandlerWasCalled = false;
 
@@ -264,7 +264,7 @@ test("should allow bubbling of events from action helper to original parent even
 
   view.$('a').trigger('click');
 
-  ok(eventHandlerWasCalled && originalEventHandlerWasCalled, "Both event handlers were called");
+  ok(eventHandlerWasCalled && !originalEventHandlerWasCalled, "Only the {{action}} event handler was called");
 });
 
 test("should be compatible with sending events to a state manager", function() {
diff --git a/packages/ember-views/lib/system/event_dispatcher.js b/packages/ember-views/lib/system/event_dispatcher.js
index e7ffbbd..a03a1b3 100644
--- a/packages/ember-views/lib/system/event_dispatcher.js
+++ b/packages/ember-views/lib/system/event_dispatcher.js
@@ -139,6 +139,8 @@ Ember.EventDispatcher = Ember.Object.extend(
           handler  = action.handler;
 
       if (action.eventName === eventName) {
+        evt.preventDefault();
+        evt.stopPropagation();
         return handler(evt);
       }
     });
@@ -164,8 +166,9 @@ Ember.EventDispatcher = Ember.Object.extend(
 
     var handler = object[eventName];
     if (Ember.typeOf(handler) === 'function') {
-      result = handler.call(object, evt, view);
+      evt.preventDefault();
       evt.stopPropagation();
+      result = handler.call(object, evt, view);
     }
     else {
       result = this._bubbleEvent(view, evt, eventName);
diff --git a/packages/ember-views/lib/views/states/in_dom.js b/packages/ember-views/lib/views/states/in_dom.js
index be1c82a..f9a8581 100644
--- a/packages/ember-views/lib/views/states/in_dom.js
+++ b/packages/ember-views/lib/views/states/in_dom.js
@@ -72,6 +72,8 @@ Ember.View.states.hasElement = {
   handleEvent: function(view, eventName, evt) {
     var handler = view[eventName];
     if (Ember.typeOf(handler) === 'function') {
+      // Handler should be able to re-dispatch events, so we don't
+      // preventDefault or stopPropagation.
       return handler.call(view, evt);
     } else {
       return true; // continue event propagation
-- 
1.7.9.5