0

JSFiddle: http://jsfiddle.net/357nf0ht/4/

If you are hovering one of the .item, a menu will appear. Hovering the "X" will show you a button. I expect, that hovering the button will create a single temporary item, which belongs to the first selected .item. But instead of that, there are multiple temporary items, if you have hovered more then one .item at the beginning. It seems, that this remembers also the events in the past.

I don't understand this behavior.

$(document).on('mouseenter', '.item', function (event) {
    var item = $(this);
    $('#item_add').css({
        "left": '5.5em',
        "top": item.offset().top - 15
    }).show();

    $('#item_add').hover(function () {
        var menue = $(this);
        menue.animate({
            "width": '7em',
            "left": '0.5em'
        }, 200, function () {
            $(this).find(".body").stop().slideDown(200);
        });
    }, function () {
        var menue = $(this);
        menue.find(".body").stop().slideUp(200, function () {
            menue.animate({
                "width": '2em',
                "left": '5.5em'
            }, 200);
        });
    });

    $('#item_element_new').hover(function () {
        item.after('<div id="item_new"></div>');
    }, function () {
        $('#item_new').remove();
    });
});
user3142695
  • 15,844
  • 47
  • 176
  • 332
  • One, events bubble up so `mouseenter` will occur on all elements within `.item` and secondly, anytime a `mouseenter` occurs you are binding a new `hover` event trigger on `#item_add` (many many times most likely). – Erik Philips Apr 10 '15 at 06:10
  • You should generally avoid binding event handlers inside event handlers. – Tomalak Apr 10 '15 at 06:29
  • `#item_add` exists throughout the lifetime of your page, right? Why would you want to bind an "hover" event handler to it *every time* the mouse enters some other element? With your current code you move the mouse once over the page, ten "mouseenter" events occur somewhere, ten event handlers are added to `#item_add` -- and every one of them does the same thing. Doesn't make sense, does it? – Tomalak Apr 10 '15 at 06:36
  • And this isn't the case in the solution of c4605, right? – user3142695 Apr 10 '15 at 06:43

2 Answers2

3

This is happening because every time you trigger mouseenter on .item you attach a new event to button.

Quick Solution: You have to remove previous events before attaching a new one, you can do this with .unbind(), here is a working example

$('#item_element_new').unbind();
$('#item_element_new').hover(function () {


$(document).on('mouseenter', '.item', function (event) {
    var item = $(this);
    $('#item_add').css({
        "left": '5.5em',
        "top": item.offset().top - 15
    }).show();

    $('#item_add').hover(function () {
        var menue = $(this);
        menue.animate({
            "width": '7em',
            "left": '0.5em'
        }, 200, function () {
            $(this).find(".body").stop().slideDown(200);
        });
    }, function () {
        var menue = $(this);
        menue.find(".body").stop().slideUp(200, function () {
            menue.animate({
                "width": '2em',
                "left": '5.5em'
            }, 200);
        });
    });
    $('#item_element_new').unbind();
    $('#item_element_new').hover(function () {
        item.after('<div id="item_new"></div>');
    }, function () {
        $('#item_new').remove();
    });

});
.item {
 border: 1px solid #e2e2e2; 
 margin: 6px;
 padding: 0px 10px 0px 10px;
 position: relative; 
    height: 40px;
 }
.wrap {
    margin-left: 8em;
}
#item_new {
    border: 1px dashed #C0C0C0 !important;
    background: none repeat scroll 0% 0% #F7F7F7;
    display: block;
    height: 2.2em;
    margin: 6px;
    padding: 0px 10px;
    position: relative;
}
#item_add {
    display: none;
    position: absolute;
    width: 2em;
    border: 1px solid #ddd;
    background-color: #f7f7f7;
    color: #aaa;
    padding: 0px 6px;
}
#item_add .body {
    display: none;
}
#item_add .arrow {
    width: 0px;
    height: 0px;
    -webkit-transform:rotate(360deg);
    border-style: solid;
    border-width: 5px 0 5px 7px;
    border-color: transparent transparent transparent #f7f7f7;
    top: 5px;
    right: -7px;
    position: absolute;
}
#item_add button {
    background: none repeat scroll 0px center #fff;
    padding: 0.2em 2em;
    margin: 3px .2em;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<div class="wrap">
    <div class="item"></div>
    <div class="item"></div>
    <div class="item"></div>
</div>
<div id="item_add">
    <header>X</header>
    <div class="body">
        <button id="item_element_new">Example</button>
    </div>
    <div class="arrow"></div>
</div>

Soulution 2: As @Regent pointed out your code have some problems, you can solve a part of them:

  1. Moving event binding outside mouseenter event handler

  2. Caching jQuery objects

  3. Not using .unbind() anymore

var $itemAdd = $('#item_add');
var $menuBody = $itemAdd.find(".body")
var $item = $('.item');

$itemAdd.hover(function () {
    $itemAdd.animate({
        "width": '7em',
        "left": '0.5em'
    }, 200, function () {
        $menuBody.stop().slideDown(200);
    });
}, function () {
    $menuBody.stop().slideUp(200, function () {
        $itemAdd.animate({
            "width": '2em',
            "left": '5.5em'
        }, 200);
    });
});

$('#item_element_new').hover(function () {
    $('.item-hovered').after('<div id="item_new"></div>');
}, function () {
    $('#item_new').remove();
});
   
$item.mouseenter(function (event) {
    var $currentItem = $(this);
     $currentItem.addClass('item-hovered');
     $currentItem.siblings().removeClass('item-hovered');
    
    $itemAdd.css({
        "left": '5.5em',
        "top":  $currentItem.offset().top - 15
    }).show(); 
});
.item {
 border: 1px solid #e2e2e2; 
 margin: 6px;
 padding: 0px 10px 0px 10px;
 position: relative; 
    height: 40px;
 }
.wrap {
    margin-left: 8em;
}
#item_new {
    border: 1px dashed #C0C0C0 !important;
    background: none repeat scroll 0% 0% #F7F7F7;
    display: block;
    height: 2.2em;
    margin: 6px;
    padding: 0px 10px;
    position: relative;
}
#item_add {
    display: none;
    position: absolute;
    width: 2em;
    border: 1px solid #ddd;
    background-color: #f7f7f7;
    color: #aaa;
    padding: 0px 6px;
}
#item_add .body {
    display: none;
}
#item_add .arrow {
    width: 0px;
    height: 0px;
    -webkit-transform:rotate(360deg);
    border-style: solid;
    border-width: 5px 0 5px 7px;
    border-color: transparent transparent transparent #f7f7f7;
    top: 5px;
    right: -7px;
    position: absolute;
}
#item_add button {
    background: none repeat scroll 0px center #fff;
    padding: 0.2em 2em;
    margin: 3px .2em;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<div class="wrap">
    <div class="item"></div>
    <div class="item"></div>
    <div class="item"></div>
</div>
<div id="item_add">
    <header>X</header>
    <div class="body">
        <button id="item_element_new">Example</button>
    </div>
    <div class="arrow"></div>
</div>
Community
  • 1
  • 1
adricadar
  • 9,971
  • 5
  • 33
  • 46
  • @user3142695 1. Not caching all jQuery objects. 2. Binding event handlers inside event handler. 3. Creating and removing element instead of just showing/hiding. 4. Delegated event handler for `.item`, while they are static elements. And if they are created dynamically, do they have more specific static parent rather than `document`? – Regent Apr 10 '15 at 06:48
  • Thank you. I just don't understand 1. Do you have a small example what you mean? And why is delegating with document a problem? – user3142695 Apr 10 '15 at 07:02
  • @Regent you are right, i made a *Solution 2* with most part of problems you pointed out. – adricadar Apr 10 '15 at 07:07
  • @user3142695 in 1 he means to avoid doing $(...); all the times, to avoid this you have to store it in a variable, please see my *Soultion 2*. – adricadar Apr 10 '15 at 07:20
  • @user3142695 1. When you write `$('#item_add')`, searching through DOM for element with ID `item_add` is perfomed. Even though searching by id is quite quick one, the good practise is to "cache" it into variable and to use this variable instead. `var item = $('#item_add'); item.hover(` and so on. 2. Because it will cause checking whether event target (element for which event is triggered) is `.item` or not on **every** `mouseenter` inside `document`. It's perfomance issue. Of course, it's all about couple of milliseconds, but on slow device and 10-20 event handlers it might be noticable. – Regent Apr 10 '15 at 07:22
  • Thank you very much for that explanation. I learned a lot of that. Think I did some of these mistakes quite often... – user3142695 Apr 10 '15 at 07:46
  • The thing is, that I have a webpage which fully works with AJAX. That means nearly everything is beeing loaded dynamically. That's why I always used delegation on document. So everything is working. I will try to make document more specific... – user3142695 Apr 10 '15 at 07:49
  • @Regent Thank you for helping me to improve my answer, if is anything i should improve, please notice me. – adricadar Apr 10 '15 at 07:55
  • @adricadar you're welcome. I suppose OP has already got some ideas from _solution 2_, so it will be enough. And I have already voted up this answer. – Regent Apr 10 '15 at 09:26
2

The easiest fix, reason is what @erik-philips said:

$(document).on('mouseenter', '.item', function (event) {
    var item = $(this);
    $('#item_add').css({
        "left": '5.5em',
        "top": item.offset().top - 15
    }).show();
    $('#item_element_new').data('hovering_item', item);
});

$('#item_add').hover(function () {
    var menue = $(this);
    menue.animate({
        "width": '7em',
        "left": '0.5em'
    }, 200, function () {
        menue.find(".body").stop().slideDown(200);
    });
}, function () {
    var menue = $(this);
    menue.find(".body").stop().slideUp(200, function () {
        menue.animate({
            "width": '2em',
            "left": '5.5em'
        }, 200);
    });
});

$('#item_element_new').hover(function () {
    var item = $('#item_element_new').data('hovering_item');
    item.after('<div id="item_new"></div>');
}, function () {
    var item = $('#item_element_new').data('hovering_item');
    $('#item_new').remove();
});
.item {
 border: 1px solid #e2e2e2; 
 margin: 6px;
 padding: 0px 10px 0px 10px;
 position: relative; 
    height: 40px;
 }
.wrap {
    margin-left: 8em;
}
#item_new {
    border: 1px dashed #C0C0C0 !important;
    background: none repeat scroll 0% 0% #F7F7F7;
    display: block;
    height: 2.2em;
    margin: 6px;
    padding: 0px 10px;
    position: relative;
}
#item_add {
    display: none;
    position: absolute;
    width: 2em;
    border: 1px solid #ddd;
    background-color: #f7f7f7;
    color: #aaa;
    padding: 0px 6px;
}
#item_add .body {
    display: none;
}
#item_add .arrow {
    width: 0px;
    height: 0px;
    -webkit-transform:rotate(360deg);
    border-style: solid;
    border-width: 5px 0 5px 7px;
    border-color: transparent transparent transparent #f7f7f7;
    top: 5px;
    right: -7px;
    position: absolute;
}
#item_add button {
    background: none repeat scroll 0px center #fff;
    padding: 0.2em 2em;
    margin: 3px .2em;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<div class="wrap">
    <div class="item"></div>
    <div class="item"></div>
    <div class="item"></div>
</div>
<div id="item_add">
    <header>X</header>
    <div class="body">
        <button id="item_element_new">Example</button>
    </div>
    <div class="arrow"></div>
</div>
Community
  • 1
  • 1
c4605
  • 185
  • 2
  • 10