Skip to content
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

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

Open
rjharmon opened this issue Jul 18, 2014 · 3 comments

Comments

@rjharmon
Copy link

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.

@rjharmon rjharmon changed the title When used in nested views, outer views should not recieve change-events that stickit has handled When used in nested views, outer views should not receive change-events that stickit has handled Jul 18, 2014
@akre54
Copy link
Contributor

akre54 commented Jul 19, 2014

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

@isochronous
Copy link

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
Copy link

Using stopPropagation() is almost always a bad idea.

  • It breaks a lot of things in unexpected ways, because there absolutely no way to know the event happened, should you need to. The most common example is some kind of menu, which you want to close if the user clicks outside of it. This is done by listening to click event bubbling from all over the DOM. If some handler calls stopPropagation(), the click will not reach the root and the menu will not close.
  • It breaks a developer's basic assumptions about events.
  • It can interact in some weird ways with third part libs, because they usually also assume events propagate normally.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants