added view and event object to parameters for trigger function

[dgbeck]

Wow. It has been a very long road to isolate this change but also a very good learning process.

This PR contains a simple change that adds two additional parameters to the function called as a result of the configuration of the triggers hash. Namely:

1. The view object that triggered the event. Sometimes you are "listening" to a triggered event from another view, and need to know which view triggered the event. For example, a parent views wants to listen to triggered events form multiple subviews. The trigger hash for each subview as the form:

   triggers : {
       "click div.name" : "click:name"
   }

   The parent view then listens for the event from two distinct subviews with a function like:

   this.subviews.A.bind( "click:name", this._subview_onNameClicked, this );
   this.subviews.B.bind( "click:name", this._subview_onNameClicked, this );

   The same function can be used to listen to both subviews. But if the subview is not passed as a parameter to this._subview_onNameClicked, there is no way to determine which subview triggered the event.

2. The second parameter that was added in this PR is the jQuery event object. Don't have a specific use case for this one yet but seems like it is bound to come up, since the nature of the event itself will also at times be of interest to the view listening to the triggered event.

[mxriverlynn]

there's been a lot of discussion about this, and several PR's for this same thing that have been shut down already.

the intent of the triggers is to be the most simple use case possible, when a simple DOM element click just needs to forward out to a controller or mediator object. specifically, handing the e arg of the event out to another object would break encapsulation and cause a spaghetti mess in the code.

for example:

SomeView = Marionette.ItemView.extend({
  // ...

  triggers: {
    "click .foo": "foo:bar"
  }
});

DoStuff = Marionette.Controller.extend({

  showIt: function(){
    var v = new SomeView();

    v.on("foo:bar", function(e){
      // if we allow "e" to come through this event, we have broken the encapsulation of views by allowing the DOM
      // concerns to leak out in to the controller / mediator concerns. this is a bad idea. i tightly couples this controller
      // object to the DOM. 
    }

  }

});

regarding passing of the view... this might be ok... but i still think it couples things together too much. it would make more sense to me, to pass an args object back that includes the model and/or collection from the view:

SomeView = Marionette.ItemView.extend({
  // ...

  triggers: {
    "click .foo": "foo:bar"
  }
});

DoStuff = Marionette.Controller.extend({

  showIt: function(){
    var v = new SomeView();

    v.on("foo:bar", function(args){

      args.model; // <= the `model` from the view
      args.collection; // <= the `collection` from the view

    }

  }

});

Would this provide the capabilities you need? It would certainly make my life easier, as I often end up doing manual event handlers, just so I can get the view's model through an event.

[dgbeck]

Hi Derick,

I thought you might have an encapsulation concern for the e argument. That makes sense, best to leave e out.

Passing args with the model and collection would work for our use case, although I would have to make some tweaks.

It is not apparent to me though why you see that approach as less coupled than just passing the view? On the other side of the argument, it is more intuitive that the view itself is passed, as that is the object that triggered the event. Also in some cases there may not be a model or collection associated with the view, but the view itself might contain logic that is of interest to the listener.

[mxriverlynn]

the view itself might contain logic that is of interest to the listener

do you have an example where this is the case? it seems like an encapsulation problem to me, without having a good example

[dgbeck]

All views do not have either a model or collection. Some views do not represent objects that are persisted on the server, or have attributes that need to be hooked to change events, etc., yet they still have properties and / or a state on the client side that the listener may want to inquire about or manipulate. Also, somes views that do have an associated model contain state and logic that is purely UI related and therefore may not be appropriate to put in the associated model.

Following from the example at the top of this ticket, let's say the subviews are represent items in a list that are expandable or contractable. The nodes themselves may be represented by models but the fact that they are currently expanded or contracted is just UI related - it is not persisted to the server. The "isExpanded" property that tracks their state in this regard is part of the view, not the model. The subviews have public methods expand and collapse, so that other views may expand and collapse them. The parent view wants to expand the subview when it gets the click:name event, but it can only do so with a reference to the subview object itself.

[mxriverlynn]

good example.

i'll work on this soon and see what API makes sense to send the view and/or model/collection through the trigger args

[mxriverlynn]

i decided to use an args object for this...

view.on("some:trigger", function(args){

  args.view;
  args.model;
  args.collection;

});

may not be the best option, but it was easy to do, kept things flexible, and avoided null parameters when the view doesn't have a model or collection.

this is done in the dev branch

[dgbeck]

Awesome! This works for our use case.

Thanks!