Skip to content
New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Study arguments passed to triggerMethods #1285

Closed
jamesplease opened this issue May 3, 2014 · 8 comments
Closed

Study arguments passed to triggerMethods #1285

jamesplease opened this issue May 3, 2014 · 8 comments
Milestone

Comments

@jamesplease
Copy link
Member

Once v2 lands we should look at the arguments we pass to triggerMethods. Maybe we can add some new cool ones to help people out.

@jamesplease jamesplease added this to the v2.1 milestone May 3, 2014
@joshbedo
Copy link

joshbedo commented May 8, 2014

@jmeas what did you have in mind?

@jamesplease
Copy link
Member Author

ha nothing in particular to be honest. I just wanted to audit each and every triggerMethod, check out the arguments we pass, and see if there's any work we can do there. There's really no goal other than seeing what information the results of the exercise yield us.

@jasonLaster jasonLaster removed the minor label May 9, 2014
@jasonLaster
Copy link
Member

I'd say step one here is to add the arguments to the events document we're working on. Then fold all of that into the doc. We can come back to cleanup tasks after the api is in a good place and 2.0 is out.

@cmaher
Copy link
Member

cmaher commented Jul 4, 2014

I ran a quick search across the project. The file/event context seems to do a decent job of telling a story, so I'm posting the results here.

Targets
    Occurrences of 'triggerMethod\(|triggerMethod.call|triggerMethod.apply' in Directory  backbone.marionette/src with mask '*.js'
        backbone.marionette  (68 usages found)
            src  (68 usages found)
                marionette.application.js  (6 usages found)
                    (37: 10) this.triggerMethod('before:start', options);
                    (39: 10) this.triggerMethod('start', options);
                    (94: 12) this.triggerMethod('before:add:region', name);
                    (99: 12) this.triggerMethod('add:region', name, region);
                    (103: 12) this.triggerMethod('before:remove:region', name);
                    (108: 12) this.triggerMethod('remove:region', name, region);
                marionette.behaviors.js  (2 usages found)
                    (67: 7) triggerMethod.apply(this, args);
                    (70: 9) triggerMethod.apply(b, args);
                marionette.collectionview.js  (21 usages found)
                    (61: 17) child.triggerMethod('show');
                    (63: 22) Marionette.triggerMethod.call(child, 'show');
                    (103: 15) child.triggerMethod('show');
                    (105: 20) Marionette.triggerMethod.call(child, 'show');
                    (115: 10) this.triggerMethod('before:render', this);
                    (117: 10) this.triggerMethod('render', this);
                    (145: 12) this.triggerMethod('before:render:collection', this);
                    (147: 12) this.triggerMethod('render:collection', this);
                    (170: 12) this.triggerMethod('before:render:empty');
                    (176: 12) this.triggerMethod('render:empty');
                    (214: 12) this.triggerMethod.call(view, 'before:show');
                    (227: 12) this.triggerMethod.call(view, 'show');
                    (299: 10) this.triggerMethod('before:add:child', view);
                    (308: 14) view.triggerMethod('show');
                    (310: 20) Marionette.triggerMethod.call(view, 'show');
                    (314: 10) this.triggerMethod('add:child', view);
                    (337: 12) this.triggerMethod('before:remove:child', view);
                    (344: 12) this.triggerMethod('remove:child', view);
                    (426: 10) this.triggerMethod('before:destroy:collection');
                    (428: 10) this.triggerMethod('destroy:collection');
                    (462: 12) this.triggerMethod.apply(this, args);
                marionette.compositeview.js  (4 usages found)
                    (76: 10) this.triggerMethod('before:render', this);
                    (81: 10) this.triggerMethod('render', this);
                    (98: 10) this.triggerMethod('before:render:template');
                    (108: 10) this.triggerMethod('render:template');
                marionette.controller.js  (2 usages found)
                    (25: 10) this.triggerMethod.apply(this, ['before:destroy'].concat(args));
                    (26: 10) this.triggerMethod.apply(this, ['destroy'].concat(args));
                marionette.domRefresh.js  (1 usage found)
                    (26: 14) view.triggerMethod('dom:refresh');
                marionette.itemview.js  (2 usages found)
                    (42: 10) this.triggerMethod('before:render', this);
                    (52: 10) this.triggerMethod('render', this);
                marionette.layoutview.js  (6 usages found)
                    (54: 10) this.triggerMethod('before:region:add', name);
                    (68: 10) this.triggerMethod('before:region:remove', name);
                    (144: 12) this.triggerMethod('before:add:region', name);
                    (149: 12) this.triggerMethod('add:region', name, region);
                    (153: 12) this.triggerMethod('before:remove:region', name);
                    (158: 12) this.triggerMethod('remove:region', name, region);
                marionette.module.js  (4 usages found)
                    (69: 10) this.triggerMethod('before:start', options);
                    (74: 10) this.triggerMethod('start', options);
                    (84: 10) this.triggerMethod('before:stop');
                    (97: 10) this.triggerMethod('stop');
                marionette.object.js  (2 usages found)
                    (24: 10) this.triggerMethod('before:destroy');
                    (25: 10) this.triggerMethod('destroy');
                marionette.region.js  (9 usages found)
                    (160: 14) this.triggerMethod('before:swap', view);
                    (163: 12) this.triggerMethod('before:show', view);
                    (164: 12) this.triggerMethod.call(view, 'before:show');
                    (170: 14) this.triggerMethod('swap', view);
                    (173: 12) this.triggerMethod('show', view);
                    (176: 14) view.triggerMethod('show');
                    (178: 14) this.triggerMethod.call(view, 'show');
                    (218: 10) this.triggerMethod('before:empty', view);
                    (224: 10) this.triggerMethod('empty', view);
                marionette.regionManager.js  (4 usages found)
                    (52: 12) this.triggerMethod('before:add:region', name, region);
                    (56: 12) this.triggerMethod('add:region', name, region);
                    (116: 12) this.triggerMethod('before:remove:region', name, region);
                    (121: 12) this.triggerMethod('remove:region', name, region);
                marionette.triggermethod.js  (2 usages found)
                    (3: 10) // `this.triggerMethod("foo")` will trigger the "foo" event and
                    (6: 10) // `this.triggerMethod("foo:bar")` will trigger the "foo:bar" event and
                marionette.view.js  (3 usages found)
                    (98: 14) this.triggerMethod(eventName, args);
                    (166: 10) this.triggerMethod.apply(this, ['before:destroy'].concat(args));
                    (172: 10) this.triggerMethod.apply(this, ['destroy'].concat(args));

Overall, the triggerMethod calls seem complete to me. There's just the issue of Application/LayoutView proxying RegionManager and having to repeat code.

@jamesplease
Copy link
Member Author

Nice audit, @cmaher. One change I'd like to see is the addition of the relevant objects to the triggerMethods per #1586. I've yet to fully review this list though :)

@samccone
Copy link
Member

samccone commented Jul 8, 2014

@jmeas any goal post we can shoot for here that you have in mind?

@samccone samccone removed this from the v2.1 milestone Jul 8, 2014
@jamesplease
Copy link
Member Author

Not yet. Still need to review. Will mark as unresolved.

@jasonLaster jasonLaster modified the milestone: v3.0.0 Aug 30, 2014
@jamesplease
Copy link
Member Author

Moved to #1796

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

5 participants