Skip to content

Commit

Permalink
Create mouseHandled flag per instance instead of globally. Fixes #886…
Browse files Browse the repository at this point in the history 
…6 - Menu: select event not firing due to mouseHandled flag reset bug
  • Loading branch information
@kborchers
kborchers committed Dec 28, 2012
1 parent 2c3d311 commit 5143b7f
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions ui/jquery.ui.menu.js
View file
Expand Up @@ -15,8 +15,6 @@
*/ */
(function( $, undefined ) { (function( $, undefined ) {


var mouseHandled = false;

$.widget( "ui.menu", { $.widget( "ui.menu", {
version: "@VERSION", version: "@VERSION",
defaultElement: "<ul>", defaultElement: "<ul>",
Expand All @@ -40,6 +38,7 @@ $.widget( "ui.menu", {


_create: function() { _create: function() {
this.activeMenu = this.element; this.activeMenu = this.element;
this.mouseHandled = false;
this.element this.element
.uniqueId() .uniqueId()
.addClass( "ui-menu ui-widget ui-widget-content ui-corner-all" ) .addClass( "ui-menu ui-widget ui-widget-content ui-corner-all" )
Expand Down Expand Up @@ -73,8 +72,8 @@ $.widget( "ui.menu", {
}, },
"click .ui-menu-item:has(a)": function( event ) { "click .ui-menu-item:has(a)": function( event ) {
var target = $( event.target ).closest( ".ui-menu-item" ); var target = $( event.target ).closest( ".ui-menu-item" );
if ( !mouseHandled && target.not( ".ui-state-disabled" ).length ) { if ( !this.mouseHandled && target.not( ".ui-state-disabled" ).length ) {
mouseHandled = true; this.mouseHandled = true;


this.select( event ); this.select( event );
// Open submenu on click // Open submenu on click
Expand Down Expand Up @@ -130,7 +129,7 @@ $.widget( "ui.menu", {
} }


// Reset the mouseHandled flag // Reset the mouseHandled flag
mouseHandled = false; this.mouseHandled = false;
} }
}); });
}, },
Expand Down

5 comments on commit 5143b7f

@jzaefferer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Could you also add a comment or two to document why this is needed?

@fnagel
Copy link
Member

@fnagel fnagel commented on 5143b7f Jan 10, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be useful for this issue too: http://bugs.jqueryui.com/ticket/8929

@jzaefferer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fnagel it landed in master - could you merge and test with selectmenu?

@fnagel
Copy link
Member

@fnagel fnagel commented on 5143b7f Jan 11, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzaefferer Perhaps its possible to remove the custom delay and use the flas var directly. I will give it a try this weekend!

@fnagel
Copy link
Member

@fnagel fnagel commented on 5143b7f Jan 11, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jzaefferer No, that did not work. Without changing the delay for the blur method the class on the active item will always be removed. We would need a way to handle the focus within Selectmenu.

Please sign in to comment.