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

dom events "leaking" from child view to parent view #2967

Open
paulovieira opened this issue Apr 3, 2016 · 7 comments
Open

dom events "leaking" from child view to parent view #2967

paulovieira opened this issue Apr 3, 2016 · 7 comments

Comments

@paulovieira
Copy link
Contributor

When using nested views that have dom events in both the parent and child, it might happen that a dom event triggered in the child view will call also call the event handler in the parent (besides calling the handler in the child, as it should).

This happens if the selectors used in the events hash are the same in the parent and in child (or if they are not specific enough). Below is a simple example reproducing the situation (jsfiddle here: https://jsfiddle.net/paulovieira/kmt8kdmy/).

If you click the save button in the child view, you see that the handler from the parent is called as well, contrary to my expectation (the child view shouldn't be aware that there is a parent)

var Child = Mn.ItemView.extend({

    attributes: {
        style: "border: 1px solid red; margin: 5px;"
    },

    template: _.template(`
        <p>child view</p>
        <button class="btn-save">save</button>
    `),

    ui: {
        saveButtonChild: "button.btn-save"
    },

    events: {
        "click @ui.saveButtonChild": function(e){
            alert("this is the child view handler!");
        }
    },

});

var Parent = Mn.LayoutView.extend({

    attributes: {
        style: "border: 1px solid blue; padding: 5px;"
    },

    template: _.template(`
        <p>parent view</p>
        <button class="btn-save">save</button>

        <div class="some-region"></div>
    `),

    ui: {
        saveButtonParent: " button.btn-save",
        someRegion: "div.some-region"
    },

    events: {
        "click @ui.saveButtonParent": function(e){
            alert("this is the parent view handler!");
        }
    },

    onBeforeAttach: function(){
        this.getRegion("someRegion").show(new Child);
    },

    regions: {
        someRegion: '@ui.someRegion'
    }
});

var mainRegion = new Mn.Region({ el: "div#root-region"});
mainRegion.show(new Parent);

This is not a bug. It's simply a consequence of the way backbone uses event delegation in jquery, and the fact that the selectors are the same. There's an informative blog post in the marionette blog about this subject: http://blog.marionettejs.com/2015/02/12/understanding-the-event-hash/index.html

But I was wondering how have been people dealing with this issue. In a situation of deep nested views this might turn out to be problematic. Child views should not care about what is happening above them. If I have to return to some marionette code 1 year later to add a new child view, I shouldn't have to review the details in the parent to make sure the events are not clashing. Do you agree?

Having to add specific dummy classes or ids everywhere ("js-child-view-person", etc) just to avoid these clashes doesn't feel right.

Another option would be to add a top-level container element in the template, so that the selectors in the events hash can start from there. Something like this:

var Child = Mn.ItemView.extend({
     template: _.template(`
        <div class="child-container">
            <p>child view</p>
            <button class="btn-save">save</button>
        </div>
    `),

    ui: {
        saveButtonChild: "div.child-container > button.btn-save"
    },

    events: {
        "click @ui.saveButtonChild": function(e){
            alert("this is the child view handler!");
        }
    }
});

But this also doesn't feel right.

What have been people doing to solve this?

@paulovieira paulovieira changed the title dom events leaking from child view to parent view dom events "leaking" from child view to parent view Apr 3, 2016
@paulovieira
Copy link
Contributor Author

I came up with a simple solution based on the "cid" property of the views (internal unique identifiers - "view1", "view2", etc).

Jsfiddle here: https://jsfiddle.net/paulovieira/kmt8kdmy/3/

It consists in this:

  1. We add view.cid to the view's element (this.el) as a data property. This is done in the initialize method.
  2. We add a new marionette utility function, "Mn.getClosestCid":
Mn.getClosestCid = function(el){
    return $(el).closest('[data-cid*="view"]').attr('data-cid');
};

Given a dom element, it will search the dom tree upwards until it finds an element with a "data-cid" attribute that starts with "view", and returns the value.
3) In the event handler, we return early if the view's cid is not Mn.getClosestCid(e.target)

It may sound a bit complex, but steps 1 and 2 could be done by default by marionette, so I only have to make the check in the beginning of the event handler:

function(e){
    if(Mn.getClosestCid(e.target)!==this.cid){ return; }
    ...
}

Is there any interest in adding this feature to marionette?

@paulfalgout
Copy link
Member

Yeah I'm not sure here. The root of this issue is more of a Backbone specific issue. Any interest in trying out an issue on Backbone? This is great and I think worth pursuing, but it'd be ideal to piggyback off of a Bb solution if at all possible. Thoughts?

@paulfalgout
Copy link
Member

cc: @jridgewell

@rsmithlal
Copy link

@paulovieira Your solution is excellent, thank you so much! I ran into the same problem when I set up a view that has nested sub views of the same type.

@paulovieira
Copy link
Contributor Author

paulovieira commented Apr 6, 2018

Probably doesn't make sense to include this feature in Mn because seems to be a bit of a particular case.

But maybe we should add this case to the documentation? It can be handled with a very simple Mn.Behaviour + a custom utility method like shown above.

@rsmithlal
Copy link

I just implemented it in my specific view because all the nested views are of the same type. However, it does seem like this is an issue that could affect other cases if the events use the same reference.

@paulfalgout
Copy link
Member

this becomes more problematic when regions are nondestructive as re-rendering the parent changes the order of the event handlers and the propogation trick for triggers doesn't work

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

No branches or pull requests

3 participants