Controlgroup: Address review comments

demos/controlgroup/toolbar.html

@@ -73,15 +73,16 @@
 		$( "#zoom" ).on( "selectmenuchange", function() {
 			page.css({ "zoom": $( this ).val() });
 		})
-		$( basicControls.concat( valueControls ).join( ", " ) ).on( "click change selectmenuchange", function() {
+		$( basicControls.concat( valueControls ).join( ", " ) ).on( "click change selectmenuchange",
+			function() {
 				document.execCommand(
 					this.id,
 					false,
 					$( this ).val()
 				);
-		});
+			} );
 		$( "form" ).on( "submit", function( event ) {
-			return false;
+			event.preventDefault();
 		});
 	</script>
 </head>
@@ -92,7 +93,7 @@
 			<button id="undo">Undo</button>
 			<button id="redo">Redo</button>
 			<select id="zoom">
-				<option value="100%" selected>Zoom</option>
+				<option selected disabled>Zoom</option>
 				<option>50%</option>
 				<option>75%</option>
 				<option>90%</option>
@@ -102,7 +103,7 @@
 				<option>200%</option>
 			</select>
 			<select id="fontname">
-				<option value="Lucida Grande" selected>Font</option>
+				<option selected disabled>Font</option>
 				<option>Arial</option>
 				<option>Comic Sans MS</option>
 				<option>Courier New</option>
@@ -113,7 +114,7 @@
 				<option>Verdana</option>
 			</select>
 			<select id="fontsize">
-				<option value="3" selected>Size</option>
+				<option selected disabled>Size</option>
 				<option value="1">8px</option>
 				<option value="2">9px</option>
 				<option value="3">10px</option>
@@ -126,7 +127,7 @@
 				<option value="10">36px</option>
 			</select>
 			<select id="hilitecolor" title="Background color">
-				<option value="white">Highlight</option>
+				<option selected disabled>Highlight</option>
 				<option value="white">None</option>
 				<option value="red">Red</option>
 				<option value="yellow">Yellow</option>
@@ -136,8 +137,8 @@
 				<option value="purple">Purple</option>
 				<option value="orange">Orange</option>
 			</select>
-			<select id="forecolor">
+			<select id="forecolor" title="Color">
-				<option value="black" selected>Color</option>
+				<option selected disabled>Color</option>
 				<option value="black">Black</option>
 				<option value="white">White</option>
 				<option value="red">Red</option>

themes/base/controlgroup.css

@@ -2,7 +2,7 @@
  * jQuery UI Controlgroup @VERSION
  * http://jqueryui.com
  *
- * Copyright 2013 jQuery Foundation and other contributors
+ * Copyright jQuery Foundation and other contributors
  * Released under the MIT license.
  * http://jquery.org/license
  *
@@ -60,7 +60,7 @@
 	margin: .4em;
 	padding: 0;
 
-	/* Support IE8, Android < 4.4 */
+	/* Support: IE8 only, Android < 4.4 only */
 	width: 85%;
 	width: calc( 100% - 22px );
 }

ui/widgets/controlgroup.js

@@ -34,16 +34,16 @@ return $.widget( "ui.controlgroup", {
 	version: "@VERSION",
 	defaultElement: "<div>",
 	options: {
+		direction: "horizontal",
 		disabled: null,
+		excludeInvisible: true,
 		items: {
 			"button": "input[type=button], input[type=submit], input[type=reset], button, a",
+			"controlgroupLabel": ".ui-controlgroup-label",
 			"checkboxradio": "input[type='checkbox'], input[type='radio']",
 			"selectmenu": "select",
-			"spinner": ".ui-spinner-input",
+			"spinner": ".ui-spinner-input"
-			"controlgroupLabel": ".ui-controlgroup-label"
+		}
-		},
-		direction: "horizontal",
-		excludeInvisible: true
 	},
 
 	_create: function() {
@@ -80,15 +80,16 @@ return $.widget( "ui.controlgroup", {
 			// Make sure the widget actually exists and has a selector set
 			if ( $.fn[ widget ] && selector ) {
 
-				// Find instances of this widget inside controlgroup and set options
+				// Find instances of this widget inside controlgroup init them
 				widgets = that.element.find( selector )[ widget ]( options );
 
 				widgets.each( function() {
-					var element = $( this ),
+					var element = $( this );
 
-					// Set data on the widget element pointing to the this.element of the widget
+					// Store an instance of the controlgroup to be able to refrence
-					// and telling us what type of widget this is
+					var widgetElement = element[ widget ]( "widget" );
-					widgetElement = element[ widget ]( "widget" ).data(
+
+					widgetElement.data(
 						"ui-controlgroup-data",
 						element.data( "ui-" + widget.charAt( 0 ).toUpperCase() + widget.slice( 1 ) )
 					);
@@ -101,7 +102,7 @@ return $.widget( "ui.controlgroup", {
 					$( this ).contents().wrapAll( "<span class='ui-controlgroup-label-contents'></span>" );
 				} );
 				that._addClass( labels, null, "ui-widget ui-widget-content ui-state-default" );
-				Array.prototype.push.apply( childWidgets, labels.get() );
+				childWidgets = childWidgets.concat( labels.get() );
 			}
 		} );
 
@@ -120,15 +121,15 @@ return $.widget( "ui.controlgroup", {
 	},
 
 	_updateCornerClass: function( element, position ) {
-		var direction = this.options.direction === "vertical",
+		var remove = "ui-corner-top ui-corner-bottom ui-corner-left ui-corner-right";
-			remove = "ui-corner-top ui-corner-bottom ui-corner-left ui-corner-right",
+		var add = this._buildSimpleOptions( position, "label" ).classes.label;
-			add = this._buildSimpleOptions( position, direction, "label" ).classes.label;
 
 		this._removeClass( element, null, remove );
 		this._addClass( element, null, add );
 	},
 
-	_buildSimpleOptions: function( position, direction, key ) {
+	_buildSimpleOptions: function( position, key ) {
+		var direction = this.options.direction === "vertical";
 		var result = {
 			classes: {}
 		};
@@ -141,8 +142,8 @@ return $.widget( "ui.controlgroup", {
 		return result;
 	},
 
-	_spinnerOptions: function( position, direction ) {
+	_spinnerOptions: function( position ) {
-		var options = this._buildSimpleOptions( position, direction, "ui-spinner" );
+		var options = this._buildSimpleOptions( position, "ui-spinner" );
 
 		options.classes[ "ui-spinner-up" ] = "";
 		options.classes[ "ui-spinner-down" ] = "";
@@ -150,15 +151,16 @@ return $.widget( "ui.controlgroup", {
 		return options;
 	},
 
-	_buttonOptions: function( position, direction ) {
+	_buttonOptions: function( position ) {
-		return this._buildSimpleOptions( position, direction, "ui-button" );
+		return this._buildSimpleOptions( position, "ui-button" );
 	},
 
-	_checkboxradioOptions: function( position, direction ) {
+	_checkboxradioOptions: function( position ) {
-		return this._buildSimpleOptions( position, direction, "ui-checkboxradio-label" );
+		return this._buildSimpleOptions( position, "ui-checkboxradio-label" );
 	},
 
-	_selectmenuOptions: function( position, direction ) {
+	_selectmenuOptions: function( position ) {
+		var direction = this.options.direction === "vertical";
 		return {
 			width: direction ? "auto" : false,
 			classes: {
@@ -167,16 +169,12 @@ return $.widget( "ui.controlgroup", {
 					"ui-selectmenu-button-closed": null
 				},
 				first: {
-					"ui-selectmenu-button-open":
+					"ui-selectmenu-button-open": "ui-corner-" + ( direction ? "top" : "tl" ),
-					"ui-corner-" + ( direction ? "top" : "tl" ),
+					"ui-selectmenu-button-closed": "ui-corner-" + ( direction ? "top" : "left" )
-					"ui-selectmenu-button-closed":
-					"ui-corner-" + ( direction ? "top" : "left" )
 				},
 				last: {
-					"ui-selectmenu-button-open":
+					"ui-selectmenu-button-open": direction ? null : "ui-corner-tr",
-					direction ? null : "ui-corner-tr",
+					"ui-selectmenu-button-closed": "ui-corner-" + ( direction ? "bottom" : "right" )
-					"ui-selectmenu-button-closed":
-					"ui-corner-" + ( direction ? "bottom" : "right" )
 				}
 
 			}[ position ]
@@ -224,10 +222,7 @@ return $.widget( "ui.controlgroup", {
 
 				if ( instance && that[ "_" + instance.widgetName + "Options" ] ) {
 					instance.element[ instance.widgetName ](
-						that[ "_" + instance.widgetName + "Options" ](
+						that[ "_" + instance.widgetName + "Options" ]( value )
-							value,
-							that.options.direction === "vertical"
-						)
 					);
 				} else {
 					that._updateCornerClass( children[ value ](), value );