Fix various pull request comments and coding standards issues

1 files changed, 60 insertions, 62 deletions

diff --git a/ui/jquery.ui.popup.js b/ui/jquery.ui.popup.js
index 7ccf0ff2e..9ea4d59e2 100644
--- a/ui/jquery.ui.popup.js
+++ b/ui/jquery.ui.popup.js
@@ -13,7 +13,7 @@
  *	jquery.ui.position.js
  */
 (function($) {
-	
+
 var idIncrement = 0;
 
 $.widget( "ui.popup", {
@@ -27,34 +27,34 @@ $.widget( "ui.popup", {
 		if ( !this.options.trigger ) {
 			this.options.trigger = this.element.prev();
 		}
-		
+
 		if ( !this.element.attr( "id" ) ) {
 			this.element.attr( "id", "ui-popup-" + idIncrement++ );
 			this.generatedId = true;
 		}
-		
+
 		if ( !this.element.attr( "role" ) ) {
 			// TODO alternatives to tooltip are dialog and menu, all three aren't generic popups
 			this.element.attr( "role", "dialog" );
 			this.generatedRole = true;
 		}
-		
+
 		this.options.trigger
 			.attr( "aria-haspopup", true )
 			.attr( "aria-owns", this.element.attr( "id" ) );
-		
+
 		this.element
-			.addClass("ui-popup")
+			.addClass( "ui-popup" )
 		this.close();
 
 		this._bind(this.options.trigger, {
 			keydown: function( event ) {
-				// prevent space-to-open to scroll the page, only hapens for anchor ui.button
-				if ( this.options.trigger.is( "a:ui-button" ) && event.keyCode == $.ui.keyCode.SPACE) {
-					event.preventDefault()
+				// prevent space-to-open to scroll the page, only happens for anchor ui.button
+				if ( this.options.trigger.is( "a:ui-button" ) && event.keyCode == $.ui.keyCode.SPACE ) {
+					event.preventDefault();
 				}
 				// TODO handle SPACE to open popup? only when not handled by ui.button
-				if ( event.keyCode == $.ui.keyCode.SPACE && this.options.trigger.is("a:not(:ui-button)") ) {
+				if ( event.keyCode == $.ui.keyCode.SPACE && this.options.trigger.is( "a:not(:ui-button)" ) ) {
 					this.options.trigger.trigger( "click", event );
 				}
 				// translate keydown to click
@@ -78,8 +78,29 @@ $.widget( "ui.popup", {
 				}, 1);
 			}
 		});
-		
-		this._bind(this.element, {
+
+		if ( !this.element.is( ":ui-menu" ) ) {
+			//default use case, wrap tab order in popup
+			this.element.bind( "keydown.ui-popup", function( event ) {
+				if ( event.keyCode !== $.ui.keyCode.TAB ) {
+					return;
+				}
+
+				var tabbables = $( ":tabbable", this ),
+					first = tabbables.first(),
+					last  = tabbables.last();
+
+				if ( event.target === last[ 0 ] && !event.shiftKey ) {
+					first.focus( 1 );
+					return false;
+				} else if ( event.target === first[ 0 ] && event.shiftKey ) {
+					last.focus( 1 );
+					return false;
+				}
+			});
+		}
+
+		this._bind({
 			focusout: function( event ) {
 				var that = this;
 				// use a timer to allow click to clear it and letting that
@@ -89,9 +110,8 @@ $.widget( "ui.popup", {
 				}, 100);
 			},
 			focusin: function( event ) {
-				var that = this;
-				clearTimeout( that.closeTimer );
-			}		
+				clearTimeout( this.closeTimer );
+			}
 		});
 
 		this._bind({
@@ -99,34 +119,35 @@ $.widget( "ui.popup", {
 			// bind to document instead?
 			// either element itself or a child should be focusable
 			keyup: function( event ) {
-				if (event.keyCode == $.ui.keyCode.ESCAPE && this.element.is( ":visible" )) {
+				if ( event.keyCode == $.ui.keyCode.ESCAPE && this.element.is( ":visible" ) ) {
 					this.close( event );
 					// TODO move this to close()? would allow menu.select to call popup.close, and get focus back to trigger
 					this.options.trigger.focus();
 				}
 			}
 		});
-		
+
 		this._bind(document, {
 			click: function( event ) {
-				if (this.isOpen && !$(event.target).closest(".ui-popup").length) {
+				if ( this.isOpen && !$(event.target).closest(".ui-popup").length ) {
 					this.close( event );
 				}
 			}
 		})
 	},
-	
+
 	_destroy: function() {
 		this.element
 			.show()
 			.removeClass( "ui-popup" )
 			.removeAttr( "aria-hidden" )
-			.removeAttr( "aria-expanded" );
+			.removeAttr( "aria-expanded" )
+			.unbind( "keypress.ui-popup");
 
 		this.options.trigger
 			.removeAttr( "aria-haspopup" )
 			.removeAttr( "aria-owns" );
-			
+
 		if ( this.generatedId ) {
 			this.element.removeAttr( "id" );
 		}
@@ -134,7 +155,7 @@ $.widget( "ui.popup", {
 			this.element.removeAttr( "role" );
 		}
 	},
-	
+
 	open: function( event ) {
 		var position = $.extend( {}, {
 			of: this.options.trigger
@@ -144,46 +165,26 @@ $.widget( "ui.popup", {
 			.show()
 			.attr( "aria-hidden", false )
 			.attr( "aria-expanded", true )
-			.position( position );		
-		
-		if (this.element.is(":ui-menu")) { //popup is a menu
-			this.element.focus();
-			this.element.menu("focus", event, this.element.children( "li" ).first() );
+			.position( position );
+
+		if (this.element.is( ":ui-menu" )) { //popup is a menu
+			this.element.menu( "focus", event, this.element.children( "li" ).first() );
 			this.element.focus();
-		}
-		// TODO add other special use cases that differ from the default dialog style keyboard mechanism
-		else {
-			//default use case, popup could be anything (e.g. a form)
-			this.element
-				.bind( "keydown.ui-popup", function( event ) {
-	                if ( event.keyCode !== $.ui.keyCode.TAB ) {
-	                    return;
-	                }
-	                var tabbables = $( ":tabbable", this ),
-	                    first = tabbables.filter( ":first" ),
-	                    last  = tabbables.filter( ":last" );
-	                if ( event.target === last[0] && !event.shiftKey ) {
-	                    first.focus( 1 );
-	                    return false;
-	                } else if ( event.target === first[0] && event.shiftKey ) {
-	                    last.focus( 1 );
-	                    return false;
-	                }
-	            });
-			
+		} else {
 			// set focus to the first tabbable element in the popup container
-	        // if there are no tabbable elements, set focus on the popup itself
-	        var tabbables = this.element.find( ":tabbable" );
-	        if (!tabbables.length) {
-	        	this.element.attr("tabindex", "0");
-	        	tabbables.add(this.element);
-	        }
-	        tabbables.eq( 0 ).focus(1);
+			// if there are no tabbable elements, set focus on the popup itself
+			var tabbables = this.element.find( ":tabbable" );
+			if ( !tabbables.length ) {
+				if ( !this.element.is(":tabbable") ) {
+					this.element.attr("tabindex", "0");
+				}
+				tabbables.add(this.element);
+			}
+			tabbables.first().focus( 1 );
 		}
-		
-		// take trigger out of tab order to allow shift-tab to skip trigger
-		this.options.trigger.attr("tabindex", -1);
 
+		// take trigger out of tab order to allow shift-tab to skip trigger
+		this.options.trigger.attr( "tabindex", -1 );
 		this.isOpen = true;
 		this._trigger( "open", event );
 	},
@@ -192,16 +193,13 @@ $.widget( "ui.popup", {
 		this.element
 			.hide()
 			.attr( "aria-hidden", true )
-			.attr( "aria-expanded", false )
-			.unbind( "keypress.ui-popup");
+			.attr( "aria-expanded", false );
 
 		this.options.trigger.attr("tabindex", 0);
 
 		this.isOpen = false;
 		this._trigger( "close", event );
 	}
-	
-	
 });
 
 }(jQuery));