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

Fixes #15157 Closes gh-1804
jquery · May 2, 2017 · a3e953b · a3e953b
1 parent abc9e7c
commit a3e953b
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 11 deletions.
diff --git a/tests/unit/menu/events.js b/tests/unit/menu/events.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();
+	} );
+} );
+
 } );
diff --git a/tests/unit/menu/menu.html b/tests/unit/menu/menu.html
@@ -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>
diff --git a/ui/widgets/menu.js b/ui/widgets/menu.js
@@ -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() );
 		}
 	},