nytimes / backbone.stickit

Backbone data binding, model binding plugin. The real logic-less templates.
MIT License
1.64k stars 176 forks source link

When used in nested views, outer views should not receive change-events that stickit has handled #246

Open rjharmon opened 10 years ago

rjharmon commented 10 years ago

Using stickit in a Marionette Composite view, the outer instance of the view is seeing and honoring events from an inner view because the inner view's stickit's default updateModel() just returns true. If we change the default behavior to { event.stopPropagation(); return true; }, this problem goes away.

This isn't specific to Marionette - it's just an example where composed views get leakage of the events that stickit has already handled. Not that a coder couldn't override the newly-proposed default to restore earlier non-stopPropagation behavior if valuable.

akre54 commented 10 years ago

I haven't used Marionette in many moons. Mind creating a jsfiddle to show what you mean?

isochronous commented 9 years ago

I don't really think this would be a good idea to do by default. There are many cases in which outer views depend on events bubbled up from inner views, even if some handler in those inner views has already responded. If a feature like this were to be implemented, I'd suggest that it would be best to do it as a flag in the binding config, like

// ...
    update: function($el, value) {
        /* snip */
    },
    getVal: function($el) {
        /* snip */
    },
    consumeEvents: true //default false
} // end of binding config
spectras commented 9 years ago

Using stopPropagation() is almost always a bad idea.

One should use preventDefault() instead. And if you need to prevent outer views from handling the events, have them check event.defaultPrevented (or event.isDefaultPrevented() if using jquery), and ignore the event if that's true.