-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
added view and event object to parameters for trigger function #402
Conversation
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 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 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. |
Hi Derick, I thought you might have an encapsulation concern for the Passing 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. |
do you have an example where this is the case? it seems like an encapsulation problem to me, without having a good example |
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 |
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 |
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 |
Awesome! This works for our use case. Thanks! |
…iew, args.model, args.collection. fixes marionettejs#402
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: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:
The parent view then listens for the event from two distinct subviews with a function like:
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.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.