Skip to content
Permalink
Browse files

Menu: Don't move focus from the active item on click. Fixes #8552 - s…

…elected value overwritten/not correctly set.
  • Loading branch information...
scottgonzalez committed Sep 5, 2012
1 parent 6abb107 commit e162fddd9547d07c6ad1badc135a456f4012ad34
Showing with 32 additions and 21 deletions.
  1. +11 −7 tests/unit/menu/menu_events.js
  2. +21 −14 ui/jquery.ui.menu.js
@@ -57,13 +57,15 @@ asyncTest( "handle blur", function() {
});

click( element, "1" );
setTimeout( function() {
setTimeout(function() {
element.blur();
start();
}, 350 );
setTimeout(function() {
start();
}, 350 );
});
});

asyncTest( "handle blur on click", function() {
asyncTest( "handle blur via click outside", function() {
expect( 1 );
var blurHandled = false,
element = $( "#menu1" ).menu({
@@ -77,10 +79,12 @@ asyncTest( "handle blur on click", function() {
});

click( element, "1" );
setTimeout( function() {
setTimeout(function() {
$( "<a>", { id: "remove"} ).appendTo( "body" ).trigger( "click" );
start();
}, 350 );
setTimeout(function() {
start();
}, 350 );
});
});

test( "handle focus of menu with active item", function() {
@@ -72,17 +72,23 @@ $.widget( "ui.menu", {
event.preventDefault();
},
"click .ui-menu-item:has(a)": function( event ) {
var target = $( event.target );
if ( !mouseHandled && target.closest( ".ui-menu-item" ).not( ".ui-state-disabled" ).length ) {
var target = $( event.target ).closest( ".ui-menu-item" );
if ( !mouseHandled && target.not( ".ui-state-disabled" ).length ) {
mouseHandled = true;

this.select( event );
// Open submenu on click
if ( this.element.has( ".ui-menu" ).length ) {
if ( target.has( ".ui-menu" ).length ) {
this.expand( event );
} else if ( !this.element.is(":focus") ) {
} else if ( !this.element.is( ":focus" ) ) {
// Redirect focus to the menu
this.element.focus();
this.element.trigger( "focus", [ true ] );

// If the active item is on the top level, let it stay active.
// Otherwise, blur the active item since it is no longer visible.
if ( this.active && this.active.parents( ".ui-menu" ).length === 1 ) {
clearTimeout( this.timer );
}
}
}
},
@@ -95,12 +101,14 @@ $.widget( "ui.menu", {
},
mouseleave: "collapseAll",
"mouseleave .ui-menu": "collapseAll",
focus: function( event ) {
focus: function( event, keepActiveItem ) {
// If there's already an active item, keep it active
// If not, activate the first item
var item = this.active || this.element.children( ".ui-menu-item" ).eq( 0 );

this.focus( event, item );
if ( !keepActiveItem ) {
this.focus( event, item );
}
},
blur: function( event ) {
this._delay(function() {
@@ -195,7 +203,7 @@ $.widget( "ui.menu", {
this.collapse( event );
break;
case $.ui.keyCode.RIGHT:
if ( !this.active.is( ".ui-state-disabled" ) ) {
if ( this.active && !this.active.is( ".ui-state-disabled" ) ) {
this.expand( event );
}
break;
@@ -587,12 +595,11 @@ $.widget( "ui.menu", {
},

select: function( event ) {
// Save active reference before collapseAll triggers blur
var ui = {
// Selecting a menu item removes the active item causing multiple clicks to be missing an item
item: this.active || $( event.target ).closest( ".ui-menu-item" )
};
if ( !ui.item.has( ".ui-menu" ).length ) {
// TODO: It should never be possible to not have an active item at this
// point, but the tests don't trigger mouseenter before click.
this.active = this.active || $( event.target ).closest( ".ui-menu-item" );
var ui = { item: this.active };
if ( !this.active.has( ".ui-menu" ).length ) {
this.collapseAll( event, true );
}
this._trigger( "select", event, ui );

0 comments on commit e162fdd

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