Skip to content

Commit 34a0479

Browse files
committed
Menu: Refactoring the collapseAll to deal with some issues selecting - Updating unit tests. Thanks @kborchers
1 parent cb372b7 commit 34a0479

File tree

3 files changed

+32
-24
lines changed

3 files changed

+32
-24
lines changed

tests/unit/menu/menu_events.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ asyncTest( "handle submenu auto collapse: mouseleave", function() {
9999
});
100100

101101
asyncTest( "handle custom menu item submenu auto collapse: mouseleave", function() {
102-
expect( 4 );
102+
expect( 5 );
103103
var $menu = $( "#menu5" ).menu( { items: "div" } );
104104

105105
$menu.children( ":nth-child(7)" ).trigger( "mouseover" );
@@ -110,13 +110,19 @@ asyncTest( "handle custom menu item submenu auto collapse: mouseleave", function
110110
equal( $menu.find( "div[aria-expanded='true']" ).length, 2, "second submenu expanded" );
111111
$menu.find( "div[aria-expanded='true']:first" ).trigger( "mouseleave" );
112112
equal( $menu.find( "div[aria-expanded='true']" ).length, 1, "second submenu collapsed" );
113+
114+
$menu.simulate( "keydown", { keyCode: $.ui.keyCode.DOWN });
115+
ok( $menu.find( ".ui-state-active" ).is( "#menu5 :nth-child(7) a" ),
116+
"down keypress selected an item from the first submenu" );
117+
113118
$menu.trigger( "mouseleave" );
114119
equal( $menu.find( "div[aria-expanded='true']" ).length, 0, "first submenu collapsed" );
115120
start();
116121
}, 400);
117122
}, 200);
118123
});
119124

125+
120126
test("handle keyboard navigation on menu without scroll and without submenus", function() {
121127
expect(12);
122128
var element = $('#menu1').menu({

tests/unit/menu/menu_test_helpers.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ function menu_log( message, clear ) {
55
if ( message === undefined ) {
66
message = $( "#log" ).data( "lastItem" );
77
}
8-
$( "#log" ).prepend( message + "," );
8+
$( "#log" ).prepend( $.trim( message ) + "," );
99
}
1010

1111
function menu_click( menu, item ) {

ui/jquery.ui.menu.js

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,19 @@ $.widget( "ui.menu", {
6262
target.siblings().children( ".ui-state-active" ).removeClass( "ui-state-active" );
6363
this.focus( event, target );
6464
},
65-
"mouseleave": "_mouseleave",
66-
"mouseleave .ui-menu": "_mouseleave",
65+
"mouseleave": "collapseAll",
66+
"mouseleave .ui-menu": "collapseAll",
6767
"mouseout .ui-menu-item": "blur",
6868
"focus": function( event ) {
6969
this.focus( event, $( event.target ).children( ".ui-menu-item:first" ) );
7070
},
71-
"blur": "collapseAll"
71+
blur: function( event ) {
72+
this._delay( function() {
73+
if ( ! $.contains( this.element[0], document.activeElement ) ) {
74+
this.collapseAll( event );
75+
}
76+
}, 0);
77+
}
7278
});
7379

7480
this.refresh();
@@ -341,25 +347,25 @@ $.widget( "ui.menu", {
341347
.position( position );
342348
},
343349

344-
collapseAll: function( event ) {
345-
var currentMenu = false;
346-
if ( event ) {
347-
var target = $( event.target );
348-
if ( target.is( "ui.menu" ) ) {
349-
currentMenu = target;
350-
} else if ( target.closest( ".ui-menu" ).length ) {
351-
currentMenu = target.closest( ".ui-menu" );
352-
}
350+
collapseAll: function( event, all ) {
351+
352+
// if we were passed an event, look for the submenu that contains the event
353+
var currentMenu = all ? this.element :
354+
$( event && event.target ).closest( this.element.find( ".ui-menu" ) );
355+
356+
// if we found no valid submenu ancestor, use the main menu to close all sub menus anyway
357+
if ( !currentMenu.length ) {
358+
currentMenu = this.element;
353359
}
354360

355361
this._close( currentMenu );
356362

357-
if ( !currentMenu ) {
358-
this.blur( event );
359-
this.activeMenu = this.element;
360-
}
363+
this.blur( event );
364+
this.activeMenu = currentMenu;
361365
},
362366

367+
// With no arguments, closes the currently active menu - if nothing is active
368+
// it closes all menus. If passed an argument, it will search for menus BELOW
363369
_close: function( startMenu ) {
364370
if ( !startMenu ) {
365371
startMenu = this.active ? this.active.parent() : this.element;
@@ -487,17 +493,13 @@ $.widget( "ui.menu", {
487493
return this.element.height() < this.element.prop( "scrollHeight" );
488494
},
489495

490-
_mouseleave: function( event ) {
491-
this.collapseAll( event );
492-
this.blur();
493-
},
494-
495496
select: function( event ) {
497+
496498
// save active reference before collapseAll triggers blur
497499
var ui = {
498500
item: this.active
499501
};
500-
this.collapseAll( event );
502+
this.collapseAll( event, true );
501503
this._trigger( "select", event, ui );
502504
}
503505
});

0 commit comments

Comments
 (0)