Skip to content
Permalink
Browse files

Menubar: Fixed an issue with autoExpand binding that caused the menu …

…to close on fast mouseenter by changing to _bind on the parent menubar item
  • Loading branch information
kborchers committed Sep 29, 2011
1 parent c2f0362 commit ac8a19b62d89a5b70b8ef89029ff81cdf889cf41
Showing with 9 additions and 16 deletions.
  1. +9 −16 ui/jquery.ui.menubar.js
@@ -123,22 +123,6 @@ $.widget( "ui.menubar", {
.attr( "aria-haspopup", "true" )
.wrapInner( "<span class='ui-button-text'></span>" );

if ( that.options.autoExpand ) {
input.bind( "mouseleave.menubar", function( event ) {
that.timer = setTimeout( function() {
that._close();
}, 150 );
});
menu.bind( "mouseleave.menubar", function( event ) {
that.timer = setTimeout( function() {
that._close();
}, 150 );
})
.bind( "mouseenter.menubar", function( event ) {
clearTimeout( that.timer );
});
}

// TODO review if these options are a good choice, maybe they can be merged
if ( that.options.menuIcon ) {
input.addClass( "ui-state-default" ).append( "<span class='ui-button-icon-secondary ui-icon ui-icon-triangle-1-s'></span>" );
@@ -169,6 +153,15 @@ $.widget( "ui.menubar", {
}, 100);
}
});
if ( that.options.autoExpand ) {
that._bind( {
"mouseleave .ui-menubar-item": function( event ) {
that.timer = setTimeout( function() {

This comment has been minimized.

Copy link
@staabm

staabm Sep 29, 2011

Will that.timer be used somewhere...? I think we can get rid of this var and simplify the code here a bit...

This comment has been minimized.

Copy link
@staabm

staabm Sep 29, 2011

Or should it read closeTimer at all?

This comment has been minimized.

Copy link
@fx

fx Sep 29, 2011

Contributor

It is used up in the items.each loop, but I had it replaced by closeTimer as well in my pull request: #477

He probably just missed the commit and only merged my first to commits. Also: my precious credit! NOOOO! :)

This comment has been minimized.

Copy link
@jzaefferer

jzaefferer Sep 30, 2011

Member

Why would it need a different timer?

This comment has been minimized.

Copy link
@staabm

staabm Sep 30, 2011

It already does... Why not use the same timer for all close() calls...?

This comment has been minimized.

Copy link
@kborchers

kborchers Sep 30, 2011

Author Member

@fx, I did just miss that commit in your pull request. Also, I like your if inside the handler better as well. When I am at an actual computer tomorrow, I will revert my commit and then if you could put all of your changes into a single commit and issue another pull, we can go from there. Then you'll get your credit too. :) @jzaefferer, sound good to you?

This comment has been minimized.

Copy link
@fx

fx Sep 30, 2011

Contributor

Reverting the commit might be a PITA, I'll pull in upstream and just re-do the changes on this commit, waaay easier.

This comment has been minimized.

Copy link
@fx

fx Sep 30, 2011

Contributor

Here you go: #479

This should be enough dorking around for 5 lines of code now, sorry :)

This comment has been minimized.

Copy link
@kborchers

kborchers Sep 30, 2011

Author Member

Heh, ok thanks. I will review when I get into Boston. A lot of this was my fault. I was just given commit access and am still learning the ropes. Thanks for your patience.

This comment has been minimized.

Copy link
@fx

fx Sep 30, 2011

Contributor

No problem at all - figured that's the case. I'm eager to watch your progress and maybe learn a thing or two myself.

that._close();
}, 150 );
}
});
}
},

_destroy : function() {

0 comments on commit ac8a19b

Please sign in to comment.
You can’t perform that action at this time.