Menubar: Refactored _next, _prev, _right and _left into a single method called _move and then created next and previous public methods that call _move. Very similar to how Menu handles keyboard interaction.

1 files changed, 43 insertions, 49 deletions

diff --git a/ui/jquery.ui.menubar.js b/ui/jquery.ui.menubar.js
index 3c564d461..5f9a2a1a0 100644
--- a/ui/jquery.ui.menubar.js
+++ b/ui/jquery.ui.menubar.js
@@ -32,19 +32,21 @@ $.widget( "ui.menubar", {
 	},
 	_create: function() {
 		var that = this;
-		var items = this.items = this.element.children( this.options.items )
+		this.menuItems = this.element.children( this.options.items );
+		this.items = this.menuItems.children( "button, a" );
+
+		this.menuItems
 			.addClass( "ui-menubar-item" )
-			.attr( "role", "presentation" )
-			.children( "button, a" );
+			.attr( "role", "presentation" );
 		// let only the first item receive focus
-		items.slice(1).attr( "tabIndex", -1 );
+		this.items.slice(1).attr( "tabIndex", -1 );
 
 		this.element
 			.addClass( "ui-menubar ui-widget-header ui-helper-clearfix" )
 			.attr( "role", "menubar" );
-		this._focusable( items );
-		this._hoverable( items );
-		items.siblings( this.options.menuElement )
+		this._focusable( this.items );
+		this._hoverable( this.items );
+		this.items.siblings( this.options.menuElement )
 			.menu({
 				position: {
 					within: this.options.position.within
@@ -69,16 +71,16 @@ $.widget( "ui.menubar", {
 					return;
 				switch ( event.keyCode ) {
 				case $.ui.keyCode.LEFT:
-					that._left( event );
+					that.previous( event );
 					event.preventDefault();
 					break;
 				case $.ui.keyCode.RIGHT:
-					that._right( event );
+					that.next( event );
 					event.preventDefault();
 					break;
 				};
 			});
-		items.each(function() {
+		this.items.each(function() {
 			var input = $(this),
 				// TODO menu var is only used on two places, doesn't quite justify the .each
 				menu = input.next( that.options.menuElement );
@@ -112,11 +114,11 @@ $.widget( "ui.menubar", {
 					event.preventDefault();
 					break;
 				case $.ui.keyCode.LEFT:
-					that._prev( event, $( this ) );
+					that.previous( event );
 					event.preventDefault();
 					break;
 				case $.ui.keyCode.RIGHT:
-					that._next( event, $( this ) );
+					that.next( event );
 					event.preventDefault();
 					break;
 				}
@@ -169,17 +171,16 @@ $.widget( "ui.menubar", {
 	},
 
 	_destroy : function() {
-		var items = this.element.children( this.options.items )
+		this.menuItems
 			.removeClass( "ui-menubar-item" )
-			.removeAttr( "role" )
-			.children( "button, a" );
+			.removeAttr( "role" );
 
 		this.element
 			.removeClass( "ui-menubar ui-widget-header ui-helper-clearfix" )
 			.removeAttr( "role" )
 			.unbind( ".menubar" );
 
-		items
+		this.items
 			.unbind( ".menubar" )
 			.removeClass( "ui-button ui-widget ui-button-text-only ui-menubar-link ui-state-default" )
 			.removeAttr( "role" )
@@ -253,48 +254,41 @@ $.widget( "ui.menubar", {
 		this.open = true;
 	},
 
-	// TODO refactor this and the next three methods
-	_prev: function( event, button ) {
-		button.attr( "tabIndex", -1 );
-		var prev = button.parent().prevAll( this.options.items ).children( ".ui-button" ).eq( 0 );
-		if ( prev.length ) {
-			prev.removeAttr( "tabIndex" )[0].focus();
-		} else {
-			var lastItem = this.element.children( this.options.items ).last().children( ".ui-button:last" );
-			lastItem.removeAttr( "tabIndex" )[0].focus();
-		}
+	next: function( event ) {
+		this._move( "next", "first", event );
 	},
 
-	_next: function( event, button ) {
-		button.attr( "tabIndex", -1 );
-		var next = button.parent().nextAll( this.options.items ).children( ".ui-button" ).eq( 0 );
-		if ( next.length ) {
-			next.removeAttr( "tabIndex")[0].focus();
-		} else {
-			var firstItem = this.element.children( this.options.items ).first().children( ".ui-button:first" );
-			firstItem.removeAttr( "tabIndex" )[0].focus();
-		}
+	previous: function( event ) {
+		this._move( "prev", "last", event );
 	},
 
-	// TODO rename to parent
-	_left: function( event ) {
-		var prev = this.active.parent().prevAll( this.options.items ).first().children( ".ui-menu" ).eq( 0 );
-		if ( prev.length ) {
-			this._open( event, prev );
+	_move: function( direction, filter, event ) {
+		var next,
+			wrapItem;
+		if ( this.open ) {
+			next = this.active.closest( ".ui-menubar-item" )[ direction + "All" ]( this.options.items ).first().children( ".ui-menu" ).eq( 0 );
+			wrapItem = this.menuItems[ filter ]().children( ".ui-menu" ).eq( 0 );
 		} else {
-			var lastItem = this.element.children( this.options.items ).last().children( ".ui-menu:first" );
-			this._open( event, lastItem );
+			if ( event ) {
+				next = $( event.target ).closest( ".ui-menubar-item" )[ direction + "All" ]( this.options.items ).children( ".ui-menubar-link" ).eq( 0 );
+				wrapItem = this.menuItems[ filter ]().children( ".ui-menubar-link" ).eq( 0 );
+			} else {
+				next = wrapItem = this.menuItems.children( "a" ).eq( 0 );
+			}
 		}
-	},
 
-	// TODO rename to child (or something like that)
-	_right: function( event ) {
-		var next = this.active.parent().nextAll( this.options.items ).first().children( ".ui-menu" ).eq( 0 );
 		if ( next.length ) {
-			this._open( event, next );
+			if ( this.open ) {
+				this._open( event, next );
+			} else {
+				next.removeAttr( "tabIndex")[0].focus();
+			}
 		} else {
-			var firstItem = this.element.children( this.options.items ).first().children( ".ui-menu:first" );
-			this._open( event, firstItem );
+			if ( this.open ) {
+				this._open( event, wrapItem );
+			} else {
+				wrapItem.removeAttr( "tabIndex")[0].focus();
+			}
 		}
 	}
 });