Skip to content
Permalink
Browse files

Menu: Don't focus dividers when wrapping via keyboard navigation

Fixes #15157
Closes gh-1804
  • Loading branch information
scottgonzalez committed May 2, 2017
1 parent abc9e7c commit a3e953b495905d0c67790e65032841451b470ce1
Showing with 45 additions and 11 deletions.
  1. +22 −0 tests/unit/menu/events.js
  2. +10 −0 tests/unit/menu/menu.html
  3. +13 −11 ui/widgets/menu.js
@@ -757,4 +757,26 @@ QUnit.test( "#10571: When typing in a menu, only menu-items should be focused",
} );
} );

QUnit.test( "#15157: Must not focus menu dividers with the keyboard", function( assert ) {
var ready = assert.async();
assert.expect( 6 );

var element = $( "#menu-with-dividers" ).menu( {
focus: function( event, ui ) {
assert.hasClasses( ui.item, "ui-menu-item", "Should have menu item class" );
log( ui.item.text() );
}
} );

element.simulate( "keydown", { keyCode: $.ui.keyCode.DOWN } );
element.simulate( "keydown", { keyCode: $.ui.keyCode.DOWN } );
element.simulate( "keydown", { keyCode: $.ui.keyCode.DOWN } );
element.simulate( "keydown", { keyCode: $.ui.keyCode.DOWN } );
element.simulate( "keydown", { keyCode: $.ui.keyCode.UP } );
setTimeout( function() {
assert.equal( logOutput(), "beginning,middle,end,beginning,end", "Should wrap around items" );
ready();
} );
} );

} );
@@ -323,6 +323,16 @@
<li class="foo"><div>Addyston</div></li>
<li class="foo"><div>Adelphi</div></li>
</ul>

<ul id="menu-with-dividers">
<li>-</li>
<li>beginning</li>
<li>-</li>
<li>middle</li>
<li>-</li>
<li>end</li>
<li>-</li>
</ul>
</div>
</body>
</html>
@@ -136,7 +136,7 @@ return $.widget( "ui.menu", {

// If there's already an active item, keep it active
// If not, activate the first item
var item = this.active || this.element.find( this.options.items ).eq( 0 );
var item = this.active || this._menuItems().first();

if ( !keepActiveItem ) {
this.focus( event, item );
@@ -538,11 +538,7 @@ return $.widget( "ui.menu", {
},

expand: function( event ) {
var newItem = this.active &&
this.active
.children( ".ui-menu " )
.find( this.options.items )
.first();
var newItem = this.active && this._menuItems( this.active.children( ".ui-menu" ) ).first();

if ( newItem && newItem.length ) {
this._open( newItem.parent() );
@@ -570,21 +566,27 @@ return $.widget( "ui.menu", {
return this.active && !this.active.nextAll( ".ui-menu-item" ).length;
},

_menuItems: function( menu ) {
return ( menu || this.element )
.find( this.options.items )
.filter( ".ui-menu-item" );
},

_move: function( direction, filter, event ) {
var next;
if ( this.active ) {
if ( direction === "first" || direction === "last" ) {
next = this.active
[ direction === "first" ? "prevAll" : "nextAll" ]( ".ui-menu-item" )
.eq( -1 );
.last();
} else {
next = this.active
[ direction + "All" ]( ".ui-menu-item" )
.eq( 0 );
.first();
}
}
if ( !next || !next.length || !this.active ) {
next = this.activeMenu.find( this.options.items )[ filter ]();
next = this._menuItems( this.activeMenu )[ filter ]();
}

this.focus( event, next );
@@ -610,7 +612,7 @@ return $.widget( "ui.menu", {

this.focus( event, item );
} else {
this.focus( event, this.activeMenu.find( this.options.items )
this.focus( event, this._menuItems( this.activeMenu )
[ !this.active ? "first" : "last" ]() );
}
},
@@ -634,7 +636,7 @@ return $.widget( "ui.menu", {

this.focus( event, item );
} else {
this.focus( event, this.activeMenu.find( this.options.items ).first() );
this.focus( event, this._menuItems( this.activeMenu ).first() );
}
},

0 comments on commit a3e953b

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