Skip to content

Commit 344ce3c

Browse files
arschmitzarschmitz
committed
Checkboxradio: Address review comments
1 parent 197061b commit 344ce3c

File tree

6 files changed

+55
-42
lines changed

6 files changed

+55
-42
lines changed

‎demos/checkboxradio/default.html

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
<link rel="stylesheet" href="../demos.css">
88
<script src="../../external/requirejs/require.js"></script>
99
<script src="../bootstrap.js">
10-
$( "input" ).checkboxradio({
11-
label: "foo"
12-
});
10+
$( "input" ).checkboxradio();
1311
</script>
1412
</head>
1513
<body>

‎demos/checkboxradio/no-icons.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,4 @@ <h2>Checkbox nested in label</h2>
6262
<p>Examples of the markup that can be used with checkboxes and radio buttons, here showing both without icons.</p>
6363
</div>
6464
</body>
65-
</html>
65+
</html>

‎demos/checkboxradio/product-selector.html

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,28 +7,45 @@
77
<link rel="stylesheet" href="../demos.css">
88
<script src="../../external/requirejs/require.js"></script>
99
<script src="../bootstrap.js" data-modules="controlgroup">
10-
$( "input" ).checkboxradio();
11-
$( "[name='shape']").on( "change", function(){
12-
$( ".shape" ).removeClass( "circle pill square rectangle" )
13-
.addClass( $( this ).val() );
14-
});
15-
$( ".toggle" ).on( "change", function(){
16-
if ( $( this ).is( ".brand-toggle" ) ) {
17-
var checked = $( this ).is( ":checked" ),
18-
value = $( "[name='brand']" ).filter( ":checked" ).attr( "data-" + this.id )
19-
$( ".shape" ).css( this.id, checked ? value : "" );
10+
function handleShape( e ) {
11+
$( ".shape" )
12+
.removeClass( "circle pill square rectangle" )
13+
.addClass( $( e.target ).val() );
14+
};
15+
function handleToggle( e ) {
16+
var target = $( e.target );
17+
18+
if ( target.is( ".brand-toggle" ) ) {
19+
var checked = target.is( ":checked" ),
20+
value = $( "[name='brand']" )
21+
.filter( ":checked" )
22+
.attr( "data-" + target[ 0 ].id )
23+
$( ".shape" ).css( target[ 0 ].id, checked ? value : "" );
2024
} else {
21-
$( ".shape" ).toggleClass( this.id, $( this ).is( ":checked") );
25+
$( ".shape" ).toggleClass( target[ 0 ].id, target.is( ":checked") );
2226
}
23-
});
24-
$( "[name='brand']").on( "change", function(){
25-
$( "input" ).filter( ":checked" ).not( this ).trigger( "change" );
26-
});
27-
$( "input" ).filter( ":checked" ).trigger( "change" );
27+
}
28+
function updateBrand() {
29+
handleShape( { target: $( "[name='shape']:checked" ) } );
30+
$( ".toggle:checked" ).each( function() {
31+
handleToggle( { target: $( this ) } );
32+
} );
33+
}
34+
35+
// Initalize widgets
36+
$( "input" ).checkboxradio();
2837
$( ".shape-bar, .brand" ).controlgroup();
29-
$( ".toggles" ).controlgroup({
38+
$( ".toggles" ).controlgroup( {
3039
direction: "vertical"
31-
});
40+
} );
41+
42+
// Bind event handlers
43+
$( "[name='shape']").on( "change", handleShape );
44+
$( ".toggle" ).on( "change", handleToggle );
45+
$( "[name='brand']").on( "change", updateBrand );
46+
47+
// Set initial values
48+
updateBrand();
3249
</script>
3350
<style>
3451
.shape {
@@ -74,15 +91,15 @@
7491
<h3>1.) Select a brand</h3>
7592
<div class="brand">
7693
<label for="brand-jquery">jQuery</label>
77-
<input data-background-color="#0769AD" data-color="#7ACEF4" data-background-image="url(images/jquery.png)" type="radio" name="brand" id="brand-jquery">
94+
<input type="radio" name="brand" id="brand-jquery" data-background-color="#0769AD" data-color="#7ACEF4" data-background-image="url(images/jquery.png)">
7895
<label for="brand-jquery-ui">jQuery UI</label>
79-
<input data-background-color="#B24926" data-color="#FAA523" data-background-image="url(images/jquery-ui.png)" type="radio" name="brand" id="brand-jquery-ui" checked>
96+
<input type="radio" name="brand" id="brand-jquery-ui" data-background-color="#B24926" data-color="#FAA523" data-background-image="url(images/jquery-ui.png)" checked>
8097
<label for="brand-jquery-mobile">jQuery Mobile</label>
81-
<input data-background-color="#108040" data-color="#3EB249" data-background-image="url(images/jquery-mobile.png)" type="radio" name="brand" id="brand-jquery-mobile">
98+
<input type="radio" name="brand" id="brand-jquery-mobile" data-background-color="#108040" data-color="#3EB249" data-background-image="url(images/jquery-mobile.png)">
8299
<label for="brand-sizzle">Sizzle</label>
83-
<input data-background-color="#9A1B1E" data-color="#FAA523" data-background-image="url(images/sizzle.png)" type="radio" name="brand" id="brand-sizzle">
100+
<input type="radio" name="brand" id="brand-sizzle" data-background-color="#9A1B1E" data-color="#FAA523" data-background-image="url(images/sizzle.png)">
84101
<label for="brand-qunit">QUnit</label>
85-
<input data-background-color="#390F39" data-color="#9C3493" data-background-image="url(images/qunit.png)" type="radio" name="brand" id="brand-qunit">
102+
<input type="radio" name="brand" id="brand-qunit" data-background-color="#390F39" data-color="#9C3493" data-background-image="url(images/qunit.png)">
86103
</div>
87104
</div>
88105
<div class="shape-wrap">

‎tests/visual/checkboxradio/checkboxradio.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
</head>
4040
<body>
4141
<h2>
42-
Easy way to toggle through various combinations of options and states to make sure non lead to
42+
Easy way to toggle through various combinations of options and states to make sure none lead to
4343
a broken appearence.
4444
</h2>
4545
<div class="controls">

‎themes/base/checkboxradio.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* jQuery UI Checkboxradio @VERSION
33
* http://jqueryui.com
44
*
5-
* Copyright 2013 jQuery Foundation and other contributors
5+
* Copyright jQuery Foundation and other contributors
66
* Released under the MIT license.
77
* http://jquery.org/license
88
*

‎ui/widgets/checkboxradio.js

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,6 @@ $.widget( "ui.checkboxradio", [ $.ui.formResetMixin, {
5757

5858
labels = this.element.labels();
5959

60-
// Todo: For now we will use the last label we need to check about the best
61-
// way to handle multiple labels with some accessability experts
6260
this.label = $( labels[ labels.length - 1 ] );
6361
if ( !this.label.length ) {
6462
$.error( "No label found for checkboxradio widget" );
@@ -70,8 +68,8 @@ $.widget( "ui.checkboxradio", [ $.ui.formResetMixin, {
7068
// input itself.
7169
this.label.contents().not( this.element ).each( function() {
7270

73-
// The label contents could be text html or a mix we concat each element to get a string
74-
// representation of the label without the input as part of it.
71+
// The label contents could be text, html, or a mix. We concat each element to get a string
72+
// representation of the label, without the input as part of it.
7573
that.originalLabel += this.nodeType === 3 ? $( this ).text() : this.outerHTML;
7674
} );
7775

@@ -96,7 +94,7 @@ $.widget( "ui.checkboxradio", [ $.ui.formResetMixin, {
9694
this.formParent = this.form.length ? this.form : $( "body" );
9795

9896
if ( this.options.disabled == null ) {
99-
this.options.disabled = this.element[ 0 ].disabled || false;
97+
this.options.disabled = this.element[ 0 ].disabled;
10098
}
10199

102100
this._setOption( "disabled", this.options.disabled );
@@ -121,11 +119,11 @@ $.widget( "ui.checkboxradio", [ $.ui.formResetMixin, {
121119
}
122120

123121
this._on( {
124-
"change": "_toggleClasses",
125-
"focus": function() {
122+
change: "_toggleClasses",
123+
focus: function() {
126124
this._addClass( this.label, null, "ui-state-focus ui-visual-focus" );
127125
},
128-
"blur": function() {
126+
blur: function() {
129127
this._removeClass( this.label, null, "ui-state-focus ui-visual-focus" );
130128
}
131129
} );
@@ -150,15 +148,15 @@ $.widget( "ui.checkboxradio", [ $.ui.formResetMixin, {
150148
},
151149

152150
_getRadioGroup: function() {
153-
var name = this.element[ 0 ].name,
154-
that = this,
155-
radios = $( [] );
151+
var name = this.element[ 0 ].name;
152+
var formParent = this.formParent[ 0 ];
153+
var radios = $( [] );
156154

157155
if ( name ) {
158156
name = $.ui.escapeSelector( name );
159157
radios = this.formParent.find( "[name='" + $.ui.escapeSelector( name ) + "']" ).filter( function() {
160158
var form = $( this ).form();
161-
return ( form.length ? form : $( "body" ) )[ 0 ] === that.formParent[ 0 ];
159+
return ( form.length ? form : $( "body" ) )[ 0 ] === formParent;
162160
} );
163161
}
164162
return radios.not( this.element );
@@ -198,7 +196,7 @@ $.widget( "ui.checkboxradio", [ $.ui.formResetMixin, {
198196

199197
_setOption: function( key, value ) {
200198

201-
// We don't alow the value to be set to nothing
199+
// We don't allow the value to be set to nothing
202200
if ( key === "label" && !value ) {
203201
return;
204202
}

0 commit comments

Comments
 (0)