Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Marionette triggerMethod - event firing order - should take an option #518

Closed
brian-mann opened this Issue Mar 16, 2013 · 7 comments

Comments

Projects
None yet
7 participants

triggerMethod will first trigger the event colon separated first.

this.triggerMethod("the:event") would fire the:event on the instance first.

Then it would call the method onTheEvent on the instance second.

I actually believe that there should be a secondary options argument that allows you to change the order, and instead fire the onTheEvent first, and the:event second.
My reasoning: application specific, but definitely comes up. There are times where you want the view to process something before anything wired up/listening to the view instance fires. Working around this means firing an additional event from the view method after its finished with the event.

Owner

derickbailey commented Mar 19, 2013

Do you have a more concrete example of when you would want this? I'm having a hard time seeing why you would want to depend on the order in which the method vs event get called. Is there a better place where an abstraction or other method override could be introduced, to solve the problem?

As it stands, this change would create a problem with the API. Currently, it allows you to pass an arbitrary number of arguments through the event. Do you have an idea of how to create an API that would allow this, and still handle the scenario of flipping the order?

Contributor

fantactuka commented Mar 29, 2013

I'd say it makes sense to call instance's method first by default, even without option to switch the order. Instance should take care of its own duties first, and then let other listeners know that something happened.

Let's say you have view's render event. IMO it's obvious that view should do its onRender first, and than let (e.g. to controller) that it's rendered. It looks like current behaviour was the reason Marionette includes events sequenced events (item:render, item:rendered).

I agree, instance's method first by default, or at list and option to change it.

bruth commented Jun 27, 2013

Agreed, the order definitely should be flipped. The concrete use case is making parent classes behaviors more transparent to subclasses without have to call super on a method override:

var ParentView = Marionette.ItemView.extend({
    initialize: function() {
        this.on('render', function() {
            // relies on something in the UI rendered by the child class..
        });
    }
});

var ChildView = ParentView.extend({
    onRender: function() {
        // render/customize additional things..
    }
});

This does not work in the current implementation since the event is triggered first before onRender is called.

Member

cobbweb commented Jan 14, 2014

👍 Make triggerMethod call the method first, then fire the event. ping @samccone

@jasonLaster jasonLaster added feature major and removed quick-fix labels Apr 5, 2014

@jasonLaster jasonLaster added this to the v2.0.0 milestone Apr 5, 2014

Owner

jasonLaster commented Apr 5, 2014

This makes sense to me. Added this to the 2.0 milestone.

Owner

jasonLaster commented Apr 12, 2014

Added to 2.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment