Menubar: Code formatting. A bit of cleanup: Replace self with that;

replace an unnecessary each.

1 files changed, 153 insertions, 135 deletions

diff --git a/tests/visual/menu/menubar.js b/tests/visual/menu/menubar.js
index a8265e69b..b9abacb9b 100644
--- a/tests/visual/menu/menubar.js
+++ b/tests/visual/menu/menubar.js
@@ -3,242 +3,260 @@
  * 
  * TODO move to jquery.ui.menubar.js
  */
-(function($) {
+(function( $ ) {
 
-// TODO code formatting
-$.widget("ui.menubar", {
+// TODO when mixing clicking menus and keyboard navigation, focus handling is broken
+// there has to be just one item that has tabindex
+$.widget( "ui.menubar", {
    options: {
       buttons: false,
       menuIcon: false
    },
 	_create: function() {
-		var self = this;
-		var items = this.items = this.element.children("li")
-			.addClass("ui-menubar-item")
-			.attr("role", "presentation")
-			.children("button, a");
-		items.slice(1).attr("tabIndex", -1);
-		var o = this.options;
-				
-		this.element.addClass('ui-menubar ui-widget-header ui-helper-clearfix').attr("role", "menubar");
-		this._focusable(items);
-		this._hoverable(items);
-		// TODO elm is used just once, so the each probably isn't nedded anymore
-		items.next("ul").each(function(i, elm) {
-			$(elm).menu({
-				select: function(event, ui) {
-					ui.item.parents("ul.ui-menu:last").hide();
-					self._trigger( "select", event, ui );
-					self._close();
+		var that = this;
+		var items = this.items = this.element.children( "li" )
+			.addClass( "ui-menubar-item" )
+			.attr( "role", "presentation" )
+			.children( "button, a" );
+		// let only the first item receive focus
+		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.next( "ul" )
+			.menu({
+				select: function( event, ui ) {
+					ui.item.parents( "ul.ui-menu:last" ).hide();
+					that._trigger( "select", event, ui );
+					that._close();
 					// TODO what is this targetting? there's probably a better way to access it
 					$(event.target).prev().focus();
 				}
-			}).hide()
-			.attr("aria-hidden", "true")
-			.attr("aria-expanded", "false")
-			.keydown(function(event) {
-				var menu = $(this);
-				if (menu.is(":hidden")) 
+			})
+			.hide()
+			.attr( "aria-hidden", "true" )
+			.attr( "aria-expanded", "false" )
+			.bind( "keydown.menubar", function( event ) {
+				var menu = $( this );
+				if ( menu.is( ":hidden" ) )
 					return;
-				switch (event.keyCode) {
+				switch ( event.keyCode ) {
 				case $.ui.keyCode.LEFT:
-					self._left(event);
+					that._left( event );
 					event.preventDefault();
 					break;
 				case $.ui.keyCode.RIGHT:
-					self._right(event);
+					that._right( event );
 					event.preventDefault();
 					break;
 				};
-			})
-		});
+			});
 		items.each(function() {
 			var input = $(this),
-				   menu = input.next("ul");
+				// TODO menu var is only used on two places, doesn't quite justify the .each
+				menu = input.next( "ul" );
 			
-			input.bind("click focus mouseenter", function(event) {
+			input.bind( "click.menubar focus.menubar mouseenter.menubar", function( event ) {
 				// ignore triggered focus event
-				if (event.type == "focus" && !event.originalEvent) {
+				if ( event.type == "focus" && !event.originalEvent ) {
 					return;
 				}
    				event.preventDefault();
 				// TODO can we simplify or extractthis check? especially the last two expressions
 				// there's a similar active[0] == menu[0] check in _open
-			   	if (event.type == "click" && menu.is(":visible") && self.active && self.active[0] == menu[0]) {
-					self._close();
+				if ( event.type == "click" && menu.is( ":visible" ) && that.active && that.active[0] == menu[0] ) {
+					that._close();
 					return;
 				}
-   				if ((self.open && event.type == "mouseenter") || event.type == "click") {
-   					self._open(event, menu);
-   				}
+				if ( ( that.open && event.type == "mouseenter" ) || event.type == "click" ) {
+					that._open( event, menu );
+				}
    			})
 			.bind( "keydown", function( event ) {
 				switch ( event.keyCode ) {
 				case $.ui.keyCode.SPACE:
 				case $.ui.keyCode.UP:
 				case $.ui.keyCode.DOWN:
-					self._open( event, $( this ).next() );
+					that._open( event, $( this ).next() );
 					event.preventDefault();
 					break;
 				case $.ui.keyCode.LEFT:
-					self._prev( event, $( this ) );
+					that._prev( event, $( this ) );
 					event.preventDefault();
 					break;
 				case $.ui.keyCode.RIGHT:
-					self._next( event, $( this ) );
+					that._next( event, $( this ) );
 					event.preventDefault();
 					break;
 				}
 			})
-			.addClass("ui-button ui-widget ui-button-text-only ui-menubar-link")
-			.attr("role", "menuitem")
-			.attr("aria-haspopup", "true")
-			.wrapInner("<span class='ui-button-text'></span>");
+			.addClass( "ui-button ui-widget ui-button-text-only ui-menubar-link" )
+			.attr( "role", "menuitem" )
+			.attr( "aria-haspopup", "true" )
+			.wrapInner( "<span class='ui-button-text'></span>" );
 
 			// TODO review if these options are a good choice, maybe they can be merged
-			if (o.menuIcon) {
-				input.addClass("ui-state-default").append("<span class='ui-button-icon-secondary ui-icon ui-icon-triangle-1-s'></span>");
-				input.removeClass("ui-button-text-only").addClass("ui-button-text-icon-secondary");
+			if ( that.options.menuIcon ) {
+				input.addClass( "ui-state-default" ).append( "<span class='ui-button-icon-secondary ui-icon ui-icon-triangle-1-s'></span>" );
+				input.removeClass( "ui-button-text-only" ).addClass( "ui-button-text-icon-secondary" );
 			}
    			
-			if (!o.buttons) {
+			if ( !that.options.buttons ) {
 				// TODO ui-menubar-link is added above, not needed here?
-				input.addClass('ui-menubar-link').removeClass('ui-state-default');
+				input.addClass( "ui-menubar-link" ).removeClass( "ui-state-default" );
 			};			
 			
 		});
-		self._bind({
-			keydown: function(event) {
-				// TODO merge the two ifs
-				if (event.keyCode == $.ui.keyCode.ESCAPE) {
-					if (self.active && self.active.menu("left", event) !== true) {
-						var active = self.active;
-						self.active.blur();
-						self._close( event );
-						active.prev().focus();
-					}
+		that._bind( {
+			keydown: function( event ) {
+				if ( event.keyCode == $.ui.keyCode.ESCAPE && that.active && that.active.menu( "left", event ) !== true ) {
+					var active = that.active;
+					that.active.blur();
+					that._close( event );
+					active.prev().focus();
 				}
 			},
-			focusout : function( event ) {
-				self.closeTimer = setTimeout(function() {
-					self._close( event );
-				}, 100);
+			focusin: function( event ) {
+				clearTimeout( that.closeTimer );
 			},
-			// TODO change order, focusin first
-			focusin :function( event ) {
-				clearTimeout(self.closeTimer);
+			focusout: function( event ) {
+				that.closeTimer = setTimeout( function() {
+					that._close( event );
+				}, 100);
 			}
 		});
 	},
 	
 	_destroy : function() {
-		var items = this.element.children("li")
-			.removeClass("ui-menubar-item")
-			.removeAttr("role", "presentation")
-			.children("button, a");
+		var items = this.element.children( "li" )
+			.removeClass( "ui-menubar-item" )
+			.removeAttr( "role", "presentation" )
+			.children( "button, a" );
 		
-		this.element.removeClass('ui-menubar ui-widget-header ui-helper-clearfix').removeAttr("role", "menubar").unbind(".menubar");;
-		items.unbind("focusin focusout click focus mouseenter keydown");
+		this.element
+			.removeClass( "ui-menubar ui-widget-header ui-helper-clearfix" )
+			.removeAttr( "role", "menubar" )
+			.unbind( ".menubar" );
 		
 		items
-			.removeClass("ui-button ui-widget ui-button-text-only ui-menubar-link ui-state-default")
-			.removeAttr("role", "menuitem")
-			.removeAttr("aria-haspopup", "true")
-			.children("span.ui-button-text").each(function(i, e) {
-				var item = $(this);
-				item.parent().html(item.html());
+			.unbind( ".menubar" )
+			.removeClass( "ui-button ui-widget ui-button-text-only ui-menubar-link ui-state-default" )
+			.removeAttr( "role", "menuitem" )
+			.removeAttr( "aria-haspopup", "true" )
+			// TODO unwrap?
+			.children( "span.ui-button-text" ).each(function( i, e ) {
+				var item = $( this );
+				item.parent().html( item.html() );
 			})
 			.end()
-			.children(".ui-icon").remove();
+			.children( ".ui-icon" ).remove();
 
-		$(document).unbind(".menubar");
-		
-		this.element.find(":ui-menu").menu("destroy")
-		.show()
-		.removeAttr("aria-hidden", "true")
-		.removeAttr("aria-expanded", "false")
-		.removeAttr("tabindex")
-		.unbind("keydown", "blur");
+		this.element.find( ":ui-menu" )
+			.menu( "destroy" )
+			.show()
+			.removeAttr( "aria-hidden", "true" )
+			.removeAttr( "aria-expanded", "false" )
+			.removeAttr( "tabindex" )
+			.unbind( ".menubar" );
 	},
 	
 	_close: function() {
-		if (!this.active || !this.active.length)
+		if ( !this.active || !this.active.length )
 			return;
-		this.active.menu("closeAll").hide().attr("aria-hidden", "true").attr("aria-expanded", "false");
-		this.active.prev().removeClass("ui-state-active").removeAttr("tabIndex");
+		this.active
+			.menu( "closeAll" )
+			.hide()
+			.attr( "aria-hidden", "true" )
+			.attr( "aria-expanded", "false" );
+		this.active
+			.prev()
+			.removeClass( "ui-state-active" )
+			.removeAttr( "tabIndex" );
 		this.active = null;
 		this.open = false;
 	},
 	
-	_open: function(event, menu) {
+	_open: function( event, menu ) {
 		// on a single-button menubar, ignore reopening the same menu
-		if (this.active && this.active[0] == menu[0]) {
+		if ( this.active && this.active[0] == menu[0] ) {
 			return;
 		}
-		// almost the same as _close above, but don't remove tabIndex
-		if (this.active) {
-			this.active.menu("closeAll").hide().attr("aria-hidden", "true").attr("aria-expanded", "false");
-			this.active.prev().removeClass("ui-state-active");
+		// TODO refactor, almost the same as _close above, but don't remove tabIndex
+		if ( this.active ) {
+			this.active
+				.menu( "closeAll" )
+				.hide()
+				.attr( "aria-hidden", "true" )
+				.attr( "aria-expanded", "false" );
+			this.active
+				.prev()
+				.removeClass( "ui-state-active" );
 		}
 		// set tabIndex -1 to have the button skipped on shift-tab when menu is open (it gets focus)
-		var button = menu.prev().addClass("ui-state-active").attr("tabIndex", -1);
-		this.active = menu.show().position({
-			my: "left top",
-			at: "left bottom",
-			of: button
-		})
-		.removeAttr("aria-hidden").attr("aria-expanded", "true")
-		.menu("focus", event, menu.children("li").first())
-		// TODO need a comment here why both events are triggered
-		.focus()
-		.focusin();
+		var button = menu.prev().addClass( "ui-state-active" ).attr( "tabIndex", -1 );
+		this.active = menu
+			.show()
+			.position( {
+				my: "left top",
+				at: "left bottom",
+				of: button
+			})
+			.removeAttr( "aria-hidden" )
+			.attr( "aria-expanded", "true" )
+			.menu("focus", event, menu.children( "li" ).first() )
+			// TODO need a comment here why both events are triggered
+			.focus()
+			.focusin();
 		this.open = true;
 	},
 	
 	// TODO refactor this and the next three methods
 	_prev: function( event, button ) {
-		button.attr("tabIndex", -1);
-		var prev = button.parent().prevAll("li").children( ".ui-button" ).eq( 0 );
-		if (prev.length) {
-			prev.removeAttr("tabIndex")[0].focus();
+		button.attr( "tabIndex", -1 );
+		var prev = button.parent().prevAll( "li" ).children( ".ui-button" ).eq( 0 );
+		if ( prev.length ) {
+			prev.removeAttr( "tabIndex" )[0].focus();
 		} else {
-			var lastItem = this.element.children("li:last").children(".ui-button:last");
-			lastItem.removeAttr("tabIndex")[0].focus();
+			var lastItem = this.element.children( "li:last" ).children( ".ui-button:last" );
+			lastItem.removeAttr( "tabIndex" )[0].focus();
 		}
 	},
 	
 	_next: function( event, button ) {
-		button.attr("tabIndex", -1);
-		var next = button.parent().nextAll("li").children( ".ui-button" ).eq( 0 );
-		if (next.length) {
-			next.removeAttr("tabIndex")[0].focus();
+		button.attr( "tabIndex", -1 );
+		var next = button.parent().nextAll( "li" ).children( ".ui-button" ).eq( 0 );
+		if ( next.length ) {
+			next.removeAttr( "tabIndex")[0].focus();
 		} else {
-			var firstItem = this.element.children("li:first").children(".ui-button:first");
-			firstItem.removeAttr("tabIndex")[0].focus();
+			var firstItem = this.element.children( "li:first" ).children( ".ui-button:first" );
+			firstItem.removeAttr( "tabIndex" )[0].focus();
 		}
 	},
 
 	// TODO rename to parent
-	_left: function(event) {
-		var prev = this.active.parent().prevAll("li:eq(0)").children( ".ui-menu" ).eq( 0 );
-		if (prev.length) {
-			this._open(event, prev);
+	_left: function( event ) {
+		var prev = this.active.parent().prevAll( "li:eq(0)" ).children( ".ui-menu" ).eq( 0 );
+		if ( prev.length ) {
+			this._open( event, prev );
 		} else {
-			var lastItem = this.element.children("li:last").children(".ui-menu:first");
-			this._open(event, lastItem);
+			var lastItem = this.element.children( "li:last" ).children( ".ui-menu:first" );
+			this._open( event, lastItem );
 		}
 	},
 	
 	// TODO rename to child (or something like that)
-	_right: function(event) {
-		var next = this.active.parent().nextAll("li:eq(0)").children( ".ui-menu" ).eq( 0 );
-		if (next.length) {
-			this._open(event, next);
+	_right: function( event ) {
+		var next = this.active.parent().nextAll( "li:eq(0)" ).children( ".ui-menu" ).eq( 0 );
+		if ( next.length ) {
+			this._open( event, next );
 		} else {
-			var firstItem = this.element.children("li:first").children(".ui-menu:first");
-			this._open(event, firstItem);
+			var firstItem = this.element.children( "li:first" ).children( ".ui-menu:first" );
+			this._open( event, firstItem );
 		}
 	}
 });
 
-}(jQuery));
+}( jQuery ));