Skip to content
This repository was archived by the owner on Oct 8, 2021. It is now read-only.

Commit dca0976

Browse files
author
Gabriel Schulhof
committed
Checkboxradio: Correctly retrieve label
This moves label-finding code into one function to help render the code unit-testable. The new code also relies more on native means of getting the label, which improves performance a great deal when the .labels property is supported, and also improves performance when such a property is not supported (such as in FF27). http://jsperf.com/checkboxradio-label-retrieval-non-nested/5 (cherry picked from commit d9f5d21) Closes gh-7293 Fixes gh-7292
1 parent 73fd265 commit dca0976

File tree

3 files changed

+128
-14
lines changed

3 files changed

+128
-14
lines changed

‎js/widgets/forms/checkboxradio.js

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,7 @@ $.widget( "mobile.checkboxradio", $.extend( {
3939
return input.jqmData( dataAttr ) ||
4040
input.closest( "form, fieldset" ).jqmData( dataAttr );
4141
},
42-
// NOTE: Windows Phone could not find the label through a selector
43-
// filter works though.
44-
parentLabel = input.closest( "label" ),
45-
label = parentLabel.length ? parentLabel :
46-
input
47-
.closest( "form, fieldset, :jqmData(role='page'), :jqmData(role='dialog')" )
48-
.find( "label" )
49-
.filter( "[for='" + escapeId( input[0].id ) + "']" )
50-
.first(),
42+
label = this._findLabel(),
5143
inputtype = input[0].type,
5244
checkedClass = "ui-" + inputtype + "-on",
5345
uncheckedClass = "ui-" + inputtype + "-off";
@@ -60,16 +52,17 @@ $.widget( "mobile.checkboxradio", $.extend( {
6052
this.options.disabled = true;
6153
}
6254

63-
o.iconpos = inheritAttr( input, "iconpos" ) || label.attr( "data-" + $.mobile.ns + "iconpos" ) || o.iconpos,
55+
o.iconpos = inheritAttr( input, "iconpos" ) ||
56+
label.element.attr( "data-" + $.mobile.ns + "iconpos" ) || o.iconpos,
6457

6558
// Establish options
6659
o.mini = inheritAttr( input, "mini" ) || o.mini;
6760

6861
// Expose for other methods
6962
$.extend( this, {
7063
input: input,
71-
label: label,
72-
parentLabel: parentLabel,
64+
label: label.element,
65+
labelIsParent: label.isParent,
7366
inputtype: inputtype,
7467
checkedClass: checkedClass,
7568
uncheckedClass: uncheckedClass
@@ -79,7 +72,7 @@ $.widget( "mobile.checkboxradio", $.extend( {
7972
this._enhance();
8073
}
8174

82-
this._on( label, {
75+
this._on( label.element, {
8376
vmouseover: "_handleLabelVMouseOver",
8477
vclick: "_handleLabelVClick"
8578
});
@@ -95,10 +88,36 @@ $.widget( "mobile.checkboxradio", $.extend( {
9588
this.refresh();
9689
},
9790

91+
_findLabel: function() {
92+
var parentLabel, label, isParent,
93+
input = this.element,
94+
labelsList = input[ 0 ].labels;
95+
96+
if( labelsList && labelsList.length > 0 ) {
97+
label = $( labelsList[ 0 ] );
98+
isParent = $.contains( label[ 0 ], input[ 0 ] );
99+
} else {
100+
parentLabel = input.closest( "label" );
101+
isParent = ( parentLabel.length > 0 );
102+
103+
// NOTE: Windows Phone could not find the label through a selector
104+
// filter works though.
105+
label = isParent ? parentLabel :
106+
$( this.document[ 0 ].getElementsByTagName( "label" ) )
107+
.filter( "[for='" + escapeId( input[ 0 ].id ) + "']" )
108+
.first();
109+
}
110+
111+
return {
112+
element: label,
113+
isParent: isParent
114+
};
115+
},
116+
98117
_enhance: function() {
99118
this.label.addClass( "ui-btn ui-corner-all");
100119

101-
if ( this.parentLabel.length > 0 ) {
120+
if ( this.labelIsParent ) {
102121
this.input.add( this.label ).wrapAll( this._wrapper() );
103122
} else {
104123
//this.element.replaceWith( this.input.add( this.label ).wrapAll( this._wrapper() ) );

‎tests/unit/checkboxradio/find-label-tests.html

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>jQuery Mobile Checkboxradio Input Set Test Suite</title>
6+
7+
<script src="../../../external/requirejs/require.js"></script>
8+
<script src="../../../js/requirejs.config.js"></script>
9+
<script src="../../../js/jquery.tag.inserter.js"></script>
10+
<script src="../../../tests/jquery.testHelper.js"></script>
11+
<script src="../../../external/qunit/qunit.js"></script>
12+
<script>
13+
$.testHelper.asyncLoad([
14+
[
15+
"widgets/forms/checkboxradio",
16+
],
17+
[
18+
"find_label_tests_core.js"
19+
]
20+
]);
21+
</script>
22+
23+
<link rel="stylesheet" href="../../../external/qunit/qunit.css"/>
24+
<link rel="stylesheet" href="../../jqm-tests.css"/>
25+
26+
<script src="../../swarminject.js"></script>
27+
</head>
28+
<body>
29+
30+
<div id="qunit"></div>
31+
32+
<label id="separate-label-outside-form-label" for="separate-label-outside-form">Label</label>
33+
<form>
34+
<label id="separate-label-in-form-label" for="separate-label-in-form">Label</label>
35+
<input type="checkbox" id="separate-label-in-form">
36+
<input type="checkbox" id="separate-label-outside-form">
37+
<label id="nested-input-inside-form-label">Label<input type="checkbox" id="nested-input-inside-form"></label>
38+
</form>
39+
40+
<label id="separate-label-input-outside-form-label" for="separate-label-input-outside-form">Label</label>
41+
<input type="checkbox" id="separate-label-input-outside-form">
42+
<label id="nested-input-outside-form-label">Label<input type="checkbox" id="nested-input-outside-form"></label>
43+
</body>
44+
</html>

‎tests/unit/checkboxradio/find_label_tests_core.js

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
var pairs = [
2+
{
3+
label: "#separate-label-outside-form-label",
4+
input: "#separate-label-outside-form",
5+
isParent: false
6+
},
7+
{
8+
label: "#separate-label-in-form-label",
9+
input: "#separate-label-in-form",
10+
isParent: false
11+
},
12+
{
13+
label: "#nested-input-inside-form-label",
14+
input: "#nested-input-inside-form",
15+
isParent: true
16+
},
17+
{
18+
label: "#separate-label-input-outside-form-label",
19+
input: "#separate-label-input-outside-form",
20+
isParent: false
21+
},
22+
{
23+
label: "#nested-input-outside-form-label",
24+
input: "#nested-input-outside-form",
25+
isParent: true
26+
}
27+
];
28+
29+
test( "_findLabel() works correctly", function() {
30+
var index, pair, result,
31+
findLabel = $.mobile.checkboxradio.prototype._findLabel;
32+
33+
for ( index in pairs ) {
34+
pair = $.extend( {}, pairs[ index ] );
35+
pair.label = $( pair.label );
36+
pair.input = $( pair.input );
37+
38+
result = findLabel.call({
39+
element: pair.input,
40+
document: $( document )
41+
});
42+
43+
deepEqual( result.element.length > 0, true,
44+
pair.input.attr( "id" ) + ": a label was found" );
45+
deepEqual( result.element[ 0 ], pair.label[ 0 ],
46+
pair.input.attr( "id" ) + ": the right label was found" );
47+
deepEqual( result.isParent, pair.isParent,
48+
pair.input.attr( "id" ) +
49+
": the label was correctly identified as (not?) the parent" );
50+
}
51+
});

0 commit comments

Comments
 (0)