Controlgroup: Address review comments

diff --git a/demos/controlgroup/toolbar.html b/demos/controlgroup/toolbar.html
index 8afd38b234cca98951c1eb1adba5f707df0deb56..d68d834b5a71261a470ac0425bbd1f758b99c54d 100644 (file)
--- a/demos/controlgroup/toolbar.html
+++ b/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">
-                               <option value="black" selected>Color</option>
+                       <select id="forecolor" title="Color">
+                               <option selected disabled>Color</option>
                                <option value="black">Black</option>
                                <option value="white">White</option>
                                <option value="red">Red</option>

diff --git a/themes/base/controlgroup.css b/themes/base/controlgroup.css
index affae98ee2052b2eacc5ba5eaa3bd4bdb2976997..481459102582750fbe1be4340552cd6c44a78824 100644 (file)
--- a/themes/base/controlgroup.css
+++ b/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 );
 }

diff --git a/ui/widgets/controlgroup.js b/ui/widgets/controlgroup.js
index 619023309d6ee62f1d8fc74e20561e32ed9cff4a..4fc348e28594cb20a6ba1ed09954ac4450d99137 100644 (file)
--- a/ui/widgets/controlgroup.js
+++ b/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",
-                       "controlgroupLabel": ".ui-controlgroup-label"
-               },
-               direction: "horizontal",
-               excludeInvisible: true
+                       "spinner": ".ui-spinner-input"
+               }
        },
 
        _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 );
+
+                                       // Store an instance of the controlgroup to be able to refrence
+                                       var widgetElement = element[ widget ]( "widget" );
 
-                                       // Set data on the widget element pointing to the this.element of the widget
-                                       // and telling us what type of widget this is
-                                       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",
-                       remove = "ui-corner-top ui-corner-bottom ui-corner-left ui-corner-right",
-                       add = this._buildSimpleOptions( position, direction, "label" ).classes.label;
+               var remove = "ui-corner-top ui-corner-bottom ui-corner-left ui-corner-right";
+               var add = this._buildSimpleOptions( position, "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 ) {
-               var options = this._buildSimpleOptions( position, direction, "ui-spinner" );
+       _spinnerOptions: function( position ) {
+               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 ) {
-               return this._buildSimpleOptions( position, direction, "ui-button" );
+       _buttonOptions: function( position ) {
+               return this._buildSimpleOptions( position, "ui-button" );
        },
 
-       _checkboxradioOptions: function( position, direction ) {
-               return this._buildSimpleOptions( position, direction, "ui-checkboxradio-label" );
+       _checkboxradioOptions: function( position ) {
+               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-corner-" + ( direction ? "top" : "tl" ),
-                                       "ui-selectmenu-button-closed":
-                                       "ui-corner-" + ( direction ? "top" : "left" )
+                                       "ui-selectmenu-button-open": "ui-corner-" + ( direction ? "top" : "tl" ),
+                                       "ui-selectmenu-button-closed": "ui-corner-" + ( direction ? "top" : "left" )
                                },
                                last: {
-                                       "ui-selectmenu-button-open":
-                                       direction ? null : "ui-corner-tr",
-                                       "ui-selectmenu-button-closed":
-                                       "ui-corner-" + ( direction ? "bottom" : "right" )
+                                       "ui-selectmenu-button-open": direction ? null : "ui-corner-tr",
+                                       "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" ](
-                                                       value,
-                                                       that.options.direction === "vertical"
-                                               )
+                                               that[ "_" + instance.widgetName + "Options" ]( value )
                                        );
                                } else {
                                        that._updateCornerClass( children[ value ](), value );